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

PPF-525 Changed payload create client Keycloak #1213

Merged
merged 1 commit into from
Jun 20, 2024

Conversation

grubolsch
Copy link
Contributor

@grubolsch grubolsch commented Jun 19, 2024

We need to sent a slight different payload to Keycloak to create a client, based on feedback from Erwin and Corneel.

Added

  • Add https://docs.publiq.be/ / https://publiq.stoplight.io/ to allow web origins (CORS)
  • Add hidden attribute "origin" to "publiq-platform" -> this will be used later to clear test clients (discussed with Erwin)
  • Set  'use.refresh.tokens' => true (discussed with Erwin)
  • Added extra test to see that the IntegrationToKeycloakClientConverter & IntegrationUrlConverter play well together

Fixed

  • BUGFIX Switch Login and Callback URL, you can only have 1 login url but multiple callback urls

Ticket: https://jira.uitdatabank.be/browse/PPF-525


/* Converts integration urls in the correct Keycloak API format */

final readonly class IntegrationUrlConverter
Copy link
Contributor

Choose a reason for hiding this comment

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

To avoid future use it is maybe best to move these methods as private methods on IntegrationToKeycloakClientConverter. This will prevent developers from using IntegrationUrlConverter directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would prefer not to do this.

  1. The argument that a developer could use the class directly in error can be made for any splitting up of code in different classes. That is the downside of the Single Responsibility Principle, and the downside of a language where you always export everything and cannot hide or seal some classes in a namespace.
  2. When I started doing this I quickly realised that this would make testing several cases in IntegrationUrlConverterTest a lot more difficult. Because you can no longer target the url creation directly the short, clear defined unit tests would become really big.

@grubolsch grubolsch merged commit 9414ea2 into main Jun 20, 2024
8 checks passed
@grubolsch grubolsch deleted the PPF-525/payload-client-keycloak-changes branch June 20, 2024 08:47
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