Skip to content

Commit

Permalink
Merge pull request #1402 from AzureAD/oldalton/correlation_id_fix
Browse files Browse the repository at this point in the history
Fixed correlation ID and suberror bugs
  • Loading branch information
oldalton authored Apr 5, 2019
2 parents 75e6593 + 494cb69 commit a0237e9
Show file tree
Hide file tree
Showing 18 changed files with 359 additions and 44 deletions.
2 changes: 1 addition & 1 deletion ADAL.podspec
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Pod::Spec.new do |s|
s.name = "ADAL"
s.module_name = "ADAL"
s.version = "2.6.8"
s.version = "2.6.9"
s.summary = "The ADAL SDK for iOS gives you the ability to add Azure Identity authentication to your application"

s.description = <<-DESC
Expand Down
4 changes: 4 additions & 0 deletions ADAL/ADAL.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@
B20DC61F1F0DA3C500957806 /* ADKeychainTokenCacheTests.m in Sources */ = {isa = PBXBuildFile; fileRef = B20DC61E1F0DA3C500957806 /* ADKeychainTokenCacheTests.m */; };
B20DC6211F0DA4BF00957806 /* ADWebAuthControllerTests.m in Sources */ = {isa = PBXBuildFile; fileRef = B20DC6201F0DA4BF00957806 /* ADWebAuthControllerTests.m */; };
B20DC6221F0DA4BF00957806 /* ADWebAuthControllerTests.m in Sources */ = {isa = PBXBuildFile; fileRef = B20DC6201F0DA4BF00957806 /* ADWebAuthControllerTests.m */; };
B219F1E92255BEEC00B39ABE /* NSDictionaryExtensionTests.m in Sources */ = {isa = PBXBuildFile; fileRef = B219F1E82255BEEC00B39ABE /* NSDictionaryExtensionTests.m */; };
B225F063217559870052334D /* NSBundle+ADTestUtils.m in Sources */ = {isa = PBXBuildFile; fileRef = B225F062217559870052334D /* NSBundle+ADTestUtils.m */; };
B23FC03E1F0DA8F5008262F2 /* ADAcquireTokenPkeyAuthTests.m in Sources */ = {isa = PBXBuildFile; fileRef = B23FC03D1F0DA8F5008262F2 /* ADAcquireTokenPkeyAuthTests.m */; };
B260CB4B1F4E166500125A70 /* NSString+ADTelemetryExtensions.h in Headers */ = {isa = PBXBuildFile; fileRef = B260CB491F4E166500125A70 /* NSString+ADTelemetryExtensions.h */; };
Expand Down Expand Up @@ -779,6 +780,7 @@
B20DC61C1F0DA39C00957806 /* ADBrokerKeyHelperTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ADBrokerKeyHelperTests.m; sourceTree = "<group>"; };
B20DC61E1F0DA3C500957806 /* ADKeychainTokenCacheTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ADKeychainTokenCacheTests.m; sourceTree = "<group>"; };
B20DC6201F0DA4BF00957806 /* ADWebAuthControllerTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ADWebAuthControllerTests.m; sourceTree = "<group>"; };
B219F1E82255BEEC00B39ABE /* NSDictionaryExtensionTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = NSDictionaryExtensionTests.m; sourceTree = "<group>"; };
B225F061217559870052334D /* NSBundle+ADTestUtils.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "NSBundle+ADTestUtils.h"; sourceTree = "<group>"; };
B225F062217559870052334D /* NSBundle+ADTestUtils.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = "NSBundle+ADTestUtils.m"; sourceTree = "<group>"; };
B23FC03D1F0DA8F5008262F2 /* ADAcquireTokenPkeyAuthTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ADAcquireTokenPkeyAuthTests.m; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1482,6 +1484,7 @@
230E16E21FB17A6200ADC904 /* ADTelemetryAPIEventTests.m */,
230E16E01FB17A2C00ADC904 /* ADTelemetryUIEventTests.m */,
B28F647F216E7BDB00CF526F /* ADClientCapabilitiesUtilTests.m */,
B219F1E82255BEEC00B39ABE /* NSDictionaryExtensionTests.m */,
B20DC5D51F0D97C600957806 /* ios */,
B20DC5D81F0D97CD00957806 /* mac */,
);
Expand Down Expand Up @@ -2124,6 +2127,7 @@
B20DC5F31F0D998A00957806 /* ADAuthenticationParametersTests.m in Sources */,
D66A9F281F7998D300144011 /* ADTokenCacheTestUtil.m in Sources */,
234F3CED1F35182500DE4AA4 /* ADAuthenticationContextTests.m in Sources */,
B219F1E92255BEEC00B39ABE /* NSDictionaryExtensionTests.m in Sources */,
603841A01DF9248F00D30F3D /* ADTelemetryTestDispatcher.m in Sources */,
B20DC5FF1F0D998A00957806 /* ADTokenCacheItemTests.m in Sources */,
A5EE637D20B4AD5C005A8085 /* ADEnrollmentGatewayTests.m in Sources */,
Expand Down
2 changes: 1 addition & 1 deletion ADAL/resources/ios/Framework/Info.plist
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
<key>CFBundlePackageType</key>
<string>FMWK</string>
<key>CFBundleShortVersionString</key>
<string>2.6.8</string>
<string>2.6.9</string>
<key>CFBundleSignature</key>
<string>????</string>
<key>CFBundleVersion</key>
Expand Down
2 changes: 1 addition & 1 deletion ADAL/resources/mac/Info.plist
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
<key>CFBundlePackageType</key>
<string>FMWK</string>
<key>CFBundleShortVersionString</key>
<string>2.6.6</string>
<string>2.6.9</string>
<key>CFBundleSignature</key>
<string>????</string>
<key>CFBundleVersion</key>
Expand Down
2 changes: 1 addition & 1 deletion ADAL/src/ADAL_Internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
// through build script. Don't change its format unless changing build script as well.)
#define ADAL_VER_HIGH 2
#define ADAL_VER_LOW 6
#define ADAL_VER_PATCH 8
#define ADAL_VER_PATCH 9

#define STR_HELPER(x) #x
#define STR(x) STR_HELPER(x)
Expand Down
30 changes: 14 additions & 16 deletions ADAL/src/ADAuthenticationContext+Internal.m
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#import "ADUserIdentifier.h"
#import "ADTokenCacheItem+Internal.h"
#import "ADHelpers.h"
#import "NSDictionary+ADExtensions.h"

NSString* const ADUnknownError = @"Uknown error.";
NSString* const ADCredentialsNeeded = @"The user credentials are needed to obtain access token. Please call the non-silent acquireTokenWithResource methods.";
Expand Down Expand Up @@ -101,30 +102,27 @@ + (ADAuthenticationError*)errorFromDictionary:(NSDictionary*)dictionary
errorCode:(ADErrorCode)errorCode
{
//First check for explicit OAuth2 protocol error:
NSString *serverOAuth2Error = [dictionary objectForKey:OAUTH2_ERROR];
if (![NSString adIsStringNilOrBlank:serverOAuth2Error])
NSString *serverOAuth2Error = [dictionary adStringForKey:OAUTH2_ERROR];
if (serverOAuth2Error)
{
NSString *errorDetails = [dictionary objectForKey:OAUTH2_ERROR_DESCRIPTION];
// Error response from the server
NSUUID *correlationId = [dictionary objectForKey:OAUTH2_CORRELATION_ID_RESPONSE] ?
[[NSUUID alloc] initWithUUIDString:[dictionary objectForKey:OAUTH2_CORRELATION_ID_RESPONSE]]:
nil;
NSString *responseCorrelationId = [dictionary adStringForKey:OAUTH2_CORRELATION_ID_RESPONSE];
NSUUID *correlationId = responseCorrelationId ? [[NSUUID alloc] initWithUUIDString:responseCorrelationId] : nil;

ADErrorCode code = errorCode;
NSString *suberror = [dictionary objectForKey:AUTH_SUBERROR];
NSMutableDictionary *userInfo = nil;
NSString *suberror = [dictionary adStringForKey:AUTH_SUBERROR];
NSMutableDictionary *userInfo = [NSMutableDictionary new];
userInfo[ADSuberrorKey] = suberror;

if (suberror && [suberror isEqualToString:AUTH_PROTECTION_POLICY_REQUIRED])
{
code = AD_ERROR_SERVER_PROTECTION_POLICY_REQUIRED;
userInfo = [[NSMutableDictionary alloc] initWithCapacity:2];
[userInfo setObject:suberror forKey:ADSuberrorKey];

// check for additional user identifier
userInfo[ADUserIdKey] = [dictionary objectForKey:AUTH_ADDITIONAL_USER_IDENTIFIER];
}


userInfo[ADUserIdKey] = [dictionary adStringForKey:AUTH_ADDITIONAL_USER_IDENTIFIER];
NSString *errorDescription = [dictionary adStringForKey:OAUTH2_ERROR_DESCRIPTION];

return [ADAuthenticationError OAuthServerError:serverOAuth2Error
description:errorDetails
description:errorDescription
code:code
correlationId:correlationId
userInfo:userInfo];
Expand Down
22 changes: 3 additions & 19 deletions ADAL/src/ADAuthenticationResult+Internal.m
Original file line number Diff line number Diff line change
Expand Up @@ -166,10 +166,7 @@ + (ADAuthenticationResult*)resultForBrokerErrorResponse:(NSDictionary*)response
errorDetails = @"Broker did not provide any details";
}

if ([response valueForKey:BROKER_APP_VERSION])
{
[userInfo setValue:[response valueForKey:BROKER_APP_VERSION] forKey:ADBrokerVersionKey];
}
userInfo[ADBrokerVersionKey] = [response adStringForKey:BROKER_APP_VERSION];

NSString* strErrorCode = [response valueForKey:@"error_code"];
NSInteger errorCode = AD_ERROR_TOKENBROKER_UNKNOWN;
Expand All @@ -178,21 +175,8 @@ + (ADAuthenticationResult*)resultForBrokerErrorResponse:(NSDictionary*)response
errorCode = [strErrorCode integerValue];
}



if (errorCode == AD_ERROR_SERVER_PROTECTION_POLICY_REQUIRED)
{
// For protection_policy_required error, add extra info for the app in the userInfo dictionary of the error
if ([response valueForKey:AUTH_SUBERROR])
{
[userInfo setValue:[response valueForKey:AUTH_SUBERROR] forKey:ADSuberrorKey];
}

if ([response valueForKey:@"user_id"])
{
[userInfo setValue:[response valueForKey:@"user_id"] forKey:ADUserIdKey];
}
}
userInfo[ADSuberrorKey] = [response adStringForKey:AUTH_SUBERROR];
userInfo[ADUserIdKey] = [response adStringForKey:@"user_id"];

NSString* protocolCode = [response valueForKey:@"protocol_code"];
if (!protocolCode)
Expand Down
7 changes: 5 additions & 2 deletions ADAL/src/request/ADAuthenticationRequest+AcquireToken.m
Original file line number Diff line number Diff line change
Expand Up @@ -298,12 +298,15 @@ - (void)requestToken:(ADAuthenticationCallback)completionBlock
}
else
{
NSDictionary* underlyingError = _underlyingError ? @{NSUnderlyingErrorKey:_underlyingError} : nil;
NSMutableDictionary *underlyingUserInfo = [NSMutableDictionary new];
[underlyingUserInfo addEntriesFromDictionary:_underlyingError.userInfo];
underlyingUserInfo[NSUnderlyingErrorKey] = _underlyingError;

ADAuthenticationError* error =
[ADAuthenticationError errorFromAuthenticationError:AD_ERROR_SERVER_USER_INPUT_NEEDED
protocolCode:nil
errorDetails:ADCredentialsNeeded
userInfo:underlyingError
userInfo:underlyingUserInfo
correlationId:correlationId];
result = [ADAuthenticationResult resultFromError:error correlationId:correlationId];
}
Expand Down
2 changes: 1 addition & 1 deletion ADAL/src/request/ADAuthenticationRequest+Broker.m
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ + (ADAuthenticationResult *)processBrokerResponse:(NSURL *)response

ADAuthenticationResult *intuneTokenResult = [[ADTokenCacheItem new] processTokenResponse:intuneTokenResponse
fromRefreshToken:nil
requestCorrelationId:intuneTokenResponse[OAUTH2_CORRELATION_ID_RESPONSE]];
requestCorrelationId:correlationId];

if (!keychainGroup)
{
Expand Down
1 change: 1 addition & 0 deletions ADAL/src/utils/NSDictionary+ADExtensions.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,6 @@

+ (NSDictionary *)adURLFormDecode:(NSString *)string;
- (NSString *)adURLFormEncode;
- (NSString *)adStringForKey:(NSString *)dictKey;

@end
17 changes: 17 additions & 0 deletions ADAL/src/utils/NSDictionary+ADExtensions.m
Original file line number Diff line number Diff line change
Expand Up @@ -84,4 +84,21 @@ - (NSString *)adURLFormEncode
return parameters;
}

- (NSString *)adStringForKey:(NSString *)dictKey
{
if (!dictKey)
{
return nil;
}

NSString *value = self[dictKey];

if (![NSString adIsStringNilOrBlank:value])
{
return value;
}

return nil;
}

@end
1 change: 1 addition & 0 deletions ADAL/tests/XCTestCase+TestHelperMethods.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
resource:(NSString *)resource
clientId:(NSString *)clientId
oauthError:(NSString *)oauthError
oauthSubError:(id)suberror
correlationId:(NSUUID *)correlationId
requestParams:(NSDictionary *)requestParams;

Expand Down
10 changes: 8 additions & 2 deletions ADAL/tests/XCTestCase+TestHelperMethods.m
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ - (ADTestURLResponse *)adResponseBadRefreshToken:(NSString *)refreshToken
resource:(NSString *)resource
clientId:(NSString *)clientId
oauthError:(NSString *)oauthError
oauthSubError:(id)suberror
correlationId:(NSUUID *)correlationId
requestParams:(NSDictionary *)requestParams
{
Expand All @@ -295,15 +296,19 @@ - (ADTestURLResponse *)adResponseBadRefreshToken:(NSString *)refreshToken

[requestParamsBody addEntriesFromDictionary:requestParams];

NSMutableDictionary *responseDict = [NSMutableDictionary new];
responseDict[OAUTH2_ERROR] = oauthError;
responseDict[OAUTH2_ERROR_DESCRIPTION] = @"oauth error description";
responseDict[@"suberror"] = suberror;

ADTestURLResponse* response =
[ADTestURLResponse requestURLString:requestUrlString
requestHeaders:requestHeaders
requestParamsBody:requestParamsBody
responseURLString:@"https://contoso.com"
responseCode:400
httpHeaderFields:@{@"x-ms-clitelem" : @"1,7000,7,255.0643,I"}
dictionaryAsJSON:@{ OAUTH2_ERROR : oauthError,
OAUTH2_ERROR_DESCRIPTION : @"oauth error description"}];
dictionaryAsJSON:responseDict];

return response;
}
Expand All @@ -315,6 +320,7 @@ - (ADTestURLResponse *)adDefaultBadRefreshTokenResponseError:(NSString*)oauthErr
resource:TEST_RESOURCE
clientId:TEST_CLIENT_ID
oauthError:oauthError
oauthSubError:nil
correlationId:TEST_CORRELATION_ID
requestParams:nil];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -744,6 +744,7 @@ - (void)testAcquireTokenSilent_whenDifferentPreferredCacheAndTokenFails_shouldRe
clientId:TEST_CLIENT_ID
// invalid_grant should result in ADAL tombstoning the token
oauthError:@"invalid_grant"
oauthSubError:nil
correlationId:TEST_CORRELATION_ID
requestParams:nil];

Expand Down
Loading

0 comments on commit a0237e9

Please sign in to comment.