-
Notifications
You must be signed in to change notification settings - Fork 205
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix passing nil for nonce and add example #476
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Firstly, apologies for missing this. The changes look good to me!
Samples/Swift/DaysUntilBirthday/Shared/Services/GoogleSignInAuthenticator.swift
Show resolved
Hide resolved
I'm going to merge since integration tests are passing locally. Seems like something is awry in the workflow/GitHub runner... |
@mdmathias do we know in what version it will be available? |
@mdmathias I would also love to see this released. When can we expect it out? It's been one year since my nonce PR in AppAuth openid/AppAuth-iOS#788 was merged. Would really appreciate it out. Thanks so much! |
Hi everyone! Inserting myself in here, and thanks for bringing it to our attention. We're planning to include this in the next minor release, which we expect to roll out in the next month or two. Additionally we have a minor issue in the workflow/GitHub runner that we're addressing, but this should not a blocker. Hope this brings some clarity! |
This PR fixes an issue introduced by #402 related to AppAuth's use of a
nonce
, fixes an issue in GSI's-[GIDSignIn maybeFetchToken:]
, and provides an example in the DaysUntilBirthday app.Issue in #402
#402 provides the ability to pass a manual nonce via GSI to AppAuth (e.g., via this OIDAuthorizationRequest initializer that takes a
nonce
parameter).Unfortunately, this addition to GSI breaks AppAuth's automatic use of its own
nonce
if one is not supplied since #402 will always pass through -[GIDSignInOptions nonce]. So, if thenonce
inGIDSignInOptions
is nil, then nil will be passed to AppAuth for thenonce
, and therefore AppAuth will not produce its ownnonce
via this initializer.Fixing
-[GIDSignIn maybeFetchToken:]
-maybeFetchToken
previously used a method on AppAuth (+[OIDAuthorizationService performTokenRequest:callback:]
)that would pass nil for the last authorization response. The consequence of this is that the last authorization response's
nonce
will not be available for comparison within AppAuth since the last auth response and its nonce would be nil in the comparison.This PR fixes the issue by always passing along the last authorization response in
-[GIDSignIn maybeFetchToken:]
.DaysUntilBirthdayExample
This PR also provides an example of the client passing in a manual
nonce
, and then comparing the receivednonce
in the authorization response to the manualnonce
. It provides a simple example that clients may use to develop their own solution for comparing their manualnonce
to the authorization's returnednonce