Skip to content

Commit 5f4bc1a

Browse files
kaisong1990kaisong1990
authored
Merge pull request #1279 from AzureAD/master
Master
2 parents 811c772 + 0b099ad commit 5f4bc1a

File tree

6 files changed

+89
-14
lines changed

6 files changed

+89
-14
lines changed

‎IdentityCore/src/util/MSIDRedirectUri.h‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,20 @@
2525

2626
#import <Foundation/Foundation.h>
2727

28+
typedef NS_ENUM(NSInteger, MSIDRedirectUriValidationResult)
29+
{
30+
MSIDRedirectUriValidationResultMatched = 0,
31+
MSIDRedirectUriValidationResultNilOrEmpty,
32+
MSIDRedirectUriValidationResultSchemeNilOrEmpty,
33+
MSIDRedirectUriValidationResultHostNilOrEmpty,
34+
MSIDRedirectUriValidationResultHttpFormatNotSupport,
35+
MSIDRedirectUriValidationResultMSALFormatBundleIdMismatched,
36+
MSIDRedirectUriValidationResultMSALFormatHostNilOrEmpty,
37+
MSIDRedirectUriValidationResultoauth20FormatNotSupport,
38+
MSIDRedirectUriValidationResultUnknownNotMatched
39+
};
40+
41+
2842
NS_ASSUME_NONNULL_BEGIN
2943

3044
/**
@@ -54,7 +68,7 @@ NS_ASSUME_NONNULL_BEGIN
5468

5569
+ (nullable NSURL *)defaultBrokerCapableRedirectUri;
5670

57-
+ (BOOL)redirectUriIsBrokerCapable:(NSURL *)redirectUri;
71+
+ (MSIDRedirectUriValidationResult)redirectUriIsBrokerCapable:(NSURL *)redirectUri;
5872

5973
@end
6074

‎IdentityCore/src/util/MSIDRedirectUri.m‎

Lines changed: 44 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,24 +70,62 @@ + (NSURL *)defaultBrokerCapableRedirectUri
7070
return [NSURL URLWithString:redirectUri];
7171
}
7272

73-
+ (BOOL)redirectUriIsBrokerCapable:(NSURL *)redirectUri
73+
+ (MSIDRedirectUriValidationResult)redirectUriIsBrokerCapable:(NSURL *)redirectUri
7474
{
75+
if ([NSString msidIsStringNilOrBlank:redirectUri.absoluteString])
76+
{
77+
MSID_LOG_WITH_CTX(MSIDLogLevelVerbose, nil, @"MSIDRedirectUri validation: redirect_uri is nil or empty");
78+
return MSIDRedirectUriValidationResultNilOrEmpty;
79+
}
80+
7581
NSURL *defaultRedirectUri = [MSIDRedirectUri defaultBrokerCapableRedirectUri];
76-
82+
7783
// Check default MSAL format
7884
if ([defaultRedirectUri isEqual:redirectUri])
7985
{
80-
return YES;
86+
return MSIDRedirectUriValidationResultMatched;
8187
}
82-
88+
8389
// Check default ADAL format
8490
if ([redirectUri.host isEqualToString:[[NSBundle mainBundle] bundleIdentifier]]
8591
&& redirectUri.scheme.length > 0)
8692
{
87-
return YES;
93+
return MSIDRedirectUriValidationResultMatched;
94+
}
95+
96+
// Add extra validation on why redirect_uri is not capable
97+
if ([redirectUri.scheme isEqualToString:@"http"] || [redirectUri.scheme isEqualToString:@"https"])
98+
{
99+
MSID_LOG_WITH_CTX(MSIDLogLevelVerbose, nil, @"MSIDRedirectUri validation: redirect_uri is (http(s)://host), and is not supported");
100+
return MSIDRedirectUriValidationResultHttpFormatNotSupport;
101+
}
102+
else if ([redirectUri.host isEqualToString:@"auth"] && [redirectUri.absoluteString hasPrefix:@"msauth"])
103+
{
104+
MSID_LOG_WITH_CTX(MSIDLogLevelVerbose, nil, @"MSIDRedirectUri validation: redirect_uri is MSAL format, but bundle_id could mismatch");
105+
return MSIDRedirectUriValidationResultMSALFormatBundleIdMismatched;
106+
}
107+
else if ([redirectUri.absoluteString hasPrefix:@"msauth"])
108+
{
109+
MSID_LOG_WITH_CTX(MSIDLogLevelVerbose, nil, @"MSIDRedirectUri validation: redirect_uri is as (msauth.bundle_id), and auth host is missing");
110+
return MSIDRedirectUriValidationResultMSALFormatHostNilOrEmpty;
111+
}
112+
else if ([NSString msidIsStringNilOrBlank:redirectUri.scheme])
113+
{
114+
MSID_LOG_WITH_CTX(MSIDLogLevelVerbose, nil, @"MSIDRedirectUri validation: redirect_uri is as (://host) without schema");
115+
return MSIDRedirectUriValidationResultSchemeNilOrEmpty;
116+
}
117+
else if ([NSString msidIsStringNilOrBlank:redirectUri.host])
118+
{
119+
MSID_LOG_WITH_CTX(MSIDLogLevelVerbose, nil, @"MSIDRedirectUri validation: redirect_uri is as (scheme://) without host");
120+
return MSIDRedirectUriValidationResultHostNilOrEmpty;
121+
}
122+
else if ([redirectUri.absoluteString isEqualToString:@"urn:ietf:wg:oauth:2.0:oob"])
123+
{
124+
MSID_LOG_WITH_CTX(MSIDLogLevelVerbose, nil, @"MSIDRedirectUri validation: redirect_uri is urn:ietf:wg:oauth:2.0:oob, and not supported");
125+
return MSIDRedirectUriValidationResultoauth20FormatNotSupport;
88126
}
89127

90-
return NO;
128+
return MSIDRedirectUriValidationResultUnknownNotMatched;
91129
}
92130

93131
@end

‎IdentityCore/src/util/ios/MSIDRedirectUriVerifier.m‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ + (MSIDRedirectUri *)msidRedirectUriWithCustomUri:(NSString *)customRedirectUri
5858
return nil;
5959
}
6060

61-
BOOL brokerCapable = !bypassRedirectValidation && [MSIDRedirectUri redirectUriIsBrokerCapable:redirectURI];
61+
BOOL brokerCapable = !bypassRedirectValidation && [MSIDRedirectUri redirectUriIsBrokerCapable:redirectURI] == MSIDRedirectUriValidationResultMatched;
6262

6363
MSIDRedirectUri *redirectUri = [[MSIDRedirectUri alloc] initWithRedirectUri:redirectURI
6464
brokerCapable:brokerCapable];

‎IdentityCore/src/util/mac/MSIDRedirectUriVerifier.m‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ + (MSIDRedirectUri *)msidRedirectUriWithCustomUri:(NSString *)customRedirectUri
4242
return nil;
4343
}
4444

45-
BOOL isBrokerCapable = [MSIDRedirectUri redirectUriIsBrokerCapable:customRedirectURL] || bypassRedirectValidation;
45+
BOOL isBrokerCapable = [MSIDRedirectUri redirectUriIsBrokerCapable:customRedirectURL] == MSIDRedirectUriValidationResultMatched || bypassRedirectValidation;
4646
return [[MSIDRedirectUri alloc] initWithRedirectUri:customRedirectURL
4747
brokerCapable:isBrokerCapable];
4848
}

‎IdentityCore/tests/MSIDBrokerRedirectUriTest.m‎

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,14 @@ - (void)test_get_default_broker_capable_redirectUri
5151
XCTAssertNotNil(redirectUri);
5252
}
5353

54-
- (void)test_redirectUri_is_broker_capable_with_invalid_url
54+
- (void)test_check_empty_redirectUri
5555
{
56-
XCTAssertFalse([MSIDRedirectUri redirectUriIsBrokerCapable:[NSURL URLWithString:@"https://fakeurl.contoso.com"]]);
56+
XCTAssertTrue([MSIDRedirectUri redirectUriIsBrokerCapable:[NSURL URLWithString:@""]] == MSIDRedirectUriValidationResultNilOrEmpty);
57+
}
58+
59+
- (void)test_redirectUri_is_broker_capable_with_https_url
60+
{
61+
XCTAssertTrue([MSIDRedirectUri redirectUriIsBrokerCapable:[NSURL URLWithString:@"https://fakeurl.contoso.com"]] == MSIDRedirectUriValidationResultHttpFormatNotSupport);
5762
}
5863

5964
- (void)test_check_default_redirect_msal_format
@@ -64,7 +69,7 @@ - (void)test_check_default_redirect_msal_format
6469
#else
6570
url = [NSURL URLWithString:@"msauth.com.apple.dt.xctest.tool://auth"];
6671
#endif
67-
XCTAssertTrue([MSIDRedirectUri redirectUriIsBrokerCapable:url]);
72+
XCTAssertTrue([MSIDRedirectUri redirectUriIsBrokerCapable:url] == MSIDRedirectUriValidationResultMatched);
6873

6974
}
7075

@@ -76,7 +81,7 @@ - (void)test_check_default_redirect_adal_format
7681
#else
7782
url = [NSURL URLWithString:@"myscheme://com.apple.dt.xctest.tool"];
7883
#endif
79-
XCTAssertTrue([MSIDRedirectUri redirectUriIsBrokerCapable:url]);
84+
XCTAssertTrue([MSIDRedirectUri redirectUriIsBrokerCapable:url] == MSIDRedirectUriValidationResultMatched);
8085

8186
}
8287

@@ -88,8 +93,23 @@ - (void)test_check_default_redirect_adal_format_without_scheme
8893
#else
8994
url = [NSURL URLWithString:@"com.apple.dt.xctest.tool"];
9095
#endif
91-
XCTAssertFalse([MSIDRedirectUri redirectUriIsBrokerCapable:url]);
96+
XCTAssertTrue([MSIDRedirectUri redirectUriIsBrokerCapable:url] == MSIDRedirectUriValidationResultSchemeNilOrEmpty);
97+
98+
}
99+
100+
- (void)test_checkRedirect_uri_miss_host
101+
{
102+
XCTAssertTrue([MSIDRedirectUri redirectUriIsBrokerCapable:[NSURL URLWithString:@"myscheme://"]] == MSIDRedirectUriValidationResultHostNilOrEmpty);
103+
}
104+
105+
- (void)test_checkRedirect_uri_msal_format_miss_host
106+
{
107+
XCTAssertTrue([MSIDRedirectUri redirectUriIsBrokerCapable:[NSURL URLWithString:@"msauth.com.microsoft.MSIDTestsHostApp://"]] == MSIDRedirectUriValidationResultMSALFormatHostNilOrEmpty);
108+
}
92109

110+
- (void)test_checkRedirect_uri_msal_format_miss_scheme
111+
{
112+
XCTAssertTrue([MSIDRedirectUri redirectUriIsBrokerCapable:[NSURL URLWithString:@"://auth"]] == MSIDRedirectUriValidationResultSchemeNilOrEmpty);
93113
}
94114

95115
@end

‎changelog.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
Version 1.7.25
2+
* Update broker redirect_uri validation with more information in invalid scenario (#1264)
3+
14
Version 1.7.24
25
* Added method name with line number for errors in telemetry (#1266)
36

0 commit comments

Comments
 (0)