Addressing review feedbacks

trento-project · May 27, 2024 · 8035315 · 8035315
1 parent 3b6d5ed
commit 8035315
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 16 deletions.
diff --git a/lib/trento/users.ex b/lib/trento/users.ex
@@ -148,12 +148,12 @@ defmodule Trento.Users do
   def initiate_totp_enrollment(%User{totp_enabled_at: nil} = user) do
     result =
       Ecto.Multi.new()
-      |> Ecto.Multi.run(:user_without_totp, fn _, _ ->
+      |> Ecto.Multi.run(:reset_totp, fn _, _ ->
         reset_totp(user)
       end)
       |> Ecto.Multi.run(
-        :user_totp_enrolled,
-        fn _, %{user_without_totp: user} ->
+        :enroll_totp,
+        fn _, %{reset_totp: user} ->
           update_user_totp(user, %{
             totp_secret: NimbleTOTP.secret()
           })
@@ -162,7 +162,7 @@ defmodule Trento.Users do
       |> Repo.transaction()
 
     case result do
-      {:ok, %{user_totp_enrolled: %User{totp_secret: totp_secret, username: username}}} ->
+      {:ok, %{enroll_totp: %User{totp_secret: totp_secret, username: username}}} ->
         {:ok,
          %{
            secret: totp_secret,
@@ -179,8 +179,11 @@ defmodule Trento.Users do
 
   def confirm_totp_enrollment(%User{id: 1}, _), do: {:error, :forbidden}
 
-  def confirm_totp_enrollment(%User{totp_secret: totp_secret, totp_enabled_at: nil} = user, totp) do
-    if NimbleTOTP.valid?(totp_secret, totp) do
+  def confirm_totp_enrollment(
+        %User{totp_secret: totp_secret, totp_enabled_at: nil} = user,
+        totp_code
+      ) do
+    if NimbleTOTP.valid?(totp_secret, totp_code) do
       now = DateTime.utc_now()
       update_user_totp(user, %{totp_enabled_at: now, totp_last_used_at: now})
     else

diff --git a/lib/trento_web/controllers/v1/profile_controller.ex b/lib/trento_web/controllers/v1/profile_controller.ex
@@ -91,10 +91,10 @@ defmodule TrentoWeb.V1.ProfileController do
 
   def confirm_totp_enrollment(%{body_params: body_params} = conn, _) do
     %User{} = user = Pow.Plug.current_user(conn)
-    totp = Map.get(body_params, :totp)
+    totp_code = Map.get(body_params, :totp_code)
 
     with {:ok, %User{totp_enabled_at: totp_enabled_at}} <-
-           Users.confirm_totp_enrollment(user, totp) do
+           Users.confirm_totp_enrollment(user, totp_code) do
       render(conn, "totp_enrollment_completed.json", %{totp_enabled_at: totp_enabled_at})
     end
   end

diff --git a/lib/trento_web/openapi/v1/schema/user.ex b/lib/trento_web/openapi/v1/schema/user.ex
@@ -11,7 +11,7 @@ defmodule TrentoWeb.OpenApi.V1.Schema.User do
 
     @schema %Schema{
       title: "UserTOTPEnrollmentPayload",
-      description: "Trento User totp enrollment payload",
+      description: "Trento User TOTP enrollment payload",
       type: :object,
       additionalProperties: false,
       properties: %{
@@ -59,13 +59,13 @@ defmodule TrentoWeb.OpenApi.V1.Schema.User do
       type: :object,
       additionalProperties: false,
       properties: %{
-        totp: %Schema{
+        totp_code: %Schema{
           type: :string,
           description: "TOTP generated from enrollment secret",
           nullable: false
         }
       },
-      required: [:totp]
+      required: [:totp_code]
     }
 
     def schema, do: @schema

diff --git a/test/trento_web/controllers/v1/profile_controller_test.exs b/test/trento_web/controllers/v1/profile_controller_test.exs
@@ -121,7 +121,7 @@ defmodule TrentoWeb.V1.ProfileControllerTest do
     |> assert_schema("Forbidden", api_spec)
   end
 
-  test "should not reset totp when a reset is requested by the default admin", %{
+  test "should not reset totp when a reset is requested for the default admin", %{
     conn: conn,
     api_spec: api_spec
   } do
@@ -165,7 +165,7 @@ defmodule TrentoWeb.V1.ProfileControllerTest do
 
     conn
     |> put_req_header("content-type", "application/json")
-    |> post("/api/v1/profile/totp_enrollment", %{totp: "12345"})
+    |> post("/api/v1/profile/totp_enrollment", %{totp_code: "12345"})
     |> json_response(:forbidden)
     |> assert_schema("Forbidden", api_spec)
   end
@@ -190,7 +190,7 @@ defmodule TrentoWeb.V1.ProfileControllerTest do
 
     conn
     |> put_req_header("content-type", "application/json")
-    |> post("/api/v1/profile/totp_enrollment", %{totp: "12345"})
+    |> post("/api/v1/profile/totp_enrollment", %{totp_code: "12345"})
     |> json_response(:unprocessable_entity)
     |> assert_schema("UnprocessableEntity", api_spec)
   end
@@ -213,7 +213,7 @@ defmodule TrentoWeb.V1.ProfileControllerTest do
 
     conn
     |> put_req_header("content-type", "application/json")
-    |> post("/api/v1/profile/totp_enrollment", %{totp: "12345"})
+    |> post("/api/v1/profile/totp_enrollment", %{totp_code: "12345"})
     |> json_response(:unprocessable_entity)
     |> assert_schema("UnprocessableEntity", api_spec)
   end
@@ -238,7 +238,7 @@ defmodule TrentoWeb.V1.ProfileControllerTest do
 
     conn
     |> put_req_header("content-type", "application/json")
-    |> post("/api/v1/profile/totp_enrollment", %{totp: NimbleTOTP.verification_code(secret)})
+    |> post("/api/v1/profile/totp_enrollment", %{totp_code: NimbleTOTP.verification_code(secret)})
     |> json_response(:ok)
     |> assert_schema("UserTOTPEnrollmentConfirmPayload", api_spec)
   end