Wire/test registration and password reset

config/config.exs

@@ -9,7 +9,8 @@ use Mix.Config
 
 config :recycledcloud,
   namespace: RecycledCloud,
-  ecto_repos: [RecycledCloud.Repo]
+  ecto_repos: [RecycledCloud.Repo],
+  enable_registration: true
 
 # Configures the endpoint
 config :recycledcloud, RecycledCloudWeb.Endpoint,

lib/recycledcloud/accounts.ex

@@ -89,12 +89,9 @@ defmodule RecycledCloud.Accounts do
         case User.register(user) do
           {:ok, result} -> {:ok, result}
           {:error, :entryAlreadyExists} ->
-            {
+            err = changeset
-              :error,
+              |> Ecto.Changeset.add_error(:username, "has already been taken")
-              Ecto.Changeset.add_error(
+              |> Ecto.Changeset.apply_action(:update)
-                changeset, :username, "has already been taken"
-              )
-            }
         end
       err -> err
     end
@@ -216,7 +213,11 @@ defmodule RecycledCloud.Accounts do
             Repo.delete_all(UserToken.user_and_contexts_query(user, :all))
             {:ok, user}
           {:error, err} ->
-            changeset_errors = [current_password: {"Unknown error: #{err}", [err: inspect(err)]}]
+            msg = {"Unknown error: #{err}", [err: inspect(err)]}
+            changeset_errors = [
+              current_password: msg,
+              password: msg
+            ]
             updated_changeset = changeset
                                 |> Map.put(:action, :update)
                                 |> Map.put(:errors, changeset_errors)
@@ -244,7 +245,7 @@ defmodule RecycledCloud.Accounts do
                 |> User.password_changeset(attrs)
                 |> User.validate_current_password(current_password)
 
-    set_user_password(user, changeset, attrs[:password])
+    set_user_password(user, changeset, attrs["password"])
   end
 
   ## Session
@@ -376,7 +377,7 @@ defmodule RecycledCloud.Accounts do
     changeset = user
                 |> User.password_changeset(attrs)
 
-    set_user_password(user, changeset, attrs[:password])
+    set_user_password(user, changeset, attrs["password"])
   end
 
   ## Keys

lib/recycledcloud/accounts/user.ex

@@ -54,15 +54,33 @@ defmodule RecycledCloud.Accounts.User do
     end
   end
 
+  defp get_last_ldap_uid() do
+    query = fn ldap_conn ->
+      LDAP.search(ldap_conn, :objectClass, 'posixAccount')
+    end
+
+    case query |> LDAP.execute do
+      {:ok, entries} ->
+        uids = Enum.map(entries, fn entry ->
+          entry.uidNumber |> Enum.at(0) |> List.to_integer
+        end)
+
+      {:ok, uids |> Enum.max()}
+      {:error, err} -> {:error, err}
+    end
+  end
+
   defp create_ldap_user(%User{} = user) do
-    # FIXME: read dn template from conf, gid_number
+    # TODO: read dn template from conf, gid_number
-    # FIXME: generate uidNumber
     conf = Application.get_env(:recycledcloud, :ldap)
     basetree = conf |> Keyword.get(:base_dn)
+    default_group_gid = conf |> Keyword.get(:default_group_gid, 1000)
+
     dn = ("uid=" <> user.username <> ",ou=Users," <> basetree) |> String.to_charlist
 
-    uid_number = 1234
+    {:ok, last_uid_number} = get_last_ldap_uid()
-    gid_number = 1000
+    uid_number =  last_uid_number + 1
+    gid_number = default_group_gid
 
     ldif = [
       {'uid', [user.username]},
@@ -222,12 +240,14 @@ defmodule RecycledCloud.Accounts.User do
     end
   end
 
-  def set_password(%User{} = user, new_password)
+  def set_password(%User{dn: nil} = _user, _), do: {:error, "User DN must be set"}
-  when byte_size(new_password) > 0 do
+  def set_password(_, nil), do: {:error, "Password cannot be nil"}
+  def set_password(%User{dn: dn} = _user, new_password) when
+  byte_size(new_password) > 0do
     query = fn ldap_conn ->
       :eldap.modify_password(
         ldap_conn,
-        to_charlist(user.dn),
+        to_charlist(dn),
         to_charlist(new_password)
       )
     end

lib/recycledcloud_web/controllers/user_registration_controller.ex

@@ -5,33 +5,46 @@ defmodule RecycledCloudWeb.UserRegistrationController do
   alias RecycledCloud.Accounts.User
   alias RecycledCloudWeb.UserAuth
 
+  defp is_registration_enabled?() do
+    Application.get_env(:recycledcloud, :enable_registration)
+  end
+
   def new(conn, _params) do
     changeset = Accounts.change_user_registration(%User{})
+
+    with_notice = unless is_registration_enabled?() do
       conn
       |> put_flash(:error, "Registration is disabled for the time being.")
+    else
+      conn
+    end
+
+    with_notice
     |> render("new.html", changeset: changeset)
   end
 
   def create(conn, %{"user" => user_params}) do
     changeset = Accounts.change_user_registration(%User{})
 
+    unless is_registration_enabled?() do
       conn
       |> redirect(to: Routes.user_registration_path(conn, :new))
+    else
+      case Accounts.register_user(user_params) do
+        {:ok, user} ->
+          {:ok, _} =
+            Accounts.deliver_user_confirmation_instructions(
+              user,
+              &Routes.user_confirmation_url(conn, :confirm, &1)
+            )
 
-    #case Accounts.register_user(user_params) do
+          conn
-    #  {:ok, user} ->
+          |> put_flash(:info, "User created successfully.")
-    #    {:ok, _} =
+          |> UserAuth.log_in_user(user)
-    #      Accounts.deliver_user_confirmation_instructions(
-    #        user,
-    #        &Routes.user_confirmation_url(conn, :confirm, &1)
-    #      )
 
-    #    conn
+        {:error, %Ecto.Changeset{} = changeset} ->
-    #    |> put_flash(:info, "User created successfully.")
+          render(conn, "new.html", changeset: changeset)
-    #    |> UserAuth.log_in_user(user)
+      end
-
+    end
-    #  {:error, %Ecto.Changeset{} = changeset} ->
-    #    render(conn, "new.html", changeset: changeset)
-    #end
   end
 end

lib/recycledcloud_web/controllers/user_reset_password_controller.ex

@@ -21,7 +21,7 @@ defmodule RecycledCloudWeb.UserResetPasswordController do
     conn
     |> put_flash(
       :info,
-      "If your email is in our system, you will receive instructions to reset your password shortly."
+      "If your account is in our system, you will receive instructions to reset your password shortly."
     )
     |> redirect(to: "/")
   end
@@ -33,7 +33,10 @@ defmodule RecycledCloudWeb.UserResetPasswordController do
   # Do not log in the user after reset password to avoid a
   # leaked token giving the user access to the account.
   def update(conn, %{"user" => user_params}) do
-    case Accounts.reset_user_password(conn.assigns.user, user_params) do
+    # FIXME: understand why conn.assigns.user is not populated with LDAP
+    # attributes.
+    user = conn.assigns.user.id |> Accounts.User.get!()
+    case Accounts.reset_user_password(user, user_params) do
       {:ok, _} ->
         conn
         |> put_flash(:info, "Password reset successfully.")

lib/recycledcloud_web/templates/user_registration/new.html.eex

@@ -12,6 +12,11 @@
 
   <br />
 
+  <%= text_input f, :email, required: true, placeholder: "E-mail" %>
+  <%= error_tag f, :email %>
+
+  <br />
+
   <%= password_input f, :password, required: true, placeholder: "Password" %>
   <%= error_tag f, :password %>
 

lib/recycledcloud_web/templates/user_reset_password/edit.html.eex

@@ -7,12 +7,12 @@
     </div>
   <% end %>
 
-  <%= label f, :password, "New password" %>
+  <%= password_input f, :password, required: true, placeholder: "New password" %>
-  <%= password_input f, :password, required: true %>
   <%= error_tag f, :password %>
 
-  <%= label f, :password_confirmation, "Confirm new password" %>
+  <br />
-  <%= password_input f, :password_confirmation, required: true %>
+
+  <%= password_input f, :password_confirmation, required: true, placeholder: "New password confirmation" %>
   <%= error_tag f, :password_confirmation %>
 
   <div>

test/recycledcloud/accounts_test.exs

@@ -262,14 +262,14 @@ defmodule RecycledCloud.AccountsTest do
       too_long = String.duplicate("db", 100)
 
       {:error, changeset} =
-        Accounts.update_user_password(user, valid_user_password(), %{password: too_long})
+        Accounts.update_user_password(user, valid_user_password(), %{"password" => too_long})
 
       assert "should be at most 80 character(s)" in errors_on(changeset).password
     end
 
     test "validates current password", %{user: user} do
       {:error, changeset} =
-        Accounts.update_user_password(user, "invalid", %{password: valid_user_password()})
+        Accounts.update_user_password(user, "invalid", %{"password" => valid_user_password()})
 
       assert %{current_password: ["is not valid"]} = errors_on(changeset)
     end
@@ -277,7 +277,7 @@ defmodule RecycledCloud.AccountsTest do
     test "updates the password", %{user: user} do
       {:ok, user} =
         Accounts.update_user_password(user, valid_user_password(), %{
-          password: "new valid password"
+          "password" => "new valid password"
         })
 
       assert is_nil(user.password)
@@ -289,7 +289,7 @@ defmodule RecycledCloud.AccountsTest do
 
       {:ok, _} =
         Accounts.update_user_password(user, valid_user_password(), %{
-          password: "new valid password"
+          "password" => "new valid password"
         })
 
       refute Repo.get_by(UserToken, user_id: user.id)
@@ -481,7 +481,7 @@ defmodule RecycledCloud.AccountsTest do
 
     test "deletes all tokens for the given user", %{user: user} do
       _ = Accounts.generate_user_session_token(user)
-      {:ok, _} = Accounts.reset_user_password(user, %{password: "new valid password"})
+      {:ok, _} = Accounts.reset_user_password(user, %{"password" => "new valid password"})
       refute Repo.get_by(UserToken, user_id: user.id)
     end
   end

test/recycledcloud_web/controllers/user_reset_password_controller_test.exs

@@ -22,22 +22,22 @@ defmodule RecycledCloudWeb.UserResetPasswordControllerTest do
     test "sends a new reset password token", %{conn: conn, user: user} do
       conn =
         post(conn, Routes.user_reset_password_path(conn, :create), %{
-          "user" => %{"email" => user.email}
+          "user" => %{"username" => user.username}
         })
 
       assert redirected_to(conn) == "/"
-      assert get_flash(conn, :info) =~ "If your email is in our system"
+      assert get_flash(conn, :info) =~ "If your account is in our system"
       assert Repo.get_by!(Accounts.UserToken, user_id: user.id).context == "reset_password"
     end
 
-    test "does not send reset password token if email is invalid", %{conn: conn} do
+    test "does not send reset password token if username is invalid", %{conn: conn} do
       conn =
         post(conn, Routes.user_reset_password_path(conn, :create), %{
-          "user" => %{"email" => "unknown@example.com"}
+          "user" => %{"username" => "unknown"}
         })
 
       assert redirected_to(conn) == "/"
-      assert get_flash(conn, :info) =~ "If your email is in our system"
+      assert get_flash(conn, :info) =~ "If your account is in our system"
       assert Repo.all(Accounts.UserToken) == []
     end
   end
@@ -86,21 +86,21 @@ defmodule RecycledCloudWeb.UserResetPasswordControllerTest do
       assert redirected_to(conn) == Routes.user_session_path(conn, :new)
       refute get_session(conn, :user_token)
       assert get_flash(conn, :info) =~ "Password reset successfully"
-      assert Accounts.get_user_by_email_and_password(user.email, "new valid password")
+      assert Accounts.get_user_by_username_and_password(user.username, "new valid password")
     end
 
     test "does not reset password on invalid data", %{conn: conn, token: token} do
       conn =
         put(conn, Routes.user_reset_password_path(conn, :update, token), %{
           "user" => %{
-            "password" => "too short",
+            "password" => "short",
             "password_confirmation" => "does not match"
           }
         })
 
       response = html_response(conn, 200)
       assert response =~ "<h1>Reset password</h1>"
-      assert response =~ "should be at least 12 character(s)"
+      assert response =~ "should be at least 6 character(s)"
       assert response =~ "does not match password"
     end
 

test/recycledcloud_web/controllers/user_settings_controller_test.exs

@@ -10,7 +10,7 @@ defmodule RecycledCloudWeb.UserSettingsControllerTest do
     test "renders settings page", %{conn: conn} do
       conn = get(conn, Routes.user_settings_path(conn, :edit))
       response = html_response(conn, 200)
-      assert response =~ "<h1>Settings</h1>"
+      assert response =~ "<h1>Account settings</h1>"
     end
 
     test "redirects if user is not logged in" do
@@ -70,7 +70,7 @@ defmodule RecycledCloudWeb.UserSettingsControllerTest do
 
       assert redirected_to(conn) == Routes.user_settings_path(conn, :edit)
       assert get_flash(conn, :info) =~ "A link to confirm your email"
-      assert Accounts.get_user_by_email(user.email)
+      assert Accounts.get_user_by_username(user.username).email == user.email
     end
 
     test "does not update email on invalid data", %{conn: conn} do
@@ -82,7 +82,6 @@ defmodule RecycledCloudWeb.UserSettingsControllerTest do
         })
 
       response = html_response(conn, 200)
-      assert response =~ "<h1>Settings</h1>"
       assert response =~ "must have the @ sign and no spaces"
       assert response =~ "is not valid"
     end
@@ -104,8 +103,7 @@ defmodule RecycledCloudWeb.UserSettingsControllerTest do
       conn = get(conn, Routes.user_settings_path(conn, :confirm_email, token))
       assert redirected_to(conn) == Routes.user_settings_path(conn, :edit)
       assert get_flash(conn, :info) =~ "Email changed successfully"
-      refute Accounts.get_user_by_email(user.email)
+      assert Accounts.get_user_by_username(user.username).email == email
-      assert Accounts.get_user_by_email(email)
 
       conn = get(conn, Routes.user_settings_path(conn, :confirm_email, token))
       assert redirected_to(conn) == Routes.user_settings_path(conn, :edit)
@@ -116,7 +114,7 @@ defmodule RecycledCloudWeb.UserSettingsControllerTest do
       conn = get(conn, Routes.user_settings_path(conn, :confirm_email, "oops"))
       assert redirected_to(conn) == Routes.user_settings_path(conn, :edit)
       assert get_flash(conn, :error) =~ "Email change link is invalid or it has expired"
-      assert Accounts.get_user_by_email(user.email)
+      assert Accounts.get_user_by_username(user.username)
     end
 
     test "redirects if user is not logged in", %{token: token} do

test/support/ldap_test_environment.ex

@@ -23,7 +23,7 @@ defmodule RecycledCloud.LDAPTestEnvironment do
           :ok ->
             # Wait for LDAP server to be populated.
             # FIXME: poll state instead of taking a nap!
-            Process.sleep(1000)
+            Process.sleep(2000)
 
             {:ok, port, container}
           :timeout -> {:error, :timeout}