Merge branch 'main' into 18842-arv-mount-disk-config
[arvados.git] / lib / controller / localdb / login_oidc_test.go
index aa437218ff79eaae26ee93d8450af72561387919..0fe3bdf7f6b684652cad9c71f3c0a63fba15b925 100644 (file)
@@ -7,16 +7,17 @@ package localdb
 import (
        "bytes"
        "context"
-       "crypto/rand"
-       "crypto/rsa"
-       "encoding/base64"
+       "crypto/hmac"
+       "crypto/sha256"
        "encoding/json"
        "fmt"
+       "io"
        "net/http"
        "net/http/httptest"
        "net/url"
        "sort"
        "strings"
+       "sync"
        "testing"
        "time"
 
@@ -26,8 +27,8 @@ import (
        "git.arvados.org/arvados.git/sdk/go/arvadostest"
        "git.arvados.org/arvados.git/sdk/go/auth"
        "git.arvados.org/arvados.git/sdk/go/ctxlog"
+       "github.com/jmoiron/sqlx"
        check "gopkg.in/check.v1"
-       jose "gopkg.in/square/go-jose.v2"
 )
 
 // Gocheck boilerplate
@@ -38,23 +39,11 @@ func Test(t *testing.T) {
 var _ = check.Suite(&OIDCLoginSuite{})
 
 type OIDCLoginSuite struct {
-       cluster               *arvados.Cluster
-       ctx                   context.Context
-       localdb               *Conn
-       railsSpy              *arvadostest.Proxy
-       fakeIssuer            *httptest.Server
-       fakePeopleAPI         *httptest.Server
-       fakePeopleAPIResponse map[string]interface{}
-       issuerKey             *rsa.PrivateKey
-
-       // expected token request
-       validCode         string
-       validClientID     string
-       validClientSecret string
-       // desired response from token endpoint
-       authEmail         string
-       authEmailVerified bool
-       authName          string
+       cluster      *arvados.Cluster
+       localdb      *Conn
+       railsSpy     *arvadostest.Proxy
+       trustedURL   *arvados.URL
+       fakeProvider *arvadostest.OIDCProvider
 }
 
 func (s *OIDCLoginSuite) TearDownSuite(c *check.C) {
@@ -65,117 +54,34 @@ func (s *OIDCLoginSuite) TearDownSuite(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)
+       s.trustedURL = &arvados.URL{Scheme: "https", Host: "app.example.com", Path: "/"}
 
-       s.authEmail = "active-user@arvados.local"
-       s.authEmailVerified = true
-       s.authName = "Fake User Name"
-       s.fakeIssuer = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
-               req.ParseForm()
-               c.Logf("fakeIssuer: got req: %s %s %s", req.Method, req.URL, req.Form)
-               w.Header().Set("Content-Type", "application/json")
-               switch req.URL.Path {
-               case "/.well-known/openid-configuration":
-                       json.NewEncoder(w).Encode(map[string]interface{}{
-                               "issuer":                 s.fakeIssuer.URL,
-                               "authorization_endpoint": s.fakeIssuer.URL + "/auth",
-                               "token_endpoint":         s.fakeIssuer.URL + "/token",
-                               "jwks_uri":               s.fakeIssuer.URL + "/jwks",
-                               "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{clientID},
-                               "sub":            "fake-user-id",
-                               "exp":            time.Now().UTC().Add(time.Minute).Unix(),
-                               "iat":            time.Now().UTC().Unix(),
-                               "nonce":          "fake-nonce",
-                               "email":          s.authEmail,
-                               "email_verified": s.authEmailVerified,
-                               "name":           s.authName,
-                       })
-                       json.NewEncoder(w).Encode(struct {
-                               AccessToken  string `json:"access_token"`
-                               TokenType    string `json:"token_type"`
-                               RefreshToken string `json:"refresh_token"`
-                               ExpiresIn    int32  `json:"expires_in"`
-                               IDToken      string `json:"id_token"`
-                       }{
-                               AccessToken:  s.fakeToken(c, []byte("fake access token")),
-                               TokenType:    "Bearer",
-                               RefreshToken: "test-refresh-token",
-                               ExpiresIn:    30,
-                               IDToken:      s.fakeToken(c, idToken),
-                       })
-               case "/jwks":
-                       json.NewEncoder(w).Encode(jose.JSONWebKeySet{
-                               Keys: []jose.JSONWebKey{
-                                       {Key: s.issuerKey.Public(), Algorithm: string(jose.RS256), KeyID: ""},
-                               },
-                       })
-               case "/auth":
-                       w.WriteHeader(http.StatusInternalServerError)
-               case "/userinfo":
-                       w.WriteHeader(http.StatusInternalServerError)
-               default:
-                       w.WriteHeader(http.StatusNotFound)
-               }
-       }))
-       s.validCode = fmt.Sprintf("abcdefgh-%d", time.Now().Unix())
-
-       s.fakePeopleAPI = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
-               req.ParseForm()
-               c.Logf("fakePeopleAPI: got req: %s %s %s", req.Method, req.URL, req.Form)
-               w.Header().Set("Content-Type", "application/json")
-               switch req.URL.Path {
-               case "/v1/people/me":
-                       if f := req.Form.Get("personFields"); f != "emailAddresses,names" {
-                               w.WriteHeader(http.StatusBadRequest)
-                               break
-                       }
-                       json.NewEncoder(w).Encode(s.fakePeopleAPIResponse)
-               default:
-                       w.WriteHeader(http.StatusNotFound)
-               }
-       }))
-       s.fakePeopleAPIResponse = map[string]interface{}{}
+       s.fakeProvider = arvadostest.NewOIDCProvider(c)
+       s.fakeProvider.AuthEmail = "active-user@arvados.local"
+       s.fakeProvider.AuthEmailVerified = true
+       s.fakeProvider.AuthName = "Fake User Name"
+       s.fakeProvider.AuthGivenName = "Fake"
+       s.fakeProvider.AuthFamilyName = "User Name"
+       s.fakeProvider.ValidCode = fmt.Sprintf("abcdefgh-%d", time.Now().Unix())
+       s.fakeProvider.PeopleAPIResponse = 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.Test.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.Login.TrustedClients = map[arvados.URL]struct{}{*s.trustedURL: {}}
        s.cluster.Users.PreferDomainForUsername = "PreferDomainForUsername.example.com"
-       s.validClientID = "test%client$id"
-       s.validClientSecret = "test#client/secret"
+       s.fakeProvider.ValidClientID = "test%client$id"
+       s.fakeProvider.ValidClientSecret = "test#client/secret"
 
        s.localdb = NewConn(s.cluster)
        c.Assert(s.localdb.loginController, check.FitsTypeOf, (*oidcLoginController)(nil))
-       s.localdb.loginController.(*oidcLoginController).Issuer = s.fakeIssuer.URL
-       s.localdb.loginController.(*oidcLoginController).peopleAPIBasePath = s.fakePeopleAPI.URL
+       s.localdb.loginController.(*oidcLoginController).Issuer = s.fakeProvider.Issuer.URL
+       s.localdb.loginController.(*oidcLoginController).peopleAPIBasePath = s.fakeProvider.PeopleAPI.URL
 
        s.railsSpy = arvadostest.NewProxy(c, s.cluster.Services.RailsAPI)
        *s.localdb.railsProxy = *rpc.NewConn(s.cluster.ClusterID, s.railsSpy.URL, true, rpc.PassthroughTokenProvider)
@@ -186,9 +92,26 @@ func (s *OIDCLoginSuite) TearDownTest(c *check.C) {
 }
 
 func (s *OIDCLoginSuite) TestGoogleLogout(c *check.C) {
+       s.cluster.Login.TrustedClients[arvados.URL{Scheme: "https", Host: "foo.example", Path: "/"}] = struct{}{}
+       s.cluster.Login.TrustPrivateNetworks = false
+
        resp, err := s.localdb.Logout(context.Background(), arvados.LogoutOptions{ReturnTo: "https://foo.example.com/bar"})
+       c.Check(err, check.NotNil)
+       c.Check(resp.RedirectLocation, check.Equals, "")
+
+       resp, err = s.localdb.Logout(context.Background(), arvados.LogoutOptions{ReturnTo: "https://127.0.0.1/bar"})
+       c.Check(err, check.NotNil)
+       c.Check(resp.RedirectLocation, check.Equals, "")
+
+       resp, err = s.localdb.Logout(context.Background(), arvados.LogoutOptions{ReturnTo: "https://foo.example/bar"})
+       c.Check(err, check.IsNil)
+       c.Check(resp.RedirectLocation, check.Equals, "https://foo.example/bar")
+
+       s.cluster.Login.TrustPrivateNetworks = true
+
+       resp, err = s.localdb.Logout(context.Background(), arvados.LogoutOptions{ReturnTo: "https://192.168.1.1/bar"})
        c.Check(err, check.IsNil)
-       c.Check(resp.RedirectLocation, check.Equals, "https://foo.example.com/bar")
+       c.Check(resp.RedirectLocation, check.Equals, "https://192.168.1.1/bar")
 }
 
 func (s *OIDCLoginSuite) TestGoogleLogin_Start_Bogus(c *check.C) {
@@ -204,7 +127,7 @@ func (s *OIDCLoginSuite) TestGoogleLogin_Start(c *check.C) {
                c.Check(err, check.IsNil)
                target, err := url.Parse(resp.RedirectLocation)
                c.Check(err, check.IsNil)
-               issuerURL, _ := url.Parse(s.fakeIssuer.URL)
+               issuerURL, _ := url.Parse(s.fakeProvider.Issuer.URL)
                c.Check(target.Host, check.Equals, issuerURL.Host)
                q := target.Query()
                c.Check(q.Get("client_id"), check.Equals, "test%client$id")
@@ -216,6 +139,13 @@ func (s *OIDCLoginSuite) TestGoogleLogin_Start(c *check.C) {
        }
 }
 
+func (s *OIDCLoginSuite) TestGoogleLogin_UnknownClient(c *check.C) {
+       resp, err := s.localdb.Login(context.Background(), arvados.LoginOptions{ReturnTo: "https://bad-app.example.com/foo?bar"})
+       c.Check(err, check.IsNil)
+       c.Check(resp.RedirectLocation, check.Equals, "")
+       c.Check(resp.HTML.String(), check.Matches, `(?ms).*requesting site is not listed in TrustedClients.*`)
+}
+
 func (s *OIDCLoginSuite) TestGoogleLogin_InvalidCode(c *check.C) {
        state := s.startLogin(c)
        resp, err := s.localdb.Login(context.Background(), arvados.LoginOptions{
@@ -230,7 +160,7 @@ func (s *OIDCLoginSuite) TestGoogleLogin_InvalidCode(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,
+               Code:  s.fakeProvider.ValidCode,
                State: "bogus-state",
        })
        c.Check(err, check.IsNil)
@@ -239,20 +169,20 @@ func (s *OIDCLoginSuite) TestGoogleLogin_InvalidState(c *check.C) {
 }
 
 func (s *OIDCLoginSuite) setupPeopleAPIError(c *check.C) {
-       s.fakePeopleAPI = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
+       s.fakeProvider.PeopleAPI = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
                w.WriteHeader(http.StatusForbidden)
                fmt.Fprintln(w, `Error 403: accessNotConfigured`)
        }))
-       s.localdb.loginController.(*oidcLoginController).peopleAPIBasePath = s.fakePeopleAPI.URL
+       s.localdb.loginController.(*oidcLoginController).peopleAPIBasePath = s.fakeProvider.PeopleAPI.URL
 }
 
 func (s *OIDCLoginSuite) TestGoogleLogin_PeopleAPIDisabled(c *check.C) {
        s.localdb.loginController.(*oidcLoginController).UseGooglePeopleAPI = false
-       s.authEmail = "joe.smith@primary.example.com"
+       s.fakeProvider.AuthEmail = "joe.smith@primary.example.com"
        s.setupPeopleAPIError(c)
        state := s.startLogin(c)
        _, err := s.localdb.Login(context.Background(), arvados.LoginOptions{
-               Code:  s.validCode,
+               Code:  s.fakeProvider.ValidCode,
                State: state,
        })
        c.Check(err, check.IsNil)
@@ -266,12 +196,14 @@ func (s *OIDCLoginSuite) TestConfig(c *check.C) {
        s.cluster.Login.OpenIDConnect.Issuer = "https://accounts.example.com/"
        s.cluster.Login.OpenIDConnect.ClientID = "oidc-client-id"
        s.cluster.Login.OpenIDConnect.ClientSecret = "oidc-client-secret"
+       s.cluster.Login.OpenIDConnect.AuthenticationRequestParameters = map[string]string{"testkey": "testvalue"}
        localdb := NewConn(s.cluster)
        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)
+       c.Check(ctrl.AuthParams["testkey"], check.Equals, "testvalue")
 
        for _, enableAltEmails := range []bool{false, true} {
                s.cluster.Login.OpenIDConnect.Enable = false
@@ -279,12 +211,14 @@ func (s *OIDCLoginSuite) TestConfig(c *check.C) {
                s.cluster.Login.Google.ClientID = "google-client-id"
                s.cluster.Login.Google.ClientSecret = "google-client-secret"
                s.cluster.Login.Google.AlternateEmailAddresses = enableAltEmails
+               s.cluster.Login.Google.AuthenticationRequestParameters = map[string]string{"testkey": "testvalue"}
                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)
+               c.Check(ctrl.AuthParams["testkey"], check.Equals, "testvalue")
        }
 }
 
@@ -292,39 +226,251 @@ func (s *OIDCLoginSuite) TestGoogleLogin_PeopleAPIError(c *check.C) {
        s.setupPeopleAPIError(c)
        state := s.startLogin(c)
        resp, err := s.localdb.Login(context.Background(), arvados.LoginOptions{
-               Code:  s.validCode,
+               Code:  s.fakeProvider.ValidCode,
                State: state,
        })
        c.Check(err, check.IsNil)
        c.Check(resp.RedirectLocation, check.Equals, "")
 }
 
-func (s *OIDCLoginSuite) TestOIDCLogin_Success(c *check.C) {
+func (s *OIDCLoginSuite) TestOIDCAuthorizer(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)
+       json.Unmarshal([]byte(fmt.Sprintf("%q", s.fakeProvider.Issuer.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}`)
+       s.cluster.Login.OpenIDConnect.AcceptAccessToken = true
+       s.cluster.Login.OpenIDConnect.AcceptAccessTokenScope = ""
+       s.fakeProvider.ValidClientID = "oidc#client#id"
+       s.fakeProvider.ValidClientSecret = "oidc#client#secret"
+       db := arvadostest.DB(c, s.cluster)
+
+       tokenCacheTTL = time.Millisecond
+       tokenCacheRaceWindow = time.Millisecond
+       tokenCacheNegativeTTL = time.Millisecond
+
+       oidcAuthorizer := OIDCAccessTokenAuthorizer(s.cluster, func(context.Context) (*sqlx.DB, error) { return db, nil })
+       accessToken := s.fakeProvider.ValidAccessToken()
+
+       mac := hmac.New(sha256.New, []byte(s.cluster.SystemRootToken))
+       io.WriteString(mac, accessToken)
+       apiToken := fmt.Sprintf("%x", mac.Sum(nil))
+
+       cleanup := func() {
+               _, err := db.Exec(`delete from api_client_authorizations where api_token=$1`, apiToken)
+               c.Check(err, check.IsNil)
+       }
+       cleanup()
+       defer cleanup()
+
+       ctx := auth.NewContext(context.Background(), &auth.Credentials{Tokens: []string{accessToken}})
+       var exp1 time.Time
+
+       concurrent := 4
+       s.fakeProvider.HoldUserInfo = make(chan *http.Request)
+       s.fakeProvider.ReleaseUserInfo = make(chan struct{})
+       go func() {
+               for i := 0; ; i++ {
+                       if i == concurrent {
+                               close(s.fakeProvider.ReleaseUserInfo)
+                       }
+                       <-s.fakeProvider.HoldUserInfo
+               }
+       }()
+       var wg sync.WaitGroup
+       for i := 0; i < concurrent; i++ {
+               i := i
+               wg.Add(1)
+               go func() {
+                       defer wg.Done()
+                       _, err := oidcAuthorizer.WrapCalls(func(ctx context.Context, opts interface{}) (interface{}, error) {
+                               c.Logf("concurrent req %d/%d", i, concurrent)
+                               var exp time.Time
+
+                               creds, ok := auth.FromContext(ctx)
+                               c.Assert(ok, check.Equals, true)
+                               c.Assert(creds.Tokens, check.HasLen, 1)
+                               c.Check(creds.Tokens[0], check.Equals, accessToken)
+
+                               err := db.QueryRowContext(ctx, `select expires_at at time zone 'UTC' from api_client_authorizations where api_token=$1`, apiToken).Scan(&exp)
+                               c.Check(err, check.IsNil)
+                               c.Check(exp.Sub(time.Now()) > -time.Second, check.Equals, true)
+                               c.Check(exp.Sub(time.Now()) < time.Second, check.Equals, true)
+                               if i == 0 {
+                                       exp1 = exp
+                               }
+                               return nil, nil
+                       })(ctx, nil)
+                       c.Check(err, check.IsNil)
+               }()
+       }
+       wg.Wait()
+       if c.Failed() {
+               c.Fatal("giving up")
+       }
+
+       // If the token is used again after the in-memory cache
+       // expires, oidcAuthorizer must re-check the token and update
+       // the expires_at value in the database.
+       time.Sleep(3 * time.Millisecond)
+       oidcAuthorizer.WrapCalls(func(ctx context.Context, opts interface{}) (interface{}, error) {
+               var exp time.Time
+               err := db.QueryRowContext(ctx, `select expires_at at time zone 'UTC' from api_client_authorizations where api_token=$1`, apiToken).Scan(&exp)
+               c.Check(err, check.IsNil)
+               c.Check(exp.Sub(exp1) > 0, check.Equals, true, check.Commentf("expect %v > 0", exp.Sub(exp1)))
+               c.Check(exp.Sub(exp1) < time.Second, check.Equals, true, check.Commentf("expect %v < 1s", exp.Sub(exp1)))
+               return nil, nil
+       })(ctx, nil)
+
+       s.fakeProvider.AccessTokenPayload = map[string]interface{}{"scope": "openid profile foobar"}
+       accessToken = s.fakeProvider.ValidAccessToken()
+       ctx = auth.NewContext(context.Background(), &auth.Credentials{Tokens: []string{accessToken}})
+
+       mac = hmac.New(sha256.New, []byte(s.cluster.SystemRootToken))
+       io.WriteString(mac, accessToken)
+       apiToken = fmt.Sprintf("%x", mac.Sum(nil))
+
+       for _, trial := range []struct {
+               configEnable bool
+               configScope  string
+               acceptable   bool
+               shouldRun    bool
+       }{
+               {true, "foobar", true, true},
+               {true, "foo", false, false},
+               {true, "", true, true},
+               {false, "", false, true},
+               {false, "foobar", false, true},
+       } {
+               c.Logf("trial = %+v", trial)
+               cleanup()
+               s.cluster.Login.OpenIDConnect.AcceptAccessToken = trial.configEnable
+               s.cluster.Login.OpenIDConnect.AcceptAccessTokenScope = trial.configScope
+               oidcAuthorizer = OIDCAccessTokenAuthorizer(s.cluster, func(context.Context) (*sqlx.DB, error) { return db, nil })
+               checked := false
+               oidcAuthorizer.WrapCalls(func(ctx context.Context, opts interface{}) (interface{}, error) {
+                       var n int
+                       err := db.QueryRowContext(ctx, `select count(*) from api_client_authorizations where api_token=$1`, apiToken).Scan(&n)
+                       c.Check(err, check.IsNil)
+                       if trial.acceptable {
+                               c.Check(n, check.Equals, 1)
+                       } else {
+                               c.Check(n, check.Equals, 0)
+                       }
+                       checked = true
+                       return nil, nil
+               })(ctx, nil)
+               c.Check(checked, check.Equals, trial.shouldRun)
+       }
+}
+
+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.fakeProvider.Issuer.URL)), &s.cluster.Login.OpenIDConnect.Issuer)
+       s.cluster.Login.OpenIDConnect.ClientID = "oidc#client#id"
+       s.cluster.Login.OpenIDConnect.ClientSecret = "oidc#client#secret"
+       s.cluster.Login.OpenIDConnect.AuthenticationRequestParameters = map[string]string{"testkey": "testvalue"}
+       s.fakeProvider.ValidClientID = "oidc#client#id"
+       s.fakeProvider.ValidClientSecret = "oidc#client#secret"
+       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.fakeProvider.AuthEmail = "user@oidc.example.com"
+                               s.fakeProvider.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.fakeProvider.AuthEmail = "user@oidc.example.com"
+                               s.fakeProvider.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.fakeProvider.AuthEmail = "user@oidc.example.com"
+                               s.fakeProvider.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.fakeProvider.AuthEmail = "bad@wrong.example.com"
+                               s.fakeProvider.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, func(form url.Values) {
+                       c.Check(form.Get("testkey"), check.Equals, "testvalue")
+               })
+               resp, err := s.localdb.Login(context.Background(), arvados.LoginOptions{
+                       Code:  s.fakeProvider.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) {
-       state := s.startLogin(c)
+       s.cluster.Login.Google.AuthenticationRequestParameters["prompt"] = "consent"
+       s.cluster.Login.Google.AuthenticationRequestParameters["foo"] = "bar"
+       state := s.startLogin(c, func(form url.Values) {
+               c.Check(form.Get("foo"), check.Equals, "bar")
+               c.Check(form.Get("prompt"), check.Equals, "consent")
+       })
        resp, err := s.localdb.Login(context.Background(), arvados.LoginOptions{
-               Code:  s.validCode,
+               Code:  s.fakeProvider.ValidCode,
                State: state,
        })
        c.Check(err, check.IsNil)
@@ -337,8 +483,8 @@ func (s *OIDCLoginSuite) TestGoogleLogin_Success(c *check.C) {
        c.Check(token, check.Matches, `v2/zzzzz-gj3su-.{15}/.{32,50}`)
 
        authinfo := getCallbackAuthInfo(c, s.railsSpy)
-       c.Check(authinfo.FirstName, check.Equals, "Fake User")
-       c.Check(authinfo.LastName, check.Equals, "Name")
+       c.Check(authinfo.FirstName, check.Equals, "Fake")
+       c.Check(authinfo.LastName, check.Equals, "User Name")
        c.Check(authinfo.Email, check.Equals, "active-user@arvados.local")
        c.Check(authinfo.AlternateEmails, check.HasLen, 0)
 
@@ -361,8 +507,9 @@ func (s *OIDCLoginSuite) TestGoogleLogin_Success(c *check.C) {
 }
 
 func (s *OIDCLoginSuite) TestGoogleLogin_RealName(c *check.C) {
-       s.authEmail = "joe.smith@primary.example.com"
-       s.fakePeopleAPIResponse = map[string]interface{}{
+       s.fakeProvider.AuthEmail = "joe.smith@primary.example.com"
+       s.fakeProvider.AuthEmailVerified = true
+       s.fakeProvider.PeopleAPIResponse = map[string]interface{}{
                "names": []map[string]interface{}{
                        {
                                "metadata":   map[string]interface{}{"primary": false},
@@ -378,7 +525,7 @@ func (s *OIDCLoginSuite) TestGoogleLogin_RealName(c *check.C) {
        }
        state := s.startLogin(c)
        s.localdb.Login(context.Background(), arvados.LoginOptions{
-               Code:  s.validCode,
+               Code:  s.fakeProvider.ValidCode,
                State: state,
        })
 
@@ -387,12 +534,14 @@ func (s *OIDCLoginSuite) TestGoogleLogin_RealName(c *check.C) {
        c.Check(authinfo.LastName, check.Equals, "Psmith")
 }
 
-func (s *OIDCLoginSuite) TestGoogleLogin_OIDCRealName(c *check.C) {
-       s.authName = "Joe P. Smith"
-       s.authEmail = "joe.smith@primary.example.com"
+func (s *OIDCLoginSuite) TestGoogleLogin_OIDCNameWithoutGivenAndFamilyNames(c *check.C) {
+       s.fakeProvider.AuthName = "Joe P. Smith"
+       s.fakeProvider.AuthGivenName = ""
+       s.fakeProvider.AuthFamilyName = ""
+       s.fakeProvider.AuthEmail = "joe.smith@primary.example.com"
        state := s.startLogin(c)
        s.localdb.Login(context.Background(), arvados.LoginOptions{
-               Code:  s.validCode,
+               Code:  s.fakeProvider.ValidCode,
                State: state,
        })
 
@@ -403,8 +552,8 @@ func (s *OIDCLoginSuite) TestGoogleLogin_OIDCRealName(c *check.C) {
 
 // People API returns some additional email addresses.
 func (s *OIDCLoginSuite) TestGoogleLogin_AlternateEmailAddresses(c *check.C) {
-       s.authEmail = "joe.smith@primary.example.com"
-       s.fakePeopleAPIResponse = map[string]interface{}{
+       s.fakeProvider.AuthEmail = "joe.smith@primary.example.com"
+       s.fakeProvider.PeopleAPIResponse = map[string]interface{}{
                "emailAddresses": []map[string]interface{}{
                        {
                                "metadata": map[string]interface{}{"verified": true},
@@ -421,7 +570,7 @@ func (s *OIDCLoginSuite) TestGoogleLogin_AlternateEmailAddresses(c *check.C) {
        }
        state := s.startLogin(c)
        s.localdb.Login(context.Background(), arvados.LoginOptions{
-               Code:  s.validCode,
+               Code:  s.fakeProvider.ValidCode,
                State: state,
        })
 
@@ -432,8 +581,8 @@ func (s *OIDCLoginSuite) TestGoogleLogin_AlternateEmailAddresses(c *check.C) {
 
 // Primary address is not the one initially returned by oidc.
 func (s *OIDCLoginSuite) TestGoogleLogin_AlternateEmailAddresses_Primary(c *check.C) {
-       s.authEmail = "joe.smith@alternate.example.com"
-       s.fakePeopleAPIResponse = map[string]interface{}{
+       s.fakeProvider.AuthEmail = "joe.smith@alternate.example.com"
+       s.fakeProvider.PeopleAPIResponse = map[string]interface{}{
                "emailAddresses": []map[string]interface{}{
                        {
                                "metadata": map[string]interface{}{"verified": true, "primary": true},
@@ -451,7 +600,7 @@ func (s *OIDCLoginSuite) TestGoogleLogin_AlternateEmailAddresses_Primary(c *chec
        }
        state := s.startLogin(c)
        s.localdb.Login(context.Background(), arvados.LoginOptions{
-               Code:  s.validCode,
+               Code:  s.fakeProvider.ValidCode,
                State: state,
        })
        authinfo := getCallbackAuthInfo(c, s.railsSpy)
@@ -461,9 +610,9 @@ func (s *OIDCLoginSuite) TestGoogleLogin_AlternateEmailAddresses_Primary(c *chec
 }
 
 func (s *OIDCLoginSuite) TestGoogleLogin_NoPrimaryEmailAddress(c *check.C) {
-       s.authEmail = "joe.smith@unverified.example.com"
-       s.authEmailVerified = false
-       s.fakePeopleAPIResponse = map[string]interface{}{
+       s.fakeProvider.AuthEmail = "joe.smith@unverified.example.com"
+       s.fakeProvider.AuthEmailVerified = false
+       s.fakeProvider.PeopleAPIResponse = map[string]interface{}{
                "emailAddresses": []map[string]interface{}{
                        {
                                "metadata": map[string]interface{}{"verified": true},
@@ -477,7 +626,7 @@ func (s *OIDCLoginSuite) TestGoogleLogin_NoPrimaryEmailAddress(c *check.C) {
        }
        state := s.startLogin(c)
        s.localdb.Login(context.Background(), arvados.LoginOptions{
-               Code:  s.validCode,
+               Code:  s.fakeProvider.ValidCode,
                State: state,
        })
 
@@ -487,33 +636,74 @@ func (s *OIDCLoginSuite) TestGoogleLogin_NoPrimaryEmailAddress(c *check.C) {
        c.Check(authinfo.Username, check.Equals, "")
 }
 
-func (s *OIDCLoginSuite) startLogin(c *check.C) (state string) {
+func (s *OIDCLoginSuite) startLogin(c *check.C, checks ...func(url.Values)) (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"})
        c.Check(err, check.IsNil)
+       c.Check(resp.HTML.String(), check.Not(check.Matches), `(?ms).*error:.*`)
        target, err := url.Parse(resp.RedirectLocation)
        c.Check(err, check.IsNil)
        state = target.Query().Get("state")
-       c.Check(state, check.Not(check.Equals), "")
+       if !c.Check(state, check.Not(check.Equals), "") {
+               c.Logf("Redirect target: %q", target)
+               c.Logf("HTML: %q", resp.HTML)
+       }
+       for _, fn := range checks {
+               fn(target.Query())
+       }
+       s.cluster.Login.OpenIDConnect.AuthenticationRequestParameters = map[string]string{"testkey": "testvalue"}
        return
 }
 
-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)
-       }
-       object, err := signer.Sign(payload)
-       if err != nil {
-               c.Error(err)
+func (s *OIDCLoginSuite) TestValidateLoginRedirectTarget(c *check.C) {
+       for _, trial := range []struct {
+               permit       bool
+               trustPrivate bool
+               url          string
+       }{
+               // wb1, wb2 => accept
+               {true, false, s.cluster.Services.Workbench1.ExternalURL.String()},
+               {true, false, s.cluster.Services.Workbench2.ExternalURL.String()},
+               // explicitly listed host => accept
+               {true, false, "https://app.example.com/"},
+               {true, false, "https://app.example.com:443/foo?bar=baz"},
+               // non-listed hostname => deny (regardless of TrustPrivateNetworks)
+               {false, false, "https://bad.example/"},
+               {false, true, "https://bad.example/"},
+               // non-listed non-private IP addr => deny (regardless of TrustPrivateNetworks)
+               {false, true, "https://1.2.3.4/"},
+               {false, true, "https://1.2.3.4/"},
+               {false, true, "https://[ab::cd]:1234/"},
+               // localhost or non-listed private IP addr => accept only if TrustPrivateNetworks is set
+               {false, false, "https://localhost/"},
+               {true, true, "https://localhost/"},
+               {false, false, "https://[10.9.8.7]:80/foo"},
+               {true, true, "https://[10.9.8.7]:80/foo"},
+               {false, false, "https://[::1]:80/foo"},
+               {true, true, "https://[::1]:80/foo"},
+               {true, true, "http://192.168.1.1/"},
+               {true, true, "http://172.17.2.0/"},
+               // bad url => deny
+               {false, true, "https://10.1.1.1:blorp/foo"},        // non-numeric port
+               {false, true, "https://app.example.com:blorp/foo"}, // non-numeric port
+               {false, true, "https://]:443"},
+               {false, true, "https://"},
+               {false, true, "https:"},
+               {false, true, ""},
+               // explicitly listed host but different port, protocol, or user/pass => deny
+               {false, true, "http://app.example.com/"},
+               {false, true, "http://app.example.com:443/"},
+               {false, true, "https://app.example.com:80/"},
+               {false, true, "https://app.example.com:4433/"},
+               {false, true, "https://u:p@app.example.com:443/foo?bar=baz"},
+       } {
+               c.Logf("trial %+v", trial)
+               s.cluster.Login.TrustPrivateNetworks = trial.trustPrivate
+               err := validateLoginRedirectTarget(s.cluster, trial.url)
+               c.Check(err == nil, check.Equals, trial.permit)
        }
-       t, err := object.CompactSerialize()
-       if err != nil {
-               c.Error(err)
-       }
-       c.Logf("fakeToken(%q) == %q", payload, t)
-       return t
+
 }
 
 func getCallbackAuthInfo(c *check.C, railsSpy *arvadostest.Proxy) (authinfo rpc.UserSessionAuthInfo) {