Skip to content

Commit cf9450c

Browse files
juan-ariasjuan-arias
authored
Merge pull request #1545 from AzureAD/release/1.14.0
Release/1.14.0
2 parents b2e54cf + f77a4ce commit cf9450c

14 files changed

+168
-29
lines changed

‎IdentityCore/src/MSIDConstants.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,7 @@ extern NSString * _Nonnull const MSID_FLIGHT_SUPPORT_DUNA_CBA;
216216
extern NSString * _Nonnull const MSID_FLIGHT_DISABLE_JIT_TROUBLESHOOTING_LEGACY_AUTH;
217217
extern NSString * _Nonnull const MSID_FLIGHT_CLIENT_SFRT_STATUS;
218218
extern NSString * _Nonnull const MSID_FLIGHT_DISABLE_PREFERRED_IDENTITY_CBA;
219+
extern NSString * _Nonnull const MSID_FLIGHT_SUPPORT_STATE_DUNA_CBA;
219220

220221
/**
221222
* Flight to indicate if remove account artifacts should be disabled

‎IdentityCore/src/MSIDConstants.m‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@
8888
NSString *const MSID_FLIGHT_DISABLE_JIT_TROUBLESHOOTING_LEGACY_AUTH = @"disable_jit_remediation_legacy_auth";
8989
NSString *const MSID_FLIGHT_CLIENT_SFRT_STATUS = @"sfrt_v2";
9090
NSString *const MSID_FLIGHT_DISABLE_PREFERRED_IDENTITY_CBA = @"dis_pre_iden_cba";
91+
NSString *const MSID_FLIGHT_SUPPORT_STATE_DUNA_CBA = @"support_state_duna_cba";
9192

9293
// Making the flight string short to avoid legacy broker url size limit
9394
NSString *const MSID_FLIGHT_DISABLE_REMOVE_ACCOUNT_ARTIFACTS = @"disable_rm_metadata";

‎IdentityCore/src/oauth2/aad_base/MSIDAADWebviewFactory.m‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,12 +206,14 @@ - (MSIDWebviewResponse *)oAuthResponseWithURL:(NSURL *)url
206206
{
207207
MSIDSwitchBrowserResponse *switchBrowserResponse = [[MSIDSwitchBrowserResponse alloc] initWithURL:url
208208
redirectUri:endRedirectUri
209+
requestState:requestState
209210
context:context
210211
error:nil];
211212
if (switchBrowserResponse) return switchBrowserResponse;
212213

213214
MSIDSwitchBrowserResumeResponse *switchBrowserResumeResponse = [[MSIDSwitchBrowserResumeResponse alloc] initWithURL:url
214215
redirectUri:endRedirectUri
216+
requestState:requestState
215217
context:context
216218
error:nil];
217219
if (switchBrowserResumeResponse) return switchBrowserResumeResponse;

‎IdentityCore/src/requests/MSIDEcdhApv.m‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,13 @@
2727

2828
@implementation MSIDEcdhApv
2929

30-
const NSUInteger kExpectedECP256KeyLength = 65;
31-
3230
- (instancetype)initWithKey:(SecKeyRef)publicKey
3331
apvPrefix:(NSString *)apvPrefix
3432
context:(id<MSIDRequestContext> _Nullable)context
3533
error:(NSError * _Nullable __autoreleasing *)error
3634
{
35+
const NSUInteger kExpectedECP256KeyLength = 65;
36+
3737
if (publicKey == NULL)
3838
{
3939
if (error) *error = MSIDCreateError(MSIDErrorDomain, MSIDErrorInternal, @"Public STK provided is not defined.", nil, nil, nil, context.correlationId, nil, NO);

‎IdentityCore/src/webview/operations/ios/MSIDSwitchBrowserOperation.m‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@
3232
#import "MSIDWebviewAuthorization.h"
3333
#import "MSIDCertAuthManager.h"
3434
#import "MSIDInteractiveTokenRequestParameters.h"
35+
#import "MSIDConstants.h"
36+
#import "MSIDFlightManager.h"
3537

3638
@interface MSIDSwitchBrowserOperation()
3739

@@ -82,6 +84,12 @@ - (void)invokeWithRequestParameters:(nonnull MSIDInteractiveTokenRequestParamete
8284
NSMutableDictionary *queryItems = [NSMutableDictionary new];
8385
queryItems[@"code"] = self.switchBrowserResponse.switchBrowserSessionToken;
8486
queryItems[MSID_OAUTH2_REDIRECT_URI] = requestParameters.redirectUri;
87+
88+
if ([MSIDFlightManager.sharedInstance boolForKey:MSID_FLIGHT_SUPPORT_STATE_DUNA_CBA] && self.switchBrowserResponse.state)
89+
{
90+
queryItems[MSID_OAUTH2_STATE] = self.switchBrowserResponse.state;
91+
}
92+
8593
NSURLComponents *requestURLComponents = [[NSURLComponents alloc] initWithString:self.switchBrowserResponse.actionUri];
8694
requestURLComponents.percentEncodedQuery = [queryItems msidURLEncode];
8795
NSURL *startURL = requestURLComponents.URL;

‎IdentityCore/src/webview/operations/ios/MSIDSwitchBrowserResumeOperation.m‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828
#import "MSIDWebviewFactory.h"
2929
#import "MSIDInteractiveTokenRequestParameters.h"
3030
#import "MSIDWebResponseOperationFactory.h"
31+
#import "MSIDConstants.h"
32+
#import "MSIDFlightManager.h"
3133

3234
@interface MSIDSwitchBrowserResumeOperation()
3335

@@ -87,6 +89,22 @@ - (void)invokeWithRequestParameters:(nonnull MSIDInteractiveTokenRequestParamete
8789
webviewResponseCompletionBlock:(nonnull MSIDWebviewAuthCompletionHandler)webviewResponseCompletionBlock
8890
authorizationCodeCompletionBlock:(nonnull MSIDInteractiveAuthorizationCodeCompletionBlock)authorizationCodeCompletionBlock
8991
{
92+
if ([MSIDFlightManager.sharedInstance boolForKey:MSID_FLIGHT_SUPPORT_STATE_DUNA_CBA])
93+
{
94+
MSIDSwitchBrowserResponse *parentResponse = (MSIDSwitchBrowserResponse *)self.switchBrowserResumeResponse.parentResponse;
95+
NSError *stateValidationError = nil;
96+
97+
BOOL stateValidated = [MSIDSwitchBrowserResponse validateStateParameter:self.switchBrowserResumeResponse.state
98+
expectedState:parentResponse.state.msidBase64UrlDecode
99+
error:&stateValidationError];
100+
if (!stateValidated)
101+
{
102+
MSID_LOG_WITH_CTX(MSIDLogLevelError, requestParameters, @"Resume operation rejected due to state validation failure");
103+
if (webviewResponseCompletionBlock) webviewResponseCompletionBlock(nil, stateValidationError);
104+
return;
105+
}
106+
}
107+
90108
webRequestConfiguration.startURL = [[NSURL alloc] initWithString:self.switchBrowserResumeResponse.actionUri];
91109
NSMutableDictionary *customHeaders = [webRequestConfiguration.customHeaders mutableCopy] ?: [NSMutableDictionary new];
92110
customHeaders[@"Authorization"] = [NSString stringWithFormat:@"Bearer %@", self.switchBrowserResumeResponse.switchBrowserSessionToken];

‎IdentityCore/src/webview/response/MSIDSwitchBrowserResponse.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ typedef NS_OPTIONS(NSInteger, MSIDSwitchBrowserModes) {
3535
@property (nonatomic, readonly) NSString *actionUri;
3636
@property (nonatomic, readonly) NSString *switchBrowserSessionToken;
3737
@property (nonatomic, readonly) BOOL useEphemeralWebBrowserSession;
38+
@property (nonatomic, readonly) NSString *state;
3839

3940
- (instancetype )init NS_UNAVAILABLE;
4041
+ (instancetype)new NS_UNAVAILABLE;
@@ -45,9 +46,13 @@ typedef NS_OPTIONS(NSInteger, MSIDSwitchBrowserModes) {
4546

4647
- (instancetype)initWithURL:(NSURL *)url
4748
redirectUri:(NSString *)redirectUri
49+
requestState:(NSString *)requestState
4850
context:(id<MSIDRequestContext>)context
4951
error:(NSError *__autoreleasing*)error;
5052

5153
+ (BOOL)isDUNAActionUrl:(NSURL *)url operation:(NSString *)operation;
5254

55+
+ (BOOL)validateStateParameter:(NSString *)receivedState
56+
expectedState:(NSString *)expectedState
57+
error:(NSError *__autoreleasing*)error;
5358
@end

‎IdentityCore/src/webview/response/MSIDSwitchBrowserResponse.m‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ + (NSString *)operation
3838

3939
- (instancetype)initWithURL:(NSURL *)url
4040
redirectUri:(NSString *)redirectUri
41+
requestState:(NSString *)requestState
4142
context:(id<MSIDRequestContext>)context
4243
error:(NSError *__autoreleasing*)error
4344
{
@@ -48,8 +49,26 @@ - (instancetype)initWithURL:(NSURL *)url
4849
if (self)
4950
{
5051
if (![self isMyUrl:url redirectUri:redirectUri]) return nil;
52+
53+
if ([MSIDFlightManager.sharedInstance boolForKey:MSID_FLIGHT_SUPPORT_STATE_DUNA_CBA])
54+
{
55+
NSError *stateCheckError = nil;
56+
BOOL stateValidated = [MSIDSwitchBrowserResponse validateStateParameter:self.parameters[MSID_OAUTH2_STATE]
57+
expectedState:requestState
58+
error:&stateCheckError];
59+
if (!stateValidated)
60+
{
61+
if (stateCheckError && error)
62+
{
63+
*error = stateCheckError;
64+
}
65+
return nil;
66+
}
67+
}
68+
5169
_actionUri = self.parameters[@"action_uri"];
5270
_useEphemeralWebBrowserSession = YES;
71+
_state = self.parameters[MSID_OAUTH2_STATE];
5372

5473
NSString* browserOptionsString = self.parameters[@"browser_modes"];
5574
if (browserOptionsString)
@@ -105,6 +124,45 @@ + (BOOL)isDUNAActionUrl:(NSURL *)url operation:(NSString *)operation
105124
return NO;
106125
}
107126

127+
+ (BOOL)validateStateParameter:(NSString *)receivedState
128+
expectedState:(NSString *)expectedState
129+
error:(NSError *__autoreleasing*)error
130+
{
131+
if (!receivedState && !expectedState)
132+
{
133+
return YES;
134+
}
135+
136+
if (!expectedState || !receivedState)
137+
{
138+
if (error)
139+
{
140+
*error = MSIDCreateError(MSIDOAuthErrorDomain,
141+
MSIDErrorServerInvalidState,
142+
[NSString stringWithFormat:@"Missing or invalid state returned state: %@", receivedState],
143+
nil, nil, nil, nil, nil, YES);
144+
}
145+
return NO;
146+
}
147+
148+
BOOL result = [receivedState.msidBase64UrlDecode isEqualToString:expectedState];
149+
150+
if (!result)
151+
{
152+
MSID_LOG_WITH_CTX(MSIDLogLevelError, nil, @"State parameter mismatch");
153+
if (error)
154+
{
155+
*error = MSIDCreateError(MSIDOAuthErrorDomain,
156+
MSIDErrorServerInvalidState,
157+
[NSString stringWithFormat:@"State parameter mismatch. Expected: %@, Received: %@", expectedState, receivedState],
158+
nil, nil, nil, nil, nil, YES);
159+
}
160+
return NO;
161+
}
162+
163+
return YES;
164+
}
165+
108166
#pragma mark - Private
109167

110168
- (BOOL)isMyUrl:(NSURL *)url

‎IdentityCore/tests/IdentityCoreTests-Bridging-Header.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,3 +20,5 @@
2020
#import "MSIDWebWPJResponse.h"
2121
#import "MSIDWebviewFactory.h"
2222
#import "MSIDSwitchBrowserResumeOperation.h"
23+
#import "MSIDFlightManagerMockProvider.h"
24+
#import "MSIDFlightManager.h"

‎IdentityCore/tests/MSIDSwitchBrowserOperationTest.swift‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,13 +78,16 @@ class MSIDAuthorizeWebRequestConfigurationMock : MSIDAuthorizeWebRequestConfigur
7878
final class MSIDSwitchBrowserOperationTest: XCTestCase
7979
{
8080
lazy var validSwitchBrowserResponse: MSIDSwitchBrowserResponse? = {
81-
let url = URL(string: "msauth.com.microsoft.msaltestapp://auth/switch_browser?action_uri=some_uri&code=some_code")!
82-
return try? MSIDSwitchBrowserResponse(url: url, redirectUri: "msauth.com.microsoft.msaltestapp://auth", context: nil)
81+
let url = URL(string: "msauth.com.microsoft.msaltestapp://auth/switch_browser?action_uri=some_uri&code=some_code&state=c3RhdGU")!
82+
return try? MSIDSwitchBrowserResponse(url: url, redirectUri: "msauth.com.microsoft.msaltestapp://auth", requestState: "state", context: nil)
8383
}()
8484

8585
override func setUpWithError() throws
8686
{
8787
// Put setup code here. This method is called before the invocation of each test method in the class.
88+
let flightProvider = MSIDFlightManagerMockProvider()
89+
flightProvider.boolForKeyContainer = [MSID_FLIGHT_SUPPORT_STATE_DUNA_CBA: true]
90+
MSIDFlightManager.sharedInstance().flightProvider = flightProvider
8891
}
8992

9093
override func tearDownWithError() throws
@@ -148,7 +151,7 @@ final class MSIDSwitchBrowserOperationTest: XCTestCase
148151

149152
XCTAssertEqual(1, certAuthManagerMock.startWithUrlInvokedCount)
150153
XCTAssertEqual(1, certAuthManagerMock.resetStateInvokedCount)
151-
XCTAssertEqual(URL(string: "some_uri?code=some_code"), certAuthManagerMock.startURLProvidedParam)
154+
XCTAssertEqual(URL(string: "some_uri?state=c3RhdGU&code=some_code"), certAuthManagerMock.startURLProvidedParam)
152155
}
153156

154157
func testInvoke_whenWebRequestConfigurationReturnError_shouldReturnError() async throws
@@ -186,7 +189,7 @@ final class MSIDSwitchBrowserOperationTest: XCTestCase
186189

187190
XCTAssertEqual(1, certAuthManagerMock.startWithUrlInvokedCount)
188191
XCTAssertEqual(1, certAuthManagerMock.resetStateInvokedCount)
189-
XCTAssertEqual(URL(string: "some_uri?code=some_code"), certAuthManagerMock.startURLProvidedParam)
192+
XCTAssertEqual(URL(string: "some_uri?state=c3RhdGU&code=some_code"), certAuthManagerMock.startURLProvidedParam)
190193
XCTAssertEqual(1, webRequestConfigurationMock.responseWithResultURLInvokedCount)
191194
}
192195

@@ -222,7 +225,7 @@ final class MSIDSwitchBrowserOperationTest: XCTestCase
222225

223226
XCTAssertEqual(1, certAuthManagerMock.startWithUrlInvokedCount)
224227
XCTAssertEqual(1, certAuthManagerMock.resetStateInvokedCount)
225-
XCTAssertEqual(URL(string: "some_uri?code=some_code"), certAuthManagerMock.startURLProvidedParam)
228+
XCTAssertEqual(URL(string: "some_uri?state=c3RhdGU&code=some_code"), certAuthManagerMock.startURLProvidedParam)
226229
XCTAssertEqual(1, webRequestConfigurationMock.responseWithResultURLInvokedCount)
227230
}
228231
}

0 commit comments

Comments
 (0)