Merge branch '21666-provision-test-improvement'
[arvados.git] / lib / controller / localdb / login_oidc_test.go
index b9f0f56e058482eb74eb527b038136e56979feff..f505f5bc4997682f759176624e851be5a700f8fa 100644 (file)
@@ -15,18 +15,18 @@ import (
        "net/http"
        "net/http/httptest"
        "net/url"
+       "regexp"
        "sort"
        "strings"
        "sync"
        "testing"
        "time"
 
-       "git.arvados.org/arvados.git/lib/config"
        "git.arvados.org/arvados.git/lib/controller/rpc"
+       "git.arvados.org/arvados.git/lib/ctrlctx"
        "git.arvados.org/arvados.git/sdk/go/arvados"
        "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"
 )
@@ -39,20 +39,14 @@ func Test(t *testing.T) {
 var _ = check.Suite(&OIDCLoginSuite{})
 
 type OIDCLoginSuite struct {
-       cluster      *arvados.Cluster
-       localdb      *Conn
-       railsSpy     *arvadostest.Proxy
+       localdbSuite
+       trustedURL   *arvados.URL
        fakeProvider *arvadostest.OIDCProvider
 }
 
-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 *OIDCLoginSuite) SetUpTest(c *check.C) {
+       s.trustedURL = &arvados.URL{Scheme: "https", Host: "app.example.com:443", Path: "/"}
+
        s.fakeProvider = arvadostest.NewOIDCProvider(c)
        s.fakeProvider.AuthEmail = "active-user@arvados.local"
        s.fakeProvider.AuthEmailVerified = true
@@ -62,35 +56,115 @@ func (s *OIDCLoginSuite) SetUpTest(c *check.C) {
        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.localdbSuite.SetUpTest(c)
+
        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.fakeProvider.ValidClientID = "test%client$id"
        s.fakeProvider.ValidClientSecret = "test#client/secret"
 
-       s.localdb = NewConn(s.cluster)
+       s.localdb = NewConn(s.ctx, s.cluster, (&ctrlctx.DBConnector{PostgreSQL: s.cluster.PostgreSQL}).GetDB)
        c.Assert(s.localdb.loginController, check.FitsTypeOf, (*oidcLoginController)(nil))
        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)
 }
 
-func (s *OIDCLoginSuite) TearDownTest(c *check.C) {
-       s.railsSpy.Close()
-}
-
 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) checkRPInitiatedLogout(c *check.C, returnTo string) {
+       if !c.Check(s.fakeProvider.EndSessionEndpoint, check.NotNil,
+               check.Commentf("buggy test: EndSessionEndpoint not configured")) {
+               return
+       }
+       expURL, err := url.Parse(s.fakeProvider.Issuer.URL)
+       if !c.Check(err, check.IsNil, check.Commentf("error parsing expected URL")) {
+               return
+       }
+       expURL.Path = expURL.Path + s.fakeProvider.EndSessionEndpoint.Path
+
+       accessToken := s.fakeProvider.ValidAccessToken()
+       ctx := ctrlctx.NewWithToken(s.ctx, s.cluster, accessToken)
+       resp, err := s.localdb.Logout(ctx, arvados.LogoutOptions{ReturnTo: returnTo})
+       if !c.Check(err, check.IsNil) {
+               return
+       }
+       loc, err := url.Parse(resp.RedirectLocation)
+       if !c.Check(err, check.IsNil, check.Commentf("error parsing response URL")) {
+               return
+       }
+
+       c.Check(loc.Scheme, check.Equals, "https")
+       c.Check(loc.Host, check.Equals, expURL.Host)
+       c.Check(loc.Path, check.Equals, expURL.Path)
+
+       var expReturn string
+       switch returnTo {
+       case "":
+               expReturn = s.cluster.Services.Workbench2.ExternalURL.String()
+       default:
+               expReturn = returnTo
+       }
+       values := loc.Query()
+       c.Check(values.Get("client_id"), check.Equals, s.cluster.Login.Google.ClientID)
+       c.Check(values.Get("post_logout_redirect_uri"), check.Equals, expReturn)
+}
+
+func (s *OIDCLoginSuite) TestRPInitiatedLogoutWithoutReturnTo(c *check.C) {
+       s.fakeProvider.EndSessionEndpoint = &url.URL{Path: "/logout/fromRP"}
+       s.checkRPInitiatedLogout(c, "")
+}
+
+func (s *OIDCLoginSuite) TestRPInitiatedLogoutWithReturnTo(c *check.C) {
+       s.fakeProvider.EndSessionEndpoint = &url.URL{Path: "/rp_logout"}
+       u := arvados.URL{Scheme: "https", Host: "foo.example", Path: "/"}
+       s.cluster.Login.TrustedClients[u] = struct{}{}
+       s.checkRPInitiatedLogout(c, u.String())
+}
+
+func (s *OIDCLoginSuite) TestEndSessionEndpointBadScheme(c *check.C) {
+       // RP-Initiated Logout 1.0 says: "This URL MUST use the https scheme..."
+       u := url.URL{Scheme: "http", Host: "example.com"}
+       s.fakeProvider.EndSessionEndpoint = &u
+       _, err := s.localdb.Logout(s.ctx, arvados.LogoutOptions{})
+       c.Check(err, check.ErrorMatches,
+               `.*\bend_session_endpoint MUST use HTTPS but does not: `+regexp.QuoteMeta(u.String()))
+}
+
+func (s *OIDCLoginSuite) TestNoRPInitiatedLogoutWithoutToken(c *check.C) {
+       endPath := "/TestNoRPInitiatedLogoutWithoutToken"
+       s.fakeProvider.EndSessionEndpoint = &url.URL{Path: endPath}
+       resp, _ := s.localdb.Logout(s.ctx, arvados.LogoutOptions{})
+       u, err := url.Parse(resp.RedirectLocation)
+       c.Check(err, check.IsNil)
+       c.Check(strings.HasSuffix(u.Path, endPath), check.Equals, false,
+               check.Commentf("logout redirected to end_session_endpoint without token"))
 }
 
 func (s *OIDCLoginSuite) TestGoogleLogin_Start_Bogus(c *check.C) {
@@ -118,6 +192,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{
@@ -169,7 +250,7 @@ func (s *OIDCLoginSuite) TestConfig(c *check.C) {
        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)
+       localdb := NewConn(context.Background(), s.cluster, (&ctrlctx.DBConnector{PostgreSQL: s.cluster.PostgreSQL}).GetDB)
        ctrl := localdb.loginController.(*oidcLoginController)
        c.Check(ctrl.Issuer, check.Equals, "https://accounts.example.com/")
        c.Check(ctrl.ClientID, check.Equals, "oidc-client-id")
@@ -184,7 +265,7 @@ func (s *OIDCLoginSuite) TestConfig(c *check.C) {
                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)
+               localdb = NewConn(context.Background(), s.cluster, (&ctrlctx.DBConnector{PostgreSQL: s.cluster.PostgreSQL}).GetDB)
                ctrl = localdb.loginController.(*oidcLoginController)
                c.Check(ctrl.Issuer, check.Equals, "https://accounts.google.com")
                c.Check(ctrl.ClientID, check.Equals, "google-client-id")
@@ -228,16 +309,73 @@ func (s *OIDCLoginSuite) TestOIDCAuthorizer(c *check.C) {
        io.WriteString(mac, accessToken)
        apiToken := fmt.Sprintf("%x", mac.Sum(nil))
 
+       checkTokenInDB := func() time.Time {
+               var exp time.Time
+               err := db.QueryRow(`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)
+               return exp
+       }
        cleanup := func() {
+               oidcAuthorizer.cache.Purge()
                _, 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
+       ctx := ctrlctx.NewWithToken(s.ctx, s.cluster, accessToken)
+
+       // Check behavior on 5xx/network errors (don't cache) vs 4xx
+       // (do cache)
+       {
+               call := oidcAuthorizer.WrapCalls(func(ctx context.Context, opts interface{}) (interface{}, error) {
+                       return nil, nil
+               })
+
+               // If fakeProvider UserInfo endpoint returns 502, we
+               // should fail, return an error, and *not* cache the
+               // negative result.
+               tokenCacheNegativeTTL = time.Minute
+               s.fakeProvider.UserInfoErrorStatus = 502
+               _, err := call(ctx, nil)
+               c.Check(err, check.NotNil)
+
+               // The negative result was not cached, so retrying
+               // immediately (with UserInfo working now) should
+               // succeed.
+               s.fakeProvider.UserInfoErrorStatus = 0
+               _, err = call(ctx, nil)
+               c.Check(err, check.IsNil)
+               checkTokenInDB()
+
+               cleanup()
+
+               // UserInfo 401 => cache the negative result, but
+               // don't return an error (just pass the token through
+               // as a v1 token)
+               s.fakeProvider.UserInfoErrorStatus = 401
+               _, err = call(ctx, nil)
+               c.Check(err, check.IsNil)
+               ent, ok := oidcAuthorizer.cache.Get(accessToken)
+               c.Check(ok, check.Equals, true)
+               c.Check(ent, check.FitsTypeOf, time.Time{})
+
+               // UserInfo succeeds now, but we still have a cached
+               // negative result.
+               s.fakeProvider.UserInfoErrorStatus = 0
+               _, err = call(ctx, nil)
+               c.Check(err, check.IsNil)
+               ent, ok = oidcAuthorizer.cache.Get(accessToken)
+               c.Check(ok, check.Equals, true)
+               c.Check(ent, check.FitsTypeOf, time.Time{})
+
+               tokenCacheNegativeTTL = time.Millisecond
+               cleanup()
+       }
 
+       var exp1 time.Time
        concurrent := 4
        s.fakeProvider.HoldUserInfo = make(chan *http.Request)
        s.fakeProvider.ReleaseUserInfo = make(chan struct{})
@@ -257,17 +395,12 @@ func (s *OIDCLoginSuite) TestOIDCAuthorizer(c *check.C) {
                        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)
+                               exp := checkTokenInDB()
                                if i == 0 {
                                        exp1 = exp
                                }
@@ -286,9 +419,7 @@ func (s *OIDCLoginSuite) TestOIDCAuthorizer(c *check.C) {
        // 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)
+               exp := checkTokenInDB()
                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
@@ -296,7 +427,7 @@ func (s *OIDCLoginSuite) TestOIDCAuthorizer(c *check.C) {
 
        s.fakeProvider.AccessTokenPayload = map[string]interface{}{"scope": "openid profile foobar"}
        accessToken = s.fakeProvider.ValidAccessToken()
-       ctx = auth.NewContext(context.Background(), &auth.Credentials{Tokens: []string{accessToken}})
+       ctx = ctrlctx.NewWithToken(s.ctx, s.cluster, accessToken)
 
        mac = hmac.New(sha256.New, []byte(s.cluster.SystemRootToken))
        io.WriteString(mac, accessToken)
@@ -399,7 +530,7 @@ func (s *OIDCLoginSuite) TestGenericOIDCLogin(c *check.C) {
                        s.railsSpy.Close()
                }
                s.railsSpy = arvadostest.NewProxy(c, s.cluster.Services.RailsAPI)
-               s.localdb = NewConn(s.cluster)
+               s.localdb = NewConn(context.Background(), s.cluster, (&ctrlctx.DBConnector{PostgreSQL: s.cluster.PostgreSQL}).GetDB)
                *s.localdb.railsProxy = *rpc.NewConn(s.cluster.ClusterID, s.railsSpy.URL, true, rpc.PassthroughTokenProvider)
 
                state := s.startLogin(c, func(form url.Values) {
@@ -462,7 +593,7 @@ func (s *OIDCLoginSuite) TestGoogleLogin_Success(c *check.C) {
 
        // Try using the returned Arvados token.
        c.Logf("trying an API call with new token %q", token)
-       ctx := auth.NewContext(context.Background(), &auth.Credentials{Tokens: []string{token}})
+       ctx := ctrlctx.NewWithToken(s.ctx, s.cluster, token)
        cl, err := s.localdb.CollectionList(ctx, arvados.ListOptions{Limit: -1})
        c.Check(cl.ItemsAvailable, check.Not(check.Equals), 0)
        c.Check(cl.Items, check.Not(check.HasLen), 0)
@@ -471,7 +602,7 @@ func (s *OIDCLoginSuite) TestGoogleLogin_Success(c *check.C) {
        // Might as well check that bogus tokens aren't accepted.
        badtoken := token + "plussomeboguschars"
        c.Logf("trying an API call with mangled token %q", badtoken)
-       ctx = auth.NewContext(context.Background(), &auth.Credentials{Tokens: []string{badtoken}})
+       ctx = ctrlctx.NewWithToken(s.ctx, s.cluster, badtoken)
        cl, err = s.localdb.CollectionList(ctx, arvados.ListOptions{Limit: -1})
        c.Check(cl.Items, check.HasLen, 0)
        c.Check(err, check.NotNil)
@@ -613,10 +744,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())
        }
@@ -624,6 +759,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)