Merge branch 'main' into 18842-arv-mount-disk-config
[arvados.git] / lib / controller / localdb / login_oidc_test.go
index 4be7d58f699c455ee67e31a206bcebfc889ed0ad..0fe3bdf7f6b684652cad9c71f3c0a63fba15b925 100644 (file)
@@ -17,6 +17,7 @@ import (
        "net/url"
        "sort"
        "strings"
+       "sync"
        "testing"
        "time"
 
@@ -41,6 +42,7 @@ type OIDCLoginSuite struct {
        cluster      *arvados.Cluster
        localdb      *Conn
        railsSpy     *arvadostest.Proxy
+       trustedURL   *arvados.URL
        fakeProvider *arvadostest.OIDCProvider
 }
 
@@ -52,10 +54,14 @@ func (s *OIDCLoginSuite) TearDownSuite(c *check.C) {
 }
 
 func (s *OIDCLoginSuite) SetUpTest(c *check.C) {
+       s.trustedURL = &arvados.URL{Scheme: "https", Host: "app.example.com", Path: "/"}
+
        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{}{}
 
@@ -67,6 +73,7 @@ func (s *OIDCLoginSuite) SetUpTest(c *check.C) {
        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.fakeProvider.ValidClientID = "test%client$id"
        s.fakeProvider.ValidClientSecret = "test#client/secret"
@@ -85,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.com/bar")
+       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://192.168.1.1/bar")
 }
 
 func (s *OIDCLoginSuite) TestGoogleLogin_Start_Bogus(c *check.C) {
@@ -115,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{
@@ -234,18 +265,49 @@ func (s *OIDCLoginSuite) TestOIDCAuthorizer(c *check.C) {
 
        ctx := auth.NewContext(context.Background(), &auth.Credentials{Tokens: []string{accessToken}})
        var exp1 time.Time
-       oidcAuthorizer.WrapCalls(func(ctx context.Context, opts interface{}) (interface{}, error) {
-               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(&exp1)
-               c.Check(err, check.IsNil)
-               c.Check(exp1.Sub(time.Now()) > -time.Second, check.Equals, true)
-               c.Check(exp1.Sub(time.Now()) < time.Second, check.Equals, true)
-               return nil, nil
-       })(ctx, nil)
+       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
@@ -255,8 +317,8 @@ func (s *OIDCLoginSuite) TestOIDCAuthorizer(c *check.C) {
                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)
-               c.Check(exp.Sub(exp1) < time.Second, check.Equals, true)
+               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)
 
@@ -421,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)
 
@@ -446,6 +508,7 @@ func (s *OIDCLoginSuite) TestGoogleLogin_Success(c *check.C) {
 
 func (s *OIDCLoginSuite) TestGoogleLogin_RealName(c *check.C) {
        s.fakeProvider.AuthEmail = "joe.smith@primary.example.com"
+       s.fakeProvider.AuthEmailVerified = true
        s.fakeProvider.PeopleAPIResponse = map[string]interface{}{
                "names": []map[string]interface{}{
                        {
@@ -471,8 +534,10 @@ func (s *OIDCLoginSuite) TestGoogleLogin_RealName(c *check.C) {
        c.Check(authinfo.LastName, check.Equals, "Psmith")
 }
 
-func (s *OIDCLoginSuite) TestGoogleLogin_OIDCRealName(c *check.C) {
+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{
@@ -576,10 +641,14 @@ func (s *OIDCLoginSuite) startLogin(c *check.C, checks ...func(url.Values)) (sta
        // 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())
        }
@@ -587,6 +656,56 @@ func (s *OIDCLoginSuite) startLogin(c *check.C, checks ...func(url.Values)) (sta
        return
 }
 
+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)
+       }
+
+}
+
 func getCallbackAuthInfo(c *check.C, railsSpy *arvadostest.Proxy) (authinfo rpc.UserSessionAuthInfo) {
        for _, dump := range railsSpy.RequestDumps {
                c.Logf("spied request: %q", dump)