diff --git a/api/v2.0/swagger.yaml b/api/v2.0/swagger.yaml index 65abd39e1..69e667000 100644 --- a/api/v2.0/swagger.yaml +++ b/api/v2.0/swagger.yaml @@ -9065,6 +9065,9 @@ definitions: oidc_extra_redirect_parms: $ref: '#/definitions/StringConfigItem' description: Extra parameters to add when redirect request to OIDC provider + oidc_logout: + $ref: '#/definitions/BoolConfigItem' + description: Extra parameters to logout user session from the OIDC provider robot_token_duration: $ref: '#/definitions/IntegerConfigItem' description: The robot account token duration in days @@ -9339,6 +9342,11 @@ definitions: description: Extra parameters to add when redirect request to OIDC provider x-omitempty: true x-isnullable: true + oidc_logout: + type: boolean + description: Logout OIDC user session + x-omitempty: true + x-isnullable: true robot_token_duration: type: integer description: The robot account token duration in days diff --git a/src/common/const.go b/src/common/const.go index b4fffad3f..e85ef5920 100644 --- a/src/common/const.go +++ b/src/common/const.go @@ -119,6 +119,7 @@ const ( OIDCExtraRedirectParms = "oidc_extra_redirect_parms" OIDCScope = "oidc_scope" OIDCUserClaim = "oidc_user_claim" + OIDCLogout = "oidc_logout" CfgDriverDB = "db" NewHarborAdminName = "admin@harbor.local" @@ -151,6 +152,7 @@ const ( OIDCCallbackPath = "/c/oidc/callback" OIDCLoginPath = "/c/oidc/login" + OIDCLoginoutPath = "/c/oidc/logout" AuthProxyRedirectPath = "/c/authproxy/redirect" diff --git a/src/core/controllers/base.go b/src/core/controllers/base.go index 01d7e9464..d5d7c0306 100644 --- a/src/core/controllers/base.go +++ b/src/core/controllers/base.go @@ -108,8 +108,36 @@ func (cc *CommonController) Login() { cc.PopulateUserSession(*user) } -// LogOut Habor UI +// LogOut Harbor UI func (cc *CommonController) LogOut() { + // redirect for OIDC logout. + securityCtx, ok := security.FromContext(cc.Context()) + if !ok { + log.Error("Failed to get security context") + cc.CustomAbort(http.StatusInternalServerError, "Internal error.") + } + principal := securityCtx.GetUsername() + if principal != "" { + if redirectForOIDC(cc.Ctx.Request.Context(), principal) { + ep, err := config.ExtEndpoint() + if err != nil { + log.Errorf("Failed to get the external endpoint, error: %v", err) + cc.CustomAbort(http.StatusUnauthorized, "") + } + url := strings.TrimSuffix(ep, "/") + common.OIDCLoginoutPath + log.Debugf("Redirect user %s to logout page of OIDC provider", principal) + // Return a json to UI with status code 403, as it cannot handle status 302 + cc.Ctx.Output.Status = http.StatusForbidden + err = cc.Ctx.Output.JSON(struct { + Location string `json:"redirect_location"` + }{url}, false, false) + if err != nil { + log.Errorf("Failed to write json to response body, error: %v", err) + } + return + } + } + if err := cc.DestroySession(); err != nil { log.Errorf("Error occurred in LogOut: %v", err) cc.CustomAbort(http.StatusInternalServerError, "Internal error.") diff --git a/src/core/controllers/oidc.go b/src/core/controllers/oidc.go index 3921e5ef2..f6f166405 100644 --- a/src/core/controllers/oidc.go +++ b/src/core/controllers/oidc.go @@ -16,9 +16,11 @@ package controllers import ( "context" + "encoding/base64" "encoding/json" "fmt" "net/http" + "net/url" "strings" "github.com/goharbor/harbor/src/common" @@ -244,6 +246,84 @@ func (oc *OIDCController) Callback() { } } +func (oc *OIDCController) RedirectLogout() { + sessionData := oc.GetSession(tokenKey) + ctx := oc.Ctx.Request.Context() + if err := oc.DestroySession(); err != nil { + log.Errorf("Error occurred in LogOut: %v", err) + oc.SendInternalServerError(err) + return + } + if sessionData == nil { + log.Warningf("OIDC session token not found.") + oc.Controller.Redirect("/", http.StatusFound) + return + } + oidcSettings, err := config.OIDCSetting(ctx) + if err != nil { + log.Errorf("Failed to get OIDC settings: %v", err) + oc.SendInternalServerError(err) + return + } + if oidcSettings == nil { + log.Error("OIDC settings is missing.") + oc.SendInternalServerError(fmt.Errorf("OIDC settings is missing")) + return + } + if !oidcSettings.Logout { + oc.Controller.Redirect("/", http.StatusFound) + return + } + tk, ok := sessionData.([]byte) + if !ok { + log.Error("Invalid OIDC session data format.") + oc.SendInternalServerError(fmt.Errorf("invalid OIDC session data format")) + return + } + token := oidc.Token{} + if err := json.Unmarshal(tk, &token); err != nil { + log.Errorf("Error occurred in Unmarshal: %v", err) + oc.SendInternalServerError(err) + return + } + if token.RefreshToken != "" { + sessionType, err := getSessionType(token.RefreshToken) + if err == nil { + // If the session is offline, try best to revoke the refresh token. + if strings.ToLower(sessionType) == "offline" && oidc.EndpointsClaims.RevokeURL != "" { + if err := oidc.RevokeOIDCRefreshToken(oidc.EndpointsClaims.RevokeURL, token.RefreshToken, oidcSettings.ClientID, oidcSettings.ClientSecret, oidcSettings.VerifyCert); err != nil { + log.Warningf("Failed to revoke the offline session: %v", err) + } + } + } + } + if token.RawIDToken == "" { + log.Warning("Empty ID token for offline session.") + oc.Controller.Redirect("/", http.StatusFound) + return + } + if oidc.EndpointsClaims.EndSessionURL == "" { + log.Warning("Unable to logout OIDC session since the 'end_session_point' is not set.") + oc.Controller.Redirect("/", http.StatusFound) + return + } + endSessionURL := oidc.EndpointsClaims.EndSessionURL + baseURL, err := config.ExtEndpoint() + if err != nil { + log.Errorf("Failed to get external endpoint: %v", err) + oc.SendInternalServerError(err) + return + } + logoutURL := fmt.Sprintf( + "%s?id_token_hint=%s&post_logout_redirect_uri=%s", + endSessionURL, + url.QueryEscape(token.RawIDToken), + url.QueryEscape(baseURL), + ) + log.Info("Redirect user to logout page of OIDC provider:", logoutURL) + oc.Controller.Redirect(logoutURL, http.StatusFound) +} + func userOnboard(ctx context.Context, oc *OIDCController, info *oidc.UserInfo, username string, tokenBytes []byte) (*models.User, bool) { s, t, err := secretAndToken(tokenBytes) if err != nil { @@ -338,3 +418,24 @@ func secretAndToken(tokenBytes []byte) (string, string, error) { } return secret, token, nil } + +// getSessionType determines if the session is offline by decoding the refresh token or not +func getSessionType(refreshToken string) (string, error) { + parts := strings.Split(refreshToken, ".") + if len(parts) != 3 { + return "", errors.Errorf("invalid refresh token") + } + payload, err := base64.RawURLEncoding.DecodeString(parts[1]) + if err != nil { + return "", errors.Errorf("failed to decode refresh token: %v", err) + } + var claims map[string]interface{} + if err := json.Unmarshal(payload, &claims); err != nil { + return "", errors.Errorf("failed to unmarshal refresh token: %v", err) + } + typ, ok := claims["typ"].(string) + if !ok { + return "", errors.New("missing 'typ' claim in refresh token") + } + return typ, nil +} diff --git a/src/core/controllers/oidc_test.go b/src/core/controllers/oidc_test.go new file mode 100644 index 000000000..1e40b9c8e --- /dev/null +++ b/src/core/controllers/oidc_test.go @@ -0,0 +1,47 @@ +package controllers + +import ( + "github.com/stretchr/testify/assert" + "testing" +) + +func TestGetSessionType(t *testing.T) { + tests := []struct { + name string + refreshToken string + expectedType string + expectedError bool + }{ + { + name: "Valid", + refreshToken: "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJ0eXAiOiJvZmZsaW5lIn0.d9fcdba7c10fc1263bf682947afabaecf3496070cd2d5a5e7b3c79dbf1545c1f", + expectedType: "offline", + expectedError: false, + }, + { + name: "Invalid", + refreshToken: "invalidToken", + expectedType: "", + expectedError: true, + }, + { + name: "Missing 'typ' claim", + refreshToken: "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJhbGciOiJIUzI1NiJ9.d9fcdba7c10fc1263bf682947afabaecf3496070cd2d5a5e7b3c79dbf1545c1f", + expectedType: "", + expectedError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + typ, err := getSessionType(tt.refreshToken) + if tt.expectedError { + assert.Error(t, err) + assert.Equal(t, tt.expectedType, typ) + } else { + assert.NoError(t, err) + assert.Equal(t, tt.expectedType, typ) + } + }) + } +} diff --git a/src/lib/config/metadata/metadatalist.go b/src/lib/config/metadata/metadatalist.go index d93f71f77..06e7ee521 100644 --- a/src/lib/config/metadata/metadatalist.go +++ b/src/lib/config/metadata/metadatalist.go @@ -147,6 +147,7 @@ var ( {Name: common.OIDCVerifyCert, Scope: UserScope, Group: OIDCGroup, DefaultValue: "true", ItemType: &BoolType{}, Description: `Verify the OIDC provider's certificate'`}, {Name: common.OIDCAutoOnboard, Scope: UserScope, Group: OIDCGroup, DefaultValue: "false", ItemType: &BoolType{}, Description: `Auto onboard the OIDC user`}, {Name: common.OIDCExtraRedirectParms, Scope: UserScope, Group: OIDCGroup, DefaultValue: "{}", ItemType: &StringToStringMapType{}, Description: `Extra parameters to add when redirect request to OIDC provider`}, + {Name: common.OIDCLogout, Scope: UserScope, Group: OIDCGroup, DefaultValue: "false", ItemType: &BoolType{}, Description: `Enable OIDC logout to log out user session from the identity provider.`}, {Name: common.WithTrivy, Scope: SystemScope, Group: BasicGroup, EnvKey: "WITH_TRIVY", DefaultValue: "false", ItemType: &BoolType{}, Editable: true}, // the unit of expiration is days diff --git a/src/lib/config/models/model.go b/src/lib/config/models/model.go index 25b41388a..e1a7c9c64 100644 --- a/src/lib/config/models/model.go +++ b/src/lib/config/models/model.go @@ -44,6 +44,7 @@ type OIDCSetting struct { Scope []string `json:"scope"` UserClaim string `json:"user_claim"` ExtraRedirectParms map[string]string `json:"extra_redirect_parms"` + Logout bool `json:"logout"` } // QuotaSetting wraps the settings for Quota diff --git a/src/lib/config/test/userconfig_test.go b/src/lib/config/test/userconfig_test.go index 2134ef368..d7922ea6d 100644 --- a/src/lib/config/test/userconfig_test.go +++ b/src/lib/config/test/userconfig_test.go @@ -257,6 +257,7 @@ func TestOIDCSetting(t *testing.T) { common.OIDCCLientID: "client", common.OIDCClientSecret: "secret", common.ExtEndpoint: "https://harbor.test", + common.OIDCLogout: "false", } InitWithSettings(m) v, e := OIDCSetting(orm.Context()) @@ -271,6 +272,7 @@ func TestOIDCSetting(t *testing.T) { assert.Equal(t, "https://harbor.test/c/oidc/callback", v.RedirectURL) assert.ElementsMatch(t, []string{"openid", "profile"}, v.Scope) assert.Equal(t, "username", v.UserClaim) + assert.False(t, v.Logout) } func TestSplitAndTrim(t *testing.T) { diff --git a/src/lib/config/userconfig.go b/src/lib/config/userconfig.go index b9fa3e45d..93806999e 100644 --- a/src/lib/config/userconfig.go +++ b/src/lib/config/userconfig.go @@ -177,6 +177,7 @@ func OIDCSetting(ctx context.Context) (*cfgModels.OIDCSetting, error) { Scope: scope, UserClaim: mgr.Get(ctx, common.OIDCUserClaim).GetString(), ExtraRedirectParms: mgr.Get(ctx, common.OIDCExtraRedirectParms).GetStringToStringMap(), + Logout: mgr.Get(ctx, common.OIDCLogout).GetBool(), }, nil } diff --git a/src/pkg/oidc/helper.go b/src/pkg/oidc/helper.go index 32c80f5e5..3ce329fad 100644 --- a/src/pkg/oidc/helper.go +++ b/src/pkg/oidc/helper.go @@ -15,10 +15,12 @@ package oidc import ( + "bytes" "context" "crypto/tls" "fmt" "net/http" + "net/url" "regexp" "strings" "sync" @@ -32,6 +34,7 @@ import ( "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/lib/config" cfgModels "github.com/goharbor/harbor/src/lib/config/models" + "github.com/goharbor/harbor/src/lib/errors" "github.com/goharbor/harbor/src/lib/log" "github.com/goharbor/harbor/src/lib/orm" "github.com/goharbor/harbor/src/pkg/usergroup" @@ -54,6 +57,11 @@ type providerHelper struct { creationTime time.Time } +var EndpointsClaims struct { + EndSessionURL string `json:"end_session_endpoint"` + RevokeURL string `json:"revocation_endpoint"` +} + func (p *providerHelper) get(ctx context.Context) (*gooidc.Provider, error) { if p.instance.Load() != nil { if time.Since(p.creationTime) > 3*time.Second { @@ -85,6 +93,10 @@ func (p *providerHelper) create(ctx context.Context) error { if err != nil { return fmt.Errorf("failed to create OIDC provider, error: %v", err) } + err = provider.Claims(&EndpointsClaims) + if err != nil { + return err + } p.instance.Store(provider) p.creationTime = time.Now() return nil @@ -485,3 +497,33 @@ func TestEndpoint(conn Conn) error { _, err := gooidc.NewProvider(ctx, conn.URL) return err } + +// RevokeOIDCRefreshToken revokes an offline session using the refresh token +func RevokeOIDCRefreshToken(revokeURL, refreshToken, clientID, clientSecret string, verifyCert bool) error { + data := url.Values{} + data.Set("token", refreshToken) + data.Set("token_type_hint", "refresh_token") + req, err := http.NewRequest("POST", revokeURL, bytes.NewBufferString(data.Encode())) + if err != nil { + return errors.Errorf("failed to create request: %v", err) + } + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.SetBasicAuth(clientID, clientSecret) + var client *http.Client + if !verifyCert { + client = &http.Client{ + Transport: insecureTransport, + } + } else { + client = &http.Client{} + } + resp, err := client.Do(req) + if err != nil { + return errors.Errorf("request failed: %v", err) + } + defer resp.Body.Close() + if resp.StatusCode >= 300 || resp.StatusCode < 200 { + return errors.Errorf("logout failed, status: %d", resp.StatusCode) + } + return nil +} diff --git a/src/portal/src/app/base/left-side-nav/config/auth/config-auth.component.html b/src/portal/src/app/base/left-side-nav/config/auth/config-auth.component.html index b5f408af7..851be8386 100644 --- a/src/portal/src/app/base/left-side-nav/config/auth/config-auth.component.html +++ b/src/portal/src/app/base/left-side-nav/config/auth/config-auth.component.html @@ -966,6 +966,32 @@ [(ngModel)]="currentConfig.oidc_auto_onboard.value" /> + + + + + +