Fix: Send the PKCE code verifier instead of the challenge in the OAuth2 token request

session/oauth2.go

@@ -156,7 +156,7 @@ func (o *OAuth2) handshake(ctx context.Context) error {
 		return fmt.Errorf("failed to get oauth2 server info: %w", err)
 	}
 
-	codeChallenge, err := o.generateCodeChallenge()
+	codeVerifier, codeChallenge, err := generatePKCE()
 	if err != nil {
 		return fmt.Errorf("failed to generate code challenge: %w", err)
 	}
@@ -166,7 +166,7 @@ func (o *OAuth2) handshake(ctx context.Context) error {
 		return fmt.Errorf("failed to retrieve authorization code: %w", err)
 	}
 
-	if err := o.retrieveToken(ctx, serverInfo, redirectURL, codeChallenge, code); err != nil {
+	if err := o.retrieveToken(ctx, serverInfo, redirectURL, codeVerifier, code); err != nil {
 		return fmt.Errorf("failed to retrieve access token: %w", err)
 	}
 
@@ -281,7 +281,7 @@ func (o *OAuth2) retrieveCode(
 func (o *OAuth2) retrieveToken(
 	ctx context.Context,
 	serverInfo *oauth2ServerInfo,
-	redirectURL, codeChallenge, code string,
+	redirectURL, codeVerifier, code string,
 ) error {
 	if !slices.Contains(serverInfo.TokenEndpointAuthMethodsSupported, "client_secret_post") {
 		return fmt.Errorf("unsupported token endpoint authentication methods: %v",
@@ -294,7 +294,7 @@ func (o *OAuth2) retrieveToken(
 	form.Add("client_id", o.clientID)
 	form.Add("client_secret", o.clientSecret)
 	form.Add("redirect_uri", redirectURL)
-	form.Add("code_verifier", codeChallenge)
+	form.Add("code_verifier", codeVerifier)
 
 	body := bytes.NewBufferString(form.Encode())
 
@@ -382,13 +382,19 @@ func (o *OAuth2) serverInfo(ctx context.Context) (*oauth2ServerInfo, error) {
 	return &info, nil
 }
 
-func (o *OAuth2) generateCodeChallenge() (string, error) {
-	verifier := make([]byte, 32)
-	if _, err := rand.Read(verifier); err != nil {
-		return "", fmt.Errorf("failed to generate random bytes: %w", err)
-	}
-	challenge := sha256.Sum256(verifier)
-	return base64.RawURLEncoding.EncodeToString(challenge[:]), nil
+// generatePKCE returns a PKCE code verifier and its S256 code challenge, as
+// defined by RFC 7636. The challenge is sent with the authorization request and
+// the verifier, which must stay secret until then, with the token request.
+//
+// https://datatracker.ietf.org/doc/html/rfc7636#section-4.1
+func generatePKCE() (verifier, challenge string, err error) {
+	random := make([]byte, 32)
+	if _, err := rand.Read(random); err != nil {
+		return "", "", fmt.Errorf("failed to generate random bytes: %w", err)
+	}
+	verifier = base64.RawURLEncoding.EncodeToString(random)
+	sum := sha256.Sum256([]byte(verifier))
+	return verifier, base64.RawURLEncoding.EncodeToString(sum[:]), nil
 }
 
 type oauth2ServerInfo struct {

session/oauth2_test.go

@@ -0,0 +1,74 @@
+package session
+
+import (
+	"context"
+	"crypto/sha256"
+	"encoding/base64"
+	"net/http"
+	"net/http/httptest"
+	"regexp"
+	"testing"
+)
+
+func TestGeneratePKCE(t *testing.T) {
+	verifier, challenge, err := generatePKCE()
+	if err != nil {
+		t.Fatalf("generatePKCE: %v", err)
+	}
+
+	// RFC 7636 section 4.1: 43 to 128 characters from the unreserved set.
+	if !regexp.MustCompile(`^[A-Za-z0-9\-._~]{43,128}$`).MatchString(verifier) {
+		t.Errorf("verifier %q is not a valid RFC 7636 code verifier", verifier)
+	}
+
+	// RFC 7636 section 4.2: S256 challenge is BASE64URL(SHA256(verifier)).
+	sum := sha256.Sum256([]byte(verifier))
+	if want := base64.RawURLEncoding.EncodeToString(sum[:]); challenge != want {
+		t.Errorf("challenge = %q, want %q", challenge, want)
+	}
+
+	if verifier == challenge {
+		t.Error("verifier must not equal the challenge")
+	}
+
+	other, _, err := generatePKCE()
+	if err != nil {
+		t.Fatalf("generatePKCE: %v", err)
+	}
+	if other == verifier {
+		t.Error("consecutive verifiers must differ")
+	}
+}
+
+func TestRetrieveTokenSendsVerifier(t *testing.T) {
+	verifier, _, err := generatePKCE()
+	if err != nil {
+		t.Fatalf("generatePKCE: %v", err)
+	}
+
+	var gotVerifier string
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if err := r.ParseForm(); err != nil {
+			t.Errorf("parse form: %v", err)
+		}
+		gotVerifier = r.PostForm.Get("code_verifier")
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`{"access_token":"token","token_type":"Bearer",` +
+			`"installation":{"apiEndpoint":"https://example.teamwork.com"}}`))
+	}))
+	defer server.Close()
+
+	o := NewOAuth2("client", "secret", WithOAuth2Client(server.Client()))
+	info := &oauth2ServerInfo{
+		TokenEndpoint:                     server.URL,
+		TokenEndpointAuthMethodsSupported: []string{"client_secret_post"},
+	}
+	redirectURL := "http://127.0.0.1/oauth2/callback"
+	if err := o.retrieveToken(context.Background(), info, redirectURL, verifier, "code"); err != nil {
+		t.Fatalf("retrieveToken: %v", err)
+	}
+
+	if gotVerifier != verifier {
+		t.Errorf("code_verifier = %q, want %q", gotVerifier, verifier)
+	}
+}