[ARVADOS] created: 1.3.0-2614-gcd3f543b2
Git user
git at public.arvados.org
Thu Jun 4 16:19:23 UTC 2020
at cd3f543b2ea20a7ac5851c118d5189df080207f2 (commit)
commit cd3f543b2ea20a7ac5851c118d5189df080207f2
Author: Tom Clegg <tom at tomclegg.ca>
Date: Thu Jun 4 11:44:29 2020 -0400
16171: Warn about OIDC issuer URL spelling sensitivity.
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 d1c47f4a2..be0123574 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -556,6 +556,18 @@ Clusters:
Enable: false
# Issuer URL, e.g., "https://login.example.com".
+ #
+ # This must be exactly equal to the URL returned by the issuer
+ # itself in its config response ("isser" key). If the
+ # configured value is "https://example" and the provider
+ # returns "https://example:443" then login will fail, even
+ # though those URLs are equivalent (RFC3986).
+ #
+ # If the configured URL's path component is just "/" then it
+ # is stripped. Therefore, an issuer advertising itself as
+ # "https://example/" cannot be used -- but "https://example",
+ # "https://example/foo", and "https://example/foo/" are
+ # supported.
Issuer: ""
# Your client ID and client secret (supplied by the provider).
diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go
index 0dda58db0..49d86c77f 100644
--- a/lib/config/generated_config.go
+++ b/lib/config/generated_config.go
@@ -562,6 +562,18 @@ Clusters:
Enable: false
# Issuer URL, e.g., "https://login.example.com".
+ #
+ # This must be exactly equal to the URL returned by the issuer
+ # itself in its config response ("isser" key). If the
+ # configured value is "https://example" and the provider
+ # returns "https://example:443" then login will fail, even
+ # though those URLs are equivalent (RFC3986).
+ #
+ # If the configured URL's path component is just "/" then it
+ # is stripped. Therefore, an issuer advertising itself as
+ # "https://example/" cannot be used -- but "https://example",
+ # "https://example/foo", and "https://example/foo/" are
+ # supported.
Issuer: ""
# Your client ID and client secret (supplied by the provider).
commit d9b8396a05f3f4d187fbcadad6f63c019865e6fc
Author: Tom Clegg <tom at tomclegg.ca>
Date: Thu Jun 4 11:21:43 2020 -0400
16171: Test non-Google OIDC login with fake issuer.
Ensures the proper credentials are used.
Exposes go-oidc's sensitivity to different spellings of equivalent
issuer URLs.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/lib/controller/localdb/login.go b/lib/controller/localdb/login.go
index 5231bcb19..04fb82bd5 100644
--- a/lib/controller/localdb/login.go
+++ b/lib/controller/localdb/login.go
@@ -39,10 +39,30 @@ func chooseLoginController(cluster *arvados.Cluster, railsProxy *railsProxy) log
UseGooglePeopleAPI: cluster.Login.Google.AlternateEmailAddresses,
}
case !wantGoogle && wantOpenIDConnect && !wantSSO && !wantPAM && !wantLDAP:
+ issuer := cluster.Login.OpenIDConnect.Issuer
+ if issuer.Path == "/" {
+ // The OIDC library returns an error if the
+ // config says "https://example/" and the
+ // issuer identifies itself as
+ // "https://example", even though those URLs
+ // are equivalent
+ // (https://tools.ietf.org/html/rfc3986#section-6.2.3).
+ //
+ // Our config loader adds "/" to URLs with
+ // empty path, so we strip it off here and
+ // count on the issuer to do the same when
+ // identifying itself, as Google does.
+ //
+ // (Non-empty paths as in
+ // "https://example/foo/" are preserved by the
+ // config loader so the config just has to
+ // match the issuer's response.)
+ issuer.Path = ""
+ }
return &oidcLoginController{
Cluster: cluster,
RailsProxy: railsProxy,
- Issuer: cluster.Login.OpenIDConnect.Issuer.String(),
+ Issuer: issuer.String(),
ClientID: cluster.Login.OpenIDConnect.ClientID,
ClientSecret: cluster.Login.OpenIDConnect.ClientSecret,
}
diff --git a/lib/controller/localdb/login_oidc_test.go b/lib/controller/localdb/login_oidc_test.go
index 4a3a2a5ee..b6e58c948 100644
--- a/lib/controller/localdb/login_oidc_test.go
+++ b/lib/controller/localdb/login_oidc_test.go
@@ -9,6 +9,7 @@ import (
"context"
"crypto/rand"
"crypto/rsa"
+ "encoding/base64"
"encoding/json"
"fmt"
"net/http"
@@ -47,7 +48,9 @@ type OIDCLoginSuite struct {
issuerKey *rsa.PrivateKey
// expected token request
- validCode string
+ validCode string
+ validClientID string
+ validClientSecret string
// desired response from token endpoint
authEmail string
authEmailVerified bool
@@ -83,13 +86,26 @@ func (s *OIDCLoginSuite) SetUpTest(c *check.C) {
"userinfo_endpoint": s.fakeIssuer.URL + "/userinfo",
})
case "/token":
+ var clientID, clientSecret string
+ auth, _ := base64.StdEncoding.DecodeString(strings.TrimPrefix(req.Header.Get("Authorization"), "Basic "))
+ authsplit := strings.Split(string(auth), ":")
+ if len(authsplit) == 2 {
+ clientID, _ = url.QueryUnescape(authsplit[0])
+ clientSecret, _ = url.QueryUnescape(authsplit[1])
+ }
+ if clientID != s.validClientID || clientSecret != s.validClientSecret {
+ c.Logf("fakeIssuer: expected (%q, %q) got (%q, %q)", s.validClientID, s.validClientSecret, clientID, clientSecret)
+ w.WriteHeader(http.StatusUnauthorized)
+ return
+ }
+
if req.Form.Get("code") != s.validCode || s.validCode == "" {
w.WriteHeader(http.StatusUnauthorized)
return
}
idToken, _ := json.Marshal(map[string]interface{}{
"iss": s.fakeIssuer.URL,
- "aud": []string{"test%client$id"},
+ "aud": []string{clientID},
"sub": "fake-user-id",
"exp": time.Now().UTC().Add(time.Minute).Unix(),
"iat": time.Now().UTC().Unix(),
@@ -145,13 +161,16 @@ func (s *OIDCLoginSuite) SetUpTest(c *check.C) {
s.fakePeopleAPIResponse = map[string]interface{}{}
cfg, err := config.NewLoader(nil, ctxlog.TestLogger(c)).Load()
+ c.Assert(err, check.IsNil)
s.cluster, err = cfg.GetCluster("")
+ c.Assert(err, check.IsNil)
s.cluster.Login.SSO.Enable = false
s.cluster.Login.Google.Enable = true
s.cluster.Login.Google.ClientID = "test%client$id"
s.cluster.Login.Google.ClientSecret = "test#client/secret"
s.cluster.Users.PreferDomainForUsername = "PreferDomainForUsername.example.com"
- c.Assert(err, check.IsNil)
+ s.validClientID = "test%client$id"
+ s.validClientSecret = "test#client/secret"
s.localdb = NewConn(s.cluster)
c.Assert(s.localdb.loginController, check.FitsTypeOf, (*oidcLoginController)(nil))
@@ -249,7 +268,7 @@ func (s *OIDCLoginSuite) TestConfig(c *check.C) {
s.cluster.Login.OpenIDConnect.ClientSecret = "oidc-client-secret"
localdb := NewConn(s.cluster)
ctrl := localdb.loginController.(*oidcLoginController)
- c.Check(ctrl.Issuer, check.Equals, "https://accounts.example.com/")
+ c.Check(ctrl.Issuer, check.Equals, "https://accounts.example.com")
c.Check(ctrl.ClientID, check.Equals, "oidc-client-id")
c.Check(ctrl.ClientSecret, check.Equals, "oidc-client-secret")
c.Check(ctrl.UseGooglePeopleAPI, check.Equals, false)
@@ -280,6 +299,28 @@ func (s *OIDCLoginSuite) TestGoogleLogin_PeopleAPIError(c *check.C) {
c.Check(resp.RedirectLocation, check.Equals, "")
}
+func (s *OIDCLoginSuite) TestOIDCLogin_Success(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)
+ s.cluster.Login.OpenIDConnect.ClientID = "oidc#client#id"
+ 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}`)
+}
+
func (s *OIDCLoginSuite) TestGoogleLogin_Success(c *check.C) {
state := s.startLogin(c)
resp, err := s.localdb.Login(context.Background(), arvados.LoginOptions{
commit 18de6db7e6f2336d69aad9dad7691ed123bec509
Author: Tom Clegg <tom at tomclegg.ca>
Date: Thu Jun 4 09:54:35 2020 -0400
16171: Move more code to one-time setup func.
Fix Google credentials used for non-Google OIDC provider.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/lib/controller/localdb/login_oidc.go b/lib/controller/localdb/login_oidc.go
index b0458a7ad..f42b8f8be 100644
--- a/lib/controller/localdb/login_oidc.go
+++ b/lib/controller/localdb/login_oidc.go
@@ -38,22 +38,45 @@ type oidcLoginController struct {
ClientSecret string
UseGooglePeopleAPI bool // Use Google People API to look up alternate email addresses
- peopleAPIBasePath string // override Google People API base URL (normally empty, set by google pkg to https://people.googleapis.com/)
- provider *oidc.Provider
- mu sync.Mutex
+ // override Google People API base URL for testing purposes
+ // (normally empty, set by google pkg to
+ // https://people.googleapis.com/)
+ peopleAPIBasePath string
+
+ provider *oidc.Provider // initialized by setup()
+ oauth2conf *oauth2.Config // initialized by setup()
+ verifier *oidc.IDTokenVerifier // initialized by setup()
+ mu sync.Mutex // protects setup()
}
-func (ctrl *oidcLoginController) getProvider() (*oidc.Provider, error) {
+// Initialize ctrl.provider and ctrl.oauth2conf.
+func (ctrl *oidcLoginController) setup() error {
ctrl.mu.Lock()
defer ctrl.mu.Unlock()
- if ctrl.provider == nil {
- provider, err := oidc.NewProvider(context.Background(), ctrl.Issuer)
- if err != nil {
- return nil, err
- }
- ctrl.provider = provider
+ if ctrl.provider != nil {
+ // already set up
+ return nil
+ }
+ redirURL, err := (*url.URL)(&ctrl.Cluster.Services.Controller.ExternalURL).Parse("/" + arvados.EndpointLogin.Path)
+ if err != nil {
+ return fmt.Errorf("error making redirect URL: %s", err)
+ }
+ provider, err := oidc.NewProvider(context.Background(), ctrl.Issuer)
+ if err != nil {
+ return err
+ }
+ ctrl.oauth2conf = &oauth2.Config{
+ ClientID: ctrl.ClientID,
+ ClientSecret: ctrl.ClientSecret,
+ Endpoint: provider.Endpoint(),
+ Scopes: []string{oidc.ScopeOpenID, "profile", "email"},
+ RedirectURL: redirURL.String(),
}
- return ctrl.provider, nil
+ ctrl.verifier = provider.Verifier(&oidc.Config{
+ ClientID: ctrl.ClientID,
+ })
+ ctrl.provider = provider
+ return nil
}
func (ctrl *oidcLoginController) Logout(ctx context.Context, opts arvados.LogoutOptions) (arvados.LogoutResponse, error) {
@@ -61,38 +84,18 @@ func (ctrl *oidcLoginController) Logout(ctx context.Context, opts arvados.Logout
}
func (ctrl *oidcLoginController) Login(ctx context.Context, opts arvados.LoginOptions) (arvados.LoginResponse, error) {
- provider, err := ctrl.getProvider()
+ err := ctrl.setup()
if err != nil {
return loginError(fmt.Errorf("error setting up OpenID Connect provider: %s", err))
}
- redirURL, err := (*url.URL)(&ctrl.Cluster.Services.Controller.ExternalURL).Parse("/login")
- if err != nil {
- return loginError(fmt.Errorf("error making redirect URL: %s", err))
- }
- conf := &oauth2.Config{
- ClientID: ctrl.Cluster.Login.Google.ClientID,
- ClientSecret: ctrl.Cluster.Login.Google.ClientSecret,
- Endpoint: provider.Endpoint(),
- Scopes: []string{oidc.ScopeOpenID, "profile", "email"},
- RedirectURL: redirURL.String(),
- }
- verifier := provider.Verifier(&oidc.Config{
- ClientID: conf.ClientID,
- })
if opts.State == "" {
// Initiate OIDC sign-in.
if opts.ReturnTo == "" {
return loginError(errors.New("missing return_to parameter"))
}
- me := url.URL(ctrl.Cluster.Services.Controller.ExternalURL)
- callback, err := me.Parse("/" + arvados.EndpointLogin.Path)
- if err != nil {
- return loginError(err)
- }
- conf.RedirectURL = callback.String()
state := ctrl.newOAuth2State([]byte(ctrl.Cluster.SystemRootToken), opts.Remote, opts.ReturnTo)
return arvados.LoginResponse{
- RedirectLocation: conf.AuthCodeURL(state.String(),
+ RedirectLocation: ctrl.oauth2conf.AuthCodeURL(state.String(),
// prompt=select_account tells Google
// to show the "choose which Google
// account" page, even if the client
@@ -106,7 +109,7 @@ func (ctrl *oidcLoginController) Login(ctx context.Context, opts arvados.LoginOp
if !state.verify([]byte(ctrl.Cluster.SystemRootToken)) {
return loginError(errors.New("invalid OAuth2 state"))
}
- oauth2Token, err := conf.Exchange(ctx, opts.Code)
+ oauth2Token, err := ctrl.oauth2conf.Exchange(ctx, opts.Code)
if err != nil {
return loginError(fmt.Errorf("error in OAuth2 exchange: %s", err))
}
@@ -114,11 +117,11 @@ func (ctrl *oidcLoginController) Login(ctx context.Context, opts arvados.LoginOp
if !ok {
return loginError(errors.New("error in OAuth2 exchange: no ID token in OAuth2 token"))
}
- idToken, err := verifier.Verify(ctx, rawIDToken)
+ idToken, err := ctrl.verifier.Verify(ctx, rawIDToken)
if err != nil {
return loginError(fmt.Errorf("error verifying ID token: %s", err))
}
- authinfo, err := ctrl.getAuthInfo(ctx, ctrl.Cluster, conf, oauth2Token, idToken)
+ authinfo, err := ctrl.getAuthInfo(ctx, oauth2Token, idToken)
if err != nil {
return loginError(err)
}
@@ -138,7 +141,7 @@ func (ctrl *oidcLoginController) UserAuthenticate(ctx context.Context, opts arva
// primary address at index 0. The provided defaultAddr is always
// included in the returned slice, and is used as the primary if the
// Google API does not indicate one.
-func (ctrl *oidcLoginController) getAuthInfo(ctx context.Context, cluster *arvados.Cluster, conf *oauth2.Config, token *oauth2.Token, idToken *oidc.IDToken) (*rpc.UserSessionAuthInfo, error) {
+func (ctrl *oidcLoginController) getAuthInfo(ctx context.Context, token *oauth2.Token, idToken *oidc.IDToken) (*rpc.UserSessionAuthInfo, error) {
var ret rpc.UserSessionAuthInfo
defer ctxlog.FromContext(ctx).WithField("ret", &ret).Debug("getAuthInfo returned")
@@ -168,7 +171,7 @@ func (ctrl *oidcLoginController) getAuthInfo(ctx context.Context, cluster *arvad
return &ret, nil
}
- svc, err := people.NewService(ctx, option.WithTokenSource(conf.TokenSource(ctx, token)), option.WithScopes(people.UserEmailsReadScope))
+ svc, err := people.NewService(ctx, option.WithTokenSource(ctrl.oauth2conf.TokenSource(ctx, token)), option.WithScopes(people.UserEmailsReadScope))
if err != nil {
return nil, fmt.Errorf("error setting up People API: %s", err)
}
commit 15dbaf151a72d5cfc00b4ea4b4bcb64c7ed9ac14
Author: Tom Clegg <tom at tomclegg.ca>
Date: Mon Jun 1 11:08:06 2020 -0400
16171: Add OIDC config keys to export whitelist.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/lib/config/export.go b/lib/config/export.go
index 26782c8ba..fc4908c15 100644
--- a/lib/config/export.go
+++ b/lib/config/export.go
@@ -151,6 +151,11 @@ var whitelist = map[string]bool{
"Login.LDAP.URL": false,
"Login.LDAP.UsernameAttribute": false,
"Login.LoginCluster": true,
+ "Login.OpenIDConnect": true,
+ "Login.OpenIDConnect.ClientID": false,
+ "Login.OpenIDConnect.ClientSecret": false,
+ "Login.OpenIDConnect.Enable": true,
+ "Login.OpenIDConnect.Issuer": false,
"Login.PAM": true,
"Login.PAM.DefaultEmailDomain": false,
"Login.PAM.Enable": true,
diff --git a/lib/controller/handler_test.go b/lib/controller/handler_test.go
index 3c7ae3a2d..c7bce9713 100644
--- a/lib/controller/handler_test.go
+++ b/lib/controller/handler_test.go
@@ -71,7 +71,10 @@ func (s *HandlerSuite) TestConfigExport(c *check.C) {
req := httptest.NewRequest(method, "/arvados/v1/config", nil)
resp := httptest.NewRecorder()
s.handler.ServeHTTP(resp, req)
- c.Check(resp.Code, check.Equals, http.StatusOK)
+ c.Log(resp.Body.String())
+ if !c.Check(resp.Code, check.Equals, http.StatusOK) {
+ continue
+ }
c.Check(resp.Header().Get("Access-Control-Allow-Origin"), check.Equals, `*`)
c.Check(resp.Header().Get("Access-Control-Allow-Methods"), check.Matches, `.*\bGET\b.*`)
c.Check(resp.Header().Get("Access-Control-Allow-Headers"), check.Matches, `.+`)
@@ -80,12 +83,11 @@ func (s *HandlerSuite) TestConfigExport(c *check.C) {
continue
}
var cluster arvados.Cluster
- c.Log(resp.Body.String())
err := json.Unmarshal(resp.Body.Bytes(), &cluster)
c.Check(err, check.IsNil)
c.Check(cluster.ManagementToken, check.Equals, "")
c.Check(cluster.SystemRootToken, check.Equals, "")
- c.Check(cluster.Collections.BlobSigning, check.DeepEquals, true)
+ c.Check(cluster.Collections.BlobSigning, check.Equals, true)
c.Check(cluster.Collections.BlobSigningTTL, check.Equals, arvados.Duration(23*time.Second))
}
}
commit 723a7500b82d9c85774a85f66fbeb489f233277c
Author: Tom Clegg <tom at tomclegg.ca>
Date: Mon Jun 1 10:28:15 2020 -0400
16171: Tidy up config test.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/lib/controller/localdb/login_oidc_test.go b/lib/controller/localdb/login_oidc_test.go
index da1dc199e..4a3a2a5ee 100644
--- a/lib/controller/localdb/login_oidc_test.go
+++ b/lib/controller/localdb/login_oidc_test.go
@@ -155,9 +155,6 @@ func (s *OIDCLoginSuite) SetUpTest(c *check.C) {
s.localdb = NewConn(s.cluster)
c.Assert(s.localdb.loginController, check.FitsTypeOf, (*oidcLoginController)(nil))
- c.Check(s.localdb.loginController.(*oidcLoginController).Issuer, check.Equals, "https://accounts.google.com")
- c.Check(s.localdb.loginController.(*oidcLoginController).ClientID, check.Equals, "test%client$id")
- c.Check(s.localdb.loginController.(*oidcLoginController).ClientSecret, check.Equals, "test#client/secret")
s.localdb.loginController.(*oidcLoginController).Issuer = s.fakeIssuer.URL
s.localdb.loginController.(*oidcLoginController).peopleAPIBasePath = s.fakePeopleAPI.URL
@@ -245,25 +242,31 @@ func (s *OIDCLoginSuite) TestGoogleLogin_PeopleAPIDisabled(c *check.C) {
}
func (s *OIDCLoginSuite) TestConfig(c *check.C) {
- // Ensure the UseGooglePeopleAPI flag follows the
- // AlternateEmailAddresses config.
- for _, v := range []bool{false, true} {
- s.cluster.Login.Google.AlternateEmailAddresses = v
- localdb := NewConn(s.cluster)
- c.Check(localdb.loginController.(*oidcLoginController).UseGooglePeopleAPI, check.Equals, v)
- }
-
s.cluster.Login.Google.Enable = false
s.cluster.Login.OpenIDConnect.Enable = true
s.cluster.Login.OpenIDConnect.Issuer = arvados.URL{Scheme: "https", Host: "accounts.example.com", Path: "/"}
s.cluster.Login.OpenIDConnect.ClientID = "oidc-client-id"
s.cluster.Login.OpenIDConnect.ClientSecret = "oidc-client-secret"
localdb := NewConn(s.cluster)
- c.Assert(localdb.loginController, check.FitsTypeOf, (*oidcLoginController)(nil))
- c.Check(localdb.loginController.(*oidcLoginController).Issuer, check.Equals, "https://accounts.example.com/")
- c.Check(localdb.loginController.(*oidcLoginController).ClientID, check.Equals, "oidc-client-id")
- c.Check(localdb.loginController.(*oidcLoginController).ClientSecret, check.Equals, "oidc-client-secret")
- c.Check(localdb.loginController.(*oidcLoginController).UseGooglePeopleAPI, check.Equals, false)
+ ctrl := localdb.loginController.(*oidcLoginController)
+ c.Check(ctrl.Issuer, check.Equals, "https://accounts.example.com/")
+ c.Check(ctrl.ClientID, check.Equals, "oidc-client-id")
+ c.Check(ctrl.ClientSecret, check.Equals, "oidc-client-secret")
+ c.Check(ctrl.UseGooglePeopleAPI, check.Equals, false)
+
+ for _, enableAltEmails := range []bool{false, true} {
+ s.cluster.Login.OpenIDConnect.Enable = false
+ s.cluster.Login.Google.Enable = true
+ s.cluster.Login.Google.ClientID = "google-client-id"
+ s.cluster.Login.Google.ClientSecret = "google-client-secret"
+ s.cluster.Login.Google.AlternateEmailAddresses = enableAltEmails
+ localdb = NewConn(s.cluster)
+ ctrl = localdb.loginController.(*oidcLoginController)
+ c.Check(ctrl.Issuer, check.Equals, "https://accounts.google.com")
+ c.Check(ctrl.ClientID, check.Equals, "google-client-id")
+ c.Check(ctrl.ClientSecret, check.Equals, "google-client-secret")
+ c.Check(ctrl.UseGooglePeopleAPI, check.Equals, enableAltEmails)
+ }
}
func (s *OIDCLoginSuite) TestGoogleLogin_PeopleAPIError(c *check.C) {
commit c3f40023778d234a76b8527b09e53e1ebb25301a
Author: Tom Clegg <tom at tomclegg.ca>
Date: Mon Jun 1 10:17:21 2020 -0400
16171: Don't use Google as example of non-Google OIDC issuer.
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 a0afedca2..d1c47f4a2 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -555,7 +555,7 @@ Clusters:
# Authenticate with an OpenID Connect provider.
Enable: false
- # Issuer URL, e.g., "https://accounts.google.com"
+ # Issuer URL, e.g., "https://login.example.com".
Issuer: ""
# Your client ID and client secret (supplied by the provider).
diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go
index 75200f8fc..0dda58db0 100644
--- a/lib/config/generated_config.go
+++ b/lib/config/generated_config.go
@@ -561,7 +561,7 @@ Clusters:
# Authenticate with an OpenID Connect provider.
Enable: false
- # Issuer URL, e.g., "https://accounts.google.com"
+ # Issuer URL, e.g., "https://login.example.com".
Issuer: ""
# Your client ID and client secret (supplied by the provider).
commit 05223e729d496ae80b8118dea3c03e1a1f6771a0
Author: Tom Clegg <tom at tomclegg.ca>
Date: Sun May 31 19:57:11 2020 -0400
16171: Support non-Google OpenID Connect auth provider.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/doc/install/setup-login.html.textile.liquid b/doc/install/setup-login.html.textile.liquid
index 3fe442c75..572a83f70 100644
--- a/doc/install/setup-login.html.textile.liquid
+++ b/doc/install/setup-login.html.textile.liquid
@@ -12,6 +12,7 @@ SPDX-License-Identifier: CC-BY-SA-3.0
Select one of the following login mechanisms for your cluster.
# If all users will authenticate with Google, "configure Google login":#google.
+# If all users will authenticate with an OpenID Connect provider (other than Google), "configure OpenID Connect":#oidc.
# If all users will authenticate with an existing LDAP service, "configure LDAP":#ldap.
# If all users will authenticate using PAM as configured on your controller node, "configure PAM":#pam.
@@ -42,6 +43,19 @@ Use the <a href="https://console.developers.google.com" target="_blank">Google D
ClientSecret: "zzzzzzzzzzzzzzzzzzzzzzzz"
</pre>
+h2(#oidc). OpenID Connect
+
+With this configuration, users will sign in with a third-party OpenID Connect provider. The provider will supply appropriate values for the issuer URL, client ID, and client secret config entries.
+
+<pre>
+ Login:
+ OpenIDConnect:
+ Enable: true
+ Issuer: https://accounts.example.com/
+ ClientID: "0123456789abcdef"
+ ClientSecret: "zzzzzzzzzzzzzzzzzzzzzzzz"
+</pre>
+
h2(#ldap). LDAP
With this configuration, authentication uses an external LDAP service like OpenLDAP or Active Directory.
diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index 204f7538b..a0afedca2 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -551,6 +551,17 @@ Clusters:
# work. If false, only the primary email address will be used.
AlternateEmailAddresses: true
+ OpenIDConnect:
+ # Authenticate with an OpenID Connect provider.
+ Enable: false
+
+ # Issuer URL, e.g., "https://accounts.google.com"
+ Issuer: ""
+
+ # Your client ID and client secret (supplied by the provider).
+ ClientID: ""
+ ClientSecret: ""
+
PAM:
# (Experimental) Use PAM to authenticate users.
Enable: false
diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go
index ec5bc187d..75200f8fc 100644
--- a/lib/config/generated_config.go
+++ b/lib/config/generated_config.go
@@ -557,6 +557,17 @@ Clusters:
# work. If false, only the primary email address will be used.
AlternateEmailAddresses: true
+ OpenIDConnect:
+ # Authenticate with an OpenID Connect provider.
+ Enable: false
+
+ # Issuer URL, e.g., "https://accounts.google.com"
+ Issuer: ""
+
+ # Your client ID and client secret (supplied by the provider).
+ ClientID: ""
+ ClientSecret: ""
+
PAM:
# (Experimental) Use PAM to authenticate users.
Enable: false
diff --git a/lib/controller/localdb/login.go b/lib/controller/localdb/login.go
index d79dc6358..5231bcb19 100644
--- a/lib/controller/localdb/login.go
+++ b/lib/controller/localdb/login.go
@@ -24,21 +24,37 @@ type loginController interface {
func chooseLoginController(cluster *arvados.Cluster, railsProxy *railsProxy) loginController {
wantGoogle := cluster.Login.Google.Enable
+ wantOpenIDConnect := cluster.Login.OpenIDConnect.Enable
wantSSO := cluster.Login.SSO.Enable
wantPAM := cluster.Login.PAM.Enable
wantLDAP := cluster.Login.LDAP.Enable
switch {
- case wantGoogle && !wantSSO && !wantPAM && !wantLDAP:
- return &oidcLoginController{Cluster: cluster, RailsProxy: railsProxy, Issuer: "https://accounts.google.com", GoogleAPI: true}
- case !wantGoogle && wantSSO && !wantPAM && !wantLDAP:
+ case wantGoogle && !wantOpenIDConnect && !wantSSO && !wantPAM && !wantLDAP:
+ return &oidcLoginController{
+ Cluster: cluster,
+ RailsProxy: railsProxy,
+ Issuer: "https://accounts.google.com",
+ ClientID: cluster.Login.Google.ClientID,
+ ClientSecret: cluster.Login.Google.ClientSecret,
+ UseGooglePeopleAPI: cluster.Login.Google.AlternateEmailAddresses,
+ }
+ case !wantGoogle && wantOpenIDConnect && !wantSSO && !wantPAM && !wantLDAP:
+ return &oidcLoginController{
+ Cluster: cluster,
+ RailsProxy: railsProxy,
+ Issuer: cluster.Login.OpenIDConnect.Issuer.String(),
+ ClientID: cluster.Login.OpenIDConnect.ClientID,
+ ClientSecret: cluster.Login.OpenIDConnect.ClientSecret,
+ }
+ case !wantGoogle && !wantOpenIDConnect && wantSSO && !wantPAM && !wantLDAP:
return &ssoLoginController{railsProxy}
- case !wantGoogle && !wantSSO && wantPAM && !wantLDAP:
+ case !wantGoogle && !wantOpenIDConnect && !wantSSO && wantPAM && !wantLDAP:
return &pamLoginController{Cluster: cluster, RailsProxy: railsProxy}
- case !wantGoogle && !wantSSO && !wantPAM && wantLDAP:
+ case !wantGoogle && !wantOpenIDConnect && !wantSSO && !wantPAM && wantLDAP:
return &ldapLoginController{Cluster: cluster, RailsProxy: railsProxy}
default:
return errorLoginController{
- error: errors.New("configuration problem: exactly one of Login.Google, Login.SSO, Login.PAM, and Login.LDAP must be enabled"),
+ error: errors.New("configuration problem: exactly one of Login.Google, Login.OpenIDConnect, Login.SSO, Login.PAM, and Login.LDAP must be enabled"),
}
}
}
diff --git a/lib/controller/localdb/login_oidc.go b/lib/controller/localdb/login_oidc.go
index e273953de..b0458a7ad 100644
--- a/lib/controller/localdb/login_oidc.go
+++ b/lib/controller/localdb/login_oidc.go
@@ -31,12 +31,14 @@ import (
)
type oidcLoginController struct {
- Cluster *arvados.Cluster
- RailsProxy *railsProxy
- Issuer string // OIDC issuer URL, e.g., "https://accounts.google.com"
- GoogleAPI bool // Issuer is Google; use additional Google APIs/extensions as needed
+ Cluster *arvados.Cluster
+ RailsProxy *railsProxy
+ 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
- peopleAPIBasePath string // override Google People API base URL (normally set by google pkg to https://people.googleapis.com/)
+ peopleAPIBasePath string // override Google People API base URL (normally empty, set by google pkg to https://people.googleapis.com/)
provider *oidc.Provider
mu sync.Mutex
}
@@ -159,7 +161,7 @@ func (ctrl *oidcLoginController) getAuthInfo(ctx context.Context, cluster *arvad
ret.Email = claims.Email
}
- if !ctrl.Cluster.Login.Google.AlternateEmailAddresses || !ctrl.GoogleAPI {
+ if !ctrl.UseGooglePeopleAPI {
if ret.Email == "" {
return nil, fmt.Errorf("cannot log in with unverified email address %q", claims.Email)
}
diff --git a/lib/controller/localdb/login_oidc_test.go b/lib/controller/localdb/login_oidc_test.go
index 59fb8ce05..da1dc199e 100644
--- a/lib/controller/localdb/login_oidc_test.go
+++ b/lib/controller/localdb/login_oidc_test.go
@@ -34,9 +34,9 @@ func Test(t *testing.T) {
check.TestingT(t)
}
-var _ = check.Suite(&LoginSuite{})
+var _ = check.Suite(&OIDCLoginSuite{})
-type LoginSuite struct {
+type OIDCLoginSuite struct {
cluster *arvados.Cluster
ctx context.Context
localdb *Conn
@@ -54,14 +54,14 @@ type LoginSuite struct {
authName string
}
-func (s *LoginSuite) TearDownSuite(c *check.C) {
+func (s *OIDCLoginSuite) TearDownSuite(c *check.C) {
// Undo any changes/additions to the user database so they
// don't affect subsequent tests.
arvadostest.ResetEnv()
c.Check(arvados.NewClientFromEnv().RequestAndDecode(nil, "POST", "database/reset", nil, nil), check.IsNil)
}
-func (s *LoginSuite) SetUpTest(c *check.C) {
+func (s *OIDCLoginSuite) SetUpTest(c *check.C) {
var err error
s.issuerKey, err = rsa.GenerateKey(rand.Reader, 2048)
c.Assert(err, check.IsNil)
@@ -91,8 +91,8 @@ func (s *LoginSuite) SetUpTest(c *check.C) {
"iss": s.fakeIssuer.URL,
"aud": []string{"test%client$id"},
"sub": "fake-user-id",
- "exp": time.Now().UTC().Add(time.Minute).UnixNano(),
- "iat": time.Now().UTC().UnixNano(),
+ "exp": time.Now().UTC().Add(time.Minute).Unix(),
+ "iat": time.Now().UTC().Unix(),
"nonce": "fake-nonce",
"email": s.authEmail,
"email_verified": s.authEmailVerified,
@@ -156,6 +156,8 @@ func (s *LoginSuite) SetUpTest(c *check.C) {
s.localdb = NewConn(s.cluster)
c.Assert(s.localdb.loginController, check.FitsTypeOf, (*oidcLoginController)(nil))
c.Check(s.localdb.loginController.(*oidcLoginController).Issuer, check.Equals, "https://accounts.google.com")
+ c.Check(s.localdb.loginController.(*oidcLoginController).ClientID, check.Equals, "test%client$id")
+ c.Check(s.localdb.loginController.(*oidcLoginController).ClientSecret, check.Equals, "test#client/secret")
s.localdb.loginController.(*oidcLoginController).Issuer = s.fakeIssuer.URL
s.localdb.loginController.(*oidcLoginController).peopleAPIBasePath = s.fakePeopleAPI.URL
@@ -163,24 +165,24 @@ func (s *LoginSuite) SetUpTest(c *check.C) {
*s.localdb.railsProxy = *rpc.NewConn(s.cluster.ClusterID, s.railsSpy.URL, true, rpc.PassthroughTokenProvider)
}
-func (s *LoginSuite) TearDownTest(c *check.C) {
+func (s *OIDCLoginSuite) TearDownTest(c *check.C) {
s.railsSpy.Close()
}
-func (s *LoginSuite) TestGoogleLogout(c *check.C) {
+func (s *OIDCLoginSuite) TestGoogleLogout(c *check.C) {
resp, err := s.localdb.Logout(context.Background(), arvados.LogoutOptions{ReturnTo: "https://foo.example.com/bar"})
c.Check(err, check.IsNil)
c.Check(resp.RedirectLocation, check.Equals, "https://foo.example.com/bar")
}
-func (s *LoginSuite) TestGoogleLogin_Start_Bogus(c *check.C) {
+func (s *OIDCLoginSuite) TestGoogleLogin_Start_Bogus(c *check.C) {
resp, err := s.localdb.Login(context.Background(), arvados.LoginOptions{})
c.Check(err, check.IsNil)
c.Check(resp.RedirectLocation, check.Equals, "")
c.Check(resp.HTML.String(), check.Matches, `.*missing return_to parameter.*`)
}
-func (s *LoginSuite) TestGoogleLogin_Start(c *check.C) {
+func (s *OIDCLoginSuite) TestGoogleLogin_Start(c *check.C) {
for _, remote := range []string{"", "zzzzz"} {
resp, err := s.localdb.Login(context.Background(), arvados.LoginOptions{Remote: remote, ReturnTo: "https://app.example.com/foo?bar"})
c.Check(err, check.IsNil)
@@ -198,7 +200,7 @@ func (s *LoginSuite) TestGoogleLogin_Start(c *check.C) {
}
}
-func (s *LoginSuite) TestGoogleLogin_InvalidCode(c *check.C) {
+func (s *OIDCLoginSuite) TestGoogleLogin_InvalidCode(c *check.C) {
state := s.startLogin(c)
resp, err := s.localdb.Login(context.Background(), arvados.LoginOptions{
Code: "first-try-a-bogus-code",
@@ -209,7 +211,7 @@ func (s *LoginSuite) TestGoogleLogin_InvalidCode(c *check.C) {
c.Check(resp.HTML.String(), check.Matches, `(?ms).*error in OAuth2 exchange.*cannot fetch token.*`)
}
-func (s *LoginSuite) TestGoogleLogin_InvalidState(c *check.C) {
+func (s *OIDCLoginSuite) TestGoogleLogin_InvalidState(c *check.C) {
s.startLogin(c)
resp, err := s.localdb.Login(context.Background(), arvados.LoginOptions{
Code: s.validCode,
@@ -220,7 +222,7 @@ func (s *LoginSuite) TestGoogleLogin_InvalidState(c *check.C) {
c.Check(resp.HTML.String(), check.Matches, `(?ms).*invalid OAuth2 state.*`)
}
-func (s *LoginSuite) setupPeopleAPIError(c *check.C) {
+func (s *OIDCLoginSuite) setupPeopleAPIError(c *check.C) {
s.fakePeopleAPI = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
w.WriteHeader(http.StatusForbidden)
fmt.Fprintln(w, `Error 403: accessNotConfigured`)
@@ -228,8 +230,8 @@ func (s *LoginSuite) setupPeopleAPIError(c *check.C) {
s.localdb.loginController.(*oidcLoginController).peopleAPIBasePath = s.fakePeopleAPI.URL
}
-func (s *LoginSuite) TestGoogleLogin_PeopleAPIDisabled(c *check.C) {
- s.cluster.Login.Google.AlternateEmailAddresses = false
+func (s *OIDCLoginSuite) TestGoogleLogin_PeopleAPIDisabled(c *check.C) {
+ s.localdb.loginController.(*oidcLoginController).UseGooglePeopleAPI = false
s.authEmail = "joe.smith at primary.example.com"
s.setupPeopleAPIError(c)
state := s.startLogin(c)
@@ -242,7 +244,29 @@ func (s *LoginSuite) TestGoogleLogin_PeopleAPIDisabled(c *check.C) {
c.Check(authinfo.Email, check.Equals, "joe.smith at primary.example.com")
}
-func (s *LoginSuite) TestGoogleLogin_PeopleAPIError(c *check.C) {
+func (s *OIDCLoginSuite) TestConfig(c *check.C) {
+ // Ensure the UseGooglePeopleAPI flag follows the
+ // AlternateEmailAddresses config.
+ for _, v := range []bool{false, true} {
+ s.cluster.Login.Google.AlternateEmailAddresses = v
+ localdb := NewConn(s.cluster)
+ c.Check(localdb.loginController.(*oidcLoginController).UseGooglePeopleAPI, check.Equals, v)
+ }
+
+ s.cluster.Login.Google.Enable = false
+ s.cluster.Login.OpenIDConnect.Enable = true
+ s.cluster.Login.OpenIDConnect.Issuer = arvados.URL{Scheme: "https", Host: "accounts.example.com", Path: "/"}
+ s.cluster.Login.OpenIDConnect.ClientID = "oidc-client-id"
+ s.cluster.Login.OpenIDConnect.ClientSecret = "oidc-client-secret"
+ localdb := NewConn(s.cluster)
+ c.Assert(localdb.loginController, check.FitsTypeOf, (*oidcLoginController)(nil))
+ c.Check(localdb.loginController.(*oidcLoginController).Issuer, check.Equals, "https://accounts.example.com/")
+ c.Check(localdb.loginController.(*oidcLoginController).ClientID, check.Equals, "oidc-client-id")
+ c.Check(localdb.loginController.(*oidcLoginController).ClientSecret, check.Equals, "oidc-client-secret")
+ c.Check(localdb.loginController.(*oidcLoginController).UseGooglePeopleAPI, check.Equals, false)
+}
+
+func (s *OIDCLoginSuite) TestGoogleLogin_PeopleAPIError(c *check.C) {
s.setupPeopleAPIError(c)
state := s.startLogin(c)
resp, err := s.localdb.Login(context.Background(), arvados.LoginOptions{
@@ -253,7 +277,7 @@ func (s *LoginSuite) TestGoogleLogin_PeopleAPIError(c *check.C) {
c.Check(resp.RedirectLocation, check.Equals, "")
}
-func (s *LoginSuite) TestGoogleLogin_Success(c *check.C) {
+func (s *OIDCLoginSuite) TestGoogleLogin_Success(c *check.C) {
state := s.startLogin(c)
resp, err := s.localdb.Login(context.Background(), arvados.LoginOptions{
Code: s.validCode,
@@ -292,7 +316,7 @@ func (s *LoginSuite) TestGoogleLogin_Success(c *check.C) {
c.Check(err, check.ErrorMatches, `.*401 Unauthorized: Not logged in.*`)
}
-func (s *LoginSuite) TestGoogleLogin_RealName(c *check.C) {
+func (s *OIDCLoginSuite) TestGoogleLogin_RealName(c *check.C) {
s.authEmail = "joe.smith at primary.example.com"
s.fakePeopleAPIResponse = map[string]interface{}{
"names": []map[string]interface{}{
@@ -319,7 +343,7 @@ func (s *LoginSuite) TestGoogleLogin_RealName(c *check.C) {
c.Check(authinfo.LastName, check.Equals, "Psmith")
}
-func (s *LoginSuite) TestGoogleLogin_OIDCRealName(c *check.C) {
+func (s *OIDCLoginSuite) TestGoogleLogin_OIDCRealName(c *check.C) {
s.authName = "Joe P. Smith"
s.authEmail = "joe.smith at primary.example.com"
state := s.startLogin(c)
@@ -334,7 +358,7 @@ func (s *LoginSuite) TestGoogleLogin_OIDCRealName(c *check.C) {
}
// People API returns some additional email addresses.
-func (s *LoginSuite) TestGoogleLogin_AlternateEmailAddresses(c *check.C) {
+func (s *OIDCLoginSuite) TestGoogleLogin_AlternateEmailAddresses(c *check.C) {
s.authEmail = "joe.smith at primary.example.com"
s.fakePeopleAPIResponse = map[string]interface{}{
"emailAddresses": []map[string]interface{}{
@@ -363,7 +387,7 @@ func (s *LoginSuite) TestGoogleLogin_AlternateEmailAddresses(c *check.C) {
}
// Primary address is not the one initially returned by oidc.
-func (s *LoginSuite) TestGoogleLogin_AlternateEmailAddresses_Primary(c *check.C) {
+func (s *OIDCLoginSuite) TestGoogleLogin_AlternateEmailAddresses_Primary(c *check.C) {
s.authEmail = "joe.smith at alternate.example.com"
s.fakePeopleAPIResponse = map[string]interface{}{
"emailAddresses": []map[string]interface{}{
@@ -392,7 +416,7 @@ func (s *LoginSuite) TestGoogleLogin_AlternateEmailAddresses_Primary(c *check.C)
c.Check(authinfo.Username, check.Equals, "jsmith")
}
-func (s *LoginSuite) TestGoogleLogin_NoPrimaryEmailAddress(c *check.C) {
+func (s *OIDCLoginSuite) TestGoogleLogin_NoPrimaryEmailAddress(c *check.C) {
s.authEmail = "joe.smith at unverified.example.com"
s.authEmailVerified = false
s.fakePeopleAPIResponse = map[string]interface{}{
@@ -419,7 +443,7 @@ func (s *LoginSuite) TestGoogleLogin_NoPrimaryEmailAddress(c *check.C) {
c.Check(authinfo.Username, check.Equals, "")
}
-func (s *LoginSuite) startLogin(c *check.C) (state string) {
+func (s *OIDCLoginSuite) startLogin(c *check.C) (state string) {
// Initiate login, but instead of following the redirect to
// the provider, just grab state from the redirect URL.
resp, err := s.localdb.Login(context.Background(), arvados.LoginOptions{ReturnTo: "https://app.example.com/foo?bar"})
@@ -431,7 +455,7 @@ func (s *LoginSuite) startLogin(c *check.C) (state string) {
return
}
-func (s *LoginSuite) fakeToken(c *check.C, payload []byte) string {
+func (s *OIDCLoginSuite) fakeToken(c *check.C, payload []byte) string {
signer, err := jose.NewSigner(jose.SigningKey{Algorithm: jose.RS256, Key: s.issuerKey}, nil)
if err != nil {
c.Error(err)
diff --git a/sdk/go/arvados/config.go b/sdk/go/arvados/config.go
index 1efc87ea7..0558de808 100644
--- a/sdk/go/arvados/config.go
+++ b/sdk/go/arvados/config.go
@@ -156,6 +156,12 @@ type Cluster struct {
ClientSecret string
AlternateEmailAddresses bool
}
+ OpenIDConnect struct {
+ Enable bool
+ Issuer URL
+ ClientID string
+ ClientSecret string
+ }
PAM struct {
Enable bool
Service string
commit 9ab021c2c2720b563f012b99dfbf3e7034a3c245
Author: Tom Clegg <tom at tomclegg.ca>
Date: Wed May 27 09:44:36 2020 -0400
16171: Rename googleLoginController to oidcLoginController.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/lib/controller/localdb/login.go b/lib/controller/localdb/login.go
index 0fd0a9ad2..d79dc6358 100644
--- a/lib/controller/localdb/login.go
+++ b/lib/controller/localdb/login.go
@@ -29,7 +29,7 @@ func chooseLoginController(cluster *arvados.Cluster, railsProxy *railsProxy) log
wantLDAP := cluster.Login.LDAP.Enable
switch {
case wantGoogle && !wantSSO && !wantPAM && !wantLDAP:
- return &googleLoginController{Cluster: cluster, RailsProxy: railsProxy}
+ return &oidcLoginController{Cluster: cluster, RailsProxy: railsProxy, Issuer: "https://accounts.google.com", GoogleAPI: true}
case !wantGoogle && wantSSO && !wantPAM && !wantLDAP:
return &ssoLoginController{railsProxy}
case !wantGoogle && !wantSSO && wantPAM && !wantLDAP:
diff --git a/lib/controller/localdb/login_google.go b/lib/controller/localdb/login_oidc.go
similarity index 87%
rename from lib/controller/localdb/login_google.go
rename to lib/controller/localdb/login_oidc.go
index 144b04c46..e273953de 100644
--- a/lib/controller/localdb/login_google.go
+++ b/lib/controller/localdb/login_oidc.go
@@ -30,25 +30,22 @@ import (
"google.golang.org/api/people/v1"
)
-type googleLoginController struct {
+type oidcLoginController struct {
Cluster *arvados.Cluster
RailsProxy *railsProxy
+ Issuer string // OIDC issuer URL, e.g., "https://accounts.google.com"
+ GoogleAPI bool // Issuer is Google; use additional Google APIs/extensions as needed
- issuer string // override OIDC issuer URL (normally https://accounts.google.com) for testing
peopleAPIBasePath string // override Google People API base URL (normally set by google pkg to https://people.googleapis.com/)
provider *oidc.Provider
mu sync.Mutex
}
-func (ctrl *googleLoginController) getProvider() (*oidc.Provider, error) {
+func (ctrl *oidcLoginController) getProvider() (*oidc.Provider, error) {
ctrl.mu.Lock()
defer ctrl.mu.Unlock()
if ctrl.provider == nil {
- issuer := ctrl.issuer
- if issuer == "" {
- issuer = "https://accounts.google.com"
- }
- provider, err := oidc.NewProvider(context.Background(), issuer)
+ provider, err := oidc.NewProvider(context.Background(), ctrl.Issuer)
if err != nil {
return nil, err
}
@@ -57,11 +54,11 @@ func (ctrl *googleLoginController) getProvider() (*oidc.Provider, error) {
return ctrl.provider, nil
}
-func (ctrl *googleLoginController) Logout(ctx context.Context, opts arvados.LogoutOptions) (arvados.LogoutResponse, error) {
+func (ctrl *oidcLoginController) Logout(ctx context.Context, opts arvados.LogoutOptions) (arvados.LogoutResponse, error) {
return noopLogout(ctrl.Cluster, opts)
}
-func (ctrl *googleLoginController) Login(ctx context.Context, opts arvados.LoginOptions) (arvados.LoginResponse, error) {
+func (ctrl *oidcLoginController) Login(ctx context.Context, opts arvados.LoginOptions) (arvados.LoginResponse, error) {
provider, err := ctrl.getProvider()
if err != nil {
return loginError(fmt.Errorf("error setting up OpenID Connect provider: %s", err))
@@ -81,7 +78,7 @@ func (ctrl *googleLoginController) Login(ctx context.Context, opts arvados.Login
ClientID: conf.ClientID,
})
if opts.State == "" {
- // Initiate Google sign-in.
+ // Initiate OIDC sign-in.
if opts.ReturnTo == "" {
return loginError(errors.New("missing return_to parameter"))
}
@@ -102,7 +99,7 @@ func (ctrl *googleLoginController) Login(ctx context.Context, opts arvados.Login
oauth2.SetAuthURLParam("prompt", "select_account")),
}, nil
} else {
- // Callback after Google sign-in.
+ // Callback after OIDC sign-in.
state := ctrl.parseOAuth2State(opts.State)
if !state.verify([]byte(ctrl.Cluster.SystemRootToken)) {
return loginError(errors.New("invalid OAuth2 state"))
@@ -131,7 +128,7 @@ func (ctrl *googleLoginController) Login(ctx context.Context, opts arvados.Login
}
}
-func (ctrl *googleLoginController) UserAuthenticate(ctx context.Context, opts arvados.UserAuthenticateOptions) (arvados.APIClientAuthorization, error) {
+func (ctrl *oidcLoginController) UserAuthenticate(ctx context.Context, opts arvados.UserAuthenticateOptions) (arvados.APIClientAuthorization, error) {
return arvados.APIClientAuthorization{}, httpserver.ErrorWithStatus(errors.New("username/password authentication is not available"), http.StatusBadRequest)
}
@@ -139,7 +136,7 @@ func (ctrl *googleLoginController) UserAuthenticate(ctx context.Context, opts ar
// primary address at index 0. The provided defaultAddr is always
// included in the returned slice, and is used as the primary if the
// Google API does not indicate one.
-func (ctrl *googleLoginController) getAuthInfo(ctx context.Context, cluster *arvados.Cluster, conf *oauth2.Config, token *oauth2.Token, idToken *oidc.IDToken) (*rpc.UserSessionAuthInfo, error) {
+func (ctrl *oidcLoginController) getAuthInfo(ctx context.Context, cluster *arvados.Cluster, conf *oauth2.Config, token *oauth2.Token, idToken *oidc.IDToken) (*rpc.UserSessionAuthInfo, error) {
var ret rpc.UserSessionAuthInfo
defer ctxlog.FromContext(ctx).WithField("ret", &ret).Debug("getAuthInfo returned")
@@ -162,7 +159,7 @@ func (ctrl *googleLoginController) getAuthInfo(ctx context.Context, cluster *arv
ret.Email = claims.Email
}
- if !ctrl.Cluster.Login.Google.AlternateEmailAddresses {
+ if !ctrl.Cluster.Login.Google.AlternateEmailAddresses || !ctrl.GoogleAPI {
if ret.Email == "" {
return nil, fmt.Errorf("cannot log in with unverified email address %q", claims.Email)
}
@@ -237,7 +234,7 @@ func loginError(sendError error) (resp arvados.LoginResponse, err error) {
return
}
-func (ctrl *googleLoginController) newOAuth2State(key []byte, remote, returnTo string) oauth2State {
+func (ctrl *oidcLoginController) newOAuth2State(key []byte, remote, returnTo string) oauth2State {
s := oauth2State{
Time: time.Now().Unix(),
Remote: remote,
@@ -254,7 +251,7 @@ type oauth2State struct {
ReturnTo string // redirect target
}
-func (ctrl *googleLoginController) parseOAuth2State(encoded string) (s oauth2State) {
+func (ctrl *oidcLoginController) parseOAuth2State(encoded string) (s oauth2State) {
// Errors are not checked. If decoding/parsing fails, the
// token will be rejected by verify().
decoded, _ := base64.RawURLEncoding.DecodeString(encoded)
diff --git a/lib/controller/localdb/login_google_test.go b/lib/controller/localdb/login_oidc_test.go
similarity index 96%
rename from lib/controller/localdb/login_google_test.go
rename to lib/controller/localdb/login_oidc_test.go
index 495fbb69b..59fb8ce05 100644
--- a/lib/controller/localdb/login_google_test.go
+++ b/lib/controller/localdb/login_oidc_test.go
@@ -154,8 +154,10 @@ func (s *LoginSuite) SetUpTest(c *check.C) {
c.Assert(err, check.IsNil)
s.localdb = NewConn(s.cluster)
- s.localdb.loginController.(*googleLoginController).issuer = s.fakeIssuer.URL
- s.localdb.loginController.(*googleLoginController).peopleAPIBasePath = s.fakePeopleAPI.URL
+ c.Assert(s.localdb.loginController, check.FitsTypeOf, (*oidcLoginController)(nil))
+ c.Check(s.localdb.loginController.(*oidcLoginController).Issuer, check.Equals, "https://accounts.google.com")
+ s.localdb.loginController.(*oidcLoginController).Issuer = s.fakeIssuer.URL
+ s.localdb.loginController.(*oidcLoginController).peopleAPIBasePath = s.fakePeopleAPI.URL
s.railsSpy = arvadostest.NewProxy(c, s.cluster.Services.RailsAPI)
*s.localdb.railsProxy = *rpc.NewConn(s.cluster.ClusterID, s.railsSpy.URL, true, rpc.PassthroughTokenProvider)
@@ -188,7 +190,7 @@ func (s *LoginSuite) TestGoogleLogin_Start(c *check.C) {
c.Check(target.Host, check.Equals, issuerURL.Host)
q := target.Query()
c.Check(q.Get("client_id"), check.Equals, "test%client$id")
- state := s.localdb.loginController.(*googleLoginController).parseOAuth2State(q.Get("state"))
+ state := s.localdb.loginController.(*oidcLoginController).parseOAuth2State(q.Get("state"))
c.Check(state.verify([]byte(s.cluster.SystemRootToken)), check.Equals, true)
c.Check(state.Time, check.Not(check.Equals), 0)
c.Check(state.Remote, check.Equals, remote)
@@ -223,7 +225,7 @@ func (s *LoginSuite) setupPeopleAPIError(c *check.C) {
w.WriteHeader(http.StatusForbidden)
fmt.Fprintln(w, `Error 403: accessNotConfigured`)
}))
- s.localdb.loginController.(*googleLoginController).peopleAPIBasePath = s.fakePeopleAPI.URL
+ s.localdb.loginController.(*oidcLoginController).peopleAPIBasePath = s.fakePeopleAPI.URL
}
func (s *LoginSuite) TestGoogleLogin_PeopleAPIDisabled(c *check.C) {
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list