oidclogout #21718

wy65701436 · 2025-03-10T11:36:09Z

enable oidc session logout

1, give the option of logging out user session from OIDC provider. 2, give the option of logging out user offline session from OIDC provider.

Thank you for contributing to Harbor!

Comprehensive Summary of your change

Issue being fixed

Fixes #(issue)

Please indicate you've done the following:

Well Written Title and Summary of the PR
Label the PR as needed. "release-note/ignore-for-release, release-note/new-feature, release-note/update, release-note/enhancement, release-note/community, release-note/breaking-change, release-note/docs, release-note/infra, release-note/deprecation"
Accepted the DCO. Commits without the DCO will delay acceptance.
Made sure tests are passing and test coverage is added if needed.
Considered the docs impact and opened a new docs issue or PR with docs changes if needed in website repository.

src/core/controllers/oidc.go

codecov · 2025-03-11T08:59:24Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 46.24%. Comparing base (c8c11b4) to head (7cae7be).
Report is 414 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #21718      +/-   ##
==========================================
+ Coverage   45.36%   46.24%   +0.87%     
==========================================
  Files         244      250       +6     
  Lines       13333    14150     +817     
  Branches     2719     2913     +194     
==========================================
+ Hits         6049     6543     +494     
- Misses       6983     7260     +277     
- Partials      301      347      +46

Flag	Coverage Δ
unittests	`46.24% <ø> (+0.87%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 494 files with indirect coverage changes

🚀 New features to boost your workflow:

❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

reasonerjt · 2025-03-11T09:06:46Z

Could you explain the difference between "oidc session" and "oidc offline session"?

src/core/controllers/base.go

src/core/controllers/oidc.go

Vad1mo · 2025-03-11T09:29:32Z

src/core/controllers/base.go

+	securityCtx, ok := security.FromContext(cc.Context())
+	if !ok {
+		log.Error("Failed to get security context")
+		cc.CustomAbort(http.StatusInternalServerError, "Internal error.")


Can we not have a precise error message, explaining what actually went wrong?

Regarding the 500 error, we used to only provide more details in the log and not expose them further.

does exposing the error, provide a threat, or might help the user to resolve the issue himself?

src/core/controllers/base.go

src/lib/config/metadata/metadatalist.go

src/core/controllers/oidc.go

wy65701436 · 2025-03-11T11:33:32Z

Could you explain the difference between "oidc session" and "oidc offline session"?

According to the OpenID Connect RP-Initiated Logout specification, RP-Initiated Logout should be performed by sending a request to the end_session_endpoint with the ID token. This approach is generally defined in the OpenID specification for all IdPs to properly terminate user sessions.

Based on the Keycloak documentation and what I observed, specifically the section on offline access, an offline session is treated like an active session, created during user authentication with the offline_access scope. To revoke an offline session, a POST request must be made to the revoke endpoint. Since I’ve only tested with Keycloak, I’m uncertain whether other IdPs follow the same flow. As this behavior isn't explicitly defined in the OpenID specification, I’ve included an option (checkbox) for logging out offline sessions.

reasonerjt · 2025-03-12T07:22:43Z

Based on the Keycloak documentation and what I observed, specifically the section on offline access, an offline session is treated like an active session, created during user authentication with the offline_access scope. To revoke an offline session, a POST request must be made to the revoke endpoint. Since I’ve only tested with Keycloak, I’m uncertain whether other IdPs follow the same flow. As this behavior isn't explicitly defined in the OpenID specification, I’ve included an option (checkbox) for logging out offline sessions.

I need to look into the details of the code for other parts, but IMO in this release we don't need to distinguish "OIDC session" and "offline session". I don't think of a use case where the user wants to end his "OIDC session" but keep the "offline session". I think we should just provide the option for the user to end "OIDC session" when he logs out from Harbor, and we always end the "offline session" when the "OIDC session" is terminated. This seems intuitive enough for me, and we won't need to debug the corner case like user log out without ending the "offline session" and login again which the ID provider may or may not terminate the existing offline session, which may impact the use case for "CLI secret"

src/portal/src/app/shared/components/navigator/navigator.component.ts

src/core/controllers/oidc.go

wy65701436 · 2025-03-12T11:37:06Z

Based on the Keycloak documentation and what I observed, specifically the section on offline access, an offline session is treated like an active session, created during user authentication with the offline_access scope. To revoke an offline session, a POST request must be made to the revoke endpoint. Since I’ve only tested with Keycloak, I’m uncertain whether other IdPs follow the same flow. As this behavior isn't explicitly defined in the OpenID specification, I’ve included an option (checkbox) for logging out offline sessions.

I need to look into the details of the code for other parts, but IMO in this release we don't need to distinguish "OIDC session" and "offline session". I don't think of a use case where the user wants to end his "OIDC session" but keep the "offline session". I think we should just provide the option for the user to end "OIDC session" when he logs out from Harbor, and we always end the "offline session" when the "OIDC session" is terminated. This seems intuitive enough for me, and we won't need to debug the corner case like user log out without ending the "offline session" and login again which the ID provider may or may not terminate the existing offline session, which may impact the use case for "CLI secret"

Sure, I will remove the option for logging out the offline session and treat it as a "nice-to-have" flow.

src/core/controllers/base.go

src/core/controllers/oidc.go

enable oidc session logout 1, give the option of logging out user session from OIDC provider. 2, give the option of logging out user offline session from OIDC provider. Signed-off-by: wang yan <[email protected]>

Signed-off-by: wang yan <[email protected]>

src/core/controllers/base.go

src/core/controllers/oidc.go

Signed-off-by: wang yan <[email protected]>

Nicolfo · 2025-05-21T15:47:49Z

I'm following this issue since I want to use Harbor with Keycloak as OIDC provider.
I've a question concerning the use of offline access functionality (to support CLI) in conjunction with OIDC RP-initiated logout.

The first thing I tried was enabling the OIDC Session Logout flag (to complete a correct RP-initiated logout) and requesting an offline_access scope. My expected behaviour is that after a logout from the web portal I can continue to use the CLI secret throught docker cli, but this is valid only for a short amount of time (tokens remaining lifespan).

If I remove the flag from OIDC Session Logout CLI secret continue to work (since I'm not terminate the session/revoking the offline token), but I completely lose the track of my offline sessions (every login create a new session) and the only way to revoke them is manually throught the keycloak console.
I don't think it is accettable to not perform complete logout with OIDC when the user terminate session with Harbor, since offline token exist to solve this specific problem.

Is there a way to setup Harbor to perform somewhere similar to AWS cli, where the two session (docker credential and browser) are generated in two different way (oauth2.0 code flow and cli secret request)?

wy65701436 requested a review from a team as a code owner March 10, 2025 11:36

wy65701436 assigned stonezdj, reasonerjt and xuelichao Mar 10, 2025

github-actions bot assigned chlins and OrlinVasilev Mar 10, 2025

wy65701436 added the release-note/new-feature New Harbor Feature label Mar 10, 2025

github-advanced-security bot found potential problems Mar 10, 2025

View reviewed changes

src/core/controllers/oidc.go Fixed Show fixed Hide fixed

wy65701436 added the target/2.13.0 issues that are targeting v2.13.0 label Mar 11, 2025

reasonerjt reviewed Mar 11, 2025

View reviewed changes

src/core/controllers/base.go Outdated Show resolved Hide resolved

Vad1mo reviewed Mar 11, 2025

View reviewed changes

wy65701436 force-pushed the oidcbase branch 2 times, most recently from bb9a4e4 to 06dbd3c Compare March 12, 2025 05:58

stonezdj reviewed Mar 12, 2025

View reviewed changes

src/portal/src/app/shared/components/navigator/navigator.component.ts Show resolved Hide resolved

stonezdj reviewed Mar 12, 2025

View reviewed changes

src/core/controllers/oidc.go Outdated Show resolved Hide resolved

stonezdj approved these changes Mar 13, 2025

View reviewed changes

wy65701436 mentioned this pull request Mar 13, 2025

OIDC user session is not ended in OIDC provider when OIDC user log out from Harbor log out #12634

Closed

reasonerjt reviewed Mar 14, 2025

View reviewed changes

wy65701436 force-pushed the oidcbase branch from baa078c to 26ba94a Compare March 17, 2025 04:50

wy65701436 added 3 commits March 17, 2025 13:16

oidclogout

55f128d

enable oidc session logout 1, give the option of logging out user session from OIDC provider. 2, give the option of logging out user offline session from OIDC provider. Signed-off-by: wang yan <[email protected]>

resolve comments

5b0d026

Signed-off-by: wang yan <[email protected]>

add i18ns

992af57

Signed-off-by: wang yan <[email protected]>

wy65701436 force-pushed the oidcbase branch 3 times, most recently from 34a5c0d to e88adfe Compare March 17, 2025 06:27

wy65701436 force-pushed the oidcbase branch from e88adfe to 95a9434 Compare March 17, 2025 06:57

reasonerjt reviewed Mar 17, 2025

View reviewed changes

src/core/controllers/base.go Outdated Show resolved Hide resolved

src/core/controllers/base.go Outdated Show resolved Hide resolved

src/core/controllers/oidc.go Outdated Show resolved Hide resolved

reasonerjt reviewed Mar 17, 2025

View reviewed changes

src/core/controllers/oidc.go Outdated Show resolved Hide resolved

reasonerjt reviewed Mar 17, 2025

View reviewed changes

src/core/controllers/oidc.go Show resolved Hide resolved

wy65701436 force-pushed the oidcbase branch 2 times, most recently from f4af7d2 to 84935bd Compare March 17, 2025 11:34

resolve comments

81f833b

Signed-off-by: wang yan <[email protected]>

wy65701436 force-pushed the oidcbase branch from 84935bd to 81f833b Compare March 17, 2025 11:43

reasonerjt approved these changes Mar 17, 2025

View reviewed changes

Merge branch 'main' into oidcbase

7cae7be

wy65701436 merged commit 5960bc8 into goharbor:main Mar 18, 2025
12 checks passed

oidclogout #21718

oidclogout #21718

Uh oh!

Conversation

wy65701436 commented Mar 10, 2025

Comprehensive Summary of your change

Issue being fixed

Uh oh!

Uh oh!

codecov bot commented Mar 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

reasonerjt commented Mar 11, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Vad1mo Mar 11, 2025

Choose a reason for hiding this comment

Uh oh!

wy65701436 Mar 11, 2025

Choose a reason for hiding this comment

Uh oh!

Vad1mo Mar 13, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wy65701436 commented Mar 11, 2025

Uh oh!

reasonerjt commented Mar 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wy65701436 commented Mar 12, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Nicolfo commented May 21, 2025

Uh oh!

Uh oh!

codecov bot commented Mar 11, 2025 •

edited

Loading

reasonerjt commented Mar 12, 2025 •

edited

Loading