-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
feat: Disable CSRF check for "/c/oidc/onboard" API for authenticating and Onboarding a User via API from Custom CLI #16969
Conversation
Codecov Report
@@ Coverage Diff @@
## main #16969 +/- ##
===========================================
- Coverage 67.39% 44.64% -22.76%
===========================================
Files 984 235 -749
Lines 106980 13089 -93891
Branches 2670 2670
===========================================
- Hits 72096 5843 -66253
+ Misses 31004 6951 -24053
+ Partials 3880 295 -3585
Flags with carried forward coverage won't be shown. Click here to find out more. |
@reasonerjt requesting review |
It would be great if it can be merged sooner, our solution is depending on this fix. Meanwhile, is there any workaround to onboard OIDC user through API ? Thanks in advance. |
@zyyw @reasonerjt requesting review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the late response but I don't understand why this is needed. To onboard a user you need to follow the SSO flow to authenticate via the OIDC provider and do token exchange. After that, you will be redirected to onboard endpoint.
Therefore, this is DESIGNED for the browser, but in the issue you mentioned you wanna call this via curl which is not supposed to work.
Maybe I am wrong but, as I understand it from @Rajpratik71 and other similar issues, is that they want to manage the user before he actually logs in to Harbor the first time. Examples are add the user to the different projects and grant them the right permissions. Am I right @Rajpratik71 ? |
This is required to do:
via custom-made cli or curl request using API. As mentioned in issue, In browser flow CSRF token got added in request, i.e. why it is happening fine but when calling direct API , csrf check is blocking. i.e. exception was added in "csrfSkipper" function to skip csrf check for "/c/oidc/onboard" API. |
@ywk253100 @wy65701436 @stonezdj @zyyw @daixiang0 @heww requesting review |
I also need the same behavior. For my use case @Vad1mo is correct. If not this then a similar API should exist for backend flow |
why blocked? we need this pr. |
@ashishkumar-07 |
@reasonerjt I think there is some confusion in understanding the scenario. Here, we don't want this for admin user and don't needs to be done by admin user. This step is required for self-service i.e signup to harbor using configured Oauth. This is required for first time signing in of a User in Harbor with configured Oauth. While from UI, First time signing in of a User in Harbor with configured Oauth got success because in request UI passes the CSRF Token after getting the token from configured Auth/Oauth server then using the token fires the "/c/oidc/onboard" API to get it onboarded in Harbor as a User. From next time it is not required as user is already there in Harbor. On the other hand While from CLI/API , First time signing in of a User in Harbor with configured Oauth got failed because of missing CSRF Token after getting the token from configured Auth/Oauth server then using the token fires the "/c/oidc/onboard" API to get it onboarded in Harbor as a User. But as CLI/API request doesn't add CSRF Header in requests i.e. the check is failing |
Further , missed the today community meeting due to time zone confusion, was thinking to discuss this. @reasonerjt and Harbor Team can we schedule a meeting to discuss this over call ? |
@Rajpratik71 |
This PR is being marked stale due to a period of inactivty. If this PR is still relevant, please comment or remove the stale label. Otherwise, this PR will close in 30 days. |
This PR was closed because it has been stalled for 30 days with no activity. If this PR is still relevant, please re-open a new PR against main. |
This PR is being marked stale due to a period of inactivty. If this PR is still relevant, please comment or remove the stale label. Otherwise, this PR will close in 30 days. |
Any updates? We also need this pr. |
… and Onboarding a User via API from Custom CLI Closes goharbor#16966 Fixes goharbor#16966 Signed-off-by: Pratik Raj <rajpratik71@gmail.com>
0573f59
to
8b57d9d
Compare
Is it still possible that it can be merged sooner or later? We want to onboard OIDC users through CLI but not GUI, and this fix can solve our problem. |
Sorry for asking: can it still be merged into harbor's main branch? And will there be a release that includes this change then? Otherwise we need to build harbor again from the source code. |
I build images from this commit with CSRF disabled
{"errors":[{"code":"BAD_REQUEST","message":"Failed to get OIDC user info from session"}]} At the same time, everything onboard works through the GUI. what else is needed? in harbor logs [DEBUG] [/server/middleware/security/basic_auth.go:79][requestID="266741f532898a29f1e2ecf1083a980f"]: a basic auth security context generated for request POST /c/oidc/onboard |
@goharbor/all-maintainers can you check please |
My error message is still |
This PR is being marked stale due to a period of inactivty. If this PR is still relevant, please comment or remove the stale label. Otherwise, this PR will close in 30 days. |
This PR was closed because it has been stalled for 30 days with no activity. If this PR is still relevant, please re-open a new PR against main. |
Issue being fixed
Fixes #16966
Please indicate you've done the following: