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

feat(DHIS2-18994): support OAuth2 client credentials flow for Route API #19953

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

cjmamo
Copy link
Contributor

@cjmamo cjmamo commented Feb 17, 2025

  • Support OAuth2 client credentials flow for Route API
  • Solve nasty security regression which dirties the route entity during decryption causing the clear text secret to be inadvertently persisted. Will attempt to implement a safeguard in a separate PR so that this doesn't happen again.

@cjmamo cjmamo requested review from a team February 17, 2025 13:40
@cjmamo cjmamo self-assigned this Feb 17, 2025
@cjmamo cjmamo marked this pull request as draft February 18, 2025 10:15
fix(security): solve regression which dirties the route entity during decryption causing the clear text secret to be inadvertently persisted

refactor: replace copy methods in AuthScheme implementations with Lombok builders
@cjmamo cjmamo marked this pull request as ready for review February 20, 2025 19:43
@cjmamo cjmamo requested a review from netroms February 20, 2025 19:47
Copy link
Contributor

@jbee jbee left a comment

Choose a reason for hiding this comment

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

I can't really judge the details of the security related code but the rest looks fine. The one comment that is important to me is about the throws Exception in the controller. If possible we want to list concrete exceptions as that is how OpenAPI knows what error responses are possible.

doc: add note about exception handling in WebhookHandler

test: replace nulls with stubs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants