Feature/configurable userid claim minimal (#499)

* Add -user-id-claim to support other claims than email

Fix #431 - This is a minimal change to allow the user to configure which claim is
the source of the "user ID".

- Add the option `user-id-claim` (defaults to email)
- OIDC extracts this claim into session.Email (to be renamed later)
- providers: add `CreateSessionStateFromBearerToken` with a default impl taken from
  `GetJwtSession` and overridden by oidc to respect `user-id-claim`

Once #466 is merged, I can continue to rename SessionState.Email to .UserID
and add HTTP headers with a corresponding name.

* Apply suggestions from code review

Co-Authored-By: Joel Speed <Joel.speed@hotmail.co.uk>

* Review feedback: Don't extract claims manually

Instead, parse them twice - it might be sligtly slower but less bug-prone as the code evolves.

* Fix indentation

Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk>

CHANGELOG.md

@@ -19,6 +19,7 @@
 
 ## Changes since v5.1.0
 
+- [#499](https://github.com/oauth2-proxy/oauth2-proxy/pull/469) Add `-user-id-claim` to support generic claims in addition to email
 - [#486](https://github.com/oauth2-proxy/oauth2-proxy/pull/486) Add new linters (@johejo)
 - [#440](https://github.com/oauth2-proxy/oauth2-proxy/pull/440) Switch Azure AD Graph API to Microsoft Graph API (@johejo)
 - [#453](https://github.com/oauth2-proxy/oauth2-proxy/pull/453) Prevent browser caching during auth flow (@johejo)

docs/configuration/configuration.md

@@ -119,6 +119,7 @@ An example [oauth2-proxy.cfg]({{ site.gitweb }}/contrib/oauth2-proxy.cfg.example
 | `-tls-cert-file` | string | path to certificate file | |
 | `-tls-key-file` | string | path to private key file | |
 | `-upstream` | string \| list | the http url(s) of the upstream endpoint, file:// paths for static files or `static://<status_code>` for static response. Routing is based on the path | |
+| `-user-id-claim` | string | which claim contains the user ID | \["email"\] |
 | `-validate-url` | string | Access token validation endpoint | |
 | `-version` | n/a | print version string | |
 | `-whitelist-domain` | string \| list | allowed domains for redirection after authentication. Prefix domain with a `.` to allow subdomains (eg `.example.com`) | |

main.go

@@ -147,6 +147,8 @@ func main() {
 	flagSet.String("pubjwk-url", "", "JWK pubkey access endpoint: required by login.gov")
 	flagSet.Bool("gcp-healthchecks", false, "Enable GCP/GKE healthcheck endpoints")
 
+	flagSet.String("user-id-claim", "email", "which claim contains the user ID")
+
 	flagSet.Parse(os.Args[1:])
 
 	if *showVersion {

oauthproxy.go

@@ -1115,6 +1115,7 @@ func (p *OAuthProxy) ErrorJSON(rw http.ResponseWriter, code int) {
 }
 
 // GetJwtSession loads a session based on a JWT token in the authorization header.
+// (see the config options skip-jwt-bearer-tokens and extra-jwt-issuers)
 func (p *OAuthProxy) GetJwtSession(req *http.Request) (*sessionsapi.SessionState, error) {
 	rawBearerToken, err := p.findBearerToken(req)
 	if err != nil {
@@ -1122,7 +1123,6 @@ func (p *OAuthProxy) GetJwtSession(req *http.Request) (*sessionsapi.SessionState
 	}
 
 	ctx := context.Background()
-	var session *sessionsapi.SessionState
 	for _, verifier := range p.jwtBearerVerifiers {
 		bearerToken, err := verifier.Verify(ctx, rawBearerToken)
 
@@ -1131,35 +1131,7 @@ func (p *OAuthProxy) GetJwtSession(req *http.Request) (*sessionsapi.SessionState
 			continue
 		}
 
-		var claims struct {
-			Subject           string `json:"sub"`
-			Email             string `json:"email"`
-			Verified          *bool  `json:"email_verified"`
-			PreferredUsername string `json:"preferred_username"`
-		}
-
-		if err := bearerToken.Claims(&claims); err != nil {
-			return nil, fmt.Errorf("failed to parse bearer token claims: %v", err)
-		}
-
-		if claims.Email == "" {
-			claims.Email = claims.Subject
-		}
-
-		if claims.Verified != nil && !*claims.Verified {
-			return nil, fmt.Errorf("email in id_token (%s) isn't verified", claims.Email)
-		}
-
-		session = &sessionsapi.SessionState{
-			AccessToken:       rawBearerToken,
-			IDToken:           rawBearerToken,
-			RefreshToken:      "",
-			ExpiresOn:         bearerToken.Expiry,
-			Email:             claims.Email,
-			User:              claims.Email,
-			PreferredUsername: claims.PreferredUsername,
-		}
-		return session, nil
+		return p.provider.CreateSessionStateFromBearerToken(rawBearerToken, bearerToken)
 	}
 	return nil, fmt.Errorf("unable to verify jwt token %s", req.Header.Get("Authorization"))
 }

options.go

@@ -107,6 +107,7 @@ type Options struct {
 	Scope                              string `flag:"scope" cfg:"scope" env:"OAUTH2_PROXY_SCOPE"`
 	Prompt                             string `flag:"prompt" cfg:"prompt" env:"OAUTH2_PROXY_PROMPT"`
 	ApprovalPrompt                     string `flag:"approval-prompt" cfg:"approval_prompt" env:"OAUTH2_PROXY_APPROVAL_PROMPT"` // Deprecated by OIDC 1.0
+	UserIDClaim                        string `flag:"user-id-claim" cfg:"user_id_claim" env:"OAUTH2_PROXY_USER_ID_CLAIM"`
 
 	// Configuration values for logging
 	LoggingFilename       string `flag:"logging-filename" cfg:"logging_filename" env:"OAUTH2_PROXY_LOGGING_FILENAME"`
@@ -179,6 +180,7 @@ func NewOptions() *Options {
 		PreferEmailToUser:                false,
 		Prompt:                           "", // Change to "login" when ApprovalPrompt officially deprecated
 		ApprovalPrompt:                   "force",
+		UserIDClaim:                      "email",
 		InsecureOIDCAllowUnverifiedEmail: false,
 		SkipOIDCDiscovery:                false,
 		LoggingFilename:                  "",
@@ -500,6 +502,7 @@ func parseProviderInfo(o *Options, msgs []string) []string {
 		p.SetRepository(o.BitbucketRepository)
 	case *providers.OIDCProvider:
 		p.AllowUnverifiedEmail = o.InsecureOIDCAllowUnverifiedEmail
+		p.UserIDClaim = o.UserIDClaim
 		if o.oidcVerifier == nil {
 			msgs = append(msgs, "oidc provider requires an oidc issuer URL")
 		} else {

providers/oidc.go

@@ -14,12 +14,15 @@ import (
 	"github.com/oauth2-proxy/oauth2-proxy/pkg/requests"
 )
 
+const emailClaim = "email"
+
 // OIDCProvider represents an OIDC based Identity Provider
 type OIDCProvider struct {
 	*ProviderData
 
 	Verifier             *oidc.IDTokenVerifier
 	AllowUnverifiedEmail bool
+	UserIDClaim          string
 }
 
 // NewOIDCProvider initiates a new OIDCProvider
@@ -148,24 +151,15 @@ func (p *OIDCProvider) findVerifiedIDToken(ctx context.Context, token *oauth2.To
 
 func (p *OIDCProvider) createSessionState(token *oauth2.Token, idToken *oidc.IDToken) (*sessions.SessionState, error) {
 
-	newSession := &sessions.SessionState{}
+	var newSession *sessions.SessionState
 
-	if idToken != nil {
-		claims, err := findClaimsFromIDToken(idToken, token.AccessToken, p.ProfileURL.String())
+	if idToken == nil {
+		newSession = &sessions.SessionState{}
+	} else {
+		var err error
+		newSession, err = p.createSessionStateInternal(token.Extra("id_token").(string), idToken, token)
 		if err != nil {
-			return nil, fmt.Errorf("couldn't extract claims from id_token (%e)", err)
-		}
-
-		if claims != nil {
-
-			if !p.AllowUnverifiedEmail && claims.Verified != nil && !*claims.Verified {
-				return nil, fmt.Errorf("email in id_token (%s) isn't verified", claims.Email)
-			}
-
-			newSession.IDToken = token.Extra("id_token").(string)
-			newSession.Email = claims.Email
-			newSession.User = claims.Subject
-			newSession.PreferredUsername = claims.PreferredUsername
+			return nil, err
 		}
 	}
 
@@ -176,6 +170,52 @@ func (p *OIDCProvider) createSessionState(token *oauth2.Token, idToken *oidc.IDT
 	return newSession, nil
 }
 
+func (p *OIDCProvider) CreateSessionStateFromBearerToken(rawIDToken string, idToken *oidc.IDToken) (*sessions.SessionState, error) {
+	newSession, err := p.createSessionStateInternal(rawIDToken, idToken, nil)
+	if err != nil {
+		return nil, err
+	}
+
+	newSession.AccessToken = rawIDToken
+	newSession.IDToken = rawIDToken
+	newSession.RefreshToken = ""
+	newSession.ExpiresOn = idToken.Expiry
+
+	return newSession, nil
+}
+
+func (p *OIDCProvider) createSessionStateInternal(rawIDToken string, idToken *oidc.IDToken, token *oauth2.Token) (*sessions.SessionState, error) {
+
+	newSession := &sessions.SessionState{}
+
+	if idToken == nil {
+		return newSession, nil
+	}
+	accessToken := ""
+	if token != nil {
+		accessToken = token.AccessToken
+	}
+
+	claims, err := p.findClaimsFromIDToken(idToken, accessToken, p.ProfileURL.String())
+	if err != nil {
+		return nil, fmt.Errorf("couldn't extract claims from id_token (%e)", err)
+	}
+
+	newSession.IDToken = rawIDToken
+
+	newSession.Email = claims.UserID // TODO Rename SessionState.Email to .UserID in the near future
+
+	newSession.User = claims.Subject
+	newSession.PreferredUsername = claims.PreferredUsername
+
+	verifyEmail := (p.UserIDClaim == emailClaim) && !p.AllowUnverifiedEmail
+	if verifyEmail && claims.Verified != nil && !*claims.Verified {
+		return nil, fmt.Errorf("email in id_token (%s) isn't verified", claims.UserID)
+	}
+
+	return newSession, nil
+}
+
 // ValidateSessionState checks that the session's IDToken is still valid
 func (p *OIDCProvider) ValidateSessionState(s *sessions.SessionState) bool {
 	ctx := context.Background()
@@ -190,15 +230,25 @@ func getOIDCHeader(accessToken string) http.Header {
 	return header
 }
 
-func findClaimsFromIDToken(idToken *oidc.IDToken, accessToken string, profileURL string) (*OIDCClaims, error) {
+func (p *OIDCProvider) findClaimsFromIDToken(idToken *oidc.IDToken, accessToken string, profileURL string) (*OIDCClaims, error) {
 
-	// Extract custom claims.
 	claims := &OIDCClaims{}
-	if err := idToken.Claims(claims); err != nil {
-		return nil, fmt.Errorf("failed to parse id_token claims: %v", err)
+	// Extract default claims.
+	if err := idToken.Claims(&claims); err != nil {
+		return nil, fmt.Errorf("failed to parse default id_token claims: %v", err)
+	}
+	// Extract custom claims.
+	if err := idToken.Claims(&claims.rawClaims); err != nil {
+		return nil, fmt.Errorf("failed to parse all id_token claims: %v", err)
 	}
 
-	if claims.Email == "" {
+	userID := claims.rawClaims[p.UserIDClaim]
+	if userID == nil {
+		return nil, fmt.Errorf("claims did not contains the required user-id-claim '%s'", p.UserIDClaim)
+	}
+	claims.UserID = fmt.Sprint(userID)
+
+	if p.UserIDClaim == emailClaim && claims.UserID == "" {
 		if profileURL == "" {
 			return nil, fmt.Errorf("id_token did not contain an email")
 		}
@@ -223,15 +273,16 @@ func findClaimsFromIDToken(idToken *oidc.IDToken, accessToken string, profileURL
 			return nil, fmt.Errorf("neither id_token nor userinfo endpoint contained an email")
 		}
 
-		claims.Email = email
+		claims.UserID = email
 	}
 
 	return claims, nil
 }
 
 type OIDCClaims struct {
+	rawClaims         map[string]interface{}
+	UserID            string
 	Subject           string `json:"sub"`
-	Email             string `json:"email"`
 	Verified          *bool  `json:"email_verified"`
 	PreferredUsername string `json:"preferred_username"`
 }

providers/oidc_test.go

@@ -31,6 +31,7 @@ const secret = "secret"
 type idTokenClaims struct {
 	Name    string `json:"name,omitempty"`
 	Email   string `json:"email,omitempty"`
+	Phone   string `json:"phone_number,omitempty"`
 	Picture string `json:"picture,omitempty"`
 	jwt.StandardClaims
 }
@@ -46,6 +47,7 @@ type redeemTokenResponse struct {
 var defaultIDToken idTokenClaims = idTokenClaims{
 	"Jane Dobbs",
 	"janed@me.com",
+	"+4798765432",
 	"http://mugbook.com/janed/me.jpg",
 	jwt.StandardClaims{
 		Audience:  "https://test.myapp.com",
@@ -106,6 +108,7 @@ func newOIDCProvider(serverURL *url.URL) *OIDCProvider {
 			fakeKeySetStub{},
 			&oidc.Config{ClientID: clientID},
 		),
+		UserIDClaim: "email",
 	}
 
 	return p
@@ -165,6 +168,26 @@ func TestOIDCProviderRedeem(t *testing.T) {
 	assert.Equal(t, "123456789", session.User)
 }
 
+func TestOIDCProviderRedeem_custom_userid(t *testing.T) {
+
+	idToken, _ := newSignedTestIDToken(defaultIDToken)
+	body, _ := json.Marshal(redeemTokenResponse{
+		AccessToken:  accessToken,
+		ExpiresIn:    10,
+		TokenType:    "Bearer",
+		RefreshToken: refreshToken,
+		IDToken:      idToken,
+	})
+
+	server, provider := newTestSetup(body)
+	provider.UserIDClaim = "phone_number"
+	defer server.Close()
+
+	session, err := provider.Redeem(provider.RedeemURL.String(), "code1234")
+	assert.Equal(t, nil, err)
+	assert.Equal(t, defaultIDToken.Phone, session.Email)
+}
+
 func TestOIDCProviderRefreshSessionIfNeededWithoutIdToken(t *testing.T) {
 
 	idToken, _ := newSignedTestIDToken(defaultIDToken)

providers/provider_default.go

@@ -10,6 +10,8 @@ import (
 	"net/url"
 	"time"
 
+	"github.com/coreos/go-oidc"
+
 	"github.com/oauth2-proxy/oauth2-proxy/pkg/apis/sessions"
 	"github.com/oauth2-proxy/oauth2-proxy/pkg/encryption"
 )
@@ -144,3 +146,37 @@ func (p *ProviderData) ValidateSessionState(s *sessions.SessionState) bool {
 func (p *ProviderData) RefreshSessionIfNeeded(s *sessions.SessionState) (bool, error) {
 	return false, nil
 }
+
+func (p *ProviderData) CreateSessionStateFromBearerToken(rawIDToken string, idToken *oidc.IDToken) (*sessions.SessionState, error) {
+	var claims struct {
+		Subject           string `json:"sub"`
+		Email             string `json:"email"`
+		Verified          *bool  `json:"email_verified"`
+		PreferredUsername string `json:"preferred_username"`
+	}
+
+	if err := idToken.Claims(&claims); err != nil {
+		return nil, fmt.Errorf("failed to parse bearer token claims: %v", err)
+	}
+
+	if claims.Email == "" {
+		claims.Email = claims.Subject
+	}
+
+	if claims.Verified != nil && !*claims.Verified {
+		return nil, fmt.Errorf("email in id_token (%s) isn't verified", claims.Email)
+	}
+
+	newSession := &sessions.SessionState{
+		Email:             claims.Email,
+		User:              claims.Email,
+		PreferredUsername: claims.PreferredUsername,
+	}
+
+	newSession.AccessToken = rawIDToken
+	newSession.IDToken = rawIDToken
+	newSession.RefreshToken = ""
+	newSession.ExpiresOn = idToken.Expiry
+
+	return newSession, nil
+}

providers/providers.go

@@ -1,6 +1,7 @@
 package providers
 
 import (
+	"github.com/coreos/go-oidc"
 	"github.com/oauth2-proxy/oauth2-proxy/pkg/apis/sessions"
 	"github.com/oauth2-proxy/oauth2-proxy/pkg/encryption"
 )
@@ -18,6 +19,7 @@ type Provider interface {
 	RefreshSessionIfNeeded(*sessions.SessionState) (bool, error)
 	SessionFromCookie(string, *encryption.Cipher) (*sessions.SessionState, error)
 	CookieForSession(*sessions.SessionState, *encryption.Cipher) (string, error)
+	CreateSessionStateFromBearerToken(rawIDToken string, idToken *oidc.IDToken) (*sessions.SessionState, error)
 }
 
 // New provides a new Provider based on the configured provider string