Skip to content

Commit dd9e701

Browse files
author
kai
committed
Add error handling for XPC fallback logic
1 parent 4e1f4bc commit dd9e701

8 files changed

+294
-50
lines changed

‎IdentityCore/src/controllers/MSIDLocalInteractiveController.m‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -242,9 +242,9 @@ - (void)acquireTokenWithRequest:(MSIDInteractiveTokenRequest *)request
242242
}];
243243
}
244244

245-
- (BOOL)shouldFallback:(NSError *)error
245+
- (void)shouldSkipAcquireTokenBasedOn:(NSError *)error
246246
{
247-
return YES;
247+
// This method is not used in this class.
248248
}
249249

250250
@end

‎IdentityCore/src/controllers/MSIDRequestControlling.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ NS_ASSUME_NONNULL_BEGIN
3131
@protocol MSIDRequestControlling <NSObject>
3232

3333
- (void)acquireToken:(nonnull MSIDRequestCompletionBlock)completionBlock;
34-
- (BOOL)shouldFallback:(nonnull NSError *)error;
34+
- (void)shouldSkipAcquireTokenBasedOn:(nonnull NSError *)error;
3535

3636
@end
3737

‎IdentityCore/src/controllers/MSIDSilentController.m‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ - (void)acquireTokenWithRequest:(MSIDSilentTokenRequest *)request
111111
MSID_LOG_WITH_CTX(MSIDLogLevelError, nil, @"Passed nil completionBlock");
112112
return;
113113
}
114-
114+
115115
CONDITIONAL_START_EVENT(CONDITIONAL_SHARED_INSTANCE, self.requestParameters.telemetryRequestId, MSID_TELEMETRY_EVENT_API_EVENT);
116116
self.currentRequest = request;
117117
[request executeRequestWithCompletion:^(MSIDTokenResult *result, NSError *error)
@@ -143,24 +143,26 @@ - (void)acquireTokenWithRequest:(MSIDSilentTokenRequest *)request
143143
self.currentRequest = nil;
144144
MSIDRequestCompletionBlock completionBlockWrapper = ^(MSIDTokenResult *fallResult, NSError *fallError)
145145
{
146-
// We don't have any meaningful information from fallback controller (edge case of SSO error) so we use the local controller result earlier
147-
if (!fallResult && (fallError.code == MSIDErrorSSOExtensionUnexpectedError))
146+
// Only return fallback when there is a valid result, otherwise, return error from main controller
147+
// (!result && !error) is a special case when using local silent controller and to skip local refreh token (broker first flow). In this case, the main controller is the 1st fallback controller, and should return the error
148+
if (fallResult || (!result && !error))
148149
{
149-
completionBlock(result, error);
150+
completionBlock(fallResult, fallError);
150151
}
151152
else
152153
{
153-
completionBlock(fallResult, fallError);
154+
completionBlock(result, error);
154155
}
155156
};
156-
157+
158+
[self.fallbackController shouldSkipAcquireTokenBasedOn:error];
157159
[self.fallbackController acquireToken:completionBlockWrapper];
158160
}];
159161
}
160162

161-
- (BOOL)shouldFallback:(NSError *)error
163+
- (void)shouldSkipAcquireTokenBasedOn:(NSError *)error
162164
{
163-
return YES;
165+
// This method is not used in this class.
164166
}
165167

166168
@end

‎IdentityCore/src/controllers/broker/MSIDSSOExtensionInteractiveTokenRequestController.m‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,4 +118,5 @@ - (BOOL)shouldFallback:(NSError *)error
118118
}
119119

120120
@end
121+
121122
#endif

‎IdentityCore/src/controllers/broker/MSIDSSOExtensionSilentTokenRequestController.m‎

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -49,33 +49,5 @@ + (BOOL)canPerformRequest
4949
return [[ASAuthorizationSingleSignOnProvider msidSharedProvider] canPerformAuthorization];
5050
}
5151

52-
- (BOOL)shouldFallback:(NSError *)error
53-
{
54-
MSID_LOG_WITH_CTX(MSIDLogLevelInfo, self.requestParameters, @"Looking if we should fallback to fallback controller, error: %ld error domain: %@.", (long)error.code, error.domain);
55-
56-
if (!self.fallbackController)
57-
{
58-
MSID_LOG_WITH_CTX(MSIDLogLevelInfo, self.requestParameters, @"fallback controller is nil, SSO extension controller should fallback: NO");
59-
return NO;
60-
}
61-
62-
// If it is MSIDErrorDomain and Sso Extension returns unexpected error, we should fall back to local controler and unblock user
63-
if (![error.domain isEqualToString:ASAuthorizationErrorDomain] && ![error.domain isEqualToString:MSIDErrorDomain]) return NO;
64-
65-
BOOL shouldFallback = NO;
66-
switch (error.code)
67-
{
68-
case ASAuthorizationErrorNotHandled:
69-
case ASAuthorizationErrorUnknown:
70-
case ASAuthorizationErrorFailed:
71-
case MSIDErrorSSOExtensionUnexpectedError:
72-
shouldFallback = YES;
73-
}
74-
75-
MSID_LOG_WITH_CTX(MSIDLogLevelInfo, self.requestParameters, @"SSO extension controller should fallback: %@", shouldFallback ? @"YES" : @"NO");
76-
77-
return shouldFallback;
78-
}
79-
8052
@end
8153
#endif

‎IdentityCore/src/controllers/broker/ios/MSIDBrokerInteractiveController.m‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -478,6 +478,11 @@ - (BOOL)hasCompletionBlock
478478
return result;
479479
}
480480

481+
- (void)shouldSkipAcquireTokenBasedOn:(NSError *)error
482+
{
483+
// This method is not used in this class.
484+
}
485+
481486
#pragma mark - Fallback
482487

483488
- (void)handleFailedOpenURL:(BOOL)shouldFallbackToLocalController

‎IdentityCore/src/controllers/broker/mac/MSIDXpcSilentTokenRequestController.m‎

Lines changed: 53 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,20 +28,44 @@
2828
#import "MSIDLogger+Internal.h"
2929
#import "MSIDXpcProviderCache.h"
3030

31+
@interface MSIDXpcSilentTokenRequestController ()
32+
33+
// shouldSkipXpcRequest is used when the fallback is not needed for non ASAuthorizationErrorDomain or MSIDErrorSSOExtensionUnexpectedError
34+
@property (nonatomic) BOOL shouldSkipAcquireToken;
35+
36+
@end
37+
3138
@implementation MSIDXpcSilentTokenRequestController
3239

3340
- (void)acquireToken:(MSIDRequestCompletionBlock)completionBlock
3441
{
35-
MSID_LOG_WITH_CTX(MSIDLogLevelInfo, self.requestParameters, @"Beginning silent broker xpc flow.");
36-
MSIDRequestCompletionBlock completionBlockWrapper = ^(MSIDTokenResult *result, NSError *error)
42+
MSID_LOG_WITH_CTX(MSIDLogLevelInfo, self.requestParameters, @"Beginning silent broker xpc flow, should use Xpc flow to acquire token: %@", @(!self.shouldSkipAcquireToken));
43+
if (!self.shouldSkipAcquireToken)
3744
{
38-
MSID_LOG_WITH_CTX(MSIDLogLevelInfo, self.requestParameters, @"Silent broker xpc flow finished. Result %@, error: %ld error domain: %@, shouldFallBack: %@", _PII_NULLIFY(result), (long)error.code, error.domain, @(self.fallbackController != nil));
39-
completionBlock(result, error);
40-
};
41-
42-
__auto_type request = [self.tokenRequestProvider silentXpcTokenRequestWithParameters:self.requestParameters
43-
forceRefresh:self.forceRefresh];
44-
[self acquireTokenWithRequest:request completionBlock:completionBlockWrapper];
45+
MSIDRequestCompletionBlock completionBlockWrapper = ^(MSIDTokenResult *result, NSError *error)
46+
{
47+
MSID_LOG_WITH_CTX(MSIDLogLevelInfo, self.requestParameters, @"Silent broker xpc flow finished. Result %@, error: %ld error domain: %@, shouldFallBack: %@", _PII_NULLIFY(result), (long)error.code, error.domain, @(self.fallbackController != nil));
48+
completionBlock(result, error);
49+
};
50+
51+
__auto_type request = [self.tokenRequestProvider silentXpcTokenRequestWithParameters:self.requestParameters
52+
forceRefresh:self.forceRefresh];
53+
[self acquireTokenWithRequest:request completionBlock:completionBlockWrapper];
54+
}
55+
else
56+
{
57+
// self.fallbackController cannot be nil here as it has been validated from caller
58+
if (self.fallbackController)
59+
{
60+
[self.fallbackController acquireToken:completionBlock];
61+
}
62+
else
63+
{
64+
// Add handler in case
65+
NSError *error = MSIDCreateError(MSIDErrorDomain, MSIDErrorInternal, @"Fallback controller is nil", nil, nil, nil, nil, nil, YES);
66+
if (completionBlock) completionBlock(nil, error);
67+
}
68+
}
4569
}
4670

4771
+ (BOOL)canPerformRequest
@@ -55,4 +79,24 @@ + (BOOL)canPerformRequest
5579
}
5680
}
5781

82+
- (void)shouldSkipAcquireTokenBasedOn:(NSError *)error
83+
{
84+
self.shouldSkipAcquireToken = YES;
85+
MSID_LOG_WITH_CTX(MSIDLogLevelInfo, self.requestParameters, @"Looking if we should fallback to Xpc controller, error: %ld error domain: %@.", (long)error.code, error.domain);
86+
// If it is MSIDErrorDomain and Sso Extension returns unexpected error, we should fall back to local controler and unblock user
87+
if (![error.domain isEqualToString:ASAuthorizationErrorDomain] && ![error.domain isEqualToString:MSIDErrorDomain]) {
88+
}
89+
90+
switch (error.code)
91+
{
92+
case ASAuthorizationErrorNotHandled:
93+
case ASAuthorizationErrorUnknown:
94+
case ASAuthorizationErrorFailed:
95+
case MSIDErrorSSOExtensionUnexpectedError:
96+
self.shouldSkipAcquireToken = NO;
97+
}
98+
99+
MSID_LOG_WITH_CTX(MSIDLogLevelInfo, self.requestParameters, @"Xpc controller should do fallback: %@", !self.shouldSkipAcquireToken ? @"YES" : @"NO");
100+
}
101+
58102
@end

0 commit comments

Comments
 (0)