From 8083501da68e19c8633726bcab87ea680d9048a0 Mon Sep 17 00:00:00 2001 From: Brian Van Klaveren Date: Thu, 17 Jan 2019 12:49:14 -0800 Subject: [PATCH 01/18] Support JWT Bearer Token and Pass through --- docs/configuration/configuration.md | 2 + main.go | 3 + oauthproxy.go | 187 ++++++++++++++++++++++------ options.go | 63 ++++++++-- 4 files changed, 213 insertions(+), 42 deletions(-) diff --git a/docs/configuration/configuration.md b/docs/configuration/configuration.md index d631eaf..1295269 100644 --- a/docs/configuration/configuration.md +++ b/docs/configuration/configuration.md @@ -41,6 +41,7 @@ Usage of oauth2_proxy: -custom-templates-dir string: path to custom html templates -display-htpasswd-form: display username / password login form if an htpasswd file is provided (default true) -email-domain value: authenticate emails with the specified domain (may be given multiple times). Use * to authenticate any email + -extra-jwt-issuers: 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) -flush-interval: period between flushing response buffers when streaming responses (default "1s") -footer string: custom footer string. Use "-" to disable default footer. -gcp-healthchecks: will enable /liveness_check, /readiness_check, and / (with the proper user-agent) endpoints that will make it work well with GCP App Engine and GKE Ingresses (default false) @@ -89,6 +90,7 @@ Usage of oauth2_proxy: -signature-key string: GAP-Signature request signature key (algorithm:secretkey) -skip-auth-preflight: will skip authentication for OPTIONS requests -skip-auth-regex value: bypass authentication for requests path's that match (may be given multiple times) + -skip-jwt-bearer-tokens: will skip requests that have verified JWT bearer tokens -skip-oidc-discovery: bypass OIDC endpoint discovery. login-url, redeem-url and oidc-jwks-url must be configured in this case -skip-provider-button: will skip sign-in-page to directly reach the next step: oauth/start -ssl-insecure-skip-verify: skip validation of certificates presented when using HTTPS diff --git a/main.go b/main.go index a66c4fc..8c86271 100644 --- a/main.go +++ b/main.go @@ -23,6 +23,7 @@ func main() { whitelistDomains := StringArray{} upstreams := StringArray{} skipAuthRegex := StringArray{} + jwtIssuers := StringArray{} googleGroups := StringArray{} redisSentinelConnectionURLs := StringArray{} @@ -48,6 +49,8 @@ func main() { flagSet.Bool("skip-auth-preflight", false, "will skip authentication for OPTIONS requests") flagSet.Bool("ssl-insecure-skip-verify", false, "skip validation of certificates presented when using HTTPS") flagSet.Duration("flush-interval", time.Duration(1)*time.Second, "period between response flushing when streaming responses") + flagSet.Bool("skip-jwt-bearer-tokens", false, "will skip requests that have verified JWT bearer tokens") + flagSet.Var(&jwtIssuers, "extra-jwt-issuers", "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)") flagSet.Var(&emailDomains, "email-domain", "authenticate emails with the specified domain (may be given multiple times). Use * to authenticate any email") flagSet.Var(&whitelistDomains, "whitelist-domain", "allowed domains for redirection after authentication. Prefix domain with a . to allow subdomains (eg .example.com)") diff --git a/oauthproxy.go b/oauthproxy.go index ef97415..f7bbef3 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -1,6 +1,7 @@ package main import ( + "context" b64 "encoding/base64" "errors" "fmt" @@ -13,6 +14,7 @@ import ( "strings" "time" + "github.com/coreos/go-oidc" "github.com/mbland/hmacauth" "github.com/pusher/oauth2_proxy/cookie" "github.com/pusher/oauth2_proxy/logger" @@ -92,6 +94,8 @@ type OAuthProxy struct { PassAuthorization bool skipAuthRegex []string skipAuthPreflight bool + skipJwtBearerTokens bool + jwtBearerVerifiers []*oidc.IDTokenVerifier compiledRegex []*regexp.Regexp templates *template.Template Footer string @@ -206,6 +210,12 @@ func NewOAuthProxy(opts *Options, validator func(string) bool) *OAuthProxy { logger.Printf("compiled skip-auth-regex => %q", u) } + if opts.SkipJwtBearerTokens { + logger.Printf("Skipping JWT tokens from configured OIDC issuer: %q", opts.OIDCIssuerURL) + for _, issuer := range opts.ExtraJwtIssuers { + logger.Printf("Skipping JWT tokens from extra JWT issuer: %q", issuer) + } + } redirectURL := opts.redirectURL if redirectURL.Path == "" { redirectURL.Path = fmt.Sprintf("%s/callback", opts.ProxyPrefix) @@ -239,25 +249,27 @@ func NewOAuthProxy(opts *Options, validator func(string) bool) *OAuthProxy { OAuthCallbackPath: fmt.Sprintf("%s/callback", opts.ProxyPrefix), AuthOnlyPath: fmt.Sprintf("%s/auth", opts.ProxyPrefix), - ProxyPrefix: opts.ProxyPrefix, - provider: opts.provider, - sessionStore: opts.sessionStore, - serveMux: serveMux, - redirectURL: redirectURL, - whitelistDomains: opts.WhitelistDomains, - skipAuthRegex: opts.SkipAuthRegex, - skipAuthPreflight: opts.SkipAuthPreflight, - compiledRegex: opts.CompiledRegex, - SetXAuthRequest: opts.SetXAuthRequest, - PassBasicAuth: opts.PassBasicAuth, - PassUserHeaders: opts.PassUserHeaders, - BasicAuthPassword: opts.BasicAuthPassword, - PassAccessToken: opts.PassAccessToken, - SetAuthorization: opts.SetAuthorization, - PassAuthorization: opts.PassAuthorization, - SkipProviderButton: opts.SkipProviderButton, - templates: loadTemplates(opts.CustomTemplatesDir), - Footer: opts.Footer, + ProxyPrefix: opts.ProxyPrefix, + provider: opts.provider, + sessionStore: opts.sessionStore, + serveMux: serveMux, + redirectURL: redirectURL, + whitelistDomains: opts.WhitelistDomains, + skipAuthRegex: opts.SkipAuthRegex, + skipAuthPreflight: opts.SkipAuthPreflight, + skipJwtBearerTokens: opts.SkipJwtBearerTokens, + jwtBearerVerifiers: opts.jwtBearerVerifiers, + compiledRegex: opts.CompiledRegex, + SetXAuthRequest: opts.SetXAuthRequest, + PassBasicAuth: opts.PassBasicAuth, + PassUserHeaders: opts.PassUserHeaders, + BasicAuthPassword: opts.BasicAuthPassword, + PassAccessToken: opts.PassAccessToken, + SetAuthorization: opts.SetAuthorization, + PassAuthorization: opts.PassAuthorization, + SkipProviderButton: opts.SkipProviderButton, + templates: loadTemplates(opts.CustomTemplatesDir), + Footer: opts.Footer, } } @@ -693,26 +705,42 @@ func (p *OAuthProxy) Proxy(rw http.ResponseWriter, req *http.Request) { // Returns nil, ErrNeedsLogin if user needs to login. // Set-Cookie headers may be set on the response as a side-effect of calling this method. func (p *OAuthProxy) getAuthenticatedSession(rw http.ResponseWriter, req *http.Request) (*sessionsapi.SessionState, error) { + var session *sessionsapi.SessionState + var err error var saveSession, clearSession, revalidated bool + + if p.skipJwtBearerTokens && req.Header.Get("Authorization") != "" { + session, err = p.GetJwtSession(req) + if err != nil { + logger.Printf("Error validating JWT token from Authorization header: %s", err) + } + if session != nil { + saveSession = false + } + } + remoteAddr := getRemoteAddr(req) - - session, err := p.LoadCookiedSession(req) - if err != nil { - logger.Printf("Error loading cookied session: %s", err) - } - if session != nil && session.Age() > p.CookieRefresh && p.CookieRefresh != time.Duration(0) { - logger.Printf("Refreshing %s old session cookie for %s (refresh after %s)", session.Age(), session, p.CookieRefresh) - saveSession = true + if session == nil { + session, err = p.LoadCookiedSession(req) + if err != nil { + logger.Printf("Error loading cookied session: %s", err) + } + if session != nil && session.Age() > p.CookieRefresh && p.CookieRefresh != time.Duration(0) { + logger.Printf("Refreshing %s old session cookie for %s (refresh after %s)", session.Age(), session, p.CookieRefresh) + saveSession = true + } } - var ok bool - if ok, err = p.provider.RefreshSessionIfNeeded(session); err != nil { - logger.Printf("%s removing session. error refreshing access token %s %s", remoteAddr, err, session) - clearSession = true - session = nil - } else if ok { - saveSession = true - revalidated = true + if session != nil { + var ok bool + if ok, err = p.provider.RefreshSessionIfNeeded(session); err != nil { + logger.Printf("%s removing session. error refreshing access token %s %s", remoteAddr, err, session) + clearSession = true + session = nil + } else if ok { + saveSession = true + revalidated = true + } } if session != nil && session.IsExpired() { @@ -854,3 +882,92 @@ func (p *OAuthProxy) ErrorJSON(rw http.ResponseWriter, code int) { rw.Header().Set("Content-Type", applicationJSON) rw.WriteHeader(code) } + +// GetJwtSession loads a session based on a JWT token in the authorization header. +func (p *OAuthProxy) GetJwtSession(req *http.Request) (*sessionsapi.SessionState, error) { + rawBearerToken, err := p.findBearerToken(req) + if err != nil { + return nil, err + } + + ctx := context.Background() + var session *sessionsapi.SessionState + for _, verifier := range p.jwtBearerVerifiers { + bearerToken, err := verifier.Verify(ctx, rawBearerToken) + + if err != nil { + logger.Printf("failed to verify bearer token: %v", err) + continue + } + + var claims struct { + Email string `json:"email"` + Verified *bool `json:"email_verified"` + } + + if err := bearerToken.Claims(&claims); err != nil { + return nil, fmt.Errorf("failed to parse bearer token claims: %v", err) + } + + if claims.Email == "" { + return nil, fmt.Errorf("id_token did not contain an email") + } + + if claims.Verified != nil && !*claims.Verified { + return nil, fmt.Errorf("email in id_token (%s) isn't verified", claims.Email) + } + user := strings.Split(claims.Email, "@")[0] + + session = &sessionsapi.SessionState{ + AccessToken: rawBearerToken, + IDToken: rawBearerToken, + RefreshToken: "", + ExpiresOn: bearerToken.Expiry, + Email: claims.Email, + User: user, + } + return session, nil + } + return nil, fmt.Errorf("unable to verify jwt token %s", req.Header.Get("Authorization")) +} + +// findBearerToken finds a valid JWT token from the Authorization header of a given request. +func (p *OAuthProxy) findBearerToken(req *http.Request) (string, error) { + auth := req.Header.Get("Authorization") + s := strings.SplitN(auth, " ", 2) + if len(s) != 2 { + return "", fmt.Errorf("invalid authorization header %s", auth) + } + + var rawBearerToken string + if s[0] == "Bearer" { + rawBearerToken = s[1] + } else if s[0] == "Basic" { + // Check if we have a Bearer token masquerading in Basic + b, err := b64.StdEncoding.DecodeString(s[1]) + if err != nil { + return "", err + } + pair := strings.SplitN(string(b), ":", 2) + if len(pair) != 2 { + return "", fmt.Errorf("invalid format %s", b) + } + user, password := pair[0], pair[1] + + // check user, user+password, or just password for a token + jwtRegex := regexp.MustCompile(`^eyJ[a-zA-Z0-9_-]*\.eyJ[a-zA-Z0-9_-]*\.[a-zA-Z0-9_-]+$`) + if jwtRegex.MatchString(user) { + // Support blank passwords or magic `x-oauth-basic` passwords - nothing else + if password == "" || password == "x-oauth-basic" { + rawBearerToken = user + } + } else if jwtRegex.MatchString(password) { + // support passwords and ignore user + rawBearerToken = password + } + } else { + return "", fmt.Errorf("invalid authorization header %s", auth) + } + + return rawBearerToken, nil +} diff --git a/options.go b/options.go index 0460bce..331c8f6 100644 --- a/options.go +++ b/options.go @@ -61,6 +61,8 @@ type Options struct { Upstreams []string `flag:"upstream" cfg:"upstreams" env:"OAUTH2_PROXY_UPSTREAMS"` SkipAuthRegex []string `flag:"skip-auth-regex" cfg:"skip_auth_regex" env:"OAUTH2_PROXY_SKIP_AUTH_REGEX"` + SkipJwtBearerTokens bool `flag:"skip-jwt-bearer-tokens" cfg:"skip_jwt_bearer_tokens"` + ExtraJwtIssuers []string `flag:"extra-jwt-issuers" cfg:"extra_jwt_issuers"` PassBasicAuth bool `flag:"pass-basic-auth" cfg:"pass_basic_auth" env:"OAUTH2_PROXY_PASS_BASIC_AUTH"` BasicAuthPassword string `flag:"basic-auth-password" cfg:"basic_auth_password" env:"OAUTH2_PROXY_BASIC_AUTH_PASSWORD"` PassAccessToken bool `flag:"pass-access-token" cfg:"pass_access_token" env:"OAUTH2_PROXY_PASS_ACCESS_TOKEN"` @@ -110,13 +112,15 @@ type Options struct { GCPHealthChecks bool `flag:"gcp-healthchecks" cfg:"gcp_healthchecks" env:"OAUTH2_PROXY_GCP_HEALTHCHECKS"` // internal values that are set after config validation - redirectURL *url.URL - proxyURLs []*url.URL - CompiledRegex []*regexp.Regexp - provider providers.Provider - sessionStore sessionsapi.SessionStore - signatureData *SignatureData - oidcVerifier *oidc.IDTokenVerifier + redirectURL *url.URL + proxyURLs []*url.URL + CompiledRegex []*regexp.Regexp + provider providers.Provider + sessionStore sessionsapi.SessionStore + signatureData *SignatureData + oidcVerifier *oidc.IDTokenVerifier + jwtIssuers [][]string + jwtBearerVerifiers []*oidc.IDTokenVerifier } // SignatureData holds hmacauth signature hash and key @@ -244,6 +248,38 @@ func (o *Options) Validate() error { } } + if o.SkipJwtBearerTokens { + // If we are using an oidc provider, go ahead and add that provider to the list + if o.oidcVerifier != nil { + o.jwtBearerVerifiers = append(o.jwtBearerVerifiers, o.oidcVerifier) + } + // Configure extra issuers + if len(o.ExtraJwtIssuers) > 0 { + msgs = parseJwtIssuers(o, msgs) + for _, pair := range o.jwtIssuers { + issuer, audience := pair[0], pair[1] + config := &oidc.Config{ + ClientID: audience, + } + // Try as an OpenID Connect Provider first + var verifier *oidc.IDTokenVerifier + provider, err := oidc.NewProvider(context.Background(), issuer) + if err != nil { + // Try as JWKS URI + jwksURI := strings.TrimSuffix(issuer, "/") + "/.well-known/jwks.json" + _, err := http.NewRequest("GET", jwksURI, nil) + if err != nil { + return err + } + verifier = oidc.NewVerifier(issuer, oidc.NewRemoteKeySet(context.Background(), jwksURI), config) + } else { + verifier = provider.Verifier(config) + } + o.jwtBearerVerifiers = append(o.jwtBearerVerifiers, verifier) + } + } + } + o.redirectURL, msgs = parseURL(o.RedirectURL, "redirect", msgs) for _, u := range o.Upstreams { @@ -430,6 +466,19 @@ func parseSignatureKey(o *Options, msgs []string) []string { return msgs } +func parseJwtIssuers(o *Options, msgs []string) []string { + for _, jwtVerifier := range o.ExtraJwtIssuers { + components := strings.Split(jwtVerifier, "=") + if len(components) < 2 { + return append(msgs, "invalid jwt verifier uri=audience spec: "+ + jwtVerifier) + } + uri, audience := components[0], strings.Join(components[1:], "=") + o.jwtIssuers = append(o.jwtIssuers, []string{uri, audience}) + } + return msgs +} + func validateCookieName(o *Options, msgs []string) []string { cookie := &http.Cookie{Name: o.CookieName} if cookie.String() == "" { From b895f49c52ef3d37fa4ab55e656b5e279d3237d9 Mon Sep 17 00:00:00 2001 From: Brian Van Klaveren Date: Tue, 12 Feb 2019 10:32:26 -0800 Subject: [PATCH 02/18] Use idToken expiry because that's the time checked for refresh RefreshSessionIfNeeded checks the token expiry, we want to use the ID token's expiry --- providers/oidc.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/providers/oidc.go b/providers/oidc.go index 08ea082..b0d2dda 100644 --- a/providers/oidc.go +++ b/providers/oidc.go @@ -128,7 +128,7 @@ func (p *OIDCProvider) createSessionState(ctx context.Context, token *oauth2.Tok IDToken: rawIDToken, RefreshToken: token.RefreshToken, CreatedAt: time.Now(), - ExpiresOn: token.Expiry, + ExpiresOn: idToken.Expiry, Email: claims.Email, User: claims.Subject, }, nil From 8413c30c26491dc2f234f88a146fdb7f4cf7db8d Mon Sep 17 00:00:00 2001 From: Brian Van Klaveren Date: Thu, 14 Feb 2019 15:00:49 -0800 Subject: [PATCH 03/18] Update changelog with info about -skip-jwt-bearer-tokens --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ef731cb..136775f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,10 @@ ## Changes since v3.2.0 +- [#65](https://github.com/pusher/oauth2_proxy/pull/65) Improvements to authenticate requests with a JWT bearer token in the `Authorization` header via + the `-skip-jwt-bearer-token` options. + - Additional verifiers can be configured via the `-extra-jwt-issuers` flag if the JWT issuers is either an OpenID provider or has a JWKS URL + (e.g. `https://example.com/.well-known/jwks.json`). - [#180](https://github.com/pusher/outh2_proxy/pull/180) Minor refactor of core proxying path (@aeijdenberg). - [#175](https://github.com/pusher/outh2_proxy/pull/175) Bump go-oidc to v2.0.0 (@aeijdenberg). - Includes fix for potential signature checking issue when OIDC discovery is skipped. From 187960e9d884b38644aa8e015b9cec9c548c9a2e Mon Sep 17 00:00:00 2001 From: Brian Van Klaveren Date: Wed, 24 Apr 2019 08:25:29 -0700 Subject: [PATCH 04/18] Improve token pattern matching Unit tests for token discovery --- oauthproxy.go | 12 ++++----- oauthproxy_test.go | 63 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 6 deletions(-) diff --git a/oauthproxy.go b/oauthproxy.go index f7bbef3..714c7a3 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -712,7 +712,7 @@ func (p *OAuthProxy) getAuthenticatedSession(rw http.ResponseWriter, req *http.R if p.skipJwtBearerTokens && req.Header.Get("Authorization") != "" { session, err = p.GetJwtSession(req) if err != nil { - logger.Printf("Error validating JWT token from Authorization header: %s", err) + logger.Printf("Error retrieving session from token in Authorization header: %s", err) } if session != nil { saveSession = false @@ -938,9 +938,9 @@ func (p *OAuthProxy) findBearerToken(req *http.Request) (string, error) { if len(s) != 2 { return "", fmt.Errorf("invalid authorization header %s", auth) } - + jwtRegex := regexp.MustCompile(`^eyJ[a-zA-Z0-9_-]*\.eyJ[a-zA-Z0-9_-]*\.[a-zA-Z0-9_-]+$`) var rawBearerToken string - if s[0] == "Bearer" { + if s[0] == "Bearer" && jwtRegex.MatchString(s[1]) { rawBearerToken = s[1] } else if s[0] == "Basic" { // Check if we have a Bearer token masquerading in Basic @@ -955,7 +955,6 @@ func (p *OAuthProxy) findBearerToken(req *http.Request) (string, error) { user, password := pair[0], pair[1] // check user, user+password, or just password for a token - jwtRegex := regexp.MustCompile(`^eyJ[a-zA-Z0-9_-]*\.eyJ[a-zA-Z0-9_-]*\.[a-zA-Z0-9_-]+$`) if jwtRegex.MatchString(user) { // Support blank passwords or magic `x-oauth-basic` passwords - nothing else if password == "" || password == "x-oauth-basic" { @@ -965,8 +964,9 @@ func (p *OAuthProxy) findBearerToken(req *http.Request) (string, error) { // support passwords and ignore user rawBearerToken = password } - } else { - return "", fmt.Errorf("invalid authorization header %s", auth) + } + if rawBearerToken == "" { + return "", fmt.Errorf("no valid bearer token found in authorization header") } return rawBearerToken, nil diff --git a/oauthproxy_test.go b/oauthproxy_test.go index 1d09bbb..493ce72 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -3,6 +3,7 @@ package main import ( "crypto" "encoding/base64" + "fmt" "io" "io/ioutil" "net" @@ -1132,3 +1133,65 @@ func TestClearSingleCookie(t *testing.T) { assert.Equal(t, 1, len(header["Set-Cookie"]), "should have 1 set-cookie header entries") } + +func TestFindJwtBearerToken(t *testing.T) { + p := OAuthProxy{CookieName: "oauth2", CookieDomain: "abc"} + getReq := &http.Request{URL: &url.URL{Scheme: "http", Host: "example.com"}} + + validToken := "eyJfoobar.eyJfoobar.12345asdf" + var token string + + // Bearer + getReq.Header = map[string][]string{ + "Authorization": {fmt.Sprintf("Bearer %s", validToken)}, + } + + token, _ = p.findBearerToken(getReq) + assert.Equal(t, validToken, token) + + // Basic - no password + getReq.SetBasicAuth(token, "") + token, _ = p.findBearerToken(getReq) + assert.Equal(t, validToken, token) + + // Basic - sentinel password + getReq.SetBasicAuth(token, "x-oauth-basic") + token, _ = p.findBearerToken(getReq) + assert.Equal(t, validToken, token) + + // Basic - any username, password matching jwt pattern + getReq.SetBasicAuth("any-username-you-could-wish-for", token) + token, _ = p.findBearerToken(getReq) + assert.Equal(t, validToken, token) + + failures := []string{ + // Too many parts + "eyJhbGciOiJSU0EtT0FFUCIsImVuYyI6IkExMjhHQ00ifQ.dGVzdA.dGVzdA.dGVzdA.dGVzdA.dGVzdA", + // Not enough parts + "eyJhbGciOiJSU0EtT0FFUCIsImVuYyI6IkExMjhHQ00ifQ.dGVzdA.dGVzdA.dGVzdA", + // Invalid encrypted key + "eyJhbGciOiJSU0EtT0FFUCIsImVuYyI6IkExMjhHQ00ifQ.//////.dGVzdA.dGVzdA.dGVzdA", + // Invalid IV + "eyJhbGciOiJSU0EtT0FFUCIsImVuYyI6IkExMjhHQ00ifQ.dGVzdA.//////.dGVzdA.dGVzdA", + // Invalid ciphertext + "eyJhbGciOiJSU0EtT0FFUCIsImVuYyI6IkExMjhHQ00ifQ.dGVzdA.dGVzdA.//////.dGVzdA", + // Invalid tag + "eyJhbGciOiJSU0EtT0FFUCIsImVuYyI6IkExMjhHQ00ifQ.dGVzdA.dGVzdA.dGVzdA.//////", + // Invalid header + "W10.dGVzdA.dGVzdA.dGVzdA.dGVzdA", + // Invalid header + "######.dGVzdA.dGVzdA.dGVzdA.dGVzdA", + // Missing alc/enc params + "e30.dGVzdA.dGVzdA.dGVzdA.dGVzdA", + } + + for _, failure := range failures { + getReq.Header = map[string][]string{ + "Authorization": {fmt.Sprintf("Bearer %s", failure)}, + } + _, err := p.findBearerToken(getReq) + assert.Error(t, err) + } + + fmt.Printf("%s", token) +} From 69cb34a04e6d00d9dca03940cddd78e99fcef4dd Mon Sep 17 00:00:00 2001 From: Brian Van Klaveren Date: Fri, 26 Apr 2019 19:16:45 -0700 Subject: [PATCH 05/18] Add unit tests for JWT -> session translation --- oauthproxy_test.go | 49 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/oauthproxy_test.go b/oauthproxy_test.go index 493ce72..042e6a5 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -1,9 +1,11 @@ package main import ( + "context" "crypto" "encoding/base64" "fmt" + "github.com/coreos/go-oidc" "io" "io/ioutil" "net" @@ -1134,6 +1136,53 @@ func TestClearSingleCookie(t *testing.T) { assert.Equal(t, 1, len(header["Set-Cookie"]), "should have 1 set-cookie header entries") } +type NoOpKeySet struct { +} + +func (NoOpKeySet) VerifySignature(ctx context.Context, jwt string) (payload []byte, err error) { + splitStrings := strings.Split(jwt, ".") + payloadString := splitStrings[1] + jsonString, err := base64.RawURLEncoding.DecodeString(payloadString) + return []byte(jsonString), err +} + +func TestGetJwtSession(t *testing.T) { + /* token payload: + { + "sub": "1234567890", + "aud": "https://test.myapp.com", + "name": "John Doe", + "email": "john@example.com", + "iss": "https://issuer.example.com", + "iat": 1553691215, + "exp": 1912151821 + } + */ + goodJwt := "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9." + + "eyJzdWIiOiIxMjM0NTY3ODkwIiwiYXVkIjoiaHR0cHM6Ly90ZXN0Lm15YXBwLmNvbSIsIm5hbWUiOiJKb2huIERvZSIsImVtY" + + "WlsIjoiam9obkBleGFtcGxlLmNvbSIsImlzcyI6Imh0dHBzOi8vaXNzdWVyLmV4YW1wbGUuY29tIiwiaWF0IjoxNTUzNjkxMj" + + "E1LCJleHAiOjE5MTIxNTE4MjF9." + + "rLVyzOnEldUq_pNkfa-WiV8TVJYWyZCaM2Am_uo8FGg11zD7l-qmz3x1seTvqpH6Y0Ty00fmv6dJnGnC8WMnPXQiodRTfhBSe" + + "OKZMu0HkMD2sg52zlKkbfLTO6ic5VnbVgwjjrB8am_Ta6w7kyFUaB5C1BsIrrLMldkWEhynbb8" + + keyset := NoOpKeySet{} + verifier := oidc.NewVerifier("https://issuer.example.com", keyset, + &oidc.Config{ClientID: "https://test.myapp.com", SkipExpiryCheck: true}) + p := OAuthProxy{} + p.jwtBearerVerifiers = append(p.jwtBearerVerifiers, verifier) + getReq := &http.Request{URL: &url.URL{Scheme: "http", Host: "example.com"}} + + // Bearer + getReq.Header = map[string][]string{ + "Authorization": {fmt.Sprintf("Bearer %s", goodJwt)}, + } + session, _ := p.GetJwtSession(getReq) + assert.Equal(t, session.User, "john") + assert.Equal(t, session.Email, "john@example.com") + assert.Equal(t, session.ExpiresOn, time.Unix(1912151821, 0)) + assert.Equal(t, session.IDToken, goodJwt) +} + func TestFindJwtBearerToken(t *testing.T) { p := OAuthProxy{CookieName: "oauth2", CookieDomain: "abc"} getReq := &http.Request{URL: &url.URL{Scheme: "http", Host: "example.com"}} From 1ff74d322a112331f0dd62524b39b1a908ce8e23 Mon Sep 17 00:00:00 2001 From: Brian Van Klaveren Date: Fri, 26 Apr 2019 19:47:53 -0700 Subject: [PATCH 06/18] Fix imports --- oauthproxy_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/oauthproxy_test.go b/oauthproxy_test.go index 042e6a5..0704596 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -5,7 +5,6 @@ import ( "crypto" "encoding/base64" "fmt" - "github.com/coreos/go-oidc" "io" "io/ioutil" "net" @@ -17,6 +16,7 @@ import ( "testing" "time" + "github.com/coreos/go-oidc" "github.com/mbland/hmacauth" "github.com/pusher/oauth2_proxy/logger" "github.com/pusher/oauth2_proxy/pkg/apis/sessions" From 10f65e03814afd55534fafc828860348d883dbe8 Mon Sep 17 00:00:00 2001 From: Brian Van Klaveren Date: Tue, 30 Apr 2019 14:06:11 -0700 Subject: [PATCH 07/18] Add a more realistic test for JWT passthrough --- oauthproxy_test.go | 60 ++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 55 insertions(+), 5 deletions(-) diff --git a/oauthproxy_test.go b/oauthproxy_test.go index 0704596..bff3682 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -1170,17 +1170,67 @@ func TestGetJwtSession(t *testing.T) { &oidc.Config{ClientID: "https://test.myapp.com", SkipExpiryCheck: true}) p := OAuthProxy{} p.jwtBearerVerifiers = append(p.jwtBearerVerifiers, verifier) - getReq := &http.Request{URL: &url.URL{Scheme: "http", Host: "example.com"}} + + req, _ := http.NewRequest("GET", "/", strings.NewReader("")) + authHeader := fmt.Sprintf("Bearer %s", goodJwt) + req.Header = map[string][]string{ + "Authorization": {authHeader}, + } // Bearer - getReq.Header = map[string][]string{ - "Authorization": {fmt.Sprintf("Bearer %s", goodJwt)}, - } - session, _ := p.GetJwtSession(getReq) + session, _ := p.GetJwtSession(req) assert.Equal(t, session.User, "john") assert.Equal(t, session.Email, "john@example.com") assert.Equal(t, session.ExpiresOn, time.Unix(1912151821, 0)) assert.Equal(t, session.IDToken, goodJwt) + + jwtProviderServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + log.Printf("%#v", r) + var payload string + payload = r.Header.Get("Authorization") + if payload == "" { + payload = "No Authorization header found." + } + w.WriteHeader(200) + w.Write([]byte(payload)) + })) + + opts := NewOptions() + opts.Upstreams = append(opts.Upstreams, jwtProviderServer.URL) + opts.PassAuthorization = true + opts.SetAuthorization = true + opts.SetXAuthRequest = true + opts.CookieSecret = "0123456789abcdef0123" + opts.SkipJwtBearerTokens = true + opts.Validate() + + // We can't actually use opts.Validate() because it will attempt to find a jwks URI + opts.jwtBearerVerifiers = append(opts.jwtBearerVerifiers, verifier) + + providerURL, _ := url.Parse(jwtProviderServer.URL) + const emailAddress = "john@example.com" + + opts.provider = NewTestProvider(providerURL, emailAddress) + jwtTestProxy := NewOAuthProxy(opts, func(email string) bool { + return email == emailAddress + }) + + rw := httptest.NewRecorder() + jwtTestProxy.ServeHTTP(rw, req) + if rw.Code >= 400 { + t.Fatalf("expected 3xx got %d", rw.Code) + } + + // Check PassAuthorization, should overwrite Basic header + assert.Equal(t, req.Header.Get("Authorization"), authHeader) + assert.Equal(t, req.Header.Get("X-Forwarded-User"), "john") + assert.Equal(t, req.Header.Get("X-Forwarded-Email"), "john@example.com") + + // SetAuthorization and SetXAuthRequest + assert.Equal(t, rw.Header().Get("Authorization"), authHeader) + assert.Equal(t, rw.Header().Get("X-Auth-Request-User"), "john") + assert.Equal(t, rw.Header().Get("X-Auth-Request-Email"), "john@example.com") + } func TestFindJwtBearerToken(t *testing.T) { From 79acef90366f24d3e898c90d78cfe19070492629 Mon Sep 17 00:00:00 2001 From: Brian Van Klaveren Date: Wed, 1 May 2019 09:18:54 -0700 Subject: [PATCH 08/18] Clarify skip-jwt-bearer-tokens default and add env tags --- main.go | 2 +- options.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/main.go b/main.go index 8c86271..054bb30 100644 --- a/main.go +++ b/main.go @@ -49,7 +49,7 @@ func main() { flagSet.Bool("skip-auth-preflight", false, "will skip authentication for OPTIONS requests") flagSet.Bool("ssl-insecure-skip-verify", false, "skip validation of certificates presented when using HTTPS") flagSet.Duration("flush-interval", time.Duration(1)*time.Second, "period between response flushing when streaming responses") - flagSet.Bool("skip-jwt-bearer-tokens", false, "will skip requests that have verified JWT bearer tokens") + flagSet.Bool("skip-jwt-bearer-tokens", false, "will skip requests that have verified JWT bearer tokens (default false)") flagSet.Var(&jwtIssuers, "extra-jwt-issuers", "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)") flagSet.Var(&emailDomains, "email-domain", "authenticate emails with the specified domain (may be given multiple times). Use * to authenticate any email") diff --git a/options.go b/options.go index 331c8f6..a8b0330 100644 --- a/options.go +++ b/options.go @@ -61,8 +61,8 @@ type Options struct { Upstreams []string `flag:"upstream" cfg:"upstreams" env:"OAUTH2_PROXY_UPSTREAMS"` SkipAuthRegex []string `flag:"skip-auth-regex" cfg:"skip_auth_regex" env:"OAUTH2_PROXY_SKIP_AUTH_REGEX"` - SkipJwtBearerTokens bool `flag:"skip-jwt-bearer-tokens" cfg:"skip_jwt_bearer_tokens"` - ExtraJwtIssuers []string `flag:"extra-jwt-issuers" cfg:"extra_jwt_issuers"` + SkipJwtBearerTokens bool `flag:"skip-jwt-bearer-tokens" cfg:"skip_jwt_bearer_tokens" env:"OAUTH2_PROXY_SKIP_JWT_BEARER_TOKENS"` + ExtraJwtIssuers []string `flag:"extra-jwt-issuers" cfg:"extra_jwt_issuers" env:"OAUTH2_PROXY_EXTRA_JWT_ISSUERS"` PassBasicAuth bool `flag:"pass-basic-auth" cfg:"pass_basic_auth" env:"OAUTH2_PROXY_PASS_BASIC_AUTH"` BasicAuthPassword string `flag:"basic-auth-password" cfg:"basic_auth_password" env:"OAUTH2_PROXY_BASIC_AUTH_PASSWORD"` PassAccessToken bool `flag:"pass-access-token" cfg:"pass_access_token" env:"OAUTH2_PROXY_PASS_ACCESS_TOKEN"` From 58b06ce761e0b706c9ba7e3081aa0441e75527de Mon Sep 17 00:00:00 2001 From: Brian Van Klaveren Date: Wed, 1 May 2019 09:22:25 -0700 Subject: [PATCH 09/18] Fall back to using sub if email is none (as in PR #57) --- oauthproxy.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/oauthproxy.go b/oauthproxy.go index 714c7a3..e79793b 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -901,6 +901,7 @@ func (p *OAuthProxy) GetJwtSession(req *http.Request) (*sessionsapi.SessionState } var claims struct { + Subject string `json:"sub"` Email string `json:"email"` Verified *bool `json:"email_verified"` } @@ -910,7 +911,7 @@ func (p *OAuthProxy) GetJwtSession(req *http.Request) (*sessionsapi.SessionState } if claims.Email == "" { - return nil, fmt.Errorf("id_token did not contain an email") + claims.Email = claims.Subject } if claims.Verified != nil && !*claims.Verified { From 350c1cd127515bea8b617944b0d2621d7017ea95 Mon Sep 17 00:00:00 2001 From: Brian Van Klaveren Date: Wed, 1 May 2019 10:00:54 -0700 Subject: [PATCH 10/18] Use JwtIssuer struct when parsing --- options.go | 68 ++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 43 insertions(+), 25 deletions(-) diff --git a/options.go b/options.go index a8b0330..511666b 100644 --- a/options.go +++ b/options.go @@ -119,7 +119,6 @@ type Options struct { sessionStore sessionsapi.SessionStore signatureData *SignatureData oidcVerifier *oidc.IDTokenVerifier - jwtIssuers [][]string jwtBearerVerifiers []*oidc.IDTokenVerifier } @@ -172,6 +171,12 @@ func NewOptions() *Options { } } +// JwtIssuer hold parsed JWT issuer info that's used to construct a verifier. +type JwtIssuer struct { + issuerURI string + audience string +} + func parseURL(toParse string, urltype string, msgs []string) (*url.URL, []string) { parsed, err := url.Parse(toParse) if err != nil { @@ -255,25 +260,12 @@ func (o *Options) Validate() error { } // Configure extra issuers if len(o.ExtraJwtIssuers) > 0 { - msgs = parseJwtIssuers(o, msgs) - for _, pair := range o.jwtIssuers { - issuer, audience := pair[0], pair[1] - config := &oidc.Config{ - ClientID: audience, - } - // Try as an OpenID Connect Provider first - var verifier *oidc.IDTokenVerifier - provider, err := oidc.NewProvider(context.Background(), issuer) + var jwtIssuers []JwtIssuer + jwtIssuers, msgs = parseJwtIssuers(o.ExtraJwtIssuers, msgs) + for _, jwtIssuer := range jwtIssuers { + verifier, err := newVerifierFromJwtIssuer(jwtIssuer) if err != nil { - // Try as JWKS URI - jwksURI := strings.TrimSuffix(issuer, "/") + "/.well-known/jwks.json" - _, err := http.NewRequest("GET", jwksURI, nil) - if err != nil { - return err - } - verifier = oidc.NewVerifier(issuer, oidc.NewRemoteKeySet(context.Background(), jwksURI), config) - } else { - verifier = provider.Verifier(config) + msgs = append(msgs, fmt.Sprintf("error building verifiers: %s", err)) } o.jwtBearerVerifiers = append(o.jwtBearerVerifiers, verifier) } @@ -466,17 +458,43 @@ func parseSignatureKey(o *Options, msgs []string) []string { return msgs } -func parseJwtIssuers(o *Options, msgs []string) []string { - for _, jwtVerifier := range o.ExtraJwtIssuers { +// parseJwtIssuers takes in an array of strings in the form of issuer=audience +// and parses to an array of JwtIssuer structs. +func parseJwtIssuers(issuers []string, msgs []string) ([]JwtIssuer, []string) { + var parsedIssuers []JwtIssuer + for _, jwtVerifier := range issuers { components := strings.Split(jwtVerifier, "=") if len(components) < 2 { - return append(msgs, "invalid jwt verifier uri=audience spec: "+ - jwtVerifier) + msgs = append(msgs, fmt.Sprintf("invalid jwt verifier uri=audience spec: %s", jwtVerifier)) + continue } uri, audience := components[0], strings.Join(components[1:], "=") - o.jwtIssuers = append(o.jwtIssuers, []string{uri, audience}) + parsedIssuers = append(parsedIssuers, JwtIssuer{issuerURI: uri, audience: audience}) } - return msgs + return parsedIssuers, msgs +} + +// newVerifierFromJwtIssuer takes in issuer information in JwtIssuer info and returns +// a verifier for that issuer. +func newVerifierFromJwtIssuer(jwtIssuer JwtIssuer) (*oidc.IDTokenVerifier, error) { + config := &oidc.Config{ + ClientID: jwtIssuer.audience, + } + // Try as an OpenID Connect Provider first + var verifier *oidc.IDTokenVerifier + provider, err := oidc.NewProvider(context.Background(), jwtIssuer.issuerURI) + if err != nil { + // Try as JWKS URI + jwksURI := strings.TrimSuffix(jwtIssuer.issuerURI, "/") + "/.well-known/jwks.json" + _, err := http.NewRequest("GET", jwksURI, nil) + if err != nil { + return nil, err + } + verifier = oidc.NewVerifier(jwtIssuer.issuerURI, oidc.NewRemoteKeySet(context.Background(), jwksURI), config) + } else { + verifier = provider.Verifier(config) + } + return verifier, nil } func validateCookieName(o *Options, msgs []string) []string { From 54d91c69cca9b90e04d4fd194760a6b0f5c687b9 Mon Sep 17 00:00:00 2001 From: Brian Van Klaveren Date: Wed, 1 May 2019 10:19:00 -0700 Subject: [PATCH 11/18] Use logger instead of log --- oauthproxy_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/oauthproxy_test.go b/oauthproxy_test.go index bff3682..decae03 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -1185,7 +1185,7 @@ func TestGetJwtSession(t *testing.T) { assert.Equal(t, session.IDToken, goodJwt) jwtProviderServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - log.Printf("%#v", r) + logger.Printf("%#v", r) var payload string payload = r.Header.Get("Authorization") if payload == "" { From 48dbb391bc1103461bfb4c84a93de2c97737fb86 Mon Sep 17 00:00:00 2001 From: Brian Van Klaveren Date: Mon, 20 May 2019 15:24:59 -0700 Subject: [PATCH 12/18] Move around CHANGELOG.md update --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 136775f..6858187 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,7 +60,6 @@ - [#111](https://github.com/pusher/oauth2_proxy/pull/111) Add option for telling where to find a login.gov JWT key file (@timothy-spencer) - [#170](https://github.com/pusher/oauth2_proxy/pull/170) Restore binary tarball contents to be compatible with bitlys original tarballs (@zeha) - [#185](https://github.com/pusher/oauth2_proxy/pull/185) Fix an unsupported protocol scheme error during token validation when using the Azure provider (@jonas) - - [#141](https://github.com/pusher/oauth2_proxy/pull/141) Check google group membership based on email address (@bchess) - Google Group membership is additionally checked via email address, allowing users outside a GSuite domain to be authorized. From 2f6dcf3b5f93ec3439e8e87298bb97688862ac34 Mon Sep 17 00:00:00 2001 From: Brian Van Klaveren Date: Wed, 5 Jun 2019 16:08:34 -0700 Subject: [PATCH 13/18] Move refreshing code to block acquiring cookied session --- oauthproxy.go | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/oauthproxy.go b/oauthproxy.go index e79793b..e0f5e48 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -725,21 +725,21 @@ func (p *OAuthProxy) getAuthenticatedSession(rw http.ResponseWriter, req *http.R if err != nil { logger.Printf("Error loading cookied session: %s", err) } - if session != nil && session.Age() > p.CookieRefresh && p.CookieRefresh != time.Duration(0) { - logger.Printf("Refreshing %s old session cookie for %s (refresh after %s)", session.Age(), session, p.CookieRefresh) - saveSession = true - } - } - if session != nil { - var ok bool - if ok, err = p.provider.RefreshSessionIfNeeded(session); err != nil { - logger.Printf("%s removing session. error refreshing access token %s %s", remoteAddr, err, session) - clearSession = true - session = nil - } else if ok { - saveSession = true - revalidated = true + if session != nil { + if session.Age() > p.CookieRefresh && p.CookieRefresh != time.Duration(0) { + logger.Printf("Refreshing %s old session cookie for %s (refresh after %s)", session.Age(), session, p.CookieRefresh) + saveSession = true + } + + if ok, err := p.provider.RefreshSessionIfNeeded(session); err != nil { + logger.Printf("%s removing session. error refreshing access token %s %s", remoteAddr, err, session) + clearSession = true + session = nil + } else if ok { + saveSession = true + revalidated = true + } } } From 100f126405043fe42ab13f9679da1c94f972708e Mon Sep 17 00:00:00 2001 From: Brian Van Klaveren Date: Wed, 5 Jun 2019 16:09:29 -0700 Subject: [PATCH 14/18] Make JwtIssuer struct private --- options.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/options.go b/options.go index 511666b..8c73eb9 100644 --- a/options.go +++ b/options.go @@ -171,8 +171,8 @@ func NewOptions() *Options { } } -// JwtIssuer hold parsed JWT issuer info that's used to construct a verifier. -type JwtIssuer struct { +// jwtIssuer hold parsed JWT issuer info that's used to construct a verifier. +type jwtIssuer struct { issuerURI string audience string } @@ -260,7 +260,7 @@ func (o *Options) Validate() error { } // Configure extra issuers if len(o.ExtraJwtIssuers) > 0 { - var jwtIssuers []JwtIssuer + var jwtIssuers []jwtIssuer jwtIssuers, msgs = parseJwtIssuers(o.ExtraJwtIssuers, msgs) for _, jwtIssuer := range jwtIssuers { verifier, err := newVerifierFromJwtIssuer(jwtIssuer) @@ -459,9 +459,9 @@ func parseSignatureKey(o *Options, msgs []string) []string { } // parseJwtIssuers takes in an array of strings in the form of issuer=audience -// and parses to an array of JwtIssuer structs. -func parseJwtIssuers(issuers []string, msgs []string) ([]JwtIssuer, []string) { - var parsedIssuers []JwtIssuer +// and parses to an array of jwtIssuer structs. +func parseJwtIssuers(issuers []string, msgs []string) ([]jwtIssuer, []string) { + var parsedIssuers []jwtIssuer for _, jwtVerifier := range issuers { components := strings.Split(jwtVerifier, "=") if len(components) < 2 { @@ -469,14 +469,14 @@ func parseJwtIssuers(issuers []string, msgs []string) ([]JwtIssuer, []string) { continue } uri, audience := components[0], strings.Join(components[1:], "=") - parsedIssuers = append(parsedIssuers, JwtIssuer{issuerURI: uri, audience: audience}) + parsedIssuers = append(parsedIssuers, jwtIssuer{issuerURI: uri, audience: audience}) } return parsedIssuers, msgs } -// newVerifierFromJwtIssuer takes in issuer information in JwtIssuer info and returns +// newVerifierFromJwtIssuer takes in issuer information in jwtIssuer info and returns // a verifier for that issuer. -func newVerifierFromJwtIssuer(jwtIssuer JwtIssuer) (*oidc.IDTokenVerifier, error) { +func newVerifierFromJwtIssuer(jwtIssuer jwtIssuer) (*oidc.IDTokenVerifier, error) { config := &oidc.Config{ ClientID: jwtIssuer.audience, } From 5a50f6223f33d2e68ea4b23e9ffae977d8423bbe Mon Sep 17 00:00:00 2001 From: Brian Van Klaveren Date: Mon, 17 Jun 2019 12:58:40 -0700 Subject: [PATCH 15/18] Do not infer username from email --- oauthproxy.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/oauthproxy.go b/oauthproxy.go index e0f5e48..9afa991 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -917,7 +917,6 @@ func (p *OAuthProxy) GetJwtSession(req *http.Request) (*sessionsapi.SessionState if claims.Verified != nil && !*claims.Verified { return nil, fmt.Errorf("email in id_token (%s) isn't verified", claims.Email) } - user := strings.Split(claims.Email, "@")[0] session = &sessionsapi.SessionState{ AccessToken: rawBearerToken, @@ -925,7 +924,7 @@ func (p *OAuthProxy) GetJwtSession(req *http.Request) (*sessionsapi.SessionState RefreshToken: "", ExpiresOn: bearerToken.Expiry, Email: claims.Email, - User: user, + User: claims.Email, } return session, nil } From 058ffd1047e7ec35e9be6288c9f4ef9ac714abb3 Mon Sep 17 00:00:00 2001 From: Brian Van Klaveren Date: Mon, 17 Jun 2019 13:11:49 -0700 Subject: [PATCH 16/18] Update unit tests for username --- oauthproxy_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/oauthproxy_test.go b/oauthproxy_test.go index decae03..34bf030 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -1179,7 +1179,7 @@ func TestGetJwtSession(t *testing.T) { // Bearer session, _ := p.GetJwtSession(req) - assert.Equal(t, session.User, "john") + assert.Equal(t, session.User, "john@example.com") assert.Equal(t, session.Email, "john@example.com") assert.Equal(t, session.ExpiresOn, time.Unix(1912151821, 0)) assert.Equal(t, session.IDToken, goodJwt) @@ -1223,12 +1223,12 @@ func TestGetJwtSession(t *testing.T) { // Check PassAuthorization, should overwrite Basic header assert.Equal(t, req.Header.Get("Authorization"), authHeader) - assert.Equal(t, req.Header.Get("X-Forwarded-User"), "john") + assert.Equal(t, req.Header.Get("X-Forwarded-User"), "john@example.com") assert.Equal(t, req.Header.Get("X-Forwarded-Email"), "john@example.com") // SetAuthorization and SetXAuthRequest assert.Equal(t, rw.Header().Get("Authorization"), authHeader) - assert.Equal(t, rw.Header().Get("X-Auth-Request-User"), "john") + assert.Equal(t, rw.Header().Get("X-Auth-Request-User"), "john@example.com") assert.Equal(t, rw.Header().Get("X-Auth-Request-Email"), "john@example.com") } From bd651df3c25c5e82eec093b5c7b5ef1067bbf489 Mon Sep 17 00:00:00 2001 From: Brian Van Klaveren Date: Thu, 20 Jun 2019 13:40:04 -0700 Subject: [PATCH 17/18] Ensure groups in JWT Bearer tokens are also validated Fix a minor auth logging bug --- oauthproxy.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/oauthproxy.go b/oauthproxy.go index 9afa991..99dfb36 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -650,7 +650,7 @@ func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) { } http.Redirect(rw, req, redirect, 302) } else { - logger.PrintAuthf(session.Email, req, logger.AuthSuccess, "Invalid authentication via OAuth2: unauthorized") + logger.PrintAuthf(session.Email, req, logger.AuthFailure, "Invalid authentication via OAuth2: unauthorized") p.ErrorPage(rw, 403, "Permission Denied", "Invalid Account") } } @@ -759,11 +759,13 @@ func (p *OAuthProxy) getAuthenticatedSession(rw http.ResponseWriter, req *http.R } } - if session != nil && session.Email != "" && !p.Validator(session.Email) { - logger.Printf(session.Email, req, logger.AuthFailure, "Invalid authentication via session: removing session %s", session) - session = nil - saveSession = false - clearSession = true + if session != nil && session.Email != "" { + if !p.Validator(session.Email) || !p.provider.ValidateGroup(session.Email) { + logger.Printf(session.Email, req, logger.AuthFailure, "Invalid authentication via session: removing session %s", session) + session = nil + saveSession = false + clearSession = true + } } if saveSession && session != nil { From 3881955605c9edbec15f2a72cb2cbb9ee4a90e2a Mon Sep 17 00:00:00 2001 From: Brian Van Klaveren Date: Thu, 20 Jun 2019 16:57:20 -0700 Subject: [PATCH 18/18] Update unit tests for ValidateGroup --- oauthproxy_test.go | 137 +++++++++++++++++++++++++++++---------------- 1 file changed, 89 insertions(+), 48 deletions(-) diff --git a/oauthproxy_test.go b/oauthproxy_test.go index 34bf030..35ed59a 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -229,8 +229,9 @@ func TestIsValidRedirect(t *testing.T) { type TestProvider struct { *providers.ProviderData - EmailAddress string - ValidToken bool + EmailAddress string + ValidToken bool + GroupValidator func(string) bool } func NewTestProvider(providerURL *url.URL, emailAddress string) *TestProvider { @@ -255,6 +256,9 @@ func NewTestProvider(providerURL *url.URL, emailAddress string) *TestProvider { Scope: "profile.email", }, EmailAddress: emailAddress, + GroupValidator: func(s string) bool { + return true + }, } } @@ -266,6 +270,13 @@ func (tp *TestProvider) ValidateSessionState(session *sessions.SessionState) boo return tp.ValidToken } +func (tp *TestProvider) ValidateGroup(email string) bool { + if tp.GroupValidator != nil { + return tp.GroupValidator(email) + } + return true +} + func TestBasicAuthPassword(t *testing.T) { providerServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { logger.Printf("%#v", r) @@ -791,6 +802,25 @@ func TestAuthOnlyEndpointUnauthorizedOnEmailValidationFailure(t *testing.T) { assert.Equal(t, "unauthorized request\n", string(bodyBytes)) } +func TestAuthOnlyEndpointUnauthorizedOnProviderGroupValidationFailure(t *testing.T) { + test := NewAuthOnlyEndpointTest() + startSession := &sessions.SessionState{ + Email: "michael.bland@gsa.gov", AccessToken: "my_access_token", CreatedAt: time.Now()} + test.SaveSession(startSession) + provider := &TestProvider{ + ValidToken: true, + GroupValidator: func(s string) bool { + return false + }, + } + + test.proxy.provider = provider + test.proxy.ServeHTTP(test.rw, test.req) + assert.Equal(t, http.StatusUnauthorized, test.rw.Code) + bodyBytes, _ := ioutil.ReadAll(test.rw.Body) + assert.Equal(t, "unauthorized request\n", string(bodyBytes)) +} + func TestAuthOnlyEndpointSetXAuthRequestHeaders(t *testing.T) { var pcTest ProcessCookieTest @@ -1168,69 +1198,80 @@ func TestGetJwtSession(t *testing.T) { keyset := NoOpKeySet{} verifier := oidc.NewVerifier("https://issuer.example.com", keyset, &oidc.Config{ClientID: "https://test.myapp.com", SkipExpiryCheck: true}) - p := OAuthProxy{} - p.jwtBearerVerifiers = append(p.jwtBearerVerifiers, verifier) - req, _ := http.NewRequest("GET", "/", strings.NewReader("")) + test := NewAuthOnlyEndpointTest(func(opts *Options) { + opts.PassAuthorization = true + opts.SetAuthorization = true + opts.SetXAuthRequest = true + opts.SkipJwtBearerTokens = true + opts.jwtBearerVerifiers = append(opts.jwtBearerVerifiers, verifier) + }) + tp, _ := test.proxy.provider.(*TestProvider) + tp.GroupValidator = func(s string) bool { + return true + } + authHeader := fmt.Sprintf("Bearer %s", goodJwt) - req.Header = map[string][]string{ + test.req.Header = map[string][]string{ "Authorization": {authHeader}, } // Bearer - session, _ := p.GetJwtSession(req) + session, _ := test.proxy.GetJwtSession(test.req) assert.Equal(t, session.User, "john@example.com") assert.Equal(t, session.Email, "john@example.com") assert.Equal(t, session.ExpiresOn, time.Unix(1912151821, 0)) assert.Equal(t, session.IDToken, goodJwt) - jwtProviderServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - logger.Printf("%#v", r) - var payload string - payload = r.Header.Get("Authorization") - if payload == "" { - payload = "No Authorization header found." - } - w.WriteHeader(200) - w.Write([]byte(payload)) - })) - - opts := NewOptions() - opts.Upstreams = append(opts.Upstreams, jwtProviderServer.URL) - opts.PassAuthorization = true - opts.SetAuthorization = true - opts.SetXAuthRequest = true - opts.CookieSecret = "0123456789abcdef0123" - opts.SkipJwtBearerTokens = true - opts.Validate() - - // We can't actually use opts.Validate() because it will attempt to find a jwks URI - opts.jwtBearerVerifiers = append(opts.jwtBearerVerifiers, verifier) - - providerURL, _ := url.Parse(jwtProviderServer.URL) - const emailAddress = "john@example.com" - - opts.provider = NewTestProvider(providerURL, emailAddress) - jwtTestProxy := NewOAuthProxy(opts, func(email string) bool { - return email == emailAddress - }) - - rw := httptest.NewRecorder() - jwtTestProxy.ServeHTTP(rw, req) - if rw.Code >= 400 { - t.Fatalf("expected 3xx got %d", rw.Code) + test.proxy.ServeHTTP(test.rw, test.req) + if test.rw.Code >= 400 { + t.Fatalf("expected 3xx got %d", test.rw.Code) } // Check PassAuthorization, should overwrite Basic header - assert.Equal(t, req.Header.Get("Authorization"), authHeader) - assert.Equal(t, req.Header.Get("X-Forwarded-User"), "john@example.com") - assert.Equal(t, req.Header.Get("X-Forwarded-Email"), "john@example.com") + assert.Equal(t, test.req.Header.Get("Authorization"), authHeader) + assert.Equal(t, test.req.Header.Get("X-Forwarded-User"), "john@example.com") + assert.Equal(t, test.req.Header.Get("X-Forwarded-Email"), "john@example.com") // SetAuthorization and SetXAuthRequest - assert.Equal(t, rw.Header().Get("Authorization"), authHeader) - assert.Equal(t, rw.Header().Get("X-Auth-Request-User"), "john@example.com") - assert.Equal(t, rw.Header().Get("X-Auth-Request-Email"), "john@example.com") + assert.Equal(t, test.rw.Header().Get("Authorization"), authHeader) + assert.Equal(t, test.rw.Header().Get("X-Auth-Request-User"), "john@example.com") + assert.Equal(t, test.rw.Header().Get("X-Auth-Request-Email"), "john@example.com") +} +func TestJwtUnauthorizedOnGroupValidationFailure(t *testing.T) { + goodJwt := "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9." + + "eyJzdWIiOiIxMjM0NTY3ODkwIiwiYXVkIjoiaHR0cHM6Ly90ZXN0Lm15YXBwLmNvbSIsIm5hbWUiOiJKb2huIERvZSIsImVtY" + + "WlsIjoiam9obkBleGFtcGxlLmNvbSIsImlzcyI6Imh0dHBzOi8vaXNzdWVyLmV4YW1wbGUuY29tIiwiaWF0IjoxNTUzNjkxMj" + + "E1LCJleHAiOjE5MTIxNTE4MjF9." + + "rLVyzOnEldUq_pNkfa-WiV8TVJYWyZCaM2Am_uo8FGg11zD7l-qmz3x1seTvqpH6Y0Ty00fmv6dJnGnC8WMnPXQiodRTfhBSe" + + "OKZMu0HkMD2sg52zlKkbfLTO6ic5VnbVgwjjrB8am_Ta6w7kyFUaB5C1BsIrrLMldkWEhynbb8" + + keyset := NoOpKeySet{} + verifier := oidc.NewVerifier("https://issuer.example.com", keyset, + &oidc.Config{ClientID: "https://test.myapp.com", SkipExpiryCheck: true}) + + test := NewAuthOnlyEndpointTest(func(opts *Options) { + opts.PassAuthorization = true + opts.SetAuthorization = true + opts.SetXAuthRequest = true + opts.SkipJwtBearerTokens = true + opts.jwtBearerVerifiers = append(opts.jwtBearerVerifiers, verifier) + }) + tp, _ := test.proxy.provider.(*TestProvider) + // Verify ValidateGroup fails JWT authorization + tp.GroupValidator = func(s string) bool { + return false + } + + authHeader := fmt.Sprintf("Bearer %s", goodJwt) + test.req.Header = map[string][]string{ + "Authorization": {authHeader}, + } + test.proxy.ServeHTTP(test.rw, test.req) + if test.rw.Code != http.StatusUnauthorized { + t.Fatalf("expected 401 got %d", test.rw.Code) + } } func TestFindJwtBearerToken(t *testing.T) {