cim/zitadel-oidc
5
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

feat(op): always verify code challenge when available (#721)

Browse source 
Finally the RFC Best Current Practice for OAuth 2.0 Security has been approved.

According to the RFC:

> Authorization servers MUST support PKCE [RFC7636].
> 
> If a client sends a valid PKCE code_challenge parameter in the authorization request, the authorization server MUST enforce the correct usage of code_verifier at the token endpoint.

Isn’t it time we strengthen PKCE support a bit more?

This PR updates the logic so that PKCE is always verified, even when the Auth Method is not "none".
This commit is contained in:
Ayato 2025-03-25 01:00:04 +09:00 • committed by GitHub
parent 7096406e71
commit c51628ea27
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 45 additions and 15 deletions
Download patch file Download diff file Expand all files Collapse all files

1
pkg/op/op_test.go
View file

@ -102,6 +102,7 @@ func TestRoutes(t *testing.T) {
authReq, err := storage.CreateAuthRequest(ctx, oidcAuthReq, "id1")
require.NoError(t, err)
storage.AuthRequestDone(authReq.GetID())
storage.SaveAuthCode(ctx, authReq.GetID(), "123")
accessToken, refreshToken, _, err := op.CreateAccessToken(ctx, authReq, op.AccessTokenTypeBearer, testProvider, client, "")
require.NoError(t, err)

2
pkg/op/server_http_routes_test.go
View file

@ -130,7 +130,7 @@ func TestServerRoutes(t *testing.T) {
"client_id": client.GetID(),
"client_secret": "secret",
"redirect_uri": "https://example.com",
"code": "123",
"code": "abc",
},
wantCode: http.StatusBadRequest,
json: `{"error":"invalid_grant", "error_description":"invalid code"}`,

26
pkg/op/token_code.go
View file

@ -74,6 +74,20 @@ func AuthorizeCodeClient(ctx context.Context, tokenReq *oidc.AccessTokenRequest,
ctx, span := tracer.Start(ctx, "AuthorizeCodeClient")
defer span.End()
request, err = AuthRequestByCode(ctx, exchanger.Storage(), tokenReq.Code)
if err != nil {
return nil, nil, err
}
codeChallenge := request.GetCodeChallenge()
if codeChallenge != nil {
err = AuthorizeCodeChallenge(tokenReq.CodeVerifier, codeChallenge)
if err != nil {
return nil, nil, err
}
}
if tokenReq.ClientAssertionType == oidc.ClientAssertionTypeJWTAssertion {
jwtExchanger, ok := exchanger.(JWTAuthorizationGrantExchanger)
if !ok || !exchanger.AuthMethodPrivateKeyJWTSupported() {
@ -83,9 +97,9 @@ func AuthorizeCodeClient(ctx context.Context, tokenReq *oidc.AccessTokenRequest,
if err != nil {
return nil, nil, err
}
request, err = AuthRequestByCode(ctx, exchanger.Storage(), tokenReq.Code)
return request, client, err
}
client, err = exchanger.Storage().GetClientByClientID(ctx, tokenReq.ClientID)
if err != nil {
return nil, nil, oidc.ErrInvalidClient().WithParent(err)
@ -94,12 +108,10 @@ func AuthorizeCodeClient(ctx context.Context, tokenReq *oidc.AccessTokenRequest,
return nil, nil, oidc.ErrInvalidClient().WithDescription("private_key_jwt not allowed for this client")
}
if client.AuthMethod() == oidc.AuthMethodNone {
request, err = AuthRequestByCode(ctx, exchanger.Storage(), tokenReq.Code)
if err != nil {
return nil, nil, err
if codeChallenge == nil {
return nil, nil, oidc.ErrInvalidRequest().WithDescription("PKCE required")
}
err = AuthorizeCodeChallenge(tokenReq.CodeVerifier, request.GetCodeChallenge())
return request, client, err
return request, client, nil
}
if client.AuthMethod() == oidc.AuthMethodPost && !exchanger.AuthMethodPostSupported() {
return nil, nil, oidc.ErrInvalidClient().WithDescription("auth_method post not supported")
@ -108,7 +120,7 @@ func AuthorizeCodeClient(ctx context.Context, tokenReq *oidc.AccessTokenRequest,
if err != nil {
return nil, nil, err
}
request, err = AuthRequestByCode(ctx, exchanger.Storage(), tokenReq.Code)
return request, client, err
}