team-alembic / ash_authentication

The Ash Authentication framework
MIT License
96 stars 55 forks source link

addon-authentication: confirm_on_create? true is not working #810

Closed demortierch closed 1 month ago

demortierch commented 1 month ago

In a new project created with igniter, email confirmation seems not working.

Project creation :

mix igniter.new auth_poc --install ash,ash_postgres --with phx.new --yes
cd auth_poc
mix igniter.update_gettext --yes
mix igniter.install ash_authentication_phoenix --yes
mix ash_authentication.add_strategy password
mix ash.setup

Authentication part of user.ex generated by the igniter tasks :

authentication do
    tokens do
      enabled? true
      token_resource AuthPoc.Accounts.Token
      signing_secret AuthPoc.Secrets
    end

    strategies do
      password :password do
        identity_field :email

        resettable do
          sender AuthPoc.Accounts.User.Senders.SendPasswordResetEmail
        end
      end
    end

    add_ons do
      confirmation :confirm_new_user do
        monitor_fields [:email]
        confirm_on_create? true
        confirm_on_update? false
        sender AuthPoc.Accounts.User.Senders.SendNewUserConfirmationEmail
      end
    end
  end

Confirmation link displayed in the console after sign in:

...
↳ AshPostgres.DataLayer.bulk_create/3, at: lib/data_layer.ex:1821
Click this link to confirm your email:

http://localhost:4000/auth/user/confirm_new_user?token=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJhY3QiOiJjb25maXJtIiwiYXVkIjoifj4gNC4yIiwiZXhwIjoxNzI5OTIxMjc1LCJpYXQiOjE3Mjk2NjIwNzUsImlzcyI6IkFzaEF1dGhlbnRpY2F0aW9uIHY0LjIuMyIsImp0aSI6IjMwMGZzdmh1czFsZm9kaTBybzAwMGFpMSIsIm5iZiI6MTcyOTY2MjA3NSwic3ViIjoidXNlcj9pZD1mNDExMjA5Yi04MzZhLTRlNTEtOWVmZi0yYmFmYjg3YzVkNGIifQ.3LZdHTkiQWrEdSgGWIpOE45qSFOE7XGOyQdTExuI4_s

[debug] QUERY OK db=0.3ms
commit []
...

Versions :

zachdaniel commented 1 month ago

This may be a problem with the generated code. Instead of the "token" query parameter, can you specify the token in the "confirm" query parameter?

It is intentional that the user is logged in immediately. If you don't want unconfirmed users to be able to log in you can implement a plug that redirects them to a page asking them to confirm their email.

I can confirm that it's the query parameter when I'm at a computer in a few hours.

demortierch commented 1 month ago

This may be a problem with the generated code. Instead of the "token" query parameter, can you specify the token in the "confirm" query parameter?

It works if the token is passed as 'confirm' parameter in the query.

http://localhost:4000/auth/user/confirm_new_user?confirm=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJhY3QiOiJjb25maXJtIiwiYXVkIjoifj4gNC4yIiwiZXhwIjoxNzI5OTIxMjc1LCJpYXQiOjE3Mjk2NjIwNzUsImlzcyI6IkFzaEF1dGhlbnRpY2F0aW9uIHY0LjIuMyIsImp0aSI6IjMwMGZzdmh1czFsZm9kaTBybzAwMGFpMSIsIm5iZiI6MTcyOTY2MjA3NSwic3ViIjoidXNlcj9pZD1mNDExMjA5Yi04MzZhLTRlNTEtOWVmZi0yYmFmYjg3YzVkNGIifQ.3LZdHTkiQWrEdSgGWIpOE45qSFOE7XGOyQdTExuI4_s

zachdaniel commented 1 month ago

Awesome, thanks for confirming. I will fix the generator in a few hours so that new projects will use the correct parameter.

sevenseacat commented 1 month ago

It is intentional that the user is logged in immediately. If you don't want unconfirmed users to be able to log in you can implement a plug that redirects them to a page asking them to confirm their email.

Then what's the purpose of confirm_on_create?? That seems kind of confusing

zachdaniel commented 1 month ago

confirm_on_create ensures that confirmed_at is not set to true, and sends a confirmation email. The confirmation plugin currently only does three things:

  1. ensures the sender is invoked whenever its conditions are met (confirm_on_create/confirm_on_update)
  2. manages the confirmed_at field when confirmation occurs
  3. prevents upserting against non-confirmed rows (for safety)

What it means to be unconfirmed is up to the application currently. @jimsynz might have a better articulated explanation here.

zachdaniel commented 1 month ago

This issue is fixed in the latest release.