21137: Support RP-initiated logout with OIDC
authorBrett Smith <brett.smith@curii.com>
Fri, 17 Nov 2023 16:13:10 +0000 (11:13 -0500)
committerBrett Smith <brett.smith@curii.com>
Fri, 17 Nov 2023 16:19:16 +0000 (11:19 -0500)
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@curii.com>

lib/controller/localdb/login_oidc.go
lib/controller/localdb/login_oidc_test.go
sdk/go/arvadostest/oidc_provider.go

index a87d13959f802584d8d8b438745da13079ea7ac6..66819fd12ad9fe78a53961b7aa0beaa5667b8c8b 100644 (file)
@@ -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) {
index 9469fdfd31e31e13802fedd09d4d51a26c13f131..1367fb61581018a564fc347a56c57d95bb857f70 100644 (file)
@@ -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)
index 529c1dca12b15a9550dbffcbb9c37e145fa39cb7..31a26671226a0a8ba3452e9e61a41a263bbd3429 100644 (file)
@@ -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 "))