Skip to content

Add PKCE support for OIDC authentication #21702

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 10, 2025

Conversation

reasonerjt
Copy link
Contributor

Fixes #19393

By default Harbor will generate a pkce code and use it in the authentication flow to interact with OIDC provider. Per OAuth spec, this should not break the flow for the OIDC provider that does not support PKCE The code_challenge_method is hard coded to SHA256 for security reason, and we may consider add more settings in future based on feedbacks.

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.

@reasonerjt reasonerjt added the release-note/enhancement Label to mark PR to be added under release notes as enhancement label Mar 6, 2025
@reasonerjt reasonerjt requested a review from a team as a code owner March 6, 2025 08:33
Copy link

codecov bot commented Mar 6, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 46.30%. Comparing base (c8c11b4) to head (10073ea).
Report is 400 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #21702      +/-   ##
==========================================
+ Coverage   45.36%   46.30%   +0.93%     
==========================================
  Files         244      250       +6     
  Lines       13333    14133     +800     
  Branches     2719     2908     +189     
==========================================
+ Hits         6049     6544     +495     
- Misses       6983     7244     +261     
- Partials      301      345      +44     
Flag Coverage Δ
unittests 46.30% <ø> (+0.93%) ⬆️

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 reasonerjt force-pushed the oidc-pkce-support branch 4 times, most recently from ad5064c to 24d747a Compare March 6, 2025 11:39
Fixes goharbor#19393

By default Harbor will generate a pkce code and use it in the
authentication flow to interact with OIDC provider.
Per OAuth spec, this should not break the flow for the OIDC provider that does not support PKCE
The code_challenge_method is hard coded to SHA256 for security reason,
and we may consider add more settings in future based on feedbacks.

Signed-off-by: Daniel Jiang <[email protected]>
Copy link
Contributor

@wy65701436 wy65701436 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@wy65701436 wy65701436 added the target/2.13.0 issues that are targeting v2.13.0 label Mar 7, 2025
@reasonerjt reasonerjt enabled auto-merge (squash) March 7, 2025 06:17
@reasonerjt reasonerjt merged commit e40db21 into goharbor:main Mar 10, 2025
13 checks passed
Liam-Zhao added a commit to Liam-Zhao/harbor that referenced this pull request Mar 19, 2025
author zhaoxinxin <[email protected]> 1741079005 +0800
committer zhaoxinxin <[email protected]> 1742381063 +0800
gpgsig -----BEGIN PGP SIGNATURE-----

 iQGzBAABCAAdFiEEjX7vigeNSJLimSPEHOcWYk0ZMdEFAmfaoAcACgkQHOcWYk0Z
 MdGa2QwArxb9MxLqdHXmb0tN18grIa6hbsKhTJ733mcUX/buOQAVa5Y4YBOoazk5
 xSGJKrnA4OLkWJq6zcMezMnzQ40eGW8cxxnCALYUqsZrrKeQa3OUAwX/WoG7yqV+
 QUYkoSfxTiSxRaSD0c0f3lSAWXvU3LonOJVilOJ0zxZHFnqRQLndYci6yHcmMEfm
 plsgRmFfsNghwYzl83TUvN4wHYeBiGW6NqQrRorGtjyJrCFAh7w7haOsHJnn2VNd
 6URNEvLuvKezZUwxQVADQS+csDUgJpMUrBK9atPvnL1Ds87WXlM41ZhCJSJSLTxi
 VGVhl4Cl5DrLLj2UbuSjCJnh+dmFdCml66hY1fbOt3GTjmq5CxVGXHWTJb4017K+
 NQWNWjaqw5RPynzBwHRNySEads+21KMaMrY2HbRdEbDHJ7yj9iQaC9q0S2G9jOah
 a/35TxR8y8jR8ne0E2gwhJTqESxpV0uIw15cg9c8gxjVZqMKomx9LJ+DVJZ2mS5M
 9fGsrVv+
 =E0hX
 -----END PGP SIGNATURE-----

parent fef9524
author zhaoxinxin <[email protected]> 1741079005 +0800
committer zhaoxinxin <[email protected]> 1742380969 +0800
gpgsig -----BEGIN PGP SIGNATURE-----

 iQGzBAABCAAdFiEEjX7vigeNSJLimSPEHOcWYk0ZMdEFAmfan6kACgkQHOcWYk0Z
 MdEtrAv/UzvPaH7F5MHDDYy8BYps5KbSDB4mwxmRJLRhcUr/scb072ifI2/5Jm6w
 1zVNWkDVR8OIp+0AfX55XeiUhBNp/08ZCLqwr1bGtIbhJOZUY3o/dXaS/YclfbXo
 zHg37hJpuFyaN1HUv6xrLcnDJMehuU8MINGIBqFu2KdX5TaArM+6OB4pEoHWmn1Z
 761Gb8OT+oP3ndNKwi9gyDTcFAnmcnDFW+U7LtX79vzoRD1yZ5Bz/6NZDGiQvYqE
 w4GHBRNdUOE6L35g3wUS/tfCcrEVcWvREB1YgfS9BEbqIhcNugndxZRdMrkWlXyy
 3JKc4fkPgEVZtgZm8XVX0Oc5LMrI953jzO31ubr7yxcjKzU2UW0xuiDqQ5KHRoQ+
 Zefqk6XgUZB/ZC/aYrq2xxKLobyz+Uklfj771IcxKUA7fWGXl7YHWzAl6kbBJasH
 0okOOonHppbp2LMbRTSfODFiGEfEJ+EIz/ALrOLx9LeuchFs6scsKHberPwFde9c
 pd3tOr0I
 =d3WH
 -----END PGP SIGNATURE-----

fix: Remove top error message about no README or license

Signed-off-by: zhaoxinxin <[email protected]>

Add PKCE support for OIDC authentication (goharbor#21702)

Fixes goharbor#19393

By default Harbor will generate a pkce code and use it in the
authentication flow to interact with OIDC provider.
Per OAuth spec, this should not break the flow for the OIDC provider that does not support PKCE
The code_challenge_method is hard coded to SHA256 for security reason,
and we may consider add more settings in future based on feedbacks.

Signed-off-by: Daniel Jiang <[email protected]>

update tlsOptions for external redis (goharbor#21681)

Signed-off-by: yminer <[email protected]>
Co-authored-by: yminer <[email protected]>

add prepare migration script for 2.13.0 (goharbor#21680)

Signed-off-by: yminer <[email protected]>

Update UI version in package.json to v2.13.0 (goharbor#21606)

update version to 2.13.0

Signed-off-by: bupd <[email protected]>
Co-authored-by: miner <[email protected]>

Change audit log label (goharbor#21703)

Add more description for update user operation change password or set sys admin

Signed-off-by: stonezdj <[email protected]>

feat: implement the CNAI model processor (goharbor#21663)

feat: implement the AI model processor

Signed-off-by: chlins <[email protected]>

fix issue 20828 (goharbor#21726)

* fix issue 20828

fix goharbor#20828

Does not fire event only when the current project is a proxy-cache project and the artifact already exists.

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

consume the downstream distribution (goharbor#21733)

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

Add Missing copyright headers in src/portal (goharbor#21693)

add missing copyright headers to files in UI

Signed-off-by: bupd <[email protected]>
Co-authored-by: Wang Yan <[email protected]>

Add Missing copyright headers in src/portal part 2 (goharbor#21694)

add missing headers part 2

Signed-off-by: bupd <[email protected]>
Co-authored-by: Wang Yan <[email protected]>

Add Missing Headers in UI part 3 (goharbor#21695)

add missing headers in UI part 3

Signed-off-by: bupd <[email protected]>

Replace Vmware to goharbor (goharbor#21696)

replace vmware with goharbor in src/portal

Signed-off-by: bupd <[email protected]>
Co-authored-by: Wang Yan <[email protected]>

Add Lint Check for Copyright Headers in UI (goharbor#21692)

add lint check for headers in UI

Signed-off-by: bupd <[email protected]>

Fix: Copy Pull Button Overlap with Tag Immutable Label (goharbor#21720)

fix: copy button overlap with tag immutable

- fix copy button overlap with tag immutable label on artifact-tag
component
- update css to fix this issue

Signed-off-by: bupd <[email protected]>
Co-authored-by: Wang Yan <[email protected]>

fix i18n issue (goharbor#21748)

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

fix: fix replication of multiple projects with numeric names (goharbor#21474)

Explicitly mark project names as strings

This keeps the server from parsing all-numeric project names as integer
values which it does not like.

Signed-off-by: Chris Girard <[email protected]>
Co-authored-by: Wang Yan <[email protected]>

update golang to v1.23.7 (goharbor#21749)

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

Bump up trivy and trivy-adapter to the latest RC tag (goharbor#21741)

Signed-off-by: Daniel Jiang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/enhancement Label to mark PR to be added under release notes as enhancement target/2.13.0 issues that are targeting v2.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OIDC login: Add support for PKCE (Proof Key for Code Exchange).
5 participants