fix: iam/cp4d token refresh logic #73
Conversation
| assert.Equal(t, "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJ1c2VybmFtZSI6ImhlbGxvIiwicm9sZSI6InVzZXIiLCJwZXJtaXNzaW9ucyI6WyJhZG1pbmlzdHJhdG9yIiwiZGVwbG95bWVudF9hZG1pbiJdLCJzdWIiOiJoZWxsbyIsImlzcyI6IkpvaG4iLCJhdWQiOiJEU1giLCJ1aWQiOiI5OTkiLCJpYXQiOjE1NjAyNzcwNTEsImV4cCI6MTU2MDI4MTgxOSwianRpIjoiMDRkMjBiMjUtZWUyZC00MDBmLTg2MjMtOGNkODA3MGI1NDY4In0.cIodB4I6CCcX8vfIImz7Cytux3GpWyObt9Gkur5g1QI", | ||
| token) | ||
| assert.NotNil(t, authenticator.tokenData) | ||
| assert.Equal(t, newRefreshTime, authenticator.tokenData.RefreshTime) |
There was a problem hiding this comment.
This line is the main case I wanted to test. Before, calling getToken would result in the next thread realizing it still needs to refresh since the previous goroutine may have not finished requesting a new token, which would mean the refresh time would be updated, and another goroutine would kick off. If the refresh time isn't updated, then we know it didn't attempt to request a new token.
| token) | ||
| assert.NotNil(t, authenticator.tokenData) | ||
| // RefreshTime should have advanced by 1 minute from the current time | ||
| newRefreshTime := GetCurrentTime() + 60 |
There was a problem hiding this comment.
I guess this validation step should occur less than a second after getToken() updates the refresh time to make sure that that assert.Equal() succeeds :)
There was a problem hiding this comment.
yeah the use of GetCurrentTime in the test is a bit worrisome but it seems to work..
jorge-ibm
force-pushed
the
fix/token-refresh-logic
branch
from
81ca287 to
abe8410
Compare
## [4.4.1](v4.4.0...v4.4.1) (2020-09-14) ### Bug Fixes * iam/cp4d token refresh logic ([#73](#73)) ([8d4685e](8d4685e))
|
🎉 This PR is included in version 4.4.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Fixes a very unique scenario described in https://github.ibm.com/arf/planning-sdk-squad/issues/2149. In the scenario where a client has been idle passed the refresh time, but before expiration time, the previous logic would result in multiple goroutines being kicked off attempting to request a new token until a new token was fetched or until the refresh time caught up to the current time. I've updated the new
refreshTimecalculation to be 1 minute ahead of the current time vs 1 minute ahead of the previousrefreshTimevalue.Java also probably has this same issue so I will update that core as well.
ref: https://github.ibm.com/arf/planning-sdk-squad/issues/2149