Merge branch '16171-oidc-config'
authorTom Clegg <tom@tomclegg.ca>
Wed, 17 Jun 2020 16:35:32 +0000 (12:35 -0400)
committerTom Clegg <tom@tomclegg.ca>
Wed, 17 Jun 2020 16:35:32 +0000 (12:35 -0400)
closes #16171

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@tomclegg.ca>

doc/install/setup-login.html.textile.liquid
lib/config/config.default.yml
lib/config/export.go
lib/config/generated_config.go
lib/controller/localdb/login.go
lib/controller/localdb/login_oidc.go
lib/controller/localdb/login_oidc_test.go
sdk/go/arvados/config.go

index 572a83f7060ab925b8c4e39b45186bf898f893dc..aec82cfe2a583dd2eaf2d251532e4d46d625ff5e 100644 (file)
@@ -56,6 +56,8 @@ With this configuration, users will sign in with a third-party OpenID Connect pr
         ClientSecret: "zzzzzzzzzzzzzzzzzzzzzzzz"
 </pre>
 
+Check the OpenIDConnect section in the "default config file":{{site.baseurl}}/admin/config.html for more details and configuration options.
+
 h2(#ldap). LDAP
 
 With this configuration, authentication uses an external LDAP service like OpenLDAP or Active Directory.
index 219f6ef0ba91a1afb2e3311ca66b94f5a989020f..b9bc9c2c5ce4a28eb25015961b687cea449d503a 100644 (file)
@@ -569,6 +569,22 @@ 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"
+
+        # OpenID claim field containing 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
index fc4908c15929d7807d4c63033d61cbcef3b4bcab..0ad4222f551ba1220d2459f4330e9a1e05240d44 100644 (file)
@@ -156,6 +156,9 @@ var whitelist = map[string]bool{
        "Login.OpenIDConnect.ClientSecret":             false,
        "Login.OpenIDConnect.Enable":                   true,
        "Login.OpenIDConnect.Issuer":                   false,
+       "Login.OpenIDConnect.EmailClaim":               false,
+       "Login.OpenIDConnect.EmailVerifiedClaim":       false,
+       "Login.OpenIDConnect.UsernameClaim":            false,
        "Login.PAM":                                    true,
        "Login.PAM.DefaultEmailDomain":                 false,
        "Login.PAM.Enable":                             true,
index 6f8cab462bce2dc15118f31454b40bb35d06e3ff..758dc2677cf233b0d4d61462e7ec73d607f69174 100644 (file)
@@ -575,6 +575,22 @@ 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"
+
+        # OpenID claim field containing 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
index 9a0ee746e64006d08ab2d87981f6f47bf8fbcfa6..905cfed15c500d95689857e36c1c3323165c4d3d 100644 (file)
@@ -37,14 +37,19 @@ 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,
+                       UsernameClaim:      cluster.Login.OpenIDConnect.UsernameClaim,
                }
        case !wantGoogle && !wantOpenIDConnect && wantSSO && !wantPAM && !wantLDAP:
                return &ssoLoginController{railsProxy}
index f42b8f8beaf1d2721a78c9883c20353eabd0e43b..9274d75d7c9fdc1973cbcad621b306599e571893 100644 (file)
@@ -36,7 +36,10 @@ 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"
+       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
@@ -145,28 +148,29 @@ 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.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.Email)
+                       return nil, fmt.Errorf("cannot log in with unverified email address %q", claims[ctrl.EmailClaim])
                }
                return &ret, nil
        }
@@ -220,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]
                        }
                }
index aa437218ff79eaae26ee93d8450af72561387919..1345e86900dd1056da5a9259bf0d5caf179253e5 100644 (file)
@@ -113,6 +113,9 @@ 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@example.com", // for custom claim tests
+                               "alt_username":   "desired-username",      // for custom claim tests
                        })
                        json.NewEncoder(w).Encode(struct {
                                AccessToken  string `json:"access_token"`
@@ -299,7 +302,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 +310,91 @@ 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@oidc.example.com",
+                       setup: func() {
+                               c.Log("=== succeed because email_verified is false but not required")
+                               s.authEmail = "user@oidc.example.com"
+                               s.authEmailVerified = false
+                               s.cluster.Login.OpenIDConnect.EmailClaim = "email"
+                               s.cluster.Login.OpenIDConnect.EmailVerifiedClaim = ""
+                               s.cluster.Login.OpenIDConnect.UsernameClaim = ""
+                       },
+               },
+               {
+                       expectEmail: "",
+                       setup: func() {
+                               c.Log("=== fail because email_verified is false and required")
+                               s.authEmail = "user@oidc.example.com"
+                               s.authEmailVerified = false
+                               s.cluster.Login.OpenIDConnect.EmailClaim = "email"
+                               s.cluster.Login.OpenIDConnect.EmailVerifiedClaim = "email_verified"
+                               s.cluster.Login.OpenIDConnect.UsernameClaim = ""
+                       },
+               },
+               {
+                       expectEmail: "user@oidc.example.com",
+                       setup: func() {
+                               c.Log("=== succeed because email_verified is false but config uses custom 'verified' claim")
+                               s.authEmail = "user@oidc.example.com"
+                               s.authEmailVerified = false
+                               s.cluster.Login.OpenIDConnect.EmailClaim = "email"
+                               s.cluster.Login.OpenIDConnect.EmailVerifiedClaim = "alt_verified"
+                               s.cluster.Login.OpenIDConnect.UsernameClaim = ""
+                       },
+               },
+               {
+                       expectEmail: "alt_email@example.com",
+                       setup: func() {
+                               c.Log("=== succeed with custom 'email' and 'email_verified' claims")
+                               s.authEmail = "bad@wrong.example.com"
+                               s.authEmailVerified = false
+                               s.cluster.Login.OpenIDConnect.EmailClaim = "alt_email"
+                               s.cluster.Login.OpenIDConnect.EmailVerifiedClaim = "alt_verified"
+                               s.cluster.Login.OpenIDConnect.UsernameClaim = "alt_username"
+                       },
+               },
+       } {
+               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)
+
+               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
+               }
+       }
 }
 
 func (s *OIDCLoginSuite) TestGoogleLogin_Success(c *check.C) {
index dbd9f71099619203bb38f4dd1118b865f5c2f662..029e223218b2a5136b8eac2238b088e2ce4fb983 100644 (file)
@@ -157,10 +157,13 @@ 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
+                       UsernameClaim      string
                }
                PAM struct {
                        Enable             bool