diff --git a/app/controllers/concerns/authentication.rb b/app/controllers/concerns/authentication.rb index 9041f5c9..cd210cca 100644 --- a/app/controllers/concerns/authentication.rb +++ b/app/controllers/concerns/authentication.rb @@ -2,6 +2,7 @@ module Authentication extend ActiveSupport::Concern included do + before_action :set_request_details before_action :authenticate_user! end @@ -12,10 +13,9 @@ module Authentication end private - def authenticate_user! - if user = User.find_by(id: session[:user_id]) - Current.user = user + if session_record = Session.find_by_id(cookies.signed[:session_token]) + Current.session = session_record else if self_hosted_first_login? redirect_to new_registration_url @@ -25,23 +25,18 @@ module Authentication end end - def login(user) - Current.user = user - reset_session - session[:user_id] = user.id - set_last_login_at - end - - def logout - Current.user = nil - reset_session - end - - def set_last_login_at - Current.user.update(last_login_at: DateTime.now) + def create_session_for(user) + session = user.sessions.create! + cookies.signed.permanent[:session_token] = { value: session.id, httponly: true } + session end def self_hosted_first_login? Rails.application.config.app_mode.self_hosted? && User.count.zero? end + + def set_request_details + Current.user_agent = request.user_agent + Current.ip_address = request.ip + end end diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb index af628048..8c55e58d 100644 --- a/app/controllers/registrations_controller.rb +++ b/app/controllers/registrations_controller.rb @@ -17,7 +17,7 @@ class RegistrationsController < ApplicationController if @user.save Category.create_default_categories(@user.family) - login @user + @session = create_session_for(@user) flash[:notice] = t(".success") redirect_to root_path else diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 9f546b98..b6a23195 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -1,4 +1,5 @@ class SessionsController < ApplicationController + before_action :set_session, only: :destroy skip_authentication only: %i[new create] layout "auth" @@ -8,7 +9,7 @@ class SessionsController < ApplicationController def create if user = User.authenticate_by(email: params[:email], password: params[:password]) - login user + @session = create_session_for(user) redirect_to root_path else flash.now[:alert] = t(".invalid_credentials") @@ -17,7 +18,12 @@ class SessionsController < ApplicationController end def destroy - logout + @session.destroy redirect_to root_path, notice: t(".logout_successful") end + + private + def set_session + @session = Current.user.sessions.find(params[:id]) + end end diff --git a/app/controllers/settings/profiles_controller.rb b/app/controllers/settings/profiles_controller.rb index e8f135ba..c6b93c2c 100644 --- a/app/controllers/settings/profiles_controller.rb +++ b/app/controllers/settings/profiles_controller.rb @@ -23,7 +23,7 @@ class Settings::ProfilesController < SettingsController def destroy if Current.user.deactivate - logout + Current.session.destroy redirect_to root_path, notice: t(".success") else redirect_to settings_profile_path, alert: Current.user.errors.full_messages.to_sentence @@ -31,7 +31,6 @@ class Settings::ProfilesController < SettingsController end private - def user_params params.require(:user).permit(:first_name, :last_name, :profile_image, family_attributes: [ :name, :id ]) diff --git a/app/helpers/auth_messages_helper.rb b/app/helpers/auth_messages_helper.rb deleted file mode 100644 index 116ea654..00000000 --- a/app/helpers/auth_messages_helper.rb +++ /dev/null @@ -1,6 +0,0 @@ -module AuthMessagesHelper - def auth_messages(form = nil) - render "shared/auth_messages", flash: flash, - errors: form&.object&.errors&.full_messages || [] - end -end diff --git a/app/models/current.rb b/app/models/current.rb index 316a2cf5..d12fc0a9 100644 --- a/app/models/current.rb +++ b/app/models/current.rb @@ -1,5 +1,7 @@ class Current < ActiveSupport::CurrentAttributes - attribute :user + attribute :session + attribute :user_agent, :ip_address + delegate :user, to: :session, allow_nil: true delegate :family, to: :user, allow_nil: true end diff --git a/app/models/session.rb b/app/models/session.rb new file mode 100644 index 00000000..8e94aa81 --- /dev/null +++ b/app/models/session.rb @@ -0,0 +1,8 @@ +class Session < ApplicationRecord + belongs_to :user + + before_create do + self.user_agent = Current.user_agent + self.ip_address = Current.ip_address + end +end diff --git a/app/models/user.rb b/app/models/user.rb index 7a1cca57..3add1ba4 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -2,9 +2,11 @@ class User < ApplicationRecord has_secure_password belongs_to :family + has_many :sessions, dependent: :destroy accepts_nested_attributes_for :family validates :email, presence: true, uniqueness: true + validate :ensure_valid_profile_image normalizes :email, with: ->(email) { email.strip.downcase } normalizes :first_name, :last_name, with: ->(value) { value.strip.presence } @@ -72,6 +74,14 @@ class User < ApplicationRecord end private + def ensure_valid_profile_image + return unless profile_image.attached? + + unless profile_image.content_type.in?(%w[image/jpeg image/png]) + errors.add(:profile_image, "must be a JPEG or PNG") + profile_image.purge + end + end def last_user_in_family? family.users.count == 1 diff --git a/app/views/layouts/_sidebar.html.erb b/app/views/layouts/_sidebar.html.erb index 0cec2340..7cb00841 100644 --- a/app/views/layouts/_sidebar.html.erb +++ b/app/views/layouts/_sidebar.html.erb @@ -63,7 +63,7 @@ <% end %>
- <%= button_to session_path, method: :delete, class: "w-full text-red-400 flex gap-1 items-center hover:bg-gray-50 rounded-lg px-3 py-2" do %> + <%= button_to session_path(Current.session), method: :delete, class: "w-full text-red-400 flex gap-1 items-center hover:bg-gray-50 rounded-lg px-3 py-2" do %> <%= lucide_icon("log-out", class: "w-5 h-5 shrink-0") %> Logout <% end %> diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 6f4851c8..52afa5e4 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -25,14 +25,14 @@ -
-
- <%= render_flash_notifications %> +
+
+ <%= render_flash_notifications %> +
-
- <%= family_notifications_stream %> - <%= family_stream %> + <%= family_notifications_stream %> + <%= family_stream %> <%= content_for?(:content) ? yield(:content) : yield %> @@ -42,7 +42,7 @@ <%= render "shared/confirm_modal" %> <% if self_hosted? %> - <%= render "shared/app_version" %> + <%= render "shared/app_version" %> <% end %> diff --git a/app/views/layouts/auth.html.erb b/app/views/layouts/auth.html.erb index 05c81e68..4bed502e 100644 --- a/app/views/layouts/auth.html.erb +++ b/app/views/layouts/auth.html.erb @@ -23,7 +23,7 @@
-

<%= link_to t(".privacy_policy"), "/privacy", class: "font-medium text-gray-600 hover:text-gray-400 transition" %> • <%= link_to t(".terms_of_service"), "/terms", class: "font-medium text-gray-600 hover:text-gray-400 transition" %>

+

<%= link_to t(".privacy_policy"), "https://maybe.co/privacy", class: "font-medium text-gray-600 hover:text-gray-400 transition" %> • <%= link_to t(".terms_of_service"), "https://maybe.co/tos", class: "font-medium text-gray-600 hover:text-gray-400 transition" %>

<% end %> diff --git a/app/views/password_resets/edit.html.erb b/app/views/password_resets/edit.html.erb index 42c87ffe..dcf563a7 100644 --- a/app/views/password_resets/edit.html.erb +++ b/app/views/password_resets/edit.html.erb @@ -3,8 +3,6 @@ %> <%= styled_form_with model: @user, url: password_reset_path(token: params[:token]), method: :patch, class: "space-y-4" do |form| %> - <%= auth_messages form %> -
<%= form.label :password, class: "p-4 pb-0 block text-sm font-medium text-gray-700" %> <%= form.password_field :password, required: "required", class: "p-4 pt-1 bg-transparent border-none opacity-50 focus:outline-none focus:ring-0 focus-within:opacity-100 w-full" %> diff --git a/app/views/password_resets/new.html.erb b/app/views/password_resets/new.html.erb index f2b455a0..f869c3f8 100644 --- a/app/views/password_resets/new.html.erb +++ b/app/views/password_resets/new.html.erb @@ -3,8 +3,6 @@ %> <%= styled_form_with url: password_reset_path, class: "space-y-4" do |form| %> - <%= auth_messages form %> - <%= form.email_field :email, label: true, autofocus: false, autocomplete: "email", required: "required", placeholder: "you@example.com" %> <%= form.submit t(".submit") %> diff --git a/app/views/passwords/edit.html.erb b/app/views/passwords/edit.html.erb index a89cabde..0ed818b1 100644 --- a/app/views/passwords/edit.html.erb +++ b/app/views/passwords/edit.html.erb @@ -1,8 +1,6 @@

<% t(".title") %>

<%= styled_form_with model: Current.user, url: password_path, class: "space-y-4" do |form| %> - <%= auth_messages form %> -
<%= form.label :password_challenge, t(".password_challenge") %> <%= form.password_field :password_challenge %> diff --git a/app/views/registrations/new.html.erb b/app/views/registrations/new.html.erb index cbcad3d7..a5ec1103 100644 --- a/app/views/registrations/new.html.erb +++ b/app/views/registrations/new.html.erb @@ -10,7 +10,6 @@ <% end %> <%= styled_form_with model: @user, url: registration_path, class: "space-y-4" do |form| %> - <%= auth_messages form %> <%= form.email_field :email, autofocus: false, autocomplete: "email", required: "required", placeholder: "you@example.com", label: true %> <%= form.password_field :password, autocomplete: "new-password", required: "required", label: true %> <%= form.password_field :password_confirmation, autocomplete: "new-password", required: "required", label: true %> diff --git a/app/views/sessions/new.html.erb b/app/views/sessions/new.html.erb index 67a9d7ed..a3233d1b 100644 --- a/app/views/sessions/new.html.erb +++ b/app/views/sessions/new.html.erb @@ -2,9 +2,7 @@ header_title t(".title") %> -<%= styled_form_with url: session_path, class: "space-y-4" do |form| %> - <%= auth_messages form %> - +<%= styled_form_with url: sessions_path, class: "space-y-4" do |form| %> <%= form.email_field :email, label: t(".email"), autofocus: false, autocomplete: "email", required: "required", placeholder: t(".email_placeholder") %> <%= form.password_field :password, label: t(".password"), required: "required" %> diff --git a/app/views/settings/_nav.html.erb b/app/views/settings/_nav.html.erb index e3f16ba0..dad0759e 100644 --- a/app/views/settings/_nav.html.erb +++ b/app/views/settings/_nav.html.erb @@ -68,7 +68,7 @@
- <%= button_to session_path, method: :delete, class: "flex items-center gap-2 px-3 py-2 rounded-xl border text-sm font-medium w-full text-error hover:bg-gray-100 border-transparent" do %> + <%= button_to session_path(Current.session), method: :delete, class: "flex items-center gap-2 px-3 py-2 rounded-xl border text-sm font-medium w-full text-error hover:bg-gray-100 border-transparent" do %> <%= lucide_icon("log-out", class: "w-5 h-5 shrink-0") %> <%= t(".logout") %> <% end %> diff --git a/app/views/settings/profiles/show.html.erb b/app/views/settings/profiles/show.html.erb index 595f749b..c937fd73 100644 --- a/app/views/settings/profiles/show.html.erb +++ b/app/views/settings/profiles/show.html.erb @@ -26,7 +26,7 @@

<%= t(".profile_image_type") %>

<%= form.label :profile_image, t(".profile_image_choose"), class: "inline-block cursor-pointer px-3 py-2 bg-gray-50 text-gray-900 rounded-md text-sm font-medium" %> - <%= form.file_field :profile_image, accept: "image/png, image/jpeg, image/gif", class: "hidden px-3 py-2 bg-gray-50 text-gray-900 rounded-md text-sm font-medium", data: {profile_image_preview_target: "fileField", action: "change->profile-image-preview#preview"} %> + <%= form.file_field :profile_image, accept: "image/png, image/jpeg", class: "hidden px-3 py-2 bg-gray-50 text-gray-900 rounded-md text-sm font-medium", data: {profile_image_preview_target: "fileField", action: "change->profile-image-preview#preview"} %> <%= form.hidden_field :delete_profile_image, value: false, data: {profile_image_preview_target: "deleteField"} %>
diff --git a/config/routes.rb b/config/routes.rb index 9db1cc04..1bd35e42 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,18 +1,14 @@ Rails.application.routes.draw do mount GoodJob::Engine => "jobs" - get "changelog" => "pages#changelog", as: :changelog - get "feedback" => "pages#feedback", as: :feedback + get "changelog", to: "pages#changelog" + get "feedback", to: "pages#feedback" resource :registration - resource :session + resources :sessions, only: %i[new create destroy] resource :password_reset resource :password - namespace :help do - resources :articles, only: :show - end - namespace :settings do resource :profile, only: %i[show update destroy] resource :preferences, only: %i[show update] diff --git a/db/migrate/20241003163448_create_sessions.rb b/db/migrate/20241003163448_create_sessions.rb new file mode 100644 index 00000000..e49bb8a6 --- /dev/null +++ b/db/migrate/20241003163448_create_sessions.rb @@ -0,0 +1,13 @@ +class CreateSessions < ActiveRecord::Migration[7.2] + def change + create_table :sessions, id: :uuid do |t| + t.references :user, null: false, foreign_key: true, type: :uuid + t.string :user_agent + t.string :ip_address + + t.timestamps + end + + remove_column :users, :last_login_at, :datetime + end +end diff --git a/db/schema.rb b/db/schema.rb index c8966692..7e6ef394 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.2].define(version: 2024_10_01_181256) do +ActiveRecord::Schema[7.2].define(version: 2024_10_03_163448) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -450,6 +450,15 @@ ActiveRecord::Schema[7.2].define(version: 2024_10_01_181256) do t.datetime "updated_at", null: false end + create_table "sessions", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| + t.uuid "user_id", null: false + t.string "user_agent" + t.string "ip_address" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["user_id"], name: "index_sessions_on_user_id" + end + create_table "settings", force: :cascade do |t| t.string "var", null: false t.text "value" @@ -485,7 +494,6 @@ ActiveRecord::Schema[7.2].define(version: 2024_10_01_181256) do t.string "password_digest" t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.datetime "last_login_at" t.string "last_prompted_upgrade_commit_sha" t.string "last_alerted_upgrade_commit_sha" t.enum "role", default: "member", null: false, enum_type: "user_role" @@ -524,6 +532,7 @@ ActiveRecord::Schema[7.2].define(version: 2024_10_01_181256) do add_foreign_key "imports", "families" add_foreign_key "institutions", "families" add_foreign_key "merchants", "families" + add_foreign_key "sessions", "users" add_foreign_key "taggings", "tags" add_foreign_key "tags", "families" add_foreign_key "users", "families" diff --git a/test/controllers/registrations_controller_test.rb b/test/controllers/registrations_controller_test.rb index 87b4405a..1c3169f6 100644 --- a/test/controllers/registrations_controller_test.rb +++ b/test/controllers/registrations_controller_test.rb @@ -24,14 +24,6 @@ class RegistrationsControllerTest < ActionDispatch::IntegrationTest end end - test "sets last_login_at on successful registration" do - post registration_url, params: { user: { - email: "john@example.com", - password: "password", - password_confirmation: "password" } } - assert_not_nil User.find_by(email: "john@example.com").last_login_at - end - test "create when hosted requires an invite code" do with_env_overrides REQUIRE_INVITE_CODE: "true" do assert_no_difference "User.count" do diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb index 43915424..6360e849 100644 --- a/test/controllers/sessions_controller_test.rb +++ b/test/controllers/sessions_controller_test.rb @@ -5,14 +5,30 @@ class SessionsControllerTest < ActionDispatch::IntegrationTest @user = users(:family_admin) end - test "can sign in" do - post session_url, params: { email: @user.email, password: "password" } - assert_redirected_to root_url + test "login page" do + get new_session_url + assert_response :success end - test "sets last_login_at on successful login" do - assert_changes -> { @user.reload.last_login_at }, from: nil do - post session_url, params: { email: @user.email, password: "password" } - end + test "can sign in" do + sign_in @user + assert_redirected_to root_url + + get root_url + assert_response :success + end + + test "fails to sign in with bad password" do + post sessions_url, params: { email: @user.email, password: "bad" } + assert_response :unprocessable_entity + assert_equal "Invalid email or password.", flash[:alert] + end + + test "can sign out" do + sign_in @user + + delete session_url(@user.sessions.order(:created_at).last) + assert_redirected_to root_url + assert_equal "You have signed out successfully.", flash[:notice] end end diff --git a/test/fixtures/sessions.yml b/test/fixtures/sessions.yml new file mode 100644 index 00000000..84139332 --- /dev/null +++ b/test/fixtures/sessions.yml @@ -0,0 +1,4 @@ +one: + user: family_admin + user_agent: MyString + ip_address: MyString diff --git a/test/models/current_test.rb b/test/models/current_test.rb index 6d12ac6c..db46db5f 100644 --- a/test/models/current_test.rb +++ b/test/models/current_test.rb @@ -3,7 +3,7 @@ require "test_helper" class CurrentTest < ActiveSupport::TestCase test "family returns user family" do user = users(:family_admin) - Current.user = user + Current.session = user.sessions.create! assert_equal user.family, Current.family end end diff --git a/test/models/session_test.rb b/test/models/session_test.rb new file mode 100644 index 00000000..47ad9065 --- /dev/null +++ b/test/models/session_test.rb @@ -0,0 +1,7 @@ +require "test_helper" + +class SessionTest < ActiveSupport::TestCase + # test "the truth" do + # assert true + # end +end diff --git a/test/test_helper.rb b/test/test_helper.rb index c2be16af..f17655f7 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -49,7 +49,7 @@ module ActiveSupport # Add more helper methods to be used by all tests here... def sign_in(user) - post session_path, params: { email: user.email, password: "password" } + post sessions_path, params: { email: user.email, password: "password" } end def with_env_overrides(overrides = {}, &block)