From ba4e90ef51a19305132bcfdb555e5a4b23331fd3 Mon Sep 17 00:00:00 2001 From: Ivan Reshetnikov Date: Fri, 6 Sep 2024 13:12:30 +0500 Subject: [PATCH] refactor: use client_id as primary key --- lib/comfycamp/rand.ex | 6 ++++ lib/comfycamp/sso/oidc_app.ex | 3 +- .../controllers/oidc_app_controller.ex | 22 ++++++++------ .../controllers/oidc_app_html/edit.html.heex | 6 ++-- .../controllers/oidc_app_html/index.html.heex | 4 +-- .../controllers/oidc_app_html/new.html.heex | 6 ++-- .../oidc_app_html/oidc_app_form.html.heex | 4 +-- .../controllers/oidc_app_html/show.html.heex | 6 ++-- .../20240906061751_alter_oidc_app_id.exs | 12 ++++++++ test/comfycamp/sso_test.exs | 6 ++-- .../controllers/oidc_app_controller_test.exs | 30 +++++++++---------- test/support/fixtures/sso_fixtures.ex | 6 ++-- 12 files changed, 67 insertions(+), 44 deletions(-) create mode 100644 lib/comfycamp/rand.ex create mode 100644 priv/repo/migrations/20240906061751_alter_oidc_app_id.exs diff --git a/lib/comfycamp/rand.ex b/lib/comfycamp/rand.ex new file mode 100644 index 0000000..ae4e27d --- /dev/null +++ b/lib/comfycamp/rand.ex @@ -0,0 +1,6 @@ +defmodule Comfycamp.Rand do + def get_random_string(length) do + :crypto.strong_rand_bytes(length) + |> Base.url_encode64() + end +end diff --git a/lib/comfycamp/sso/oidc_app.ex b/lib/comfycamp/sso/oidc_app.ex index 2686173..a27133e 100644 --- a/lib/comfycamp/sso/oidc_app.ex +++ b/lib/comfycamp/sso/oidc_app.ex @@ -2,10 +2,11 @@ defmodule Comfycamp.SSO.OIDCApp do use Ecto.Schema import Ecto.Changeset + @derive {Phoenix.Param, key: :client_id} + @primary_key {:client_id, :string, autogenerate: false} schema "oidc_apps" do field :enabled, :boolean, default: false field :name, :string - field :client_id, :string field :client_secret, :string timestamps(type: :utc_datetime) diff --git a/lib/comfycamp_web/controllers/oidc_app_controller.ex b/lib/comfycamp_web/controllers/oidc_app_controller.ex index 1744907..7317077 100644 --- a/lib/comfycamp_web/controllers/oidc_app_controller.ex +++ b/lib/comfycamp_web/controllers/oidc_app_controller.ex @@ -3,6 +3,7 @@ defmodule ComfycampWeb.OIDCAppController do alias Comfycamp.SSO alias Comfycamp.SSO.OIDCApp + alias Comfycamp.Rand def index(conn, _params) do oidc_apps = SSO.list_oidc_apps() @@ -13,7 +14,10 @@ defmodule ComfycampWeb.OIDCAppController do end def new(conn, _params) do - changeset = SSO.change_oidc_app(%OIDCApp{}) + changeset = SSO.change_oidc_app(%OIDCApp{ + client_id: Rand.get_random_string(20), + client_secret: Rand.get_random_string(32), + }) conn |> put_layout(html: :admin) @@ -34,16 +38,16 @@ defmodule ComfycampWeb.OIDCAppController do end end - def show(conn, %{"id" => id}) do - oidc_app = SSO.get_oidc_app!(id) + def show(conn, %{"id" => client_id}) do + oidc_app = SSO.get_oidc_app!(client_id) conn |> put_layout(html: :admin) |> render(:show, oidc_app: oidc_app) end - def edit(conn, %{"id" => id}) do - oidc_app = SSO.get_oidc_app!(id) + def edit(conn, %{"id" => client_id}) do + oidc_app = SSO.get_oidc_app!(client_id) changeset = SSO.change_oidc_app(oidc_app) conn @@ -51,8 +55,8 @@ defmodule ComfycampWeb.OIDCAppController do |> render(:edit, oidc_app: oidc_app, changeset: changeset) end - def update(conn, %{"id" => id, "oidc_app" => oidc_app_params}) do - oidc_app = SSO.get_oidc_app!(id) + def update(conn, %{"id" => client_id, "oidc_app" => oidc_app_params}) do + oidc_app = SSO.get_oidc_app!(client_id) case SSO.update_oidc_app(oidc_app, oidc_app_params) do {:ok, oidc_app} -> @@ -65,8 +69,8 @@ defmodule ComfycampWeb.OIDCAppController do end end - def delete(conn, %{"id" => id}) do - oidc_app = SSO.get_oidc_app!(id) + def delete(conn, %{"id" => client_id}) do + oidc_app = SSO.get_oidc_app!(client_id) {:ok, _oidc_app} = SSO.delete_oidc_app(oidc_app) conn diff --git a/lib/comfycamp_web/controllers/oidc_app_html/edit.html.heex b/lib/comfycamp_web/controllers/oidc_app_html/edit.html.heex index e25f823..b2b16d0 100644 --- a/lib/comfycamp_web/controllers/oidc_app_html/edit.html.heex +++ b/lib/comfycamp_web/controllers/oidc_app_html/edit.html.heex @@ -1,10 +1,10 @@
<.header> - Edit Oidc app <%= @oidc_app.id %> + Edit OpenID app "<%= @oidc_app.name %>" <:subtitle>Use this form to manage oidc_app records in your database. - <.oidc_app_form changeset={@changeset} action={~p"/admin/oidc_apps/#{@oidc_app}"} /> + <.back navigate={~p"/admin/oidc_apps"}>Back to OpenID apps - <.back navigate={~p"/admin/oidc_apps"}>Back to oidc_apps + <.oidc_app_form changeset={@changeset} action={~p"/admin/oidc_apps/#{@oidc_app}"} />
diff --git a/lib/comfycamp_web/controllers/oidc_app_html/index.html.heex b/lib/comfycamp_web/controllers/oidc_app_html/index.html.heex index 875f345..f214687 100644 --- a/lib/comfycamp_web/controllers/oidc_app_html/index.html.heex +++ b/lib/comfycamp_web/controllers/oidc_app_html/index.html.heex @@ -1,9 +1,9 @@
<.header> - Listing Oidc apps + Listing OpenID Connect apps <:actions> <.link href={~p"/admin/oidc_apps/new"}> - <.button>New Oidc app + <.button>New OpenID Connect app diff --git a/lib/comfycamp_web/controllers/oidc_app_html/new.html.heex b/lib/comfycamp_web/controllers/oidc_app_html/new.html.heex index 802ec6d..1971d84 100644 --- a/lib/comfycamp_web/controllers/oidc_app_html/new.html.heex +++ b/lib/comfycamp_web/controllers/oidc_app_html/new.html.heex @@ -1,10 +1,10 @@
<.header> - New Oidc app + New OpenID Connect app <:subtitle>Use this form to manage oidc_app records in your database. - <.oidc_app_form changeset={@changeset} action={~p"/admin/oidc_apps"} /> + <.back navigate={~p"/admin/oidc_apps"}>Back to OpenID apps - <.back navigate={~p"/admin/oidc_apps"}>Back to oidc_apps + <.oidc_app_form changeset={@changeset} action={~p"/admin/oidc_apps"} />
diff --git a/lib/comfycamp_web/controllers/oidc_app_html/oidc_app_form.html.heex b/lib/comfycamp_web/controllers/oidc_app_html/oidc_app_form.html.heex index b44d18b..ad856de 100644 --- a/lib/comfycamp_web/controllers/oidc_app_html/oidc_app_form.html.heex +++ b/lib/comfycamp_web/controllers/oidc_app_html/oidc_app_form.html.heex @@ -3,9 +3,9 @@ <.error :if={@changeset.action}> Oops, something went wrong! Please check the errors below. + <.input field={f[:client_id]} type="text" label="Client ID" readonly /> + <.input field={f[:client_secret]} type="password" label="Client secret" readonly /> <.input field={f[:name]} type="text" label="Name" /> - <.input field={f[:client_id]} type="text" label="Client" /> - <.input field={f[:client_secret]} type="text" label="Client secret" /> <.input field={f[:enabled]} type="checkbox" label="Enabled" /> <:actions> <.button>Save Oidc app diff --git a/lib/comfycamp_web/controllers/oidc_app_html/show.html.heex b/lib/comfycamp_web/controllers/oidc_app_html/show.html.heex index 88402f1..001686d 100644 --- a/lib/comfycamp_web/controllers/oidc_app_html/show.html.heex +++ b/lib/comfycamp_web/controllers/oidc_app_html/show.html.heex @@ -1,10 +1,10 @@
<.header> - Oidc app <%= @oidc_app.id %> + OpenID app "<%= @oidc_app.name %>" <:subtitle>This is a oidc_app record from your database. <:actions> <.link href={~p"/admin/oidc_apps/#{@oidc_app}/edit"}> - <.button>Edit oidc_app + <.button>Edit OpenID app @@ -16,5 +16,5 @@ <:item title="Enabled"><%= @oidc_app.enabled %> - <.back navigate={~p"/admin/oidc_apps"}>Back to oidc_apps + <.back navigate={~p"/admin/oidc_apps"}>Back to OpenID apps
diff --git a/priv/repo/migrations/20240906061751_alter_oidc_app_id.exs b/priv/repo/migrations/20240906061751_alter_oidc_app_id.exs new file mode 100644 index 0000000..8767df0 --- /dev/null +++ b/priv/repo/migrations/20240906061751_alter_oidc_app_id.exs @@ -0,0 +1,12 @@ +defmodule Comfycamp.Repo.Migrations.AlterOidcAppId do + use Ecto.Migration + + def change do + drop(constraint("oidc_apps", "oidc_apps_pkey")) + + alter table(:oidc_apps) do + modify(:client_id, :string, primary_key: true) + remove :id + end + end +end diff --git a/test/comfycamp/sso_test.exs b/test/comfycamp/sso_test.exs index 73ed684..ec46802 100644 --- a/test/comfycamp/sso_test.exs +++ b/test/comfycamp/sso_test.exs @@ -17,7 +17,7 @@ defmodule Comfycamp.SSOTest do test "get_oidc_app!/1 returns the oidc_app with given id" do oidc_app = oidc_app_fixture() - assert SSO.get_oidc_app!(oidc_app.id) == oidc_app + assert SSO.get_oidc_app!(oidc_app.client_id) == oidc_app end test "create_oidc_app/1 with valid data creates a oidc_app" do @@ -59,13 +59,13 @@ defmodule Comfycamp.SSOTest do test "update_oidc_app/2 with invalid data returns error changeset" do oidc_app = oidc_app_fixture() assert {:error, %Ecto.Changeset{}} = SSO.update_oidc_app(oidc_app, @invalid_attrs) - assert oidc_app == SSO.get_oidc_app!(oidc_app.id) + assert oidc_app == SSO.get_oidc_app!(oidc_app.client_id) end test "delete_oidc_app/1 deletes the oidc_app" do oidc_app = oidc_app_fixture() assert {:ok, %OIDCApp{}} = SSO.delete_oidc_app(oidc_app) - assert_raise Ecto.NoResultsError, fn -> SSO.get_oidc_app!(oidc_app.id) end + assert_raise Ecto.NoResultsError, fn -> SSO.get_oidc_app!(oidc_app.client_id) end end test "change_oidc_app/1 returns a oidc_app changeset" do diff --git a/test/comfycamp_web/controllers/oidc_app_controller_test.exs b/test/comfycamp_web/controllers/oidc_app_controller_test.exs index 17260f0..dce266b 100644 --- a/test/comfycamp_web/controllers/oidc_app_controller_test.exs +++ b/test/comfycamp_web/controllers/oidc_app_controller_test.exs @@ -4,16 +4,16 @@ defmodule ComfycampWeb.OIDCAppControllerTest do import Comfycamp.SSOFixtures @create_attrs %{ - enabled: true, + client_id: "some_client_id", + client_secret: "some client_secret", name: "some name", - client_id: "some client_id", - client_secret: "some client_secret" + enabled: true } @update_attrs %{ - enabled: false, + client_id: "some_client_id", + client_secret: "some updated client_secret", name: "some updated name", - client_id: "some updated client_id", - client_secret: "some updated client_secret" + enabled: false } @invalid_attrs %{enabled: nil, name: nil, client_id: nil, client_secret: nil} @@ -22,7 +22,7 @@ defmodule ComfycampWeb.OIDCAppControllerTest do test "lists all oidc_apps", %{conn: conn} do conn = get(conn, ~p"/admin/oidc_apps") - assert html_response(conn, 200) =~ "Listing Oidc apps" + assert html_response(conn, 200) =~ "Listing OpenID Connect apps" end end @@ -31,7 +31,7 @@ defmodule ComfycampWeb.OIDCAppControllerTest do test "renders form", %{conn: conn} do conn = get(conn, ~p"/admin/oidc_apps/new") - assert html_response(conn, 200) =~ "New Oidc app" + assert html_response(conn, 200) =~ "New OpenID Connect app" end end @@ -41,16 +41,16 @@ defmodule ComfycampWeb.OIDCAppControllerTest do test "redirects to show when data is valid", %{conn: conn} do conn = post(conn, ~p"/admin/oidc_apps", oidc_app: @create_attrs) - assert %{id: id} = redirected_params(conn) - assert redirected_to(conn) == ~p"/admin/oidc_apps/#{id}" + assert %{id: client_id} = redirected_params(conn) + assert redirected_to(conn) == ~p"/admin/oidc_apps/#{client_id}" - conn = get(conn, ~p"/admin/oidc_apps/#{id}") - assert html_response(conn, 200) =~ "Oidc app #{id}" + conn = get(conn, ~p"/admin/oidc_apps/#{client_id}") + assert html_response(conn, 200) =~ "OpenID app" end test "renders errors when data is invalid", %{conn: conn} do conn = post(conn, ~p"/admin/oidc_apps", oidc_app: @invalid_attrs) - assert html_response(conn, 200) =~ "New Oidc app" + assert html_response(conn, 200) =~ "New OpenID Connect app" end end @@ -59,7 +59,7 @@ defmodule ComfycampWeb.OIDCAppControllerTest do test "renders form for editing chosen oidc_app", %{conn: conn, oidc_app: oidc_app} do conn = get(conn, ~p"/admin/oidc_apps/#{oidc_app}/edit") - assert html_response(conn, 200) =~ "Edit Oidc app" + assert html_response(conn, 200) =~ "Edit OpenID app" end end @@ -76,7 +76,7 @@ defmodule ComfycampWeb.OIDCAppControllerTest do test "renders errors when data is invalid", %{conn: conn, oidc_app: oidc_app} do conn = put(conn, ~p"/admin/oidc_apps/#{oidc_app}", oidc_app: @invalid_attrs) - assert html_response(conn, 200) =~ "Edit Oidc app" + assert html_response(conn, 200) =~ "Edit OpenID app" end end diff --git a/test/support/fixtures/sso_fixtures.ex b/test/support/fixtures/sso_fixtures.ex index cd2e5cb..33f7de4 100644 --- a/test/support/fixtures/sso_fixtures.ex +++ b/test/support/fixtures/sso_fixtures.ex @@ -11,10 +11,10 @@ defmodule Comfycamp.SSOFixtures do {:ok, oidc_app} = attrs |> Enum.into(%{ - client_id: "some client_id", + client_id: "some_client_id", client_secret: "some client_secret", - enabled: true, - name: "some name" + name: "some name", + enabled: true }) |> Comfycamp.SSO.create_oidc_app()