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

Keycloak API stability improvements #1362

Merged
merged 1 commit into from Nov 2, 2019
Merged

Conversation

@rocketeerbkw
Copy link
Member

rocketeerbkw commented Oct 31, 2019

Checklist

  • Affected Issues have been mentioned in the Closing issues section
  • Documentation has been written/updated.
  • Changelog entry has been written

There are multiple reports of random API failures, all point back to keycloak. It appears that keycloak will just randomly not return data we know it has.

I'm still not able to reliably reproduce the bug, but I did find a couple issues that are likely culprits:

  1. The keycloak-admin library only authenticates for 1 minute and has a 55 second refresh timer. However, if there are keycloak requests happening when this refresh timer goes off, the new token is not reflected in the active requests and they fail with 401 not authenticated errors
  2. The API error handling erroneously sees the 401 errors in some cases and reports them as "404 not found" which caused some dead ends while hunting for the root issue.

This PR uses a cloned and hacked version of keycloak-admin that will check for a valid access token before every request and get a new one if necessary.

Still TODO is better error handling. In my testing, this change makes the 404 group errors go from at least every minute to non-existent. However I still am getting some user 404 errors.

At the very least, this as a hotfix should improve things.

Changelog Entry

Bugfix - Check for expired keycloak token before requesting data (#1337)

Closing issues

… request
@Schnitzel Schnitzel merged commit f1657e6 into master Nov 2, 2019
1 check passed
1 check passed
continuous-integration/jenkins/pr-merge This commit looks good
Details
@tobybellwood tobybellwood added this to the v1.2.0 milestone Dec 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.