improvement: add apple strategy

[miguel-s]

Improvement

Adds a new auth strategy for Sign in with Apple. Based on the Oidc strategy.

Many thanks to kurmetaubanov for laying the groundwork.

Discussion

Sign in with Apple is based on OpenID Connect but has a couple of unique things, because it's Apple.

Apple and Assent require a few new config fields:

• team_id
• private_key_id
• private_key_path

To reuse as much of the implementation of the Oidc/Oauth2, I added these new fields where required:

• lib/ash_authentication/strategies/oauth2.ex
• lib/ash_authentication/strategies/oauth2/plug.ex
• .formatter.exs

Not sure this is the right approach, as these fields are very Apple specific, wdyt?

Issues

• When adding the confirmation add-on for the email field the confirmation email is sent on every signin/register. Not sure if this is an issue with just the Apple strategy or also affects other oauth strategies.

Todo

• [x] Implementation
• [ ] Tests
• [ ] Documentation

[zachdaniel]

This looks great! I think the issue w/ sending emails each time is related to the fact that an upsert on user is done on sign_in. In your sender, you'd probably want to check if created_at == updated_at and avoid sending an email in that case.

[miguel-s]

Never thought of checking in the sender, will try it out 👍 Is it possible to not update the email in the upsert if it's exactly the same, which would then hopefully not trigger the confirm addon?

Another (maybe) issue I forgot to mention:

Apple sends a POST request with a form body to the oauth callback. The usual :browser pipeline doesn't like this, as the :protect_from_forgery plug will look for a csrf token and send back a 403 forbidden response.

I had to create a new pipeline without that plug, something like this

pipeline :oauth do
  plug :accepts, ["html"]
  plug :fetch_session
  plug :fetch_live_flash
  plug :put_secure_browser_headers
end

And move the ash_authentication auth routes to this new pipeline

scope "/", MyAppWeb do
  pipe_through :oauth
  auth_routes_for MyAppMa.Accounts.User, to: AuthController
end

As far as I understand ash_authentication and Assent already handle forgery protection for the Apple strategy and other strategies send GET requests anyways. Is this ok?

[zachdaniel]

I'll leave that final question for @jimsynz, but AFAIK that would be handled by Assent so might be best to check there.

[zachdaniel]

Linking this here so we remember to merge this after: https://github.com/team-alembic/ash_authentication_phoenix/pull/482

[jimsynz]

Hi @miguel-s 👋

This is awesome work. I'm not sure why CI hasn't been run - but presuming all of that passes then I'm keen to get this merged.

[miguel-s]

Thanks @jimsynz!

Maybe because this PR was still in draft. Changing it now and hopefully the CI will run.

[jimsynz]

I don't know why CI isn't running for this PR It's super weird. I pulled your branch and ran mix check and a bug in this code found a bug in my code which I've fixed on main. It looks like your changes are requiring that private_key_id, private_key_path and team_id are required for all OAuth2 strategies which is definitely wrong. I'm thinking that you should change !!strategy.team_id et al to strategy.assent_strategy == Assent.Strategy.Apple or something similar. There's also some credo and sobelow warnings that need to be fixed or silenced.

[miguel-s]

Thanks for checking!

Tests, Credo and Sobelow are now green on my side.

[jimsynz]

Thanks for your contribution @miguel-s 🚀