mirror of
https://github.com/discourse/discourse.git
synced 2024-11-27 03:10:46 -06:00
FEATURE: Allow users to confirm session with passkeys (#24337)
We ask users to confirm their session if they are making a sensitive action, such as adding/updating second factors or passkeys. This commit adds the ability to confirm sessions with passkeys as an option to the password confirmation.
This commit is contained in:
parent
c36ee3bc02
commit
c6ead3f5c4
@ -6,11 +6,17 @@ import { inject as service } from "@ember/service";
|
||||
import DButton from "discourse/components/d-button";
|
||||
import UserLink from "discourse/components/user-link";
|
||||
import { ajax } from "discourse/lib/ajax";
|
||||
import { popupAjaxError } from "discourse/lib/ajax-error";
|
||||
import {
|
||||
getPasskeyCredential,
|
||||
isWebauthnSupported,
|
||||
} from "discourse/lib/webauthn";
|
||||
import I18n from "discourse-i18n";
|
||||
|
||||
export default class ConfirmSession extends Component {
|
||||
@service dialog;
|
||||
@service currentUser;
|
||||
@service siteSettings;
|
||||
|
||||
@tracked errorMessage;
|
||||
|
||||
@ -19,8 +25,48 @@ export default class ConfirmSession extends Component {
|
||||
loggedInAs = I18n.t("user.confirm_access.logged_in_as");
|
||||
finePrint = I18n.t("user.confirm_access.fine_print");
|
||||
|
||||
get canUsePasskeys() {
|
||||
return (
|
||||
this.siteSettings.enable_local_logins &&
|
||||
this.siteSettings.enable_passkeys &&
|
||||
this.currentUser.user_passkeys?.length > 0 &&
|
||||
isWebauthnSupported()
|
||||
);
|
||||
}
|
||||
|
||||
@action
|
||||
async confirmWithPasskey() {
|
||||
try {
|
||||
const publicKeyCredential = await getPasskeyCredential((e) =>
|
||||
this.dialog.alert(e)
|
||||
);
|
||||
|
||||
if (!publicKeyCredential) {
|
||||
return;
|
||||
}
|
||||
|
||||
const result = await ajax("/u/confirm-session", {
|
||||
type: "POST",
|
||||
data: { publicKeyCredential },
|
||||
});
|
||||
|
||||
if (result.success) {
|
||||
this.errorMessage = null;
|
||||
this.dialog.didConfirmWrapped();
|
||||
} else {
|
||||
this.errorMessage = I18n.t("user.confirm_access.incorrect_passkey");
|
||||
}
|
||||
} catch (e) {
|
||||
popupAjaxError(e);
|
||||
}
|
||||
}
|
||||
|
||||
@action
|
||||
async submit() {
|
||||
this.errorMessage = this.password
|
||||
? null
|
||||
: I18n.t("user.confirm_access.incorrect_password");
|
||||
|
||||
const result = await ajax("/u/confirm-session", {
|
||||
type: "POST",
|
||||
data: {
|
||||
@ -73,6 +119,15 @@ export default class ConfirmSession extends Component {
|
||||
@label="user.password.confirm"
|
||||
/>
|
||||
</div>
|
||||
{{#if this.canUsePasskeys}}
|
||||
<div class="confirm-session__passkey">
|
||||
<DButton
|
||||
class="btn-flat"
|
||||
@action={{this.confirmWithPasskey}}
|
||||
@label="user.passkeys.confirm_button"
|
||||
/>
|
||||
</div>
|
||||
{{/if}}
|
||||
</div>
|
||||
</form>
|
||||
|
||||
|
@ -118,28 +118,10 @@ export default class Login extends Component {
|
||||
@action
|
||||
async passkeyLogin(mediation = "optional") {
|
||||
try {
|
||||
// we need to check isConditionalMediationAvailable for Firefox
|
||||
// without it, Firefox will throw console errors
|
||||
// We cannot do a general check because iOS Safari and Chrome in Selenium quietly support the feature
|
||||
// but they do not support the PublicKeyCredential.isConditionalMediationAvailable() method
|
||||
if (
|
||||
mediation === "conditional" &&
|
||||
this.capabilities.isFirefox &&
|
||||
window.PublicKeyCredential
|
||||
) {
|
||||
const isCMA =
|
||||
// eslint-disable-next-line no-undef
|
||||
await PublicKeyCredential.isConditionalMediationAvailable();
|
||||
if (!isCMA) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
const response = await ajax("/session/passkey/challenge.json");
|
||||
|
||||
const publicKeyCredential = await getPasskeyCredential(
|
||||
response.challenge,
|
||||
(errorMessage) => this.dialog.alert(errorMessage),
|
||||
mediation
|
||||
(e) => this.dialog.alert(e),
|
||||
mediation,
|
||||
this.capabilities.isFirefox
|
||||
);
|
||||
|
||||
if (publicKeyCredential) {
|
||||
|
@ -9,7 +9,11 @@ import PasskeyOptionsDropdown from "discourse/components/user-preferences/passke
|
||||
import RenamePasskey from "discourse/components/user-preferences/rename-passkey";
|
||||
import formatDate from "discourse/helpers/format-date";
|
||||
import { popupAjaxError } from "discourse/lib/ajax-error";
|
||||
import { bufferToBase64, stringToBuffer } from "discourse/lib/webauthn";
|
||||
import {
|
||||
bufferToBase64,
|
||||
stringToBuffer,
|
||||
WebauthnAbortHandler,
|
||||
} from "discourse/lib/webauthn";
|
||||
import I18n from "discourse-i18n";
|
||||
|
||||
export default class UserPasskeys extends Component {
|
||||
@ -71,6 +75,7 @@ export default class UserPasskeys extends Component {
|
||||
|
||||
const credential = await navigator.credentials.create({
|
||||
publicKey: publicKeyCredentialCreationOptions,
|
||||
signal: WebauthnAbortHandler.signal(),
|
||||
});
|
||||
|
||||
let credentialParam = {
|
||||
|
@ -1,3 +1,6 @@
|
||||
/* global PublicKeyCredential */
|
||||
|
||||
import { ajax } from "discourse/lib/ajax";
|
||||
import I18n from "discourse-i18n";
|
||||
|
||||
export function stringToBuffer(str) {
|
||||
@ -36,7 +39,7 @@ class WebauthnAbortService {
|
||||
|
||||
// Need to use a singleton here to reset the active webauthn ceremony
|
||||
// Inspired by the BaseWebAuthnAbortService in https://github.com/MasterKale/SimpleWebAuthn
|
||||
const WebauthnAbortHandler = new WebauthnAbortService();
|
||||
export const WebauthnAbortHandler = new WebauthnAbortService();
|
||||
|
||||
export function getWebauthnCredential(
|
||||
challenge,
|
||||
@ -117,18 +120,31 @@ export function getWebauthnCredential(
|
||||
}
|
||||
|
||||
export async function getPasskeyCredential(
|
||||
challenge,
|
||||
errorCallback,
|
||||
mediation = "optional"
|
||||
mediation = "optional",
|
||||
isFirefox = false
|
||||
) {
|
||||
if (!isWebauthnSupported()) {
|
||||
return errorCallback(I18n.t("login.security_key_support_missing_error"));
|
||||
}
|
||||
|
||||
// we need to check isConditionalMediationAvailable for Firefox
|
||||
// without it, Firefox will throw console errors
|
||||
// We cannot do a general check because iOS Safari and Chrome in Selenium quietly support the feature
|
||||
// but they do not support the PublicKeyCredential.isConditionalMediationAvailable() method
|
||||
if (mediation === "conditional" && isFirefox) {
|
||||
const isCMA = await PublicKeyCredential.isConditionalMediationAvailable();
|
||||
if (!isCMA) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
try {
|
||||
const resp = await ajax("/session/passkey/challenge.json");
|
||||
|
||||
const credential = await navigator.credentials.get({
|
||||
publicKey: {
|
||||
challenge: stringToBuffer(challenge),
|
||||
challenge: stringToBuffer(resp.challenge),
|
||||
// https://www.w3.org/TR/webauthn-2/#user-verification
|
||||
// for passkeys (first factor), user verification should be marked as required
|
||||
// it ensures browser requests PIN or biometrics before authenticating
|
||||
|
@ -136,6 +136,14 @@ acceptance("User Preferences - Security", function (needs) {
|
||||
"displays a dialog to confirm the user's identity before adding a passkey"
|
||||
);
|
||||
|
||||
assert
|
||||
.dom(".dialog-body #password")
|
||||
.exists("dialog includes a password field");
|
||||
|
||||
assert
|
||||
.dom(".dialog-body .confirm-session__passkey")
|
||||
.exists("dialog includes a passkey button");
|
||||
|
||||
await click(".dialog-close");
|
||||
|
||||
const dropdown = selectKit(".passkey-options-dropdown");
|
||||
@ -176,6 +184,12 @@ acceptance("User Preferences - Security", function (needs) {
|
||||
.exists(
|
||||
"displays a dialog to confirm the user's identity before adding a passkey"
|
||||
);
|
||||
|
||||
assert.dom(".dialog-body #password").exists("includes a password field");
|
||||
|
||||
assert
|
||||
.dom(".dialog-body .confirm-session__passkey")
|
||||
.doesNotExist("does not include a passkey button");
|
||||
});
|
||||
|
||||
test("Viewing Passkeys - another user has a key", async function (assert) {
|
||||
|
@ -803,6 +803,11 @@
|
||||
margin: 1.5em 0;
|
||||
}
|
||||
|
||||
&__passkey .btn {
|
||||
padding-left: 0.25em;
|
||||
padding-right: 0.25em;
|
||||
}
|
||||
|
||||
&__fine-print {
|
||||
font-size: var(--font-down-1);
|
||||
color: var(--primary-medium);
|
||||
|
@ -1510,9 +1510,6 @@ class UsersController < ApplicationController
|
||||
end
|
||||
|
||||
def confirm_session
|
||||
# TODO(pmusaraj): add support for confirming via passkey, 2FA
|
||||
params.require(:password)
|
||||
|
||||
if SiteSetting.enable_discourse_connect || !SiteSetting.enable_local_logins
|
||||
raise Discourse::NotFound
|
||||
end
|
||||
@ -1520,8 +1517,10 @@ class UsersController < ApplicationController
|
||||
if confirm_secure_session
|
||||
render json: success_json
|
||||
else
|
||||
render json: failed_json.merge(error: I18n.t("login.incorrect_password"))
|
||||
render json: failed_json.merge(error: I18n.t("login.incorrect_password_or_passkey"))
|
||||
end
|
||||
rescue ::DiscourseWebauthn::SecurityKeyError => err
|
||||
render_json_error(err.message, status: 401)
|
||||
end
|
||||
|
||||
def trusted_session
|
||||
@ -2183,13 +2182,32 @@ class UsersController < ApplicationController
|
||||
SiteSetting.max_logins_per_ip_per_minute,
|
||||
1.minute,
|
||||
).performed!
|
||||
return false if !current_user.confirm_password?(params[:password])
|
||||
|
||||
secure_session["confirmed-password-#{current_user.id}"] = "true"
|
||||
if !params[:password].present? && !params[:publicKeyCredential].present?
|
||||
raise Discourse::InvalidParameters.new "Missing password or passkey"
|
||||
end
|
||||
|
||||
if params[:password].present?
|
||||
return false if !current_user.confirm_password?(params[:password])
|
||||
end
|
||||
|
||||
if params[:publicKeyCredential].present?
|
||||
passkey =
|
||||
::DiscourseWebauthn::AuthenticationService.new(
|
||||
current_user,
|
||||
params[:publicKeyCredential],
|
||||
session: secure_session,
|
||||
factor_type: UserSecurityKey.factor_types[:first_factor],
|
||||
).authenticate_security_key
|
||||
|
||||
return false if !passkey
|
||||
end
|
||||
|
||||
secure_session["confirmed-session-#{current_user.id}"] = "true"
|
||||
end
|
||||
|
||||
def secure_session_confirmed?
|
||||
secure_session["confirmed-password-#{current_user.id}"] == "true"
|
||||
secure_session["confirmed-session-#{current_user.id}"] == "true"
|
||||
end
|
||||
|
||||
def summary_cache_key(user)
|
||||
|
@ -1526,6 +1526,7 @@ en:
|
||||
never_used: "Never Used"
|
||||
not_allowed_error: "The passkey registration process either timed out, was cancelled or is not allowed."
|
||||
already_added_error: "You have already registered this passkey. You don’t have to register it again."
|
||||
confirm_button: "or use a passkey"
|
||||
|
||||
change_about:
|
||||
title: "Change About Me"
|
||||
@ -1845,6 +1846,7 @@ en:
|
||||
confirm_access:
|
||||
title: "Confirm access"
|
||||
incorrect_password: "The entered password is incorrect."
|
||||
incorrect_passkey: "That passkey is incorrect."
|
||||
logged_in_as: "You are logged in as: "
|
||||
instructions: "Please confirm your identity in order to complete this action."
|
||||
fine_print: "We are asking you to confirm your identity because this is a potentially sensitive action. Once authenticated, you will only be asked to re-authenticate again after a few hours of inactivity."
|
||||
|
@ -2676,6 +2676,7 @@ en:
|
||||
not_approved: "Your account hasn't been approved yet. You will be notified by email when you are ready to log in."
|
||||
incorrect_username_email_or_password: "Incorrect username, email or password"
|
||||
incorrect_password: "Incorrect password"
|
||||
incorrect_password_or_passkey: "Incorrect password or passkey"
|
||||
wait_approval: "Thanks for signing up. We will notify you when your account has been approved."
|
||||
active: "Your account is activated and ready to use."
|
||||
activate_email: "<p>You’re almost done! We sent an activation mail to <b>%{email}</b>. Please follow the instructions in the mail to activate your account.</p><p>If it doesn’t arrive, check your spam folder.</p>"
|
||||
|
@ -5447,7 +5447,7 @@ RSpec.describe UsersController do
|
||||
ApplicationController
|
||||
.any_instance
|
||||
.expects(:secure_session)
|
||||
.returns("confirmed-password-#{user1.id}" => "false")
|
||||
.returns("confirmed-session-#{user1.id}" => "false")
|
||||
post "/users/create_second_factor_totp.json"
|
||||
|
||||
expect(response.status).to eq(403)
|
||||
@ -5478,7 +5478,7 @@ RSpec.describe UsersController do
|
||||
ApplicationController
|
||||
.any_instance
|
||||
.stubs(:secure_session)
|
||||
.returns("confirmed-password-#{user1.id}" => "true")
|
||||
.returns("confirmed-session-#{user1.id}" => "true")
|
||||
post "/users/create_second_factor_totp.json"
|
||||
|
||||
expect(response.status).to eq(200)
|
||||
@ -5704,7 +5704,7 @@ RSpec.describe UsersController do
|
||||
ApplicationController
|
||||
.any_instance
|
||||
.stubs(:secure_session)
|
||||
.returns("confirmed-password-#{user1.id}" => "true")
|
||||
.returns("confirmed-session-#{user1.id}" => "true")
|
||||
end
|
||||
it "should allow second factor backup for the user to be disabled" do
|
||||
put "/users/second_factor.json",
|
||||
@ -5744,7 +5744,7 @@ RSpec.describe UsersController do
|
||||
ApplicationController
|
||||
.any_instance
|
||||
.expects(:secure_session)
|
||||
.returns("confirmed-password-#{user1.id}" => "false")
|
||||
.returns("confirmed-session-#{user1.id}" => "false")
|
||||
put "/users/second_factors_backup.json"
|
||||
|
||||
expect(response.status).to eq(403)
|
||||
@ -5775,7 +5775,7 @@ RSpec.describe UsersController do
|
||||
ApplicationController
|
||||
.any_instance
|
||||
.expects(:secure_session)
|
||||
.returns("confirmed-password-#{user1.id}" => "true")
|
||||
.returns("confirmed-session-#{user1.id}" => "true")
|
||||
|
||||
put "/users/second_factors_backup.json"
|
||||
|
||||
@ -6388,18 +6388,25 @@ RSpec.describe UsersController do
|
||||
end
|
||||
end
|
||||
|
||||
context "when the site settings allow second factors" do
|
||||
context "when the site settings allow local logins" do
|
||||
before do
|
||||
SiteSetting.enable_local_logins = true
|
||||
SiteSetting.enable_discourse_connect = false
|
||||
end
|
||||
|
||||
context "when the password is wrong" do
|
||||
it "returns incorrect password response" do
|
||||
context "when params are incorrect" do
|
||||
it "returns 400 response if no password or passkey is supplied" do
|
||||
post "/u/confirm-session.json"
|
||||
|
||||
expect(response.status).to eq(400)
|
||||
expect(response.parsed_body["errors"][0]).to include("Missing password or passkey")
|
||||
end
|
||||
|
||||
it "returns incorrect response on a wrong password" do
|
||||
post "/u/confirm-session.json", params: { password: password }
|
||||
|
||||
expect(response.status).to eq(200)
|
||||
expect(response.parsed_body["error"]).to eq("Incorrect password")
|
||||
expect(response.parsed_body["error"]).to eq("Incorrect password or passkey")
|
||||
end
|
||||
end
|
||||
|
||||
@ -6413,6 +6420,69 @@ RSpec.describe UsersController do
|
||||
expect(response.parsed_body["error"]).to eq(nil)
|
||||
end
|
||||
end
|
||||
|
||||
context "with an invalid passkey" do
|
||||
it "returns invalid response" do
|
||||
post "/u/confirm-session.json", params: { publicKeyCredential: "someboringstring" }
|
||||
|
||||
expect(response.status).to eq(401)
|
||||
|
||||
json = response.parsed_body
|
||||
expect(json["errors"][0]).to eq(
|
||||
I18n.t("webauthn.validation.malformed_public_key_credential_error"),
|
||||
)
|
||||
end
|
||||
end
|
||||
|
||||
context "with a valid passkey" do
|
||||
fab!(:user2) { Fabricate(:user) }
|
||||
let!(:passkey) do
|
||||
Fabricate(
|
||||
:user_security_key,
|
||||
credential_id: valid_passkey_data[:credential_id],
|
||||
public_key: valid_passkey_data[:public_key],
|
||||
user: user1,
|
||||
factor_type: UserSecurityKey.factor_types[:first_factor],
|
||||
last_used: nil,
|
||||
name: "passkey",
|
||||
)
|
||||
end
|
||||
|
||||
it "returns a successful response for the correct user" do
|
||||
simulate_localhost_passkey_challenge
|
||||
user1.create_or_fetch_secure_identifier
|
||||
|
||||
post "/u/confirm-session.json",
|
||||
params: {
|
||||
publicKeyCredential:
|
||||
valid_passkey_auth_data.merge(
|
||||
{ userHandle: Base64.strict_encode64(user1.secure_identifier) },
|
||||
),
|
||||
}
|
||||
|
||||
expect(response.status).to eq(200)
|
||||
expect(response.parsed_body["error"]).to eq(nil)
|
||||
end
|
||||
|
||||
it "returns invalid response when key belongs to a different user" do
|
||||
sign_in(user2)
|
||||
simulate_localhost_passkey_challenge
|
||||
user2.create_or_fetch_secure_identifier
|
||||
|
||||
post "/u/confirm-session.json",
|
||||
params: {
|
||||
publicKeyCredential:
|
||||
valid_passkey_auth_data.merge(
|
||||
{ userHandle: Base64.strict_encode64(user2.secure_identifier) },
|
||||
),
|
||||
}
|
||||
|
||||
expect(response.status).to eq(401)
|
||||
|
||||
json = response.parsed_body
|
||||
expect(json["errors"][0]).to eq(I18n.t("webauthn.validation.ownership_error"))
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user