From d174d12c831989bf1d5d3ca54d4f26d28c2c8925 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Mon, 21 Jun 2021 17:07:30 +0200 Subject: [PATCH] Add authentication history (#16408) --- .../auth/omniauth_callbacks_controller.rb | 9 ++++ app/controllers/auth/sessions_controller.rb | 44 ++++++++++++++++--- .../sign_in_token_authentication_concern.rb | 5 +-- .../two_factor_authentication_concern.rb | 10 ++--- .../settings/login_activities_controller.rb | 7 +++ app/javascript/styles/mastodon/forms.scss | 18 ++++++++ app/models/concerns/ldap_authenticable.rb | 2 +- app/models/login_activity.rb | 35 +++++++++++++++ .../auth/registrations/_sessions.html.haml | 5 ++- .../_login_activity.html.haml | 17 +++++++ .../settings/login_activities/index.html.haml | 15 +++++++ app/workers/scheduler/ip_cleanup_scheduler.rb | 1 + config/locales/en.yml | 12 +++++ config/navigation.rb | 2 +- config/routes.rb | 3 +- .../20210609202149_create_login_activities.rb | 14 ++++++ db/schema.rb | 15 ++++++- spec/fabricators/login_activity_fabricator.rb | 8 ++++ spec/models/login_activity_spec.rb | 5 +++ 19 files changed, 206 insertions(+), 21 deletions(-) create mode 100644 app/controllers/settings/login_activities_controller.rb create mode 100644 app/models/login_activity.rb create mode 100644 app/views/settings/login_activities/_login_activity.html.haml create mode 100644 app/views/settings/login_activities/index.html.haml create mode 100644 db/migrate/20210609202149_create_login_activities.rb create mode 100644 spec/fabricators/login_activity_fabricator.rb create mode 100644 spec/models/login_activity_spec.rb diff --git a/app/controllers/auth/omniauth_callbacks_controller.rb b/app/controllers/auth/omniauth_callbacks_controller.rb index 682c77016..7925e23cb 100644 --- a/app/controllers/auth/omniauth_callbacks_controller.rb +++ b/app/controllers/auth/omniauth_callbacks_controller.rb @@ -10,6 +10,15 @@ class Auth::OmniauthCallbacksController < Devise::OmniauthCallbacksController @user = User.find_for_oauth(request.env['omniauth.auth'], current_user) if @user.persisted? + LoginActivity.create( + user: user, + success: true, + authentication_method: :omniauth, + provider: provider, + ip: request.remote_ip, + user_agent: request.user_agent + ) + sign_in_and_redirect @user, event: :authentication set_flash_message(:notice, :success, kind: provider_id.capitalize) if is_navigational_format? else diff --git a/app/controllers/auth/sessions_controller.rb b/app/controllers/auth/sessions_controller.rb index 13d158c67..9c73b39e2 100644 --- a/app/controllers/auth/sessions_controller.rb +++ b/app/controllers/auth/sessions_controller.rb @@ -25,9 +25,11 @@ class Auth::SessionsController < Devise::SessionsController def create super do |resource| - resource.update_sign_in!(request, new_sign_in: true) - remember_me(resource) - flash.delete(:notice) + # We only need to call this if this hasn't already been + # called from one of the two-factor or sign-in token + # authentication methods + + on_authentication_success(resource, :password) unless @on_authentication_success_called end end @@ -42,10 +44,8 @@ class Auth::SessionsController < Devise::SessionsController def webauthn_options user = find_user - if user.webauthn_enabled? - options_for_get = WebAuthn::Credential.options_for_get( - allow: user.webauthn_credentials.pluck(:external_id) - ) + if user&.webauthn_enabled? + options_for_get = WebAuthn::Credential.options_for_get(allow: user.webauthn_credentials.pluck(:external_id)) session[:webauthn_challenge] = options_for_get.challenge @@ -136,4 +136,34 @@ class Auth::SessionsController < Devise::SessionsController session.delete(:attempt_user_id) session.delete(:attempt_user_updated_at) end + + def on_authentication_success(user, security_measure) + @on_authentication_success_called = true + + clear_attempt_from_session + + user.update_sign_in!(request, new_sign_in: true) + remember_me(user) + sign_in(user) + flash.delete(:notice) + + LoginActivity.create( + user: user, + success: true, + authentication_method: security_measure, + ip: request.remote_ip, + user_agent: request.user_agent + ) + end + + def on_authentication_failure(user, security_measure, failure_reason) + LoginActivity.create( + user: user, + success: false, + authentication_method: security_measure, + failure_reason: failure_reason, + ip: request.remote_ip, + user_agent: request.user_agent + ) + end end diff --git a/app/controllers/concerns/sign_in_token_authentication_concern.rb b/app/controllers/concerns/sign_in_token_authentication_concern.rb index 3c95a4afd..cbee84569 100644 --- a/app/controllers/concerns/sign_in_token_authentication_concern.rb +++ b/app/controllers/concerns/sign_in_token_authentication_concern.rb @@ -29,10 +29,9 @@ module SignInTokenAuthenticationConcern def authenticate_with_sign_in_token_attempt(user) if valid_sign_in_token_attempt?(user) - clear_attempt_from_session - remember_me(user) - sign_in(user) + on_authentication_success(user, :sign_in_token) else + on_authentication_failure(user, :sign_in_token, :invalid_sign_in_token) flash.now[:alert] = I18n.t('users.invalid_sign_in_token') prompt_for_sign_in_token(user) end diff --git a/app/controllers/concerns/two_factor_authentication_concern.rb b/app/controllers/concerns/two_factor_authentication_concern.rb index 4d4ccf49c..909ab7717 100644 --- a/app/controllers/concerns/two_factor_authentication_concern.rb +++ b/app/controllers/concerns/two_factor_authentication_concern.rb @@ -52,21 +52,19 @@ module TwoFactorAuthenticationConcern webauthn_credential = WebAuthn::Credential.from_get(user_params[:credential]) if valid_webauthn_credential?(user, webauthn_credential) - clear_attempt_from_session - remember_me(user) - sign_in(user) + on_authentication_success(user, :webauthn) render json: { redirect_path: root_path }, status: :ok else + on_authentication_failure(user, :webauthn, :invalid_credential) render json: { error: t('webauthn_credentials.invalid_credential') }, status: :unprocessable_entity end end def authenticate_with_two_factor_via_otp(user) if valid_otp_attempt?(user) - clear_attempt_from_session - remember_me(user) - sign_in(user) + on_authentication_success(user, :otp) else + on_authentication_failure(user, :otp, :invalid_otp_token) flash.now[:alert] = I18n.t('users.invalid_otp_token') prompt_for_two_factor(user) end diff --git a/app/controllers/settings/login_activities_controller.rb b/app/controllers/settings/login_activities_controller.rb new file mode 100644 index 000000000..57fa6aef0 --- /dev/null +++ b/app/controllers/settings/login_activities_controller.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class Settings::LoginActivitiesController < Settings::BaseController + def index + @login_activities = LoginActivity.where(user: current_user).order(id: :desc).page(params[:page]) + end +end diff --git a/app/javascript/styles/mastodon/forms.scss b/app/javascript/styles/mastodon/forms.scss index ef4a08c59..5b71b6334 100644 --- a/app/javascript/styles/mastodon/forms.scss +++ b/app/javascript/styles/mastodon/forms.scss @@ -11,6 +11,24 @@ code { margin: 0 auto; } +.indicator-icon { + display: flex; + align-items: center; + justify-content: center; + width: 40px; + height: 40px; + border-radius: 50%; + color: $primary-text-color; + + &.success { + background: $success-green; + } + + &.failure { + background: $error-red; + } +} + .simple_form { &.hidden { display: none; diff --git a/app/models/concerns/ldap_authenticable.rb b/app/models/concerns/ldap_authenticable.rb index e3f94bb6c..dc5abcd5a 100644 --- a/app/models/concerns/ldap_authenticable.rb +++ b/app/models/concerns/ldap_authenticable.rb @@ -15,10 +15,10 @@ module LdapAuthenticable def ldap_get_user(attributes = {}) safe_username = attributes[Devise.ldap_uid.to_sym].first + if Devise.ldap_uid_conversion_enabled keys = Regexp.union(Devise.ldap_uid_conversion_search.chars) replacement = Devise.ldap_uid_conversion_replace - safe_username = safe_username.gsub(keys, replacement) end diff --git a/app/models/login_activity.rb b/app/models/login_activity.rb new file mode 100644 index 000000000..52a0fd01d --- /dev/null +++ b/app/models/login_activity.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true +# == Schema Information +# +# Table name: login_activities +# +# id :bigint(8) not null, primary key +# user_id :bigint(8) not null +# authentication_method :string +# provider :string +# success :boolean +# failure_reason :string +# ip :inet +# user_agent :string +# created_at :datetime +# + +class LoginActivity < ApplicationRecord + enum authentication_method: { password: 'password', otp: 'otp', webauthn: 'webauthn', sign_in_token: 'sign_in_token', omniauth: 'omniauth' } + + belongs_to :user + + validates :authentication_method, inclusion: { in: authentication_methods.keys } + + def detection + @detection ||= Browser.new(user_agent) + end + + def browser + detection.id + end + + def platform + detection.platform.id + end +end diff --git a/app/views/auth/registrations/_sessions.html.haml b/app/views/auth/registrations/_sessions.html.haml index d3a04c00e..5d993f574 100644 --- a/app/views/auth/registrations/_sessions.html.haml +++ b/app/views/auth/registrations/_sessions.html.haml @@ -1,5 +1,7 @@ %h3= t 'sessions.title' -%p.muted-hint= t 'sessions.explanation' +%p.muted-hint + = t 'sessions.explanation' + = link_to t('sessions.view_authentication_history'), settings_login_activities_path %hr.spacer/ @@ -29,3 +31,4 @@ %td - if current_session.session_id != session.session_id && !current_account.suspended? = table_link_to 'times', t('sessions.revoke'), settings_session_path(session), method: :delete + diff --git a/app/views/settings/login_activities/_login_activity.html.haml b/app/views/settings/login_activities/_login_activity.html.haml new file mode 100644 index 000000000..19a3cc3dd --- /dev/null +++ b/app/views/settings/login_activities/_login_activity.html.haml @@ -0,0 +1,17 @@ +- method_str = content_tag(:span, login_activity.omniauth? ? t(login_activity.provider, scope: 'auth.providers') : t(login_activity.authentication_method, scope: 'login_activities.authentication_methods'), class: 'target') +- ip_str = content_tag(:span, login_activity.ip, class: 'target') +- browser_str = content_tag(:span, t('sessions.description', browser: t("sessions.browsers.#{login_activity.browser}", default: "#{login_activity.browser}"), platform: t("sessions.platforms.#{login_activity.platform}", default: "#{login_activity.platform}")), class: 'target') + +.log-entry + .log-entry__header + .log-entry__avatar + .indicator-icon{ class: login_activity.success? ? 'success' : 'failure' } + = fa_icon login_activity.success? ? 'check' : 'times' + .log-entry__content + .log-entry__title + - if login_activity.success? + = t('login_activities.successful_sign_in_html', method: method_str, ip: ip_str, browser: browser_str) + - else + = t('login_activities.failed_sign_in_html', method: method_str, ip: ip_str, browser: browser_str) + .log-entry__timestamp + %time.formatted{ datetime: login_activity.created_at.iso8601 } diff --git a/app/views/settings/login_activities/index.html.haml b/app/views/settings/login_activities/index.html.haml new file mode 100644 index 000000000..ce524fbef --- /dev/null +++ b/app/views/settings/login_activities/index.html.haml @@ -0,0 +1,15 @@ +- content_for :page_title do + = t 'login_activities.title' + +%p= t('login_activities.description_html') + +%hr.spacer/ + +- if @login_activities.empty? + %div.muted-hint.center-text + = t 'login_activities.empty' +- else + .announcements-list + = render partial: 'login_activity', collection: @login_activities + += paginate @login_activities diff --git a/app/workers/scheduler/ip_cleanup_scheduler.rb b/app/workers/scheduler/ip_cleanup_scheduler.rb index df7e6ad56..918c10ac9 100644 --- a/app/workers/scheduler/ip_cleanup_scheduler.rb +++ b/app/workers/scheduler/ip_cleanup_scheduler.rb @@ -17,6 +17,7 @@ class Scheduler::IpCleanupScheduler def clean_ip_columns! SessionActivation.where('updated_at < ?', IP_RETENTION_PERIOD.ago).in_batches.destroy_all User.where('current_sign_in_at < ?', IP_RETENTION_PERIOD.ago).in_batches.update_all(last_sign_in_ip: nil, current_sign_in_ip: nil, sign_up_ip: nil) + LoginActivity.where('created_at < ?', IP_RETENTION_PERIOD.ago).in_batches.destroy_all end def clean_expired_ip_blocks! diff --git a/config/locales/en.yml b/config/locales/en.yml index 6274031dc..cdb2e3df7 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1004,6 +1004,17 @@ en: lists: errors: limit: You have reached the maximum amount of lists + login_activities: + authentication_methods: + otp: two-factor authentication app + password: password + sign_in_token: e-mail security code + webauthn: security keys + description_html: If you see activity that you don't recognize, consider changing your password and enabling two-factor authentication. + empty: No authentication history available + failed_sign_in_html: Failed sign-in attempt with %{method} from %{ip} (%{browser}) + successful_sign_in_html: Successful sign-in with %{method} from %{ip} (%{browser}) + title: Authentication history media_attachments: validations: images_and_video: Cannot attach a video to a post that already contains images @@ -1211,6 +1222,7 @@ en: revoke: Revoke revoke_success: Session successfully revoked title: Sessions + view_authentication_history: View authentication history of your account settings: account: Account account_settings: Account settings diff --git a/config/navigation.rb b/config/navigation.rb index b3462c48d..5d1f55d74 100644 --- a/config/navigation.rb +++ b/config/navigation.rb @@ -20,7 +20,7 @@ SimpleNavigation::Configuration.run do |navigation| n.item :filters, safe_join([fa_icon('filter fw'), t('filters.index.title')]), filters_path, highlights_on: %r{/filters}, if: -> { current_user.functional? } n.item :security, safe_join([fa_icon('lock fw'), t('settings.account')]), edit_user_registration_url do |s| - s.item :password, safe_join([fa_icon('lock fw'), t('settings.account_settings')]), edit_user_registration_url, highlights_on: %r{/auth/edit|/settings/delete|/settings/migration|/settings/aliases} + s.item :password, safe_join([fa_icon('lock fw'), t('settings.account_settings')]), edit_user_registration_url, highlights_on: %r{/auth/edit|/settings/delete|/settings/migration|/settings/aliases|/settings/login_activities} s.item :two_factor_authentication, safe_join([fa_icon('mobile fw'), t('settings.two_factor_authentication')]), settings_two_factor_authentication_methods_url, highlights_on: %r{/settings/two_factor_authentication|/settings/otp_authentication|/settings/security_keys} s.item :authorized_apps, safe_join([fa_icon('list fw'), t('settings.authorized_apps')]), oauth_authorized_applications_url end diff --git a/config/routes.rb b/config/routes.rb index 2373d8a51..eb618324a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -164,6 +164,7 @@ Rails.application.routes.draw do resources :aliases, only: [:index, :create, :destroy] resources :sessions, only: [:destroy] resources :featured_tags, only: [:index, :create, :destroy] + resources :login_activities, only: [:index] end resources :media, only: [:show] do @@ -222,7 +223,7 @@ Rails.application.routes.draw do post :stop_delivery end end - + resources :rules resources :reports, only: [:index, :show] do diff --git a/db/migrate/20210609202149_create_login_activities.rb b/db/migrate/20210609202149_create_login_activities.rb new file mode 100644 index 000000000..38e147c32 --- /dev/null +++ b/db/migrate/20210609202149_create_login_activities.rb @@ -0,0 +1,14 @@ +class CreateLoginActivities < ActiveRecord::Migration[6.1] + def change + create_table :login_activities do |t| + t.belongs_to :user, null: false, foreign_key: { on_delete: :cascade } + t.string :authentication_method + t.string :provider + t.boolean :success + t.string :failure_reason + t.inet :ip + t.string :user_agent + t.datetime :created_at + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 3fcee2d30..31bf565b4 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.define(version: 2021_05_26_193025) do +ActiveRecord::Schema.define(version: 2021_06_09_202149) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -494,6 +494,18 @@ ActiveRecord::Schema.define(version: 2021_05_26_193025) do t.index ["account_id"], name: "index_lists_on_account_id" end + create_table "login_activities", force: :cascade do |t| + t.bigint "user_id", null: false + t.string "authentication_method" + t.string "provider" + t.boolean "success" + t.string "failure_reason" + t.inet "ip" + t.string "user_agent" + t.datetime "created_at" + t.index ["user_id"], name: "index_login_activities_on_user_id" + end + create_table "markers", force: :cascade do |t| t.bigint "user_id" t.string "timeline", default: "", null: false @@ -1010,6 +1022,7 @@ ActiveRecord::Schema.define(version: 2021_05_26_193025) do add_foreign_key "list_accounts", "follows", on_delete: :cascade add_foreign_key "list_accounts", "lists", on_delete: :cascade add_foreign_key "lists", "accounts", on_delete: :cascade + add_foreign_key "login_activities", "users", on_delete: :cascade add_foreign_key "markers", "users", on_delete: :cascade add_foreign_key "media_attachments", "accounts", name: "fk_96dd81e81b", on_delete: :nullify add_foreign_key "media_attachments", "scheduled_statuses", on_delete: :nullify diff --git a/spec/fabricators/login_activity_fabricator.rb b/spec/fabricators/login_activity_fabricator.rb new file mode 100644 index 000000000..931d3082c --- /dev/null +++ b/spec/fabricators/login_activity_fabricator.rb @@ -0,0 +1,8 @@ +Fabricator(:login_activity) do + user + strategy 'password' + success true + failure_reason nil + ip { Faker::Internet.ip_v4_address } + user_agent { Faker::Internet.user_agent } +end diff --git a/spec/models/login_activity_spec.rb b/spec/models/login_activity_spec.rb new file mode 100644 index 000000000..ba2d207c9 --- /dev/null +++ b/spec/models/login_activity_spec.rb @@ -0,0 +1,5 @@ +require 'rails_helper' + +RSpec.describe LoginActivity, type: :model do + +end