From 1e9df0a6bb648938b4ad33a5b1ea71140ed22e2d Mon Sep 17 00:00:00 2001 From: Carl Zhou Date: Tue, 3 Sep 2024 10:53:02 -0700 Subject: [PATCH 01/10] fallback to env variable for oidc client id and secret Signed-off-by: Carl Zhou --- filters/auth/oidc.go | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/filters/auth/oidc.go b/filters/auth/oidc.go index 98cec128a0..38bf442263 100644 --- a/filters/auth/oidc.go +++ b/filters/auth/oidc.go @@ -13,6 +13,7 @@ import ( "net" "net/http" "net/url" + "os" "sort" "strconv" "strings" @@ -242,19 +243,29 @@ func (s *tokenOidcSpec) CreateFilter(args []interface{}) (filters.Filter, error) validity = defaultCookieValidity } + oidcClientId := sargs[paramClientID] + if oidcClientId == "" { + oidcClientId, _ = os.LookupEnv("OIDC_CLIENT_ID") + } + + oidcClientSecret := sargs[paramClientSecret] + if oidcClientSecret == "" { + oidcClientSecret, _ = os.LookupEnv("OIDC_CLIENT_SECRET") + } + f := &tokenOidcFilter{ typ: s.typ, redirectPath: redirectURL.Path, config: &oauth2.Config{ - ClientID: sargs[paramClientID], - ClientSecret: sargs[paramClientSecret], + ClientID: oidcClientId, + ClientSecret: oidcClientSecret, RedirectURL: sargs[paramCallbackURL], // self endpoint Endpoint: provider.Endpoint(), Scopes: []string{oidc.ScopeOpenID}, // mandatory scope by spec }, provider: provider, verifier: provider.Verifier(&oidc.Config{ - ClientID: sargs[paramClientID], + ClientID: oidcClientId, }), validity: validity, cookiename: generatedCookieName, From e7ddc41fc2fa80b4662e3c57c5c2bde33e0bc42f Mon Sep 17 00:00:00 2001 From: Carl Zhou Date: Tue, 3 Sep 2024 11:42:05 -0700 Subject: [PATCH 02/10] fallback to env variable for oauth grant client id and secret Signed-off-by: Carl Zhou --- filters/auth/grantconfig.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/filters/auth/grantconfig.go b/filters/auth/grantconfig.go index 97a9dd2661..c03fa4e64d 100644 --- a/filters/auth/grantconfig.go +++ b/filters/auth/grantconfig.go @@ -6,6 +6,7 @@ import ( "net" "net/http" "net/url" + "os" "path/filepath" "strings" "time" @@ -200,6 +201,10 @@ func (c *OAuthConfig) Init() error { c.flowState = newFlowState(c.Secrets, c.SecretFile) + if c.ClientID == "" { + c.ClientID, _ = os.LookupEnv("OAUTH2_CLIENT_ID") + } + if c.ClientID != "" { c.getClientId = func(*http.Request) (string, error) { return c.ClientID, nil @@ -227,6 +232,10 @@ func (c *OAuthConfig) Init() error { return ErrMissingClientID } + if c.ClientSecret == "" { + c.ClientSecret, _ = os.LookupEnv("OAUTH2_CLIENT_ID") + } + if c.ClientSecret != "" { c.getClientSecret = func(*http.Request) (string, error) { return c.ClientSecret, nil From a8794804b6bb4bc962628828a2ad344215cb02ed Mon Sep 17 00:00:00 2001 From: Carl Zhou Date: Tue, 3 Sep 2024 11:43:01 -0700 Subject: [PATCH 03/10] fix env var name Signed-off-by: Carl Zhou --- filters/auth/grantconfig.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/filters/auth/grantconfig.go b/filters/auth/grantconfig.go index c03fa4e64d..a35b6de57d 100644 --- a/filters/auth/grantconfig.go +++ b/filters/auth/grantconfig.go @@ -233,7 +233,7 @@ func (c *OAuthConfig) Init() error { } if c.ClientSecret == "" { - c.ClientSecret, _ = os.LookupEnv("OAUTH2_CLIENT_ID") + c.ClientSecret, _ = os.LookupEnv("OAUTH2_CLIENT_SECRET") } if c.ClientSecret != "" { From 803ede000d70d2c1f7b11a18cc28729a6dd8c0cc Mon Sep 17 00:00:00 2001 From: Carl Zhou Date: Wed, 4 Sep 2024 10:47:50 -0700 Subject: [PATCH 04/10] move env lookup to skipper.go Signed-off-by: Carl Zhou --- filters/auth/grantconfig.go | 9 --------- filters/auth/oidc.go | 15 ++++++++------- skipper.go | 18 ++++++++++++++---- 3 files changed, 22 insertions(+), 20 deletions(-) diff --git a/filters/auth/grantconfig.go b/filters/auth/grantconfig.go index a35b6de57d..97a9dd2661 100644 --- a/filters/auth/grantconfig.go +++ b/filters/auth/grantconfig.go @@ -6,7 +6,6 @@ import ( "net" "net/http" "net/url" - "os" "path/filepath" "strings" "time" @@ -201,10 +200,6 @@ func (c *OAuthConfig) Init() error { c.flowState = newFlowState(c.Secrets, c.SecretFile) - if c.ClientID == "" { - c.ClientID, _ = os.LookupEnv("OAUTH2_CLIENT_ID") - } - if c.ClientID != "" { c.getClientId = func(*http.Request) (string, error) { return c.ClientID, nil @@ -232,10 +227,6 @@ func (c *OAuthConfig) Init() error { return ErrMissingClientID } - if c.ClientSecret == "" { - c.ClientSecret, _ = os.LookupEnv("OAUTH2_CLIENT_SECRET") - } - if c.ClientSecret != "" { c.getClientSecret = func(*http.Request) (string, error) { return c.ClientSecret, nil diff --git a/filters/auth/oidc.go b/filters/auth/oidc.go index 38bf442263..6d4e502b7e 100644 --- a/filters/auth/oidc.go +++ b/filters/auth/oidc.go @@ -13,7 +13,6 @@ import ( "net" "net/http" "net/url" - "os" "sort" "strconv" "strings" @@ -89,10 +88,12 @@ const ( ) type OidcOptions struct { - MaxIdleConns int - CookieValidity time.Duration - Timeout time.Duration - Tracer opentracing.Tracer + MaxIdleConns int + CookieValidity time.Duration + Timeout time.Duration + Tracer opentracing.Tracer + OidcClientId string + OidcClientSecret string } type ( @@ -245,12 +246,12 @@ func (s *tokenOidcSpec) CreateFilter(args []interface{}) (filters.Filter, error) oidcClientId := sargs[paramClientID] if oidcClientId == "" { - oidcClientId, _ = os.LookupEnv("OIDC_CLIENT_ID") + oidcClientId = s.options.OidcClientId } oidcClientSecret := sargs[paramClientSecret] if oidcClientSecret == "" { - oidcClientSecret, _ = os.LookupEnv("OIDC_CLIENT_SECRET") + oidcClientSecret = s.options.OidcClientSecret } f := &tokenOidcFilter{ diff --git a/skipper.go b/skipper.go index 8b4bdc2697..83a12a673a 100644 --- a/skipper.go +++ b/skipper.go @@ -981,7 +981,13 @@ func (o *Options) OAuthGrantOptions() *auth.OAuthConfig { oauthConfig.TokeninfoURL = o.OAuthTokeninfoURL oauthConfig.SecretFile = o.OAuth2SecretFile oauthConfig.ClientID = o.OAuth2ClientID + if oauthConfig.ClientID == "" { + oauthConfig.ClientID, _ = os.LookupEnv("OAUTH2_CLIENT_ID") + } oauthConfig.ClientSecret = o.OAuth2ClientSecret + if oauthConfig.ClientSecret == "" { + oauthConfig.ClientSecret, _ = os.LookupEnv("OAUTH2_CLIENT_SECRET") + } oauthConfig.ClientIDFile = o.OAuth2ClientIDFile oauthConfig.ClientSecretFile = o.OAuth2ClientSecretFile oauthConfig.CallbackPath = o.OAuth2CallbackPath @@ -1672,11 +1678,15 @@ func run(o Options, sig chan os.Signal, idleConnsCH chan struct{}) error { ) if o.OIDCSecretsFile != "" { + oidcClientId, _ := os.LookupEnv("OIDC_CLIENT_ID") + oidcClientSecret, _ := os.LookupEnv("OIDC_CLIENT_SECRET") opts := auth.OidcOptions{ - CookieValidity: o.OIDCCookieValidity, - Timeout: o.OIDCDistributedClaimsTimeout, - MaxIdleConns: o.IdleConnectionsPerHost, - Tracer: tracer, + CookieValidity: o.OIDCCookieValidity, + Timeout: o.OIDCDistributedClaimsTimeout, + MaxIdleConns: o.IdleConnectionsPerHost, + Tracer: tracer, + OidcClientId: oidcClientId, + OidcClientSecret: oidcClientSecret, } o.CustomFilters = append(o.CustomFilters, From f2a7ef0d302dbe879175272e0904ec8643cc00c1 Mon Sep 17 00:00:00 2001 From: Carl Zhou Date: Wed, 4 Sep 2024 14:19:12 -0700 Subject: [PATCH 05/10] add documentation for client and secret env vars Signed-off-by: Carl Zhou --- config/config.go | 4 ++-- docs/reference/filters.md | 42 +++++++++++++++++++-------------------- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/config/config.go b/config/config.go index d9cb8d25c5..efec40e43b 100644 --- a/config/config.go +++ b/config/config.go @@ -481,8 +481,8 @@ func NewConfig() *Config { flag.StringVar(&cfg.Oauth2RevokeTokenURL, "oauth2-revoke-token-url", "", "the url where the access and refresh tokens can be revoked when logging out") flag.StringVar(&cfg.Oauth2TokeninfoURL, "oauth2-tokeninfo-url", "", "sets the default tokeninfo URL to query information about an incoming OAuth2 token in oauth2Tokeninfo filters") flag.StringVar(&cfg.Oauth2SecretFile, "oauth2-secret-file", "", "sets the filename with the encryption key for the authentication cookie and grant flow state stored in secrets registry") - flag.StringVar(&cfg.Oauth2ClientID, "oauth2-client-id", "", "sets the OAuth2 client id of the current service, used to exchange the access code") - flag.StringVar(&cfg.Oauth2ClientSecret, "oauth2-client-secret", "", "sets the OAuth2 client secret associated with the oauth2-client-id, used to exchange the access code") + flag.StringVar(&cfg.Oauth2ClientID, "oauth2-client-id", "", "sets the OAuth2 client id of the current service, used to exchange the access code. Can be an empty string if env variable OAUTH2_CLIENT_ID is set.") + flag.StringVar(&cfg.Oauth2ClientSecret, "oauth2-client-secret", "", "sets the OAuth2 client secret associated with the oauth2-client-id, used to exchange the access code. Can be an empty string if env variable OAUTH2_CLIENT_SECRET is set.") flag.StringVar(&cfg.Oauth2ClientIDFile, "oauth2-client-id-file", "", "sets the path of the file containing the OAuth2 client id of the current service, used to exchange the access code. "+ "File name may contain {host} placeholder which will be replaced by the request host") flag.StringVar(&cfg.Oauth2ClientSecretFile, "oauth2-client-secret-file", "", "sets the path of the file containing the OAuth2 client secret associated with the oauth2-client-id, used to exchange the access code. "+ diff --git a/docs/reference/filters.md b/docs/reference/filters.md index c203d0919a..9cad952364 100644 --- a/docs/reference/filters.md +++ b/docs/reference/filters.md @@ -1736,25 +1736,25 @@ single_page_app: Skipper arguments: -| Argument | Required? | Description | -| -------- | --------- | ----------- | -| `-enable-oauth2-grant-flow` | **yes** | toggle flag to enable the `oauthGrant()` filter. Must be set if you use the filter in routes. Example: `-enable-oauth2-grant-flow` | -| `-oauth2-auth-url` | **yes** | URL of the OAuth2 provider's authorize endpoint. Example: `-oauth2-auth-url=https://identity.example.com/oauth2/authorize` | -| `-oauth2-token-url` | **yes** | URL of the OAuth2 provider's token endpoint. Example: `-oauth2-token-url=https://identity.example.com/oauth2/token` | -| `-oauth2-tokeninfo-url` | **yes** | URL of the OAuth2 provider's tokeninfo endpoint. Example: `-oauth2-tokeninfo-url=https://identity.example.com/oauth2/tokeninfo` | -| `-oauth2-secret-file` | **yes** | path to the file containing the secret for encrypting and decrypting the grant token cookie (the secret can be anything). Example: `-oauth2-secret-file=/path/to/secret` | -| `-oauth2-client-id-file` | conditional | path to the file containing the OAuth2 client ID. Required if you have not set `-oauth2-client-id`. Example: `-oauth2-client-id-file=/path/to/client_id` | -| `-oauth2-client-secret-file` | conditional | path to the file containing the OAuth2 client secret. Required if you have not set `-oauth2-client-secret`. Example: `-oauth2-client-secret-file=/path/to/client_secret` | -| `-oauth2-client-id` | conditional | OAuth2 client ID for authenticating with your OAuth2 provider. Required if you have not set `-oauth2-client-id-file`. Example: `-oauth2-client-id=myclientid` | -| `-oauth2-client-secret` | conditional | OAuth2 client secret for authenticating with your OAuth2 provider. Required if you have not set `-oauth2-client-secret-file`. Example: `-oauth2-client-secret=myclientsecret` | -| `-credentials-update-interval` | no | the time interval for updating client id and client secret from files. Example: `-credentials-update-interval=30s` | -| `-oauth2-access-token-header-name` | no | the name of the request header where the user's bearer token should be set. Example: `-oauth2-access-token-header-name=X-Grant-Authorization` | -| `-oauth2-grant-tokeninfo-keys` | no | comma separated list of keys to preserve in OAuth2 Grant Flow tokeninfo. Default: empty, preserves all tokeninfo keys. Example: `-oauth2-grant-tokeninfo-keys=scope,realm,expires_in` | -| `-oauth2-auth-url-parameters` | no | any additional URL query parameters to set for the OAuth2 provider's authorize and token endpoint calls. Example: `-oauth2-auth-url-parameters=key1=foo,key2=bar` | -| `-oauth2-callback-path` | no | path of the Skipper route containing the `grantCallback()` filter for accepting an authorization code and using it to get an access token. Example: `-oauth2-callback-path=/oauth/callback` | -| `-oauth2-token-cookie-name` | no | the name of the cookie where the access tokens should be stored in encrypted form. Default: `oauth-grant`. Example: `-oauth2-token-cookie-name=SESSION` | -| `-oauth2-token-cookie-remove-subdomains` | no | the number of subdomains to remove from the callback request hostname to obtain token cookie domain. Default: `1`. Example: `-oauth2-token-cookie-remove-subdomains=0` | -| `-oauth2-grant-insecure` | no | omits `Secure` attribute of the token cookie and uses `http` scheme for callback url. Default: `false` | +| Argument | Required? | Description | +| -------- | --------- |----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `-enable-oauth2-grant-flow` | **yes** | toggle flag to enable the `oauthGrant()` filter. Must be set if you use the filter in routes. Example: `-enable-oauth2-grant-flow` | +| `-oauth2-auth-url` | **yes** | URL of the OAuth2 provider's authorize endpoint. Example: `-oauth2-auth-url=https://identity.example.com/oauth2/authorize` | +| `-oauth2-token-url` | **yes** | URL of the OAuth2 provider's token endpoint. Example: `-oauth2-token-url=https://identity.example.com/oauth2/token` | +| `-oauth2-tokeninfo-url` | **yes** | URL of the OAuth2 provider's tokeninfo endpoint. Example: `-oauth2-tokeninfo-url=https://identity.example.com/oauth2/tokeninfo` | +| `-oauth2-secret-file` | **yes** | path to the file containing the secret for encrypting and decrypting the grant token cookie (the secret can be anything). Example: `-oauth2-secret-file=/path/to/secret` | +| `-oauth2-client-id-file` | conditional | path to the file containing the OAuth2 client ID. Required if you have not set `-oauth2-client-id`. Example: `-oauth2-client-id-file=/path/to/client_id` | +| `-oauth2-client-secret-file` | conditional | path to the file containing the OAuth2 client secret. Required if you have not set `-oauth2-client-secret`. Example: `-oauth2-client-secret-file=/path/to/client_secret` | +| `-oauth2-client-id` | conditional | OAuth2 client ID for authenticating with your OAuth2 provider. Required if you have not set `-oauth2-client-id-file` or `OAUTH2_CLIENT_ID` env variable. Example: `-oauth2-client-id=myclientid` | +| `-oauth2-client-secret` | conditional | OAuth2 client secret for authenticating with your OAuth2 provider. Required if you have not set `-oauth2-client-secret-file` or `OAUTH2_CLIENT_SECRET` env variable. Example: `-oauth2-client-secret=myclientsecret` | +| `-credentials-update-interval` | no | the time interval for updating client id and client secret from files. Example: `-credentials-update-interval=30s` | +| `-oauth2-access-token-header-name` | no | the name of the request header where the user's bearer token should be set. Example: `-oauth2-access-token-header-name=X-Grant-Authorization` | +| `-oauth2-grant-tokeninfo-keys` | no | comma separated list of keys to preserve in OAuth2 Grant Flow tokeninfo. Default: empty, preserves all tokeninfo keys. Example: `-oauth2-grant-tokeninfo-keys=scope,realm,expires_in` | +| `-oauth2-auth-url-parameters` | no | any additional URL query parameters to set for the OAuth2 provider's authorize and token endpoint calls. Example: `-oauth2-auth-url-parameters=key1=foo,key2=bar` | +| `-oauth2-callback-path` | no | path of the Skipper route containing the `grantCallback()` filter for accepting an authorization code and using it to get an access token. Example: `-oauth2-callback-path=/oauth/callback` | +| `-oauth2-token-cookie-name` | no | the name of the cookie where the access tokens should be stored in encrypted form. Default: `oauth-grant`. Example: `-oauth2-token-cookie-name=SESSION` | +| `-oauth2-token-cookie-remove-subdomains` | no | the number of subdomains to remove from the callback request hostname to obtain token cookie domain. Default: `1`. Example: `-oauth2-token-cookie-remove-subdomains=0` | +| `-oauth2-grant-insecure` | no | omits `Secure` attribute of the token cookie and uses `http` scheme for callback url. Default: `false` | #### grantCallback @@ -1835,8 +1835,8 @@ oauthOidcUserInfo("https://oidc-provider.example.com", "client_id", "client_secr The filter needs the following parameters: * **OpenID Connect Provider URL** For example Google OpenID Connect is available on `https://accounts.google.com` -* **Client ID** This value is obtained from the provider upon registration of the application. -* **Client Secret** Also obtained from the provider +* **Client ID** This value is obtained from the provider upon registration of the application. Can be also passed in via ```OIDC_CLIENT_ID``` env variable. +* **Client Secret** Also obtained from the provider. Can be also passed in via ```OIDC_CLIENT_SECRET``` env variable. * **Callback URL** The entire path to the callback from the provider on which the token will be received. It can be any value which is a subpath on which the filter is applied. * **Scopes** The OpenID scopes separated by spaces which need to be specified when requesting the token from the provider. From 8d032c6fd77c4d09d188249b56ed80e66fe78ff0 Mon Sep 17 00:00:00 2001 From: Carl Zhou Date: Wed, 4 Sep 2024 14:24:42 -0700 Subject: [PATCH 06/10] fix wording Signed-off-by: Carl Zhou --- config/config.go | 4 ++-- docs/reference/filters.md | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/config/config.go b/config/config.go index efec40e43b..20799722e1 100644 --- a/config/config.go +++ b/config/config.go @@ -481,8 +481,8 @@ func NewConfig() *Config { flag.StringVar(&cfg.Oauth2RevokeTokenURL, "oauth2-revoke-token-url", "", "the url where the access and refresh tokens can be revoked when logging out") flag.StringVar(&cfg.Oauth2TokeninfoURL, "oauth2-tokeninfo-url", "", "sets the default tokeninfo URL to query information about an incoming OAuth2 token in oauth2Tokeninfo filters") flag.StringVar(&cfg.Oauth2SecretFile, "oauth2-secret-file", "", "sets the filename with the encryption key for the authentication cookie and grant flow state stored in secrets registry") - flag.StringVar(&cfg.Oauth2ClientID, "oauth2-client-id", "", "sets the OAuth2 client id of the current service, used to exchange the access code. Can be an empty string if env variable OAUTH2_CLIENT_ID is set.") - flag.StringVar(&cfg.Oauth2ClientSecret, "oauth2-client-secret", "", "sets the OAuth2 client secret associated with the oauth2-client-id, used to exchange the access code. Can be an empty string if env variable OAUTH2_CLIENT_SECRET is set.") + flag.StringVar(&cfg.Oauth2ClientID, "oauth2-client-id", "", "sets the OAuth2 client id of the current service, used to exchange the access code. Falls back to env variable OAUTH2_CLIENT_ID if value is empty.") + flag.StringVar(&cfg.Oauth2ClientSecret, "oauth2-client-secret", "", "sets the OAuth2 client secret associated with the oauth2-client-id, used to exchange the access code. Falls back to env variable OAUTH2_CLIENT_SECRET if value is empty.") flag.StringVar(&cfg.Oauth2ClientIDFile, "oauth2-client-id-file", "", "sets the path of the file containing the OAuth2 client id of the current service, used to exchange the access code. "+ "File name may contain {host} placeholder which will be replaced by the request host") flag.StringVar(&cfg.Oauth2ClientSecretFile, "oauth2-client-secret-file", "", "sets the path of the file containing the OAuth2 client secret associated with the oauth2-client-id, used to exchange the access code. "+ diff --git a/docs/reference/filters.md b/docs/reference/filters.md index 9cad952364..584333ba68 100644 --- a/docs/reference/filters.md +++ b/docs/reference/filters.md @@ -1835,8 +1835,8 @@ oauthOidcUserInfo("https://oidc-provider.example.com", "client_id", "client_secr The filter needs the following parameters: * **OpenID Connect Provider URL** For example Google OpenID Connect is available on `https://accounts.google.com` -* **Client ID** This value is obtained from the provider upon registration of the application. Can be also passed in via ```OIDC_CLIENT_ID``` env variable. -* **Client Secret** Also obtained from the provider. Can be also passed in via ```OIDC_CLIENT_SECRET``` env variable. +* **Client ID** This value is obtained from the provider upon registration of the application. Falls back to```OIDC_CLIENT_ID``` env variable for empty value. +* **Client Secret** Also obtained from the provider. Falls back to ```OIDC_CLIENT_SECRET``` env variable for empty value. * **Callback URL** The entire path to the callback from the provider on which the token will be received. It can be any value which is a subpath on which the filter is applied. * **Scopes** The OpenID scopes separated by spaces which need to be specified when requesting the token from the provider. From 79c9f934c646193c3502495ab7846020dead817b Mon Sep 17 00:00:00 2001 From: Carl Zhou Date: Mon, 16 Sep 2024 09:48:43 -0700 Subject: [PATCH 07/10] Add redirect_uri override headers Signed-off-by: Carl Zhou --- docs/reference/filters.md | 4 ++++ filters/auth/grantconfig.go | 17 +++++++++++++++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/docs/reference/filters.md b/docs/reference/filters.md index 584333ba68..96441164a3 100644 --- a/docs/reference/filters.md +++ b/docs/reference/filters.md @@ -1687,6 +1687,10 @@ to untrusted downstream services. The filter will inject the OAuth2 bearer token into the request headers if the flag `oauth2-access-token-header-name` is set. +The filter will substitute the base URL of redirect_uri, if "X-Skipper-Redirect-Base-Uri" header is passed in the request. +The value will be in the form of "http://host.tld" or "https://host.tld". +Otherwise, the "Host" of the request is used as the base URL of the redirect_uri. + The filter must be used in conjunction with the [grantCallback](#grantcallback) filter where the OAuth2 provider can redirect authenticated users with an authorization code. Skipper will make sure to add the `grantCallback` filter for you to your routes when diff --git a/filters/auth/grantconfig.go b/filters/auth/grantconfig.go index 97a9dd2661..9e7268ee49 100644 --- a/filters/auth/grantconfig.go +++ b/filters/auth/grantconfig.go @@ -358,6 +358,7 @@ func (c *OAuthConfig) GetAuthURLParameters(redirectURI string) []oauth2.AuthCode // RedirectURLs constructs the redirect URI based on the request and the // configured CallbackPath. +// X-Skipper-Redirect-Host header overrides the host generated in the redirect URL func (c *OAuthConfig) RedirectURLs(req *http.Request) (redirect, original string) { u := *req.URL @@ -367,10 +368,22 @@ func (c *OAuthConfig) RedirectURLs(req *http.Request) (redirect, original string u.Scheme = "https" } - u.Host = req.Host - original = u.String() + redirectBaseOverride := req.Header.Get("X-Skipper-Redirect-Base-Uri") + if redirectBaseOverride != "" { + u, err := url.Parse(redirectBaseOverride) + if err == nil { + redirect = (&url.URL{ + Scheme: u.Scheme, + Host: u.Host, + Path: c.CallbackPath, + }).String() + return + } + } + + u.Host = req.Host redirect = (&url.URL{ Scheme: u.Scheme, Host: u.Host, From 237a43386e63bfc8edf6455373fe2e213480ae08 Mon Sep 17 00:00:00 2001 From: Carl Zhou Date: Wed, 18 Sep 2024 13:49:56 -0700 Subject: [PATCH 08/10] Add Custom Cookie Name for OIDC filter Signed-off-by: Carl Zhou --- docs/reference/filters.md | 1 + filters/auth/oidc.go | 36 +++++++++++++++++++++--------------- filters/auth/oidc_test.go | 17 +++++++++++++++++ 3 files changed, 39 insertions(+), 15 deletions(-) diff --git a/docs/reference/filters.md b/docs/reference/filters.md index 96441164a3..3664a8dc75 100644 --- a/docs/reference/filters.md +++ b/docs/reference/filters.md @@ -1848,6 +1848,7 @@ The filter needs the following parameters: * **Auth Code Options** (optional) Passes key/value parameters to a provider's authorization endpoint. The value can be dynamically set by a query parameter with the same key name if the placeholder `skipper-request-query` is used. * **Upstream Headers** (optional) The upstream endpoint will receive these headers which values are parsed from the OIDC information. The header definition can be one or more header-query pairs, space delimited. The query syntax is [GJSON](https://github.com/tidwall/gjson/blob/master/SYNTAX.md). * **SubdomainsToRemove** (optional, default "1") Configures number of subdomains to remove from the request hostname to derive OIDC cookie domain. By default one subdomain is removed, e.g. for the www.example.com request hostname the OIDC cookie domain will be example.com (to support SSO for all subdomains of the example.com). Configure "0" to use the same hostname. Note that value is a string. +* **Custom Cookie Name** (optional) Defines a constant cookie name generated by the OIDC filter. By default the cookie name is SkipperOauthOIDC{hash}, where {hash} is a generated value. #### oauthOidcAnyClaims diff --git a/filters/auth/oidc.go b/filters/auth/oidc.go index 6d4e502b7e..88c952c52c 100644 --- a/filters/auth/oidc.go +++ b/filters/auth/oidc.go @@ -85,6 +85,7 @@ const ( paramAuthCodeOpts paramUpstrHeaders paramSubdomainsToRemove + paramCookieName ) type OidcOptions struct { @@ -201,22 +202,27 @@ func (s *tokenOidcSpec) CreateFilter(args []interface{}) (filters.Filter, error) return nil, filters.ErrInvalidFilterParameters } - h := sha256.New() - for i, s := range sargs { - // CallbackURL not taken into account for cookie hashing for additional sub path ingresses - if i == paramCallbackURL { - continue - } - // SubdomainsToRemove not taken into account for cookie hashing for additional sub-domain ingresses - if i == paramSubdomainsToRemove { - continue + var cookieName string + if len(sargs) > paramCookieName && sargs[paramCookieName] != "" { + cookieName = sargs[paramCookieName] + } else { + h := sha256.New() + for i, s := range sargs { + // CallbackURL not taken into account for cookie hashing for additional sub path ingresses + if i == paramCallbackURL { + continue + } + // SubdomainsToRemove not taken into account for cookie hashing for additional sub-domain ingresses + if i == paramSubdomainsToRemove { + continue + } + h.Write([]byte(s)) } - h.Write([]byte(s)) + byteSlice := h.Sum(nil) + sargsHash := fmt.Sprintf("%x", byteSlice)[:8] + cookieName = oauthOidcCookieName + sargsHash + "-" } - byteSlice := h.Sum(nil) - sargsHash := fmt.Sprintf("%x", byteSlice)[:8] - generatedCookieName := oauthOidcCookieName + sargsHash + "-" - log.Debugf("Generated Cookie Name: %s", generatedCookieName) + log.Debugf("Cookie Name: %s", cookieName) redirectURL, err := url.Parse(sargs[paramCallbackURL]) if err != nil || sargs[paramCallbackURL] == "" { @@ -269,7 +275,7 @@ func (s *tokenOidcSpec) CreateFilter(args []interface{}) (filters.Filter, error) ClientID: oidcClientId, }), validity: validity, - cookiename: generatedCookieName, + cookiename: cookieName, encrypter: encrypter, compressor: newDeflatePoolCompressor(flate.BestCompression), subdomainsToRemove: subdomainsToRemove, diff --git a/filters/auth/oidc_test.go b/filters/auth/oidc_test.go index 7198bd223d..771fde01fe 100644 --- a/filters/auth/oidc_test.go +++ b/filters/auth/oidc_test.go @@ -711,6 +711,7 @@ func TestOIDCSetup(t *testing.T) { expectCookieDomain string filterCookies []string extraClaims jwt.MapClaims + expectCookieName string }{{ msg: "wrong provider", filter: `oauthOidcAnyClaims("no url", "", "", "{{ .RedirectURL }}", "", "")`, @@ -848,6 +849,16 @@ func TestOIDCSetup(t *testing.T) { expected: 200, expectCookieDomain: "bar.foo.skipper.test", filterCookies: []string{"badheader", "malformed"}, + }, { + msg: "custom cookie name", + filter: `oauthOidcUserInfo("{{ .OIDCServerURL }}", "valid-client", "mysec", "{{ .RedirectURL }}", "", "", "", "", "", "custom-cookie")`, + expected: 200, + expectCookieName: "custom-cookie", + }, { + msg: "default cookie name when not specified", + filter: `oauthOidcUserInfo("{{ .OIDCServerURL }}", "valid-client", "mysec", "{{ .RedirectURL }}", "", "")`, + expected: 200, + expectCookieName: "skipperOauthOidc", }} { t.Run(tc.msg, func(t *testing.T) { backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -976,6 +987,12 @@ func TestOIDCSetup(t *testing.T) { assert.True(t, c.Value == "") } } + + // Check for custom cookie name + if tc.expectCookieName != "" { + assert.True(t, strings.HasPrefix(c.Name, tc.expectCookieName), + "Cookie name should start with %s, but got %s", tc.expectCookieName, c.Name) + } } } }) From 062a6c9ae9075b7e4690752bc5e6acfd298f64b8 Mon Sep 17 00:00:00 2001 From: Carl Zhou Date: Wed, 18 Sep 2024 13:55:19 -0700 Subject: [PATCH 09/10] revert grantconfig code to master Signed-off-by: Carl Zhou --- docs/reference/filters.md | 4 ---- filters/auth/grantconfig.go | 17 ++--------------- 2 files changed, 2 insertions(+), 19 deletions(-) diff --git a/docs/reference/filters.md b/docs/reference/filters.md index 3664a8dc75..03b0706987 100644 --- a/docs/reference/filters.md +++ b/docs/reference/filters.md @@ -1687,10 +1687,6 @@ to untrusted downstream services. The filter will inject the OAuth2 bearer token into the request headers if the flag `oauth2-access-token-header-name` is set. -The filter will substitute the base URL of redirect_uri, if "X-Skipper-Redirect-Base-Uri" header is passed in the request. -The value will be in the form of "http://host.tld" or "https://host.tld". -Otherwise, the "Host" of the request is used as the base URL of the redirect_uri. - The filter must be used in conjunction with the [grantCallback](#grantcallback) filter where the OAuth2 provider can redirect authenticated users with an authorization code. Skipper will make sure to add the `grantCallback` filter for you to your routes when diff --git a/filters/auth/grantconfig.go b/filters/auth/grantconfig.go index 9e7268ee49..97a9dd2661 100644 --- a/filters/auth/grantconfig.go +++ b/filters/auth/grantconfig.go @@ -358,7 +358,6 @@ func (c *OAuthConfig) GetAuthURLParameters(redirectURI string) []oauth2.AuthCode // RedirectURLs constructs the redirect URI based on the request and the // configured CallbackPath. -// X-Skipper-Redirect-Host header overrides the host generated in the redirect URL func (c *OAuthConfig) RedirectURLs(req *http.Request) (redirect, original string) { u := *req.URL @@ -368,22 +367,10 @@ func (c *OAuthConfig) RedirectURLs(req *http.Request) (redirect, original string u.Scheme = "https" } - original = u.String() + u.Host = req.Host - redirectBaseOverride := req.Header.Get("X-Skipper-Redirect-Base-Uri") - if redirectBaseOverride != "" { - u, err := url.Parse(redirectBaseOverride) - if err == nil { - redirect = (&url.URL{ - Scheme: u.Scheme, - Host: u.Host, - Path: c.CallbackPath, - }).String() - return - } - } + original = u.String() - u.Host = req.Host redirect = (&url.URL{ Scheme: u.Scheme, Host: u.Host, From d31828b875a4dd1108788808a123e9a2e5060d99 Mon Sep 17 00:00:00 2001 From: Carl Zhou Date: Wed, 18 Sep 2024 14:42:48 -0700 Subject: [PATCH 10/10] fix capitalization Signed-off-by: Carl Zhou --- docs/reference/filters.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/reference/filters.md b/docs/reference/filters.md index 03b0706987..9bd4af47e3 100644 --- a/docs/reference/filters.md +++ b/docs/reference/filters.md @@ -1844,7 +1844,7 @@ The filter needs the following parameters: * **Auth Code Options** (optional) Passes key/value parameters to a provider's authorization endpoint. The value can be dynamically set by a query parameter with the same key name if the placeholder `skipper-request-query` is used. * **Upstream Headers** (optional) The upstream endpoint will receive these headers which values are parsed from the OIDC information. The header definition can be one or more header-query pairs, space delimited. The query syntax is [GJSON](https://github.com/tidwall/gjson/blob/master/SYNTAX.md). * **SubdomainsToRemove** (optional, default "1") Configures number of subdomains to remove from the request hostname to derive OIDC cookie domain. By default one subdomain is removed, e.g. for the www.example.com request hostname the OIDC cookie domain will be example.com (to support SSO for all subdomains of the example.com). Configure "0" to use the same hostname. Note that value is a string. -* **Custom Cookie Name** (optional) Defines a constant cookie name generated by the OIDC filter. By default the cookie name is SkipperOauthOIDC{hash}, where {hash} is a generated value. +* **Custom Cookie Name** (optional) Defines a constant cookie name generated by the OIDC filter. By default the cookie name is SkipperOauthOidc{hash}, where {hash} is a generated value. #### oauthOidcAnyClaims