From b7d18bfd0284b42a7dfc86dfcbf283cab19fbbd6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20M=C3=B6hlmann?= Date: Tue, 28 Mar 2023 14:58:57 +0300 Subject: [PATCH] chore: document non-standard glob client (#328) * op: correct typo rename checkURIAginstRedirects to checkURIAgainstRedirects * chore: document standard deviation when using globs add example on how to toggle the underlying client implementation based on DevMode. --------- Co-authored-by: David Sharnoff --- example/server/storage/client.go | 35 ++++++++++++++++++++++--------- example/server/storage/storage.go | 2 +- pkg/op/auth_request.go | 10 ++++----- pkg/op/client.go | 6 ++++++ 4 files changed, 37 insertions(+), 16 deletions(-) diff --git a/example/server/storage/client.go b/example/server/storage/client.go index b850053d..b8b99604 100644 --- a/example/server/storage/client.go +++ b/example/server/storage/client.go @@ -32,6 +32,8 @@ type Client struct { devMode bool idTokenUserinfoClaimsAssertion bool clockSkew time.Duration + postLogoutRedirectURIGlobs []string + redirectURIGlobs []string } // GetID must return the client_id @@ -44,21 +46,11 @@ 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 @@ -200,3 +192,26 @@ func WebClient(id, secret string, redirectURIs ...string) *Client { clockSkew: 0, } } + +type hasRedirectGlobs struct { + *Client +} + +// RedirectURIGlobs provide wildcarding for additional valid redirects +func (c hasRedirectGlobs) RedirectURIGlobs() []string { + return c.redirectURIGlobs +} + +// PostLogoutRedirectURIGlobs provide extra wildcarding for additional valid redirects +func (c hasRedirectGlobs) PostLogoutRedirectURIGlobs() []string { + return c.postLogoutRedirectURIGlobs +} + +// RedirectGlobsClient wraps the client in a op.HasRedirectGlobs +// only if DevMode is enabled. +func RedirectGlobsClient(client *Client) op.Client { + if client.devMode { + return hasRedirectGlobs{client} + } + return client +} diff --git a/example/server/storage/storage.go b/example/server/storage/storage.go index acac5711..a4c4f46d 100644 --- a/example/server/storage/storage.go +++ b/example/server/storage/storage.go @@ -418,7 +418,7 @@ func (s *Storage) GetClientByClientID(ctx context.Context, clientID string) (op. if !ok { return nil, fmt.Errorf("client not found") } - return client, nil + return RedirectGlobsClient(client), nil } // AuthorizeClientIDSecret implements the op.Storage interface diff --git a/pkg/op/auth_request.go b/pkg/op/auth_request.go index b3120988..1f9fc459 100644 --- a/pkg/op/auth_request.go +++ b/pkg/op/auth_request.go @@ -274,9 +274,9 @@ func ValidateAuthReqScopes(client Client, scopes []string) ([]string, error) { return scopes, nil } -// checkURIAginstRedirects just checks aginst the valid redirect URIs and ignores +// checkURIAgainstRedirects just checks aginst the valid redirect URIs and ignores // other factors. -func checkURIAginstRedirects(client Client, uri string) error { +func checkURIAgainstRedirects(client Client, uri string) error { if str.Contains(client.RedirectURIs(), uri) { return nil } @@ -303,12 +303,12 @@ 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://") { - return checkURIAginstRedirects(client, uri) + return checkURIAgainstRedirects(client, uri) } if client.ApplicationType() == ApplicationTypeNative { return validateAuthReqRedirectURINative(client, uri, responseType) } - if err := checkURIAginstRedirects(client, uri); err != nil { + if err := checkURIAgainstRedirects(client, uri); err != nil { return err } if strings.HasPrefix(uri, "http://") { @@ -329,7 +329,7 @@ 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 err := checkURIAginstRedirects(client, uri); err == nil { + if err := checkURIAgainstRedirects(client, uri); err == nil { if client.DevMode() { return nil } diff --git a/pkg/op/client.go b/pkg/op/client.go index af4724a8..9da44a7d 100644 --- a/pkg/op/client.go +++ b/pkg/op/client.go @@ -56,6 +56,12 @@ type Client interface { // 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. +// +// Note that globbing / wildcards are not permitted by the OIDC +// standard and implementing this interface can have security implications. +// It is advised to only return a client of this type in rare cases, +// such as DevMode for the client being enabled. +// https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest type HasRedirectGlobs interface { RedirectURIGlobs() []string PostLogoutRedirectURIGlobs() []string