Skip to content

Commit 9c814da

Browse files
oldaltonoldalton
committed
Addressed feedback comments
1 parent 3b2ce31 commit 9c814da

File tree

10 files changed

+152
-20
lines changed

10 files changed

+152
-20
lines changed

‎IdentityCore/IdentityCore.xcodeproj/project.pbxproj

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,8 @@
317317
B275520B208185E600AA7A38 /* KeyvaultAuthentication.swift in Sources */ = {isa = PBXBuildFile; fileRef = B275520A208185E600AA7A38 /* KeyvaultAuthentication.swift */; };
318318
B275520C208185E600AA7A38 /* KeyvaultAuthentication.swift in Sources */ = {isa = PBXBuildFile; fileRef = B275520A208185E600AA7A38 /* KeyvaultAuthentication.swift */; };
319319
B275520E208186F500AA7A38 /* MSIDAutomation-Bridging-Header.h in Headers */ = {isa = PBXBuildFile; fileRef = B275520D208186F500AA7A38 /* MSIDAutomation-Bridging-Header.h */; };
320+
B279835A20D33A0A0051167F /* MSIDIdTokenClaimsTests.m in Sources */ = {isa = PBXBuildFile; fileRef = B279835920D33A090051167F /* MSIDIdTokenClaimsTests.m */; };
321+
B279835B20D33A0A0051167F /* MSIDIdTokenClaimsTests.m in Sources */ = {isa = PBXBuildFile; fileRef = B279835920D33A090051167F /* MSIDIdTokenClaimsTests.m */; };
320322
B2807FF7204CAFDF00944D89 /* MSIDHelpers.h in Headers */ = {isa = PBXBuildFile; fileRef = B2807FF5204CAFDF00944D89 /* MSIDHelpers.h */; };
321323
B2807FF8204CAFDF00944D89 /* MSIDHelpers.m in Sources */ = {isa = PBXBuildFile; fileRef = B2807FF6204CAFDF00944D89 /* MSIDHelpers.m */; };
322324
B2807FF9204CAFDF00944D89 /* MSIDHelpers.m in Sources */ = {isa = PBXBuildFile; fileRef = B2807FF6204CAFDF00944D89 /* MSIDHelpers.m */; };
@@ -839,6 +841,7 @@
839841
B27551FD208182BE00AA7A38 /* AuthHeader.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = AuthHeader.swift; sourceTree = "<group>"; };
840842
B275520A208185E600AA7A38 /* KeyvaultAuthentication.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = KeyvaultAuthentication.swift; sourceTree = "<group>"; };
841843
B275520D208186F500AA7A38 /* MSIDAutomation-Bridging-Header.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = "MSIDAutomation-Bridging-Header.h"; sourceTree = "<group>"; };
844+
B279835920D33A090051167F /* MSIDIdTokenClaimsTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = MSIDIdTokenClaimsTests.m; sourceTree = "<group>"; };
842845
B2807FF5204CAFDF00944D89 /* MSIDHelpers.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = MSIDHelpers.h; sourceTree = "<group>"; };
843846
B2807FF6204CAFDF00944D89 /* MSIDHelpers.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = MSIDHelpers.m; sourceTree = "<group>"; };
844847
B2807FFA204CB16B00944D89 /* MSIDHelperTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = MSIDHelperTests.m; sourceTree = "<group>"; };
@@ -1824,6 +1827,7 @@
18241827
238EF0882091655D0035ABE6 /* MSIDHttpRequestTelemetryTests.m */,
18251828
B252913A2096698100E78695 /* MSIDAADIdTokenClaimsFactoryTests.m */,
18261829
B2DD4B4320A936FE0047A66E /* MSIDAccountCredentialsCacheTests.m */,
1830+
B279835920D33A090051167F /* MSIDIdTokenClaimsTests.m */,
18271831
);
18281832
path = tests;
18291833
sourceTree = "<group>";
@@ -2302,6 +2306,7 @@
23022306
B2936F8020ABFFC50050C585 /* MSIDAADOauth2FactoryTests.m in Sources */,
23032307
B2936F5020AA954A0050C585 /* MSIDAccountCacheItemTests.m in Sources */,
23042308
B2808004204CB2A700944D89 /* MSIDAADV1TokenResponseTests.m in Sources */,
2309+
B279835A20D33A0A0051167F /* MSIDIdTokenClaimsTests.m in Sources */,
23052310
B2936F4F20AA908F0050C585 /* MSIDCredentialCacheItemTests.m in Sources */,
23062311
B2936F7520ABF4940050C585 /* MSIDIdTokenTests.m in Sources */,
23072312
B2936F9120AE913E0050C585 /* MSIDLegacyTokenCacheIntegrationTests.m in Sources */,
@@ -2537,6 +2542,7 @@
25372542
B2DD5B9F204761550084313F /* MSIDAccessTokenTests.m in Sources */,
25382543
B2DD5BC120479AA80084313F /* MSIDJsonSerializerTests.m in Sources */,
25392544
B2DD5BB2204789FB0084313F /* MSIDIdTokenTests.m in Sources */,
2545+
B279835B20D33A0A0051167F /* MSIDIdTokenClaimsTests.m in Sources */,
25402546
B20657D01FC92B8F00412B7D /* MSIDTelemetryUIEventTests.m in Sources */,
25412547
B26A0B972072B9CB006BD95A /* MSIDAADV1Oauth2FactoryTests.m in Sources */,
25422548
B2DD5BA2204761660084313F /* MSIDLegacySingleResourceTokenTests.m in Sources */,

‎IdentityCore/src/cache/MSIDCacheAccessor.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@
6161
error:(NSError **)error;
6262

6363
/*!
64-
This method saves only the SSO artifacts to the cache based on the broker response.
64+
This method saves only the SSO artifacts to the cache based on the response.
6565
*/
6666
- (BOOL)saveSSOStateWithConfiguration:(MSIDConfiguration *)configuration
6767
response:(MSIDTokenResponse *)response

‎IdentityCore/src/cache/accessor/MSIDDefaultTokenCacheAccessor.m

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ - (MSIDAccessToken *)getAccessTokenForAccount:(MSIDAccountIdentifier *)account
233233
query.realm = configuration.authority.msidTenant;
234234
query.clientId = configuration.clientId;
235235
query.target = configuration.target;
236-
query.targetMatchingOptions = SubSet;
236+
query.targetMatchingOptions = MSIDSubSet;
237237
query.credentialType = MSIDAccessTokenType;
238238

239239
return (MSIDAccessToken *) [self getTokenWithAuthority:configuration.authority
@@ -286,7 +286,7 @@ - (MSIDIdToken *)getIDTokenForAccount:(MSIDAccountIdentifier *)account
286286
credentialsQuery.credentialType = MSIDRefreshTokenType;
287287
credentialsQuery.clientId = clientId;
288288
credentialsQuery.familyId = familyId;
289-
credentialsQuery.clientIdMatchingOptions = SuperSet;
289+
credentialsQuery.clientIdMatchingOptions = MSIDSuperSet;
290290
credentialsQuery.environmentAliases = environmentAliases;
291291

292292
NSArray<MSIDCredentialCacheItem *> *resultCredentials = [_accountCredentialCache getCredentialsWithQuery:credentialsQuery legacyUserId:nil context:context error:error];
@@ -538,7 +538,7 @@ - (BOOL)saveAccessToken:(MSIDAccessToken *)accessToken
538538
query.realm = accessToken.authority.msidTenant;
539539
query.clientId = accessToken.clientId;
540540
query.target = [accessToken.scopes msidToString];
541-
query.targetMatchingOptions = Intersect;
541+
query.targetMatchingOptions = MSIDIntersect;
542542
query.credentialType = MSIDAccessTokenType;
543543

544544
BOOL result = [_accountCredentialCache removeCredetialsWithQuery:query context:context error:error];

‎IdentityCore/src/cache/key/MSIDDefaultCredentialCacheQuery.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,10 @@
2626
#import "MSIDDefaultCredentialCacheKey.h"
2727

2828
typedef NS_ENUM(NSUInteger, MSIDComparisonOptions) {
29-
ExactStringMatch,
30-
SubSet,
31-
Intersect,
32-
SuperSet
29+
MSIDExactStringMatch,
30+
MSIDSubSet,
31+
MSIDIntersect,
32+
MSIDSuperSet
3333
};
3434

3535
@interface MSIDDefaultCredentialCacheQuery : MSIDDefaultCredentialCacheKey

‎IdentityCore/src/cache/key/MSIDDefaultCredentialCacheQuery.m

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ - (instancetype)init
3131

3232
if (self)
3333
{
34-
_targetMatchingOptions = ExactStringMatch;
35-
_clientIdMatchingOptions = ExactStringMatch;
34+
_targetMatchingOptions = MSIDExactStringMatch;
35+
_clientIdMatchingOptions = MSIDExactStringMatch;
3636
_matchAnyCredentialType = NO;
3737
}
3838

@@ -82,7 +82,7 @@ - (NSString *)serviceForAccessToken
8282
if (self.queryClientId
8383
&& self.realm
8484
&& self.target
85-
&& self.targetMatchingOptions == ExactStringMatch)
85+
&& self.targetMatchingOptions == MSIDExactStringMatch)
8686
{
8787
return [self serviceWithType:self.credentialType clientID:self.queryClientId realm:self.realm target:self.target];
8888
}
@@ -155,7 +155,7 @@ - (BOOL)exactMatch
155155
- (NSString *)queryClientId
156156
{
157157
if ((self.clientId || self.familyId)
158-
&& (self.clientIdMatchingOptions == ExactStringMatch))
158+
&& (self.clientIdMatchingOptions == MSIDExactStringMatch))
159159
{
160160
return self.familyId ? self.familyId : self.clientId;
161161
}

‎IdentityCore/src/cache/token/MSIDCredentialCacheItem.m

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -228,11 +228,11 @@ - (BOOL)matchesTarget:(NSString *)target comparisonOptions:(MSIDComparisonOption
228228
}
229229

230230
switch (comparisonOptions) {
231-
case ExactStringMatch:
231+
case MSIDExactStringMatch:
232232
return [self.target isEqualToString:target];
233-
case SubSet:
233+
case MSIDSubSet:
234234
return [[target scopeSet] isSubsetOfOrderedSet:[self.target scopeSet]];
235-
case Intersect:
235+
case MSIDIntersect:
236236
return [[target scopeSet] intersectsOrderedSet:[self.target scopeSet]];
237237
default:
238238
return NO;
@@ -299,7 +299,7 @@ - (BOOL)matchesWithRealm:(nullable NSString *)realm
299299
targetMatching:(MSIDComparisonOptions)matchingOptions
300300
clientIdMatching:(MSIDComparisonOptions)clientIDMatchingOptions
301301
{
302-
if (clientIDMatchingOptions == SuperSet && (clientId || familyId))
302+
if (clientIDMatchingOptions == MSIDSuperSet && (clientId || familyId))
303303
{
304304
if (![self.clientId isEqualToString:clientId]
305305
&& ![self.familyId isEqualToString:familyId])

‎IdentityCore/src/oauth2/MSIDIdTokenClaims.m

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,11 @@ - (instancetype)initWithRawIdToken:(NSString *)rawIdTokenString error:(NSError *
5050
{
5151
if ([NSString msidIsStringNilOrBlank:rawIdTokenString])
5252
{
53+
if (error)
54+
{
55+
*error = MSIDCreateError(MSIDErrorDomain, MSIDErrorServerInvalidResponse, @"Nil id_token passed", nil, nil, nil, nil, nil);
56+
}
57+
5358
return nil;
5459
}
5560

@@ -65,6 +70,12 @@ - (instancetype)initWithRawIdToken:(NSString *)rawIdTokenString error:(NSError *
6570
if (parts.count < 1)
6671
{
6772
MSID_LOG_ERROR(nil, @"Id token is invalid");
73+
74+
if (error)
75+
{
76+
*error = MSIDCreateError(MSIDErrorDomain, MSIDErrorServerInvalidResponse, @"Server returned empty id token", nil, nil, nil, nil, nil);
77+
}
78+
6879
return nil;
6980
}
7081

@@ -83,12 +94,20 @@ - (instancetype)initWithRawIdToken:(NSString *)rawIdTokenString error:(NSError *
8394
{
8495
MSID_LOG_WARN(nil, @"Failed to deserialize part of the id_token");
8596
MSID_LOG_WARN_PII(nil, @"Failed to deserialize part of the id_token %@", jsonError);
97+
98+
if (error) *error = jsonError;
8699
return nil;
87100
}
88101

89102
if (![jsonObject isKindOfClass:[NSDictionary class]])
90103
{
91104
MSID_LOG_WARN(nil, @"Invalid id token format");
105+
106+
if (error)
107+
{
108+
*error = MSIDCreateError(MSIDErrorDomain, MSIDErrorServerInvalidResponse, @"Server returned invalid id token", nil, nil, nil, nil, nil);
109+
}
110+
92111
return nil;
93112
}
94113

@@ -99,6 +118,12 @@ - (instancetype)initWithRawIdToken:(NSString *)rawIdTokenString error:(NSError *
99118
if (![allClaims count])
100119
{
101120
MSID_LOG_WARN(nil, @"Id token is invalid");
121+
122+
if (error)
123+
{
124+
*error = MSIDCreateError(MSIDErrorDomain, MSIDErrorServerInvalidResponse, @"Server returned id token without any claims", nil, nil, nil, nil, nil);
125+
}
126+
102127
return nil;
103128
}
104129

@@ -109,7 +134,6 @@ - (instancetype)initWithRawIdToken:(NSString *)rawIdTokenString error:(NSError *
109134
}
110135

111136
[self initDerivedProperties];
112-
113137
return self;
114138
}
115139

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ - (NSURL *)cacheURLForAuthority:(NSURL *)originalAuthority
158158

159159
if ([MSIDAuthority isTenantless:originalAuthority])
160160
{
161-
// If it's a tenantless authority, lookupb by universal "common" authority, which is supported by both v1 and v2
161+
// If it's a tenantless authority, lookup by universal "common" authority, which is supported by both v1 and v2
162162
[lookupAuthorities addObject:[MSIDAuthority universalAuthorityURL:originalAuthority]];
163163
}
164164
else

‎IdentityCore/tests/MSIDAccountCredentialsCacheTests.m

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -831,7 +831,7 @@ - (void)testGetCredentialsWithQuery_whenNotExactMatch_andAccessTokenQuery_matchB
831831
MSIDDefaultCredentialCacheQuery *query = [MSIDDefaultCredentialCacheQuery new];
832832
query.credentialType = MSIDAccessTokenType;
833833
query.target = @"user.write";
834-
query.targetMatchingOptions = SubSet;
834+
query.targetMatchingOptions = MSIDSubSet;
835835

836836
XCTAssertFalse(query.exactMatch);
837837
NSError *error = nil;
@@ -856,7 +856,7 @@ - (void)testGetCredentialsWithQuery_whenNotExactMatch_andAccessTokenQuery_matchB
856856
MSIDDefaultCredentialCacheQuery *query = [MSIDDefaultCredentialCacheQuery new];
857857
query.credentialType = MSIDAccessTokenType;
858858
query.target = @"user.write user.sing";
859-
query.targetMatchingOptions = Intersect;
859+
query.targetMatchingOptions = MSIDIntersect;
860860

861861
XCTAssertFalse(query.exactMatch);
862862
NSError *error = nil;

‎IdentityCore/tests/MSIDIdTokenClaimsTests.m

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
//------------------------------------------------------------------------------
2+
//
3+
// Copyright (c) Microsoft Corporation.
4+
// All rights reserved.
5+
//
6+
// This code is licensed under the MIT License.
7+
//
8+
// Permission is hereby granted, free of charge, to any person obtaining a copy
9+
// of this software and associated documentation files(the "Software"), to deal
10+
// in the Software without restriction, including without limitation the rights
11+
// to use, copy, modify, merge, publish, distribute, sublicense, and / or sell
12+
// copies of the Software, and to permit persons to whom the Software is
13+
// furnished to do so, subject to the following conditions :
14+
//
15+
// The above copyright notice and this permission notice shall be included in
16+
// all copies or substantial portions of the Software.
17+
//
18+
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
19+
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
20+
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.IN NO EVENT SHALL THE
21+
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
22+
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
23+
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
24+
// THE SOFTWARE.
25+
//
26+
//------------------------------------------------------------------------------
27+
28+
#import <XCTest/XCTest.h>
29+
#import "MSIDIdTokenClaims.h"
30+
31+
@interface MSIDIdTokenClaimsTests : XCTestCase
32+
33+
@end
34+
35+
@implementation MSIDIdTokenClaimsTests
36+
37+
- (void)testInitWithRawIdToken_whenInvalidJSONInOnePart_shouldReturnNilNonNilError
38+
{
39+
NSString *testIdToken = @"W10.e30.";
40+
NSError *error = nil;
41+
MSIDIdTokenClaims *claims = [[MSIDIdTokenClaims alloc] initWithRawIdToken:testIdToken error:&error];
42+
XCTAssertNil(claims);
43+
XCTAssertNotNil(error);
44+
}
45+
46+
- (void)testInitWithRawIdToken_whenNonDictionaryJSONInOnePart_shouldReturnNilNonNilError
47+
{
48+
NSString *testIdToken = @"e30.e30.";
49+
NSError *error = nil;
50+
MSIDIdTokenClaims *claims = [[MSIDIdTokenClaims alloc] initWithRawIdToken:testIdToken error:&error];
51+
XCTAssertNil(claims);
52+
XCTAssertNil(claims);
53+
}
54+
55+
- (void)testInitWithRawIdToken_whenNilToken_shouldReturnNilAndNonNilError
56+
{
57+
NSError *error = nil;
58+
MSIDIdTokenClaims *claims = [[MSIDIdTokenClaims alloc] initWithRawIdToken:nil error:&error];
59+
XCTAssertNil(claims);
60+
XCTAssertNotNil(error);
61+
}
62+
63+
- (void)testInitWithRawIdToken_whenValidIDToken_andUnsignedWithTwoParts_shouldReturnNoNilClaimsNilError
64+
{
65+
NSString *testIdToken = @"eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IlRlc3QiLCJpYXQiOjE1MTYyMzkwMjJ9";
66+
NSError *error = nil;
67+
MSIDIdTokenClaims *claims = [[MSIDIdTokenClaims alloc] initWithRawIdToken:testIdToken error:&error];
68+
XCTAssertNotNil(claims);
69+
XCTAssertNil(error);
70+
XCTAssertNotNil([claims jsonDictionary]);
71+
XCTAssertEqualObjects([claims jsonDictionary][@"name"], @"Test");
72+
XCTAssertEqualObjects([claims jsonDictionary][@"sub"], @"1234567890");
73+
XCTAssertEqualObjects([claims jsonDictionary][@"iat"], @1516239022);
74+
}
75+
76+
- (void)testInitWithRawIdToken_whenValidIDToken_andUnsigned_shouldReturnNoNilClaimsNilError
77+
{
78+
NSString *testIdToken = @"eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IlRlc3QiLCJpYXQiOjE1MTYyMzkwMjJ9.";
79+
NSError *error = nil;
80+
MSIDIdTokenClaims *claims = [[MSIDIdTokenClaims alloc] initWithRawIdToken:testIdToken error:&error];
81+
XCTAssertNotNil(claims);
82+
XCTAssertNil(error);
83+
XCTAssertNotNil([claims jsonDictionary]);
84+
XCTAssertEqualObjects([claims jsonDictionary][@"name"], @"Test");
85+
XCTAssertEqualObjects([claims jsonDictionary][@"sub"], @"1234567890");
86+
XCTAssertEqualObjects([claims jsonDictionary][@"iat"], @1516239022);
87+
}
88+
89+
- (void)testInitWithRawIdToken_whenValidIDToken_andSigned_shouldReturnNoNilClaimsNilError
90+
{
91+
NSString *testIdToken = @"eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IlRlc3QiLCJpYXQiOjE1MTYyMzkwMjJ9.N8AkaYjoO2a_G5vpSKJxL-YCPWfX47VWoUzLwneVb8Y";
92+
NSError *error = nil;
93+
MSIDIdTokenClaims *claims = [[MSIDIdTokenClaims alloc] initWithRawIdToken:testIdToken error:&error];
94+
XCTAssertNotNil(claims);
95+
XCTAssertNil(error);
96+
XCTAssertNotNil([claims jsonDictionary]);
97+
XCTAssertEqualObjects([claims jsonDictionary][@"name"], @"Test");
98+
XCTAssertEqualObjects([claims jsonDictionary][@"sub"], @"1234567890");
99+
XCTAssertEqualObjects([claims jsonDictionary][@"iat"], @1516239022);
100+
}
101+
102+
@end

0 commit comments

Comments
 (0)