From 771c9d4ba87a388dc306c58139d11bf510680c98 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Thu, 8 Jul 2021 05:31:28 +0200 Subject: [PATCH] Add ability to skip sign-in token authentication for specific users (#16427) Remove "active within last two weeks" exception for sign in token requirement Change admin reset password to lock access until the password is reset --- app/controllers/admin/resets_controller.rb | 4 +- ...ign_in_token_authentications_controller.rb | 27 ++++++++++++ .../two_factor_authentications_controller.rb | 2 +- app/models/user.rb | 25 ++++++++++- app/policies/user_policy.rb | 8 ++++ app/views/admin/accounts/show.html.haml | 24 +++++++++-- config/locales/en.yml | 42 ++++++++++++------- config/routes.rb | 1 + ...1221010_add_skip_sign_in_token_to_users.rb | 5 +++ db/schema.rb | 1 + lib/mastodon/accounts_cli.rb | 15 +++++-- .../admin/resets_controller_spec.rb | 2 +- ..._factor_authentications_controller_spec.rb | 8 ++-- spec/models/user_spec.rb | 28 +++++++++++++ 14 files changed, 160 insertions(+), 32 deletions(-) create mode 100644 app/controllers/admin/sign_in_token_authentications_controller.rb create mode 100644 db/migrate/20210621221010_add_skip_sign_in_token_to_users.rb diff --git a/app/controllers/admin/resets_controller.rb b/app/controllers/admin/resets_controller.rb index db8f61d64cb..7962b7a58c6 100644 --- a/app/controllers/admin/resets_controller.rb +++ b/app/controllers/admin/resets_controller.rb @@ -6,9 +6,9 @@ module Admin def create authorize @user, :reset_password? - @user.send_reset_password_instructions + @user.reset_password! log_action :reset_password, @user - redirect_to admin_accounts_path + redirect_to admin_account_path(@user.account_id) end end end diff --git a/app/controllers/admin/sign_in_token_authentications_controller.rb b/app/controllers/admin/sign_in_token_authentications_controller.rb new file mode 100644 index 00000000000..e620ab2926d --- /dev/null +++ b/app/controllers/admin/sign_in_token_authentications_controller.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module Admin + class SignInTokenAuthenticationsController < BaseController + before_action :set_target_user + + def create + authorize @user, :enable_sign_in_token_auth? + @user.update(skip_sign_in_token: false) + log_action :enable_sign_in_token_auth, @user + redirect_to admin_account_path(@user.account_id) + end + + def destroy + authorize @user, :disable_sign_in_token_auth? + @user.update(skip_sign_in_token: true) + log_action :disable_sign_in_token_auth, @user + redirect_to admin_account_path(@user.account_id) + end + + private + + def set_target_user + @user = User.find(params[:user_id]) + end + end +end diff --git a/app/controllers/admin/two_factor_authentications_controller.rb b/app/controllers/admin/two_factor_authentications_controller.rb index 0652c3a7a4a..f7fb7eb8fe3 100644 --- a/app/controllers/admin/two_factor_authentications_controller.rb +++ b/app/controllers/admin/two_factor_authentications_controller.rb @@ -9,7 +9,7 @@ module Admin @user.disable_two_factor! log_action :disable_2fa, @user UserMailer.two_factor_disabled(@user).deliver_later! - redirect_to admin_accounts_path + redirect_to admin_account_path(@user.account_id) end private diff --git a/app/models/user.rb b/app/models/user.rb index 4973c68b61b..4059c96b569 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -42,6 +42,7 @@ # sign_in_token_sent_at :datetime # webauthn_id :string # sign_up_ip :inet +# skip_sign_in_token :boolean # class User < ApplicationRecord @@ -200,7 +201,7 @@ class User < ApplicationRecord end def suspicious_sign_in?(ip) - !otp_required_for_login? && current_sign_in_at.present? && current_sign_in_at < 2.weeks.ago && !recent_ip?(ip) + !otp_required_for_login? && !skip_sign_in_token? && current_sign_in_at.present? && !recent_ip?(ip) end def functional? @@ -329,12 +330,32 @@ class User < ApplicationRecord super end - def reset_password!(new_password, new_password_confirmation) + def reset_password(new_password, new_password_confirmation) return false if encrypted_password.blank? super end + def reset_password! + # First, change password to something random, invalidate the remember-me token, + # and deactivate all sessions + transaction do + update(remember_token: nil, remember_created_at: nil, password: SecureRandom.hex) + session_activations.destroy_all + end + + # Then, remove all authorized applications and connected push subscriptions + Doorkeeper::AccessGrant.by_resource_owner(self).in_batches.update_all(revoked_at: Time.now.utc) + + Doorkeeper::AccessToken.by_resource_owner(self).in_batches do |batch| + batch.update_all(revoked_at: Time.now.utc) + Web::PushSubscription.where(access_token_id: batch).delete_all + end + + # Finally, send a reset password prompt to the user + send_reset_password_instructions + end + def show_all_media? setting_display_media == 'show_all' end diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb index d832bff75d4..6695a0ddf89 100644 --- a/app/policies/user_policy.rb +++ b/app/policies/user_policy.rb @@ -13,6 +13,14 @@ class UserPolicy < ApplicationPolicy admin? && !record.staff? end + def disable_sign_in_token_auth? + staff? + end + + def enable_sign_in_token_auth? + staff? + end + def confirm? staff? && !record.confirmed? end diff --git a/app/views/admin/accounts/show.html.haml b/app/views/admin/accounts/show.html.haml index 27e1f80a722..66eb4934275 100644 --- a/app/views/admin/accounts/show.html.haml +++ b/app/views/admin/accounts/show.html.haml @@ -129,6 +129,27 @@ - else = t('admin.accounts.confirming') %td= table_link_to 'refresh', t('admin.accounts.resend_confirmation.send'), resend_admin_account_confirmation_path(@account.id), method: :post if can?(:confirm, @account.user) + %tr + %th{ rowspan: can?(:reset_password, @account.user) ? 2 : 1 }= t('admin.accounts.security') + %td{ rowspan: can?(:reset_password, @account.user) ? 2 : 1 } + - if @account.user&.two_factor_enabled? + = t 'admin.accounts.security_measures.password_and_2fa' + - elsif @account.user&.skip_sign_in_token? + = t 'admin.accounts.security_measures.only_password' + - else + = t 'admin.accounts.security_measures.password_and_sign_in_token' + %td + - if @account.user&.two_factor_enabled? + = table_link_to 'unlock', t('admin.accounts.disable_two_factor_authentication'), admin_user_two_factor_authentication_path(@account.user.id), method: :delete if can?(:disable_2fa, @account.user) + - elsif @account.user&.skip_sign_in_token? + = table_link_to 'lock', t('admin.accounts.enable_sign_in_token_auth'), admin_user_sign_in_token_authentication_path(@account.user.id), method: :post if can?(:enable_sign_in_token_auth, @account.user) + - else + = table_link_to 'unlock', t('admin.accounts.disable_sign_in_token_auth'), admin_user_sign_in_token_authentication_path(@account.user.id), method: :delete if can?(:disable_sign_in_token_auth, @account.user) + + - if can?(:reset_password, @account.user) + %tr + %td + = table_link_to 'key', t('admin.accounts.reset_password'), admin_account_reset_path(@account.id), method: :create, data: { confirm: t('admin.accounts.are_you_sure') } %tr %th= t('simple_form.labels.defaults.locale') @@ -221,9 +242,6 @@ %div - if @account.local? - = link_to t('admin.accounts.reset_password'), admin_account_reset_path(@account.id), method: :create, class: 'button' if can?(:reset_password, @account.user) - - if @account.user&.otp_required_for_login? - = link_to t('admin.accounts.disable_two_factor_authentication'), admin_user_two_factor_authentication_path(@account.user.id), method: :delete, class: 'button' if can?(:disable_2fa, @account.user) - if !@account.memorial? && @account.user_approved? = link_to t('admin.accounts.memorialize'), memorialize_admin_account_path(@account.id), method: :post, data: { confirm: t('admin.accounts.are_you_sure') }, class: 'button button--destructive' if can?(:memorialize, @account) - else diff --git a/config/locales/en.yml b/config/locales/en.yml index cdb2e3df7f3..51764a0e169 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -44,7 +44,7 @@ en: rejecting_media: 'Media files from these servers will not be processed or stored, and no thumbnails will be displayed, requiring manual click-through to the original file:' rejecting_media_title: Filtered media silenced: 'Posts from these servers will be hidden in public timelines and conversations, and no notifications will be generated from their users interactions, unless you are following them:' - silenced_title: Silenced servers + silenced_title: Limited servers suspended: 'No data from these servers will be processed, stored or exchanged, making any interaction or communication with users from these servers impossible:' suspended_title: Suspended servers unavailable_content_html: Mastodon generally allows you to view content from and interact with users from any other server in the fediverse. These are the exceptions that have been made on this particular server. @@ -119,6 +119,7 @@ en: demote: Demote destroyed_msg: "%{username}'s data is now queued to be deleted imminently" disable: Freeze + disable_sign_in_token_auth: Disable e-mail token authentication disable_two_factor_authentication: Disable 2FA disabled: Frozen display_name: Display name @@ -127,6 +128,7 @@ en: email: Email email_status: Email status enable: Unfreeze + enable_sign_in_token_auth: Enable e-mail token authentication enabled: Enabled enabled_msg: Successfully unfroze %{username}'s account followers: Followers @@ -151,7 +153,7 @@ en: active: Active all: All pending: Pending - silenced: Silenced + silenced: Limited suspended: Suspended title: Moderation moderation_notes: Moderation notes @@ -191,8 +193,12 @@ en: search: Search search_same_email_domain: Other users with the same e-mail domain search_same_ip: Other users with the same IP - sensitive: Sensitive - sensitized: marked as sensitive + security_measures: + only_password: Only password + password_and_2fa: Password and 2FA + password_and_sign_in_token: Password and e-mail token + sensitive: Force-sensitive + sensitized: Marked as sensitive shared_inbox_url: Shared inbox URL show: created_reports: Made reports @@ -207,10 +213,10 @@ en: time_in_queue: Waiting in queue %{time} title: Accounts unconfirmed_email: Unconfirmed email - undo_sensitized: Undo sensitive - undo_silenced: Undo silence + undo_sensitized: Undo force-sensitive + undo_silenced: Undo limit undo_suspension: Undo suspension - unsilenced_msg: Successfully unlimited %{username}'s account + unsilenced_msg: Successfully undid limit of %{username}'s account unsubscribe: Unsubscribe unsuspended_msg: Successfully unsuspended %{username}'s account username: Username @@ -236,14 +242,16 @@ en: destroy_custom_emoji: Delete Custom Emoji destroy_domain_allow: Delete Domain Allow destroy_domain_block: Delete Domain Block - destroy_email_domain_block: Delete e-mail domain block + destroy_email_domain_block: Delete E-mail Domain Block destroy_ip_block: Delete IP rule destroy_status: Delete Post destroy_unavailable_domain: Delete Unavailable Domain disable_2fa_user: Disable 2FA disable_custom_emoji: Disable Custom Emoji + disable_sign_in_token_auth_user: Disable E-mail Token Authentication for User disable_user: Disable User enable_custom_emoji: Enable Custom Emoji + enable_sign_in_token_auth_user: Enable E-mail Token Authentication for User enable_user: Enable User memorialize_account: Memorialize Account promote_user: Promote User @@ -251,12 +259,12 @@ en: reopen_report: Reopen Report reset_password_user: Reset Password resolve_report: Resolve Report - sensitive_account: Mark the media in your account as sensitive - silence_account: Silence Account + sensitive_account: Force-Sensitive Account + silence_account: Limit Account suspend_account: Suspend Account unassigned_report: Unassign Report - unsensitive_account: Unmark the media in your account as sensitive - unsilence_account: Unsilence Account + unsensitive_account: Undo Force-Sensitive Account + unsilence_account: Undo Limit Account unsuspend_account: Unsuspend Account update_announcement: Update Announcement update_custom_emoji: Update Custom Emoji @@ -285,8 +293,10 @@ en: destroy_unavailable_domain_html: "%{name} resumed delivery to domain %{target}" disable_2fa_user_html: "%{name} disabled two factor requirement for user %{target}" disable_custom_emoji_html: "%{name} disabled emoji %{target}" + disable_sign_in_token_auth_user_html: "%{name} disabled e-mail token authentication for %{target}" disable_user_html: "%{name} disabled login for user %{target}" enable_custom_emoji_html: "%{name} enabled emoji %{target}" + enable_sign_in_token_auth_user_html: "%{name} enabled e-mail token authentication for %{target}" enable_user_html: "%{name} enabled login for user %{target}" memorialize_account_html: "%{name} turned %{target}'s account into a memoriam page" promote_user_html: "%{name} promoted user %{target}" @@ -295,11 +305,11 @@ en: reset_password_user_html: "%{name} reset password of user %{target}" resolve_report_html: "%{name} resolved report %{target}" sensitive_account_html: "%{name} marked %{target}'s media as sensitive" - silence_account_html: "%{name} silenced %{target}'s account" + silence_account_html: "%{name} limited %{target}'s account" suspend_account_html: "%{name} suspended %{target}'s account" unassigned_report_html: "%{name} unassigned report %{target}" unsensitive_account_html: "%{name} unmarked %{target}'s media as sensitive" - unsilence_account_html: "%{name} unsilenced %{target}'s account" + unsilence_account_html: "%{name} undid limit of %{target}'s account" unsuspend_account_html: "%{name} unsuspended %{target}'s account" update_announcement_html: "%{name} updated announcement %{target}" update_custom_emoji_html: "%{name} updated emoji %{target}" @@ -421,14 +431,14 @@ en: rejecting_media: rejecting media files rejecting_reports: rejecting reports severity: - silence: silenced + silence: limited suspend: suspended show: affected_accounts: one: One account in the database affected other: "%{count} accounts in the database affected" retroactive: - silence: Unsilence existing affected accounts from this domain + silence: Undo limit of existing affected accounts from this domain suspend: Unsuspend existing affected accounts from this domain title: Undo domain block for %{domain} undo: Undo diff --git a/config/routes.rb b/config/routes.rb index eb618324a97..0c4b2954649 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -283,6 +283,7 @@ Rails.application.routes.draw do resources :users, only: [] do resource :two_factor_authentication, only: [:destroy] + resource :sign_in_token_authentication, only: [:create, :destroy] end resources :custom_emojis, only: [:index, :new, :create] do diff --git a/db/migrate/20210621221010_add_skip_sign_in_token_to_users.rb b/db/migrate/20210621221010_add_skip_sign_in_token_to_users.rb new file mode 100644 index 00000000000..43ad9b95455 --- /dev/null +++ b/db/migrate/20210621221010_add_skip_sign_in_token_to_users.rb @@ -0,0 +1,5 @@ +class AddSkipSignInTokenToUsers < ActiveRecord::Migration[6.1] + def change + add_column :users, :skip_sign_in_token, :boolean + end +end diff --git a/db/schema.rb b/db/schema.rb index 935e2a56429..b2929f693c9 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -927,6 +927,7 @@ ActiveRecord::Schema.define(version: 2021_06_30_000137) do t.datetime "sign_in_token_sent_at" t.string "webauthn_id" t.inet "sign_up_ip" + t.boolean "skip_sign_in_token" t.index ["account_id"], name: "index_users_on_account_id" t.index ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true t.index ["created_by_application_id"], name: "index_users_on_created_by_application_id" diff --git a/lib/mastodon/accounts_cli.rb b/lib/mastodon/accounts_cli.rb index 74162256ff7..05019480162 100644 --- a/lib/mastodon/accounts_cli.rb +++ b/lib/mastodon/accounts_cli.rb @@ -54,7 +54,8 @@ module Mastodon option :email, required: true option :confirmed, type: :boolean - option :role, default: 'user' + option :role, default: 'user', enum: %w(user moderator admin) + option :skip_sign_in_token, type: :boolean option :reattach, type: :boolean option :force, type: :boolean desc 'create USERNAME', 'Create a new user' @@ -68,6 +69,9 @@ module Mastodon With the --role option one of "user", "admin" or "moderator" can be supplied. Defaults to "user" + With the --skip-sign-in-token option, you can ensure that + the user is never asked for an e-mailed security code. + With the --reattach option, the new user will be reattached to a given existing username of an old account. If the old account is still in use by someone else, you can supply @@ -77,7 +81,7 @@ module Mastodon def create(username) account = Account.new(username: username) password = SecureRandom.hex - user = User.new(email: options[:email], password: password, agreement: true, approved: true, admin: options[:role] == 'admin', moderator: options[:role] == 'moderator', confirmed_at: options[:confirmed] ? Time.now.utc : nil, bypass_invite_request_check: true) + user = User.new(email: options[:email], password: password, agreement: true, approved: true, admin: options[:role] == 'admin', moderator: options[:role] == 'moderator', confirmed_at: options[:confirmed] ? Time.now.utc : nil, bypass_invite_request_check: true, skip_sign_in_token: options[:skip_sign_in_token]) if options[:reattach] account = Account.find_local(username) || Account.new(username: username) @@ -113,7 +117,7 @@ module Mastodon end end - option :role + option :role, enum: %w(user moderator admin) option :email option :confirm, type: :boolean option :enable, type: :boolean @@ -121,6 +125,7 @@ module Mastodon option :disable_2fa, type: :boolean option :approve, type: :boolean option :reset_password, type: :boolean + option :skip_sign_in_token, type: :boolean desc 'modify USERNAME', 'Modify a user' long_desc <<-LONG_DESC Modify a user account. @@ -142,6 +147,9 @@ module Mastodon With the --reset-password option, the user's password is replaced by a randomly-generated one, printed in the output. + + With the --skip-sign-in-token option, you can ensure that + the user is never asked for an e-mailed security code. LONG_DESC def modify(username) user = Account.find_local(username)&.user @@ -163,6 +171,7 @@ module Mastodon user.disabled = true if options[:disable] user.approved = true if options[:approve] user.otp_required_for_login = false if options[:disable_2fa] + user.skip_sign_in_token = options[:skip_sign_in_token] unless options[:skip_sign_in_token].nil? user.confirm if options[:confirm] if user.save diff --git a/spec/controllers/admin/resets_controller_spec.rb b/spec/controllers/admin/resets_controller_spec.rb index a20a460bd05..c1e34b7f968 100644 --- a/spec/controllers/admin/resets_controller_spec.rb +++ b/spec/controllers/admin/resets_controller_spec.rb @@ -16,7 +16,7 @@ describe Admin::ResetsController do post :create, params: { account_id: account.id } - expect(response).to redirect_to(admin_accounts_path) + expect(response).to redirect_to(admin_account_path(account.id)) end end end diff --git a/spec/controllers/admin/two_factor_authentications_controller_spec.rb b/spec/controllers/admin/two_factor_authentications_controller_spec.rb index b0e82d3d663..c650957290f 100644 --- a/spec/controllers/admin/two_factor_authentications_controller_spec.rb +++ b/spec/controllers/admin/two_factor_authentications_controller_spec.rb @@ -15,12 +15,12 @@ describe Admin::TwoFactorAuthenticationsController do user.update(otp_required_for_login: true) end - it 'redirects to admin accounts page' do + it 'redirects to admin account page' do delete :destroy, params: { user_id: user.id } user.reload expect(user.otp_enabled?).to eq false - expect(response).to redirect_to(admin_accounts_path) + expect(response).to redirect_to(admin_account_path(user.account_id)) end end @@ -38,13 +38,13 @@ describe Admin::TwoFactorAuthenticationsController do nickname: 'Security Key') end - it 'redirects to admin accounts page' do + it 'redirects to admin account page' do delete :destroy, params: { user_id: user.id } user.reload expect(user.otp_enabled?).to eq false expect(user.webauthn_enabled?).to eq false - expect(response).to redirect_to(admin_accounts_path) + expect(response).to redirect_to(admin_account_path(user.account_id)) end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 5db249be2a9..54bb6db7f96 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -344,6 +344,34 @@ RSpec.describe User, type: :model do end end + describe '#reset_password!' do + subject(:user) { Fabricate(:user, password: 'foobar12345') } + + let!(:session_activation) { Fabricate(:session_activation, user: user) } + let!(:access_token) { Fabricate(:access_token, resource_owner_id: user.id) } + let!(:web_push_subscription) { Fabricate(:web_push_subscription, access_token: access_token) } + + before do + user.reset_password! + end + + it 'changes the password immediately' do + expect(user.external_or_valid_password?('foobar12345')).to be false + end + + it 'deactivates all sessions' do + expect(user.session_activations.count).to eq 0 + end + + it 'revokes all access tokens' do + expect(Doorkeeper::AccessToken.active_for(user).count).to eq 0 + end + + it 'removes push subscriptions' do + expect(Web::PushSubscription.where(user: user).or(Web::PushSubscription.where(access_token: access_token)).count).to eq 0 + end + end + describe '#confirm!' do subject(:user) { Fabricate(:user, confirmed_at: confirmed_at) }