MOB-439: Access token refreshing #22
Conversation
ID.me WebVerify SDK/IDmeWebVerify.m
Outdated
| } | ||
|
|
||
| NSString* token = [self.keychainData accessTokenForScope:scope]; |
There was a problem hiding this comment.
shouldn't you use latestScope instead of scope here?
| @@ -11,6 +11,7 @@ | |||
| /// Keychain Constants | |||
| #define IDME_KEYCHAIN_DATA_ACCOUNT @"IDME_KEYCHAIN_DATA" | |||
| #define IDME_EXPIRATION_DATE @"IDME_EXPIRATION_DATE" | |||
| #define IDME_REFRESH_EXPIRATION_DATE @"IDME_REFRESH_EXPIRATION_DATE" | |||
There was a problem hiding this comment.
when you load scope tokens from keychain, are you loading this new property?
There was a problem hiding this comment.
Yes, it is loaded.
I load a dictionary and save it directly in tokensByScope that is why this variable is not used when loading
ID.me WebVerify SDK/IDmeWebVerify.m
Outdated
| } | ||
|
|
||
| #pragma mark - Keychain access | ||
| - (void)saveDataFromJson:(NSDictionary* _Nonnull)json { |
There was a problem hiding this comment.
It is not clear enough what this function is expecting/saving just by reading its name. Please consider changing it for something more descriptive
| @@ -304,6 +308,39 @@ - (void)getAccessTokenWithScope:(NSString* _Nullable)scope forceRefreshing:(BOOL | |||
|
|
|||
| } | |||
|
|
|||
| - (void)refreshTokenForScope:(NSString* _Nonnull)scope callback:(IDmeVerifyWebVerifyTokenResults _Nonnull)callback { | |||
There was a problem hiding this comment.
getAccessTokenWithScope accepts a nullable scope value. Shouldn't this be similar to be consistent?
There was a problem hiding this comment.
I do not think so. This method is just called internally and only if there is a scope (either passed to getAccesToken or the latest used scope from the keychain)
ID.me WebVerify SDK/IDmeWebVerify.m
Outdated
| // TODO: refresh token | ||
| callback(nil, [self notAuthorizedErrorWithUserInfo:nil]); | ||
| // token has expired | ||
| [self refreshTokenForScope:scope callback:callback]; |
There was a problem hiding this comment.
here too, latestScope vs scope?
ID.me WebVerify SDK/IDmeWebVerify.m
Outdated
| #define IDME_WEB_VERIFY_ERROR_DESCRIPTION_PARAM @"error_description" | ||
|
|
||
| // HTTP methods | ||
| #define GET_METHOD @"GET" |
There was a problem hiding this comment.
this doesn't seem to be used
ID.me WebVerify SDK/IDmeWebVerify.m
Outdated
| #define IDME_WEB_VERIFY_SIGN_UP_OR_LOGIN IDME_WEB_VERIFY_BASE_URL @"oauth/authorize?client_id=%@&redirect_uri=%@&response_type=token&scope=%@&op=%@" | ||
| #define IDME_WEB_VERIFY_GET_AUTH_URI IDME_WEB_VERIFY_BASE_URL @"oauth/authorize?client_id=%@&redirect_uri=%@&response_type=code&scope=%@" | ||
| #define IDME_WEB_VERIFY_SIGN_UP_OR_LOGIN IDME_WEB_VERIFY_BASE_URL @"oauth/authorize?client_id=%@&redirect_uri=%@&response_type=code&scope=%@&op=%@" | ||
| #define IDME_WEB_VERIFY_REFRESH_CODE_URL IDME_WEB_VERIFY_BASE_URL @"oauth/token" |
There was a problem hiding this comment.
please keep this alphabetically sorted
ID.me WebVerify SDK/IDmeWebVerify.m
Outdated
| #define IDME_WEB_VERIFY_GET_USER_PROFILE IDME_WEB_VERIFY_BASE_URL @"api/public/v2/data.json?access_token=%@" | ||
| #define IDME_WEB_VERIFY_REGISTER_CONNECTION_URI IDME_WEB_VERIFY_BASE_URL @"oauth/authorize?client_id=%@&redirect_uri=%@&response_type=code&op=signin&scope=%@&connect=%@&access_token=%@" | ||
| #define IDME_WEB_VERIFY_REGISTER_AFFILIATION_URI IDME_WEB_VERIFY_BASE_URL @"oauth/authorize?client_id=%@&redirect_uri=%@&response_type=code&scope=%@&access_token=%@" | ||
|
|
||
| /// Data Constants | ||
| #define IDME_WEB_VERIFY_ACCESS_TOKEN_PARAM @"access_token" | ||
| #define IDME_WEB_VERIFY_REFRESH_TOKEN_PARAM @"refresh_token" |
ID.me WebVerify SDK/IDmeWebVerify.m
Outdated
| @@ -643,7 +642,7 @@ - (void)makePostRequestWithUrl:(NSString *_Nonnull)urlString parameters:(NSStrin | |||
| } | |||
|
|
|||
| #pragma mark - Keychain access | |||
| - (void)saveDataFromJson:(NSDictionary* _Nonnull)json { | |||
| - (void)saveKeychainDataFromJson:(NSDictionary* _Nonnull)json scope:(NSString* _Nonnull)scope { | |||
There was a problem hiding this comment.
I was thinking something like saveTokenDataFromJson, I think it's more clear, if you read it then you will know that this functions expect a JSON representation of a token object (access token + refresh token + etc.) What do you think?
Fixes #7
Description
Access token refreshing implemented using the OAuth flow.
The expiration date of the refresh token is also saved so that if the refresh token is expired then the SDK can return an error immediately
Todos