Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(pkce): session generated needlessly #759

Merged
merged 1 commit into from
Feb 13, 2024
Merged

fix(pkce): session generated needlessly #759

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
59 changes: 40 additions & 19 deletions handler/pkce/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ func (c *Handler) HandleAuthorizeEndpointRequest(ctx context.Context, ar fosite.
return err
}

// We don't need a session if it's not enforced and the PKCE parameters are not provided by the client.
if challenge == "" && method == "" {
return nil
}

code := resp.GetCode()
if len(code) == 0 {
return errorsx.WithStack(fosite.ErrServerError.WithDebug("The PKCE handler must be loaded after the authorize code handler."))
Expand All @@ -64,24 +69,14 @@ func (c *Handler) HandleAuthorizeEndpointRequest(ctx context.Context, ar fosite.
}

func (c *Handler) validate(ctx context.Context, challenge, method string, client fosite.Client) error {
if challenge == "" {
if len(challenge) == 0 {
// If the server requires Proof Key for Code Exchange (PKCE) by OAuth
// clients and the client does not send the "code_challenge" in
// the request, the authorization endpoint MUST return the authorization
// error response with the "error" value set to "invalid_request". The
// "error_description" or the response of "error_uri" SHOULD explain the
// nature of error, e.g., code challenge required.
if c.Config.GetEnforcePKCE(ctx) {
return errorsx.WithStack(fosite.ErrInvalidRequest.
WithHint("Clients must include a code_challenge when performing the authorize code flow, but it is missing.").
WithDebug("The server is configured in a way that enforces PKCE for clients."))
}
if c.Config.GetEnforcePKCEForPublicClients(ctx) && client.IsPublic() {
return errorsx.WithStack(fosite.ErrInvalidRequest.
WithHint("This client must include a code_challenge when performing the authorize code flow, but it is missing.").
WithDebug("The server is configured in a way that enforces PKCE for this client."))
}
return nil
return c.validateNoPKCE(ctx, client)
}

// If the server supporting PKCE does not support the requested
Expand All @@ -108,6 +103,20 @@ func (c *Handler) validate(ctx context.Context, challenge, method string, client
return nil
}

func (c *Handler) validateNoPKCE(ctx context.Context, client fosite.Client) error {
if c.Config.GetEnforcePKCE(ctx) {
return errorsx.WithStack(fosite.ErrInvalidRequest.
WithHint("Clients must include a code_challenge when performing the authorize code flow, but it is missing.").
WithDebug("The server is configured in a way that enforces PKCE for clients."))
}
if c.Config.GetEnforcePKCEForPublicClients(ctx) && client.IsPublic() {
return errorsx.WithStack(fosite.ErrInvalidRequest.
WithHint("This client must include a code_challenge when performing the authorize code flow, but it is missing.").
WithDebug("The server is configured in a way that enforces PKCE for this client."))
}
return nil
}

func (c *Handler) HandleTokenEndpointRequest(ctx context.Context, request fosite.AccessRequester) error {
if !c.CanHandleTokenEndpointRequest(ctx, request) {
return errorsx.WithStack(fosite.ErrUnknownRequest)
Expand All @@ -123,8 +132,15 @@ func (c *Handler) HandleTokenEndpointRequest(ctx context.Context, request fosite

code := request.GetRequestForm().Get("code")
signature := c.AuthorizeCodeStrategy.AuthorizeCodeSignature(ctx, code)
authorizeRequest, err := c.Storage.GetPKCERequestSession(ctx, signature, request.GetSession())
pkceRequest, err := c.Storage.GetPKCERequestSession(ctx, signature, request.GetSession())

nv := len(verifier)

if errors.Is(err, fosite.ErrNotFound) {
if nv == 0 {
return c.validateNoPKCE(ctx, request.GetClient())
}

return errorsx.WithStack(fosite.ErrInvalidGrant.WithHint("Unable to find initial PKCE data tied to this request").WithWrap(err).WithDebug(err.Error()))
} else if err != nil {
return errorsx.WithStack(fosite.ErrServerError.WithWrap(err).WithDebug(err.Error()))
Expand All @@ -134,14 +150,16 @@ func (c *Handler) HandleTokenEndpointRequest(ctx context.Context, request fosite
return errorsx.WithStack(fosite.ErrServerError.WithWrap(err).WithDebug(err.Error()))
}

challenge := authorizeRequest.GetRequestForm().Get("code_challenge")
method := authorizeRequest.GetRequestForm().Get("code_challenge_method")
client := authorizeRequest.GetClient()
challenge := pkceRequest.GetRequestForm().Get("code_challenge")
method := pkceRequest.GetRequestForm().Get("code_challenge_method")
client := pkceRequest.GetClient()
if err := c.validate(ctx, challenge, method, client); err != nil {
return err
}

if !c.Config.GetEnforcePKCE(ctx) && challenge == "" && verifier == "" {
nc := len(challenge)

if !c.Config.GetEnforcePKCE(ctx) && nc == 0 && nv == 0 {
return nil
}

Expand All @@ -152,15 +170,18 @@ func (c *Handler) HandleTokenEndpointRequest(ctx context.Context, request fosite
// 43-octet URL safe string to use as the code verifier.

// Validation
if len(verifier) < 43 {
if nv < 43 {
return errorsx.WithStack(fosite.ErrInvalidGrant.
WithHint("The PKCE code verifier must be at least 43 characters."))
} else if len(verifier) > 128 {
} else if nv > 128 {
return errorsx.WithStack(fosite.ErrInvalidGrant.
WithHint("The PKCE code verifier can not be longer than 128 characters."))
} else if verifierWrongFormat.MatchString(verifier) {
return errorsx.WithStack(fosite.ErrInvalidGrant.
WithHint("The PKCE code verifier must only contain [a-Z], [0-9], '-', '.', '_', '~'."))
} else if nc == 0 {
return errorsx.WithStack(fosite.ErrInvalidGrant.
WithHint("The PKCE code verifier was provided but the code challenge was absent from the authorization request."))
}

// Upon receipt of the request at the token endpoint, the server
Expand Down
Loading
Loading