FEATURE: Use second factor for admin confirmation ()

Administrators can use second factor to confirm granting admin access
without using email. The old method of confirmation via email is still
used as a fallback when second factor is unavailable.
This commit is contained in:
Bianca Nenciu 2021-09-14 15:19:28 +03:00 committed by GitHub
parent f517b6997c
commit 6a7ea66670
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 253 additions and 6 deletions
app
assets/javascripts
admin/addon
discourse
controllers/admin
config/locales
spec/requests/admin

@ -218,8 +218,15 @@ export default Controller.extend(CanCheckEmails, {
grantAdmin() {
return this.model
.grantAdmin()
.then(() => {
.then((result) => {
if (result.email_confirmation_required) {
bootbox.alert(I18n.t("admin.user.grant_admin_confirm"));
} else {
const controller = showModal("grant-admin-second-factor", {
model: this.model,
});
controller.setResult(result);
}
})
.catch(popupAjaxError);
},

@ -99,9 +99,20 @@ const AdminUser = User.extend({
});
},
grantAdmin() {
grantAdmin(data) {
return ajax(`/admin/users/${this.id}/grant_admin`, {
type: "PUT",
data,
}).then((resp) => {
if (resp.success && !resp.email_confirmation_required) {
this.setProperties({
admin: true,
can_grant_admin: false,
can_revoke_admin: true,
});
}
return resp;
});
},

@ -334,7 +334,7 @@
{{/if}}
{{#if model.can_grant_admin}}
{{d-button
class="btn-default"
class="btn-default grant-admin"
action=(action "grantAdmin")
icon="shield-alt"
label="admin.user.grant_admin"}}

@ -0,0 +1,83 @@
import Controller from "@ember/controller";
import { action } from "@ember/object";
import discourseComputed from "discourse-common/utils/decorators";
import { getWebauthnCredential } from "discourse/lib/webauthn";
import ModalFunctionality from "discourse/mixins/modal-functionality";
import { SECOND_FACTOR_METHODS } from "discourse/models/user";
import I18n from "I18n";
export default Controller.extend(ModalFunctionality, {
showSecondFactor: false,
secondFactorMethod: SECOND_FACTOR_METHODS.TOTP,
secondFactorToken: null,
securityKeyCredential: null,
inProgress: false,
onShow() {
this.setProperties({
showSecondFactor: false,
secondFactorMethod: SECOND_FACTOR_METHODS.TOTP,
secondFactorToken: null,
securityKeyCredential: null,
});
},
@discourseComputed("inProgress", "securityKeyCredential", "secondFactorToken")
disabled(inProgress, securityKeyCredential, secondFactorToken) {
return inProgress || (!securityKeyCredential && !secondFactorToken);
},
setResult(result) {
this.setProperties({
otherMethodAllowed: result.multiple_second_factor_methods,
secondFactorRequired: true,
showLoginButtons: false,
backupEnabled: result.backup_enabled,
showSecondFactor: result.totp_enabled,
showSecurityKey: result.security_key_enabled,
secondFactorMethod: result.security_key_enabled
? SECOND_FACTOR_METHODS.SECURITY_KEY
: SECOND_FACTOR_METHODS.TOTP,
securityKeyChallenge: result.challenge,
securityKeyAllowedCredentialIds: result.allowed_credential_ids,
});
},
@action
authenticateSecurityKey() {
getWebauthnCredential(
this.securityKeyChallenge,
this.securityKeyAllowedCredentialIds,
(credentialData) => {
this.set("securityKeyCredential", credentialData);
this.send("authenticate");
},
(errorMessage) => {
this.flash(errorMessage, "error");
}
);
},
@action
authenticate() {
this.set("inProgress", true);
this.model
.grantAdmin({
second_factor_token:
this.securityKeyCredential || this.secondFactorToken,
second_factor_method: this.secondFactorMethod,
timezone: moment.tz.guess(),
})
.then((result) => {
if (result.success) {
this.send("closeModal");
bootbox.alert(I18n.t("admin.user.grant_admin_success"));
} else {
this.flash(result.error, "error");
this.setResult(result);
}
})
.finally(() => this.set("inProgress", false));
},
});

@ -0,0 +1,33 @@
{{#d-modal-body title="admin.user.grant_admin"}}
{{#second-factor-form
secondFactorMethod=secondFactorMethod
secondFactorToken=secondFactorToken
class=secondFactorClass
backupEnabled=backupEnabled
}}
{{#if showSecurityKey}}
{{#security-key-form
allowedCredentialIds=securityKeyAllowedCredentialIds
challenge=securityKeyChallenge
showSecurityKey=showSecurityKey
showSecondFactor=showSecondFactor
secondFactorMethod=secondFactorMethod
otherMethodAllowed=otherMethodAllowed
action=(action "authenticateSecurityKey")}}
{{/security-key-form}}
{{else}}
{{second-factor-input value=secondFactorToken inputId="second-factor-confirmation" secondFactorMethod=secondFactorMethod backupEnabled=backupEnabled}}
{{/if}}
{{/second-factor-form}}
{{#unless showSecurityKey}}
<div class="modal-footer">
{{d-button
action=(action "authenticate")
icon="shield-alt"
label="admin.user.grant_admin"
disabled=disabled
class="btn btn-primary"}}
</div>
{{/unless}}
{{/d-modal-body}}

@ -1,11 +1,13 @@
import {
acceptance,
exists,
query,
queryAll,
} from "discourse/tests/helpers/qunit-helpers";
import { click, currentURL, fillIn, visit } from "@ember/test-helpers";
import selectKit from "discourse/tests/helpers/select-kit-helper";
import { test } from "qunit";
import I18n from "I18n";
acceptance("Admin - User Index", function (needs) {
needs.user();
@ -40,6 +42,60 @@ acceptance("Admin - User Index", function (needs) {
server.put("/users/sam/preferences/username", () => {
return helper.response({ id: 2, username: "new-sam" });
});
server.get("/admin/users/3.json", () => {
return helper.response({
id: 3,
username: "user1",
name: null,
avatar_template: "/letter_avatar_proxy/v4/letter/b/f0a364/{size}.png",
active: true,
admin: false,
moderator: false,
can_grant_admin: true,
can_revoke_admin: false,
can_grant_moderation: true,
can_revoke_moderation: false,
});
});
server.put("/admin/users/3/grant_admin", () => {
return helper.response({
success: "OK",
email_confirmation_required: true,
});
});
server.get("/admin/users/4.json", () => {
return helper.response({
id: 4,
username: "user2",
name: null,
avatar_template: "/letter_avatar_proxy/v4/letter/b/f0a364/{size}.png",
active: true,
admin: false,
moderator: false,
can_grant_admin: true,
can_revoke_admin: false,
can_grant_moderation: true,
can_revoke_moderation: false,
});
});
server.put("/admin/users/4/grant_admin", () => {
return helper.response({
failed: "FAILED",
ok: false,
error: "The selected two-factor method is invalid.",
reason: "invalid_second_factor_method",
backup_enabled: true,
security_key_enabled: true,
totp_enabled: true,
multiple_second_factor_methods: true,
allowed_credential_ids: ["allowed_credential_ids"],
challenge: "challenge",
});
});
});
test("can edit username", async function (assert) {
@ -124,4 +180,21 @@ acceptance("Admin - User Index", function (needs) {
"group should not be set"
);
});
test("grant admin - shows the confirmation bootbox", async function (assert) {
await visit("/admin/users/3/user1");
await click(".grant-admin");
assert.ok(exists(".bootbox"));
assert.equal(
I18n.t("admin.user.grant_admin_confirm"),
query(".modal-body").textContent.trim()
);
await click(".bootbox .btn-primary");
});
test("grant admin - shows the second factor modal", async function (assert) {
await visit("/admin/users/4/user2");
await click(".grant-admin");
assert.ok(exists(".grant-admin-second-factor-modal"));
});
});

@ -191,8 +191,25 @@ class Admin::UsersController < Admin::AdminController
end
def grant_admin
AdminConfirmation.new(@user, current_user).create_confirmation
guardian.ensure_can_grant_admin!(@user)
if current_user.has_any_second_factor_methods_enabled?
second_factor_authentication_result = current_user.authenticate_second_factor(params, secure_session)
if second_factor_authentication_result.ok
@user.grant_admin!
StaffActionLogger.new(current_user).log_grant_admin(@user)
render json: success_json
else
failure_payload = second_factor_authentication_result.to_h
if current_user.security_keys_enabled?
Webauthn.stage_challenge(current_user, secure_session)
failure_payload.merge!(Webauthn.allowed_credentials(current_user, secure_session))
end
render json: failed_json.merge(failure_payload)
end
else
AdminConfirmation.new(@user, current_user).create_confirmation
render json: success_json.merge(email_confirmation_required: true)
end
end
def revoke_moderation

@ -4960,6 +4960,7 @@ en:
logged_out: "User was logged out on all devices"
revoke_admin: "Revoke Admin"
grant_admin: "Grant Admin"
grant_admin_success: "New administrator was confirmed."
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"

@ -2,6 +2,7 @@
require 'rails_helper'
require 'discourse_ip_info'
require 'rotp'
RSpec.describe Admin::UsersController do
fab!(:admin) { Fabricate(:admin) }
@ -362,6 +363,27 @@ RSpec.describe Admin::UsersController do
expect(response.status).to eq(200)
expect(AdminConfirmation.exists_for?(another_user.id)).to eq(true)
end
it 'asks user for second factor if it is enabled' do
user_second_factor = Fabricate(:user_second_factor_totp, user: admin)
put "/admin/users/#{another_user.id}/grant_admin.json"
expect(response.parsed_body["failed"]).to eq("FAILED")
expect(another_user.reload.admin).to eq(false)
end
it 'grants admin if second factor is correct' do
user_second_factor = Fabricate(:user_second_factor_totp, user: admin)
put "/admin/users/#{another_user.id}/grant_admin.json", params: {
second_factor_token: ROTP::TOTP.new(user_second_factor.data).now,
second_factor_method: UserSecondFactor.methods[:totp]
}
expect(response.parsed_body["success"]).to eq("OK")
expect(another_user.reload.admin).to eq(true)
end
end
describe '#add_group' do