Skip to content
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.

Sign up for GitHub

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

Jump to bottom

update new var in configuration to allow user by pass URI check #1013

Merged
merged 3 commits into from
Jul 25, 2020
Merged

update new var in configuration to allow user by pass URI check #1013

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b653190
update new var in configuration to allow user by pass URI check
hieunguyenmsft Jul 22, 2020
7dba3e8
address comment
hieunguyenmsft Jul 24, 2020
23ee298
update Changelog
hieunguyenmsft Jul 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
## [TBD] - TBD
* update new variable in configuration to allow user by pass URI check #1013

## [1.1.5] - 2020-06-19

### Added
Expand Down
1 change: 1 addition & 0 deletions MSAL/src/MSALPublicClientApplication.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ - (instancetype)initWithConfiguration:(MSALPublicClientApplicationConfig *)confi
NSError *msidError = nil;
MSALRedirectUri *msalRedirectUri = [MSALRedirectUriVerifier msalRedirectUriWithCustomUri:config.redirectUri
clientId:config.clientId
bypassRedirectValidation:config.bypassRedirectURIValidation
error:&msidError];

if (!msalRedirectUri)
Expand Down
6 changes: 6 additions & 0 deletions MSAL/src/public/configuration/MSALPublicClientApplicationConfig.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,12 @@ to target MSAL at a specific test slice & flight. These apply to all requests ma
*/
- (nonnull instancetype)initWithClientId:(NSString *)clientId;

/**
For client that wants to bypass redirectURI check in MSAL, set this to YES. NO by default.
If set to YES, MSAL will skip the verification of redirectURI. Brokered authentication will be disabled in this case.
*/
@property BOOL bypassRedirectURIValidation;

/**
Initialize a MSALPublicClientApplicationConfig with a given clientId

Expand Down
1 change: 1 addition & 0 deletions MSAL/src/util/MSALRedirectUriVerifier.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@

+ (MSALRedirectUri *)msalRedirectUriWithCustomUri:(NSString *)customRedirectUri
clientId:(NSString *)clientId
bypassRedirectValidation:(BOOL)bypassRedirectValidation
error:(NSError * __autoreleasing *)error;

+ (BOOL)verifyAdditionalRequiredSchemesAreRegistered:(NSError **)error;
Expand Down
7 changes: 4 additions & 3 deletions MSAL/src/util/ios/MSALRedirectUriVerifier.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ @implementation MSALRedirectUriVerifier

+ (MSALRedirectUri *)msalRedirectUriWithCustomUri:(NSString *)customRedirectUri
clientId:(NSString *)clientId
bypassRedirectValidation:(BOOL)bypassRedirectValidation
error:(NSError * __autoreleasing *)error
{
#if AD_BROKER
Expand All @@ -50,13 +51,13 @@ + (MSALRedirectUri *)msalRedirectUriWithCustomUri:(NSString *)customRedirectUri
if (![NSString msidIsStringNilOrBlank:customRedirectUri])
{
NSURL *redirectURI = [NSURL URLWithString:customRedirectUri];

if (![self verifySchemeIsRegistered:redirectURI error:error])
if (!bypassRedirectValidation && ![self verifySchemeIsRegistered:redirectURI error:error])
{
return nil;
}

BOOL brokerCapable = [MSALRedirectUri redirectUriIsBrokerCapable:redirectURI];
BOOL brokerCapable = !bypassRedirectValidation && [MSALRedirectUri redirectUriIsBrokerCapable:redirectURI];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bypassing of Redirect URI validation shouldn't impact whether URL is broker capable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought we designed to forward the request to broker only when the redirectURI is verified? I might misunderstand the purpose of verifying redirectURI in MSAL.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad, I've misread the code :) So this is to ensure that we don't forward request to broker, unless redirect was validated.


MSALRedirectUri *redirectUri = [[MSALRedirectUri alloc] initWithRedirectUri:redirectURI
brokerCapable:brokerCapable];
Expand Down
1 change: 1 addition & 0 deletions MSAL/src/util/mac/MSALRedirectUriVerifier.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ @implementation MSALRedirectUriVerifier

+ (MSALRedirectUri *)msalRedirectUriWithCustomUri:(NSString *)customRedirectUri
clientId:(__unused NSString *)clientId
bypassRedirectValidation:(BOOL)bypassRedirectValidation
error:(__unused NSError * __autoreleasing *)error
{
if (![NSString msidIsStringNilOrBlank:customRedirectUri])
Expand Down
23 changes: 23 additions & 0 deletions MSAL/test/unit/MSALPublicClientApplicationTests.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3112,6 +3112,29 @@ - (void)testSignoutWithAccount_whenNonNilAccount_andSignoutFromBrowserTrue_andBr
[self waitForExpectations:@[expectation] timeout:1];
}

- (void)testInitWithConfiguration_WhenBypassRedirectURIIsDefault_ShouldBlockInvalidURI
{
MSALPublicClientApplicationConfig *pcaConfig = [[MSALPublicClientApplicationConfig alloc] initWithClientId:@"test_client_id"
redirectUri:@"invalid_uri"
authority:nil];
NSError *error;
MSALPublicClientApplication *application = [[MSALPublicClientApplication alloc] initWithConfiguration:pcaConfig error:&error];
XCTAssertTrue(error);
XCTAssertNil(application);
}

- (void)testInitWithConfiguration_WhenBypassRedirectURIIsDYes_ShouldAllowInvalidURI
{
MSALPublicClientApplicationConfig *pcaConfig = [[MSALPublicClientApplicationConfig alloc] initWithClientId:@"test_client_id"
redirectUri:@"invalid_uri"
authority:nil];
pcaConfig.bypassRedirectURIValidation = YES;
NSError *error;
MSALPublicClientApplication *application = [[MSALPublicClientApplication alloc] initWithConfiguration:pcaConfig error:&error];
XCTAssertTrue(application);
XCTAssertNil(error);
}

#endif

#pragma mark - Helpers
Expand Down
34 changes: 27 additions & 7 deletions MSAL/test/unit/ios/MSALRedirectUriVerifierTests.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,10 @@ - (void)testMSALRedirectUri_whenCustomRedirectUri_andNotBrokerCapable_shouldRetu
NSString *clientId = @"msalclient";

NSError *error = nil;
MSALRedirectUri *result = [MSALRedirectUriVerifier msalRedirectUriWithCustomUri:redirectUri clientId:clientId error:&error];
MSALRedirectUri *result = [MSALRedirectUriVerifier msalRedirectUriWithCustomUri:redirectUri
clientId:clientId
bypassRedirectValidation:NO
error:&error];

XCTAssertNotNil(result);
XCTAssertEqualObjects(result.url.absoluteString, redirectUri);
Expand All @@ -65,7 +68,10 @@ - (void)testMSALRedirectUri_whenCustomRedirectUri_andBrokerCapable_shouldReturnU
NSString *clientId = @"msalclient";

NSError *error = nil;
MSALRedirectUri *result = [MSALRedirectUriVerifier msalRedirectUriWithCustomUri:redirectUri clientId:clientId error:&error];
MSALRedirectUri *result = [MSALRedirectUriVerifier msalRedirectUriWithCustomUri:redirectUri
clientId:clientId
bypassRedirectValidation:NO
error:&error];

XCTAssertNotNil(result);
XCTAssertEqualObjects(result.url.absoluteString, redirectUri);
Expand All @@ -83,7 +89,10 @@ - (void)testMSALRedirectUri_whenCustomRedirectUri_andLegacyBrokerCapable_shouldR
NSString *clientId = @"msalclient";

NSError *error = nil;
MSALRedirectUri *result = [MSALRedirectUriVerifier msalRedirectUriWithCustomUri:redirectUri clientId:clientId error:&error];
MSALRedirectUri *result = [MSALRedirectUriVerifier msalRedirectUriWithCustomUri:redirectUri
clientId:clientId
bypassRedirectValidation:NO
error:&error];

XCTAssertNotNil(result);
XCTAssertEqualObjects(result.url.absoluteString, redirectUri);
Expand All @@ -101,7 +110,10 @@ - (void)testMSALRedirectUri_whenCustomRedirectUri_andNotRegistered_shouldReturnN
NSString *clientId = @"msalclient";

NSError *error = nil;
MSALRedirectUri *result = [MSALRedirectUriVerifier msalRedirectUriWithCustomUri:redirectUri clientId:clientId error:&error];
MSALRedirectUri *result = [MSALRedirectUriVerifier msalRedirectUriWithCustomUri:redirectUri
clientId:clientId
bypassRedirectValidation:NO
error:&error];

XCTAssertNil(result);
XCTAssertNotNil(error);
Expand All @@ -117,7 +129,10 @@ - (void)testMSALRedirectUri_whenDefaultRedirectUri_andBrokerCapableUrlRegistered
NSString *clientId = @"msalclient";

NSError *error = nil;
MSALRedirectUri *result = [MSALRedirectUriVerifier msalRedirectUriWithCustomUri:nil clientId:clientId error:&error];
MSALRedirectUri *result = [MSALRedirectUriVerifier msalRedirectUriWithCustomUri:nil
clientId:clientId
bypassRedirectValidation:NO
error:&error];

XCTAssertNotNil(result);
XCTAssertEqualObjects(result.url.absoluteString, @"msauth.test.bundle.identifier://auth");
Expand All @@ -134,7 +149,9 @@ - (void)testMSALRedirectUri_whenDefaultRedirectUri_andDefaultUrlRegistered_shoul
NSString *clientId = @"msalclient";

NSError *error = nil;
MSALRedirectUri *result = [MSALRedirectUriVerifier msalRedirectUriWithCustomUri:nil clientId:clientId error:&error];
MSALRedirectUri *result = [MSALRedirectUriVerifier msalRedirectUriWithCustomUri:nil
clientId:clientId
bypassRedirectValidation:NO error:&error];

XCTAssertNotNil(result);
XCTAssertEqualObjects(result.url.absoluteString, @"msalmsalclient://auth");
Expand All @@ -150,7 +167,10 @@ - (void)testMSALRedirectUri_whenNoRedirectUriRegistered_shouldReturnNilAndFillEr
NSString *clientId = @"msalclient";
NSError *error = nil;

MSALRedirectUri *result = [MSALRedirectUriVerifier msalRedirectUriWithCustomUri:nil clientId:clientId error:&error];
MSALRedirectUri *result = [MSALRedirectUriVerifier msalRedirectUriWithCustomUri:nil
clientId:clientId
bypassRedirectValidation:NO
error:&error];

XCTAssertNil(result);
XCTAssertNotNil(error);
Expand Down