-
Notifications
You must be signed in to change notification settings - Fork 37
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
Use single FRT #1470
base: dev
Are you sure you want to change the base?
Use single FRT #1470
Changes from 19 commits
b7ac9d4
7250799
63a1fa9
034c23a
c22fe16
3aa7008
2e732d1
0e2175f
340f3d3
748fed2
9adf79b
357ac47
3c5a825
cee6ee5
2f5fb2f
248c3b1
477991e
006c43d
c443a0b
d8c4614
0fbe5d1
48800fb
742807e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -122,6 +122,8 @@ extern NSString * _Nonnull const MSID_DEVICE_MODEL_KEY;//E.g. iPhone 5S | |
extern NSString * _Nonnull const MSID_APP_NAME_KEY; | ||
extern NSString * _Nonnull const MSID_APP_VER_KEY; | ||
extern NSString * _Nonnull const MSID_CCS_HINT_KEY; | ||
extern NSString * _Nonnull const MSID_WEBAUTH_IGNORE_SSO_KEY; | ||
extern NSString * _Nonnull const MSID_WEBAUTH_REFRESH_TOKEN_KEY; | ||
|
||
extern NSString * _Nonnull const MSID_DEFAULT_FAMILY_ID; | ||
extern NSString * _Nonnull const MSID_ADAL_SDK_NAME; | ||
|
@@ -149,6 +151,9 @@ extern NSString * _Nonnull const MSID_POP_TOKEN_KEY_LABEL; | |
extern NSString * _Nonnull const MSID_THROTTLING_METADATA_KEYCHAIN; | ||
extern NSString * _Nonnull const MSID_THROTTLING_METADATA_KEYCHAIN_VERSION; | ||
|
||
extern NSString * _Nonnull const MSID_USE_SINGLE_FRT_KEYCHAIN; | ||
extern NSString * _Nonnull const MSID_USE_SINGLE_FRT_KEY; | ||
|
||
extern NSString * _Nonnull const MSID_SHARED_MODE_CURRENT_ACCOUNT_CHANGED_NOTIFICATION_KEY; | ||
|
||
extern NSString * _Nonnull const MSID_PREFERRED_AUTH_METHOD_KEY; | ||
|
@@ -171,6 +176,27 @@ typedef NS_ENUM(NSInteger, MSIDPlatformSequenceIndex) | |
MSIDPlatformSequenceIndexLast = MSIDPlatformSequenceIndexBrowserCore, | ||
}; | ||
|
||
typedef NS_ENUM(NSInteger, MSIDIsFRTEnabledStatus) | ||
{ | ||
// FRT has not been explicitly enabled with keychain item | ||
MSIDIsFRTEnabledStatusNotEnabled = 0, | ||
|
||
// FRT is enabled | ||
MSIDIsFRTEnabledStatusActive, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question: Is there a difference between enabled and active? If not, can we use the same (enable vs disable) or active/not active There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No difference, updated to enable/disable |
||
|
||
// Client app has disabled FRT through MSIDRequestParameters or was disabled previuosly by keychain item | ||
MSIDIsFRTEnabledStatusDisabledByClientApp, | ||
|
||
// There was an error reading keychain item | ||
MSIDIsFRTEnabledStatusDisabledByKeychainError, | ||
|
||
// There was an error deserializing keychain item | ||
MSIDIsFRTEnabledStatusDisabledByDeserializationError, | ||
|
||
// FRT has been disabled with keychain item | ||
MSIDIsFRTEnabledStatusDisabledByKeychainItem | ||
}; | ||
|
||
extern NSString * _Nonnull const MSID_BROWSER_RESPONSE_SWITCH_BROWSER; | ||
extern NSString * _Nonnull const MSID_BROWSER_RESPONSE_SWITCH_BROWSER_RESUME; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,6 +36,10 @@ | |
#import "MSIDAppMetadataCacheKey.h" | ||
#import "MSIDAppMetadataCacheQuery.h" | ||
#import "MSIDExtendedTokenCacheDataSource.h" | ||
#import "MSIDConfiguration.h" | ||
#import "MSIDConstants.h" | ||
#import "MSIDJsonObject.h" | ||
#import "MSIDFlightManager.h" | ||
|
||
@interface MSIDAccountCredentialCache() | ||
{ | ||
|
@@ -382,7 +386,7 @@ - (BOOL)removeCredential:(nonnull MSIDCredentialCacheItem *)credential | |
|
||
BOOL result = [_dataSource removeTokensWithKey:key context:context error:error]; | ||
|
||
if (result && credential.credentialType == MSIDRefreshTokenType) | ||
if (result && (credential.credentialType == MSIDRefreshTokenType || credential.credentialType == MSIDFamilyRefreshTokenType)) | ||
{ | ||
[_dataSource saveWipeInfoWithContext:context error:nil]; | ||
} | ||
|
@@ -556,4 +560,159 @@ - (BOOL)removeAppMetadata:(nonnull MSIDAppMetadataCacheItem *)appMetadata | |
return cacheItems; | ||
} | ||
|
||
- (MSIDIsFRTEnabledStatus)checkFRTEnabled:(nullable id<MSIDRequestContext>)context | ||
error:(NSError * _Nullable __autoreleasing * _Nullable)error | ||
{ | ||
// This block will be used to check feature flags and update FRT settings if needed, depending on the current status | ||
// of the keychain item, avoiding an unnecessary read or update if status is the same | ||
MSIDIsFRTEnabledStatus (^checkFeatureFlagsAndReturn)(MSIDIsFRTEnabledStatus) = ^MSIDIsFRTEnabledStatus(MSIDIsFRTEnabledStatus status) { | ||
|
||
// Check if FRT is enabled by feature flight | ||
MSIDFlightManager *flightManager = [MSIDFlightManager sharedInstance]; | ||
BOOL flagEnableFRT = [flightManager boolForKey:@"enable_client_sfrt_by_tenant_id"]; // TODO: Replace this by the constant from the other branch, and remove the hardcoded YES | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: do you still need TODO here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I will replace this when the other PR (#1489) is merged. |
||
BOOL flagDisableAllFRT = [flightManager boolForKey:@"disable_client_sfrt_for_all"]; | ||
BOOL shouldEnableFRT = flagEnableFRT && !flagDisableAllFRT; | ||
|
||
switch (status) | ||
{ | ||
// No entry in cache | ||
case MSIDIsFRTEnabledStatusNotEnabled: | ||
if (shouldEnableFRT) | ||
{ | ||
[self updateFRTSettings:YES context:context error:nil]; | ||
kaisong1990 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return MSIDIsFRTEnabledStatusActive; | ||
} | ||
break; | ||
|
||
// Deserialization error, try to enable/disable if needed | ||
case MSIDIsFRTEnabledStatusDisabledByDeserializationError: | ||
if (shouldEnableFRT) | ||
{ | ||
[self updateFRTSettings:YES context:context error:nil]; | ||
return MSIDIsFRTEnabledStatusActive; | ||
} | ||
else if (flagDisableAllFRT) | ||
{ | ||
[self updateFRTSettings:NO context:context error:nil]; | ||
return MSIDIsFRTEnabledStatusDisabledByKeychainItem; | ||
} | ||
break; | ||
|
||
// FRT is currently enabled, check to see if should be disabled | ||
case MSIDIsFRTEnabledStatusActive: | ||
if (flagDisableAllFRT) | ||
{ | ||
[self updateFRTSettings:NO context:context error:nil]; | ||
return MSIDIsFRTEnabledStatusDisabledByKeychainItem; | ||
} | ||
break; | ||
|
||
// FRT is disabled, check to see if should be enabled | ||
case MSIDIsFRTEnabledStatusDisabledByKeychainItem: | ||
if (shouldEnableFRT) | ||
{ | ||
[self updateFRTSettings:YES context:context error:nil]; | ||
return MSIDIsFRTEnabledStatusActive; | ||
} | ||
break; | ||
|
||
// Error reading keychain item, do not update settings | ||
case MSIDIsFRTEnabledStatusDisabledByKeychainError: | ||
break; | ||
|
||
// Feature is disabled by client app, do nothing with keychain item | ||
case MSIDIsFRTEnabledStatusDisabledByClientApp: | ||
break; | ||
} | ||
|
||
return status; | ||
}; | ||
|
||
// Check if FRT is disabled by client | ||
kaisong1990 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (context.disableFRT) | ||
{ | ||
MSID_LOG_WITH_CTX(MSIDLogLevelInfo, context, @"FRT disabled by MSAL client app, returning NO"); | ||
return checkFeatureFlagsAndReturn(MSIDIsFRTEnabledStatusDisabledByClientApp); | ||
} | ||
|
||
NSError *readError = nil; | ||
NSArray<MSIDJsonObject *> *jsonObjects = [_dataSource jsonObjectsWithKey:[MSIDAccountCredentialCache checkFRTCacheKey] | ||
serializer:[MSIDCacheItemJsonSerializer new] | ||
context:context | ||
error:&readError]; | ||
|
||
if (readError) | ||
{ | ||
MSID_LOG_WITH_CTX(MSIDLogLevelError, context, @"Failed to retrieve FRT cache entry, error: %@", readError); | ||
if (error) | ||
{ | ||
*error = readError; | ||
} | ||
return checkFeatureFlagsAndReturn(MSIDIsFRTEnabledStatusDisabledByKeychainError); | ||
} | ||
|
||
if (![jsonObjects count]) | ||
{ | ||
MSID_LOG_WITH_CTX(MSIDLogLevelInfo, context, @"No FRT cache entry found, returning NO"); | ||
return checkFeatureFlagsAndReturn(MSIDIsFRTEnabledStatusNotEnabled); | ||
} | ||
|
||
NSDictionary *dict = [jsonObjects[0] jsonDictionary]; | ||
if (!dict || ![dict isKindOfClass:[NSDictionary class]] || [dict objectForKey:MSID_USE_SINGLE_FRT_KEY] == nil) | ||
{ | ||
MSID_LOG_WITH_CTX(MSIDLogLevelError, context, @"Failed to deserialize FRT cache entry, returning NO"); | ||
return checkFeatureFlagsAndReturn(MSIDIsFRTEnabledStatusDisabledByDeserializationError); | ||
} | ||
|
||
id useFRT = dict[MSID_USE_SINGLE_FRT_KEY]; | ||
if(([useFRT isKindOfClass:[NSNumber class]] || [useFRT isKindOfClass:[NSString class]]) && [useFRT boolValue]) | ||
{ | ||
MSID_LOG_WITH_CTX(MSIDLogLevelInfo, context, @"FRT is enabled"); | ||
return checkFeatureFlagsAndReturn(MSIDIsFRTEnabledStatusActive); | ||
} | ||
|
||
MSID_LOG_WITH_CTX(MSIDLogLevelInfo, context, @"FRT is disabled"); | ||
return checkFeatureFlagsAndReturn(MSIDIsFRTEnabledStatusDisabledByKeychainItem); | ||
} | ||
|
||
- (void)updateFRTSettings:(BOOL)enableFRT | ||
context:(nullable id<MSIDRequestContext>)context | ||
error:(NSError * _Nullable __autoreleasing * _Nullable)error | ||
{ | ||
MSID_LOG_WITH_CTX(MSIDLogLevelInfo, context, @"Updating UseSingleFRT Item with enableFRT:%@", enableFRT ? @"YES" : @"NO"); | ||
|
||
NSDictionary *settings = @{MSID_USE_SINGLE_FRT_KEY: @(enableFRT)}; | ||
|
||
NSError *saveError = nil; | ||
MSIDJsonObject *jsonObject = [[MSIDJsonObject alloc] initWithJSONDictionary:settings error:&saveError]; | ||
|
||
[_dataSource saveJsonObject:jsonObject | ||
serializer:[MSIDCacheItemJsonSerializer new] | ||
key:[MSIDAccountCredentialCache checkFRTCacheKey] | ||
context:context | ||
error:&saveError]; | ||
|
||
if (saveError) | ||
{ | ||
MSID_LOG_WITH_CTX(MSIDLogLevelError, context, @"Failed to save FRT cache entry, error: %@", saveError); | ||
if (error) | ||
{ | ||
*error = saveError; | ||
} | ||
} | ||
} | ||
|
||
+ (MSIDCacheKey *)checkFRTCacheKey | ||
{ | ||
static MSIDCacheKey *cacheKey = nil; | ||
static dispatch_once_t onceToken; | ||
dispatch_once(&onceToken, ^{ | ||
cacheKey = [[MSIDCacheKey alloc] initWithAccount:MSID_USE_SINGLE_FRT_KEYCHAIN | ||
service:MSID_USE_SINGLE_FRT_KEYCHAIN | ||
generic:nil | ||
type:nil]; | ||
}); | ||
return cacheKey; | ||
} | ||
|
||
@end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: readwrite is by default I think, can be removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated