Add support for auth API v2 #5305
Conversation
lmac012
force-pushed
the
feature/lukasz/auth-v2
branch
3 times, most recently
from
976f3fc to
8fb36d2
Compare
lmac012
force-pushed
the
feature/lukasz/auth-v2
branch
from
ada3fa9 to
8f89042
Compare
| @@ -277,10 +327,16 @@ class RealSubscriptionsManager @Inject constructor( | |||
| } | |||
|
|
|||
| override suspend fun signOut() { | |||
| authRepository.getAccessTokenV2()?.run { | |||
| coroutineScope.launch { authClient.tryLogout(accessTokenV2 = jwt) } | |||
There was a problem hiding this comment.
Why are we doing coroutineScope.launch {} here? Can’t we put everything in this function in withContext(dispatcher.io()) ?
There was a problem hiding this comment.
There is no need to specify io dispatcher anywhere in this method since there are no thread-blocking operations here. Calling /logout endpoint is launched in a separate coroutine just because this is a side-effect that the caller of signOut() shouldn't care for and wait for. This matters e.g., when the user taps "Remove subscription from device" in settings and we want the repository transition to the signed out state ASAP, so that the UI can be updated accordingly.
There was a problem hiding this comment.
How is the Entitlement here different from Entitlement in AuthRepository?
There was a problem hiding this comment.
It's not different, just avoiding having a dependency between AuthRepository and AuthJwtValidator, since the latter has nothing to do with storage. Now that both of those classes live in the same module, we could extract Entitlement from AuthRepository/AuthJwtValidator and unify - I'll look into it.
| val workManager: WorkManager, | ||
| ) : BackgroundTokenRefresh { | ||
| override fun schedule() { | ||
| val workRequest = PeriodicWorkRequestBuilder<TokenRefreshWorker>(repeatInterval = Duration.ofDays(7)) |
There was a problem hiding this comment.
Why are we scheduling this work to repeat every 7 days? Access token expires every 4 hours and refresh tokens expire every month?
There was a problem hiding this comment.
It's a compromise between the overhead of running it to often and the risk of missing the 1-month deadline to refresh. 7 days gives us a healthy buffer even if the device happens to be offline for a week or two.
this will not be supported by auth api v2
lmac012
force-pushed
the
feature/lukasz/auth-v2
branch
from
8f89042 to
414b2bd
Compare
Task/Issue URL: https://app.asana.com/0/1205648422731273/1208821656071388/f
Description
This PR adds support for new version of the authentication API that uses OAuth 2.0 with PKCE extension. Usage of the new API is controlled via a feature flag which is currently disabled.
Steps to test this PR
See task.
No UI changes