[ARVADOS] created: 1.3.0-2654-gae07356a2
Git user
git at public.arvados.org
Tue Jun 9 14:55:31 UTC 2020
at ae07356a2530607f2ed79d229a64bf7466e41fd5 (commit)
commit ae07356a2530607f2ed79d229a64bf7466e41fd5
Author: Tom Clegg <tom at tomclegg.ca>
Date: Tue Jun 9 10:55:08 2020 -0400
16171: Configurable "username" OIDC claim key.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index 8b54d94c6..15ffeaadb 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -580,6 +580,11 @@ Clusters:
# use the empty string "".
EmailVerifiedClaim: "email_verified"
+ # OpenID claim field contianing the user's preferred
+ # username. If empty, use the mailbox part of the user's email
+ # address.
+ UsernameClaim: ""
+
PAM:
# (Experimental) Use PAM to authenticate users.
Enable: false
diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go
index e4ad42a08..94d7f74dd 100644
--- a/lib/config/generated_config.go
+++ b/lib/config/generated_config.go
@@ -586,6 +586,11 @@ Clusters:
# use the empty string "".
EmailVerifiedClaim: "email_verified"
+ # OpenID claim field contianing the user's preferred
+ # username. If empty, use the mailbox part of the user's email
+ # address.
+ UsernameClaim: ""
+
PAM:
# (Experimental) Use PAM to authenticate users.
Enable: false
diff --git a/lib/controller/localdb/login.go b/lib/controller/localdb/login.go
index adc22c7e5..905cfed15 100644
--- a/lib/controller/localdb/login.go
+++ b/lib/controller/localdb/login.go
@@ -49,6 +49,7 @@ func chooseLoginController(cluster *arvados.Cluster, railsProxy *railsProxy) log
ClientSecret: cluster.Login.OpenIDConnect.ClientSecret,
EmailClaim: cluster.Login.OpenIDConnect.EmailClaim,
EmailVerifiedClaim: cluster.Login.OpenIDConnect.EmailVerifiedClaim,
+ UsernameClaim: cluster.Login.OpenIDConnect.UsernameClaim,
}
case !wantGoogle && !wantOpenIDConnect && wantSSO && !wantPAM && !wantLDAP:
return &ssoLoginController{railsProxy}
diff --git a/lib/controller/localdb/login_oidc.go b/lib/controller/localdb/login_oidc.go
index a9047995c..9274d75d7 100644
--- a/lib/controller/localdb/login_oidc.go
+++ b/lib/controller/localdb/login_oidc.go
@@ -39,6 +39,7 @@ type oidcLoginController struct {
UseGooglePeopleAPI bool // Use Google People API to look up alternate email addresses
EmailClaim string // OpenID claim to use as email address; typically "email"
EmailVerifiedClaim string // If non-empty, ensure claim value is true before accepting EmailClaim; typically "email_verified"
+ UsernameClaim string // If non-empty, use as preferred username
// override Google People API base URL for testing purposes
// (normally empty, set by google pkg to
@@ -163,6 +164,10 @@ func (ctrl *oidcLoginController) getAuthInfo(ctx context.Context, token *oauth2.
ret.Email, _ = claims[ctrl.EmailClaim].(string)
}
+ if ctrl.UsernameClaim != "" {
+ ret.Username, _ = claims[ctrl.UsernameClaim].(string)
+ }
+
if !ctrl.UseGooglePeopleAPI {
if ret.Email == "" {
return nil, fmt.Errorf("cannot log in with unverified email address %q", claims[ctrl.EmailClaim])
@@ -219,9 +224,13 @@ func (ctrl *oidcLoginController) getAuthInfo(ctx context.Context, token *oauth2.
return nil, errors.New("cannot log in without a verified email address")
}
for ae := range altEmails {
- if ae != ret.Email {
- ret.AlternateEmails = append(ret.AlternateEmails, ae)
- if i := strings.Index(ae, "@"); i > 0 && strings.ToLower(ae[i+1:]) == strings.ToLower(ctrl.Cluster.Users.PreferDomainForUsername) {
+ if ae == ret.Email {
+ continue
+ }
+ ret.AlternateEmails = append(ret.AlternateEmails, ae)
+ if ret.Username == "" {
+ i := strings.Index(ae, "@")
+ if i > 0 && strings.ToLower(ae[i+1:]) == strings.ToLower(ctrl.Cluster.Users.PreferDomainForUsername) {
ret.Username = strings.SplitN(ae[:i], "+", 2)[0]
}
}
diff --git a/lib/controller/localdb/login_oidc_test.go b/lib/controller/localdb/login_oidc_test.go
index abbd4b98f..1345e8690 100644
--- a/lib/controller/localdb/login_oidc_test.go
+++ b/lib/controller/localdb/login_oidc_test.go
@@ -115,6 +115,7 @@ func (s *OIDCLoginSuite) SetUpTest(c *check.C) {
"name": s.authName,
"alt_verified": true, // for custom claim tests
"alt_email": "alt_email at example.com", // for custom claim tests
+ "alt_username": "desired-username", // for custom claim tests
})
json.NewEncoder(w).Encode(struct {
AccessToken string `json:"access_token"`
@@ -319,7 +320,9 @@ func (s *OIDCLoginSuite) TestGenericOIDCLogin(c *check.C) {
c.Log("=== succeed because email_verified is false but not required")
s.authEmail = "user at oidc.example.com"
s.authEmailVerified = false
+ s.cluster.Login.OpenIDConnect.EmailClaim = "email"
s.cluster.Login.OpenIDConnect.EmailVerifiedClaim = ""
+ s.cluster.Login.OpenIDConnect.UsernameClaim = ""
},
},
{
@@ -328,7 +331,9 @@ func (s *OIDCLoginSuite) TestGenericOIDCLogin(c *check.C) {
c.Log("=== fail because email_verified is false and required")
s.authEmail = "user at oidc.example.com"
s.authEmailVerified = false
+ s.cluster.Login.OpenIDConnect.EmailClaim = "email"
s.cluster.Login.OpenIDConnect.EmailVerifiedClaim = "email_verified"
+ s.cluster.Login.OpenIDConnect.UsernameClaim = ""
},
},
{
@@ -337,7 +342,9 @@ func (s *OIDCLoginSuite) TestGenericOIDCLogin(c *check.C) {
c.Log("=== succeed because email_verified is false but config uses custom 'verified' claim")
s.authEmail = "user at oidc.example.com"
s.authEmailVerified = false
+ s.cluster.Login.OpenIDConnect.EmailClaim = "email"
s.cluster.Login.OpenIDConnect.EmailVerifiedClaim = "alt_verified"
+ s.cluster.Login.OpenIDConnect.UsernameClaim = ""
},
},
{
@@ -348,6 +355,7 @@ func (s *OIDCLoginSuite) TestGenericOIDCLogin(c *check.C) {
s.authEmailVerified = false
s.cluster.Login.OpenIDConnect.EmailClaim = "alt_email"
s.cluster.Login.OpenIDConnect.EmailVerifiedClaim = "alt_verified"
+ s.cluster.Login.OpenIDConnect.UsernameClaim = "alt_username"
},
},
} {
@@ -377,6 +385,15 @@ func (s *OIDCLoginSuite) TestGenericOIDCLogin(c *check.C) {
c.Check(token, check.Matches, `v2/zzzzz-gj3su-.{15}/.{32,50}`)
authinfo := getCallbackAuthInfo(c, s.railsSpy)
c.Check(authinfo.Email, check.Equals, trial.expectEmail)
+
+ switch s.cluster.Login.OpenIDConnect.UsernameClaim {
+ case "alt_username":
+ c.Check(authinfo.Username, check.Equals, "desired-username")
+ case "":
+ c.Check(authinfo.Username, check.Equals, "")
+ default:
+ c.Fail() // bad test case
+ }
}
}
diff --git a/sdk/go/arvados/config.go b/sdk/go/arvados/config.go
index 7b07175f1..029e22321 100644
--- a/sdk/go/arvados/config.go
+++ b/sdk/go/arvados/config.go
@@ -163,6 +163,7 @@ type Cluster struct {
ClientSecret string
EmailClaim string
EmailVerifiedClaim string
+ UsernameClaim string
}
PAM struct {
Enable bool
commit 4282836ca705ed77bf4374ce04db7384c49bb326
Author: Tom Clegg <tom at tomclegg.ca>
Date: Tue Jun 9 10:22:09 2020 -0400
16171: Configurable "email" and "email_verified" OIDC claim keys.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index 219f6ef0b..8b54d94c6 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -569,6 +569,17 @@ Clusters:
ClientID: ""
ClientSecret: ""
+ # OpenID claim field containing the user's email
+ # address. Normally "email"; see
+ # https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims
+ EmailClaim: "email"
+
+ # OpenID claim field containing the email verification
+ # flag. Normally "email_verified". To accept every returned
+ # email address without checking a "verified" field at all,
+ # use the empty string "".
+ EmailVerifiedClaim: "email_verified"
+
PAM:
# (Experimental) Use PAM to authenticate users.
Enable: false
diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go
index 6f8cab462..e4ad42a08 100644
--- a/lib/config/generated_config.go
+++ b/lib/config/generated_config.go
@@ -575,6 +575,17 @@ Clusters:
ClientID: ""
ClientSecret: ""
+ # OpenID claim field containing the user's email
+ # address. Normally "email"; see
+ # https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims
+ EmailClaim: "email"
+
+ # OpenID claim field containing the email verification
+ # flag. Normally "email_verified". To accept every returned
+ # email address without checking a "verified" field at all,
+ # use the empty string "".
+ EmailVerifiedClaim: "email_verified"
+
PAM:
# (Experimental) Use PAM to authenticate users.
Enable: false
diff --git a/lib/controller/localdb/login.go b/lib/controller/localdb/login.go
index 9a0ee746e..adc22c7e5 100644
--- a/lib/controller/localdb/login.go
+++ b/lib/controller/localdb/login.go
@@ -37,14 +37,18 @@ func chooseLoginController(cluster *arvados.Cluster, railsProxy *railsProxy) log
ClientID: cluster.Login.Google.ClientID,
ClientSecret: cluster.Login.Google.ClientSecret,
UseGooglePeopleAPI: cluster.Login.Google.AlternateEmailAddresses,
+ EmailClaim: "email",
+ EmailVerifiedClaim: "email_verified",
}
case !wantGoogle && wantOpenIDConnect && !wantSSO && !wantPAM && !wantLDAP:
return &oidcLoginController{
- Cluster: cluster,
- RailsProxy: railsProxy,
- Issuer: cluster.Login.OpenIDConnect.Issuer,
- ClientID: cluster.Login.OpenIDConnect.ClientID,
- ClientSecret: cluster.Login.OpenIDConnect.ClientSecret,
+ Cluster: cluster,
+ RailsProxy: railsProxy,
+ Issuer: cluster.Login.OpenIDConnect.Issuer,
+ ClientID: cluster.Login.OpenIDConnect.ClientID,
+ ClientSecret: cluster.Login.OpenIDConnect.ClientSecret,
+ EmailClaim: cluster.Login.OpenIDConnect.EmailClaim,
+ EmailVerifiedClaim: cluster.Login.OpenIDConnect.EmailVerifiedClaim,
}
case !wantGoogle && !wantOpenIDConnect && wantSSO && !wantPAM && !wantLDAP:
return &ssoLoginController{railsProxy}
diff --git a/lib/controller/localdb/login_oidc.go b/lib/controller/localdb/login_oidc.go
index f42b8f8be..a9047995c 100644
--- a/lib/controller/localdb/login_oidc.go
+++ b/lib/controller/localdb/login_oidc.go
@@ -36,7 +36,9 @@ type oidcLoginController struct {
Issuer string // OIDC issuer URL, e.g., "https://accounts.google.com"
ClientID string
ClientSecret string
- UseGooglePeopleAPI bool // Use Google People API to look up alternate email addresses
+ UseGooglePeopleAPI bool // Use Google People API to look up alternate email addresses
+ EmailClaim string // OpenID claim to use as email address; typically "email"
+ EmailVerifiedClaim string // If non-empty, ensure claim value is true before accepting EmailClaim; typically "email_verified"
// override Google People API base URL for testing purposes
// (normally empty, set by google pkg to
@@ -145,28 +147,25 @@ func (ctrl *oidcLoginController) getAuthInfo(ctx context.Context, token *oauth2.
var ret rpc.UserSessionAuthInfo
defer ctxlog.FromContext(ctx).WithField("ret", &ret).Debug("getAuthInfo returned")
- var claims struct {
- Name string `json:"name"`
- Email string `json:"email"`
- Verified bool `json:"email_verified"`
- }
+ var claims map[string]interface{}
if err := idToken.Claims(&claims); err != nil {
return nil, fmt.Errorf("error extracting claims from ID token: %s", err)
- } else if claims.Verified {
+ } else if verified, _ := claims[ctrl.EmailVerifiedClaim].(bool); verified || ctrl.EmailVerifiedClaim == "" {
// Fall back to this info if the People API call
// (below) doesn't return a primary && verified email.
- if names := strings.Fields(strings.TrimSpace(claims.Name)); len(names) > 1 {
+ name, _ := claims["name"].(string)
+ if names := strings.Fields(strings.TrimSpace(name)); len(names) > 1 {
ret.FirstName = strings.Join(names[0:len(names)-1], " ")
ret.LastName = names[len(names)-1]
} else {
ret.FirstName = names[0]
}
- ret.Email = claims.Email
+ ret.Email, _ = claims[ctrl.EmailClaim].(string)
}
if !ctrl.UseGooglePeopleAPI {
if ret.Email == "" {
- return nil, fmt.Errorf("cannot log in with unverified email address %q", claims.Email)
+ return nil, fmt.Errorf("cannot log in with unverified email address %q", claims[ctrl.EmailClaim])
}
return &ret, nil
}
diff --git a/lib/controller/localdb/login_oidc_test.go b/lib/controller/localdb/login_oidc_test.go
index aa437218f..abbd4b98f 100644
--- a/lib/controller/localdb/login_oidc_test.go
+++ b/lib/controller/localdb/login_oidc_test.go
@@ -113,6 +113,8 @@ func (s *OIDCLoginSuite) SetUpTest(c *check.C) {
"email": s.authEmail,
"email_verified": s.authEmailVerified,
"name": s.authName,
+ "alt_verified": true, // for custom claim tests
+ "alt_email": "alt_email at example.com", // for custom claim tests
})
json.NewEncoder(w).Encode(struct {
AccessToken string `json:"access_token"`
@@ -299,7 +301,7 @@ func (s *OIDCLoginSuite) TestGoogleLogin_PeopleAPIError(c *check.C) {
c.Check(resp.RedirectLocation, check.Equals, "")
}
-func (s *OIDCLoginSuite) TestOIDCLogin_Success(c *check.C) {
+func (s *OIDCLoginSuite) TestGenericOIDCLogin(c *check.C) {
s.cluster.Login.Google.Enable = false
s.cluster.Login.OpenIDConnect.Enable = true
json.Unmarshal([]byte(fmt.Sprintf("%q", s.fakeIssuer.URL)), &s.cluster.Login.OpenIDConnect.Issuer)
@@ -307,18 +309,75 @@ func (s *OIDCLoginSuite) TestOIDCLogin_Success(c *check.C) {
s.cluster.Login.OpenIDConnect.ClientSecret = "oidc#client#secret"
s.validClientID = "oidc#client#id"
s.validClientSecret = "oidc#client#secret"
- s.localdb = NewConn(s.cluster)
- state := s.startLogin(c)
- resp, err := s.localdb.Login(context.Background(), arvados.LoginOptions{
- Code: s.validCode,
- State: state,
- })
- c.Assert(err, check.IsNil)
- c.Check(resp.HTML.String(), check.Equals, "")
- target, err := url.Parse(resp.RedirectLocation)
- c.Assert(err, check.IsNil)
- token := target.Query().Get("api_token")
- c.Check(token, check.Matches, `v2/zzzzz-gj3su-.{15}/.{32,50}`)
+ for _, trial := range []struct {
+ expectEmail string // "" if failure expected
+ setup func()
+ }{
+ {
+ expectEmail: "user at oidc.example.com",
+ setup: func() {
+ c.Log("=== succeed because email_verified is false but not required")
+ s.authEmail = "user at oidc.example.com"
+ s.authEmailVerified = false
+ s.cluster.Login.OpenIDConnect.EmailVerifiedClaim = ""
+ },
+ },
+ {
+ expectEmail: "",
+ setup: func() {
+ c.Log("=== fail because email_verified is false and required")
+ s.authEmail = "user at oidc.example.com"
+ s.authEmailVerified = false
+ s.cluster.Login.OpenIDConnect.EmailVerifiedClaim = "email_verified"
+ },
+ },
+ {
+ expectEmail: "user at oidc.example.com",
+ setup: func() {
+ c.Log("=== succeed because email_verified is false but config uses custom 'verified' claim")
+ s.authEmail = "user at oidc.example.com"
+ s.authEmailVerified = false
+ s.cluster.Login.OpenIDConnect.EmailVerifiedClaim = "alt_verified"
+ },
+ },
+ {
+ expectEmail: "alt_email at example.com",
+ setup: func() {
+ c.Log("=== succeed with custom 'email' and 'email_verified' claims")
+ s.authEmail = "bad at wrong.example.com"
+ s.authEmailVerified = false
+ s.cluster.Login.OpenIDConnect.EmailClaim = "alt_email"
+ s.cluster.Login.OpenIDConnect.EmailVerifiedClaim = "alt_verified"
+ },
+ },
+ } {
+ trial.setup()
+ if s.railsSpy != nil {
+ s.railsSpy.Close()
+ }
+ s.railsSpy = arvadostest.NewProxy(c, s.cluster.Services.RailsAPI)
+ s.localdb = NewConn(s.cluster)
+ *s.localdb.railsProxy = *rpc.NewConn(s.cluster.ClusterID, s.railsSpy.URL, true, rpc.PassthroughTokenProvider)
+
+ state := s.startLogin(c)
+ resp, err := s.localdb.Login(context.Background(), arvados.LoginOptions{
+ Code: s.validCode,
+ State: state,
+ })
+ c.Assert(err, check.IsNil)
+ if trial.expectEmail == "" {
+ c.Check(resp.HTML.String(), check.Matches, `(?ms).*Login error.*`)
+ c.Check(resp.RedirectLocation, check.Equals, "")
+ continue
+ }
+ c.Check(resp.HTML.String(), check.Equals, "")
+ target, err := url.Parse(resp.RedirectLocation)
+ c.Assert(err, check.IsNil)
+ token := target.Query().Get("api_token")
+ c.Check(token, check.Matches, `v2/zzzzz-gj3su-.{15}/.{32,50}`)
+ authinfo := getCallbackAuthInfo(c, s.railsSpy)
+ c.Check(authinfo.Email, check.Equals, trial.expectEmail)
+ }
}
func (s *OIDCLoginSuite) TestGoogleLogin_Success(c *check.C) {
diff --git a/sdk/go/arvados/config.go b/sdk/go/arvados/config.go
index dbd9f7109..7b07175f1 100644
--- a/sdk/go/arvados/config.go
+++ b/sdk/go/arvados/config.go
@@ -157,10 +157,12 @@ type Cluster struct {
AlternateEmailAddresses bool
}
OpenIDConnect struct {
- Enable bool
- Issuer string
- ClientID string
- ClientSecret string
+ Enable bool
+ Issuer string
+ ClientID string
+ ClientSecret string
+ EmailClaim string
+ EmailVerifiedClaim string
}
PAM struct {
Enable bool
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list