Skip to content

Commit

Permalink
Basic User management (#2529)
Browse files Browse the repository at this point in the history
* Enforce password security un user schema

* Argon2 hash algorithm for user password

* Add fullname and email fields to user schema

* Mix phx gen json scaffolding for users

* Users context

* mix format

* User controller refactored for the new users context

* Pow custom context for getting non-deleted users

* Unique email for users

* Update release task and seed for users

* Users context prevents deletion of user with id 1

* Username cannot be updated

* Created and updated timestamps in users view

* Change of the username when users are deleted

* Add lock feature to Users

* Locked and deleted users are excluded from renew

* Session controller tests updated

* /me endpoint returns also user id

* User socket authenticatio

* User controller broadcast user actions on user topic

* Access token expiration to 3 minutes

* Fix could not delete admin user in context

* Trento default admin enabled

* User controller with spex

* Fix app jwt auth plug test typo

* User controller actions are dispatched to per user channels

* fix access token test

* fix alias in factory file

* mix credo

* fix mispell

* mix credo

* removed comments from user_controller_test

* Addressing review feedbacks

* Introduced users factory

* Addresing feedbacks

* Updated trento_dev sql regression dump

* Revert "Updated trento_dev sql regression dump"

This reverts commit ccce860.

* Ecto seed with user password update on conflict

* Run db seed on regression tests
  • Loading branch information
CDimonaco authored Apr 22, 2024
1 parent 0f44680 commit f4ed280
Show file tree
Hide file tree
Showing 33 changed files with 1,328 additions and 40 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,8 @@ jobs:
postgresql_version: "15"
- name: Run DB migrations
run: mix ecto.migrate
- name: Run DB seed
run: mix run priv/repo/seeds.exs
- name: Run trento detached
run: mix phx.server &
- name: Install photofinish
Expand Down
5 changes: 4 additions & 1 deletion assets/js/lib/network/socket.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// eslint-disable-next-line
import { Socket } from 'phoenix';
import { logMessage, logError } from '@lib/log';
import { getAccessTokenFromStore } from '@lib/auth';

export const joinChannel = (channel) => {
channel
Expand All @@ -12,7 +13,9 @@ export const joinChannel = (channel) => {
};

export const initSocketConnection = () => {
const socket = new Socket('/socket', {});
const socket = new Socket('/socket', {
params: { access_token: getAccessTokenFromStore() },
});
socket.connect();

return socket;
Expand Down
5 changes: 3 additions & 2 deletions config/config.exs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ config :trento, event_stores: [Trento.EventStore]
config :trento, :pow,
user: Trento.Users.User,
repo: Trento.Repo,
users_context: Trento.Users,
web_module: TrentoWeb,
extensions: [PowPersistentSession],
controller_callbacks: Pow.Extension.Phoenix.ControllerCallbacks
Expand Down Expand Up @@ -168,8 +169,8 @@ config :trento, :jwt_authentication,
issuer: "https://github.com/trento-project/web",
app_audience: "trento_app",
api_key_audience: "trento_api_key",
# Seconds, 10 minutes
access_token_expiration: 600,
# Seconds, 3 minutes
access_token_expiration: 180,
# Seconds, 6 hours
refresh_token_expiration: 21600

Expand Down
9 changes: 6 additions & 3 deletions lib/trento/release.ex
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ defmodule Trento.Release do
installed.
"""

alias Pow.Ecto.Schema.Password
alias Trento.Settings.ApiKeySettings

@app :trento
Expand Down Expand Up @@ -71,15 +70,19 @@ defmodule Trento.Release do

admin_user = System.get_env("ADMIN_USER", "admin")
admin_password = System.get_env("ADMIN_PASSWORD", "adminpassword")
admin_email = System.get_env("ADMIN_EMAIL", "admin@trento.suse.com")

%Trento.Users.User{}
|> Trento.Users.User.changeset(%{
username: admin_user,
password: admin_password,
confirm_password: admin_password
confirm_password: admin_password,
email: admin_email,
enabled: true,
fullname: "Trento Default Admin"
})
|> Trento.Repo.insert!(
on_conflict: [set: [password_hash: Password.pbkdf2_hash(admin_password)]],
on_conflict: [set: [password_hash: Argon2.hash_pwd_salt(admin_password)]],
conflict_target: :username
)
end
Expand Down
78 changes: 78 additions & 0 deletions lib/trento/users.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
defmodule Trento.Users do
@moduledoc """
The Users context.
"""

use Pow.Ecto.Context,
repo: Trento.Repo,
user: Trento.Users.User

import Ecto.Query, warn: false
alias Trento.Repo

alias Trento.Users.User

@impl true
@doc """
get_by function overrides the one defined in Pow.Ecto.Context,
we retrieve the user by username as traditional Pow flow but we also exclude
deleted and locked users
"""
def get_by(clauses) do
username = clauses[:username]

User
|> where([u], is_nil(u.deleted_at) and is_nil(u.locked_at) and u.username == ^username)
|> Repo.one()
end

def list_users do
User
|> where([u], is_nil(u.deleted_at))
|> Repo.all()
end

def get_user(id) do
case User
|> where([u], is_nil(u.deleted_at) and u.id == ^id)
|> Repo.one() do
nil -> {:error, :not_found}
user -> {:ok, user}
end
end

def create_user(attrs \\ %{}) do
%User{}
|> User.changeset(attrs)
|> Repo.insert()
end

def update_user(%User{id: 1}, _), do: {:error, :operation_not_permitted}

def update_user(%User{locked_at: nil} = user, %{enabled: false} = attrs) do
do_update(user, Map.put(attrs, :locked_at, DateTime.utc_now()))
end

def update_user(%User{locked_at: locked_at} = user, %{enabled: false} = attrs)
when not is_nil(locked_at) do
do_update(user, attrs)
end

def update_user(%User{} = user, attrs) do
do_update(user, Map.put(attrs, :locked_at, nil))
end

def delete_user(%User{id: 1}), do: {:error, :operation_not_permitted}

def delete_user(%User{} = user) do
user
|> User.delete_changeset(%{deleted_at: DateTime.utc_now()})
|> Repo.update()
end

defp do_update(user, attrs) do
user
|> User.update_changeset(attrs)
|> Repo.update()
end
end
92 changes: 91 additions & 1 deletion lib/trento/users/user.ex
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,112 @@ defmodule Trento.Users.User do
@moduledoc false

use Ecto.Schema
import Ecto.Changeset

use Pow.Ecto.Schema,
user_id_field: :username
user_id_field: :username,
password_hash_methods: {&Argon2.hash_pwd_salt/1, &Argon2.verify_pass/2}

use Pow.Extension.Ecto.Schema,
extensions: [PowPersistentSession]

alias EctoCommons.EmailValidator

@sequences ["01234567890", "abcdefghijklmnopqrstuvwxyz"]
@max_sequential_chars 3

schema "users" do
pow_user_fields()

field :email, :string
field :fullname, :string
field :deleted_at, :utc_datetime_usec
field :locked_at, :utc_datetime_usec

timestamps(type: :utc_datetime_usec)
end

def changeset(user, attrs) do
user
|> pow_changeset(attrs)
|> pow_extension_changeset(attrs)
|> validate_password()
|> custom_fields_changeset(attrs)
end

def update_changeset(user, attrs) do
user
|> pow_password_changeset(attrs)
|> pow_extension_changeset(attrs)
|> validate_password()
|> custom_fields_changeset(attrs)
|> lock_changeset(attrs)
end

def delete_changeset(%__MODULE__{username: username} = user, %{deleted_at: deleted_at} = attrs) do
user
|> cast(attrs, [:deleted_at])
|> validate_required(:deleted_at)
|> put_change(:username, "#{username}__#{deleted_at}")
end

defp lock_changeset(user, attrs) do
cast(user, attrs, [:locked_at])
end

defp validate_password(changeset) do
changeset
|> validate_no_repetitive_characters()
|> validate_no_sequential_characters()
end

defp validate_no_repetitive_characters(changeset) do
Ecto.Changeset.validate_change(changeset, :password, fn :password, password ->
case repetitive_characters?(password) do
true -> [password: "has repetitive characters"]
false -> []
end
end)
end

defp repetitive_characters?(password) when is_binary(password) do
password
|> String.to_charlist()
|> repetitive_characters?()
end

defp repetitive_characters?([c, c, c | _rest]), do: true
defp repetitive_characters?([_c | rest]), do: repetitive_characters?(rest)
defp repetitive_characters?([]), do: false

defp validate_no_sequential_characters(changeset) do
Ecto.Changeset.validate_change(changeset, :password, fn :password, password ->
case sequential_characters?(password) do
true -> [password: "has sequential characters"]
false -> []
end
end)
end

defp sequential_characters?(password) do
Enum.any?(@sequences, &sequential_characters?(password, &1))
end

defp sequential_characters?(password, sequence) do
max = String.length(sequence) - 1 - @max_sequential_chars

Enum.any?(0..max, fn x ->
pattern = String.slice(sequence, x, @max_sequential_chars + 1)

String.contains?(password, pattern)
end)
end

defp custom_fields_changeset(user, attrs) do
user
|> cast(attrs, [:email, :fullname])
|> validate_required([:email, :fullname])
|> EmailValidator.validate_email(:email, checks: [:pow])
|> unique_constraint(:email)
end
end
33 changes: 33 additions & 0 deletions lib/trento_web/channels/user_channel.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
defmodule TrentoWeb.UserChannel do
@moduledoc """
User channel, each user is subscribed to his channel,
to receive personal broadcasts
Users can't join other users channel
"""
require Logger
use TrentoWeb, :channel

@impl true
def join(
"users:" <> user_id,
_payload,
%{assigns: %{current_user_id: current_user_id}} = socket
) do
user_id = String.to_integer(user_id)

if user_id == current_user_id do
{:ok, socket}
else
Logger.error(
"Could not join user channel, requested user id: #{user_id}, authenticated user id: #{current_user_id}"
)

{:error, :unauthorized}
end
end

def join("users:" <> _user_id, _payload, _socket) do
{:error, :user_not_logged}
end
end
22 changes: 19 additions & 3 deletions lib/trento_web/channels/user_socket.ex
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
defmodule TrentoWeb.UserSocket do
use Phoenix.Socket

require Logger
alias TrentoWeb.Auth.AccessToken

# A Socket handler
#
# It's possible to control the websocket connection and
Expand All @@ -9,6 +12,7 @@ defmodule TrentoWeb.UserSocket do
## Channels

channel "monitoring:*", TrentoWeb.MonitoringChannel
channel "users:*", TrentoWeb.UserChannel

# Socket params are passed from the client and can
# be used to verify and authenticate a user. After
Expand All @@ -22,8 +26,20 @@ defmodule TrentoWeb.UserSocket do
# See `Phoenix.Token` documentation for examples in
# performing token verification on connect.
@impl true
def connect(_params, socket, _connect_info) do
{:ok, socket}
def connect(%{"access_token" => access_token}, socket, _connect_info) do
case AccessToken.verify_and_validate(access_token) do
{:ok, %{"sub" => user_id}} ->
{:ok, assign(socket, :current_user_id, user_id)}

{:error, reason} ->
Logger.error("Could not authenticate user socket: #{inspect(reason)}")
{:error, reason}
end
end

def connect(_, _, _) do
Logger.error("Could not authenticate user socket: missing auth token")
{:error, :missing_auth_token}
end

# Socket id's are topics that allow you to identify all sockets for a given user:
Expand All @@ -37,5 +53,5 @@ defmodule TrentoWeb.UserSocket do
#
# Returning `nil` makes this socket anonymous.
@impl true
def id(_socket), do: nil
def id(socket), do: "user_socket:#{socket.assigns.current_user_id}"
end
7 changes: 7 additions & 0 deletions lib/trento_web/controllers/fallback_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,13 @@ defmodule TrentoWeb.FallbackController do
|> render(:"422", reason: "Connection with software updates provider failed.")
end

def call(conn, {:error, :operation_not_permitted}) do
conn
|> put_status(:forbidden)
|> put_view(ErrorView)
|> render(:"403")
end

def call(conn, {:error, [error | _]}), do: call(conn, {:error, error})

def call(conn, {:error, _}) do
Expand Down
6 changes: 5 additions & 1 deletion lib/trento_web/controllers/session_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,15 @@ defmodule TrentoWeb.SessionController do
title: "TrentoUser",
type: :object,
example: %{
username: "admin"
username: "admin",
id: 1
},
properties: %{
username: %Schema{
type: :string
},
id: %Schema{
type: :integer
}
}
}}
Expand Down
Loading

0 comments on commit f4ed280

Please sign in to comment.