-
Notifications
You must be signed in to change notification settings - Fork 35
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
Webview interaction and system webview #126
Changes from all commits
5f61177
e905d25
ba71b3d
fc1d8db
f29dcb3
c669afe
d8813b5
c8c826f
4dd6ec6
d987e60
8c99592
d99bc6f
10aae32
0e34d81
9e7fb0b
0bd3d7f
7b5255b
69e15a1
fbbb81e
c5675ec
f1207ee
d58eed6
6f737f0
b2c9929
0dc3a61
243fb01
9ea3817
a4d2286
459dc3a
a234566
a4eacd1
af44efc
36b3ef2
517a114
a5d230e
5ed28e3
2b8b756
1230844
dda26c7
375e1d4
f54800a
ec90fb5
b31e99f
8718d82
a9cff24
1ea816d
7b8979c
7d9ed31
58b7c6a
dad2296
9801600
9d505c8
97ddaa1
9ec3d45
804186a
17834c5
e5da20a
865eff9
185db0d
6183514
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
<?xml version="1.0" encoding="UTF-8"?> | ||
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd"> | ||
<plist version="1.0"> | ||
<dict> | ||
<key>IDEDidComputeMac32BitWarning</key> | ||
<true/> | ||
</dict> | ||
</plist> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -68,11 +68,53 @@ typedef NS_ENUM(NSInteger, MSIDErrorCode) | |
MSIDErrorInvalidRequest = -51014, | ||
MSIDErrorInvalidClient = -51015, | ||
MSIDErrorInvalidGrant = -51016, | ||
MSIDErrorInvalidParameter = -51017, | ||
MSIDErrorInvalidScope = -51017, | ||
MSIDErrorInvalidParameter = -51018, | ||
|
||
/*! | ||
The user or application failed to authenticate in the interactive flow. | ||
Inspect MSALOAuthErrorKey and MSALErrorDescriptionKey in the userInfo | ||
dictionary for more detailed information about the specific error. | ||
*/ | ||
MSIDErrorAuthorizationFailed = -52020, | ||
|
||
/*! | ||
The state returned by the server does not match the state that was sent to | ||
the server at the beginning of the authorization attempt. | ||
*/ | ||
MSIDErrorInvalidState = -52501, | ||
/*! | ||
Interaction required errors occur because of a wide variety of errors | ||
returned by the authentication service. | ||
*/ | ||
MSIDErrorMismatchedUser = -52101, | ||
MSIDErrorNoAuthorizationResponse = -52102, | ||
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: no authorization code? 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, it's just url nil response. |
||
MSIDErrorBadAuthorizationResponse = -52103, | ||
|
||
|
||
MSIDErrorUserCancel = -51019, | ||
/*! | ||
The authentication request was cancelled programmatically. | ||
*/ | ||
MSIDErrorSessionCanceled = -51020, | ||
/*! | ||
An interactive authentication session is already running with the | ||
SafariViewController visible. Another authentication session can not be | ||
launched yet. | ||
*/ | ||
MSIDErrorInteractiveSessionAlreadyRunning = -51021, | ||
/*! | ||
An interactive authentication session failed to start. | ||
*/ | ||
MSIDErrorInteractiveSessionStartFailure = -51022, | ||
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. what is the case when this would happen? 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. SFAuthenticationSession: when start is called on a cancelled session |
||
|
||
MSIDErrorUnsupportedFunctionality = -51018, | ||
|
||
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. both MSIDErrorInvalidParameter = -51018, and MSIDErrorUnsupportedFunctionality = -51018 are -51018? 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. Let's address errors in a separate PR |
||
MSIDErrorCodeFirst = MSIDErrorInternal, | ||
MSIDErrorCodeLast = MSIDErrorUnsupportedFunctionality | ||
}; | ||
|
||
extern NSError *MSIDCreateError(NSString *domain, NSInteger code, NSString *errorDescription, NSString *oauthError, NSString *subError, NSError *underlyingError, NSUUID *correlationId, NSDictionary *additionalUserInfo); | ||
|
||
extern MSIDErrorCode MSIDErrorCodeForOAuthError(NSString *oauthError, MSIDErrorCode defaultCode); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,6 +23,7 @@ | |
|
||
#import "MSIDConfiguration.h" | ||
#import "NSOrderedSet+MSIDExtensions.h" | ||
#import "MSIDPkce.h" | ||
|
||
@implementation MSIDConfiguration | ||
|
||
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. should line 50 be 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. yeah it should've been exactly that |
||
|
@@ -33,27 +34,15 @@ - (instancetype)copyWithZone:(NSZone*)zone | |
configuration.redirectUri = [_redirectUri copyWithZone:zone]; | ||
configuration.target = [_target copyWithZone:zone]; | ||
configuration.clientId = [_clientId copyWithZone:zone]; | ||
configuration.correlationId = [_correlationId copyWithZone:zone]; | ||
configuration.loginHint = [_loginHint copyWithZone:zone]; | ||
configuration.sliceParameters = [_sliceParameters copyWithZone:zone]; | ||
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. I think we need slice parameters for all requests, both interactive and silent 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. I think it is to be determined whether it will use MSIDConfiguration class or not. |
||
configuration.networkConfig = [_networkConfig copyWithZone:zone]; | ||
|
||
return configuration; | ||
} | ||
|
||
- (instancetype)initWithAuthority:(NSURL *)authority | ||
redirectUri:(NSString *)redirectUri | ||
clientId:(NSString *)clientId | ||
target:(NSString *)target | ||
{ | ||
return [[MSIDConfiguration alloc] initWithAuthority:authority redirectUri:redirectUri clientId:clientId target:target correlationId:nil]; | ||
} | ||
|
||
- (instancetype)initWithAuthority:(NSURL *)authority | ||
redirectUri:(NSString *)redirectUri | ||
clientId:(NSString *)clientId | ||
target:(NSString *)target | ||
correlationId:(NSUUID *)correlationId | ||
{ | ||
self = [super init]; | ||
|
||
|
@@ -63,7 +52,6 @@ - (instancetype)initWithAuthority:(NSURL *)authority | |
_redirectUri = redirectUri; | ||
_clientId = clientId; | ||
_target = target; | ||
_correlationId = correlationId; | ||
} | ||
|
||
return self; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,11 +24,9 @@ | |
#import <Foundation/Foundation.h> | ||
|
||
|
||
@interface MSIDNetworkConfiguration : NSObject <NSCopying> | ||
@interface MSIDNetworkConfiguration : NSObject | ||
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. Should MSIDNetworkConfiguration be singleton? If no, how are we going to access it in the code? 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. Both it's properties are class properties with setters and getters. 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. https://developer.apple.com/videos/play/wwdc2016/405/ at 5:07 for reference. |
||
|
||
@property (readwrite) NSTimeInterval timeout; | ||
@property (readwrite) int retryCount; | ||
|
||
- (instancetype)initWithTimeout:(NSTimeInterval)timeout retryCount:(int)retryCount; | ||
@property (class, nonatomic, readwrite) NSTimeInterval timeout; | ||
@property (class, nonatomic, readwrite) NSInteger retryCount; | ||
|
||
@end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,41 +23,28 @@ | |
|
||
#import "MSIDNetworkConfiguration.h" | ||
|
||
static NSTimeInterval const s_defaultTimeout = 30; | ||
static int const s_defaultRetryCount = 2; | ||
static NSTimeInterval s_timeout = 30; | ||
static NSInteger s_retryCount = 2; | ||
|
||
@implementation MSIDNetworkConfiguration | ||
|
||
- (instancetype)init | ||
+ (void)setTimeout:(NSTimeInterval)timeout | ||
{ | ||
self = [super init]; | ||
if (self) | ||
{ | ||
_timeout = s_defaultTimeout; | ||
_retryCount = s_defaultRetryCount; | ||
} | ||
return self; | ||
s_timeout = timeout; | ||
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. Maybe it is better to have default timeout & retryCount setter & getters and assign default values to them in initializer? In such case you will not need to override setters & getters. 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. these are class properties, which means they don't have default synthesizer. |
||
} | ||
|
||
- (instancetype)initWithTimeout:(NSTimeInterval)timeout retryCount:(int)retryCount | ||
+ (NSTimeInterval)timeout | ||
{ | ||
self = [super init]; | ||
if (self) | ||
{ | ||
_timeout = timeout; | ||
_retryCount = retryCount; | ||
} | ||
return self; | ||
return s_timeout; | ||
} | ||
|
||
- (instancetype)copyWithZone:(NSZone*)zone | ||
+(void)setRetryCount:(NSInteger)retryCount | ||
{ | ||
MSIDNetworkConfiguration *configuration = [[MSIDNetworkConfiguration allocWithZone:zone] init]; | ||
configuration.timeout = _timeout; | ||
configuration.retryCount = _retryCount; | ||
|
||
return configuration; | ||
s_retryCount = retryCount; | ||
} | ||
|
||
+ (NSInteger)retryCount | ||
{ | ||
return s_retryCount; | ||
} | ||
|
||
@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.
just curious, when is this error returned?
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.
When parameter sets a user, and we force webview to login as a different user.
We handled it as an error in MSAL, we can discuss about this.
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: can we put errors in increasing order? (MSIDErrorInvalidState is in the middle)