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

Fix client creation by adding serviceAccountsEnabled: true to allow access token fetching #12

Merged
merged 4 commits into from
Oct 2, 2024

Conversation

inFocus7
Copy link
Contributor

@inFocus7 inFocus7 commented Oct 2, 2024

A fast follow to this issue: https://github.com/solo-io/solo-projects/issues/6993. We would likely want to discuss making different flows optional, but defaulting to setting this to true to allow a flow to be automatically set.

Because we previously weren't setting the serviceAccountsEnabled: true, we didn't have a method to use the client id + secret to retrieve an access token for a client (app). With this addition now we can use the client_credentials flow to get an access token.

I verified by running my oauth branch in solo-projects, building + injecting a build of the idp with this branch, and going through the reproduction, where now when an oauth credential is generated (a client for an app) the service account (client_credential flow) is enabled.


Updated the circle CI image because the old one was deprecated and caused failures. Updated to ubuntu-2204:2024.01.1 to match GME's image, which we previously were as well.


Note: no tests, because unit tests verify the output (which hasn't changed) and e2e tests are for cognito. personally it would be overkill to create a whole new suite of keycloak tests for this addition + it would impact the completion of my client credential work in solo-projects.

@inFocus7
Copy link
Contributor Author

inFocus7 commented Oct 2, 2024

This job was rejected because the image 'ubuntu-2004:2022.07.1' is [unavailable](https://discuss.circleci.com/t/linux-image-deprecations-and-eol-for-2024/)

#9

Copy link

@djannot djannot left a comment

Choose a reason for hiding this comment

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

LGTM

@inFocus7 inFocus7 merged commit dc856d7 into main Oct 2, 2024
4 checks passed
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.

3 participants