Skip to content

Add phx.gen.auth generator - #4077

Merged
aaronrenner merged 2 commits into
phoenixframework:masterfrom
aaronrenner:ar-phx-gen-auth
Nov 26, 2020
Merged

aaronrenner merged 2 commits into
phoenixframework:masterfrom
aaronrenner:ar-phx-gen-auth

Conversation

@aaronrenner

Copy link
Copy Markdown
Member

This adds the mix phx.gen.auth task from phx_gen_auth v0.6.0 into phoenix master. The only additional change is to address a test failure caused by some changes to current_path/1 on phoenix master (see aaronrenner/phx_gen_auth#96).

This PR is missing the phx.gen.auth overview doc (https://github.com/aaronrenner/phx_gen_auth/blob/master/guides/overview.md). Is that something you'd like as part of this PR and if so, where would be a good place to work this in? We'll also probably want to add this to phoenix's mix tasks guide in a future PR.

Let me if you see anything stylistically that needs to be updated to match the rest of the phoenix codebase or anything else that would be good to change. Thanks!

/cc @josevalim

end

@tag database: :postgresql
test "has a passing test suite" do

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be moved inside the describe block above?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same applies to the test files, btw. :)

@aaronrenner aaronrenner Nov 25, 2020 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching that. I think I fixed them all in 02e8988.

Comment thread integration_test/test/code_generation/app_with_defaults_test.exs Outdated
Comment thread lib/mix/tasks/phx.gen.auth.ex Outdated
@josevalim

josevalim commented Nov 25, 2020 •

Copy link
Copy Markdown
Member

@aaronrenner just some minor comments but otherwise it looks great to me!

Regarding the guides, we need to do three changes which can be done separately:

  1. Bring in the guide under a new subsection called "Authentication", that comes right under subsection "Guides". The current guide can now be called "mix phx.gen.auth" and I believe it should be a mixture of the current README and the guide. I.e. it should explain how to use the generator and also describe the security considerations.

  2. Update the mix task guide to mention mix phx.gen.auth and simply link to the guide above

  3. We should probably update the context guides to use another example beyond authentication /cc @chrismccord

EDIT: Once we do step 1, let's also add a link to the guides from the generator.

Comment thread lib/mix/tasks/phx.gen.auth/hasing_library.ex

@josevalim josevalim left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Feel free to merge it at your discretion, if you would rather add the guides now or later, it is your call. ❤️

@aaronrenner
aaronrenner merged commit 31c75eb into phoenixframework:master Nov 26, 2020
@aaronrenner
aaronrenner deleted the ar-phx-gen-auth branch November 26, 2020 00:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants