diff --git a/CHANGELOG.md b/CHANGELOG.md index 46171109..c671a650 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ - [#2269](https://github.com/oauth2-proxy/oauth2-proxy/pull/2269) Added Azure China (and other air gaped cloud) support (@mblaschke) - [#2237](https://github.com/oauth2-proxy/oauth2-proxy/pull/2237) adds an option to append CA certificates (@emsixteeen) - [#2128](https://github.com/oauth2-proxy/oauth2-proxy/pull/2128) Update dependencies (@vllvll) +- [#2239](https://github.com/oauth2-proxy/oauth2-proxy/pull/2312) Add possibility to encode the state param as UrlEncodedBase64 (@brezinajn) - [#2274](https://github.com/oauth2-proxy/oauth2-proxy/pull/2274) Upgrade golang.org/x/net to v0.17.0 (@pierluigilenoci) - [#2278](https://github.com/oauth2-proxy/oauth2-proxy/pull/2278) Improve the Nginx auth_request example (@akunzai) - [#2282](https://github.com/oauth2-proxy/oauth2-proxy/pull/2282) Fixed checking Google Groups membership using Google Application Credentials (@kvanzuijlen) diff --git a/oauthproxy.go b/oauthproxy.go index 5f6f8a58..964039ba 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -3,6 +3,7 @@ package main import ( "context" "embed" + "encoding/base64" "encoding/json" "errors" "fmt" @@ -112,6 +113,7 @@ type OAuthProxy struct { redirectValidator redirect.Validator appDirector redirect.AppDirector + encodeState bool AuditClient *audit.Client } @@ -253,6 +255,7 @@ func NewOAuthProxy(opts *options.Options, validator func(string) bool) (*OAuthPr upstreamProxy: upstreamProxy, redirectValidator: redirectValidator, appDirector: appDirector, + encodeState: opts.EncodeState, AuditClient: auditClient, } p.buildServeMux(opts.ProxyPrefix) @@ -820,7 +823,7 @@ func (p *OAuthProxy) doOAuthStart(rw http.ResponseWriter, req *http.Request, ove callbackRedirect := p.getOAuthRedirectURI(req) loginURL := p.provider.GetLoginURL( callbackRedirect, - encodeState(csrf.HashOAuthState(), appRedirect), + encodeState(csrf.HashOAuthState(), appRedirect, p.encodeState), csrf.HashOIDCNonce(), extraParams, ) @@ -878,7 +881,7 @@ func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) { csrf.ClearCookie(rw, req) - nonce, appRedirect, err := decodeState(req) + nonce, appRedirect, err := decodeState(req.Form.Get("state"), p.encodeState) if err != nil { logger.Errorf("Error while parsing OAuth2 state: %v", err) p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error()) @@ -1221,18 +1224,28 @@ func checkAllowedEmails(req *http.Request, s *sessionsapi.SessionState) bool { // encodedState builds the OAuth state param out of our nonce and // original application redirect -func encodeState(nonce string, redirect string) string { - return fmt.Sprintf("%v:%v", nonce, redirect) +func encodeState(nonce string, redirect string, encode bool) string { + rawString := fmt.Sprintf("%v:%v", nonce, redirect) + if encode { + return base64.RawURLEncoding.EncodeToString([]byte(rawString)) + } + return rawString } // decodeState splits the reflected OAuth state response back into // the nonce and original application redirect -func decodeState(req *http.Request) (string, string, error) { - state := strings.SplitN(req.Form.Get("state"), ":", 2) - if len(state) != 2 { +func decodeState(state string, encode bool) (string, string, error) { + toParse := state + if encode { + decoded, _ := base64.RawURLEncoding.DecodeString(state) + toParse = string(decoded) + } + + parsedState := strings.SplitN(toParse, ":", 2) + if len(parsedState) != 2 { return "", "", errors.New("invalid length") } - return state[0], state[1], nil + return parsedState[0], parsedState[1], nil } // addHeadersForProxying adds the appropriate headers the request / response for proxying diff --git a/oauthproxy_test.go b/oauthproxy_test.go index a27db6e2..772543f3 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -413,7 +413,7 @@ func (patTest *PassAccessTokenTest) getCallbackEndpoint() (httpCode int, cookie http.MethodGet, fmt.Sprintf( "/oauth2/callback?code=callback_code&state=%s", - encodeState(csrf.HashOAuthState(), "%2F"), + encodeState(csrf.HashOAuthState(), "%2F", false), ), strings.NewReader(""), ) @@ -3288,6 +3288,29 @@ func TestAuthOnlyAllowedEmailDomains(t *testing.T) { } } +func TestStateEncodesCorrectly(t *testing.T) { + state := "some_state_to_test" + nonce := "some_nonce_to_test" + + encodedResult := encodeState(nonce, state, true) + assert.Equal(t, "c29tZV9ub25jZV90b190ZXN0OnNvbWVfc3RhdGVfdG9fdGVzdA", encodedResult) + + notEncodedResult := encodeState(nonce, state, false) + assert.Equal(t, "some_nonce_to_test:some_state_to_test", notEncodedResult) +} + +func TestStateDecodesCorrectly(t *testing.T) { + nonce, redirect, _ := decodeState("c29tZV9ub25jZV90b190ZXN0OnNvbWVfc3RhdGVfdG9fdGVzdA", true) + + assert.Equal(t, "some_nonce_to_test", nonce) + assert.Equal(t, "some_state_to_test", redirect) + + nonce2, redirect2, _ := decodeState("some_nonce_to_test:some_state_to_test", false) + + assert.Equal(t, "some_nonce_to_test", nonce2) + assert.Equal(t, "some_state_to_test", redirect2) +} + func TestAuthOnlyAllowedEmails(t *testing.T) { testCases := []struct { name string diff --git a/pkg/apis/options/options.go b/pkg/apis/options/options.go index 95c45729..a1a85816 100644 --- a/pkg/apis/options/options.go +++ b/pkg/apis/options/options.go @@ -61,6 +61,7 @@ type Options struct { SSLInsecureSkipVerify bool `flag:"ssl-insecure-skip-verify" cfg:"ssl_insecure_skip_verify"` SkipAuthPreflight bool `flag:"skip-auth-preflight" cfg:"skip_auth_preflight"` ForceJSONErrors bool `flag:"force-json-errors" cfg:"force_json_errors"` + EncodeState bool `flag:"encode-state" cfg:"encode_state"` AllowQuerySemicolons bool `flag:"allow-query-semicolons" cfg:"allow_query_semicolons"` SignatureKey string `flag:"signature-key" cfg:"signature_key"` @@ -135,6 +136,7 @@ func NewFlagSet() *pflag.FlagSet { flagSet.Bool("ssl-insecure-skip-verify", false, "skip validation of certificates presented when using HTTPS providers") flagSet.Bool("skip-jwt-bearer-tokens", false, "will skip requests that have verified JWT bearer tokens (default false)") flagSet.Bool("force-json-errors", false, "will force JSON errors instead of HTTP error pages or redirects") + flagSet.Bool("encode-state", false, "will encode oauth state with base64") flagSet.Bool("allow-query-semicolons", false, "allow the use of semicolons in query args") flagSet.StringSlice("extra-jwt-issuers", []string{}, "if skip-jwt-bearer-tokens is set, a list of extra JWT issuer=audience pairs (where the issuer URL has a .well-known/openid-configuration or a .well-known/jwks.json)")