-
Notifications
You must be signed in to change notification settings - Fork 556
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
Update authentication flow #18527
Update authentication flow #18527
Conversation
...ter-camunda-sdk/src/main/java/io/camunda/zeebe/spring/client/configuration/PropertyUtil.java
Show resolved
Hide resolved
.../main/java/io/camunda/zeebe/spring/client/properties/ZeebeClientConfigurationProperties.java
Show resolved
Hide resolved
.../main/java/io/camunda/zeebe/spring/client/properties/ZeebeClientConfigurationProperties.java
Show resolved
Hide resolved
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.
Hey @ana-vinogradova-camunda great work, I left a couple of questions. Please have a look
...src/main/java/io/camunda/zeebe/spring/client/configuration/ZeebeClientConfigurationImpl.java
Show resolved
Hide resolved
spring-boot-starter-camunda-sdk/src/main/resources/application-camunda-oidc.yaml
Show resolved
Hide resolved
...nda-sdk/src/test/java/io/camunda/zeebe/spring/client/config/CredentialsProviderSaasTest.java
Show resolved
Hide resolved
.../src/test/java/io/camunda/zeebe/spring/client/config/CredentialsProviderSelfManagedTest.java
Show resolved
Hide resolved
8feea7a
to
057ac00
Compare
I'm starting to backport your changes to |
Revert to initial authentication approach: leave only OAuthCredentialsProvider for all authentications, remove Authentication and its implementation.
Removing usages of Authentication classes and it's implementation as we are using OAuthCredentialsProvider now
Moving TokenRequest and TokenResponse to "token" package
Removing "simple" mode as it is not used
Adjusting tests to use OAuthCredentialsProvider instead of IdentityCredentialsProvider
Use CredentialsProvider instead of Authentication. Updated relevant tests.
Renaming the CamundaClientPropertiesOidcTest to CamundaClientPropertiesSelfManagedTest as it reflects the actual usage of this profile
Changing "isPlaintextConnectionEnabled" return type to support null value indicating that there is no configuration for it and avoiding using default value when the value is actually not set "getGrpcAddress" and "getRestAddress": Return null if not set so default value could be set after checking new config. This done because we check the old config first and only then - the new one.
This should be tested in configuration tests.
- add test for default properties - fixing getGrpcAddress default
- Adding correct issuer for SM - Removing client mode for SM as not needed - Removing unused dependency - Adding missing property to yaml files
using grpc address instead of deprecated gateway address
6417605
to
3fd915e
Compare
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.
I'm approving this PR as we paired on this 🚀
Description
Updating authentication flow as per this issue description
Related issues
closes #17886