Skip to content

Commit

Permalink
fix: openid ignores missing redirect uri
Browse files Browse the repository at this point in the history
Fixes an issue where Authorize Requests which were intended for an OpenID Connect 1.0 client would incorrectly be allowed when missing the redirect URI when it's required by the specification.
  • Loading branch information
james-d-elliott committed Aug 4, 2023
1 parent 039746c commit f89f709
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 3 deletions.
15 changes: 13 additions & 2 deletions authorize_request_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,17 @@ func (f *Fosite) validateAuthorizeRedirectURI(_ *http.Request, request *Authoriz
// Fetch redirect URI from request
rawRedirURI := request.Form.Get("redirect_uri")

// This ensures that the 'redirect_uri' parameter is present for OpenID Connect 1.0 authorization requests as per:
//
// Authorization Code Flow - https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest
// Implicit Flow - https://openid.net/specs/openid-connect-core-1_0.html#ImplicitAuthRequest
// Hybrid Flow - https://openid.net/specs/openid-connect-core-1_0.html#HybridAuthRequest
//
// Note: as per the Hybrid Flow documentation the Hybrid Flow has the same requirements as the Authorization Code Flow.
if len(rawRedirURI) == 0 && request.GetRequestedScopes().Has("openid") {
return errorsx.WithStack(ErrInvalidRequest.WithHint("The 'redirect_uri' parameter is required when using OpenID Connect 1.0."))
}

// Validate redirect uri
redirectURI, err := MatchRedirectURIWithClientRedirectURIs(rawRedirURI, request.Client)
if err != nil {
Expand Down Expand Up @@ -358,11 +369,11 @@ func (f *Fosite) newAuthorizeRequest(ctx context.Context, r *http.Request, isPAR
return request, err
}

if err := f.validateAuthorizeRedirectURI(r, request); err != nil {
if err := f.validateAuthorizeScope(ctx, r, request); err != nil {
return request, err
}

if err := f.validateAuthorizeScope(ctx, r, request); err != nil {
if err := f.validateAuthorizeRedirectURI(r, request); err != nil {
return request, err
}

Expand Down
15 changes: 14 additions & 1 deletion integration/oidc_explicit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,18 @@ func TestOpenIDConnectExplicitFlow(t *testing.T) {
},
authStatusCode: http.StatusOK,
},
{
session: newIDSession(&jwt.IDTokenClaims{Subject: "peter"}),
description: "should fail registered single redirect uri but no redirect uri in request",
setup: func(oauthClient *oauth2.Config) string {
oauthClient.Scopes = []string{"openid"}
oauthClient.RedirectURL = ""

return oauthClient.AuthCodeURL("12345678901234567890") + "&nonce=11234123"
},
authStatusCode: http.StatusBadRequest,
expectAuthErr: `{"error":"invalid_request","error_description":"The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed. The 'redirect_uri' parameter is required when using OpenID Connect 1.0."}`,
},
{
session: newIDSession(&jwt.IDTokenClaims{Subject: "peter"}),
description: "should fail because nonce is not long enough",
Expand Down Expand Up @@ -121,7 +133,8 @@ func TestOpenIDConnectExplicitFlow(t *testing.T) {
defer ts.Close()

oauthClient := newOAuth2Client(ts)
fositeStore.Clients["my-client"].(*fosite.DefaultClient).RedirectURIs[0] = ts.URL + "/callback"

fositeStore.Clients["my-client"].(*fosite.DefaultClient).RedirectURIs = []string{ts.URL + "/callback"}

resp, err := http.Get(c.setup(oauthClient))
require.NoError(t, err)
Expand Down

0 comments on commit f89f709

Please sign in to comment.