{{ message }}
Repository navigation
Fix passing nil for nonce and add example - #476
Merged
Merged
Conversation
Collaborator
Author
brnnmrls
approved these changes
Sep 17, 2024
vonovak
approved these changes
Sep 17, 2024
vonovak
left a comment
Contributor
There was a problem hiding this comment.
Firstly, apologies for missing this. The changes look good to me!
yalkhder
approved these changes
Oct 1, 2024
Collaborator
Author
|
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? |
Contributor
|
@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! |
Member
2 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

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
nonceparameter).Unfortunately, this addition to GSI breaks AppAuth's automatic use of its own
nonceif one is not supplied since #402 will always pass through -[GIDSignInOptions nonce]. So, if thenonceinGIDSignInOptionsis nil, then nil will be passed to AppAuth for thenonce, and therefore AppAuth will not produce its ownnoncevia this initializer.Fixing
-[GIDSignIn maybeFetchToken:]-maybeFetchTokenpreviously 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
noncewill 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 receivednoncein the authorization response to the manualnonce. It provides a simple example that clients may use to develop their own solution for comparing their manualnonceto the authorization's returnednonce