aaronrenner / phx_gen_auth

An authentication system generator for Phoenix 1.5 applications.
772 stars 55 forks source link

Question: UUID instead of integer for user id #57

Closed pdgonzalez872 closed 4 years ago

pdgonzalez872 commented 4 years ago

Hi @aaronrenner thanks for the awesome lib. Auth is so important and I am happy that we now have an open source, community vetted solution that works out of the box (so nicely) with our beloved stack. Another feather on the Phoenix stack's hat.

I was playing around with the generators and realized we are using integers for the id column in user instead of using uuids. I was wondering if adding support to uuids is in your roadmap. If so, could you please give some details about what you'd be looking for? Would a --uuid flag be an option?

My use case is really just having the generator use uuids instead of common integer ids and have the existing code play nicely with that change (there are a few dependencies - tests, db relationships, a couple of mentions in the user_token context).

Thanks again for the awesome work, have a great Sunday.

josevalim commented 4 years ago

Phoenix generators support UUIDs, I believe there is a binary generator configuration that you can set. So we should probably do the same if said flag is set. Although it doesn't change the name of the id column, only its underlying type. The change should be straight-forward (probably adding a single line to the generated schemas).

pdgonzalez872 commented 4 years ago

This is what I had to do to make accounts work with uuids after running the generator. Tests pass.

commit 79076446b196790d2fabd9354dfdeee3c9d25446
Author: Paulo Daniel Gonzalez <pdgonzalez872@gmail.com>
Date:   Sun Jul 26 15:20:00 2020 -0500

    Use uuids instead of ids for a user

diff --git a/lib/auth_test/accounts/user.ex b/lib/auth_test/accounts/user.ex
index 724bb61..4930440 100644
--- a/lib/auth_test/accounts/user.ex
+++ b/lib/auth_test/accounts/user.ex
@@ -2,6 +2,10 @@ defmodule AuthTest.Accounts.User do
   use Ecto.Schema
   import Ecto.Changeset

+  @primary_key {:id, :binary_id, autogenerate: true}
+  @foreign_key_type :binary_id
+  @derive {Phoenix.Param, key: :id}
+
   @derive {Inspect, except: [:password]}
   schema "users" do
     field :email, :string
diff --git a/priv/repo/migrations/20200722022205_create_users_auth_tables.exs b/priv/repo/migrations/20200722022205_create_users_auth_tables.exs
index 633b028..da566b5 100644
--- a/priv/repo/migrations/20200722022205_create_users_auth_tables.exs
+++ b/priv/repo/migrations/20200722022205_create_users_auth_tables.exs
@@ -4,7 +4,8 @@ defmodule AuthTest.Repo.Migrations.CreateUsersAuthTables do
   def change do
     execute "CREATE EXTENSION IF NOT EXISTS citext", ""

-    create table(:users) do
+    create table(:users, primary_key: false) do
+      add :id, :uuid, primary_key: true
       add :email, :citext, null: false
       add :hashed_password, :string, null: false
       add :confirmed_at, :naive_datetime
@@ -14,7 +15,7 @@ defmodule AuthTest.Repo.Migrations.CreateUsersAuthTables do
     create unique_index(:users, [:email])

     create table(:users_tokens) do
-      add :user_id, references(:users, on_delete: :delete_all), null: false
+      add :user_id, references(:users, type: :uuid, column: :id, on_delete: :delete_all), null: false
       add :token, :binary, null: false
       add :context, :string, null: false
       add :sent_to, :string
diff --git a/test/auth_test/accounts_test.exs b/test/auth_test/accounts_test.exs
index bdb2185..8d4b093 100644
--- a/test/auth_test/accounts_test.exs
+++ b/test/auth_test/accounts_test.exs
@@ -1,6 +1,7 @@
 defmodule AuthTest.AccountsTest do
   use AuthTest.DataCase

+  alias Ecto.UUID
   alias AuthTest.Accounts
   import AuthTest.AccountsFixtures
   alias AuthTest.Accounts.{User, UserToken}
@@ -37,7 +38,7 @@ defmodule AuthTest.AccountsTest do
   describe "get_user!/1" do
     test "raises if id is invalid" do
       assert_raise Ecto.NoResultsError, fn ->
-        Accounts.get_user!(-1)
+        Accounts.get_user!(UUID.generate())
       end
     end

Hope this helps.

aaronrenner commented 4 years ago

@pdgonzalez872 Thanks for reaching out. We actually already have UUID support but the option name is --binary-id. Here's the documentation for this option: https://hexdocs.pm/phx_gen_auth/Mix.Tasks.Phx.Gen.Auth.html#module-binary-ids.

Let me know if you have any issues.

pdgonzalez872 commented 4 years ago

Oh sweet!! Thanks @aaronrenner ! I guess I was just too excited to try the generator and didn't read the docs all the way. I added a note to the readme about the flag here: https://github.com/aaronrenner/phx_gen_auth/pull/58. If you think it adds any value, feel free to merge it! if not, no worries!

Thanks for your work!