From 20da986c25cfef84282c674a8e020cc8fd698d60 Mon Sep 17 00:00:00 2001 From: Matt Mathias Date: Fri, 13 Sep 2024 12:44:34 -0700 Subject: [PATCH 1/5] Fix passing nil for nonce and add example --- GoogleSignIn/Sources/GIDSignIn.m | 32 +++++++---- .../Services/GoogleSignInAuthenticator.swift | 55 ++++++++++++++++++- 2 files changed, 74 insertions(+), 13 deletions(-) diff --git a/GoogleSignIn/Sources/GIDSignIn.m b/GoogleSignIn/Sources/GIDSignIn.m index de7b6681..f924d387 100644 --- a/GoogleSignIn/Sources/GIDSignIn.m +++ b/GoogleSignIn/Sources/GIDSignIn.m @@ -726,14 +726,23 @@ - (void)authorizationRequestWithOptions:(GIDSignInInternalOptions *)options comp - (OIDAuthorizationRequest *) authorizationRequestWithOptions:(GIDSignInInternalOptions *)options additionalParameters:(NSDictionary *)additionalParameters { - OIDAuthorizationRequest *request = - [[OIDAuthorizationRequest alloc] initWithConfiguration:_appAuthConfiguration - clientId:options.configuration.clientID - scopes:options.scopes - redirectURL:[self redirectURLWithOptions:options] - responseType:OIDResponseTypeCode - nonce:options.nonce - additionalParameters:additionalParameters]; + OIDAuthorizationRequest *request; + if (options.nonce) { + request = [[OIDAuthorizationRequest alloc] initWithConfiguration:_appAuthConfiguration + clientId:options.configuration.clientID + scopes:options.scopes + redirectURL:[self redirectURLWithOptions:options] + responseType:OIDResponseTypeCode + nonce:options.nonce + additionalParameters:additionalParameters]; + } else { + request = [[OIDAuthorizationRequest alloc] initWithConfiguration:_appAuthConfiguration + clientId:options.configuration.clientID + scopes:options.scopes + redirectURL:[self redirectURLWithOptions:options] + responseType:OIDResponseTypeCode + additionalParameters:additionalParameters]; + } return request; } @@ -918,9 +927,10 @@ - (void)maybeFetchToken:(GIDAuthFlow *)authFlow { [authFlow wait]; [OIDAuthorizationService - performTokenRequest:tokenRequest - callback:^(OIDTokenResponse *_Nullable tokenResponse, - NSError *_Nullable error) { + performTokenRequest:tokenRequest + originalAuthorizationResponse:authFlow.authState.lastAuthorizationResponse + callback:^(OIDTokenResponse *_Nullable tokenResponse, + NSError *_Nullable error) { [authState updateWithTokenResponse:tokenResponse error:error]; authFlow.error = error; diff --git a/Samples/Swift/DaysUntilBirthday/Shared/Services/GoogleSignInAuthenticator.swift b/Samples/Swift/DaysUntilBirthday/Shared/Services/GoogleSignInAuthenticator.swift index deacb35b..24da109c 100644 --- a/Samples/Swift/DaysUntilBirthday/Shared/Services/GoogleSignInAuthenticator.swift +++ b/Samples/Swift/DaysUntilBirthday/Shared/Services/GoogleSignInAuthenticator.swift @@ -35,12 +35,26 @@ final class GoogleSignInAuthenticator: ObservableObject { print("There is no root view controller!") return } - - GIDSignIn.sharedInstance.signIn(withPresenting: rootViewController) { signInResult, error in + let manualNonce = UUID().uuidString + + GIDSignIn.sharedInstance.signIn( + withPresenting: rootViewController, + hint: nil, + additionalScopes: nil, + nonce: manualNonce + ) { signInResult, error in guard let signInResult = signInResult else { print("Error! \(String(describing: error))") return } + + // Per OpenID Connect Core section 3.1.3.7, rule #11, compare returned nonce to manual + guard let idToken = signInResult.user.idToken?.tokenString, + let returnedNonce = self.decodeNonce(fromJWT: idToken), + returnedNonce == manualNonce else { + print("ERROR: Returned nonce doesn't match manual nonce!") + return + } self.authViewModel.state = .signedIn(signInResult.user) } @@ -125,3 +139,40 @@ final class GoogleSignInAuthenticator: ObservableObject { } } + +// MARK: Parse nonce from JWT ID Token + +private extension GoogleSignInAuthenticator { + func decodeNonce(fromJWT jwt: String) -> String? { + let segments = jwt.components(separatedBy: ".") + guard let parts = decodeJWTSegment(segments[1]), + let nonce = parts["nonce"] as? String else { + return nil + } + return nonce + } + + func decodeJWTSegment(_ segment: String) -> [String: Any]? { + guard let segmentData = base64UrlDecode(segment), + let segmentJSON = try? JSONSerialization.jsonObject(with: segmentData, options: []), + let payload = segmentJSON as? [String: Any] else { + return nil + } + return payload + } + + func base64UrlDecode(_ value: String) -> Data? { + var base64 = value + .replacingOccurrences(of: "-", with: "+") + .replacingOccurrences(of: "_", with: "/") + + let length = Double(base64.lengthOfBytes(using: String.Encoding.utf8)) + let requiredLength = 4 * ceil(length / 4.0) + let paddingLength = requiredLength - length + if paddingLength > 0 { + let padding = "".padding(toLength: Int(paddingLength), withPad: "=", startingAt: 0) + base64 = base64 + padding + } + return Data(base64Encoded: base64, options: .ignoreUnknownCharacters) + } +} From 05b8ac96d1a2ba6d2bdc468d49392888ab41576f Mon Sep 17 00:00:00 2001 From: Matt Mathias Date: Fri, 13 Sep 2024 14:01:44 -0700 Subject: [PATCH 2/5] Update print message in sample to be assertionFailure --- .../Shared/Services/GoogleSignInAuthenticator.swift | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Samples/Swift/DaysUntilBirthday/Shared/Services/GoogleSignInAuthenticator.swift b/Samples/Swift/DaysUntilBirthday/Shared/Services/GoogleSignInAuthenticator.swift index 24da109c..a383705f 100644 --- a/Samples/Swift/DaysUntilBirthday/Shared/Services/GoogleSignInAuthenticator.swift +++ b/Samples/Swift/DaysUntilBirthday/Shared/Services/GoogleSignInAuthenticator.swift @@ -52,7 +52,9 @@ final class GoogleSignInAuthenticator: ObservableObject { guard let idToken = signInResult.user.idToken?.tokenString, let returnedNonce = self.decodeNonce(fromJWT: idToken), returnedNonce == manualNonce else { - print("ERROR: Returned nonce doesn't match manual nonce!") + // Assert a failure for convenience so that integration tests with thissample app fail upon + // `nonce` mismatch + assertionFailure("ERROR: Returned nonce doesn't match manual nonce!") return } self.authViewModel.state = .signedIn(signInResult.user) From 3a58e5e1324c62e442cb1287d189f8ab8e9aa101 Mon Sep 17 00:00:00 2001 From: Matt Mathias Date: Fri, 13 Sep 2024 14:03:11 -0700 Subject: [PATCH 3/5] Add missing space in comment --- .../Shared/Services/GoogleSignInAuthenticator.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Samples/Swift/DaysUntilBirthday/Shared/Services/GoogleSignInAuthenticator.swift b/Samples/Swift/DaysUntilBirthday/Shared/Services/GoogleSignInAuthenticator.swift index a383705f..de227f08 100644 --- a/Samples/Swift/DaysUntilBirthday/Shared/Services/GoogleSignInAuthenticator.swift +++ b/Samples/Swift/DaysUntilBirthday/Shared/Services/GoogleSignInAuthenticator.swift @@ -52,7 +52,7 @@ final class GoogleSignInAuthenticator: ObservableObject { guard let idToken = signInResult.user.idToken?.tokenString, let returnedNonce = self.decodeNonce(fromJWT: idToken), returnedNonce == manualNonce else { - // Assert a failure for convenience so that integration tests with thissample app fail upon + // Assert a failure for convenience so that integration tests with this sample app fail upon // `nonce` mismatch assertionFailure("ERROR: Returned nonce doesn't match manual nonce!") return From 030294edc0873340d0f39bd132bce83f55b61826 Mon Sep 17 00:00:00 2001 From: Matt Mathias Date: Fri, 13 Sep 2024 18:52:37 -0700 Subject: [PATCH 4/5] Fix GIDSignInTest to stub correct token request method --- GoogleSignIn/Tests/Unit/GIDSignInTest.m | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/GoogleSignIn/Tests/Unit/GIDSignInTest.m b/GoogleSignIn/Tests/Unit/GIDSignInTest.m index 6bdd19d1..7d5195d8 100644 --- a/GoogleSignIn/Tests/Unit/GIDSignInTest.m +++ b/GoogleSignIn/Tests/Unit/GIDSignInTest.m @@ -297,7 +297,7 @@ - (void)setUp { #elif TARGET_OS_OSX _presentingWindow = OCMStrictClassMock([NSWindow class]); #endif // TARGET_OS_IOS || TARGET_OS_MACCATALYST - _authState = OCMStrictClassMock([OIDAuthState class]); + _authState = OCMClassMock([OIDAuthState class]); OCMStub([_authState alloc]).andReturn(_authState); OCMStub([_authState initWithAuthorizationResponse:OCMOCK_ANY]).andReturn(_authState); _tokenResponse = OCMStrictClassMock([OIDTokenResponse class]); @@ -327,7 +327,8 @@ - (void)setUp { callback:COPY_TO_ARG_BLOCK(self->_savedAuthorizationCallback)]); OCMStub([self->_oidAuthorizationService performTokenRequest:SAVE_TO_ARG_BLOCK(self->_savedTokenRequest) - callback:COPY_TO_ARG_BLOCK(self->_savedTokenCallback)]); + originalAuthorizationResponse:[OCMArg any] + callback:COPY_TO_ARG_BLOCK(self->_savedTokenCallback)]); // Fakes _fetcherService = [[GIDFakeFetcherService alloc] init]; From 7d6dbb83e41a7e8d0f96de0783438372fe816ba6 Mon Sep 17 00:00:00 2001 From: Matt Mathias Date: Thu, 3 Oct 2024 11:06:23 -0700 Subject: [PATCH 5/5] Update identation --- GoogleSignIn/Sources/GIDSignIn.m | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/GoogleSignIn/Sources/GIDSignIn.m b/GoogleSignIn/Sources/GIDSignIn.m index f924d387..a6a2953a 100644 --- a/GoogleSignIn/Sources/GIDSignIn.m +++ b/GoogleSignIn/Sources/GIDSignIn.m @@ -926,11 +926,10 @@ - (void)maybeFetchToken:(GIDAuthFlow *)authFlow { } [authFlow wait]; - [OIDAuthorizationService - performTokenRequest:tokenRequest - originalAuthorizationResponse:authFlow.authState.lastAuthorizationResponse - callback:^(OIDTokenResponse *_Nullable tokenResponse, - NSError *_Nullable error) { + [OIDAuthorizationService performTokenRequest:tokenRequest + originalAuthorizationResponse:authFlow.authState.lastAuthorizationResponse + callback:^(OIDTokenResponse *_Nullable tokenResponse, + NSError *_Nullable error) { [authState updateWithTokenResponse:tokenResponse error:error]; authFlow.error = error;