Skip to content

fix: improve check for validity of token#76

Merged
ewanharris merged 4 commits into
mainfrom
fix/refactor-validity-check
Apr 29, 2024
Merged

fix: improve check for validity of token#76
ewanharris merged 4 commits into
mainfrom
fix/refactor-validity-check

Conversation

@ewanharris

@ewanharris ewanharris commented Apr 25, 2024

Copy link
Copy Markdown
Member

Description

  • 1d20960 - Extends the testing of the token validity check to include checks for closer to the expiry time (expires in 6 minutes, 5 minutes)
  • 2084b0a - Refactors the validity check to split it out into separate null checks and time checks.

I've removed the jitter here as I'm not sure what it added and it made testing harder as we didn't have predictability. If we want to guard against a potential "thundering herd" on the token refresh then I think we'd be better off attempting to only allow one call to the token endpoints to occur at a time (not sure how to do this in Java myself, maybe if we used synchronized on exchangeToken?

The jitter calculation has been changed to use nextInt(TOKEN_EXPIRY_BUFFER_THRESHOLD_IN_SEC), this will return a value up to our 300 seconds limit that we can use to prevent multiple calls at the same time. From my testing nextLong could potentially return negative values so I believe this is not safe to use, given we want our jitter to be up to 300 seconds more then we nextInt with the bound is safer.

References

Resolves #75

Review Checklist

  • I have clicked on "allow edits by maintainers".
  • I have added documentation for new/changed functionality in this PR or in a PR to openfga.dev [Provide a link to any relevant PRs in the references section above]
  • The correct base branch is being used, if not main
  • I have added tests to validate that the change in functionality is working as expected

@ewanharris ewanharris requested a review from a team as a code owner April 25, 2024 21:34
@codecov-commenter

codecov-commenter commented Apr 25, 2024

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 28.65%. Comparing base (6717967) to head (f98569c).

Additional details and impacted files
@@             Coverage Diff              @@
##               main      #76      +/-   ##
============================================
+ Coverage     28.51%   28.65%   +0.13%     
- Complexity      721      723       +2     
============================================
  Files           145      145              
  Lines          5277     5283       +6     
  Branches        567      567              
============================================
+ Hits           1505     1514       +9     
+ Misses         3714     3712       -2     
+ Partials         58       57       -1     

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

@ewanharris ewanharris changed the title test: extend tests for access token validity check fix: improve check for validity of token Apr 26, 2024
@ewanharris ewanharris force-pushed the fix/refactor-validity-check branch 2 times, most recently from 4676fa4 to a0ce129 Compare April 26, 2024 14:11
@ewanharris ewanharris force-pushed the fix/refactor-validity-check branch from a0ce129 to c34968c Compare April 26, 2024 14:12
Comment thread src/main/java/dev/openfga/sdk/api/auth/AccessToken.java Outdated
Comment thread src/test/java/dev/openfga/sdk/api/auth/AccessTokenTest.java Outdated
Comment thread src/test/java/dev/openfga/sdk/api/auth/AccessTokenTest.java Outdated
Comment thread src/test/java/dev/openfga/sdk/api/auth/AccessTokenTest.java Outdated
Comment thread src/test/java/dev/openfga/sdk/api/client/OpenFgaClientTest.java
@ewanharris ewanharris added this pull request to the merge queue Apr 29, 2024
Merged via the queue into main with commit a7ad2c9 Apr 29, 2024
@ewanharris ewanharris deleted the fix/refactor-validity-check branch April 29, 2024 13:43
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.

Intermittent FgaApiAuthenticationError when using Oauth2 Client Credentials

3 participants