Skip to content

Commit 650f8ec

Browse files
authored
Merge pull request #2590 from AzureAD/swagup/valid_parentController
Ensuing valid parantViewController is required for MSAL Interactive APIs
2 parents ab60498 + 152db72 commit 650f8ec

File tree

3 files changed

+149
-9
lines changed

3 files changed

+149
-9
lines changed

MSAL/src/MSIDInteractiveRequestParameters+MSALRequest.m

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,18 @@ - (BOOL)fillWithWebViewParameters:(MSALWebviewParameters *)webParameters
3737
customWebView:(WKWebView *)customWebView
3838
error:(NSError **)error
3939
{
40+
if (webParameters == nil)
41+
{
42+
NSError *msidError = MSIDCreateError(MSIDErrorDomain, MSIDErrorInvalidDeveloperParameter, @"webviewParameters is a required parameter.", nil, nil, nil, nil, nil, YES);
43+
if (error) *error = msidError;
44+
return NO;
45+
}
46+
4047
__typeof__(webParameters.parentViewController) parentViewController = webParameters.parentViewController;
4148

42-
#if TARGET_OS_IPHONE
4349
if (parentViewController == nil)
4450
{
45-
NSError *msidError = MSIDCreateError(MSIDErrorDomain, MSIDErrorInvalidDeveloperParameter, @"parentViewController is a required parameter on iOS.", nil, nil, nil, nil, nil, YES);
51+
NSError *msidError = MSIDCreateError(MSIDErrorDomain, MSIDErrorInvalidDeveloperParameter, @"parentViewController is a required parameter.", nil, nil, nil, nil, nil, YES);
4652
if (error) *error = msidError;
4753
return NO;
4854
}
@@ -54,9 +60,10 @@ - (BOOL)fillWithWebViewParameters:(MSALWebviewParameters *)webParameters
5460
return NO;
5561
}
5662

63+
#if TARGET_OS_IPHONE
5764
self.presentationType = webParameters.presentationStyle;
5865
#endif
59-
66+
6067
self.parentViewController = parentViewController;
6168

6269
self.prefersEphemeralWebBrowserSession = webParameters.prefersEphemeralWebBrowserSession;

MSAL/src/public/MSALWebviewParameters.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,10 @@ NS_ASSUME_NONNULL_BEGIN
4444
#pragma mark - Configuration options
4545

4646
/**
47-
The view controller to present from. If nil, MSAL will attempt to still proceed with the authentication, but the results will be unexpected.
48-
It is strongly recommended to provide a non-null parentViewController to avoid unexpected results.
47+
The view controller to present from. This property must be valid. If nil is provided, or if the view controller is not attached to a window (i.e., parentViewController.view.window is nil), MSAL will return an error and will not proceed with authentication.
48+
It is required to provide a valid parentViewController with a window to proceed with authentication.
4949
*/
50-
@property (nullable, weak, nonatomic) MSALViewController *parentViewController;
50+
@property (nonatomic, strong, nonnull) MSALViewController *parentViewController;
5151

5252
#if TARGET_OS_IPHONE
5353

@@ -83,10 +83,10 @@ NS_ASSUME_NONNULL_BEGIN
8383

8484
/**
8585
Creates an instance of MSALWebviewParameters with a provided parentViewController.
86-
@param parentViewController The view controller to present authorization UI from.
87-
@note parentViewController is mandatory on iOS 13+ and macOS 10.15+. Your app will experience unexpected results if parentViewController is not provided.
86+
@param parentViewController The view controller to present authorization UI from. This property must be valid
87+
@note parentViewController is mandatory on iOS 13+ and macOS 10.15+. If nil is provided, or if the view controller is not attached to a window (i.e., parentViewController.view.window is nil), MSAL will return an error and authentication will not proceed. It is required to provide a valid parentViewController with a window to proceed with authentication.
8888
*/
89-
- (nonnull instancetype)initWithAuthPresentationViewController:(MSALViewController *)parentViewController;
89+
- (nonnull instancetype)initWithAuthPresentationViewController:(nonnull MSALViewController *)parentViewController;
9090

9191

9292
/**

MSAL/test/unit/MSALPublicClientApplicationTests.m

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -508,6 +508,139 @@ - (void)testAcquireTokenScopes
508508
application = nil;
509509
}
510510

511+
- (void)testAcquireTokenScopes_WithNilParentViewController_shouldReturnError
512+
{
513+
__auto_type authority = [@"https://login.microsoftonline.com/common" msalAuthority];
514+
515+
MSALPublicClientApplicationConfig *config = [[MSALPublicClientApplicationConfig alloc] initWithClientId:UNIT_TEST_CLIENT_ID redirectUri:nil authority:authority];
516+
config.sliceConfig = [MSALSliceConfig configWithSlice:@"slice" dc:@"dc"];
517+
518+
NSError *error = nil;
519+
__auto_type application = [[MSALPublicClientApplication alloc] initWithConfiguration:config
520+
error:&error];
521+
XCTAssertNotNil(application);
522+
XCTAssertNil(error);
523+
524+
__block dispatch_semaphore_t dsem = dispatch_semaphore_create(0);
525+
526+
[MSIDTestSwizzle instanceMethod:@selector(acquireToken:)
527+
class:[MSIDLocalInteractiveController class]
528+
block:(id)^(MSIDLocalInteractiveController *obj, MSIDRequestCompletionBlock completionBlock)
529+
{
530+
XCTAssertTrue([obj isKindOfClass:[MSIDLocalInteractiveController class]]);
531+
completionBlock(nil, nil);
532+
}];
533+
534+
MSALViewController *parentViewController = nil;
535+
MSALWebviewParameters *webParameters = [[MSALWebviewParameters alloc] initWithAuthPresentationViewController:parentViewController];
536+
MSALInteractiveTokenParameters *parameters = [[MSALInteractiveTokenParameters alloc] initWithScopes:@[@"fakescope"]
537+
webviewParameters:webParameters];
538+
539+
[application acquireTokenWithParameters:parameters
540+
completionBlock:^(MSALResult *result, NSError *error)
541+
{
542+
XCTAssertNil(result);
543+
XCTAssertNotNil(error);
544+
545+
dispatch_semaphore_signal(dsem);
546+
}];
547+
548+
dispatch_semaphore_wait(dsem, DISPATCH_TIME_NOW);
549+
application = nil;
550+
}
551+
552+
- (void)testAcquireTokenScopes_WithInvalidParentViewController_shouldReturnError
553+
{
554+
__auto_type authority = [@"https://login.microsoftonline.com/common" msalAuthority];
555+
556+
MSALPublicClientApplicationConfig *config = [[MSALPublicClientApplicationConfig alloc] initWithClientId:UNIT_TEST_CLIENT_ID redirectUri:nil authority:authority];
557+
config.sliceConfig = [MSALSliceConfig configWithSlice:@"slice" dc:@"dc"];
558+
559+
NSError *error = nil;
560+
__auto_type application = [[MSALPublicClientApplication alloc] initWithConfiguration:config
561+
error:&error];
562+
XCTAssertNotNil(application);
563+
XCTAssertNil(error);
564+
565+
__block dispatch_semaphore_t dsem = dispatch_semaphore_create(0);
566+
567+
[MSIDTestSwizzle instanceMethod:@selector(acquireToken:)
568+
class:[MSIDLocalInteractiveController class]
569+
block:(id)^(MSIDLocalInteractiveController *obj, MSIDRequestCompletionBlock completionBlock)
570+
{
571+
XCTAssertTrue([obj isKindOfClass:[MSIDLocalInteractiveController class]]);
572+
completionBlock(nil, nil);
573+
}];
574+
575+
static dispatch_once_t once;
576+
static MSALViewController *controller;
577+
578+
#if TARGET_OS_IPHONE
579+
dispatch_once(&once, ^{
580+
controller = [UIViewController new];
581+
});
582+
#else
583+
dispatch_once(&once, ^{
584+
controller = [NSViewController new];
585+
});
586+
#endif
587+
588+
MSALWebviewParameters *webParameters = [[MSALWebviewParameters alloc] initWithAuthPresentationViewController:controller];
589+
MSALInteractiveTokenParameters *parameters = [[MSALInteractiveTokenParameters alloc] initWithScopes:@[@"fakescope"]
590+
webviewParameters:webParameters];
591+
592+
[application acquireTokenWithParameters:parameters
593+
completionBlock:^(MSALResult *result, NSError *error)
594+
{
595+
XCTAssertNil(result);
596+
XCTAssertNotNil(error);
597+
598+
dispatch_semaphore_signal(dsem);
599+
}];
600+
601+
dispatch_semaphore_wait(dsem, DISPATCH_TIME_NOW);
602+
application = nil;
603+
}
604+
605+
- (void)testAcquireTokenScopes_WithNilWebviewParameters_shouldReturnError
606+
{
607+
__auto_type authority = [@"https://login.microsoftonline.com/common" msalAuthority];
608+
609+
MSALPublicClientApplicationConfig *config = [[MSALPublicClientApplicationConfig alloc] initWithClientId:UNIT_TEST_CLIENT_ID redirectUri:nil authority:authority];
610+
config.sliceConfig = [MSALSliceConfig configWithSlice:@"slice" dc:@"dc"];
611+
612+
NSError *error = nil;
613+
__auto_type application = [[MSALPublicClientApplication alloc] initWithConfiguration:config
614+
error:&error];
615+
XCTAssertNotNil(application);
616+
XCTAssertNil(error);
617+
618+
__block dispatch_semaphore_t dsem = dispatch_semaphore_create(0);
619+
620+
[MSIDTestSwizzle instanceMethod:@selector(acquireToken:)
621+
class:[MSIDLocalInteractiveController class]
622+
block:(id)^(MSIDLocalInteractiveController *obj, MSIDRequestCompletionBlock completionBlock)
623+
{
624+
XCTAssertTrue([obj isKindOfClass:[MSIDLocalInteractiveController class]]);
625+
completionBlock(nil, nil);
626+
}];
627+
628+
MSALWebviewParameters *webParameters = nil;
629+
MSALInteractiveTokenParameters *parameters = [[MSALInteractiveTokenParameters alloc] initWithScopes:@[@"fakescope"]
630+
webviewParameters:webParameters];
631+
632+
[application acquireTokenWithParameters:parameters
633+
completionBlock:^(MSALResult *result, NSError *error)
634+
{
635+
XCTAssertNil(result);
636+
XCTAssertNotNil(error);
637+
638+
dispatch_semaphore_signal(dsem);
639+
}];
640+
641+
dispatch_semaphore_wait(dsem, DISPATCH_TIME_NOW);
642+
application = nil;
643+
}
511644
#pragma mark - Known authorities
512645

513646
- (void)testAcquireToken_whenKnownAADAuthority_shouldNotForceValidation

0 commit comments

Comments
 (0)