feat: make nonce handling configurable

[bvogel]

This PR will introduce a individual handling of the nonce validation that is significantly hindered by Apple with using a POST callback.

Added specs, README too.

fixes #102 and
fixes #103

[bvogel]

@nhosoya bump

[bvogel]

@nhosoya bump again

[bvogel]

@nhosoya bump again

[bvogel]

@nhosoya and another bump. Can we please either merge this or get some kind of reaction?

[nov]

don't ignore nonce. at least as a library.

[bvogel]

WHAT!!! This PR exactly enables that nonce won't be ignored, it makes it even possible to use it securely! Please reconsider closing this.

To rephrase: without this PR you either have to make your session insecure or you just can't use this gem. With this PR you at least have a choice between a secure and a stupid option (I may rephrase the README even more, so it's clear that using ignore isn't a sane option) but using a session open to CSFR is even worse.

[bvogel]

@nov @nhosoya Another fun fact, if you check almost all recent forks from this repo - what users do is DISABLE nonce validation, so offering them a secure alternative and make it a choice to them to ignore it is at least miles better than having no option and people just randomly disabling nonce while there is a better option.

TBH I'm very disappointed that this wasn't even discussed or deeper looked into, but rather just closed, which leaves this implementation in a worse state than WITH this PR.

[davismatt]

Hopping on this thread - at the very least we need to evaluate whether we should be calling verify_nonce! if nonce isn't present in the jwt at all.

[bvogel]

@davismatt thank you for hopping on. Unfortunately @nov is very unresponsive and I think this gem is mostly dead, which is a shame. The situation that nonce checking is severely broken is ongoing for quite some time and I thought my proposed solution would be a good balance between security and feasibility, alas it was shot down without further discussion

[davismatt]

I ended up monkey patching the verify method to verify_nonce!(id_token) if id_token[:nonce_supported] && id_token[:nonce].present?. Apple's documentation makes it clear the nonce is an optional value and will not be present in the JWT if not included in the initial configuration. I don't know why this gem treats it like a required field.

[GastonThese]

@bvogel Hello,

I configured nonce: :ignore, and now the "Sign in with Apple" button doesn't work. It fails to access the "users/auth/apple" route. Before I made this configuration, I could access the route and enter my username and password successfully, but it was failing at the callback.

The error I'm getting is:
E, [2023-10-26T11:23:00.952628 #297744] ERROR -- omniauth: (apple) Authentication failure! no implicit conversion of nil into Hash: TypeError, no implicit conversion of nil into Hash
:/

---

this monkypatch work

def authorize_params
  super if options[:nonce] == :ignore

  super.merge(nonce: new_nonce)
end 

[bvogel]

will look into this next week, sorry about that. However, I strongly suggest to not use ignore but prefer the new local option I introduced (and I know will work) which will maintain nonce safety

[GastonThese]

@bvogel
In the end, it didn't work out. I was able to obtain the data from Apple, and indeed, in the callback controller, I could create the user. However, when I use sign_in_and_redirect from Devise, it doesn't work. The user isn't logged in. While debugging, I can see that the current user is created and more, but in the redirect, it says that I need to log in.

One thing I noticed is that the Google Omniauth session has different data compared to this one :/

I also tried configuring it in :local, but it didn't work.

Is there any example or tutorial on how to implement this gem? Thanks for your response :)

[bvogel]

@GastonThese if you get the data correctly from Apple and are able to create the user than the job of this gem is done. If you aren't logged in that's something that you'll have to check with the general devise/omniauth integration, as all this gem does is provide the necessary authentication and user info from Apple, how you proceed from there isn't part of this gem.

[bvogel]

@GastonThese I fixed the error in the parameter handling. See #111