Skip to content

Commit 26ba94a

Browse files
wy65701436wy65701436
committed
resolve comments
Signed-off-by: wang yan <[email protected]>
1 parent 06dbd3c commit 26ba94a

File tree

19 files changed

+21
-95
lines changed

19 files changed

+21
-95
lines changed

‎api/v2.0/swagger.yaml

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9061,9 +9061,6 @@ definitions:
90619061
oidc_logout:
90629062
$ref: '#/definitions/BoolConfigItem'
90639063
description: Extra parameters to logout user session from the OIDC provider
9064-
oidc_logout_offline:
9065-
$ref: '#/definitions/BoolConfigItem'
9066-
description: Extra parameters to logout user offline session from the OIDC provider
90679064
robot_token_duration:
90689065
$ref: '#/definitions/IntegerConfigItem'
90699066
description: The robot account token duration in days
@@ -9343,11 +9340,6 @@ definitions:
93439340
description: Logout OIDC user session
93449341
x-omitempty: true
93459342
x-isnullable: true
9346-
oidc_logout_offline:
9347-
type: boolean
9348-
description: Logout OIDC user offline session
9349-
x-omitempty: true
9350-
x-isnullable: true
93519343
robot_token_duration:
93529344
type: integer
93539345
description: The robot account token duration in days

‎src/common/const.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,6 @@ const (
120120
OIDCScope = "oidc_scope"
121121
OIDCUserClaim = "oidc_user_claim"
122122
OIDCLogout = "oidc_logout"
123-
OIDCLogoutOffline = "oidc_logout_offline"
124123

125124
CfgDriverDB = "db"
126125
NewHarborAdminName = "[email protected]"

‎src/core/controllers/base.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,10 +121,11 @@ func (cc *CommonController) LogOut() {
121121
if redirectForOIDC(cc.Ctx.Request.Context(), principal) {
122122
u, err := user.Ctl.GetByName(cc.Context(), principal)
123123
if err != nil {
124-
log.Errorf("Failed to get user by name: %s, error: %v", principal, err)
124+
log.Errorf("Error fetching user by name: %s, error: %v", principal, err)
125125
cc.CustomAbort(http.StatusInternalServerError, "Internal error.")
126126
}
127127
if u == nil {
128+
log.Errorf("User not found: %s, empty result", principal)
128129
cc.CustomAbort(http.StatusInternalServerError, "Internal error.")
129130
}
130131
if u.UserID != 1 {

‎src/core/controllers/oidc.go

Lines changed: 8 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -288,33 +288,20 @@ func (oc *OIDCController) RedirectLogout() {
288288
oc.SendInternalServerError(err)
289289
return
290290
}
291-
if oidcSettings.LogoutOffline && token.RefreshToken != "" {
291+
if token.RefreshToken != "" {
292292
sessionType, err := getSessionType(token.RefreshToken)
293293
if err == nil {
294-
// If the session is offline, revoke the refresh token
295-
if strings.ToLower(sessionType) == "offline" {
296-
if oidc.EndpointsClaims.RevokeURL != "" {
297-
if err := revokeOIDCRefreshToken(oidc.EndpointsClaims.RevokeURL, token.RefreshToken, oidcSettings.ClientID, oidcSettings.ClientSecret); err != nil {
298-
log.Errorf("Failed to revoke the offline session: %v", err)
299-
oc.SendInternalServerError(err)
300-
return
301-
}
302-
} else {
303-
log.Warning("Unable to logout OIDC offline session since the 'revoke_endpoint' is not set.")
294+
// If the session is offline, try best to revoke the refresh token.
295+
if strings.ToLower(sessionType) == "offline" && oidc.EndpointsClaims.RevokeURL != "" {
296+
if err := revokeOIDCRefreshToken(oidc.EndpointsClaims.RevokeURL, token.RefreshToken, oidcSettings.ClientID, oidcSettings.ClientSecret); err != nil {
297+
log.Warningf("Failed to revoke the offline session: %v", err)
304298
}
305299
}
306-
} else {
307-
log.Warningf("Invalid refresh token for offline session: %s, error: %v", token.RefreshToken, err)
308300
}
309301
}
310-
311302
if token.RawIDToken == "" {
312303
log.Warning("Empty ID token for offline session.")
313-
oc.Controller.Redirect(".", http.StatusFound)
314-
return
315-
}
316-
if _, err := oidc.VerifyToken(ctx, token.RawIDToken); err != nil {
317-
oc.SendInternalServerError(err)
304+
oc.Controller.Redirect("/", http.StatusFound)
318305
return
319306
}
320307
if oidc.EndpointsClaims.EndSessionURL == "" {
@@ -329,7 +316,7 @@ func (oc *OIDCController) RedirectLogout() {
329316
oc.SendInternalServerError(err)
330317
return
331318
}
332-
postLogoutRedirectURI := fmt.Sprintf("%s/harbor/projects", baseURL)
319+
postLogoutRedirectURI := fmt.Sprintf("%s", baseURL)
333320
logoutURL := fmt.Sprintf(
334321
"%s?id_token_hint=%s&post_logout_redirect_uri=%s",
335322
endSessionURL,
@@ -467,7 +454,7 @@ func revokeOIDCRefreshToken(revokeURL, refreshToken, clientID, clientSecret stri
467454
return errors.Errorf("failed to create request: %v", err)
468455
}
469456
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
470-
req.Header.Set("Authorization", "Basic "+auth)
457+
req.SetBasicAuth(clientID, clientSecret)
471458
client := &http.Client{
472459
Transport: &http.Transport{
473460
TLSClientConfig: &tls.Config{InsecureSkipVerify: true},

‎src/lib/config/metadata/metadatalist.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,6 @@ var (
148148
{Name: common.OIDCAutoOnboard, Scope: UserScope, Group: OIDCGroup, DefaultValue: "false", ItemType: &BoolType{}, Description: `Auto onboard the OIDC user`},
149149
{Name: common.OIDCExtraRedirectParms, Scope: UserScope, Group: OIDCGroup, DefaultValue: "{}", ItemType: &StringToStringMapType{}, Description: `Extra parameters to add when redirect request to OIDC provider`},
150150
{Name: common.OIDCLogout, Scope: UserScope, Group: OIDCGroup, DefaultValue: "false", ItemType: &BoolType{}, Description: `Enable OIDC logout to log out user session from the identity provider.`},
151-
{Name: common.OIDCLogoutOffline, Scope: UserScope, Group: OIDCGroup, DefaultValue: "false", ItemType: &BoolType{}, Description: `Enable OIDC Offline logout to log out offline_access session from the identity provider.`},
152151

153152
{Name: common.WithTrivy, Scope: SystemScope, Group: BasicGroup, EnvKey: "WITH_TRIVY", DefaultValue: "false", ItemType: &BoolType{}, Editable: true},
154153
// the unit of expiration is days

‎src/lib/config/models/model.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@ type OIDCSetting struct {
4545
UserClaim string `json:"user_claim"`
4646
ExtraRedirectParms map[string]string `json:"extra_redirect_parms"`
4747
Logout bool `json:"logout"`
48-
LogoutOffline bool `json:"logout_offline"`
4948
}
5049

5150
// QuotaSetting wraps the settings for Quota

‎src/lib/config/test/userconfig_test.go

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -247,18 +247,17 @@ y1bQusZMygQezfCuEzsewF+OpANFovCTUEs6s5vyoVNP8lk=
247247

248248
func TestOIDCSetting(t *testing.T) {
249249
m := map[string]interface{}{
250-
common.OIDCName: "test",
251-
common.OIDCEndpoint: "https://oidc.test",
252-
common.OIDCVerifyCert: "true",
253-
common.OIDCAutoOnboard: "false",
254-
common.OIDCScope: "openid, profile",
255-
common.OIDCGroupsClaim: "my_group",
256-
common.OIDCUserClaim: "username",
257-
common.OIDCCLientID: "client",
258-
common.OIDCClientSecret: "secret",
259-
common.ExtEndpoint: "https://harbor.test",
260-
common.OIDCLogout: "false",
261-
common.OIDCLogoutOffline: "false",
250+
common.OIDCName: "test",
251+
common.OIDCEndpoint: "https://oidc.test",
252+
common.OIDCVerifyCert: "true",
253+
common.OIDCAutoOnboard: "false",
254+
common.OIDCScope: "openid, profile",
255+
common.OIDCGroupsClaim: "my_group",
256+
common.OIDCUserClaim: "username",
257+
common.OIDCCLientID: "client",
258+
common.OIDCClientSecret: "secret",
259+
common.ExtEndpoint: "https://harbor.test",
260+
common.OIDCLogout: "false",
262261
}
263262
InitWithSettings(m)
264263
v, e := OIDCSetting(orm.Context())
@@ -274,7 +273,6 @@ func TestOIDCSetting(t *testing.T) {
274273
assert.ElementsMatch(t, []string{"openid", "profile"}, v.Scope)
275274
assert.Equal(t, "username", v.UserClaim)
276275
assert.False(t, v.Logout)
277-
assert.False(t, v.LogoutOffline)
278276
}
279277

280278
func TestSplitAndTrim(t *testing.T) {

‎src/lib/config/userconfig.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,6 @@ func OIDCSetting(ctx context.Context) (*cfgModels.OIDCSetting, error) {
178178
UserClaim: mgr.Get(ctx, common.OIDCUserClaim).GetString(),
179179
ExtraRedirectParms: mgr.Get(ctx, common.OIDCExtraRedirectParms).GetStringToStringMap(),
180180
Logout: mgr.Get(ctx, common.OIDCLogout).GetBool(),
181-
LogoutOffline: mgr.Get(ctx, common.OIDCLogoutOffline).GetBool(),
182181
}, nil
183182
}
184183

‎src/portal/src/app/base/left-side-nav/config/auth/config-auth.component.html

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -994,34 +994,6 @@
994994
[(ngModel)]="currentConfig.oidc_logout.value" />
995995
</clr-checkbox-wrapper>
996996
</clr-checkbox-container>
997-
<clr-checkbox-container>
998-
<label for="oidcLogout"
999-
>{{ 'CONFIG.OIDC.OIDC_LOGOUT_OFFLINE' | translate }}
1000-
<clr-tooltip>
1001-
<clr-icon
1002-
clrTooltipTrigger
1003-
shape="info-circle"
1004-
size="24"></clr-icon>
1005-
<clr-tooltip-content
1006-
clrPosition="top-right"
1007-
clrSize="lg"
1008-
*clrIfOpen>
1009-
<span>{{
1010-
'TOOLTIP.OIDC_LOGOUT_OFFLINE' | translate
1011-
}}</span>
1012-
</clr-tooltip-content>
1013-
</clr-tooltip>
1014-
</label>
1015-
<clr-checkbox-wrapper>
1016-
<input
1017-
type="checkbox"
1018-
clrCheckbox
1019-
name="oidcLogoutOffline"
1020-
id="oidcLogoutOffline"
1021-
[disabled]="disabled(currentConfig.oidc_logout_offline)"
1022-
[(ngModel)]="currentConfig.oidc_logout_offline.value" />
1023-
</clr-checkbox-wrapper>
1024-
</clr-checkbox-container>
1025997
<clr-input-container>
1026998
<label for="oidcUserClaim"
1027999
>{{ 'CONFIG.OIDC.USER_CLAIM' | translate }}

‎src/portal/src/app/base/left-side-nav/config/config.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,6 @@ export class Configuration {
106106
oidc_scope?: StringValueItem;
107107
oidc_user_claim?: StringValueItem;
108108
oidc_logout?: BoolValueItem;
109-
oidc_logout_offline?: BoolValueItem;
110109
count_per_project: NumberValueItem;
111110
storage_per_project: NumberValueItem;
112111
cfg_expiration: NumberValueItem;
@@ -190,7 +189,6 @@ export class Configuration {
190189
this.oidc_group_filter = new StringValueItem('', true);
191190
this.oidc_user_claim = new StringValueItem('', true);
192191
this.oidc_logout = new BoolValueItem(false, true);
193-
this.oidc_logout_offline = new BoolValueItem(false, true);
194192
this.count_per_project = new NumberValueItem(-1, true);
195193
this.storage_per_project = new NumberValueItem(-1, true);
196194
this.audit_log_forward_endpoint = new StringValueItem('', true);

0 commit comments

Comments
 (0)