Manual nonce generation option #244

toshi0383 · 2022-10-27T01:19:24Z

Fixes: #135
Also partly fixes: #28

google-cla · 2022-10-27T01:19:28Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

toshi0383 · 2022-10-27T01:40:11Z

@jsamol Could you sign the CLA? I cherry-picked your commit from this repository.

jsamol · 2022-10-27T07:32:50Z

@toshi0383 done

theniceboy · 2023-01-24T20:05:15Z

@toshi0383 Any updates on this?

toshi0383 · 2023-01-25T00:07:16Z

Oh I should resolve conflict. Will do. Thanks for the heads-up!

Conflicts: GoogleSignIn/Sources/GIDSignIn.m

toshi0383 · 2023-01-25T11:53:03Z

Fixed conflict. Confirmed that build succeeds.
Could somebody please give a proper review for this? Maybe @petea ?

theniceboy · 2023-01-25T21:56:44Z

Will this be unblocking the progress on implementing native sign in (#5)?

Thanks!

elliottetzkorn · 2023-04-24T22:45:02Z

@toshi0383 @petea any chance this could be prioritized?

toshi0383 · 2023-04-25T01:36:59Z

@petea Any change merging this?
Let me know if any further action required.

elliottetzkorn · 2023-04-29T07:13:16Z

@petea bump

gregorym · 2023-05-01T05:57:18Z

@toshi0383, @ch40w31 any chance someone can take a look? it's blocking supabase/supabase-flutter#5

toshi0383 · 2023-05-01T06:08:28Z

Please do not mention to me. I don't have permission to merge in this repository.

elliottetzkorn · 2023-05-05T05:53:54Z

@ch40w31 or @mdmathias would either of you be willing to review this?

mdmathias · 2023-06-01T22:31:29Z

Apologies for the delay and thanks for the PR!

I added a comment to the issue with a request for a little more information. In principle, this change should be fine.

mdmathias · 2023-06-01T22:33:01Z

GoogleSignIn/Sources/GIDSignIn.m

@@ -269,6 +269,7 @@ - (void)signInWithPresentingViewController:(UIViewController *)presentingViewCon
 - (void)addScopes:(NSArray<NSString *> *)scopes
    presentingViewController:(UIViewController *)presentingViewController
                  completion:(nullable GIDSignInCompletion)completion {
+


Remove space.

mdmathias · 2023-06-01T22:33:24Z

GoogleSignIn/Sources/GIDSignIn.m

-      [GIDEMMSupport parametersWithParameters:options.extraParams
-                                   emmSupport:emmSupport
-                       isPasscodeInfoRequired:NO]];
+  [GIDEMMSupport parametersWithParameters:options.extraParams


Fix indentation.

mdmathias · 2023-06-01T22:41:41Z

GoogleSignIn/Sources/GIDSignIn.m

 #elif TARGET_OS_OSX || TARGET_OS_MACCATALYST
  [additionalParameters addEntriesFromDictionary:options.extraParams];
 #endif // TARGET_OS_OSX || TARGET_OS_MACCATALYST
  additionalParameters[kSDKVersionLoggingParameter] = GIDVersion();
  additionalParameters[kEnvironmentLoggingParameter] = GIDEnvironment();

+  NSString *codeVerifier = [OIDAuthorizationRequest generateCodeVerifier];


This part is a little tricky for me because it looks like GSI is taking on some of the implementation details within AppAuth-iOS. IIUC, we have to create codeVerifier, codeChallenge, and the nonce because we don't have an initializer that we can easily call on OIDAuthorizationRequest that takes only the nonce.

Ideally, we would have an initializer on OIDAuthorizationRequest that only adds the nonce parameter to the simplest initializer. That way, we can keep these implementation details/default values inside AppAuth.

What do you think?

mdmathias · 2023-06-01T22:49:47Z

GoogleSignIn/Tests/Unit/GIDSignInTest.m

@@ -674,10 +675,12 @@ - (void)testOAuthLogin_LoginHint {
 }



I know that GIDSignInTest is cluttered and that GIDSignIn is a difficult class to test (we're working on this), but could you add a test that verifies the passed in nonce is matched in the OIDAuthorizationResponse we back in GIDSignIn?

It'd be great to have a unit test that asserts: XCTAssertEqualObjects(originalNonce, authorizationResponse.request.nonce).

elliottetzkorn · 2023-06-01T22:55:05Z

@toshi0383 would you like to address these changes? I am also happy to contribute to this PR.

toshi0383 · 2023-06-02T01:03:02Z

@elliottetzkorn Sounds great! I’ve been busy lately.
Please go ahead and take over.

ozasadnyy · 2023-07-24T18:10:18Z

Hello @elliottetzkorn, did you have a change to take a look to this PR?

hf · 2023-08-18T17:19:19Z

Hey @elliottetzkorn, I'm on the Supabase Auth team and this fix would greatly improve the Sign in with Google experience in mobile apps while remaining as secure as the OIDC spec intends.

I would kindly ask for an expedited review.

cc @mdmathias

singhalvipul53 · 2023-09-11T11:01:20Z

Hello,
Is there no way to add google login in react-native IOS app until google fixes the issue..??

azlekov · 2023-09-12T11:34:28Z

Hello,
Is there no way to add google login in react-native IOS app until google fixes the issue..??

Asking the same!

And is there any timeline where nonce support will be available?

vonovak · 2023-09-12T12:03:44Z

Hello, please be so kind and stop spamming the thread. Me (and others) subscribed to this thread to receive valuable updates, not questions such as "when is this going to happen" or "me too" - please be mindful of others and when asking a question, think about whether it really contributes something valuable to the discussion, thank you.

To answer the question: this is open source. A feature will be available, when someone manages to open a PR which is reviewed and merged by the maintainers. That someone could be you, or someone else (could be me, if someone hires me). Until then, there are no updates. So please either wait or write the code yourself.
Thank you for your understanding 👍 .

…n ODIC flow (#1264) It appears that in certain client libraries that deal with the OIDC authentication flow, such as [this one for React Native on iOS](google/GoogleSignIn-iOS#244), the clients are unable to extract the nonce that is generated randomly by the library. This option allows to temporarily drop the enforcement at the GoTrue level when performing the OIDC flow. This does remove an important security barrier, which could potentially allow "stolen" ID tokens to be used on third-party services (that have opted in to this configuration) however in the interest of flexibility and broad platform support the option is being added.

…n ODIC flow (supabase#1264) It appears that in certain client libraries that deal with the OIDC authentication flow, such as [this one for React Native on iOS](google/GoogleSignIn-iOS#244), the clients are unable to extract the nonce that is generated randomly by the library. This option allows to temporarily drop the enforcement at the GoTrue level when performing the OIDC flow. This does remove an important security barrier, which could potentially allow "stolen" ID tokens to be used on third-party services (that have opted in to this configuration) however in the interest of flexibility and broad platform support the option is being added.

GhostWalker562 · 2024-03-29T09:08:58Z

Seems like custom nonce was released 3 weeks ago https://github.com/openid/AppAuth-iOS/releases/tag/1.7.0 , are we able to revisit this PR?

vonovak · 2024-03-29T12:23:43Z

Hello,
I'm the author of the custom nonce functionality that was merged and released in AppAuth.
I'm planning to add this feature to Google sign in SDK as well.

However, I don't have a precise timeline for this. My estimate is that I will have time to do it in 2 months. If anyone wants to support me in prioritizing it, please consider sponsoring me on GH sponsors, or get in touch.
Alternatively, someone else can do it before me, of course.

I will edit this comment once I start working on it, so that there's no duplication of effort.

SunilKividor · 2024-04-13T17:51:47Z

can anyone help me adding the nonce parameter in the google sign in package . I really need to send a nonce in the parameter while signing in. I am ready to fork and make changes.

vonovak · 2024-04-28T13:44:44Z

I have opened a PR to support this: #402

RedBird96 · 2024-06-04T18:21:07Z

So when this PR will be merged?

Aleks86 · 2024-06-20T21:33:04Z

Eagerly looking for merge!

toshi0383 · 2024-09-13T02:51:35Z

Superseded by #402 .
Huge shout-out for @vonovak thank you!

…n ODIC flow (supabase#1264) It appears that in certain client libraries that deal with the OIDC authentication flow, such as [this one for React Native on iOS](google/GoogleSignIn-iOS#244), the clients are unable to extract the nonce that is generated randomly by the library. This option allows to temporarily drop the enforcement at the GoTrue level when performing the OIDC flow. This does remove an important security barrier, which could potentially allow "stolen" ID tokens to be used on third-party services (that have opted in to this configuration) however in the interest of flexibility and broad platform support the option is being added.

jsamol and others added 2 commits October 25, 2022 13:16

feat(sign-in): configure sign in request with external nonce value

a593b4f

fix: compile

d0cf7cd

toshi0383 mentioned this pull request Oct 27, 2022

How to pass in nonce? #135

Closed

Merge branch 'main' into main

6673687

toshi0383 added 2 commits January 25, 2023 20:33

Merge remote-tracking branch 'upstream/main'

b0543cd

Conflicts: GoogleSignIn/Sources/GIDSignIn.m

Fix conflicts

76acf9c

toshi0383 force-pushed the main branch from ec2dddb to 76acf9c Compare January 25, 2023 11:50

DanMossa mentioned this pull request Mar 13, 2023

Full Native Auth support supabase/supabase-flutter#399

Closed

5 tasks

elliottetzkorn mentioned this pull request May 1, 2023

Native Sign In supabase/supabase-flutter#5

Closed

mansueli approved these changes May 22, 2023

View reviewed changes

gregorym approved these changes May 22, 2023

View reviewed changes

mdmathias reviewed Jun 1, 2023

View reviewed changes

trongtindev mentioned this pull request Jun 22, 2023

Added signInWithCredential method to perform native supabase/supabase-flutter#518

Closed

nick-barth mentioned this pull request Aug 1, 2023

[BUG]: Supabase does not work with React Native Google signin supabase/auth#1205

Closed

2 tasks

vonovak mentioned this pull request Sep 20, 2023

feat: allow custom nonce in OIDAuthorizationRequest openid/AppAuth-iOS#788

Merged

hf mentioned this pull request Oct 3, 2023

feat: add GOTRUE_<PROVIDER>_SKIP_NONCE_CHECK to skip nonce checks in ODIC flow supabase/auth#1264

Merged

vonovak mentioned this pull request Apr 28, 2024

feat: allow providing custom nonce #402

Merged

toshi0383 closed this Sep 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Manual nonce generation option #244

Manual nonce generation option #244

toshi0383 commented Oct 27, 2022

google-cla bot commented Oct 27, 2022

toshi0383 commented Oct 27, 2022

jsamol commented Oct 27, 2022

theniceboy commented Jan 24, 2023

toshi0383 commented Jan 25, 2023

toshi0383 commented Jan 25, 2023

theniceboy commented Jan 25, 2023

elliottetzkorn commented Apr 24, 2023

toshi0383 commented Apr 25, 2023

elliottetzkorn commented Apr 29, 2023

gregorym commented May 1, 2023 •

edited

Loading

toshi0383 commented May 1, 2023

elliottetzkorn commented May 5, 2023

mdmathias commented Jun 1, 2023

mdmathias Jun 1, 2023

mdmathias Jun 1, 2023

mdmathias Jun 1, 2023

mdmathias Jun 1, 2023

elliottetzkorn commented Jun 1, 2023

toshi0383 commented Jun 2, 2023

ozasadnyy commented Jul 24, 2023

hf commented Aug 18, 2023 •

edited

Loading

singhalvipul53 commented Sep 11, 2023

azlekov commented Sep 12, 2023

vonovak commented Sep 12, 2023

GhostWalker562 commented Mar 29, 2024

vonovak commented Mar 29, 2024 •

edited

Loading

SunilKividor commented Apr 13, 2024

vonovak commented Apr 28, 2024

RedBird96 commented Jun 4, 2024

Aleks86 commented Jun 20, 2024

toshi0383 commented Sep 13, 2024

Manual nonce generation option #244

Manual nonce generation option #244

Conversation

toshi0383 commented Oct 27, 2022

google-cla bot commented Oct 27, 2022

toshi0383 commented Oct 27, 2022

jsamol commented Oct 27, 2022

theniceboy commented Jan 24, 2023

toshi0383 commented Jan 25, 2023

toshi0383 commented Jan 25, 2023

theniceboy commented Jan 25, 2023

elliottetzkorn commented Apr 24, 2023

toshi0383 commented Apr 25, 2023

elliottetzkorn commented Apr 29, 2023

gregorym commented May 1, 2023 • edited Loading

toshi0383 commented May 1, 2023

elliottetzkorn commented May 5, 2023

mdmathias commented Jun 1, 2023

mdmathias Jun 1, 2023

Choose a reason for hiding this comment

mdmathias Jun 1, 2023

Choose a reason for hiding this comment

mdmathias Jun 1, 2023

Choose a reason for hiding this comment

mdmathias Jun 1, 2023

Choose a reason for hiding this comment

elliottetzkorn commented Jun 1, 2023

toshi0383 commented Jun 2, 2023

ozasadnyy commented Jul 24, 2023

hf commented Aug 18, 2023 • edited Loading

singhalvipul53 commented Sep 11, 2023

azlekov commented Sep 12, 2023

vonovak commented Sep 12, 2023

GhostWalker562 commented Mar 29, 2024

vonovak commented Mar 29, 2024 • edited Loading

SunilKividor commented Apr 13, 2024

vonovak commented Apr 28, 2024

RedBird96 commented Jun 4, 2024

Aleks86 commented Jun 20, 2024

toshi0383 commented Sep 13, 2024

gregorym commented May 1, 2023 •

edited

Loading

hf commented Aug 18, 2023 •

edited

Loading

vonovak commented Mar 29, 2024 •

edited

Loading