-
Notifications
You must be signed in to change notification settings - Fork 16
Auth #1
base: master
Are you sure you want to change the base?
Auth #1
Changes from 10 commits
65f49a9
e84f9be
eff8376
d517b54
64ed8f9
d4ea2b4
b677abc
1befca8
6625019
153b7ce
fe52685
3e30e6d
5a2f0ed
d3b7a93
653d4ed
ba4b093
c157742
f137822
5b550b8
e7dd9e1
4e02461
39ccccb
3558bac
bdef99d
e61a7aa
fc2c735
76ff10a
44a6bac
7c07c02
085dd92
0e3efd9
fcf4a8b
9113466
0712d9c
9c22870
19470c8
4796235
f801205
2a7616a
b53f616
6049142
97bd003
7f3a55e
f698bc8
ef46ae1
d3aca05
326db9c
4abf899
f3a1571
8655c00
5fbf0ba
13de852
6637c85
ead269e
c73cba4
01194c3
288c67a
9b1a270
cd2ab05
9912c79
25d083d
fbd52ec
ecc8eb5
6ae63ab
e51d52a
bb13f6b
1eebcae
4113391
2b92a22
3003342
6f2ef5d
83d5deb
78cb46e
57bf7df
ed0e210
4664c37
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,20 +1,26 @@ | ||
# Demo | ||
|
||
To start your Phoenix server: | ||
## Spec | ||
|
||
* Install dependencies with `mix deps.get` | ||
* Create and migrate your database with `mix ecto.setup` | ||
* Install Node.js dependencies with `npm install --prefix assets` | ||
* Start Phoenix endpoint with `mix phx.server` | ||
We will two database tables: "users" and "users_tokens": | ||
josevalim marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
Now you can visit [`localhost:4000`](http://localhost:4000) from your browser. | ||
* "users" will have the "encrypted_password", "email" and "confirmed_at" fields plus timestamps | ||
* "users_tokens" will have "token", "context", "sent_to", "inserted_at" fields | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Have you considered moving away from pre-required/specific database tables? Consider the scenario where the user table already exists, so all they need to do is to add the missing columns and use your logic. This would virtually enable absolute flexibility and stay out of the end-user/dev way. For example, the way I look at this current implementation is that the spec assumes there will be a password always (which is not true). For example, you could consider replacing the I recently worked on a passwordless authentication solution, and it's unfortunate that most (if not all) the existing authentication systems are strictly following patterns that will always stay in your developer way and most probably enforce some pretty painful UX on the end-user side. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I personally have no interest in covering all possible use cases. :) I have done that, it didn't work. I intend this solution to be pretty much as is. If you want to move away from what it does, you can either:
In regards to JWT and in general non DB-backed tokens, we discussed this extensively, and we believe that DB tokens are more secure and provide better UI, as they allow you to expire and track them individually or collectively. In any case, I don't guarantee this generator provides the best UI possible, if you believe that passwordless is the best way to go for certain apps, then I look forward to a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for explaining this @josevalim I arrived here having in mind Devise as a context. I'm glad this is a more specific solution vs. all-case one. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, in case you didn't see it, there is more context here: https://dashbit.co/blog/a-new-authentication-solution-for-phoenix I was not expecting folks to find this PR outside of the blog post. :) |
||
|
||
Ready to run in production? Please [check our deployment guides](https://hexdocs.pm/phoenix/deployment.html). | ||
On sign in, we need to renew the session and delete the CSRF token. The password should be limited to 80 characters, the email to 160. | ||
|
||
## Learn more | ||
Confirmation, resetting passwords, remember me, and session management will be all based on tokens. Tokens are generated randomly using `:crypto.strong_rand_bytes/1` and then hashed using sha2/sha3. The hashing helps protect against timing attacks. The hashed token is the one stored in the database, the original token is not stored. | ||
|
||
* Official website: https://www.phoenixframework.org/ | ||
* Guides: https://hexdocs.pm/phoenix/overview.html | ||
* Docs: https://hexdocs.pm/phoenix | ||
* Forum: https://elixirforum.com/c/phoenix-forum | ||
* Source: https://github.com/phoenixframework/phoenix | ||
Whenever a token is generated, a context has to be given (such as "session", "reset", etc). The context is used to avoid one token being used against its original purpose. Verifying the token will hash the token and look-up its hashed result in the database. Verification also takes a ttl. The current date is compared against the token `inserted_at + ttl` and the token is deemed as expired if enough time has passed. | ||
|
||
For confirming e-mail addresses, we will generate a token, and e-mail it to the user. We will store the hashed token, the context ("confirm"), and store the confirmation e-mail under the `sent_to` column. Once the user clicks the link, we will verify the token, and see if the current user e-mail matches the one in the `sent_to` column. Once the token is used, it is deleted. Note we won't automatically sign the user in after confirmation - this protects us from someone getting access to the account via confirmation tokens. This also allows us to set long expiry dates in the tokens. | ||
|
||
For resetting the password, the process is very similar. Once the link is clicked, it will go to the reset password page. The reset password page will ask for the new password and for the password confirmation. Once the user sends the form, we will verify the token and proceed to reset the password. Resetting the password will delete all tokens. Generally speaking, changing the password always deletes all tokens. Resetting the password won't sign the user in - so if anyone intercepts the token, they cannot gain access to the system as they are missing the e-mail/username. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Hi @josevalim. This is super helpful! I just want a bit of clarification here: if we're worried about the token being leaked (for confirmation/password reset) then shouldn't we also be worried that we're advocating sending this token in the body of plain text email with a relatively long expiry (1 week)? We can never guarantee secure transmission of email, and in the case of an intercepted email an attacker would also have access to the email address, so that is enough data for an account takeover using the password reset functionality as they have the 2 bits of info required (token + email) and can set their own password and login immediately regardless of whether we do it for them or not. So I guess my questions are: a) why would we take the UX-hit of not logging users in automatically given the most likely token leak scenario is via email, where the attacker would have access to the email address anyway? b) would it be wise to shorten the token expiry here, to reduce the interception window? Django uses a default of 3 days, and allows automatic login after reset but both are configurable. Laravel uses a much stricter one hour window, but also automatically auths after resets. I guess you could say people can change it but the wording of the text above: "This also allows us to set long expiry dates in the tokens." seems to suggest people should push for longer token expiry. It's late here and I may have missed something super obvious but thought I'd ask anyway. Thanks! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think those are good considerations. I will reduce the initial period is a safe call. Thanks! |
||
|
||
For changing the e-mail, the user will put the new e-mail in a field. This will create a new token with change:CURRENT_EMAIL as context and the new e-mail stored in the `sent_to` column. A hashed token will be sent to the new e-mail. Once the user clicks on the e-mail link, the raw token will be hashed and we will change the user to the new e-mail as long as the hashed token and old email match to the currently signed in user. Once this is done, the token is deleted. This token will have a short expiry date by default. | ||
|
||
## Pending for generators | ||
|
||
mix phx.gen.auth Account User users ...extrafields... | ||
|
||
The authentication mechanism should be an option. Default to bcrypt for Unix systems, pdkdf2 for Windows systems. The line to config/test.exs must always be added. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,3 +20,6 @@ config :demo, DemoWeb.Endpoint, | |
|
||
# Print only warnings and errors during test | ||
config :logger, level: :warn | ||
|
||
# Only in tests, remove the complexity from the password encryption algorithm | ||
josevalim marked this conversation as resolved.
Show resolved
Hide resolved
|
||
config :bcrypt_elixir, :log_rounds, 1 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should support all three comeonin adapters. We should default to bcrypt except on Windows which should use pkbdf2. Each adapter will require a different line here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Argon2 is recommended here. Any reason not to use it as the default? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I am aware folks will have different opinions and takes on the password hashing, that's why I prefer to follow recommendations from OWASP and similar and, AFAIK, it is still bcrypt. If someone has any updated insights or data on that, I would love to hear. :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems that the recommendation is still
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. any thoughts on passwordless auth? it solves a lot of hassle in pass world like reset, change, forgot, keeps email as source of truth and solve password hashing issues There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
In my opinion, passwordless auth should be avoided most of the time. I guess it depends on your user base, but in my experience, many people use their work email to sign up for services, even on non work related sites. Unless you can verify their identity some other way during sign up, that means they will lose access to their accounts when they change jobs, since you can't trust email update requests from any other email address. This is especially important if your project falls under GDPR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
even in that cenario, its safer passwordless since the emails belongs to the company and that could be abused by bad actors, I manage and managed the email myself a couple of times, its usual to help someone to recover stuff left in company emails they dont own anymore, passwords dont solve this issue, specially for car rentals, and some other kind of stuff they get important purchase info by mail, the mistake is to use a email you dont own, and when you know is passwordless you think twice to use one you do not own. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Someone is welcome to implement There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. any opinions on https://github.com/zanderxyz/veil? |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,341 @@ | ||||||
defmodule Demo.Accounts do | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A question after reading through the code: why is this module called I'd expect it to be simply called I could every well be missing something here! Just wanted to ask as there may be an opportunity to simplify function naming (e.g. removing some "user_" prefixes in this module) and avoiding possible terminology confusion between "Account" and "User". Off-topic: Great work on this! Feels like a great approach in general. (Speaking here as a former Devise user who has been trying out multiple Devise-like frameworks with Elixir/Phoenix, leading to similar experiences as outlined in the Dashbit blogpost 😄) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The module name is whatever you specify during generation for the context. So There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The Phoenix docs for Contexts may be (one of) the reasons why this was chosen. If nothing else it could allow people new to the framework to connect the documentation examples. |
||||||
@moduledoc """ | ||||||
The Accounts context. | ||||||
""" | ||||||
|
||||||
alias Demo.Repo | ||||||
alias Demo.Accounts.{User, UserToken, UserNotifier} | ||||||
|
||||||
## Database getters | ||||||
|
||||||
@doc """ | ||||||
Gets a user by email. | ||||||
|
||||||
## Examples | ||||||
|
||||||
iex> get_user_by_email("[email protected]") | ||||||
%User{} | ||||||
|
||||||
iex> get_user_by_emai("[email protected]") | ||||||
josevalim marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
nil | ||||||
|
||||||
""" | ||||||
def get_user_by_email(email) when is_binary(email) do | ||||||
Repo.get_by(User, email: email) | ||||||
end | ||||||
|
||||||
@doc """ | ||||||
Gets a user by email and password. | ||||||
|
||||||
## Examples | ||||||
|
||||||
iex> get_user_by_email_and_password("[email protected]", "correct_password") | ||||||
%User{} | ||||||
|
||||||
iex> get_user_by_email_and_password("[email protected]", "invalid_password") | ||||||
nil | ||||||
|
||||||
""" | ||||||
def get_user_by_email_and_password(email, password) | ||||||
when is_binary(email) and is_binary(password) do | ||||||
user = Repo.get_by(User, email: email) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
if User.valid_password?(user, password), do: user | ||||||
end | ||||||
|
||||||
@doc """ | ||||||
Gets a single user. | ||||||
|
||||||
Raises `Ecto.NoResultsError` if the User does not exist. | ||||||
|
||||||
## Examples | ||||||
|
||||||
iex> get_user!(123) | ||||||
%User{} | ||||||
|
||||||
iex> get_user!(456) | ||||||
** (Ecto.NoResultsError) | ||||||
|
||||||
""" | ||||||
def get_user!(id), do: Repo.get!(User, id) | ||||||
|
||||||
## User registration | ||||||
|
||||||
@doc """ | ||||||
Registers a user. | ||||||
|
||||||
## Examples | ||||||
|
||||||
iex> register_user(%{field: value}) | ||||||
{:ok, %User{}} | ||||||
|
||||||
iex> register_user(%{field: bad_value}) | ||||||
{:error, %Ecto.Changeset{}} | ||||||
|
||||||
""" | ||||||
def register_user(attrs \\ %{}) do | ||||||
%User{} | ||||||
|> User.registration_changeset(attrs) | ||||||
|> Repo.insert() | ||||||
end | ||||||
|
||||||
@doc """ | ||||||
Returns an `%Ecto.Changeset{}` for tracking user changes. | ||||||
|
||||||
## Examples | ||||||
|
||||||
iex> change_user_registration(user) | ||||||
%Ecto.Changeset{data: %User{}} | ||||||
|
||||||
""" | ||||||
def change_user_registration(%User{} = user, attrs \\ %{}) do | ||||||
User.registration_changeset(user, attrs) | ||||||
end | ||||||
|
||||||
## Settings | ||||||
|
||||||
@doc """ | ||||||
Returns an `%Ecto.Changeset{}` for changing the user e-mail. | ||||||
|
||||||
## Examples | ||||||
|
||||||
iex> change_user_email(user) | ||||||
%Ecto.Changeset{data: %User{}} | ||||||
|
||||||
""" | ||||||
def change_user_email(user, attrs \\ %{}) do | ||||||
User.email_changeset(user, attrs) | ||||||
end | ||||||
|
||||||
@doc """ | ||||||
Emulates that the e-mail will change without actually changing | ||||||
it in the database. | ||||||
|
||||||
## Examples | ||||||
|
||||||
iex> apply_user_email(user, "valid password", %{email: ...}) | ||||||
{:ok, %User{}} | ||||||
|
||||||
iex> apply_user_email(user, "invalid password", %{email: ...}) | ||||||
{:error, %Ecto.Changeset{}} | ||||||
|
||||||
""" | ||||||
def apply_user_email(user, password, attrs) do | ||||||
user | ||||||
|> User.email_changeset(attrs) | ||||||
|> User.validate_current_password(password) | ||||||
|> Ecto.Changeset.apply_action(:update) | ||||||
end | ||||||
|
||||||
@doc """ | ||||||
Updates the user e-mail in token. | ||||||
|
||||||
If the token matches, the user email is updated and the token is deleted. | ||||||
""" | ||||||
def update_user_email(user, token) do | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @josevalim Wouldn't be safer (e.g. in a multi-node system) to pass around the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ecto only tracks the fields that changed. So in a multinode system, assuming both are changing the e-mail at the same time, only one of them is going to win anyway. The time between fetching and updating is not really relevant given the chance of two e-mail updates happening at the same time are very unlikely. The order of the updates is most likely to be affected by the internet speed of the requests changing the email than by the system itself. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about renaming this to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or maybe better yet |
||||||
context = "change:#{user.email}" | ||||||
|
||||||
with {:ok, query} <- UserToken.verify_user_change_email_token_query(token, context), | ||||||
%UserToken{sent_to: email} <- Repo.one(query), | ||||||
{:ok, _} <- Repo.transaction(user_email_multi(user, email, context)) do | ||||||
:ok | ||||||
else | ||||||
_ -> :error | ||||||
end | ||||||
end | ||||||
|
||||||
defp user_email_multi(user, email, context) do | ||||||
Ecto.Multi.new() | ||||||
|> Ecto.Multi.update(:user, User.email_changeset(user, %{email: email})) | ||||||
|> Ecto.Multi.delete_all(:tokens, UserToken.user_and_contexts_query(user, [context])) | ||||||
end | ||||||
|
||||||
@doc """ | ||||||
Delivers the update e-mail instructions to the given user. | ||||||
|
||||||
## Examples | ||||||
|
||||||
iex> deliver_update_email_instructions(user, current_email, &Routes.user_update_email_url(conn, :edit)) | ||||||
josevalim marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
:ok | ||||||
|
||||||
""" | ||||||
def deliver_update_email_instructions(%User{} = user, current_email, update_email_url_fun) | ||||||
when is_function(update_email_url_fun, 1) do | ||||||
{encoded_token, user_token} = | ||||||
UserToken.build_user_email_token(user, "change:#{current_email}") | ||||||
|
||||||
Repo.insert!(user_token) | ||||||
UserNotifier.deliver_update_email_instructions(user, update_email_url_fun.(encoded_token)) | ||||||
:ok | ||||||
end | ||||||
|
||||||
@doc """ | ||||||
Returns an `%Ecto.Changeset{}` for changing the user password. | ||||||
|
||||||
## Examples | ||||||
|
||||||
iex> change_user_password(user) | ||||||
%Ecto.Changeset{data: %User{}} | ||||||
|
||||||
""" | ||||||
def change_user_password(user, attrs \\ %{}) do | ||||||
User.password_changeset(user, attrs) | ||||||
end | ||||||
|
||||||
@doc """ | ||||||
Returns an `%Ecto.Changeset{}` for changing the user password. | ||||||
josevalim marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
||||||
## Examples | ||||||
|
||||||
iex> update_user_password(user, "valid password", %{password: ...}) | ||||||
{:ok, %User{}} | ||||||
|
||||||
iex> update_user_password(user, "invalid password", %{password: ...}) | ||||||
{:error, %Ecto.Changeset{}} | ||||||
|
||||||
""" | ||||||
def update_user_password(user, password, attrs \\ %{}) do | ||||||
user | ||||||
|> User.password_changeset(attrs) | ||||||
|> User.validate_current_password(password) | ||||||
|> Repo.update() | ||||||
end | ||||||
|
||||||
## Session | ||||||
|
||||||
@doc """ | ||||||
Generates a session token. | ||||||
""" | ||||||
def generate_session_token(user) do | ||||||
{token, user_token} = UserToken.build_session_token(user) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why does this return a tuple, isn't the token in the struct anyway? user_token = %{token: token} = UserToken.build_session_token(user) in fact: %{token: token} =
UserToken.build_session_token(user)
|> Repo.insert!
token There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mostly for consistency with the other build functions in the same module. |
||||||
Repo.insert!(user_token) | ||||||
token | ||||||
end | ||||||
|
||||||
@doc """ | ||||||
Gets the user with the given signed token. | ||||||
""" | ||||||
def get_user_by_session_token(token) do | ||||||
{:ok, query} = UserToken.verify_session_token_query(token) | ||||||
Repo.one(query) | ||||||
end | ||||||
|
||||||
@doc """ | ||||||
Deletes the signed token with the given context. | ||||||
""" | ||||||
def delete_session_token(token) do | ||||||
Repo.delete_all(UserToken.token_and_context_query(token, "session")) | ||||||
:ok | ||||||
end | ||||||
|
||||||
## Confirmation | ||||||
|
||||||
@doc """ | ||||||
Delivers the confirmation e-mail instructions to the given user. | ||||||
|
||||||
## Examples | ||||||
|
||||||
iex> deliver_confirmation_instructions(user, &Routes.user_confirmation_url(conn, :confirm)) | ||||||
:ok | ||||||
|
||||||
iex> deliver_confirmation_instructions(confirmed_user, &Routes.user_confirmation_url(conn, :confirm)) | ||||||
{:error, :already_confirmed} | ||||||
|
||||||
""" | ||||||
def deliver_confirmation_instructions(%User{} = user, confirmation_url_fun) | ||||||
when is_function(confirmation_url_fun, 1) do | ||||||
if user.confirmed_at do | ||||||
{:error, :already_confirmed} | ||||||
else | ||||||
{encoded_token, user_token} = UserToken.build_user_email_token(user, "confirm") | ||||||
Repo.insert!(user_token) | ||||||
UserNotifier.deliver_confirmation_instructions(user, confirmation_url_fun.(encoded_token)) | ||||||
:ok | ||||||
end | ||||||
end | ||||||
|
||||||
@doc """ | ||||||
Confirms a user by the given token. | ||||||
|
||||||
If the token matches, the user account is marked as confirmed | ||||||
and the token is deleted. | ||||||
""" | ||||||
def confirm_user(token) do | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since this now returns There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Your call. :) |
||||||
with {:ok, query} <- UserToken.verify_user_email_token_query(token, "confirm"), | ||||||
%User{} = user <- Repo.one(query), | ||||||
{:ok, _} <- Repo.transaction(confirm_user_multi(user)) do | ||||||
:ok | ||||||
else | ||||||
_ -> :error | ||||||
end | ||||||
end | ||||||
|
||||||
defp confirm_user_multi(user) do | ||||||
Ecto.Multi.new() | ||||||
|> Ecto.Multi.update(:user, User.confirm_changeset(user)) | ||||||
|> Ecto.Multi.delete_all(:tokens, UserToken.user_and_contexts_query(user, ["confirm"])) | ||||||
end | ||||||
|
||||||
## Reset passwword | ||||||
josevalim marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
||||||
@doc """ | ||||||
Delivers the reset password e-mail to the given user. | ||||||
|
||||||
## Examples | ||||||
|
||||||
iex> deliver_reset_password_instructions(user, &Routes.user_reset_password_url(conn, :edit)) | ||||||
:ok | ||||||
|
||||||
""" | ||||||
def deliver_reset_password_instructions(%User{} = user, reset_password_url_fun) | ||||||
when is_function(reset_password_url_fun, 1) do | ||||||
{encoded_token, user_token} = UserToken.build_user_email_token(user, "reset_password") | ||||||
Repo.insert!(user_token) | ||||||
UserNotifier.deliver_reset_password_instructions(user, reset_password_url_fun.(encoded_token)) | ||||||
:ok | ||||||
end | ||||||
|
||||||
@doc """ | ||||||
Gets the user by reset password token. | ||||||
|
||||||
## Examples | ||||||
|
||||||
iex> get_user_by_reset_password_token("validtoken-sadsadsa") | ||||||
%User{} | ||||||
|
||||||
iex> get_user_by_reset_password_token("invalidtoken-sadsadsa") | ||||||
nil | ||||||
|
||||||
""" | ||||||
def get_user_by_reset_password_token(token) do | ||||||
with {:ok, query} <- UserToken.verify_user_email_token_query(token, "reset_password"), | ||||||
%User{} = user <- Repo.one(query) do | ||||||
user | ||||||
else | ||||||
_ -> nil | ||||||
end | ||||||
end | ||||||
|
||||||
@doc """ | ||||||
Returns an `%Ecto.Changeset{}` for tracking user reset password. | ||||||
josevalim marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
||||||
## Examples | ||||||
|
||||||
|
||||||
iex> reset_user_password(user, %{password: "new long password", password_confirmation: "new long password"}) | ||||||
{:ok, %User{}} | ||||||
|
||||||
iex> reset_user_password(user, %{password: "valid", password_confirmation: "not the same"}) | ||||||
{:error, %Ecto.Changeset{}} | ||||||
|
||||||
""" | ||||||
def reset_user_password(user, attrs \\ %{}) do | ||||||
Ecto.Multi.new() | ||||||
|> Ecto.Multi.update(:user, User.password_changeset(user, attrs)) | ||||||
|> Ecto.Multi.delete_all(:tokens, UserToken.user_and_contexts_query(user, :all)) | ||||||
|> Repo.transaction() | ||||||
|> case do | ||||||
{:ok, %{user: user}} -> {:ok, user} | ||||||
{:error, :user, changeset, _} -> {:error, changeset} | ||||||
end | ||||||
end | ||||||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where do you see this spec going? Will it be in the Readme of phx_gen_auth, in the moduledoc of generated code, or just in this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I originally thought it would only be part of this PR but I think you will be more capable of assessing it. If you feel like there is important information in here, then maybe some of it should be added to
mix phx.gen.auth
docs OR added to the module docs of the related modules, especially theuser
anduser_token
ones. WDYT?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. I'll look for a good place to put it. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding this as
@moduledoc
is something that I wanted to suggest after runningmix phx.gen.auth
in my project. It would make it so much easier to read through the code, especially considering the fact that when it's generated via mix it's a single commit. I personally found it a bit hard to figure out where to start reading the generated code.