identity/oidc: reorder authorization endpoint validation for invalid redirect uris (#16601)

* identity/oidc: reorder authorization endpoint validation for invalid redirect uris

* adds changelog

* use provider.allowedClientID
This commit is contained in:
Austin Gebauer 2022-08-08 09:02:18 -07:00 committed by GitHub
parent 326936b1ef
commit ed143c5678
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 42 additions and 39 deletions

4
changelog/16601.txt Normal file
View File

@ -0,0 +1,4 @@
```release-note:bug
identity/oidc: Detect invalid `redirect_uri` values sooner in validation of the
Authorization Endpoint.
```

View File

@ -1617,11 +1617,27 @@ func (i *IdentityStore) keyIDsReferencedByTargetClientIDs(ctx context.Context, s
func (i *IdentityStore) pathOIDCAuthorize(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { func (i *IdentityStore) pathOIDCAuthorize(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) {
state := d.Get("state").(string) state := d.Get("state").(string)
// Get the namespace // Validate the client ID
ns, err := namespace.FromContext(ctx) clientID := d.Get("client_id").(string)
if clientID == "" {
return authResponse("", state, ErrAuthInvalidClientID, "client_id parameter is required")
}
client, err := i.clientByID(ctx, req.Storage, clientID)
if err != nil { if err != nil {
return authResponse("", state, ErrAuthServerError, err.Error()) return authResponse("", state, ErrAuthServerError, err.Error())
} }
if client == nil {
return authResponse("", state, ErrAuthInvalidClientID, "client with client_id not found")
}
// Validate the redirect URI
redirectURI := d.Get("redirect_uri").(string)
if redirectURI == "" {
return authResponse("", state, ErrAuthInvalidRequest, "redirect_uri parameter is required")
}
if !validRedirect(redirectURI, client.RedirectURIs) {
return authResponse("", state, ErrAuthInvalidRedirectURI, "redirect_uri is not allowed for the client")
}
// Get the OIDC provider // Get the OIDC provider
name := d.Get("name").(string) name := d.Get("name").(string)
@ -1632,6 +1648,20 @@ func (i *IdentityStore) pathOIDCAuthorize(ctx context.Context, req *logical.Requ
if provider == nil { if provider == nil {
return authResponse("", state, ErrAuthInvalidRequest, "provider not found") return authResponse("", state, ErrAuthInvalidRequest, "provider not found")
} }
if !provider.allowedClientID(clientID) {
return authResponse("", state, ErrAuthUnauthorizedClient, "client is not authorized to use the provider")
}
// We don't support the request or request_uri parameters. If they're provided,
// the appropriate errors must be returned. For details, see the spec at:
// https://openid.net/specs/openid-connect-core-1_0.html#RequestObject
// https://openid.net/specs/openid-connect-core-1_0.html#RequestUriParameter
if _, ok := d.Raw["request"]; ok {
return authResponse("", "", ErrAuthRequestNotSupported, "request parameter is not supported")
}
if _, ok := d.Raw["request_uri"]; ok {
return authResponse("", "", ErrAuthRequestURINotSupported, "request_uri parameter is not supported")
}
// Validate that a scope parameter is present and contains the openid scope value // Validate that a scope parameter is present and contains the openid scope value
requestedScopes := strutil.ParseDedupAndSortStrings(d.Get("scope").(string), scopesDelimiter) requestedScopes := strutil.ParseDedupAndSortStrings(d.Get("scope").(string), scopesDelimiter)
@ -1657,43 +1687,6 @@ func (i *IdentityStore) pathOIDCAuthorize(ctx context.Context, req *logical.Requ
return authResponse("", state, ErrAuthUnsupportedResponseType, "unsupported response_type value") return authResponse("", state, ErrAuthUnsupportedResponseType, "unsupported response_type value")
} }
// Validate the client ID
clientID := d.Get("client_id").(string)
if clientID == "" {
return authResponse("", state, ErrAuthInvalidClientID, "client_id parameter is required")
}
client, err := i.clientByID(ctx, req.Storage, clientID)
if err != nil {
return authResponse("", state, ErrAuthServerError, err.Error())
}
if client == nil {
return authResponse("", state, ErrAuthInvalidClientID, "client with client_id not found")
}
if !provider.allowedClientID(clientID) {
return authResponse("", state, ErrAuthUnauthorizedClient, "client is not authorized to use the provider")
}
// Validate the redirect URI
redirectURI := d.Get("redirect_uri").(string)
if redirectURI == "" {
return authResponse("", state, ErrAuthInvalidRequest, "redirect_uri parameter is required")
}
if !validRedirect(redirectURI, client.RedirectURIs) {
return authResponse("", state, ErrAuthInvalidRedirectURI, "redirect_uri is not allowed for the client")
}
// We don't support the request or request_uri parameters. If they're provided,
// the appropriate errors must be returned. For details, see the spec at:
// https://openid.net/specs/openid-connect-core-1_0.html#RequestObject
// https://openid.net/specs/openid-connect-core-1_0.html#RequestUriParameter
if _, ok := d.Raw["request"]; ok {
return authResponse("", state, ErrAuthRequestNotSupported, "request parameter is not supported")
}
if _, ok := d.Raw["request_uri"]; ok {
return authResponse("", state, ErrAuthRequestURINotSupported, "request_uri parameter is not supported")
}
// Validate that there is an identity entity associated with the request // Validate that there is an identity entity associated with the request
if req.EntityID == "" { if req.EntityID == "" {
return authResponse("", state, ErrAuthAccessDenied, "identity entity must be associated with the request") return authResponse("", state, ErrAuthAccessDenied, "identity entity must be associated with the request")
@ -1797,6 +1790,12 @@ func (i *IdentityStore) pathOIDCAuthorize(ctx context.Context, req *logical.Requ
return authResponse("", state, ErrAuthServerError, err.Error()) return authResponse("", state, ErrAuthServerError, err.Error())
} }
// Get the namespace
ns, err := namespace.FromContext(ctx)
if err != nil {
return authResponse("", state, ErrAuthServerError, err.Error())
}
// Cache the authorization code for a subsequent token exchange // Cache the authorization code for a subsequent token exchange
if err := i.oidcAuthCodeCache.SetDefault(ns, code, authCodeEntry); err != nil { if err := i.oidcAuthCodeCache.SetDefault(ns, code, authCodeEntry); err != nil {
return authResponse("", state, ErrAuthServerError, err.Error()) return authResponse("", state, ErrAuthServerError, err.Error())