feat: add builder

[fschmtt]

No description provided.

[codecov]

Codecov Report

❌ Patch coverage is 83.78378% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.18%. Comparing base (c76c1c5) to head (4a7d0fb).
⚠️ Report is 8 commits behind head on main.

Files with missing lines	Patch %	Lines
src/Builder.php	40.00%	12 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #166      +/-   ##
==========================================
- Coverage   93.01%   92.18%   -0.83%     
==========================================
  Files          27       32       +5     
  Lines         916      973      +57     
==========================================
+ Hits          852      897      +45     
- Misses         64       76      +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[phillipfickl]

Great start!
Note about the Client Credential grant: apparently it's not supposed to work with refresh tokens. By default Keycloak has it turned off, one would need to manually enable it in the client, otherwise it'll throw an exception because we expect it to be set in Client.php:121

Other than that: It works well! I just tested the Password and Client Grant.

Thank you Frederik!

[fschmtt]

Hey @phillipfickl, thanks a lot for this feedback! I wasn't aware of this scenario.

I'm now checking if the refresh_token is part of the token response, and if not, I'm simply not storing it.

This will make the Client.php request a new access token instead of trying to refresh.

From my POV this should fix it, maybe you find the time to try it? 😄

[phillipfickl]

Hey @fschmtt, I just tested and it works fine! I wasn't aware either, I found out by debugging yesterday 😅

[fschmtt]

Okay, before I dive into writing the missing tests and make the PR "mergable" - WDYT about having a flag bool $useRefreshToken on the GrantType generally?

For all grant types I would set this to true by default (IDK if it even need to be configurable, but the UI screenshot you provided makes it seem like you could disable refresh tokens in general 🤔).
The ClientCredentials would default to false but could be configured to be true.

I think in the end it does not change from the consumer's POV, but the current approach is rather implicit while having a config flag is more explicit.

I'll think about it for a bit 😄

[phillipfickl]

Okay, before I dive into writing the missing tests and make the PR "mergable" - WDYT about having a flag bool $useRefreshToken on the GrantType generally?

For all grant types I would set this to true by default (IDK if it even need to be configurable, but the UI screenshot you provided makes it seem like you could disable refresh tokens in general 🤔). The ClientCredentials would default to false but could be configured to be true.

I think in the end it does not change from the consumer's POV, but the current approach is rather implicit while having a config flag is more explicit.

I'll think about it for a bit 😄

Hmm good question. I guess this is more of a general design choice. Explicitly configuring it in the package as well removes a bit of magic and forces the user to make an explicit choice. Automatically falling back to AccessToken-only would make the package work without additional configuration in case somebody connects to a keycloak server where they do not have control over the configuration.

Looking at the official Java implementation, I do not see any explicit configuration, they seem to just check if the
refreshToken is there, and if not, return the grantToken:
https://github.com/keycloak/keycloak-client/blob/main/admin-client/src/main/java/org/keycloak/admin/client/token/TokenManager.java#L113

        if (currentToken.getRefreshToken() == null || refreshTokenExpired()) {
            return grantToken();
        }

So I'm in favour of the implicit approach, but in the end it's up to you 😊

[fschmtt]

I sticked to the implicit approach now.

Thank you so much for your feedback @phillipfickl, I appreciate it! 🥳

[phillipfickl]

Happily! 😊 Thank you for maintaining this package.
Once you have the other PR (events) ready for testing, please let me know, I'll again happily test and provide feedback!