From 4ddccc5e435dc4358cc528272adfb8c9d7a708a9 Mon Sep 17 00:00:00 2001 From: pinlu Date: Thu, 12 Jan 2023 18:36:13 -0800 Subject: [PATCH 1/5] Add GIDAuthorizationUtil class and tests. --- .../GIDAuthorizationFlowProcessor.m | 56 +---- GoogleSignIn/Sources/GIDAuthorizationUtil.h | 48 ++++ GoogleSignIn/Sources/GIDAuthorizationUtil.m | 105 +++++++++ GoogleSignIn/Sources/GIDGoogleUser.m | 2 - GoogleSignIn/Sources/GIDSignIn.m | 45 ++-- GoogleSignIn/Sources/GIDSignInPreferences.h | 9 + GoogleSignIn/Sources/GIDSignInPreferences.m | 6 + GoogleSignIn/Sources/GIDSignIn_Private.h | 6 +- .../Tests/Unit/GIDAuthorizationFlowUtilTest.m | 186 +++++++++++++++ GoogleSignIn/Tests/Unit/GIDSignInTest.m | 221 +----------------- 10 files changed, 390 insertions(+), 294 deletions(-) create mode 100644 GoogleSignIn/Sources/GIDAuthorizationUtil.h create mode 100644 GoogleSignIn/Sources/GIDAuthorizationUtil.m create mode 100644 GoogleSignIn/Tests/Unit/GIDAuthorizationFlowUtilTest.m diff --git a/GoogleSignIn/Sources/GIDAuthorizationFlowProcessor/Implementations/GIDAuthorizationFlowProcessor.m b/GoogleSignIn/Sources/GIDAuthorizationFlowProcessor/Implementations/GIDAuthorizationFlowProcessor.m index b8b62d2d..e79fde5a 100644 --- a/GoogleSignIn/Sources/GIDAuthorizationFlowProcessor/Implementations/GIDAuthorizationFlowProcessor.m +++ b/GoogleSignIn/Sources/GIDAuthorizationFlowProcessor/Implementations/GIDAuthorizationFlowProcessor.m @@ -16,12 +16,9 @@ #import "GoogleSignIn/Sources/GIDAuthorizationFlowProcessor/Implementations/GIDAuthorizationFlowProcessor.h" -#import "GoogleSignIn/Sources/Public/GoogleSignIn/GIDConfiguration.h" +#import "GoogleSignIn/Sources/GIDAuthorizationUtil.h" -#import "GoogleSignIn/Sources/GIDEMMSupport.h" -#import "GoogleSignIn/Sources/GIDSignInCallbackSchemes.h" #import "GoogleSignIn/Sources/GIDSignInInternalOptions.h" -#import "GoogleSignIn/Sources/GIDSignInPreferences.h" #ifdef SWIFT_PACKAGE @import AppAuth; @@ -31,21 +28,11 @@ NS_ASSUME_NONNULL_BEGIN -// Parameters for the auth and token exchange endpoints. -static NSString *const kAudienceParameter = @"audience"; - -static NSString *const kIncludeGrantedScopesParameter = @"include_granted_scopes"; -static NSString *const kLoginHintParameter = @"login_hint"; -static NSString *const kHostedDomainParameter = @"hd"; - @interface GIDAuthorizationFlowProcessor () /// AppAuth external user-agent session state. @property(nonatomic, nullable)id currentAuthorizationFlow; -/// AppAuth configuration object. -@property(nonatomic)OIDServiceConfiguration *appAuthConfiguration; - @end @implementation GIDAuthorizationFlowProcessor @@ -60,47 +47,10 @@ - (void)startWithOptions:(GIDSignInInternalOptions *)options emmSupport:(nullable NSString *)emmSupport completion:(void (^)(OIDAuthorizationResponse *_Nullable authorizationResponse, NSError *_Nullable error))completion { - GIDSignInCallbackSchemes *schemes = - [[GIDSignInCallbackSchemes alloc] initWithClientIdentifier:options.configuration.clientID]; - NSString *urlString = [NSString stringWithFormat:@"%@:%@", - [schemes clientIdentifierScheme], kBrowserCallbackPath]; - NSURL *redirectURL = [NSURL URLWithString:urlString]; - - NSMutableDictionary *additionalParameters = [@{} mutableCopy]; - additionalParameters[kIncludeGrantedScopesParameter] = @"true"; - if (options.configuration.serverClientID) { - additionalParameters[kAudienceParameter] = options.configuration.serverClientID; - } - if (options.loginHint) { - additionalParameters[kLoginHintParameter] = options.loginHint; - } - if (options.configuration.hostedDomain) { - additionalParameters[kHostedDomainParameter] = options.configuration.hostedDomain; - } - -#if TARGET_OS_IOS && !TARGET_OS_MACCATALYST - [additionalParameters addEntriesFromDictionary: - [GIDEMMSupport parametersWithParameters:options.extraParams - emmSupport:emmSupport - isPasscodeInfoRequired:NO]]; -#elif TARGET_OS_OSX || TARGET_OS_MACCATALYST - [additionalParameters addEntriesFromDictionary:options.extraParams]; -#endif // TARGET_OS_OSX || TARGET_OS_MACCATALYST - additionalParameters[kSDKVersionLoggingParameter] = GIDVersion(); - additionalParameters[kEnvironmentLoggingParameter] = GIDEnvironment(); - NSURL *authorizationEndpointURL = [GIDSignInPreferences authorizationEndpointURL]; - NSURL *tokenEndpointURL = [GIDSignInPreferences tokenEndpointURL]; - OIDServiceConfiguration *appAuthConfiguration = - [[OIDServiceConfiguration alloc] initWithAuthorizationEndpoint:authorizationEndpointURL - tokenEndpoint:tokenEndpointURL]; OIDAuthorizationRequest *request = - [[OIDAuthorizationRequest alloc] initWithConfiguration:appAuthConfiguration - clientId:options.configuration.clientID - scopes:options.scopes - redirectURL:redirectURL - responseType:OIDResponseTypeCode - additionalParameters:additionalParameters]; + [GIDAuthorizationUtil createAuthorizationRequestWithOptions:options + emmSupport:emmSupport]; _currentAuthorizationFlow = [OIDAuthorizationService presentAuthorizationRequest:request diff --git a/GoogleSignIn/Sources/GIDAuthorizationUtil.h b/GoogleSignIn/Sources/GIDAuthorizationUtil.h new file mode 100644 index 00000000..bff7eb93 --- /dev/null +++ b/GoogleSignIn/Sources/GIDAuthorizationUtil.h @@ -0,0 +1,48 @@ +/* + * Copyright 2023 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#import + +@class OIDAuthorizationRequest; +@class GIDSignInInternalOptions; + +NS_ASSUME_NONNULL_BEGIN + +/// The util class for authorization process. +@interface GIDAuthorizationUtil : NSObject + +/// Creates the request to AppAuth to start the authorization flow. +/// +/// @param options The `GIDSignInInternalOptions` object to provide serverClientID, hostedDomain, +/// clientID, scopes, loginHint and extraParams. +/// @param emmSupport The EMM support info string. ++ (OIDAuthorizationRequest *) + createAuthorizationRequestWithOptions:(GIDSignInInternalOptions *)options + emmSupport:(nullable NSString *)emmSupport; + +/// Unions two scopes or returns error if the new scopes are the subset of the existing scopes. +/// +/// @param scopes The existing scopes. +/// @param newScopes The new scopes to add. +/// @param error The reference to the error. +/// @return The array of all scopes or nil if there ia an error. ++ (nullable NSArray *)unionScopes:(NSArray *)scopes + withNewScopes:(NSArray *)newScopes + error:(NSError * __autoreleasing *)error; + +@end + +NS_ASSUME_NONNULL_END diff --git a/GoogleSignIn/Sources/GIDAuthorizationUtil.m b/GoogleSignIn/Sources/GIDAuthorizationUtil.m new file mode 100644 index 00000000..52b445f9 --- /dev/null +++ b/GoogleSignIn/Sources/GIDAuthorizationUtil.m @@ -0,0 +1,105 @@ +/* + * Copyright 2023 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#import "GoogleSignIn/Sources/GIDAuthorizationUtil.h" + +#import "GoogleSignIn/Sources/Public/GoogleSignIn/GIDConfiguration.h" + +#import "GoogleSignIn/Sources/GIDEMMSupport.h" +#import "GoogleSignIn/Sources/GIDSignInCallbackSchemes.h" +#import "GoogleSignIn/Sources/GIDSignInInternalOptions.h" +#import "GoogleSignIn/Sources/GIDSignInPreferences.h" + +#ifdef SWIFT_PACKAGE +@import AppAuth; +#else +#import +#endif + +NS_ASSUME_NONNULL_BEGIN + +@implementation GIDAuthorizationUtil + ++ (OIDAuthorizationRequest *) + createAuthorizationRequestWithOptions:(GIDSignInInternalOptions *)options + emmSupport:(nullable NSString *)emmSupport { + GIDSignInCallbackSchemes *schemes = + [[GIDSignInCallbackSchemes alloc] initWithClientIdentifier:options.configuration.clientID]; + NSString *urlString = [NSString stringWithFormat:@"%@:%@", + [schemes clientIdentifierScheme], kBrowserCallbackPath]; + NSURL *redirectURL = [NSURL URLWithString:urlString]; + + NSMutableDictionary *additionalParameters = [@{} mutableCopy]; + additionalParameters[kIncludeGrantedScopesParameter] = @"true"; + if (options.configuration.serverClientID) { + additionalParameters[kAudienceParameter] = options.configuration.serverClientID; + } + if (options.loginHint) { + additionalParameters[kLoginHintParameter] = options.loginHint; + } + if (options.configuration.hostedDomain) { + additionalParameters[kHostedDomainParameter] = options.configuration.hostedDomain; + } + +#if TARGET_OS_IOS && !TARGET_OS_MACCATALYST + [additionalParameters addEntriesFromDictionary: + [GIDEMMSupport parametersWithParameters:options.extraParams + emmSupport:emmSupport + isPasscodeInfoRequired:NO]]; +#elif TARGET_OS_OSX || TARGET_OS_MACCATALYST + [additionalParameters addEntriesFromDictionary:options.extraParams]; +#endif // TARGET_OS_OSX || TARGET_OS_MACCATALYST + additionalParameters[kSDKVersionLoggingParameter] = GIDVersion(); + additionalParameters[kEnvironmentLoggingParameter] = GIDEnvironment(); + + NSURL *authorizationEndpointURL = [GIDSignInPreferences authorizationEndpointURL]; + NSURL *tokenEndpointURL = [GIDSignInPreferences tokenEndpointURL]; + OIDServiceConfiguration *appAuthConfiguration = + [[OIDServiceConfiguration alloc] initWithAuthorizationEndpoint:authorizationEndpointURL + tokenEndpoint:tokenEndpointURL]; + OIDAuthorizationRequest *request = + [[OIDAuthorizationRequest alloc] initWithConfiguration:appAuthConfiguration + clientId:options.configuration.clientID + scopes:options.scopes + redirectURL:redirectURL + responseType:OIDResponseTypeCode + additionalParameters:additionalParameters]; + + return request; +} + ++ (nullable NSArray *)unionScopes:(NSArray *)scopes + withNewScopes:(NSArray *)newScopes + error:(NSError * __autoreleasing *)error { + NSSet *requestedScopes = [NSSet setWithArray:newScopes]; + NSMutableSet *grantedScopes = [NSMutableSet setWithArray:scopes]; + + if ([requestedScopes isSubsetOfSet:grantedScopes]) { + // All requested scopes have already been granted, notify callback of failure. + *error = [NSError errorWithDomain:kGIDSignInErrorDomain + code:kGIDSignInErrorCodeScopesAlreadyGranted + userInfo:nil]; + return nil; + } + + // Use the union of granted and requested scopes. + [grantedScopes unionSet:requestedScopes]; + return [grantedScopes allObjects]; +} + +@end + +NS_ASSUME_NONNULL_END diff --git a/GoogleSignIn/Sources/GIDGoogleUser.m b/GoogleSignIn/Sources/GIDGoogleUser.m index 828ac5b6..b113c266 100644 --- a/GoogleSignIn/Sources/GIDGoogleUser.m +++ b/GoogleSignIn/Sources/GIDGoogleUser.m @@ -42,8 +42,6 @@ static NSString *const kProfileDataKey = @"profileData"; static NSString *const kAuthStateKey = @"authState"; -// Parameters for the token exchange endpoint. -static NSString *const kAudienceParameter = @"audience"; static NSString *const kOpenIDRealmParameter = @"openid.realm"; // Additional parameter names for EMM. diff --git a/GoogleSignIn/Sources/GIDSignIn.m b/GoogleSignIn/Sources/GIDSignIn.m index 0a84fce8..4a2e09f5 100644 --- a/GoogleSignIn/Sources/GIDSignIn.m +++ b/GoogleSignIn/Sources/GIDSignIn.m @@ -23,6 +23,7 @@ #import "GoogleSignIn/Sources/GIDAuthorizationFlowProcessor/API/GIDAuthorizationFlowProcessor.h" #import "GoogleSignIn/Sources/GIDAuthorizationFlowProcessor/Implementations/GIDAuthorizationFlowProcessor.h" +#import "GoogleSignIn/Sources/GIDAuthorizationUtil.h" #import "GoogleSignIn/Sources/GIDHTTPFetcher/API/GIDHTTPFetcher.h" #import "GoogleSignIn/Sources/GIDHTTPFetcher/Implementations/GIDHTTPFetcher.h" #import "GoogleSignIn/Sources/GIDEMMSupport.h" @@ -75,6 +76,9 @@ NS_ASSUME_NONNULL_BEGIN +// The EMM support version +NSString *const kEMMVersion = @"1"; + // The name of the query parameter used for logging the restart of auth from EMM callback. static NSString *const kEMMRestartAuthParameter = @"emmres"; @@ -84,9 +88,6 @@ // Expected path for EMM callback. static NSString *const kEMMCallbackPath = @"/emmcallback"; -// The EMM support version -static NSString *const kEMMVersion = @"1"; - // The error code for Google Identity. NSErrorDomain const kGIDSignInErrorDomain = @"com.google.GIDSignIn"; @@ -111,8 +112,6 @@ // The delay before the new sign-in flow can be presented after the existing one is cancelled. static const NSTimeInterval kPresentationDelayAfterCancel = 1.0; -// Parameters for the auth and token exchange endpoints. -static NSString *const kAudienceParameter = @"audience"; // See b/11669751 . static NSString *const kOpenIDRealmParameter = @"openid.realm"; @@ -260,24 +259,11 @@ - (void)signInWithPresentingViewController:(UIViewController *)presentingViewCon - (void)addScopes:(NSArray *)scopes presentingViewController:(UIViewController *)presentingViewController completion:(nullable GIDSignInCompletion)completion { - GIDConfiguration *configuration = self.currentUser.configuration; - GIDSignInInternalOptions *options = - [GIDSignInInternalOptions defaultOptionsWithConfiguration:configuration - presentingViewController:presentingViewController - loginHint:self.currentUser.profile.email - addScopesFlow:YES - completion:completion]; - - NSSet *requestedScopes = [NSSet setWithArray:scopes]; - NSMutableSet *grantedScopes = - [NSMutableSet setWithArray:self.currentUser.grantedScopes]; - - // Check to see if all requested scopes have already been granted. - if ([requestedScopes isSubsetOfSet:grantedScopes]) { - // All requested scopes have already been granted, notify callback of failure. - NSError *error = [NSError errorWithDomain:kGIDSignInErrorDomain - code:kGIDSignInErrorCodeScopesAlreadyGranted - userInfo:nil]; + NSError *error; + NSArray *allScopes = [GIDAuthorizationUtil unionScopes:self.currentUser.grantedScopes + withNewScopes:scopes + error:&error]; + if (error) { if (completion) { dispatch_async(dispatch_get_main_queue(), ^{ completion(nil, error); @@ -285,10 +271,15 @@ - (void)addScopes:(NSArray *)scopes } return; } - - // Use the union of granted and requested scopes. - [grantedScopes unionSet:requestedScopes]; - options.scopes = [grantedScopes allObjects]; + + GIDConfiguration *configuration = self.currentUser.configuration; + GIDSignInInternalOptions *options = + [GIDSignInInternalOptions defaultOptionsWithConfiguration:configuration + presentingViewController:presentingViewController + loginHint:self.currentUser.profile.email + addScopesFlow:YES + completion:completion]; + options.scopes = allScopes; [self signInWithOptions:options]; } diff --git a/GoogleSignIn/Sources/GIDSignInPreferences.h b/GoogleSignIn/Sources/GIDSignInPreferences.h index f7f0a637..3304f9df 100644 --- a/GoogleSignIn/Sources/GIDSignInPreferences.h +++ b/GoogleSignIn/Sources/GIDSignInPreferences.h @@ -24,6 +24,15 @@ extern NSString *const kSDKVersionLoggingParameter; /// The name of the query parameter used for logging the Apple execution environment. extern NSString *const kEnvironmentLoggingParameter; +/// The name of the query parameter for the token exchange endpoint. +extern NSString *const kAudienceParameter; + +extern NSString *const kIncludeGrantedScopesParameter; + +extern NSString *const kLoginHintParameter; + +extern NSString *const kHostedDomainParameter; + /// Expected path in the URL scheme to be handled. extern NSString *const kBrowserCallbackPath; diff --git a/GoogleSignIn/Sources/GIDSignInPreferences.m b/GoogleSignIn/Sources/GIDSignInPreferences.m index 366f3bbb..6696b491 100644 --- a/GoogleSignIn/Sources/GIDSignInPreferences.m +++ b/GoogleSignIn/Sources/GIDSignInPreferences.m @@ -16,8 +16,14 @@ NS_ASSUME_NONNULL_BEGIN +/// Parameters for the auth and token exchange endpoints. +NSString *const kAudienceParameter = @"audience"; +NSString *const kIncludeGrantedScopesParameter = @"include_granted_scopes"; +NSString *const kLoginHintParameter = @"login_hint"; +NSString *const kHostedDomainParameter = @"hd"; NSString *const kSDKVersionLoggingParameter = @"gpsdk"; NSString *const kEnvironmentLoggingParameter = @"gidenv"; + NSString *const kBrowserCallbackPath = @"/oauth2callback"; static NSString *const kLSOServer = @"accounts.google.com"; diff --git a/GoogleSignIn/Sources/GIDSignIn_Private.h b/GoogleSignIn/Sources/GIDSignIn_Private.h index 35a45c93..0d0cf545 100644 --- a/GoogleSignIn/Sources/GIDSignIn_Private.h +++ b/GoogleSignIn/Sources/GIDSignIn_Private.h @@ -24,8 +24,6 @@ #import #endif -NS_ASSUME_NONNULL_BEGIN - @class GIDGoogleUser; @class GIDSignInInternalOptions; @@ -34,6 +32,10 @@ NS_ASSUME_NONNULL_BEGIN @protocol GIDKeychainHandler; @protocol GIDProfileDataFetcher; +NS_ASSUME_NONNULL_BEGIN + +extern NSString *const kEMMVersion; + /// Represents a completion block that takes a `GIDSignInResult` on success or an error if the /// operation was unsuccessful. typedef void (^GIDSignInCompletion)(GIDSignInResult *_Nullable signInResult, diff --git a/GoogleSignIn/Tests/Unit/GIDAuthorizationFlowUtilTest.m b/GoogleSignIn/Tests/Unit/GIDAuthorizationFlowUtilTest.m new file mode 100644 index 00000000..9928e88f --- /dev/null +++ b/GoogleSignIn/Tests/Unit/GIDAuthorizationFlowUtilTest.m @@ -0,0 +1,186 @@ +// Copyright 2023 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#import + +#import + +#import "GoogleSignIn/Sources/GIDAuthorizationUtil.h" + +#import "GoogleSignIn/Sources/GIDSignInInternalOptions.h" +#import "GoogleSignIn/Sources/GIDSignInPreferences.h" + +#import "GoogleSignIn/Tests/Unit/OIDTokenResponse+Testing.h" + + +#ifdef SWIFT_PACKAGE +@import AppAuth; +#else +#import +#endif + +NS_ASSUME_NONNULL_BEGIN + +static NSString * const kClientId = @"FakeClientID"; +static NSString * const kUserEmail = @"FakeUserEmail"; +static NSString * const kServerClientId = @"FakeServerClientID"; + +static NSString * const kScopeBirthday = @"birthday"; +static NSString * const kScopeEmail = @"email"; +static NSString * const kScopeProfile = @"profile"; + +@interface GIDAuthorizationFlowUtilTest : XCTestCase + +@end + +@implementation GIDAuthorizationFlowUtilTest { + GIDConfiguration *_configuration; +} + +- (void)setUp { + [super setUp]; + _configuration = [[GIDConfiguration alloc] initWithClientID:kClientId + serverClientID:kServerClientId + hostedDomain:kHostedDomain + openIDRealm:nil]; +} + +- (void)testCreateAuthorizationRequest_signInFlow { + GIDSignInInternalOptions *options = + [GIDSignInInternalOptions defaultOptionsWithConfiguration:_configuration + presentingViewController:nil + loginHint:kUserEmail + addScopesFlow:NO + completion:nil]; + OIDAuthorizationRequest *request = + [GIDAuthorizationUtil createAuthorizationRequestWithOptions:options + emmSupport:nil]; + + NSDictionary *params = request.additionalParameters; + XCTAssertEqualObjects(params[kIncludeGrantedScopesParameter], @"true"); + XCTAssertEqualObjects(params[kSDKVersionLoggingParameter], GIDVersion()); + XCTAssertEqualObjects(params[kEnvironmentLoggingParameter], GIDEnvironment()); + XCTAssertEqualObjects(params[kLoginHintParameter], kUserEmail, @"login hint should match"); + XCTAssertEqualObjects(params[kHostedDomainParameter], kHostedDomain, + @"hosted domain should match"); + XCTAssertEqualObjects(params[kAudienceParameter], kServerClientId, @"client ID should match"); + + NSArray *defaultScopes = @[kScopeEmail, kScopeProfile]; + NSString *expectedScopeString = [defaultScopes componentsJoinedByString:@" "]; + XCTAssertEqualObjects(request.scope, expectedScopeString); +} + +- (void)testCreateAuthorizationRequest_additionalScopes { + NSArray *addtionalScopes = @[kScopeBirthday]; + GIDSignInInternalOptions *options = + [GIDSignInInternalOptions defaultOptionsWithConfiguration:_configuration + presentingViewController:nil + loginHint:kUserEmail + addScopesFlow:NO + scopes:addtionalScopes + completion:nil]; + OIDAuthorizationRequest *request = + [GIDAuthorizationUtil createAuthorizationRequestWithOptions:options + emmSupport:nil]; + + NSArray *expectedScopes = @[kScopeBirthday, kScopeEmail, kScopeProfile]; + NSString *expectedScopeString = [expectedScopes componentsJoinedByString:@" "]; + XCTAssertEqualObjects(request.scope, expectedScopeString); +} + +- (void)testCreateAuthrizationRequest_addScopes { + NSArray *scopes = @[kScopeEmail, kScopeProfile, kScopeBirthday]; + GIDSignInInternalOptions *options = + [GIDSignInInternalOptions defaultOptionsWithConfiguration:_configuration + presentingViewController:nil + loginHint:kUserEmail + addScopesFlow:YES + scopes:scopes + completion:nil]; + OIDAuthorizationRequest *request = + [GIDAuthorizationUtil createAuthorizationRequestWithOptions:options + emmSupport:nil]; + + NSString *expectedScopeString = [scopes componentsJoinedByString:@" "]; + XCTAssertEqualObjects(request.scope, expectedScopeString); +} + +#if TARGET_OS_IOS && !TARGET_OS_MACCATALYST + +- (void)testCreateAuthorizationRequest_signInFlow_EMM { + GIDSignInInternalOptions *options = + [GIDSignInInternalOptions defaultOptionsWithConfiguration:_configuration + presentingViewController:nil + loginHint:kUserEmail + addScopesFlow:NO + completion:nil]; + OIDAuthorizationRequest *request = + [GIDAuthorizationUtil createAuthorizationRequestWithOptions:options + emmSupport:kEMMVersion]; + + NSString *systemName = [UIDevice currentDevice].systemName; + if ([systemName isEqualToString:@"iPhone OS"]) { + systemName = @"iOS"; + } + NSString *expectedOSVersion = [NSString stringWithFormat:@"%@ %@", + systemName, [UIDevice currentDevice].systemVersion]; + NSDictionary *authParams = request.additionalParameters; + + BOOL isEligibleForEMM = [UIDevice currentDevice].systemVersion.integerValue >= 9; + if (isEligibleForEMM) { + XCTAssertEqualObjects(authParams[@"emm_support"], kEMMVersion, + @"EMM support should match in auth request"); + XCTAssertEqualObjects(authParams[@"device_os"], expectedOSVersion, + @"OS version should match in auth request"); + } else { + XCTAssertNil(authParams[@"emm_support"], + @"EMM support should not be in auth request for unsupported OS"); + XCTAssertNil(authParams[@"device_os"], + @"OS version should not be in auth request for unsupported OS"); + } +} + +#endif // TARGET_OS_IOS && !TARGET_OS_MACCATALYST + +- (void)testUnionScopes_success { + NSArray *scopes = @[kScopeEmail, kScopeProfile]; + NSArray *newScopes = @[kScopeBirthday]; + + NSError *error; + NSArray *allScopes = [GIDAuthorizationUtil unionScopes:scopes + withNewScopes:newScopes + error:&error]; + + NSArray *expectedScopes = @[kScopeEmail, kScopeProfile, kScopeBirthday]; + XCTAssertEqualObjects(allScopes, expectedScopes); + XCTAssertNil(error); +} + +- (void)testUnitScopes_addExistingScopes_error { + NSArray *scopes = @[kScopeEmail, kScopeProfile, kScopeBirthday]; + NSArray *newScopes = @[kScopeBirthday]; + + NSError *error; + NSArray *allScopes = [GIDAuthorizationUtil unionScopes:scopes + withNewScopes:newScopes + error:&error]; + + XCTAssertNil(allScopes); + XCTAssertEqualObjects(error.domain, kGIDSignInErrorDomain); + XCTAssertEqual(error.code, kGIDSignInErrorCodeScopesAlreadyGranted); +} + +@end + +NS_ASSUME_NONNULL_END diff --git a/GoogleSignIn/Tests/Unit/GIDSignInTest.m b/GoogleSignIn/Tests/Unit/GIDSignInTest.m index d0fc72fa..0385c584 100644 --- a/GoogleSignIn/Tests/Unit/GIDSignInTest.m +++ b/GoogleSignIn/Tests/Unit/GIDSignInTest.m @@ -91,8 +91,6 @@ static NSString * const kClientId2 = @"FakeClientID2"; static NSString * const kServerClientId = @"FakeServerClientID"; static NSString * const kLanguage = @"FakeLanguage"; -static NSString * const kScope = @"FakeScope"; -static NSString * const kScope2 = @"FakeScope2"; static NSString * const kAuthCode = @"FakeAuthCode"; static NSString * const kKeychainName = @"auth"; static NSString * const kUserEmail = @"FakeUserEmail"; @@ -133,13 +131,6 @@ static NSString * const kUserIDKey = @"userID"; static NSString * const kHostedDomainKey = @"hostedDomain"; static NSString * const kIDTokenExpirationKey = @"idTokenExp"; -static NSString * const kScopeKey = @"scope"; - -// Basic profile (Fat ID Token / userinfo endpoint) keys -static NSString *const kBasicProfilePictureKey = @"picture"; -static NSString *const kBasicProfileNameKey = @"name"; -static NSString *const kBasicProfileGivenNameKey = @"given_name"; -static NSString *const kBasicProfileFamilyNameKey = @"family_name"; static NSString * const kCustomKeychainName = @"CUSTOM_KEYCHAIN_NAME"; static NSString * const kAddActivity = @"http://schemas.google.com/AddActivity"; @@ -152,9 +143,6 @@ static NSString *const kFakeURL = @"http://foo.com"; -static NSString *const kEMMSupport = @"1"; - -static NSString *const kGrantedScope = @"grantedScope"; static NSString *const kNewScope = @"newScope"; #if TARGET_OS_IOS || TARGET_OS_MACCATALYST @@ -238,9 +226,6 @@ @interface GIDSignInTest : XCTestCase { // The completion to be used when testing |GIDSignIn|. GIDSignInCompletion _completion; - // The saved authorization request. - OIDAuthorizationRequest *_savedAuthorizationRequest; - #if TARGET_OS_IOS || TARGET_OS_MACCATALYST // The saved presentingViewController from the authorization request. UIViewController *_savedPresentingViewController; @@ -290,11 +275,11 @@ - (void)setUp { _user = OCMStrictClassMock([GIDGoogleUser class]); _oidAuthorizationService = OCMStrictClassMock([OIDAuthorizationService class]); OCMStub([_oidAuthorizationService - presentAuthorizationRequest:SAVE_TO_ARG_BLOCK(self->_savedAuthorizationRequest) + presentAuthorizationRequest:OCMOCK_ANY #if TARGET_OS_IOS || TARGET_OS_MACCATALYST - presentingViewController:SAVE_TO_ARG_BLOCK(self->_savedPresentingViewController) + presentingViewController:SAVE_TO_ARG_BLOCK(self->_savedPresentingViewController) #elif TARGET_OS_OSX - presentingWindow:SAVE_TO_ARG_BLOCK(self->_savedPresentingWindow) + presentingWindow:SAVE_TO_ARG_BLOCK(self->_savedPresentingWindow) #endif // TARGET_OS_IOS || TARGET_OS_MACCATALYST callback:COPY_TO_ARG_BLOCK(self->_savedAuthorizationCallback)]); OCMStub([self->_oidAuthorizationService @@ -521,108 +506,6 @@ - (void)testOAuthLogin_RestoredSignInOldAccessToken { modalCancel:NO]; } -- (void)testOAuthLogin_AdditionalScopes { - NSString *expectedScopeString; - - [self OAuthLoginWithAddScopesFlow:NO - authError:nil - tokenError:nil - emmPasscodeInfoRequired:NO - keychainError:NO - restoredSignIn:NO - oldAccessToken:NO - modalCancel:NO - useAdditionalScopes:YES - additionalScopes:nil]; - - expectedScopeString = [@[ @"email", @"profile" ] componentsJoinedByString:@" "]; - XCTAssertEqualObjects(_savedAuthorizationRequest.scope, expectedScopeString); - - [self OAuthLoginWithAddScopesFlow:NO - authError:nil - tokenError:nil - emmPasscodeInfoRequired:NO - keychainError:NO - restoredSignIn:NO - oldAccessToken:NO - modalCancel:NO - useAdditionalScopes:YES - additionalScopes:@[ kScope ]]; - - expectedScopeString = [@[ kScope, @"email", @"profile" ] componentsJoinedByString:@" "]; - XCTAssertEqualObjects(_savedAuthorizationRequest.scope, expectedScopeString); - - [self OAuthLoginWithAddScopesFlow:NO - authError:nil - tokenError:nil - emmPasscodeInfoRequired:NO - keychainError:NO - restoredSignIn:NO - oldAccessToken:NO - modalCancel:NO - useAdditionalScopes:YES - additionalScopes:@[ kScope, kScope2 ]]; - - expectedScopeString = [@[ kScope, kScope2, @"email", @"profile" ] componentsJoinedByString:@" "]; - XCTAssertEqualObjects(_savedAuthorizationRequest.scope, expectedScopeString); -} - -- (void)testAddScopes { - // Restore the previous sign-in account. This is the preparation for adding scopes. - [self OAuthLoginWithAddScopesFlow:NO - authError:nil - tokenError:nil - emmPasscodeInfoRequired:NO - keychainError:NO - restoredSignIn:YES - oldAccessToken:NO - modalCancel:NO]; - - XCTAssertNotNil(_signIn.currentUser); - - id profile = OCMStrictClassMock([GIDProfileData class]); - OCMStub([profile email]).andReturn(kUserEmail); - - // Mock for the method `addScopes`. - GIDConfiguration *configuration = [[GIDConfiguration alloc] initWithClientID:kClientId - serverClientID:nil - hostedDomain:nil - openIDRealm:kOpenIDRealm]; - OCMStub([_user configuration]).andReturn(configuration); - OCMStub([_user profile]).andReturn(profile); - OCMStub([_user grantedScopes]).andReturn(@[kGrantedScope]); - - [self OAuthLoginWithAddScopesFlow:YES - authError:nil - tokenError:nil - emmPasscodeInfoRequired:NO - keychainError:NO - restoredSignIn:NO - oldAccessToken:NO - modalCancel:NO]; - - NSArray *grantedScopes; - NSString *grantedScopeString = _savedAuthorizationRequest.scope; - - if (grantedScopeString) { - grantedScopeString = [grantedScopeString stringByTrimmingCharactersInSet: - [NSCharacterSet whitespaceCharacterSet]]; - // Tokenize with space as a delimiter. - NSMutableArray *parsedScopes = - [[grantedScopeString componentsSeparatedByString:@" "] mutableCopy]; - // Remove empty strings. - [parsedScopes removeObject:@""]; - grantedScopes = [parsedScopes copy]; - } - - NSArray *expectedScopes = @[kNewScope, kGrantedScope]; - XCTAssertEqualObjects(grantedScopes, expectedScopes); - - [_user verify]; - [profile verify]; - [profile stopMocking]; -} - - (void)testOpenIDRealm { _signIn.configuration = [[GIDConfiguration alloc] initWithClientID:kClientId serverClientID:nil @@ -642,41 +525,6 @@ - (void)testOpenIDRealm { XCTAssertEqual(params[kOpenIDRealmKey], kOpenIDRealm, @"OpenID Realm should match."); } -- (void)testOAuthLogin_LoginHint { - _hint = kUserEmail; - - [self OAuthLoginWithAddScopesFlow:NO - authError:nil - tokenError:nil - emmPasscodeInfoRequired:NO - keychainError:NO - restoredSignIn:NO - oldAccessToken:NO - modalCancel:NO]; - - NSDictionary *params = _savedAuthorizationRequest.additionalParameters; - XCTAssertEqualObjects(params[@"login_hint"], kUserEmail, @"login hint should match"); -} - -- (void)testOAuthLogin_HostedDomain { - _signIn.configuration = [[GIDConfiguration alloc] initWithClientID:kClientId - serverClientID:nil - hostedDomain:kHostedDomain - openIDRealm:nil]; - - [self OAuthLoginWithAddScopesFlow:NO - authError:nil - tokenError:nil - emmPasscodeInfoRequired:NO - keychainError:NO - restoredSignIn:NO - oldAccessToken:NO - modalCancel:NO]; - - NSDictionary *params = _savedAuthorizationRequest.additionalParameters; - XCTAssertEqualObjects(params[@"hd"], kHostedDomain, @"hosted domain should match"); -} - - (void)testOAuthLogin_ConsentCanceled { [self OAuthLoginWithAddScopesFlow:NO authError:@"access_denied" @@ -1005,15 +853,10 @@ - (void)testEmmSupportRequestParameters { } NSString *expectedOSVersion = [NSString stringWithFormat:@"%@ %@", systemName, [UIDevice currentDevice].systemVersion]; - NSDictionary *authParams = - _savedAuthorizationRequest.additionalParameters; + NSDictionary *tokenParams = _savedTokenRequest.additionalParameters; if (_isEligibleForEMM) { - XCTAssertEqualObjects(authParams[@"emm_support"], kEMMSupport, - @"EMM support should match in auth request"); - XCTAssertEqualObjects(authParams[@"device_os"], expectedOSVersion, - @"OS version should match in auth request"); - XCTAssertEqualObjects(tokenParams[@"emm_support"], kEMMSupport, + XCTAssertEqualObjects(tokenParams[@"emm_support"], kEMMVersion, @"EMM support should match in token request"); XCTAssertEqualObjects(tokenParams[@"device_os"], expectedOSVersion, @@ -1021,10 +864,6 @@ - (void)testEmmSupportRequestParameters { XCTAssertNil(tokenParams[@"emm_passcode_info"], @"no passcode info should be in token request"); } else { - XCTAssertNil(authParams[@"emm_support"], - @"EMM support should not be in auth request for unsupported OS"); - XCTAssertNil(authParams[@"device_os"], - @"OS version should not be in auth request for unsupported OS"); XCTAssertNil(tokenParams[@"emm_support"], @"EMM support should not be in token request for unsupported OS"); XCTAssertNil(tokenParams[@"device_os"], @@ -1153,26 +992,6 @@ - (void)verifyRevokeRequest:(NSURLRequest *)request withToken:(NSString *)token XCTAssertEqualObjects(strings[1], token); } -- (void)OAuthLoginWithAddScopesFlow:(BOOL)addScopesFlow - authError:(NSString *)authError - tokenError:(NSError *)tokenError - emmPasscodeInfoRequired:(BOOL)emmPasscodeInfoRequired - keychainError:(BOOL)keychainError - restoredSignIn:(BOOL)restoredSignIn - oldAccessToken:(BOOL)oldAccessToken - modalCancel:(BOOL)modalCancel { - [self OAuthLoginWithAddScopesFlow:addScopesFlow - authError:authError - tokenError:tokenError - emmPasscodeInfoRequired:emmPasscodeInfoRequired - keychainError:keychainError - restoredSignIn:restoredSignIn - oldAccessToken:oldAccessToken - modalCancel:modalCancel - useAdditionalScopes:NO - additionalScopes:nil]; -} - // The authorization flow with parameters to control which branches to take. - (void)OAuthLoginWithAddScopesFlow:(BOOL)addScopesFlow authError:(NSString *)authError @@ -1181,9 +1000,7 @@ - (void)OAuthLoginWithAddScopesFlow:(BOOL)addScopesFlow keychainError:(BOOL)keychainError restoredSignIn:(BOOL)restoredSignIn oldAccessToken:(BOOL)oldAccessToken - modalCancel:(BOOL)modalCancel - useAdditionalScopes:(BOOL)useAdditionalScopes - additionalScopes:(NSArray *)additionalScopes { + modalCancel:(BOOL)modalCancel { if (restoredSignIn) { // clearAndAuthenticateWithOptions [_keychainHandler saveAuthState:_authState]; @@ -1253,33 +1070,17 @@ - (void)OAuthLoginWithAddScopesFlow:(BOOL)addScopesFlow #endif // TARGET_OS_IOS || TARGET_OS_MACCATALYST completion:completion]; } else { - if (useAdditionalScopes) { #if TARGET_OS_IOS || TARGET_OS_MACCATALYST - [_signIn signInWithPresentingViewController:_presentingViewController + [_signIn signInWithPresentingViewController:_presentingViewController #elif TARGET_OS_OSX - [_signIn signInWithPresentingWindow:_presentingWindow + [_signIn signInWithPresentingWindow:_presentingWindow #endif // TARGET_OS_IOS || TARGET_OS_MACCATALYST - hint:_hint - additionalScopes:additionalScopes - completion:completion]; - } else { -#if TARGET_OS_IOS || TARGET_OS_MACCATALYST - [_signIn signInWithPresentingViewController:_presentingViewController -#elif TARGET_OS_OSX - [_signIn signInWithPresentingWindow:_presentingWindow -#endif // TARGET_OS_IOS || TARGET_OS_MACCATALYST - hint:_hint - completion:completion]; - } + hint:_hint + completion:completion]; } [_authState verify]; - - XCTAssertNotNil(_savedAuthorizationRequest); - NSDictionary *params = _savedAuthorizationRequest.additionalParameters; - XCTAssertEqualObjects(params[@"include_granted_scopes"], @"true"); - XCTAssertEqualObjects(params[kSDKVersionLoggingParameter], GIDVersion()); - XCTAssertEqualObjects(params[kEnvironmentLoggingParameter], GIDEnvironment()); + XCTAssertNotNil(_savedAuthorizationCallback); #if TARGET_OS_IOS || TARGET_OS_MACCATALYST XCTAssertEqual(_savedPresentingViewController, _presentingViewController); From d5f9837afea0c1802a178b9e0e1dcdf6e45c3f22 Mon Sep 17 00:00:00 2001 From: pinlu Date: Thu, 12 Jan 2023 19:01:02 -0800 Subject: [PATCH 2/5] Style improvement. --- GoogleSignIn/Tests/Unit/GIDAuthorizationFlowUtilTest.m | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/GoogleSignIn/Tests/Unit/GIDAuthorizationFlowUtilTest.m b/GoogleSignIn/Tests/Unit/GIDAuthorizationFlowUtilTest.m index 9928e88f..4d5b4971 100644 --- a/GoogleSignIn/Tests/Unit/GIDAuthorizationFlowUtilTest.m +++ b/GoogleSignIn/Tests/Unit/GIDAuthorizationFlowUtilTest.m @@ -18,12 +18,12 @@ #import "GoogleSignIn/Sources/GIDAuthorizationUtil.h" +#import "GoogleSignIn/Sources/Public/GoogleSignIn/GIDConfiguration.h" + #import "GoogleSignIn/Sources/GIDSignInInternalOptions.h" #import "GoogleSignIn/Sources/GIDSignInPreferences.h" - #import "GoogleSignIn/Tests/Unit/OIDTokenResponse+Testing.h" - #ifdef SWIFT_PACKAGE @import AppAuth; #else @@ -108,6 +108,7 @@ - (void)testCreateAuthrizationRequest_addScopes { addScopesFlow:YES scopes:scopes completion:nil]; + OIDAuthorizationRequest *request = [GIDAuthorizationUtil createAuthorizationRequestWithOptions:options emmSupport:nil]; From 82b01bb2f3c999a6c6ca52d8c8488a041c820f35 Mon Sep 17 00:00:00 2001 From: pinlu Date: Fri, 13 Jan 2023 13:35:33 -0800 Subject: [PATCH 3/5] Fix tests for macOS. --- ...wUtilTest.m => GIDAuthorizationUtilTest.m} | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) rename GoogleSignIn/Tests/Unit/{GIDAuthorizationFlowUtilTest.m => GIDAuthorizationUtilTest.m} (91%) diff --git a/GoogleSignIn/Tests/Unit/GIDAuthorizationFlowUtilTest.m b/GoogleSignIn/Tests/Unit/GIDAuthorizationUtilTest.m similarity index 91% rename from GoogleSignIn/Tests/Unit/GIDAuthorizationFlowUtilTest.m rename to GoogleSignIn/Tests/Unit/GIDAuthorizationUtilTest.m index 4d5b4971..46575439 100644 --- a/GoogleSignIn/Tests/Unit/GIDAuthorizationFlowUtilTest.m +++ b/GoogleSignIn/Tests/Unit/GIDAuthorizationUtilTest.m @@ -40,11 +40,11 @@ static NSString * const kScopeEmail = @"email"; static NSString * const kScopeProfile = @"profile"; -@interface GIDAuthorizationFlowUtilTest : XCTestCase +@interface GIDAuthorizationUtilTest : XCTestCase @end -@implementation GIDAuthorizationFlowUtilTest { +@implementation GIDAuthorizationUtilTest { GIDConfiguration *_configuration; } @@ -59,7 +59,11 @@ - (void)setUp { - (void)testCreateAuthorizationRequest_signInFlow { GIDSignInInternalOptions *options = [GIDSignInInternalOptions defaultOptionsWithConfiguration:_configuration +#if TARGET_OS_IOS || TARGET_OS_MACCATALYST presentingViewController:nil +#elif TARGET_OS_OSX + presentingWindow:nil +#endif // TARGET_OS_OSX loginHint:kUserEmail addScopesFlow:NO completion:nil]; @@ -85,7 +89,11 @@ - (void)testCreateAuthorizationRequest_additionalScopes { NSArray *addtionalScopes = @[kScopeBirthday]; GIDSignInInternalOptions *options = [GIDSignInInternalOptions defaultOptionsWithConfiguration:_configuration +#if TARGET_OS_IOS || TARGET_OS_MACCATALYST presentingViewController:nil +#elif TARGET_OS_OSX + presentingWindow:nil +#endif // TARGET_OS_OSX loginHint:kUserEmail addScopesFlow:NO scopes:addtionalScopes @@ -103,7 +111,11 @@ - (void)testCreateAuthrizationRequest_addScopes { NSArray *scopes = @[kScopeEmail, kScopeProfile, kScopeBirthday]; GIDSignInInternalOptions *options = [GIDSignInInternalOptions defaultOptionsWithConfiguration:_configuration +#if TARGET_OS_IOS || TARGET_OS_MACCATALYST presentingViewController:nil +#elif TARGET_OS_OSX + presentingWindow:nil +#endif // TARGET_OS_OSX loginHint:kUserEmail addScopesFlow:YES scopes:scopes @@ -122,7 +134,11 @@ - (void)testCreateAuthrizationRequest_addScopes { - (void)testCreateAuthorizationRequest_signInFlow_EMM { GIDSignInInternalOptions *options = [GIDSignInInternalOptions defaultOptionsWithConfiguration:_configuration +#if TARGET_OS_IOS || TARGET_OS_MACCATALYST presentingViewController:nil +#elif TARGET_OS_OSX + presentingWindow:nil +#endif // TARGET_OS_OSX loginHint:kUserEmail addScopesFlow:NO completion:nil]; From 532247d16cf5bb6a4ed9c6301ad8cbd7e414b528 Mon Sep 17 00:00:00 2001 From: pinlu Date: Fri, 13 Jan 2023 14:20:30 -0800 Subject: [PATCH 4/5] Improve method naming. --- .../GIDAuthorizationFlowProcessor.m | 5 ++--- GoogleSignIn/Sources/GIDAuthorizationUtil.h | 8 ++++---- GoogleSignIn/Sources/GIDAuthorizationUtil.m | 8 ++++---- .../Tests/Unit/GIDAuthorizationUtilTest.m | 16 ++++++++-------- 4 files changed, 18 insertions(+), 19 deletions(-) diff --git a/GoogleSignIn/Sources/GIDAuthorizationFlowProcessor/Implementations/GIDAuthorizationFlowProcessor.m b/GoogleSignIn/Sources/GIDAuthorizationFlowProcessor/Implementations/GIDAuthorizationFlowProcessor.m index e79fde5a..069d9e4b 100644 --- a/GoogleSignIn/Sources/GIDAuthorizationFlowProcessor/Implementations/GIDAuthorizationFlowProcessor.m +++ b/GoogleSignIn/Sources/GIDAuthorizationFlowProcessor/Implementations/GIDAuthorizationFlowProcessor.m @@ -47,10 +47,9 @@ - (void)startWithOptions:(GIDSignInInternalOptions *)options emmSupport:(nullable NSString *)emmSupport completion:(void (^)(OIDAuthorizationResponse *_Nullable authorizationResponse, NSError *_Nullable error))completion { - OIDAuthorizationRequest *request = - [GIDAuthorizationUtil createAuthorizationRequestWithOptions:options - emmSupport:emmSupport]; + [GIDAuthorizationUtil authorizationRequestWithOptions:options + emmSupport:emmSupport]; _currentAuthorizationFlow = [OIDAuthorizationService presentAuthorizationRequest:request diff --git a/GoogleSignIn/Sources/GIDAuthorizationUtil.h b/GoogleSignIn/Sources/GIDAuthorizationUtil.h index bff7eb93..3ed72237 100644 --- a/GoogleSignIn/Sources/GIDAuthorizationUtil.h +++ b/GoogleSignIn/Sources/GIDAuthorizationUtil.h @@ -30,15 +30,15 @@ NS_ASSUME_NONNULL_BEGIN /// clientID, scopes, loginHint and extraParams. /// @param emmSupport The EMM support info string. + (OIDAuthorizationRequest *) - createAuthorizationRequestWithOptions:(GIDSignInInternalOptions *)options - emmSupport:(nullable NSString *)emmSupport; + authorizationRequestWithOptions:(GIDSignInInternalOptions *)options + emmSupport:(nullable NSString *)emmSupport; -/// Unions two scopes or returns error if the new scopes are the subset of the existing scopes. +/// Unions two scopes or returns an error if the new scopes are the subset of the existing scopes. /// /// @param scopes The existing scopes. /// @param newScopes The new scopes to add. /// @param error The reference to the error. -/// @return The array of all scopes or nil if there ia an error. +/// @return The array of all scopes or nil if there is an error. + (nullable NSArray *)unionScopes:(NSArray *)scopes withNewScopes:(NSArray *)newScopes error:(NSError * __autoreleasing *)error; diff --git a/GoogleSignIn/Sources/GIDAuthorizationUtil.m b/GoogleSignIn/Sources/GIDAuthorizationUtil.m index 52b445f9..05b123a0 100644 --- a/GoogleSignIn/Sources/GIDAuthorizationUtil.m +++ b/GoogleSignIn/Sources/GIDAuthorizationUtil.m @@ -34,8 +34,8 @@ @implementation GIDAuthorizationUtil + (OIDAuthorizationRequest *) - createAuthorizationRequestWithOptions:(GIDSignInInternalOptions *)options - emmSupport:(nullable NSString *)emmSupport { + authorizationRequestWithOptions:(GIDSignInInternalOptions *)options + emmSupport:(nullable NSString *)emmSupport { GIDSignInCallbackSchemes *schemes = [[GIDSignInCallbackSchemes alloc] initWithClientIdentifier:options.configuration.clientID]; NSString *urlString = [NSString stringWithFormat:@"%@:%@", @@ -84,11 +84,11 @@ @implementation GIDAuthorizationUtil + (nullable NSArray *)unionScopes:(NSArray *)scopes withNewScopes:(NSArray *)newScopes error:(NSError * __autoreleasing *)error { - NSSet *requestedScopes = [NSSet setWithArray:newScopes]; NSMutableSet *grantedScopes = [NSMutableSet setWithArray:scopes]; + NSSet *requestedScopes = [NSSet setWithArray:newScopes]; if ([requestedScopes isSubsetOfSet:grantedScopes]) { - // All requested scopes have already been granted, notify callback of failure. + // All requested scopes have already been granted, generate an error. *error = [NSError errorWithDomain:kGIDSignInErrorDomain code:kGIDSignInErrorCodeScopesAlreadyGranted userInfo:nil]; diff --git a/GoogleSignIn/Tests/Unit/GIDAuthorizationUtilTest.m b/GoogleSignIn/Tests/Unit/GIDAuthorizationUtilTest.m index 46575439..3a4ab1e2 100644 --- a/GoogleSignIn/Tests/Unit/GIDAuthorizationUtilTest.m +++ b/GoogleSignIn/Tests/Unit/GIDAuthorizationUtilTest.m @@ -68,8 +68,8 @@ - (void)testCreateAuthorizationRequest_signInFlow { addScopesFlow:NO completion:nil]; OIDAuthorizationRequest *request = - [GIDAuthorizationUtil createAuthorizationRequestWithOptions:options - emmSupport:nil]; + [GIDAuthorizationUtil authorizationRequestWithOptions:options + emmSupport:nil]; NSDictionary *params = request.additionalParameters; XCTAssertEqualObjects(params[kIncludeGrantedScopesParameter], @"true"); @@ -99,8 +99,8 @@ - (void)testCreateAuthorizationRequest_additionalScopes { scopes:addtionalScopes completion:nil]; OIDAuthorizationRequest *request = - [GIDAuthorizationUtil createAuthorizationRequestWithOptions:options - emmSupport:nil]; + [GIDAuthorizationUtil authorizationRequestWithOptions:options + emmSupport:nil]; NSArray *expectedScopes = @[kScopeBirthday, kScopeEmail, kScopeProfile]; NSString *expectedScopeString = [expectedScopes componentsJoinedByString:@" "]; @@ -122,8 +122,8 @@ - (void)testCreateAuthrizationRequest_addScopes { completion:nil]; OIDAuthorizationRequest *request = - [GIDAuthorizationUtil createAuthorizationRequestWithOptions:options - emmSupport:nil]; + [GIDAuthorizationUtil authorizationRequestWithOptions:options + emmSupport:nil]; NSString *expectedScopeString = [scopes componentsJoinedByString:@" "]; XCTAssertEqualObjects(request.scope, expectedScopeString); @@ -143,8 +143,8 @@ - (void)testCreateAuthorizationRequest_signInFlow_EMM { addScopesFlow:NO completion:nil]; OIDAuthorizationRequest *request = - [GIDAuthorizationUtil createAuthorizationRequestWithOptions:options - emmSupport:kEMMVersion]; + [GIDAuthorizationUtil authorizationRequestWithOptions:options + emmSupport:kEMMVersion]; NSString *systemName = [UIDevice currentDevice].systemName; if ([systemName isEqualToString:@"iPhone OS"]) { From 62be44aa8a46ca0d3513d8c711efaf8ba2145571 Mon Sep 17 00:00:00 2001 From: pinlu Date: Fri, 13 Jan 2023 20:59:47 -0800 Subject: [PATCH 5/5] Improve documentation and style. --- .../GIDAuthorizationFlowProcessor.m | 3 +-- GoogleSignIn/Sources/GIDAuthorizationUtil.h | 10 ++++++---- GoogleSignIn/Sources/GIDAuthorizationUtil.m | 7 ++++--- GoogleSignIn/Sources/GIDSignIn.m | 7 ++++--- GoogleSignIn/Sources/GIDSignIn_Private.h | 1 + .../Tests/Unit/GIDAuthorizationUtilTest.m | 16 +++++++++------- GoogleSignIn/Tests/Unit/GIDSignInTest.m | 6 +++--- 7 files changed, 28 insertions(+), 22 deletions(-) diff --git a/GoogleSignIn/Sources/GIDAuthorizationFlowProcessor/Implementations/GIDAuthorizationFlowProcessor.m b/GoogleSignIn/Sources/GIDAuthorizationFlowProcessor/Implementations/GIDAuthorizationFlowProcessor.m index 069d9e4b..8ce9ce60 100644 --- a/GoogleSignIn/Sources/GIDAuthorizationFlowProcessor/Implementations/GIDAuthorizationFlowProcessor.m +++ b/GoogleSignIn/Sources/GIDAuthorizationFlowProcessor/Implementations/GIDAuthorizationFlowProcessor.m @@ -17,7 +17,6 @@ #import "GoogleSignIn/Sources/GIDAuthorizationFlowProcessor/Implementations/GIDAuthorizationFlowProcessor.h" #import "GoogleSignIn/Sources/GIDAuthorizationUtil.h" - #import "GoogleSignIn/Sources/GIDSignInInternalOptions.h" #ifdef SWIFT_PACKAGE @@ -31,7 +30,7 @@ @interface GIDAuthorizationFlowProcessor () /// AppAuth external user-agent session state. -@property(nonatomic, nullable)id currentAuthorizationFlow; +@property(nonatomic, nullable) id currentAuthorizationFlow; @end diff --git a/GoogleSignIn/Sources/GIDAuthorizationUtil.h b/GoogleSignIn/Sources/GIDAuthorizationUtil.h index 3ed72237..a93ff3d7 100644 --- a/GoogleSignIn/Sources/GIDAuthorizationUtil.h +++ b/GoogleSignIn/Sources/GIDAuthorizationUtil.h @@ -33,15 +33,17 @@ NS_ASSUME_NONNULL_BEGIN authorizationRequestWithOptions:(GIDSignInInternalOptions *)options emmSupport:(nullable NSString *)emmSupport; -/// Unions two scopes or returns an error if the new scopes are the subset of the existing scopes. +/// Unions granted scopes with new scopes or returns an error if the new scopes are the subset of +/// the granted scopes. /// /// @param scopes The existing scopes. /// @param newScopes The new scopes to add. /// @param error The reference to the error. /// @return The array of all scopes or nil if there is an error. -+ (nullable NSArray *)unionScopes:(NSArray *)scopes - withNewScopes:(NSArray *)newScopes - error:(NSError * __autoreleasing *)error; ++ (nullable NSArray *) + resolvedScopesFromGrantedScoped:(NSArray *)scopes + withNewScopes:(NSArray *)newScopes + error:(NSError * __autoreleasing *)error; @end diff --git a/GoogleSignIn/Sources/GIDAuthorizationUtil.m b/GoogleSignIn/Sources/GIDAuthorizationUtil.m index 05b123a0..fd14632f 100644 --- a/GoogleSignIn/Sources/GIDAuthorizationUtil.m +++ b/GoogleSignIn/Sources/GIDAuthorizationUtil.m @@ -81,9 +81,10 @@ @implementation GIDAuthorizationUtil return request; } -+ (nullable NSArray *)unionScopes:(NSArray *)scopes - withNewScopes:(NSArray *)newScopes - error:(NSError * __autoreleasing *)error { ++ (nullable NSArray *) + resolvedScopesFromGrantedScoped:(NSArray *)scopes + withNewScopes:(NSArray *)newScopes + error:(NSError * __autoreleasing *)error { NSMutableSet *grantedScopes = [NSMutableSet setWithArray:scopes]; NSSet *requestedScopes = [NSSet setWithArray:newScopes]; diff --git a/GoogleSignIn/Sources/GIDSignIn.m b/GoogleSignIn/Sources/GIDSignIn.m index 4a2e09f5..56c43a85 100644 --- a/GoogleSignIn/Sources/GIDSignIn.m +++ b/GoogleSignIn/Sources/GIDSignIn.m @@ -260,9 +260,10 @@ - (void)addScopes:(NSArray *)scopes presentingViewController:(UIViewController *)presentingViewController completion:(nullable GIDSignInCompletion)completion { NSError *error; - NSArray *allScopes = [GIDAuthorizationUtil unionScopes:self.currentUser.grantedScopes - withNewScopes:scopes - error:&error]; + NSArray *allScopes = + [GIDAuthorizationUtil resolvedScopesFromGrantedScoped:self.currentUser.grantedScopes + withNewScopes:scopes + error:&error]; if (error) { if (completion) { dispatch_async(dispatch_get_main_queue(), ^{ diff --git a/GoogleSignIn/Sources/GIDSignIn_Private.h b/GoogleSignIn/Sources/GIDSignIn_Private.h index 0d0cf545..234d7705 100644 --- a/GoogleSignIn/Sources/GIDSignIn_Private.h +++ b/GoogleSignIn/Sources/GIDSignIn_Private.h @@ -34,6 +34,7 @@ NS_ASSUME_NONNULL_BEGIN +/// The EMM support version. extern NSString *const kEMMVersion; /// Represents a completion block that takes a `GIDSignInResult` on success or an error if the diff --git a/GoogleSignIn/Tests/Unit/GIDAuthorizationUtilTest.m b/GoogleSignIn/Tests/Unit/GIDAuthorizationUtilTest.m index 3a4ab1e2..b87770c2 100644 --- a/GoogleSignIn/Tests/Unit/GIDAuthorizationUtilTest.m +++ b/GoogleSignIn/Tests/Unit/GIDAuthorizationUtilTest.m @@ -175,23 +175,25 @@ - (void)testUnionScopes_success { NSArray *newScopes = @[kScopeBirthday]; NSError *error; - NSArray *allScopes = [GIDAuthorizationUtil unionScopes:scopes - withNewScopes:newScopes - error:&error]; + NSArray *allScopes = + [GIDAuthorizationUtil resolvedScopesFromGrantedScoped:scopes + withNewScopes:newScopes + error:&error]; NSArray *expectedScopes = @[kScopeEmail, kScopeProfile, kScopeBirthday]; XCTAssertEqualObjects(allScopes, expectedScopes); XCTAssertNil(error); } -- (void)testUnitScopes_addExistingScopes_error { +- (void)testUnionScopes_addExistingScopes_error { NSArray *scopes = @[kScopeEmail, kScopeProfile, kScopeBirthday]; NSArray *newScopes = @[kScopeBirthday]; NSError *error; - NSArray *allScopes = [GIDAuthorizationUtil unionScopes:scopes - withNewScopes:newScopes - error:&error]; + NSArray *allScopes = + [GIDAuthorizationUtil resolvedScopesFromGrantedScoped:scopes + withNewScopes:newScopes + error:&error]; XCTAssertNil(allScopes); XCTAssertEqualObjects(error.domain, kGIDSignInErrorDomain); diff --git a/GoogleSignIn/Tests/Unit/GIDSignInTest.m b/GoogleSignIn/Tests/Unit/GIDSignInTest.m index 0385c584..1ae41732 100644 --- a/GoogleSignIn/Tests/Unit/GIDSignInTest.m +++ b/GoogleSignIn/Tests/Unit/GIDSignInTest.m @@ -1073,10 +1073,10 @@ - (void)OAuthLoginWithAddScopesFlow:(BOOL)addScopesFlow #if TARGET_OS_IOS || TARGET_OS_MACCATALYST [_signIn signInWithPresentingViewController:_presentingViewController #elif TARGET_OS_OSX - [_signIn signInWithPresentingWindow:_presentingWindow + [_signIn signInWithPresentingWindow:_presentingWindow #endif // TARGET_OS_IOS || TARGET_OS_MACCATALYST - hint:_hint - completion:completion]; + hint:_hint + completion:completion]; } [_authState verify];