Skip to content

Commit

Permalink
(Task AB#1268089) | [Token Refresh Flow] Defer token store unlock unt…
Browse files Browse the repository at this point in the history
…il refresh response action occurs (#168)

- [z] I've read, understood, and done my best to follow the [*CONTRIBUTING guidelines*](https://github.com/mindbody/conduit/blob/master/CONTRIBUTING.md).

This pull request includes (pick all that apply):

- [x] Bugfixes
- [ ] New features
- [ ] Breaking changes
- [ ] Documentation updates
- [ ] Unit tests
- [ ] Other

### Summary
The goal of this pull request is to ensure that during a token refresh, the token store lock stays engaged until after any token store I/O resulting from the outcome of the token refresh has occurred. 

Presently the token store lock disengages immediately after a token refresh response is received _but before_ any actions are taken on the token store (such as saving the new token to the token store). This, theoretically, could allow premature token store access before the outcome of the token refresh call has had a chance to propagate its result to the token store.  If this were to occur, the premature access would result in the token store returning a token that was just burnt by the token refresh call, resulting in call failure due to either an access token that's no longer valid, or a refresh token that was just used.

### Implementation
Move & split the disengagement of the token store lock from where it is now (executing immediately after receiving a refresh response), into two places: 
1) In the case of a token refresh success, the token store unluck now occurs directly after the new token is stored to the token store.
2) In the case of a token refresh failure, the token store unlock now occurs directly after the previous token is purged from the token store.

### Test Plan
Induce token refreshes in any client apps that consume this framework & ensure regular functionality is observed.  I have been testing these changes in MBApp while hunting down a refresh token race condition and all is working as expected
  • Loading branch information
brettwellmanmbo authored Feb 2, 2024
1 parent 8485e1f commit 62ec0e6
Showing 1 changed file with 5 additions and 1 deletion.
6 changes: 5 additions & 1 deletion Sources/Conduit/Auth/OAuth2RequestPipelineMiddleware.swift
Original file line number Diff line number Diff line change
Expand Up @@ -72,19 +72,23 @@ public struct OAuth2RequestPipelineMiddleware: RequestPipelineMiddleware {
tokenStorage.lockRefreshToken(timeout: tokenRefreshLockRelinquishInterval, client: clientConfiguration, authorization: authorization)
logger.info("Token is expired, proceeding to refresh token")
OAuth2TokenRefreshCoordinator.shared.beginTokenRefresh()

refresh(token: token) { result in
self.tokenStorage.unlockRefreshTokenFor(client: self.clientConfiguration, authorization: self.authorization)
switch result {
case .error(let error):
logger.warn("There was an error refreshing the token")
if case OAuth2Error.clientFailure = error {
self.tokenStorage.removeTokenFor(client: self.clientConfiguration, authorization: self.authorization)
}
tokenStorage.unlockRefreshTokenFor(client: clientConfiguration, authorization: authorization)
completion(.error(error))
case .value(let newToken):
logger.info("Successfully refreshed token")
logger.debug("New token issued: \(newToken)")
self.tokenStorage.store(token: newToken, for: self.clientConfiguration, with: self.authorization)
tokenStorage.unlockRefreshTokenFor(client: clientConfiguration, authorization: authorization)

/// Resume original request using freshly obtained bearer token
self.makeRequestByApplyingAuthorizationHeader(to: request, with: newToken, completion: completion)
}
OAuth2TokenRefreshCoordinator.shared.endTokenRefresh()
Expand Down

0 comments on commit 62ec0e6

Please sign in to comment.