[arvados] created: 2.7.0-5358-gbd471a9ead

git repository hosting git at public.arvados.org
Fri Nov 17 16:22:46 UTC 2023


        at  bd471a9eadaf564fb4beafd7db995b7762942c1d (commit)


commit bd471a9eadaf564fb4beafd7db995b7762942c1d
Author: Brett Smith <brett.smith at curii.com>
Date:   Fri Nov 17 11:13:10 2023 -0500

    21137: Support RP-initiated logout with OIDC
    
    With OIDC, logout only has a permanent effect if it is done by the
    OP. If a user attempts to logout with an OIDC token, and the OP provides
    an endpoint to end the session, send the user there to help them
    accomplish that.
    
    Arvados-DCO-1.1-Signed-off-by: Brett Smith <brett.smith at curii.com>

diff --git a/lib/controller/localdb/login_oidc.go b/lib/controller/localdb/login_oidc.go
index a87d13959f..66819fd12a 100644
--- a/lib/controller/localdb/login_oidc.go
+++ b/lib/controller/localdb/login_oidc.go
@@ -68,10 +68,11 @@ type oidcLoginController struct {
 	// 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()
+	provider      *oidc.Provider        // initialized by setup()
+	endSessionURL *url.URL              // initialized by setup()
+	oauth2conf    *oauth2.Config        // initialized by setup()
+	verifier      *oidc.IDTokenVerifier // initialized by setup()
+	mu            sync.Mutex            // protects setup()
 }
 
 // Initialize ctrl.provider and ctrl.oauth2conf.
@@ -101,11 +102,47 @@ func (ctrl *oidcLoginController) setup() error {
 		ClientID: ctrl.ClientID,
 	})
 	ctrl.provider = provider
+	var claims struct {
+		EndSessionEndpoint string `json:"end_session_endpoint"`
+	}
+	err = provider.Claims(&claims)
+	if err != nil {
+		return fmt.Errorf("error parsing OIDC discovery metadata: %v", err)
+	} else if claims.EndSessionEndpoint == "" {
+		ctrl.endSessionURL = nil
+	} else {
+		u, err := url.Parse(claims.EndSessionEndpoint)
+		if err != nil {
+			return fmt.Errorf("OIDC end_session_endpoint is not a valid URL: %v", err)
+		} else if u.Scheme != "https" {
+			return fmt.Errorf("OIDC end_session_endpoint MUST use HTTPS but does not: %v", u.String())
+		} else {
+			ctrl.endSessionURL = u
+		}
+	}
 	return nil
 }
 
 func (ctrl *oidcLoginController) Logout(ctx context.Context, opts arvados.LogoutOptions) (arvados.LogoutResponse, error) {
-	return logout(ctx, ctrl.Cluster, opts)
+	err := ctrl.setup()
+	if err != nil {
+		return arvados.LogoutResponse{}, fmt.Errorf("error setting up OpenID Connect provider: %s", err)
+	}
+	resp, err := logout(ctx, ctrl.Cluster, opts)
+	creds, credsOK := auth.FromContext(ctx)
+	if err == nil && ctrl.endSessionURL != nil && credsOK && len(creds.Tokens) > 0 {
+		values := ctrl.endSessionURL.Query()
+		values.Set("client_id", ctrl.ClientID)
+		values.Set("post_logout_redirect_uri", resp.RedirectLocation)
+		values.Del("id_token_hint")
+		for _, token := range creds.Tokens {
+			values.Add("id_token_hint", token)
+		}
+		u := *ctrl.endSessionURL
+		u.RawQuery = values.Encode()
+		resp.RedirectLocation = u.String()
+	}
+	return resp, err
 }
 
 func (ctrl *oidcLoginController) Login(ctx context.Context, opts arvados.LoginOptions) (arvados.LoginResponse, error) {
diff --git a/lib/controller/localdb/login_oidc_test.go b/lib/controller/localdb/login_oidc_test.go
index 9469fdfd31..1367fb6158 100644
--- a/lib/controller/localdb/login_oidc_test.go
+++ b/lib/controller/localdb/login_oidc_test.go
@@ -97,6 +97,74 @@ func (s *OIDCLoginSuite) TestGoogleLogout(c *check.C) {
 	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("id_token_hint"), check.Equals, accessToken)
+	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..."
+	s.fakeProvider.EndSessionEndpoint = &url.URL{Scheme: "http", Host: "example.com"}
+	_, err := s.localdb.Logout(s.ctx, arvados.LogoutOptions{})
+	c.Check(err, check.NotNil)
+}
+
+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) {
 	resp, err := s.localdb.Login(context.Background(), arvados.LoginOptions{})
 	c.Check(err, check.IsNil)
diff --git a/sdk/go/arvadostest/oidc_provider.go b/sdk/go/arvadostest/oidc_provider.go
index 529c1dca12..31a2667122 100644
--- a/sdk/go/arvadostest/oidc_provider.go
+++ b/sdk/go/arvadostest/oidc_provider.go
@@ -33,6 +33,10 @@ type OIDCProvider struct {
 	AuthGivenName      string
 	AuthFamilyName     string
 	AccessTokenPayload map[string]interface{}
+	// end_session_endpoint metadata URL.
+	// If nil or empty, not included in discovery.
+	// If relative, built from Issuer.URL.
+	EndSessionEndpoint *url.URL
 
 	PeopleAPIResponse map[string]interface{}
 
@@ -71,13 +75,26 @@ func (p *OIDCProvider) serveOIDC(w http.ResponseWriter, req *http.Request) {
 	w.Header().Set("Content-Type", "application/json")
 	switch req.URL.Path {
 	case "/.well-known/openid-configuration":
-		json.NewEncoder(w).Encode(map[string]interface{}{
+		configuration := map[string]interface{}{
 			"issuer":                 p.Issuer.URL,
 			"authorization_endpoint": p.Issuer.URL + "/auth",
 			"token_endpoint":         p.Issuer.URL + "/token",
 			"jwks_uri":               p.Issuer.URL + "/jwks",
 			"userinfo_endpoint":      p.Issuer.URL + "/userinfo",
-		})
+		}
+		if p.EndSessionEndpoint == nil {
+			// Not included in configuration
+		} else if p.EndSessionEndpoint.Scheme != "" {
+			configuration["end_session_endpoint"] = p.EndSessionEndpoint.String()
+		} else {
+			u, err := url.Parse(p.Issuer.URL)
+			p.c.Check(err, check.IsNil,
+				check.Commentf("error parsing IssuerURL for EndSessionEndpoint"))
+			u.Scheme = "https"
+			u.Path = u.Path + p.EndSessionEndpoint.Path
+			configuration["end_session_endpoint"] = u.String()
+		}
+		json.NewEncoder(w).Encode(configuration)
 	case "/token":
 		var clientID, clientSecret string
 		auth, _ := base64.StdEncoding.DecodeString(strings.TrimPrefix(req.Header.Get("Authorization"), "Basic "))

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list