From 9dd90426a6e3d36d0d9ab1e1af7c8e30d994db0b Mon Sep 17 00:00:00 2001 From: p53 Date: Wed, 1 May 2024 01:17:38 +0200 Subject: [PATCH] Add possibility to forward query params to idp (#454) --- pkg/apperrors/apperrors.go | 1 + pkg/config/core/core.go | 1 + pkg/google/config/config.go | 7 +++++++ pkg/keycloak/config/config.go | 7 +++++++ pkg/keycloak/proxy/handlers.go | 23 +++++++++++++++++++++++ pkg/keycloak/proxy/misc.go | 19 +++++++++++++++++++ pkg/keycloak/proxy/server.go | 2 ++ pkg/proxy/cli.go | 8 ++++++++ 8 files changed, 68 insertions(+) diff --git a/pkg/apperrors/apperrors.go b/pkg/apperrors/apperrors.go index 72817543..236d02de 100644 --- a/pkg/apperrors/apperrors.go +++ b/pkg/apperrors/apperrors.go @@ -34,6 +34,7 @@ var ( ErrPKCEWithCodeOnly = errors.New("pkce can be enabled only with no-redirect=false") ErrPKCECodeCreation = errors.New("creation of code verifier failed") ErrPKCECookieEmpty = errors.New("seems that pkce code verifier cookie value is empty string") + ErrQueryParamValueMismatch = errors.New("query param value is not allowed") ErrSessionExpiredVerifyOff = errors.New("the session has expired and verification switch off") ErrSessionExpiredRefreshOff = errors.New("session expired and access token refreshing is disabled") diff --git a/pkg/config/core/core.go b/pkg/config/core/core.go index 7cb48bd8..80a4423a 100644 --- a/pkg/config/core/core.go +++ b/pkg/config/core/core.go @@ -25,6 +25,7 @@ type Configs interface { GetHeaders() map[string]string GetMatchClaims() map[string]string GetTags() map[string]string + GetAllowedQueryParams() map[string]string } type CommonConfig struct{} diff --git a/pkg/google/config/config.go b/pkg/google/config/config.go index a430b6eb..b850c997 100644 --- a/pkg/google/config/config.go +++ b/pkg/google/config/config.go @@ -94,6 +94,8 @@ type Config struct { ResponseHeaders map[string]string `json:"response-headers" usage:"custom headers to added to the http response key=value" yaml:"response-headers"` // CustomHTTPMethods is a list of additional non-standard http methods. If additional method is required it has to explicitly allowed at resource allowed method definition. CustomHTTPMethods []string `json:"custom-http-methods" usage:"list of additional non-standard http methods" yaml:"custom-http-methods"` + // Allowed Query Params sent to IDP + AllowedQueryParams map[string]string `json:"allowed-query-params" usage:"allowed query params, sent to IDP key=optional value" yaml:"allowed-query-params"` // EnableSelfSignedTLS indicates we should create a self-signed ceritificate for the service EnabledSelfSignedTLS bool `env:"ENABLE_SELF_SIGNED_TLS" json:"enable-self-signed-tls" usage:"create self signed certificates for the proxy" yaml:"enable-self-signed-tls"` @@ -332,6 +334,7 @@ func NewDefaultConfig() *Config { EnableTokenHeader: true, HTTPOnlyCookie: true, Headers: make(map[string]string), + AllowedQueryParams: make(map[string]string), LetsEncryptCacheDir: "./cache/", MatchClaims: make(map[string]string), MaxIdleConns: 100, @@ -390,6 +393,10 @@ func (r *Config) GetTags() map[string]string { return r.Tags } +func (r *Config) GetAllowedQueryParams() map[string]string { + return r.AllowedQueryParams +} + // readConfigFile reads and parses the configuration file func (r *Config) ReadConfigFile(filename string) error { content, err := os.ReadFile(filename) diff --git a/pkg/keycloak/config/config.go b/pkg/keycloak/config/config.go index 9b2873b2..2bc1aa88 100644 --- a/pkg/keycloak/config/config.go +++ b/pkg/keycloak/config/config.go @@ -101,6 +101,8 @@ type Config struct { ResponseHeaders map[string]string `json:"response-headers" usage:"custom headers to added to the http response key=value" yaml:"response-headers"` // CustomHTTPMethods is a list of additional non-standard http methods. If additional method is required it has to explicitly allowed at resource allowed method definition. CustomHTTPMethods []string `json:"custom-http-methods" usage:"list of additional non-standard http methods" yaml:"custom-http-methods"` + // Allowed Query Params sent to IDP + AllowedQueryParams map[string]string `json:"allowed-query-params" usage:"allowed query params, sent to IDP key=optional value" yaml:"allowed-query-params"` // EnableSelfSignedTLS indicates we should create a self-signed ceritificate for the service EnabledSelfSignedTLS bool `env:"ENABLE_SELF_SIGNED_TLS" json:"enable-self-signed-tls" usage:"create self signed certificates for the proxy" yaml:"enable-self-signed-tls"` @@ -348,6 +350,7 @@ func NewDefaultConfig() *Config { EnableIDPSessionCheck: true, HTTPOnlyCookie: true, Headers: make(map[string]string), + AllowedQueryParams: make(map[string]string), LetsEncryptCacheDir: "./cache/", MatchClaims: make(map[string]string), MaxIdleConns: 100, @@ -406,6 +409,10 @@ func (r *Config) GetTags() map[string]string { return r.Tags } +func (r *Config) GetAllowedQueryParams() map[string]string { + return r.AllowedQueryParams +} + // readConfigFile reads and parses the configuration file func (r *Config) ReadConfigFile(filename string) error { content, err := os.ReadFile(filename) diff --git a/pkg/keycloak/proxy/handlers.go b/pkg/keycloak/proxy/handlers.go index 37cc1c4d..95fe0be4 100644 --- a/pkg/keycloak/proxy/handlers.go +++ b/pkg/keycloak/proxy/handlers.go @@ -106,6 +106,8 @@ func getRedirectionURL( } // oauthAuthorizationHandler is responsible for performing the redirection to oauth provider +// +//nolint:cyclop func oauthAuthorizationHandler( logger *zap.Logger, skipTokenVerification bool, @@ -116,6 +118,7 @@ func oauthAuthorizationHandler( newOAuth2Config func(redirectionURL string) *oauth2.Config, getRedirectionURL func(wrt http.ResponseWriter, req *http.Request) string, customSignInPage func(wrt http.ResponseWriter, authURL string), + allowedQueryParams map[string]string, ) func(wrt http.ResponseWriter, req *http.Request) { return func(wrt http.ResponseWriter, req *http.Request) { if skipTokenVerification { @@ -161,6 +164,26 @@ func oauthAuthorizationHandler( cookManager.DropPKCECookie(wrt, codeVerifier) } + if len(allowedQueryParams) > 0 { + for key, val := range allowedQueryParams { + if param := req.URL.Query().Get(key); param != "" { + if val != "" { + if val != param { + logger.Error( + apperrors.ErrQueryParamValueMismatch.Error(), + zap.String("param", key), + ) + return + } + } + authCodeOptions = append( + authCodeOptions, + oauth2.SetAuthURLParam(key, param), + ) + } + } + } + authURL := conf.AuthCodeURL( req.URL.Query().Get("state"), authCodeOptions..., diff --git a/pkg/keycloak/proxy/misc.go b/pkg/keycloak/proxy/misc.go index f9c24aef..a6e4b540 100644 --- a/pkg/keycloak/proxy/misc.go +++ b/pkg/keycloak/proxy/misc.go @@ -171,6 +171,8 @@ func WithOAuthURI(baseURI string, oauthURI string) func(uri string) string { } // redirectToAuthorization redirects the user to authorization handler +// +//nolint:cyclop func redirectToAuthorization( logger *zap.Logger, noRedirects bool, @@ -179,6 +181,7 @@ func redirectToAuthorization( noProxy bool, baseURI string, oAuthURI string, + allowedQueryParams map[string]string, ) func(wrt http.ResponseWriter, req *http.Request) context.Context { return func(wrt http.ResponseWriter, req *http.Request) context.Context { if noRedirects { @@ -190,6 +193,22 @@ func redirectToAuthorization( uuid := cookManager.DropStateParameterCookie(req, wrt) authQuery := fmt.Sprintf("?state=%s", uuid) + if len(allowedQueryParams) > 0 { + query := "" + for key, val := range allowedQueryParams { + if param := req.URL.Query().Get(key); param != "" { + if val != "" { + if val != param { + wrt.WriteHeader(http.StatusForbidden) + return revokeProxy(logger, req) + } + } + query += fmt.Sprintf("&%s=%s", key, param) + } + } + authQuery += query + } + // step: if verification is switched off, we can't authorization if skipTokenVerification { logger.Error( diff --git a/pkg/keycloak/proxy/server.go b/pkg/keycloak/proxy/server.go index 3d7a225f..502041aa 100644 --- a/pkg/keycloak/proxy/server.go +++ b/pkg/keycloak/proxy/server.go @@ -365,6 +365,7 @@ func (r *OauthProxy) CreateReverseProxy() error { r.Config.NoProxy, r.Config.BaseURI, r.Config.OAuthURI, + r.Config.AllowedQueryParams, ) if r.Config.EnableHmac { @@ -532,6 +533,7 @@ func (r *OauthProxy) CreateReverseProxy() error { r.newOAuth2Config, r.getRedirectionURL, r.customSignInPage, + r.Config.AllowedQueryParams, ) // step: add the routing for oauth diff --git a/pkg/proxy/cli.go b/pkg/proxy/cli.go index 2c350e0d..3a3269f7 100644 --- a/pkg/proxy/cli.go +++ b/pkg/proxy/cli.go @@ -240,6 +240,14 @@ func parseCLIOptions(cliCtx *cli.Context, config core.Configs) error { utils.MergeMaps(config.GetHeaders(), headers) } + if cliCtx.IsSet("allowed-query-params") { + headers, err := utils.DecodeKeyPairs(cliCtx.StringSlice("allowed-query-params")) + if err != nil { + return err + } + utils.MergeMaps(config.GetAllowedQueryParams(), headers) + } + if cliCtx.IsSet("resources") { for _, x := range cliCtx.StringSlice("resources") { resource, err := authorization.NewResource().Parse(x)