From 3f2d21dde97135c97b7e15c7c9378e2c20e4da02 Mon Sep 17 00:00:00 2001 From: Phil Taprogge Date: Tue, 7 May 2019 10:35:30 +0100 Subject: [PATCH 1/5] Remove go list - no longer needed --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index b04d139..11e2ed3 100644 --- a/Makefile +++ b/Makefile @@ -75,7 +75,7 @@ docker-push-all: docker-push .PHONY: test test: dep lint - $(GO) test -v -race $(go list ./... | grep -v /vendor/) + $(GO) test -v -race ./... .PHONY: release release: lint test From 15f48fb95e48d740d62c40eda706681cde582889 Mon Sep 17 00:00:00 2001 From: Phil Taprogge Date: Tue, 7 May 2019 10:36:00 +0100 Subject: [PATCH 2/5] Don't infer username from email local part if username not set --- oauthproxy_test.go | 9 ++++----- providers/session_state.go | 2 +- providers/session_state_test.go | 8 ++++---- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/oauthproxy_test.go b/oauthproxy_test.go index 359ef4d..8409cd5 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -291,8 +291,7 @@ func TestBasicAuthPassword(t *testing.T) { opts.Validate() providerURL, _ := url.Parse(providerServer.URL) - const emailAddress = "michael.bland@gsa.gov" - const username = "michael.bland" + const emailAddress = "john.doe@example.com" opts.provider = NewTestProvider(providerURL, emailAddress) proxy := NewOAuthProxy(opts, func(email string) bool { @@ -335,7 +334,7 @@ func TestBasicAuthPassword(t *testing.T) { rw = httptest.NewRecorder() proxy.ServeHTTP(rw, req) - expectedHeader := "Basic " + base64.StdEncoding.EncodeToString([]byte(username+":"+opts.BasicAuthPassword)) + expectedHeader := "Basic " + base64.StdEncoding.EncodeToString([]byte(emailAddress+":"+opts.BasicAuthPassword)) assert.Equal(t, expectedHeader, rw.Body.String()) providerServer.Close() } @@ -654,13 +653,13 @@ func (p *ProcessCookieTest) LoadCookiedSession() (*providers.SessionState, time. func TestLoadCookiedSession(t *testing.T) { pcTest := NewProcessCookieTestWithDefaults() - startSession := &providers.SessionState{Email: "michael.bland@gsa.gov", AccessToken: "my_access_token"} + startSession := &providers.SessionState{Email: "john.doe@example.com", AccessToken: "my_access_token"} pcTest.SaveSession(startSession, time.Now()) session, _, err := pcTest.LoadCookiedSession() assert.Equal(t, nil, err) assert.Equal(t, startSession.Email, session.Email) - assert.Equal(t, "michael.bland", session.User) + assert.Equal(t, "john.doe@example.com", session.User) assert.Equal(t, startSession.AccessToken, session.AccessToken) } diff --git a/providers/session_state.go b/providers/session_state.go index 5d4a892..c3402ac 100644 --- a/providers/session_state.go +++ b/providers/session_state.go @@ -218,7 +218,7 @@ func DecodeSessionState(v string, c *cookie.Cipher) (*SessionState, error) { } } if ss.User == "" { - ss.User = strings.Split(ss.Email, "@")[0] + ss.User = ss.Email } return ss, nil } diff --git a/providers/session_state_test.go b/providers/session_state_test.go index dee81bb..78957c6 100644 --- a/providers/session_state_test.go +++ b/providers/session_state_test.go @@ -30,7 +30,7 @@ func TestSessionStateSerialization(t *testing.T) { ss, err := DecodeSessionState(encoded, c) t.Logf("%#v", ss) assert.Equal(t, nil, err) - assert.Equal(t, "user", ss.User) + assert.Equal(t, "user@domain.com", ss.User) assert.Equal(t, s.Email, ss.Email) assert.Equal(t, s.AccessToken, ss.AccessToken) assert.Equal(t, s.IDToken, ss.IDToken) @@ -41,7 +41,7 @@ func TestSessionStateSerialization(t *testing.T) { ss, err = DecodeSessionState(encoded, c2) t.Logf("%#v", ss) assert.Equal(t, nil, err) - assert.NotEqual(t, "user", ss.User) + assert.NotEqual(t, "user@domain.com", ss.User) assert.NotEqual(t, s.Email, ss.Email) assert.Equal(t, s.ExpiresOn.Unix(), ss.ExpiresOn.Unix()) assert.NotEqual(t, s.AccessToken, ss.AccessToken) @@ -97,7 +97,7 @@ func TestSessionStateSerializationNoCipher(t *testing.T) { // only email should have been serialized ss, err := DecodeSessionState(encoded, nil) assert.Equal(t, nil, err) - assert.Equal(t, "user", ss.User) + assert.Equal(t, "user@domain.com", ss.User) assert.Equal(t, s.Email, ss.Email) assert.Equal(t, "", ss.AccessToken) assert.Equal(t, "", ss.RefreshToken) @@ -203,7 +203,7 @@ func TestDecodeSessionState(t *testing.T) { { SessionState: SessionState{ Email: "user@domain.com", - User: "user", + User: "user@domain.com", }, Encoded: `{"Email":"user@domain.com"}`, }, From 56da8387c0c2c432b83a82fa87d556c1e4f183dc Mon Sep 17 00:00:00 2001 From: Phil Taprogge Date: Tue, 7 May 2019 11:44:19 +0100 Subject: [PATCH 3/5] Include JWT sub as User --- providers/google.go | 35 +++++++++++++++++++---------------- providers/oidc.go | 1 + 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/providers/google.go b/providers/google.go index f031bfc..e3cb380 100644 --- a/providers/google.go +++ b/providers/google.go @@ -29,6 +29,12 @@ type GoogleProvider struct { GroupValidator func(string) bool } +type claims struct { + Subject string `json:"sub"` + Email string `json:"email"` + EmailVerified bool `json:"email_verified"` +} + // NewGoogleProvider initiates a new GoogleProvider func NewGoogleProvider(p *ProviderData) *GoogleProvider { p.ProviderName = "Google" @@ -64,7 +70,7 @@ func NewGoogleProvider(p *ProviderData) *GoogleProvider { } } -func emailFromIDToken(idToken string) (string, error) { +func claimsFromIDToken(idToken string) (*claims, error) { // id_token is a base64 encode ID token payload // https://developers.google.com/accounts/docs/OAuth2Login#obtainuserinfo @@ -72,24 +78,21 @@ func emailFromIDToken(idToken string) (string, error) { jwtData := strings.TrimSuffix(jwt[1], "=") b, err := base64.RawURLEncoding.DecodeString(jwtData) if err != nil { - return "", err + return nil, err } - var email struct { - Email string `json:"email"` - EmailVerified bool `json:"email_verified"` - } - err = json.Unmarshal(b, &email) + c := &claims{} + err = json.Unmarshal(b, c) if err != nil { - return "", err + return nil, err } - if email.Email == "" { - return "", errors.New("missing email") + if c.Email == "" { + return nil, errors.New("missing email") } - if !email.EmailVerified { - return "", fmt.Errorf("email %s not listed as verified", email.Email) + if !c.EmailVerified { + return nil, fmt.Errorf("email %s not listed as verified", c.Email) } - return email.Email, nil + return c, nil } // Redeem exchanges the OAuth2 authentication token for an ID token @@ -138,8 +141,7 @@ func (p *GoogleProvider) Redeem(redirectURL, code string) (s *SessionState, err if err != nil { return } - var email string - email, err = emailFromIDToken(jsonResponse.IDToken) + c, err := claimsFromIDToken(jsonResponse.IDToken) if err != nil { return } @@ -148,7 +150,8 @@ func (p *GoogleProvider) Redeem(redirectURL, code string) (s *SessionState, err IDToken: jsonResponse.IDToken, ExpiresOn: time.Now().Add(time.Duration(jsonResponse.ExpiresIn) * time.Second).Truncate(time.Second), RefreshToken: jsonResponse.RefreshToken, - Email: email, + Email: c.Email, + User: c.Subject, } return } diff --git a/providers/oidc.go b/providers/oidc.go index fe26ef1..d751be5 100644 --- a/providers/oidc.go +++ b/providers/oidc.go @@ -128,6 +128,7 @@ func (p *OIDCProvider) createSessionState(ctx context.Context, token *oauth2.Tok RefreshToken: token.RefreshToken, ExpiresOn: token.Expiry, Email: claims.Email, + User: claims.Subject, }, nil } From 39d2f28a40ea7060575986dcf48fe35569103914 Mon Sep 17 00:00:00 2001 From: Phil Taprogge Date: Thu, 9 May 2019 10:14:01 +0100 Subject: [PATCH 4/5] Add comment; update changelog --- CHANGELOG.md | 1 + oauthproxy_test.go | 2 ++ 2 files changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 564f1d5..3893abd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Changes since v3.2.0 +- [#146](https://github.com/pusher/oauth2_proxy/pull/146) Use full email address as `User` if the auth response did not contains a `User` field (@gargath) - [#144](https://github.com/pusher/oauth2_proxy/pull/144) Use GO 1.12 for ARM builds (@kskewes) - [#142](https://github.com/pusher/oauth2_proxy/pull/142) ARM Docker USER fix (@kskewes) - [#52](https://github.com/pusher/oauth2_proxy/pull/52) Logging Improvements (@MisterWil) diff --git a/oauthproxy_test.go b/oauthproxy_test.go index 8409cd5..65a8fe1 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -334,6 +334,8 @@ func TestBasicAuthPassword(t *testing.T) { rw = httptest.NewRecorder() proxy.ServeHTTP(rw, req) + // The username in the basic auth credentials is expected to be equal to the email address from the + // auth response, so we use the same variable here. expectedHeader := "Basic " + base64.StdEncoding.EncodeToString([]byte(emailAddress+":"+opts.BasicAuthPassword)) assert.Equal(t, expectedHeader, rw.Body.String()) providerServer.Close() From d4341ec40cbd98f4659fe385d466892eaafdf0be Mon Sep 17 00:00:00 2001 From: Phil Taprogge Date: Thu, 9 May 2019 10:26:40 +0100 Subject: [PATCH 5/5] Add breaking changes section to changelog --- CHANGELOG.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3893abd..bfe2708 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,16 @@ # Vx.x.x (Pre-release) +## Breaking Changes + +- [#146](https://github.com/pusher/oauth2_proxy/pull/146) Use full email address as `User` if the auth response did not contain a `User` field (@gargath) + - This change modifies the contents of the `X-Forwarded-User` header supplied by the proxy for users where the auth response from the IdP did not contain + a username. + In that case, this header used to only contain the local part of the user's email address (e.g. `john.doe` for `john.doe@example.com`) but now contains + the user's full email address instead. + ## Changes since v3.2.0 -- [#146](https://github.com/pusher/oauth2_proxy/pull/146) Use full email address as `User` if the auth response did not contains a `User` field (@gargath) +- [#146](https://github.com/pusher/oauth2_proxy/pull/146) Use full email address as `User` if the auth response did not contain a `User` field (@gargath) - [#144](https://github.com/pusher/oauth2_proxy/pull/144) Use GO 1.12 for ARM builds (@kskewes) - [#142](https://github.com/pusher/oauth2_proxy/pull/142) ARM Docker USER fix (@kskewes) - [#52](https://github.com/pusher/oauth2_proxy/pull/52) Logging Improvements (@MisterWil)