[ARVADOS] created: 1.3.0-2203-ge459f4e2d

Git user git at public.arvados.org
Mon Feb 17 18:55:35 UTC 2020


        at  e459f4e2d40762f67ffedafbe988c8da6f4f04d4 (commit)


commit e459f4e2d40762f67ffedafbe988c8da6f4f04d4
Author: Tom Clegg <tom at tomclegg.ca>
Date:   Mon Feb 17 13:55:27 2020 -0500

    16101: Handle logout without sso-provider.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>

diff --git a/lib/controller/federation/conn.go b/lib/controller/federation/conn.go
index 42083cb83..56f117ee7 100644
--- a/lib/controller/federation/conn.go
+++ b/lib/controller/federation/conn.go
@@ -222,6 +222,32 @@ func (conn *Conn) Login(ctx context.Context, options arvados.LoginOptions) (arva
 	}
 }
 
+func (conn *Conn) Logout(ctx context.Context, options arvados.LogoutOptions) (arvados.LogoutResponse, error) {
+	// If the logout request comes with an API token from a known
+	// remote cluster, redirect to that cluster's logout handler
+	// so it has an opportunity to clear sessions, expire tokens,
+	// etc. Otherwise use the local endpoint.
+	reqauth, ok := auth.FromContext(ctx)
+	if !ok || len(reqauth.Tokens) == 0 || len(reqauth.Tokens[0]) < 8 || !strings.HasPrefix(reqauth.Tokens[0], "v2/") {
+		return conn.local.Logout(ctx, options)
+	}
+	id := reqauth.Tokens[0][3:8]
+	if id == conn.cluster.ClusterID {
+		return conn.local.Logout(ctx, options)
+	}
+	remote, ok := conn.remotes[id]
+	if !ok {
+		return conn.local.Logout(ctx, options)
+	}
+	baseURL := remote.BaseURL()
+	target, err := baseURL.Parse(arvados.EndpointLogout.Path)
+	if err != nil {
+		return arvados.LogoutResponse{}, fmt.Errorf("internal error getting redirect target: %s", err)
+	}
+	target.RawQuery = url.Values{"return_to": {options.ReturnTo}}.Encode()
+	return arvados.LogoutResponse{RedirectLocation: target.String()}, nil
+}
+
 func (conn *Conn) CollectionGet(ctx context.Context, options arvados.GetOptions) (arvados.Collection, error) {
 	if len(options.UUID) == 27 {
 		// UUID is really a UUID
diff --git a/lib/controller/federation/login_test.go b/lib/controller/federation/login_test.go
index ab39619c7..3cc5cb842 100644
--- a/lib/controller/federation/login_test.go
+++ b/lib/controller/federation/login_test.go
@@ -10,6 +10,7 @@ import (
 
 	"git.arvados.org/arvados.git/sdk/go/arvados"
 	"git.arvados.org/arvados.git/sdk/go/arvadostest"
+	"git.arvados.org/arvados.git/sdk/go/auth"
 	check "gopkg.in/check.v1"
 )
 
@@ -38,3 +39,32 @@ func (s *LoginSuite) TestDeferToLoginCluster(c *check.C) {
 		c.Check(remotePresent, check.Equals, remote != "")
 	}
 }
+
+func (s *LoginSuite) TestLogout(c *check.C) {
+	s.cluster.Login.GoogleClientID = "zzzzzzzzzzzzzz"
+	s.addHTTPRemote(c, "zhome", &arvadostest.APIStub{})
+	s.cluster.Login.LoginCluster = "zhome"
+
+	returnTo := "https://app.example.com/foo?bar"
+	for _, trial := range []struct {
+		token  string
+		target string
+	}{
+		{token: "", target: returnTo},
+		{token: "zzzzzzzzzzzzzzzzzzzzz", target: returnTo},
+		{token: "v2/zzzzz-aaaaa-aaaaaaaaaaaaaaa/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", target: returnTo},
+		{token: "v2/zhome-aaaaa-aaaaaaaaaaaaaaa/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", target: "http://" + s.cluster.RemoteClusters["zhome"].Host + "/logout?" + url.Values{"return_to": {returnTo}}.Encode()},
+	} {
+		c.Logf("trial %#v", trial)
+		ctx := context.Background()
+		if trial.token != "" {
+			ctx = auth.NewContext(ctx, &auth.Credentials{Tokens: []string{trial.token}})
+		}
+		resp, err := s.fed.Logout(ctx, arvados.LogoutOptions{ReturnTo: returnTo})
+		c.Assert(err, check.IsNil)
+		c.Logf("  RedirectLocation %q", resp.RedirectLocation)
+		target, err := url.Parse(resp.RedirectLocation)
+		c.Check(err, check.IsNil)
+		c.Check(target.String(), check.Equals, trial.target)
+	}
+}
diff --git a/lib/controller/handler.go b/lib/controller/handler.go
index 935a1b6cb..e3869261a 100644
--- a/lib/controller/handler.go
+++ b/lib/controller/handler.go
@@ -86,6 +86,7 @@ func (h *Handler) setup() {
 		mux.Handle("/arvados/v1/users", rtr)
 		mux.Handle("/arvados/v1/users/", rtr)
 		mux.Handle("/login", rtr)
+		mux.Handle("/logout", rtr)
 	}
 
 	hs := http.NotFoundHandler()
diff --git a/lib/controller/handler_test.go b/lib/controller/handler_test.go
index d54f50cf1..6b1fa1a0b 100644
--- a/lib/controller/handler_test.go
+++ b/lib/controller/handler_test.go
@@ -180,6 +180,32 @@ func (s *HandlerSuite) TestProxyRedirect(c *check.C) {
 	c.Check(resp.Header().Get("Location"), check.Matches, `(https://0.0.0.0:1)?/auth/joshid\?return_to=%2Cfoo&?`)
 }
 
+func (s *HandlerSuite) TestLogoutSSO(c *check.C) {
+	s.cluster.Login.ProviderAppID = "test"
+	req := httptest.NewRequest("GET", "https://0.0.0.0:1/logout?return_to=https://example.com/foo", nil)
+	resp := httptest.NewRecorder()
+	s.handler.ServeHTTP(resp, req)
+	if !c.Check(resp.Code, check.Equals, http.StatusFound) {
+		c.Log(resp.Body.String())
+	}
+	c.Check(resp.Header().Get("Location"), check.Equals, "http://localhost:3002/users/sign_out?"+url.Values{"redirect_uri": {"https://example.com/foo"}}.Encode())
+}
+
+func (s *HandlerSuite) TestLogoutGoogle(c *check.C) {
+	if s.cluster.ForceLegacyAPI14 {
+		// Google login N/A
+		return
+	}
+	s.cluster.Login.GoogleClientID = "test"
+	req := httptest.NewRequest("GET", "https://0.0.0.0:1/logout?return_to=https://example.com/foo", nil)
+	resp := httptest.NewRecorder()
+	s.handler.ServeHTTP(resp, req)
+	if !c.Check(resp.Code, check.Equals, http.StatusFound) {
+		c.Log(resp.Body.String())
+	}
+	c.Check(resp.Header().Get("Location"), check.Equals, "https://example.com/foo")
+}
+
 func (s *HandlerSuite) TestValidateV1APIToken(c *check.C) {
 	req := httptest.NewRequest("GET", "/arvados/v1/users/current", nil)
 	user, ok, err := s.handler.(*Handler).validateAPItoken(req, arvadostest.ActiveToken)
diff --git a/lib/controller/localdb/conn.go b/lib/controller/localdb/conn.go
index 4139b270c..ac092382d 100644
--- a/lib/controller/localdb/conn.go
+++ b/lib/controller/localdb/conn.go
@@ -29,6 +29,15 @@ func NewConn(cluster *arvados.Cluster) *Conn {
 	}
 }
 
+func (conn *Conn) Logout(ctx context.Context, opts arvados.LogoutOptions) (arvados.LogoutResponse, error) {
+	if conn.cluster.Login.ProviderAppID != "" {
+		// Proxy to RailsAPI, which hands off to sso-provider.
+		return conn.railsProxy.Logout(ctx, opts)
+	} else {
+		return conn.googleLoginController.Logout(ctx, conn.cluster, conn.railsProxy, opts)
+	}
+}
+
 func (conn *Conn) Login(ctx context.Context, opts arvados.LoginOptions) (arvados.LoginResponse, error) {
 	wantGoogle := conn.cluster.Login.GoogleClientID != ""
 	wantSSO := conn.cluster.Login.ProviderAppID != ""
diff --git a/lib/controller/localdb/login.go b/lib/controller/localdb/login.go
index b1ebb27e4..e96b940ef 100644
--- a/lib/controller/localdb/login.go
+++ b/lib/controller/localdb/login.go
@@ -52,6 +52,10 @@ func (ctrl *googleLoginController) getProvider() (*oidc.Provider, error) {
 	return ctrl.provider, nil
 }
 
+func (ctrl *googleLoginController) Logout(ctx context.Context, cluster *arvados.Cluster, railsproxy *railsProxy, opts arvados.LogoutOptions) (arvados.LogoutResponse, error) {
+	return arvados.LogoutResponse{RedirectLocation: opts.ReturnTo}, nil
+}
+
 func (ctrl *googleLoginController) Login(ctx context.Context, cluster *arvados.Cluster, railsproxy *railsProxy, opts arvados.LoginOptions) (arvados.LoginResponse, error) {
 	provider, err := ctrl.getProvider()
 	if err != nil {
diff --git a/lib/controller/localdb/login_test.go b/lib/controller/localdb/login_test.go
index 9f3267cef..4fb0fbcee 100644
--- a/lib/controller/localdb/login_test.go
+++ b/lib/controller/localdb/login_test.go
@@ -163,6 +163,12 @@ func (s *LoginSuite) TearDownTest(c *check.C) {
 	s.railsSpy.Close()
 }
 
+func (s *LoginSuite) 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) {
 	resp, err := s.localdb.Login(context.Background(), arvados.LoginOptions{})
 	c.Check(err, check.IsNil)
diff --git a/lib/controller/router/router.go b/lib/controller/router/router.go
index c41f103dc..69d707703 100644
--- a/lib/controller/router/router.go
+++ b/lib/controller/router/router.go
@@ -54,6 +54,13 @@ func (rtr *router) addRoutes() {
 				return rtr.fed.Login(ctx, *opts.(*arvados.LoginOptions))
 			},
 		},
+		{
+			arvados.EndpointLogout,
+			func() interface{} { return &arvados.LogoutOptions{} },
+			func(ctx context.Context, opts interface{}) (interface{}, error) {
+				return rtr.fed.Logout(ctx, *opts.(*arvados.LogoutOptions))
+			},
+		},
 		{
 			arvados.EndpointCollectionCreate,
 			func() interface{} { return &arvados.CreateOptions{} },
diff --git a/lib/controller/rpc/conn.go b/lib/controller/rpc/conn.go
index bf6166f44..4b143b770 100644
--- a/lib/controller/rpc/conn.go
+++ b/lib/controller/rpc/conn.go
@@ -145,6 +145,14 @@ func (conn *Conn) Login(ctx context.Context, options arvados.LoginOptions) (arva
 	return resp, err
 }
 
+func (conn *Conn) Logout(ctx context.Context, options arvados.LogoutOptions) (arvados.LogoutResponse, error) {
+	ep := arvados.EndpointLogout
+	var resp arvados.LogoutResponse
+	err := conn.requestAndDecode(ctx, &resp, ep, nil, options)
+	resp.RedirectLocation = conn.relativeToBaseURL(resp.RedirectLocation)
+	return resp, err
+}
+
 // If the given location is a valid URL and its origin is the same as
 // conn.baseURL, return it as a relative URL. Otherwise, return it
 // unmodified.
diff --git a/lib/controller/rpc/conn_test.go b/lib/controller/rpc/conn_test.go
index 83a80e878..b97c0f87b 100644
--- a/lib/controller/rpc/conn_test.go
+++ b/lib/controller/rpc/conn_test.go
@@ -51,6 +51,16 @@ func (s *RPCSuite) TestLogin(c *check.C) {
 	c.Check(resp.RedirectLocation, check.Equals, "/auth/joshid?return_to="+url.QueryEscape(","+opts.ReturnTo))
 }
 
+func (s *RPCSuite) TestLogout(c *check.C) {
+	s.ctx = context.Background()
+	opts := arvados.LogoutOptions{
+		ReturnTo: "https://foo.example.com/bar",
+	}
+	resp, err := s.conn.Logout(s.ctx, opts)
+	c.Check(err, check.IsNil)
+	c.Check(resp.RedirectLocation, check.Equals, "http://localhost:3002/users/sign_out?redirect_uri="+url.QueryEscape(opts.ReturnTo))
+}
+
 func (s *RPCSuite) TestCollectionCreate(c *check.C) {
 	coll, err := s.conn.CollectionCreate(s.ctx, arvados.CreateOptions{Attrs: map[string]interface{}{
 		"owner_uuid":         arvadostest.ActiveUserUUID,
diff --git a/sdk/go/arvados/api.go b/sdk/go/arvados/api.go
index aa670c539..5c8d4f629 100644
--- a/sdk/go/arvados/api.go
+++ b/sdk/go/arvados/api.go
@@ -19,6 +19,7 @@ type APIEndpoint struct {
 var (
 	EndpointConfigGet                     = APIEndpoint{"GET", "arvados/v1/config", ""}
 	EndpointLogin                         = APIEndpoint{"GET", "login", ""}
+	EndpointLogout                        = APIEndpoint{"GET", "logout", ""}
 	EndpointCollectionCreate              = APIEndpoint{"POST", "arvados/v1/collections", "collection"}
 	EndpointCollectionUpdate              = APIEndpoint{"PATCH", "arvados/v1/collections/{uuid}", "collection"}
 	EndpointCollectionGet                 = APIEndpoint{"GET", "arvados/v1/collections/{uuid}", ""}
@@ -140,9 +141,14 @@ type LoginOptions struct {
 	State    string `json:"state,omitempty"`  // OAuth2 callback state
 }
 
+type LogoutOptions struct {
+	ReturnTo string `json:"return_to"` // Redirect to this URL after logging out
+}
+
 type API interface {
 	ConfigGet(ctx context.Context) (json.RawMessage, error)
 	Login(ctx context.Context, options LoginOptions) (LoginResponse, error)
+	Logout(ctx context.Context, options LogoutOptions) (LogoutResponse, error)
 	CollectionCreate(ctx context.Context, options CreateOptions) (Collection, error)
 	CollectionUpdate(ctx context.Context, options UpdateOptions) (Collection, error)
 	CollectionGet(ctx context.Context, options GetOptions) (Collection, error)
diff --git a/sdk/go/arvados/login.go b/sdk/go/arvados/login.go
index 7107ac57a..75ebc81c1 100644
--- a/sdk/go/arvados/login.go
+++ b/sdk/go/arvados/login.go
@@ -24,3 +24,12 @@ func (resp LoginResponse) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 		w.Write(resp.HTML.Bytes())
 	}
 }
+
+type LogoutResponse struct {
+	RedirectLocation string
+}
+
+func (resp LogoutResponse) ServeHTTP(w http.ResponseWriter, req *http.Request) {
+	w.Header().Set("Location", resp.RedirectLocation)
+	w.WriteHeader(http.StatusFound)
+}
diff --git a/sdk/go/arvadostest/api.go b/sdk/go/arvadostest/api.go
index b5cea5c18..9019d33cf 100644
--- a/sdk/go/arvadostest/api.go
+++ b/sdk/go/arvadostest/api.go
@@ -37,6 +37,10 @@ func (as *APIStub) Login(ctx context.Context, options arvados.LoginOptions) (arv
 	as.appendCall(as.Login, ctx, options)
 	return arvados.LoginResponse{}, as.Error
 }
+func (as *APIStub) Logout(ctx context.Context, options arvados.LogoutOptions) (arvados.LogoutResponse, error) {
+	as.appendCall(as.Logout, ctx, options)
+	return arvados.LogoutResponse{}, as.Error
+}
 func (as *APIStub) CollectionCreate(ctx context.Context, options arvados.CreateOptions) (arvados.Collection, error) {
 	as.appendCall(as.CollectionCreate, ctx, options)
 	return arvados.Collection{}, as.Error

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list