Merge pull request #146 from pusher/no-infer-username

Do not infer username from email

CHANGELOG.md

@@ -1,7 +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 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)

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

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,9 @@ func TestBasicAuthPassword(t *testing.T) {
 	rw = httptest.NewRecorder()
 	proxy.ServeHTTP(rw, req)
 
-	expectedHeader := "Basic " + base64.StdEncoding.EncodeToString([]byte(username+":"+opts.BasicAuthPassword))
+	// 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()
 }
@@ -654,13 +655,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)
 }
 

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
 }

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
 }
 

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
 }

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"}`,
 		},