diff --git a/app/assets/javascripts/admin/models/admin-user.js.es6 b/app/assets/javascripts/admin/models/admin-user.js.es6
index 4c952d953f8..b1e1202a906 100644
--- a/app/assets/javascripts/admin/models/admin-user.js.es6
+++ b/app/assets/javascripts/admin/models/admin-user.js.es6
@@ -115,11 +115,10 @@ const AdminUser = Discourse.User.extend({
},
revokeAdmin() {
- const self = this;
- return ajax("/admin/users/" + this.get('id') + "/revoke_admin", {
+ return ajax(`/admin/users/${this.get('id')}/revoke_admin`, {
type: 'PUT'
- }).then(function() {
- self.setProperties({
+ }).then(() => {
+ this.setProperties({
admin: false,
can_grant_admin: true,
can_revoke_admin: false
@@ -128,15 +127,10 @@ const AdminUser = Discourse.User.extend({
},
grantAdmin() {
- const self = this;
- return ajax("/admin/users/" + this.get('id') + "/grant_admin", {
+ return ajax(`/admin/users/${this.get('id')}/grant_admin`, {
type: 'PUT'
- }).then(function() {
- self.setProperties({
- admin: true,
- can_grant_admin: false,
- can_revoke_admin: true
- });
+ }).then(() => {
+ bootbox.alert(I18n.t("admin.user.grant_admin_confirm"));
}).catch(popupAjaxError);
},
diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb
index c0e97911169..3f007d48c51 100644
--- a/app/controllers/admin/users_controller.rb
+++ b/app/controllers/admin/users_controller.rb
@@ -1,5 +1,6 @@
require_dependency 'user_destroyer'
require_dependency 'admin_user_index_query'
+require_dependency 'admin_confirmation'
class Admin::UsersController < Admin::AdminController
@@ -103,10 +104,8 @@ class Admin::UsersController < Admin::AdminController
end
def grant_admin
- guardian.ensure_can_grant_admin!(@user)
- @user.grant_admin!
- StaffActionLogger.new(current_user).log_grant_admin(@user)
- render_serialized(@user, AdminUserSerializer)
+ AdminConfirmation.new(@user, current_user).create_confirmation
+ render json: success_json
end
def revoke_moderation
@@ -321,6 +320,7 @@ class Admin::UsersController < Admin::AdminController
end
def invite_admin
+ raise Discourse::InvalidAccess.new unless is_api?
email = params[:email]
unless user = User.find_by_email(email)
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 28f53334e13..7ca6f0d3801 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -3,11 +3,12 @@ require_dependency 'user_name_suggester'
require_dependency 'rate_limiter'
require_dependency 'wizard'
require_dependency 'wizard/builder'
+require_dependency 'admin_confirmation'
class UsersController < ApplicationController
skip_before_filter :authorize_mini_profiler, only: [:avatar]
- skip_before_filter :check_xhr, only: [:show, :password_reset, :update, :account_created, :activate_account, :perform_account_activation, :user_preferences_redirect, :avatar, :my_redirect, :toggle_anon, :admin_login]
+ skip_before_filter :check_xhr, only: [:show, :password_reset, :update, :account_created, :activate_account, :perform_account_activation, :user_preferences_redirect, :avatar, :my_redirect, :toggle_anon, :admin_login, :confirm_admin]
before_filter :ensure_logged_in, only: [:username, :update, :user_preferences_redirect, :upload_user_image,
:pick_avatar, :destroy_user_image, :destroy, :check_emails, :topic_tracking_state]
@@ -27,7 +28,8 @@ class UsersController < ApplicationController
:send_activation_email,
:password_reset,
:confirm_email_token,
- :admin_login]
+ :admin_login,
+ :confirm_admin]
def index
end
@@ -726,6 +728,21 @@ class UsersController < ApplicationController
render json: result
end
+ def confirm_admin
+ @confirmation = AdminConfirmation.find_by_code(params[:token])
+
+ raise Discourse::NotFound unless @confirmation
+ raise Discourse::InvalidAccess.new unless
+ @confirmation.performed_by.id == (current_user&.id || @confirmation.performed_by.id)
+
+ if request.post?
+ @confirmation.email_confirmed!
+ @confirmed = true
+ end
+
+ render layout: 'no_ember'
+ end
+
private
def honeypot_value
diff --git a/app/jobs/regular/admin_confirmation_email.rb b/app/jobs/regular/admin_confirmation_email.rb
new file mode 100644
index 00000000000..dbebf609629
--- /dev/null
+++ b/app/jobs/regular/admin_confirmation_email.rb
@@ -0,0 +1,21 @@
+require_dependency 'email/sender'
+
+module Jobs
+ class AdminConfirmationEmail < Jobs::Base
+ sidekiq_options queue: 'critical'
+
+ def execute(args)
+ to_address = args[:to_address]
+ token = args[:token]
+ target_username = args[:target_username]
+
+ raise Discourse::InvalidParameters.new(:to_address) if to_address.blank?
+ raise Discourse::InvalidParameters.new(:token) if token.blank?
+ raise Discourse::InvalidParameters.new(:target_username) if target_username.blank?
+
+ message = AdminConfirmationMailer.send_email(to_address, target_username, token)
+ Email::Sender.new(message, :admin_confirmation_message).send
+ end
+
+ end
+end
diff --git a/app/mailers/admin_confirmation_mailer.rb b/app/mailers/admin_confirmation_mailer.rb
new file mode 100644
index 00000000000..ea4d444ad8c
--- /dev/null
+++ b/app/mailers/admin_confirmation_mailer.rb
@@ -0,0 +1,15 @@
+require_dependency 'email/message_builder'
+
+class AdminConfirmationMailer < ActionMailer::Base
+ include Email::BuildEmailHelper
+
+ def send_email(to_address, target_username, token)
+ build_email(
+ to_address,
+ template: 'admin_confirmation_mailer',
+ target_username: target_username,
+ admin_confirm_url: confirm_admin_url(token: token, host: Discourse.base_url_no_prefix)
+ )
+ end
+end
+
diff --git a/app/views/users/confirm_admin.html.erb b/app/views/users/confirm_admin.html.erb
new file mode 100644
index 00000000000..4e996da74a7
--- /dev/null
+++ b/app/views/users/confirm_admin.html.erb
@@ -0,0 +1,20 @@
+
+
<%= t 'activation.admin_confirm.title' %>
+
+
+ <%- unless @confirmed %>
+ <%=raw (t('activation.admin_confirm.description', target_username: @confirmation.target_user.username)) %>
+
+
+
+ <%= form_tag do %>
+ <%= submit_tag t('activation.admin_confirm.grant'), class: 'btn btn-primary' %>
+ <% end %>
+ <% else %>
+ <%=raw (t('activation.admin_confirm.complete', target_username: @confirmation.target_user.username)) %>
+
+
+ <%= link_to t("activation.admin_confirm.back_to", title: SiteSetting.title), "/", class: 'btn btn-primary' %>
+ <% end %>
+
+
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index 9850258dcc0..ab0caede55c 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -3111,6 +3111,7 @@ en:
logged_out: "User was logged out on all devices"
revoke_admin: 'Revoke Admin'
grant_admin: 'Grant Admin'
+ grant_admin_confirm: "We've sent you an email to verify the new administrator. Please open it and follow the instructions."
revoke_moderation: 'Revoke Moderation'
grant_moderation: 'Grant Moderation'
unblock: 'Unblock'
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index d83e26658a8..e952de369d0 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -624,6 +624,12 @@ en:
approval_required: "A moderator must manually approve your new account before you can access this forum. You'll get an email when your account is approved!"
missing_session: "We cannot detect if your account was created, please ensure you have cookies enabled."
activated: "Sorry, this account has already been activated."
+ admin_confirm:
+ title: "Confirm Admin Account"
+ description: "Are you sure you want %{target_username} to be an administrator?"
+ grant: "Grant Admin Access"
+ complete: "%{target_username} is now an administrator."
+ back_to: "Return to %{title}"
post_action_types:
off_topic:
title: 'Off-Topic'
@@ -1811,6 +1817,14 @@ en:
no_token: |
Sorry, this backup download link has already been used or has expired.
+ admin_confirmation_mailer:
+ title: "Admin Confirmation"
+ subject_template: "[%{email_prefix}] Confirm new Admin Account"
+ text_body_template: |
+ Please confirm that you'd like to add **%{target_username}** as an administrator for your forum.
+
+ [Confirm Administrator Account](%{admin_confirm_url})
+
test_mailer:
title: "Test Mailer"
subject_template: "[%{email_prefix}] Email Deliverability Test"
diff --git a/config/routes.rb b/config/routes.rb
index b33b21830cb..8456bbb50b6 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -322,6 +322,11 @@ Discourse::Application.routes.draw do
get "#{root_path}/activate-account/:token" => "users#activate_account"
put({ "#{root_path}/activate-account/:token" => "users#perform_account_activation" }.merge(index == 1 ? { as: 'perform_activate_account' } : {}))
get "#{root_path}/authorize-email/:token" => "users_email#confirm"
+ get({
+ "#{root_path}/confirm-admin/:token" => "users#confirm_admin",
+ constraints: { token: /[0-9a-f]+/ }
+ }.merge(index == 1 ? { as: 'confirm_admin' } : {}))
+ post "#{root_path}/confirm-admin/:token" => "users#confirm_admin", constraints: { token: /[0-9a-f]+/ }
get "#{root_path}/:username/private-messages" => "user_actions#private_messages", constraints: {username: USERNAME_ROUTE_FORMAT}
get "#{root_path}/:username/private-messages/:filter" => "user_actions#private_messages", constraints: {username: USERNAME_ROUTE_FORMAT}
get "#{root_path}/:username/messages" => "user_actions#private_messages", constraints: {username: USERNAME_ROUTE_FORMAT}
diff --git a/lib/admin_confirmation.rb b/lib/admin_confirmation.rb
new file mode 100644
index 00000000000..cb756750525
--- /dev/null
+++ b/lib/admin_confirmation.rb
@@ -0,0 +1,59 @@
+class AdminConfirmation
+ attr_accessor :token
+ attr_reader :performed_by
+ attr_reader :target_user
+
+ def initialize(target_user, performed_by)
+ @target_user = target_user
+ @performed_by = performed_by
+ end
+
+ def create_confirmation
+ guardian = Guardian.new(@performed_by)
+ guardian.ensure_can_grant_admin!(@target_user)
+
+ @token = SecureRandom.hex
+ $redis.setex("admin-confirmation:#{@target_user.id}", 3.hours.to_i, @token)
+
+ payload = {
+ target_user_id: @target_user.id,
+ performed_by: @performed_by.id
+ }
+ $redis.setex("admin-confirmation-token:#{@token}", 3.hours.to_i, payload.to_json)
+
+ Jobs.enqueue(
+ :admin_confirmation_email,
+ to_address: @performed_by.email,
+ target_username: @target_user.username,
+ token: @token
+ )
+ end
+
+ def email_confirmed!
+ guardian = Guardian.new(@performed_by)
+ guardian.ensure_can_grant_admin!(@target_user)
+
+ @target_user.grant_admin!
+ StaffActionLogger.new(@performed_by).log_grant_admin(@target_user)
+ $redis.del "admin-confirmation:#{@target_user.id}"
+ $redis.del "admin-confirmation-token:#{@token}"
+ end
+
+ def self.exists_for?(user_id)
+ $redis.exists "admin-confirmation:#{user_id}"
+ end
+
+ def self.find_by_code(token)
+ json = $redis.get("admin-confirmation-token:#{token}")
+ return nil unless json
+
+ parsed = JSON.parse(json)
+ target_user = User.find(parsed['target_user_id'].to_i)
+ performed_by = User.find(parsed['performed_by'].to_i)
+
+ ac = AdminConfirmation.new(target_user, performed_by)
+ ac.token = token
+ ac
+ end
+
+end
diff --git a/spec/components/admin_confirmation_spec.rb b/spec/components/admin_confirmation_spec.rb
new file mode 100644
index 00000000000..20d74fec96b
--- /dev/null
+++ b/spec/components/admin_confirmation_spec.rb
@@ -0,0 +1,54 @@
+require 'admin_confirmation'
+require 'rails_helper'
+
+describe AdminConfirmation do
+
+ let(:admin) { Fabricate(:admin) }
+ let(:user) { Fabricate(:user) }
+
+ describe "create_confirmation" do
+ it "raises an error for non-admins" do
+ ac = AdminConfirmation.new(user, Fabricate(:moderator))
+ expect { ac.create_confirmation }.to raise_error(Discourse::InvalidAccess)
+ end
+ end
+
+ describe "email_confirmed!" do
+ before do
+ ac = AdminConfirmation.new(user, admin)
+ ac.create_confirmation
+ @token = ac.token
+ end
+
+ it "cannot confirm if the user loses admin access" do
+ ac = AdminConfirmation.find_by_code(@token)
+ ac.performed_by.update_column(:admin, false)
+ expect { ac.email_confirmed! }.to raise_error(Discourse::InvalidAccess)
+ end
+
+ it "can confirm admin accounts" do
+ ac = AdminConfirmation.find_by_code(@token)
+ expect(ac.performed_by).to eq(admin)
+ expect(ac.target_user).to eq(user)
+ expect(ac.token).to eq(@token)
+ ac.email_confirmed!
+
+ user.reload
+ expect(user.admin?).to eq(true)
+
+ # It creates a staff log
+ logs = UserHistory.where(
+ action: UserHistory.actions[:grant_admin],
+ target_user_id: user.id
+ )
+ expect(logs).to be_present
+
+ # It removes the redis keys for another user
+ expect(AdminConfirmation.find_by_code(ac.token)).to eq(nil)
+ expect(AdminConfirmation.exists_for?(user.id)).to eq(false)
+ end
+
+ end
+
+end
+
diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb
index 912338f262c..23bb1ebe8c7 100644
--- a/spec/controllers/admin/users_controller_spec.rb
+++ b/spec/controllers/admin/users_controller_spec.rb
@@ -166,9 +166,9 @@ describe Admin::UsersController do
end
it 'updates the admin flag' do
+ expect(AdminConfirmation.exists_for?(@another_user.id)).to eq(false)
xhr :put, :grant_admin, user_id: @another_user.id
- @another_user.reload
- expect(@another_user).to be_admin
+ expect(AdminConfirmation.exists_for?(@another_user.id)).to eq(true)
end
end
@@ -491,7 +491,14 @@ describe Admin::UsersController do
end
context ".invite_admin" do
+ it "doesn't work when not via API" do
+ controller.stubs(:is_api?).returns(false)
+ xhr :post, :invite_admin, name: 'Bill', username: 'bill22', email: 'bill@bill.com'
+ expect(response).not_to be_success
+ end
+
it 'should invite admin' do
+ controller.stubs(:is_api?).returns(true)
Jobs.expects(:enqueue).with(:critical_user_email, anything).returns(true)
xhr :post, :invite_admin, name: 'Bill', username: 'bill22', email: 'bill@bill.com'
expect(response).to be_success
@@ -503,6 +510,7 @@ describe Admin::UsersController do
end
it "doesn't send the email with send_email falsy" do
+ controller.stubs(:is_api?).returns(true)
Jobs.expects(:enqueue).with(:user_email, anything).never
xhr :post, :invite_admin, name: 'Bill', username: 'bill22', email: 'bill@bill.com', send_email: '0'
expect(response).to be_success
diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb
index 119859d3f5e..5d6ed231056 100644
--- a/spec/controllers/users_controller_spec.rb
+++ b/spec/controllers/users_controller_spec.rb
@@ -1807,4 +1807,69 @@ describe UsersController do
end
end
+
+ describe ".confirm_admin" do
+ it "fails without a valid token" do
+ expect {
+ get :confirm_admin, token: 'invalid-token'
+ }.to raise_error(ActionController::UrlGenerationError)
+ end
+
+ it "fails with a missing token" do
+ get :confirm_admin, token: 'a0a0a0a0a0'
+ expect(response).to_not be_success
+ end
+
+ it "succeeds with a valid code as anonymous" do
+ user = Fabricate(:user)
+ ac = AdminConfirmation.new(user, Fabricate(:admin))
+ ac.create_confirmation
+ get :confirm_admin, token: ac.token
+ expect(response).to be_success
+
+ user.reload
+ expect(user.admin?).to eq(false)
+ end
+
+ it "succeeds with a valid code when logged in as that user" do
+ admin = log_in(:admin)
+ user = Fabricate(:user)
+
+ ac = AdminConfirmation.new(user, admin)
+ ac.create_confirmation
+ get :confirm_admin, token: ac.token
+ expect(response).to be_success
+
+ user.reload
+ expect(user.admin?).to eq(false)
+ end
+
+ it "fails if you're logged in as a different account" do
+ log_in(:admin)
+ user = Fabricate(:user)
+
+ ac = AdminConfirmation.new(user, Fabricate(:admin))
+ ac.create_confirmation
+ get :confirm_admin, token: ac.token
+ expect(response).to_not be_success
+
+ user.reload
+ expect(user.admin?).to eq(false)
+ end
+
+ describe "post" do
+ it "gives the user admin access when POSTed" do
+ user = Fabricate(:user)
+ ac = AdminConfirmation.new(user, Fabricate(:admin))
+ ac.create_confirmation
+ post :confirm_admin, token: ac.token
+ expect(response).to be_success
+
+ user.reload
+ expect(user.admin?).to eq(true)
+ end
+ end
+
+ end
+
end