team-alembic / ash_authentication Goto Github PK

For resources which use the PasswordAuthentication provider, we need the ability to request a password reset process.

For ash_authentication this probably means a new extension which defines new actions which can generate a reset token and call a function (maybe a callback or behaviour in a similar vein to HashProvider?) and to verify the token and perform a password reset.

For ash_authentication_phoenix this means a nice DX where you can easily wire it up to Phoenix's swoosh setup.

Questions:

• Can we use a JWT with a shorter lifetime and extra claim so that we don't have to actually store reset tokens anywhere?
• How does this integrate with the existing components in ash_authentication_phoenix - I suspect we provide components but leave them up to the user to wire into their UI.

Write documentation explaining how to set up authentication for various common combinations:

• Plain-old Ash application
• Ash/Phoenix/LiveView web application
• Ash/Phoenix/GraphQL application
• Ash/Phoenix/JsonApi application

** (exit) an exception was raised:
    ** (ArgumentError) errors were found at the given arguments:

  * 1st argument: not an already existing atom

        :erlang.binary_to_existing_atom("current_user", :utf8)

Nothing in my application yet refers to :current_user and so therefore we get an error about a non existing atom. I think perhaps we should make these atoms at compile time for each resource.

Trying to set up Auth0 using this guide I got to the login error page, but no errors were raised.

Here is the reason from the AuthController:

reason #=> %AshAuthentication.Errors.MissingSecret{
  resource: Red.Accounts.User,
  changeset: nil,
  query: nil,
  error_context: [],
  vars: [],
  path: [:authentication, :strategies, :auth0, :client_id],
  stacktrace: #Stacktrace<>,
  class: :forbidden
}

Eventually I tried this:

  def secret_for([:authentication, :strategies, :auth0, :redirect_uri], Red.Accounts.User, _) do
    get_config(:redirect_uri)
    |> dbg()
  end

Which showed me this:

get_config(:redirect_uri) #=> :error

Only here did I realize I made the mistake of saving the wrong key in my config file redirect_url

What I think should happen, is that if my secret_for function does not return the correct type, a helpful error is raised, letting me know what I returned.

In Getting started with authentication tutorial, to use the type ci_string in the email attribute the citext extension of postgres must be installed.

I have an application with ash_authentication and ash_authentication_phoenix.

I got the following error after I wanted to go to the sign-in page:

At the moment it duplicates things to have a usage guide as well as a getting started guide, IMO. I think it would be better to keep the readme page small and describe the broad use case and then point to ash_hq or hexdocs. Ideally ash_hq but at the moment its not actually on ash_hq so thats obviously not doable 😆

Conversation from discord:

zimt28 — Today at 2:49 PM
Why is it required to set a strategy when calling the :register_with_password action?
Is it because there could be multiple strategies? :thinkies:
jart we should consider maybe making the actions dynamically defined for each strategy and including the strategy in the name
Then the actions are usable without knowing about the internals of setting strategies
jart — Today at 4:03 PM
Well they are at the moment. We could define the actions to contain the strategy themselves. There are reused modules (eg GenerateTokenChange and SignInPreparation) that are reused across strategies so they need the specific strategy in the context but they don’t care how it gets there.
When I say “they are at the moment” I’m saying they do define the actions with the strategy in the name.
Eg AshAuthentication.Strategy.OAuth2.Transformer.set_defaults/1
I considered adding change set_context(%{strategy: strategy}) to each action but I couldn’t think of a nice way to validate it so just made it a requirement to pass it in.
Since I anticipated only calling them via the AshAuthentication.Strategy protocol anyway but this assumption was proved wrong by AshPhoenix.Form of course (which I could have seen coming).
The problem with looking up the strategy based on the action name is that we allow the user to override the action names.
Zach Daniel — Today at 4:14 PM
🤔 but we could persist something on the resource with the mapping
jart — Today at 4:15 PM
Yup. Totally could.
Zach Daniel — Today at 4:15 PM
Transformer.persist(:authentication_action_mapping, %{...}

Extension.get_persisted(...)

will do arbitrary key/value persistence on the resource
jart — Today at 4:15 PM
Just a map of action names to strategies.
Zach Daniel — Today at 4:15 PM
Yep
jart — Today at 4:16 PM
I didn’t think of that.
Zach Daniel — Today at 4:16 PM
So if we do it that way, the change we actually need is something like change GetStrategyFromAction and you can validate that that has been placed on the action potentially.
jart — Today at 4:17 PM
Or we can just remove it from the context completely
Zach Daniel — Today at 4:17 PM
🤔 yeah
good point
🙂
jart — Today at 4:17 PM
And have the modules that need it look in the right place.
Zach Daniel — Today at 4:17 PM
Whatever was getting the context before will just look the action name up on the resource

I would love to use Ash for the next version of https://www.vutuv.de (a social network Phoenix application). We don't store any passwords but send a magic_link by email or a PIN code by SMS. It would be great if ash_authentication would support sending a magic_link. Thank you for the great work!

The soon to be merged PR for storing all tokens is the first step towards this. We want an option that says that only tokens that appear in the database are valid, and then an additional option that says that revoking a token is just deleting it. These options may take a different form, not too concerned with that, just with the end result. This has come together so well 🔥

Because:

1. DSL sections are singletons (meaning we can't have multiples of the same provider)
2. The provider API was too rigid
3. There's an unnecessary proliferation of extensions.

Should lead to a much improved DX.

for convenience.

People want this. Let's do it.

Rather than the messy way it does it now with configuration.

1. tokens should be enabled by default
2. sign_in_tokens_enabled should be enabled by default

I'm working through the Ash Authentication tutorials and keep seeing references to AshAuthentication.Secret which results in a 404.

Additionally, the examples found here seem to be missing an end on the last line.

An idea I'm writing down for posterity, and to help us avoid locking ourselves out of this in the future:

In order to support a "bring your own SSO" feature set (i.e people using Ash have customers who want to use their own SSO), we will want to support something like this:

strategies do
  custom :bring_your_own_sso, BringYourOwnSSO
end

and then we get a strategy struct by calling BringYourOwnSSO.strategy(conn) (or something like that).

Additionally, it might honestly be easier to just support this intrinsically in the framework, i.e have them create a resource and use a DynamicOauthProvider extension, and then they could hook it in like so:

strategies do
  dynamic :dynamic_sso, MyApp.Resources.DynamicOauthProvider
end

And then we can provide actions and components and what-not for managing instances of these dynamic oauth configurations.

In the example, the password is used in the username field for the sign in step.

Instead of in the JWT.

Also, the token revocation resource will just become the token resource.

Based on feedback from multiple users, for example: https://elixirforum.com/t/onboarding-feedback-the-good-the-bad-and-the-ugly/58527/6?u=zachdaniel

Ultimately what should be done is that we should rework the guide to assume that the user has not already set up ash_authentication, and then highlight some areas that might be different if they already have set it up for password authentication, for example.

If the password strategy is resettable it should check that token generation is enabled on the resource.

As part of token generation, it could be useful to allow the creation of a refresh_token (RFC 6749 §1.5) instead of the default long-lived access tokens.

Apologies if this is already an included capability; I wasn't able to make the determination in the documentation. If the recommended behavior is to manually create a second token to use as a refresh token, it could be helpful to include documentation specifying that.

I noticed that when AshAuthentication try to run the token expunge action, it will trigger a warning because it is missing notifications:

[warning] Missed 1 notifications in action FeedbackCupcake.Accounts.Token.expunge_expired.

This happens when the resources are in a transaction, and you did not pass
`return_notifications?: true`. If you are in a changeset hook, you can
return the notifications. If not, you can send the notifications using
`Ash.Notifier.notify/1` once your resources are out of a transaction.

    (elixir 1.15.5) lib/process.ex:860: Process.info/2
    (ash 2.14.6) lib/ash/actions/helpers.ex:239: Ash.Actions.Helpers.warn_missed!/3
    (ash 2.14.6) lib/ash/actions/destroy.ex:385: Ash.Actions.Destroy.add_notifications/4
    (ash 2.14.6) lib/ash/actions/destroy.ex:118: Ash.Actions.Destroy.do_run/4
    (ash 2.14.6) lib/ash/actions/destroy.ex:37: Ash.Actions.Destroy.run/4
    (feedback_cupcake 0.1.0) lib/feedback_cupcake/accounts.ex:1: FeedbackCupcake.Accounts.destroy/2
    (ash_authentication 3.11.8) lib/ash_authentication/token_resource/actions.ex:215: anonymous fn/5 in AshAuthentication.TokenResource.Actions.expunge_inside_transaction/3
    (elixir 1.15.5) lib/enum.ex:4830: Enumerable.List.reduce/3
    (elixir 1.15.5) lib/enum.ex:2564: Enum.reduce_while/3
    (ecto_sql 3.10.2) lib/ecto/adapters/sql.ex:1352: anonymous fn/3 in Ecto.Adapters.SQL.checkout_or_transaction/4
    (db_connection 2.5.0) lib/db_connection.ex:1630: DBConnection.run_transaction/4
    (ash_authentication 3.11.8) lib/ash_authentication/token_resource/actions.ex:37: AshAuthentication.TokenResource.Actions.expunge_expired/2
    (ash_authentication 3.11.8) lib/ash_authentication/token_resource/expunger.ex:57: AshAuthentication.TokenResource.Expunger.handle_info/2
    (stdlib 5.0.2) gen_server.erl:1077: :gen_server.try_handle_info/3
    (stdlib 5.0.2) gen_server.erl:1165: :gen_server.handle_msg/6
    (stdlib 5.0.2) proc_lib.erl:241: :proc_lib.init_p_do_apply/3

For good reasons, we always return the same error struct, but in some cases were users want to handle different failures differently (our practical example was preventing sign in with a custom message for users that have confirmed_at as nil), they can't currently see what went wrong.

While upgrading to 3.11, I received the following error:

== Compilation error in file lib/ash_authentication/strategies/github.ex ==
** (UndefinedFunctionError) function Code.Identifier.inspect_as_key/1 is undefined or private
    (elixir 1.14.1) Code.Identifier.inspect_as_key(:scope)
    (elixir 1.14.1) inspect_overrides/1_6.ex:95: Inspect.List.keyword/2
    (elixir 1.14.1) lib/inspect/algebra.ex:472: Inspect.Algebra.container_each/6
    (elixir 1.14.1) lib/inspect/algebra.ex:449: Inspect.Algebra.container_doc/6
    (elixir 1.14.1) lib/kernel.ex:2254: Kernel.inspect/2
    lib/ash_authentication/strategies/github/dsl.ex:46: anonymous fn/1 in AshAuthentication.Strategy.Github.Dsl.strategy_override_docs/1
    (elixir 1.14.1) lib/enum.ex:1755: anonymous fn/2 in Enum.map_join/3
    (elixir 1.14.1) lib/enum.ex:4292: Enum.map_intersperse_list/3
could not compile dependency :ash_authentication, "mix compile" failed. Errors may have been logged above. You can recompile this dependency with "mix deps.compile ash_authentication", update it with "mix deps.update ash_authentication" or clean it with "mix deps.clean ash_authentication"

I'm on Windows 11 and Elixir 1.14.

Security best practices dictate that access tokens should generally be short-lived (15 minutes). Because the token lifetime is specified as an integer and uses hours, this is not possible. To avoid a breaking change, an additional property could be added additionally (token_lifetime_minutes), and the implementation could multiply the number of hours by 60 and add the minutes to set the expiration time.

Because we use the token's JTI as the primary key we can't revoke an existing token record (eg a confirmation token). We need to make revoke upsert so that it merely changes the purpose of the record.

This is a reminder to ensure that we are passing otp app down in to every place that needs to get lists of resources. https://team-alembic.slack.com/archives/C0486QKBA1K/p1670189103124109?thread_ts=1670188463.020659&channel=C0486QKBA1K&message_ts=1670189103.124109

We had a case recently where the DB host in our app went down, but the homepage of our app continued to render, despite performing database queries relating to showing current-user-specific information.

AshAuthentication swallows errors relating to database connectivity under a "user not found", as part of looking up the user that a JWT belongs to - https://github.com/team-alembic/ash_authentication/blob/main/lib/ash_authentication.ex#L221-L234 - and also as part of sign in (but I'm not sure where that happens).

So in our case, the web app behaved as if no-one was logged in (even when they were), but would not allow sign-in (because DB connection errors there were also being swallowed?) which wasn't an optimal scenario!

https://hexdocs.pm/ash_authentication/AshAuthentication.Sender.html has an error in the code example:

defmodule MyApp.PasswordResetSender do
  use AshAuthentication.PasswordReset.Sender
  import Swoosh.Email

  def send(user, reset_token, _opts) do
    new()
    |> to({user.name, user.email})
    |> from({"Doc Brown", "[email protected]"})
    |> subject("Password reset instructions")
    |> html_body("
      <h1>Password reset instructions</h1>
      <p>
        Hi #{user.name},<br />

        Someone (maybe you) has requested a password reset for your account.
        If you did not initiate this request then please ignore this email.
      </p>
      <a href="#{"https://example.com/user/password/reset?#{URI.encode_query(reset_token: reset_token)}}">
        Click here to reset
      </a>
    ")
    |> MyApp.Mailer.deliver()
  end
end

The <a href part is missing escapes and a ". It should be:

<a href=\"#{"https://example.com/user/password/reset?#{URI.encode_query(reset_token: reset_token)}"}\">
    Click here to reset
</a>

When following the default setup, if something goes wrong with authentication in some way, we currently "blow up" (if following the getting started guide). I think what we should do is, instead of rendering failure.html we should redirect to the sign in url with a flash message in our example auth controller.

Primary keys aren't currently being counted as an identity for magic_link creation. I've gotten around it by adding the primary key as an identity, which creates an extra unique index on it:

identities do
  identity :id, [:id]
end

My use case was that I wanted a single-click unsubscribe link for a Subscription resource that used the magic_link functionality (which works great!).

As of #22 we have a behaviour for notifying users of things (AshAuthentication.PasswordReset.Sender) this should probably be moved out of PasswordReset and made more generic for dealing with any kind of user notification.

We want the ability to confirm a when they sign up. We don't actually know what sort of data is in the identity field so we need to leave it up to the developer to decide how to send the notices to the user.
This probably means a new extension/transformer pair in a similar vein to PasswordReset except that it needs to add a notifier to the authentication resource so that it can react to create and maybe update events.

1. Similarly to PasswordReset the extension should generate a short-lived JWT with the act claim set to the name of the resource's confirmation action when the resource is created.
2. Needs to provide a confirmation endpoint (plug).
3. Needs to optionally generate and validate a new confirmation action on the resource.

This can be seen in this line of the file:

Basically if a user adds the confirmation add on with a different name than the one suggested in the documentation, it will fail the strategy function call.

I followed the example here (https://ash-hq.org/docs/guides/ash_authentication/latest/github-quickstart) but ran into this error. It talks about password_reset which I'm not sure where it's coming from.

** (EXIT from #PID<0.104.0>) an exception was raised:
    ** (Spark.Error.DslError) [nil]
 password_reset:
  `MyApp.Secrets` must implement the `AshAuthentication.Secret` behaviour.
        lib/MyApp/accounts/resources/user.ex:1: anonymous fn/1 in MyApp.Accounts.User.__verify_ash_dsl__/1
        (elixir 1.14.1) lib/enum.ex:975: Enum."-each/2-lists^foreach/1-0-"/2
        lib/MyApp/accounts/resources/user.ex:1: MyApp.Accounts.User.__verify_ash_dsl__/1
        (elixir 1.14.1) lib/enum.ex:975: Enum."-each/2-lists^foreach/1-0-"/2
        (elixir 1.14.1) lib/module/parallel_checker.ex:239: Module.ParallelChecker.check_module/2
        (elixir 1.14.1) lib/module/parallel_checker.ex:78: anonymous fn/5 in Module.ParallelChecker.spawn/3

Right now a token generated to reset a password can be used as a token to sign in and run any protected API

The OIDC strategy branch is coming along nicely and I wanted to add some additional feedback for the current setup.

1. Nonce should default to false as it may not be supported by all providers
2. openid_configuration should pass the authorize_url and token_url but currently they are required from the OAuth2
3. openid_configuration map doesn't seem pass this information to Assent. In my case, I need to pass a different issuer than whats at /.well-known/openid-configuration but I still receive an error for an invalid issuer. For reference, here is the config I was using with POW assent which worked as expected
4. trusted_audiences was recently added to Assent's config and it would be great if that were to be added.

I'm going through the doco today setting up ash_authentication on a play app, I've noticed a couple of broken links and oddities on ash-hq, so will add to the list as I go:

This link doesn't work for me in ash-hq on the getting started page (page has no content and appears to crash liveview?) but it does work on hexdocs:

Could be a ash-hq thing?

I've got the same issue with the 'Using with Phoenix' link in the summary:

And the same with the 'Getting started with Ash Authentication' link on the integrating with phoenix page:

Unformed links are showing on the integrating with phoenix page:

I think what we're going to need to do is make a compile-time friendly authenticated_resources that is a macro and calls Api.depend_on_resources. This will need to happen before the package can be released otherwise people will have strange issues on recompile:

We've already talked about this, just making an issue to ensure its tracked.

All actions should take opts so that they can be passed down to the underlying actions.

When a user tries to logut from a phoenix application using auth0 strategy for ash, the session will be cleared but auth0 is not notified about the logout.

A workaround that I found is to redirect the user to https://[YOUR_APP_ON_AUTH0]/v2/logout?returnTo=http%3A%2F%2F[YOUR_APP_HOME_PAGE] endpoint when loggin out. This is a sample code from my authcontroller

  def sign_out(conn, _params) do

    conn
    |> clear_session()
    |> redirect(external: "https://[auth0_endpoint]/v2/logout?returnTo=#{AppWeb.Endpoint.url()}")
  end

It would be helpful if the ash_authentication feature could automatically initiate a call to the auth0 endpoint. Additionally, it would be beneficial to include this workaround in the documentation, as new users of ash might not be aware of this functionality, just like myself.

Not sure where this is coming from, but it was picked up by Dependabot - our app no longer compiled on CI.

== Compilation error in file lib/my_app/accounts/user.ex ==
** (RuntimeError) Exception in transformer AshAuthentication.Strategy.Custom.Transformer on MyApp.Accounts.User:

expected a map, got: nil
    (elixir 1.15.5) lib/map.ex:486: Map.take/2
    (ash_authentication 3.11.13) lib/ash_authentication/strategies/password/transformer.ex:78: anonymous fn/2 in AshAuthentication.Strategy.Password.Transformer.transform/2
    (ash_authentication 3.11.13) lib/ash_authentication/strategies/password/transformer.ex:75: AshAuthentication.Strategy.Password.Transformer.transform/2
    (ash_authentication 3.11.13) lib/ash_authentication/strategies/custom/transformer.ex:90: AshAuthentication.Strategy.Custom.Transformer.do_transform/3
    (ash_authentication 3.11.13) lib/ash_authentication/strategies/custom/transformer.ex:44: anonymous fn/2 in AshAuthentication.Strategy.Custom.Transformer.do_strategy_transforms/1
    (elixir 1.15.5) lib/enum.ex:4830: Enumerable.List.reduce/3
    (elixir 1.15.5) lib/enum.ex:2564: Enum.reduce_while/3
    /Users/me/Projects/work/my-app/lib/my_app/accounts/user.ex:1: (file)

I was attempting to overwrite the :register_with_password action to expand the arguments and fields, but get the following error even though the :hashed_password attribute includes allow_nil?: true:

Expected the action `:register_with_password` to allow nil input for the field `:hashed_password`

For more info, see original Discord discussion:
https://discord.com/channels/711271361523351632/1050299714408550400/1065296028405878805

Looks like there is some kind of compiler deadlock where it can't validate the api even though it should be happening in a verifier. @zachdaniel is on the case.