next-auth icon indicating copy to clipboard operation
next-auth copied to clipboard

fix: Add features missing for Apple provider

Open ChrGrb opened this issue 2 years ago โ€ข 11 comments

โ˜•๏ธ Reasoning

Changes to routes/callback.ts

  • If the response_mode of authorization is set to 'form_post', the information will be returned in the body and not the query. Therefor the callback handler should check for the information there to pass it on to the handleOAuth function

Changes to providers/oauth.ts

  • The Apple provider returns the user information with the authorization response. There should be a way to get to this data in the provider configuration

Changes to providers/apple.ts

  • The apple token endpoint expects authentication data to be passed in the body. Therefor the token_endpoint_auth_method has to be set to 'client_secret_post'
  • wellKnown endpoint should be defined in the default provider
  • authorization url should be defined in the default provider
  • authorization params should be set correctly
  • token url should be defined in the default provider
  • The profile function should correctly handle the available data
  • The documentation should mention the necessary pkceCodeVerifier cookie

๐Ÿงข Checklist

  • [x] Documentation
  • [ ] Tests
  • [ ] Ready to be merged

๐ŸŽซ Affected issues

Fixes:

๐Ÿ“Œ Resources

ChrGrb avatar Jul 31 '23 18:07 ChrGrb

The latest updates on your projects. Learn more about Vercel for Git โ†—๏ธŽ

Name Status Preview Comments Updated (UTC)
auth-docs โŒ Failed (Inspect) Aug 8, 2023 4:28pm
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
next-auth-docs โฌœ๏ธ Ignored (Inspect) Visit Preview Aug 8, 2023 4:28pm

vercel[bot] avatar Jul 31 '23 18:07 vercel[bot]

@ChrGrb is attempting to deploy a commit to the authjs Team on Vercel.

A member of the Team first needs to authorize it.

vercel[bot] avatar Jul 31 '23 18:07 vercel[bot]

I'm hesitant about merging this yet, as we don't want to make exceptions for handling providers specifically in the core library. Apple is making this hard here, not adhering to the OAuth spec... User information should be in id_token, and not part of a query param. :grimacing:

I recently figured out how to finally test an Apple provider (had to pay for it :-1:) myself, so I'll think about it a bit more.

*Venting*: Apple really does not want people to use their provider. :sweat_smile:. By far the most convoluted/worst documented provider, and not even free...

balazsorban44 avatar Aug 03 '23 14:08 balazsorban44

I 100% agree with what you say. Figuring out what is going wrong and why is a nightmare with the Apple provider and changing the core did feel off.

I did have 2 possible alternatives to this, that could be considered:

Make the provider usable with the current core functionality By changing the authorization params to

scope: "",
response_mode: "query",

the response is returned in the url, instead of the body. This would limit the functionality in so far that the call does neither provide an email, nor a name. But the general sign in works and the id_token is still returned. The email and name could then be requested and stored in the database on first signIn.

Add the ability to provide a custom handleOAuth function in the provider configuration This would move the bulk of changes to the Apple provider configuration (except for determining whether the data of the callback is in the body or query). On the other hand I assume that you have a good reason, why this is not yet an implemented feature.

Either way, if you need any assistance or get stuck, just hit me up. I know how hard it is to find documentation or information on this topic.

This article highlights some of the differences between OAuth2 and the Apple version pretty well. It also explains, why the pkce cookie does not arrive back with the callback as expected: https://www.bscotch.net/post/sign-in-with-apple-implementation-hurdles

ChrGrb avatar Aug 03 '23 14:08 ChrGrb

Doing some more digging I found that Apple actually does adhere to the OIDC spec. This is the actual, expected behaviour for an OAuth2.0 form_post request. It is just that apparently no other provider in Auth.js uses this. https://openid.net/specs/oauth-v2-form-post-response-mode-1_0.html#FormPostResponseExample

This does, in my opinion, justify changing the core. It is practically speaking not changing it for a single provider, but instead for all potential future providers using the same form_post spec.

ChrGrb avatar Aug 03 '23 22:08 ChrGrb

My ick is with specifically returning the user data in a user search param as s stringified JSON objoect. That is not part of the spec AFAICT.

balazsorban44 avatar Aug 04 '23 12:08 balazsorban44

(closed by accident, sorry).

Add the ability to provide a custom handleOAuth function in the provider configuration

I'm thinking of something similar.

See https://github.com/nextauthjs/next-auth/blob/60c5037ee1dd7b11f180e83de74b66aab88d4c47/packages/core/src/lib/oauth/callback.ts#L111-L115 Here, if a provider is not adhering to the spec, we let them call into this .conform method, signaling that this is really on the provider and not on us. Also warning for the users so they can let us know when a provider finally fixed their stuff.

An example is Twitch:

https://github.com/nextauthjs/next-auth/blob/60c5037ee1dd7b11f180e83de74b66aab88d4c47/packages/core/src/providers/twitch.ts#L88-L115

balazsorban44 avatar Aug 04 '23 12:08 balazsorban44

I tried my best to implement your idea. I could not figure out a way on how to do it with only the existing conform methods, as the authorization.conform is afaict not being used.

What I did instead is create a provider.profileConform(profile, query) method that does, what the changes to the provider.profile(..) callback have done before. I feel like this is more readable, but in the end it still comes down to basically the same changes to the core.

Additionally I added the provider.token.url back in. While it is fetched from the issuer, Apple does not provide a userinfo endpoint, which then leads to an error. Defining it skips the discovery phase and the callback succeeds.

ChrGrb avatar Aug 08 '23 16:08 ChrGrb

May I ask if there's any progress in merging this PR?

I'm currently using the fallback suggested here but with that the email is reported as null since it can't be part of the scope.

hesselbom avatar Feb 15 '24 14:02 hesselbom

I can successfully sign into Apple, but this issue continues to be a problem for me since the failure to be able to get an email returned from Apple on a Vercel implementation makes the Apple provier unusable for my purposes.

After digging into it, it looks like the fix should be pretty simple: since Apple returns the state in the body of a POST (which one would expect for a POST, and is frankly better security practice) rather than as a GET query parameter (which is what the state check is looking for), the following approach should address the problem:

  1. In lib/actions/callback/index.ts:
      const { proxyRedirect, randomState } = handleState(
        query,
        body,
        provider,
        options.isOnRedirectProxy
      )

  1. In the handleState function in lib/actions/callback/oauth/checks.ts, the state can taken from query or body, depending on which is non falsy, and proceed accordingly.

I'd try to fix this myself, except I've never tried to submit a pull request and I'm on a tight time deadline on this project and the other providers (Google, Facebook, Auth0, and Github) all work fine so I can proceed without Apple. But given the ubiquity of iPhones, it would be great if Apple would return an email address in a Vercel implementation.

Love this library, and appreciate all of the hard work that has gone into what is an elegant solution to a tough problem.

Cheers, L3

leo3linbeck avatar Mar 10 '24 22:03 leo3linbeck

We've hit this issue ourselves and, short of modifying the core which I don't feel qualified to do, it doesn't seem we have any recourse on this other than to hope this PR exposing the form data vs query approach gets merged.

We'd really appreciate if this could keep moving forward ๐Ÿ™

tristan-warner-smith avatar Jun 27 '24 17:06 tristan-warner-smith