Skip to content
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

alternative fix: allow to change or delete a relyingPartySecret on IdP #2887

Closed
wants to merge 4 commits into from

Conversation

strehle
Copy link
Member

@strehle strehle commented May 14, 2024

Alternative to PR: #2882
and PR: #2885

Only one call in backend, but more input on client side.

Extract the delete change into an own PR, because this missing method is really
a bug / issue in UAA. A UAA operator cannot remove a secret currently, but there are
situation where this is needed, e.g. public flows or change to private_key_jwt.
Alternative to PR: #2882
and PR: #2885

Only one call in backend, but more input on client side.
@cf-gitbot
Copy link

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/187601153

The labels on this github issue will be updated when the story is started.

@strehle strehle changed the title Feature/change idp secret alternative fix: allow to change or delete a relyingPartySecret on IdP May 14, 2024
@strehle strehle linked an issue May 14, 2024 that may be closed by this pull request
strehle added a commit that referenced this pull request May 16, 2024
…n IdP

Alternative to PR: #2882, #2887
and PR: #2885

Only one call in backend, but more input on client side.

authMethod is used to make this change non-breakable.
Because before the update prevented the removal of the relyingPartySecret.
No if authMethod is not client_secret_basic or client_secret_post, then the relyingPartySecret can be overwritten with null.
@peterhaochen47
Copy link
Member

Thanks! I prefer the alternative, so let's move our discussion there (#2896).

@strehle strehle requested a review from hsinn0 May 17, 2024 19:13
strehle added a commit that referenced this pull request May 23, 2024
…n IdP (#2896)

* WIP: idp secret

* feature: delete secret on existing IdP

- allow to delete a relyingPartySecret on IdP
- Filter IdP list by origin
- Return the auth_method to show current configured client authentication method

* Documentation

* fix names

* sonar

* sonar

* Add patch call to change a secret from an external IdP

* Alias handling

* 2nd alternative fix: allow to change or delete a relyingPartySecret on IdP

Alternative to PR: #2882, #2887
and PR: #2885

Only one call in backend, but more input on client side.

authMethod is used to make this change non-breakable.
Because before the update prevented the removal of the relyingPartySecret.
No if authMethod is not client_secret_basic or client_secret_post, then the relyingPartySecret can be overwritten with null.

* sonar

* Tests added

* Sonar smell

* Review

* more checks for edge cases
* more tests to cover edge cases

* Review

Again fixed an edge case
CLIENT_SECRET_BASIC, CLIENT_SECRET_POST both same method, therefore treat them equal

* Review

Again fixed an edge case
Found during tests

* Test refactored

* small doc fixes

- some clarification & formatting
- no need to call out what the default is in the description
because the `.optional("client_secret_basic")` syntax would
automatically add that language.

* doc: clarify when external OIDC client auth requirements

- clarify when config.jwtClientAuthentication and config.relyingPartySecret
are required in relation to the new field config.authMethod

* Review, removed auth_method which is not used, but we ue authMethod field only

* revert this

* remove deprecated

---------

Co-authored-by: Peter Chen <peter-h.chen@broadcom.com>
@strehle
Copy link
Member Author

strehle commented May 23, 2024

close because we usee #2896

@strehle strehle closed this May 23, 2024
@strehle strehle deleted the feature/change-idp-secret branch May 23, 2024 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

Cannot remove a secret from IdP via REST
3 participants