-
Notifications
You must be signed in to change notification settings - Fork 247
feat: allow providing custom nonce #402
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
Conversation
|
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. |
f81e466 to
699a005
Compare
|
@vonovak did you sign the CLA? I can take a look at the PR after you confirm. |
|
hello @mdmathias yes I did! :)
|
|
hello @mdmathias, would you please re-review the PR? Thank you! 🙂 |
|
Can we get this merged? This issue is blocking my app |
|
Bumpziez |
|
@vonovak could you take a look at the test failures? An example:
|
|
@mdmathias can you re-run the workflows? TY 🙂 |
|
Tests all passing @mdmathias 👀 |
|
@mdmathias may I ask if there's anything blocking this PR from merging? The discussion is resolved and CI is green. |
|
Update to those using Google Sign-In 8.1.0-vwg-eap-1.0.x to fix this issue, you should know that we are getting rid of these pre-releases soon. Please use official release Google Sign-In 9.0.0 instead. |
@brnnmrls Can you please clarify what you mean by that? That version has been published more than half a year ago, and people depend on it. It's also about compatibility with other libraries like firebase, not everybody can upgrade to latest right away. Thank you |
I appreciate you raising this! We recognize that many clients, particularly with dependencies like Firebase, are using 8.1.0-vwg-eap-1.0.0 and may face difficulties upgrading immediately. However, these versions were always temporary pre-releases. As pre-releases, they inherently carried the risk of change or eventual deprecation, even if they included the fix for the |
|
@brnnmrls okay, can you please give a timeline for this?
The release notes didn't say these versions were "temporary"; please document that next time you have these intentions. Personally, I find the idea of publishing something "temporarily" odd but that's for a different discussion. Thank you |
Apologies for the confusion! By "temporary," I meant that, as pre-releases, they inherently carried the risk of being changed, having their APIs evolve, or eventually being deleted. This differs from our official, stable releases. Though, we understand many clients came to rely on these. Yes, we'll do a better job at documenting the lifecycle and potential for deletion of any future pre-releases. As for timeline, we plan to remove these pre-releases from GitHub by July 18th. As far as I know, migrating to Google Sign-In 9.0.0 should largely involve simply updating the dependency. There were no critical API changes in the major release affecting core Sign-In functionality. However, macOS users will need to add If you have further information on how we can help make this a smoother process for your team, please let me know. |
Pre-release software carries the risk of being broken, unstable or changed (that goes for any software) - I agree. But I'm having hard time seeing why software that's out there and depended on would be removed, and to have that happen in 2 weeks after you publish the next stable - I'm sorry but can we make that at least 2 months (pretty please)? In some ecosystems, unpublishing packages isn't possible unless there's a security reason for that. Not that it matters, but I find it anecdotal that it took ~ 22 months since my original PR in AppAuth-iOS for the nonce feature to make into a stable release and now it'll be 2 weeks to migrate. It's not a problem for me, but it could be for a lot of others. Please give more time to migrate. eidt: I hope we're talking about the same thing. Are you talking about removing the release from cocapods? (that's what I'm talking about). You mentioned removing from GitHub. I dont' care about GitHub releases, I care about the cocoapods. Thank you ❤️ |
I hear your concern about the timeline and the impact of removing those pre-releases. We definitely want to find the best transition for everyone, especially given the dependencies involved. I'm going to discuss this directly with my team and will get back to you with an update next week. Thanks for sharing! |
|
Responding with an update! We're delaying the removal of these pre-releases until two months after the 9.0.0 release was made. As a reminder, developers will need to migrate off of these pre-releases by early September.
@vonovak Replying to your edit, the releases will be removed from both GitHub and CocoaPods. Thanks for your help! |
|
Woo, it's an early Christmas! |
|
@brnnmrls thank you, much appreciated! 🙏 |
|
Hey everyone! This is a final reminder that these pre-releases will be removed very soon from GitHub and CocoaPods. If you haven't already, please migrate to the official 9.0.0 release, which is our stable, long-term solution. Thanks! |
|
@brnnmrls would you please consider releasing the previous EAP with custom nonce as a stable version 8.1? Version 9 comes with issues: #547 and openid/AppAuth-iOS#933, which I'm seeing reports of only after switching to v9. These are no public reproducers of those issues, but they are happening and forcing people to downgrade to v8 (without this feature), or use broken v9. Thank you |
|
Hey @joannetsaii . Some interesting observations that we have had :
I find these behaviours very weird and I am not able to link them. But wanted to flag out my findings in case it helps. |
|
@joannetsaii @brnnmrls hello, could you please please release the previous EAP with custom nonce as a stable version 8.1? The issue #547 is now open for more than 2 months with no resolution. People are forced to downgrade to v8 (without custom nonce), or use broken v9. Can we please get that stable released? |

Uses: openid/AppAuth-iOS#788. Motivation is explained there and also in issue #135
Fixes: #135
Supersedes: #244. It takes a slightly different approach where nonce is not provided via GIDConfiguration but via a parameter to
signInWithPresentingViewController/Window