From 7e5798569b48f70971d3abf0ce92bbd109bcb61b Mon Sep 17 00:00:00 2001 From: David Sharnoff Date: Mon, 6 Mar 2023 04:13:35 -0800 Subject: [PATCH] fix: glob support for RedirectURIs Fixes #293 --- example/server/storage/client.go | 18 +++++++++++---- pkg/op/auth_request.go | 38 +++++++++++++++++++++++--------- pkg/op/client.go | 10 +++++++++ pkg/op/session.go | 12 ++++++++++ 4 files changed, 64 insertions(+), 14 deletions(-) diff --git a/example/server/storage/client.go b/example/server/storage/client.go index 0f3a703..0b98679 100644 --- a/example/server/storage/client.go +++ b/example/server/storage/client.go @@ -44,11 +44,21 @@ func (c *Client) RedirectURIs() []string { return c.redirectURIs } +// RedirectURIGlobs provide wildcarding for additional valid redirects +func (c *Client) RedirectURIGlobs() []string { + return nil +} + // PostLogoutRedirectURIs must return the registered post_logout_redirect_uris for sign-outs func (c *Client) PostLogoutRedirectURIs() []string { return []string{} } +// PostLogoutRedirectURIGlobs provide extra wildcarding for additional valid redirects +func (c *Client) PostLogoutRedirectURIGlobs() []string { + return nil +} + // ApplicationType must return the type of the client (app, native, user agent) func (c *Client) ApplicationType() op.ApplicationType { return c.applicationType @@ -113,14 +123,14 @@ func (c *Client) IsScopeAllowed(scope string) bool { // IDTokenUserinfoClaimsAssertion allows specifying if claims of scope profile, email, phone and address are asserted into the id_token // even if an access token if issued which violates the OIDC Core spec -//(5.4. Requesting Claims using Scope Values: https://openid.net/specs/openid-connect-core-1_0.html#ScopeClaims) +// (5.4. Requesting Claims using Scope Values: https://openid.net/specs/openid-connect-core-1_0.html#ScopeClaims) // some clients though require that e.g. email is always in the id_token when requested even if an access_token is issued func (c *Client) IDTokenUserinfoClaimsAssertion() bool { return c.idTokenUserinfoClaimsAssertion } // ClockSkew enables clients to instruct the OP to apply a clock skew on the various times and expirations -//(subtract from issued_at, add to expiration, ...) +// (subtract from issued_at, add to expiration, ...) func (c *Client) ClockSkew() time.Duration { return c.clockSkew } @@ -141,7 +151,7 @@ func RegisterClients(registerClients ...*Client) { // user-defined redirectURIs may include: // - http://localhost without port specification (e.g. http://localhost/auth/callback) // - custom protocol (e.g. custom://auth/callback) -//(the examples will be used as default, if none is provided) +// (the examples will be used as default, if none is provided) func NativeClient(id string, redirectURIs ...string) *Client { if len(redirectURIs) == 0 { redirectURIs = []string{ @@ -168,7 +178,7 @@ func NativeClient(id string, redirectURIs ...string) *Client { // WebClient will create a client of type web, which will always use Basic Auth and allow the use of refresh tokens // user-defined redirectURIs may include: // - http://localhost with port specification (e.g. http://localhost:9999/auth/callback) -//(the example will be used as default, if none is provided) +// (the example will be used as default, if none is provided) func WebClient(id, secret string, redirectURIs ...string) *Client { if len(redirectURIs) == 0 { redirectURIs = []string{ diff --git a/pkg/op/auth_request.go b/pkg/op/auth_request.go index d8c960e..ecfde28 100644 --- a/pkg/op/auth_request.go +++ b/pkg/op/auth_request.go @@ -6,6 +6,7 @@ import ( "net" "net/http" "net/url" + "path" "strings" "time" @@ -274,6 +275,28 @@ func ValidateAuthReqScopes(client Client, scopes []string) ([]string, error) { return scopes, nil } +// checkURIAginstRedirects just checks aginst the valid redirect URIs and ignores +// other factors. +func checkURIAginstRedirects(client Client, uri string) error { + if str.Contains(client.RedirectURIs(), uri) { + return nil + } + if globClient, ok := client.(HasRedirectGlobs); ok { + for _, uriGlob := range globClient.RedirectURIGlobs() { + isMatch, err := path.Match(uriGlob, uri) + if err != nil { + return oidc.ErrServerError().WithParent(err) + } + if isMatch { + return nil + } + } + } + return oidc.ErrInvalidRequestRedirectURI(). + WithDescription("The requested redirect_uri is missing in the client configuration. " + + "If you have any questions, you may contact the administrator of the application.") +} + // ValidateAuthReqRedirectURI validates the passed redirect_uri and response_type to the registered uris and client type func ValidateAuthReqRedirectURI(client Client, uri string, responseType oidc.ResponseType) error { if uri == "" { @@ -281,19 +304,13 @@ func ValidateAuthReqRedirectURI(client Client, uri string, responseType oidc.Res "Please ensure it is added to the request. If you have any questions, you may contact the administrator of the application.") } if strings.HasPrefix(uri, "https://") { - if !str.Contains(client.RedirectURIs(), uri) { - return oidc.ErrInvalidRequestRedirectURI(). - WithDescription("The requested redirect_uri is missing in the client configuration. " + - "If you have any questions, you may contact the administrator of the application.") - } - return nil + return checkURIAginstRedirects(client, uri) } if client.ApplicationType() == ApplicationTypeNative { return validateAuthReqRedirectURINative(client, uri, responseType) } - if !str.Contains(client.RedirectURIs(), uri) { - return oidc.ErrInvalidRequestRedirectURI().WithDescription("The requested redirect_uri is missing in the client configuration. " + - "If you have any questions, you may contact the administrator of the application.") + if err := checkURIAginstRedirects(client, uri); err != nil { + return err } if strings.HasPrefix(uri, "http://") { if client.DevMode() { @@ -313,10 +330,11 @@ func ValidateAuthReqRedirectURI(client Client, uri string, responseType oidc.Res func validateAuthReqRedirectURINative(client Client, uri string, responseType oidc.ResponseType) error { parsedURL, isLoopback := HTTPLoopbackOrLocalhost(uri) isCustomSchema := !strings.HasPrefix(uri, "http://") - if str.Contains(client.RedirectURIs(), uri) { + if err := checkURIAginstRedirects(client, uri); err == nil { if client.DevMode() { return nil } + // The RedirectURIs are only valid for native clients when localhost or non-"http://" if isLoopback || isCustomSchema { return nil } diff --git a/pkg/op/client.go b/pkg/op/client.go index d9f7ab0..db3d69b 100644 --- a/pkg/op/client.go +++ b/pkg/op/client.go @@ -45,6 +45,16 @@ type Client interface { ClockSkew() time.Duration } +// HasRedirectGlobs is an optional interface that can be implemented by implementors of +// Client. See https://pkg.go.dev/path#Match for glob +// interpretation. Redirect URIs that match either the non-glob version or the +// glob version will be accepted. Glob URIs are only partially supported for native +// clients: "http://" is not allowed except for loopback or in dev mode. +type HasRedirectGlobs interface { + RedirectURIGlobs() []string + PostLogoutRedirectURIGlobs() []string +} + func ContainsResponseType(types []oidc.ResponseType, responseType oidc.ResponseType) bool { for _, t := range types { if t == responseType { diff --git a/pkg/op/session.go b/pkg/op/session.go index c4984fc..737bb86 100644 --- a/pkg/op/session.go +++ b/pkg/op/session.go @@ -4,6 +4,7 @@ import ( "context" "net/http" "net/url" + "path" httphelper "github.com/zitadel/oidc/pkg/http" "github.com/zitadel/oidc/pkg/oidc" @@ -98,5 +99,16 @@ func ValidateEndSessionPostLogoutRedirectURI(postLogoutRedirectURI string, clien return nil } } + if globClient, ok := client.(HasRedirectGlobs); ok { + for _, uriGlob := range globClient.PostLogoutRedirectURIGlobs() { + isMatch, err := path.Match(uriGlob, postLogoutRedirectURI) + if err != nil { + return oidc.ErrServerError().WithParent(err) + } + if isMatch { + return nil + } + } + } return oidc.ErrInvalidRequest().WithDescription("post_logout_redirect_uri invalid") }