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

SDK: Refresh Token implementation #1319

Merged
merged 21 commits into from
Jan 31, 2022
Merged

Conversation

langleyd
Copy link
Contributor

@langleyd langleyd commented Dec 9, 2021

Supports changes in element-hq/element-ios#5293 to resolve element-hq/element-ios#5292

Included in this PR

  • Introduces a new SDK option MXSDKOptions.authEnableRefreshTokens to enable refresh tokens. Access token auth will remain in place for the user while logged. Refresh token auth will be used on next login/register.
  • Implemented the login/register, refresh endpoints as detailed in Implement Refresh Tokens element-hq/element-ios#5292.
  • The refresh token are now stored on MXCredential, it has a function for updating the tokens when they are refreshed. It is the source of truth for these values.
  • Previously the access token was long-lived, was passed around and multiple references to it were held without ill-effect. Now that access tokens are short-lived, the approach is to have MXCredential be the source of truth and other classes that require it such as MXHTTPClient to pull in the value on demand.
  • MXHTTPClient requestWithMethod functions in the past forked, with half of the function signatures supporting an auth mechanism for the Identity server and the others being for homeserver requests. Rather than maintaining two different auth mechanisms for the http client there I created one that servers both identity and homeserver auth. It comes at the risk of updating the identity server auth but with the benefit of approach/code actually being understandable(vs having two implementations).
  • MXHTTPClient is designated as authenticated or not in the init(the later meaning the auth closures are not invoked) and similar to the old identity server auth, individual requests can be downgraded to remove the auth headers. All auth requests are now wrapped in an access token check that verifies its validity/requests a new token if required. All authenticated responses are also checked for the UNKNOWN_TOKEN and retried after again verifying/requesting an access token.
  • MXRestClients used to interact with the homeserver are responsible for the refresh token exchange. A dispatch group and queue are used to ensure that the first request for an access token that requests an exchange with the server leads and all other requests for the token block until the leader completes. When they are unblocked the new token is in place.
  • Refreshes can also take place in other processes like extensions. To ensure there is only 1 leader across processes we do a read + write of the credentials from disk effectively within a single transaction using NSFileCoordinator. If the read tells use the credential is good we us it, if not we refresh the token with the server and write it back within the transaction so that the next read get is and we don't have race condition.

TODO

  • Finish preemptive request of tokens at set interval before expiration.
  • Make sure with the new code and in place and MXSDKOptions.authEnableRefreshTokens enabled that access token auth continues to work until the users logs out.
  • Make sure with the new code and in place and MXSDKOptions.authEnableRefreshTokens disabled that access token auth continues to work.

…enewal apis(used for identity server but requires re-design for refresh tokens).
@langleyd langleyd marked this pull request as draft December 9, 2021 17:38
@langleyd langleyd changed the title First pass at refresh token support using existing httpClient token r… Refresh Token implementation Dec 9, 2021
@langleyd langleyd changed the title Refresh Token implementation SDK: Refresh Token implementation Dec 14, 2021
@langleyd langleyd marked this pull request as ready for review January 24, 2022 12:45
MatrixSDK/Data/MXCredentials.h Outdated Show resolved Hide resolved
MatrixSDK/Data/MXCredentials.m Show resolved Hide resolved
MatrixSDK/Data/MXRefreshTokenData.h Show resolved Hide resolved
MatrixSDK/JSONModels/MXJSONModels.h Outdated Show resolved Hide resolved
MatrixSDK/MXRestClient.h Outdated Show resolved Hide resolved
MatrixSDK/Utils/MXHTTPClient.m Outdated Show resolved Hide resolved
MatrixSDK/Utils/MXHTTPClient.m Outdated Show resolved Hide resolved
MatrixSDK/Utils/MXHTTPClient.m Outdated Show resolved Hide resolved
MatrixSDK/Utils/MXHTTPClient.m Outdated Show resolved Hide resolved
MatrixSDK/Utils/MXHTTPClient.m Outdated Show resolved Hide resolved
@ismailgulek
Copy link
Contributor

ismailgulek commented Jan 27, 2022

I wonder if it would make sense to have default valued parameters on MXRestClient Swift initializer. We had to make a lot of changes in the tests. Totally up to you.

As per the spec the refresh token can be nil on the refresh request, in which case we assume the existing refresh token is still valid.
@langleyd langleyd merged commit cd839f5 into develop Jan 31, 2022
@langleyd langleyd deleted the langleyd/5292_refresh_tokens branch January 31, 2022 08:58
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.

Implement Refresh Tokens
2 participants