Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion internal/api/jwks.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func (a *API) WellKnownOpenID(w http.ResponseWriter, r *http.Request) error {
SubjectTypesSupported: []string{"public"},
IDTokenSigningAlgValuesSupported: []string{"RS256", "HS256", "ES256"}, // TODO :: should create this based on signing key config?
TokenEndpointAuthMethodsSupported: []string{"client_secret_basic", "client_secret_post", "none"},
CodeChallengeMethodsSupported: []string{"S256", "plain"},
CodeChallengeMethodsSupported: []string{"S256"},
ScopesSupported: models.SupportedOAuthScopes,

// OIDC Standard Claims
Expand Down
13 changes: 8 additions & 5 deletions internal/api/oauthserver/authorize.go
Original file line number Diff line number Diff line change
Expand Up @@ -476,15 +476,18 @@ func (s *Server) validateRemainingAuthorizeParams(params *AuthorizeParams) error
}

func (s *Server) validatePKCEParams(codeChallengeMethod, codeChallenge string) error {
// PKCE is mandatory for the authorization code flow OAuth2.1
// Both code_challenge and code_challenge_method must be provided together
// PKCE is mandatory for the authorization code flow per OAuth 2.1.
// Both code_challenge and code_challenge_method must be provided together.
if codeChallenge == "" || codeChallengeMethod == "" {
return errors.New("PKCE flow requires both code_challenge and code_challenge_method")
}

// Validate code challenge method (case-insensitive)
if strings.ToLower(codeChallengeMethod) != "s256" && strings.ToLower(codeChallengeMethod) != "plain" {
return errors.New("code_challenge_method must be 'S256' or 'plain'")
// OAuth 2.1 (draft-ietf-oauth-v2-1-12, Section 4.1.1) permits only S256.
// The plain method is excluded: code_challenge is transmitted in the
// authorization request URL and may appear in server logs, browser history,
// or Referer headers — with plain, that directly exposes the code_verifier.
if strings.ToLower(codeChallengeMethod) != "s256" {
return errors.New("code_challenge_method must be 'S256'")
}

// Validate code challenge format and length (per OAuth2 spec)
Expand Down
83 changes: 83 additions & 0 deletions internal/api/oauthserver/authorize_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -666,3 +666,86 @@ func (ts *OAuthAuthorizeTestSuite) TestConsent_InvalidActionRejected() {
err := ts.Server.OAuthServerConsent(w, req)
ts.assertHTTPError(err, http.StatusBadRequest, apierrors.ErrorCodeValidationFailed)
}

// TestValidatePKCEParams_OAuth21 verifies that the plain PKCE method is rejected
// per OAuth 2.1 (draft-ietf-oauth-v2-1-12, Section 4.1.1) and that S256 is accepted.
func TestValidatePKCEParams_OAuth21(t *testing.T) {
globalConfig, err := confload.LoadGlobal(oauthServerTestConfig)
require.NoError(t, err)

conn, err := test.SetupDBConnection(globalConfig)
require.NoError(t, err)
defer conn.Close()

hooksMgr := &v0hooks.Manager{}
tokenService := tokens.NewService(globalConfig, hooksMgr)
server := NewServer(globalConfig, conn, tokenService)

tests := []struct {
name string
codeChallengeMethod string
codeChallenge string
wantErr bool
errContains string
}{
{
name: "S256 accepted",
codeChallengeMethod: "S256",
codeChallenge: "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM",
wantErr: false,
},
{
name: "s256 lowercase accepted",
codeChallengeMethod: "s256",
codeChallenge: "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM",
wantErr: false,
},
{
name: "plain rejected",
codeChallengeMethod: "plain",
codeChallenge: "abcdefghijklmnopqrstuvwxyz0123456789ABCDEFG",
wantErr: true,
errContains: "S256",
},
{
name: "PLAIN rejected case-insensitively",
codeChallengeMethod: "PLAIN",
codeChallenge: "abcdefghijklmnopqrstuvwxyz0123456789ABCDEFG",
wantErr: true,
errContains: "S256",
},
{
name: "unknown method rejected",
codeChallengeMethod: "rs256",
codeChallenge: "abcdefghijklmnopqrstuvwxyz0123456789ABCDEFG",
wantErr: true,
errContains: "S256",
},
{
name: "missing method rejected",
codeChallengeMethod: "",
codeChallenge: "abcdefghijklmnopqrstuvwxyz0123456789ABCDEFG",
wantErr: true,
errContains: "requires both",
},
{
name: "missing challenge rejected",
codeChallengeMethod: "S256",
codeChallenge: "",
wantErr: true,
errContains: "requires both",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := server.validatePKCEParams(tt.codeChallengeMethod, tt.codeChallenge)
if tt.wantErr {
assert.Error(t, err)
assert.Contains(t, err.Error(), tt.errContains)
} else {
assert.NoError(t, err)
}
})
}
}
5 changes: 3 additions & 2 deletions internal/models/oauth_authorization.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,9 @@ func NewOAuthServerAuthorization(params NewOAuthServerAuthorizationParams) *OAut
auth.CodeChallenge = &params.CodeChallenge
}
if params.CodeChallengeMethod != "" {
// Normalize code challenge method to lowercase for database storage
// Database enum expects 's256' and 'plain' (lowercase)
// Normalize code challenge method to lowercase for database storage.
// The OAuth 2.1 server only accepts 's256'; the legacy flow may still
// pass 'plain', which is handled at the validation layer.
normalizedMethod := strings.ToLower(params.CodeChallengeMethod)
auth.CodeChallengeMethod = &normalizedMethod
}
Expand Down