From 021a02c3d880448a9478ebdbd766bdfebd9c7cfe Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Mon, 12 Feb 2024 16:27:24 -0500 Subject: [PATCH] FIX: Webauthn origin was incorrect for subfolder setups (#25651) --- lib/discourse_webauthn.rb | 4 +--- .../lib/concern/second_factor_manager_spec.rb | 2 ++ .../authentication_service_spec.rb | 4 +++- .../discourse_webauthn_spec.rb | 16 ++++++++++++++ .../registration_service_spec.rb | 2 +- spec/requests/session_controller_spec.rb | 6 ++++++ spec/requests/users_controller_spec.rb | 21 +++++++++++++------ 7 files changed, 44 insertions(+), 11 deletions(-) create mode 100644 spec/lib/discourse_webauthn/discourse_webauthn_spec.rb diff --git a/lib/discourse_webauthn.rb b/lib/discourse_webauthn.rb index b6857f68315..eedefb6fff9 100644 --- a/lib/discourse_webauthn.rb +++ b/lib/discourse_webauthn.rb @@ -92,10 +92,8 @@ module DiscourseWebauthn # you might need to change this and the rp_id above # if you are using a non-default port/hostname locally "http://localhost:4200" - when "test" - "http://localhost:3000" else - Discourse.base_url + Discourse.base_url_no_prefix end end diff --git a/spec/lib/concern/second_factor_manager_spec.rb b/spec/lib/concern/second_factor_manager_spec.rb index 7cdf4a04a9b..f141e7b94eb 100644 --- a/spec/lib/concern/second_factor_manager_spec.rb +++ b/spec/lib/concern/second_factor_manager_spec.rb @@ -183,6 +183,7 @@ RSpec.describe SecondFactorManager do disable_totp simulate_localhost_webauthn_challenge DiscourseWebauthn.stage_challenge(user, secure_session) + DiscourseWebauthn.stubs(:origin).returns("http://localhost:3000") end context "when security key params are valid" do @@ -265,6 +266,7 @@ RSpec.describe SecondFactorManager do before do simulate_localhost_webauthn_challenge DiscourseWebauthn.stage_challenge(user, secure_session) + DiscourseWebauthn.stubs(:origin).returns("http://localhost:3000") end context "when method selected is invalid" do diff --git a/spec/lib/discourse_webauthn/authentication_service_spec.rb b/spec/lib/discourse_webauthn/authentication_service_spec.rb index 01df15769ec..cd93623c3cc 100644 --- a/spec/lib/discourse_webauthn/authentication_service_spec.rb +++ b/spec/lib/discourse_webauthn/authentication_service_spec.rb @@ -97,8 +97,10 @@ RSpec.describe DiscourseWebauthn::AuthenticationService do let(:current_user) { Fabricate(:user) } before do - # we have to stub here because the public key was created using this specific challenge + # we have to stub here because the test public key was created + # using this specific challenge and this origin DiscourseWebauthn.stubs(:challenge).returns(challenge) + DiscourseWebauthn.stubs(:origin).returns("http://localhost:3000") end it "updates last_used when the security key and params are valid" do diff --git a/spec/lib/discourse_webauthn/discourse_webauthn_spec.rb b/spec/lib/discourse_webauthn/discourse_webauthn_spec.rb new file mode 100644 index 00000000000..b1a3f01c5aa --- /dev/null +++ b/spec/lib/discourse_webauthn/discourse_webauthn_spec.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +RSpec.describe DiscourseWebauthn do + describe "#origin" do + it "returns the current hostname" do + expect(DiscourseWebauthn.origin).to eq("http://test.localhost") + end + + context "with subfolder" do + it "does not append /forum to origin" do + set_subfolder "/forum" + expect(DiscourseWebauthn.origin).to eq("http://test.localhost") + end + end + end +end diff --git a/spec/lib/discourse_webauthn/registration_service_spec.rb b/spec/lib/discourse_webauthn/registration_service_spec.rb index 077f9d20235..a9d2bdeb79f 100644 --- a/spec/lib/discourse_webauthn/registration_service_spec.rb +++ b/spec/lib/discourse_webauthn/registration_service_spec.rb @@ -7,7 +7,7 @@ RSpec.describe DiscourseWebauthn::RegistrationService do let(:secure_session) { SecureSession.new("tester") } let(:client_data_challenge) { Base64.encode64(challenge) } let(:client_data_webauthn_type) { "webauthn.create" } - let(:client_data_origin) { "http://localhost:3000" } + let(:client_data_origin) { "http://test.localhost" } let(:client_data_param) do { challenge: client_data_challenge, diff --git a/spec/requests/session_controller_spec.rb b/spec/requests/session_controller_spec.rb index f180efab1b0..f0393f9e748 100644 --- a/spec/requests/session_controller_spec.rb +++ b/spec/requests/session_controller_spec.rb @@ -403,6 +403,7 @@ RSpec.describe SessionController do before do simulate_localhost_webauthn_challenge + DiscourseWebauthn.stubs(:origin).returns("http://localhost:3000") # store challenge in secure session by visiting the email login page get "/session/email-login/#{email_token.token}.json" @@ -422,6 +423,7 @@ RSpec.describe SessionController do expect(response_body["error"]).to eq(I18n.t("login.not_enabled_second_factor_method")) end end + context "when the security key params are invalid" do it "shows an error message and denies login" do post "/session/email-login/#{email_token.token}.json", @@ -442,6 +444,7 @@ RSpec.describe SessionController do expect(response_body["error"]).to eq(I18n.t("webauthn.validation.not_found_error")) end end + context "when the security key params are valid" do it "logs the user in" do post "/session/email-login/#{email_token.token}.json", @@ -2021,6 +2024,7 @@ RSpec.describe SessionController do before do simulate_localhost_webauthn_challenge + DiscourseWebauthn.stubs(:origin).returns("http://localhost:3000") # store challenge in secure session by failing login once post "/session.json", params: { login: user.username, password: "myawesomepassword" } @@ -3097,6 +3101,8 @@ RSpec.describe SessionController do end describe "#passkey_login" do + before { DiscourseWebauthn.stubs(:origin).returns("http://localhost:3000") } + it "returns 404 if feature is not enabled" do SiteSetting.enable_passkeys = false diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 41ac3941d81..1c118466ab1 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -438,6 +438,7 @@ RSpec.describe UsersController do before do simulate_localhost_webauthn_challenge + DiscourseWebauthn.stubs(:origin).returns("http://localhost:3000") # store challenge in secure session by visiting the email login page get "/u/password-reset/#{email_token.token}" @@ -5824,9 +5825,13 @@ RSpec.describe UsersController do end describe "#register_second_factor_security_key" do + before do + simulate_localhost_webauthn_challenge + DiscourseWebauthn.stubs(:origin).returns("http://localhost:3000") + end + context "when creation parameters are valid" do it "creates a security key for the user" do - simulate_localhost_webauthn_challenge create_second_factor_security_key _response_parsed = response.parsed_body @@ -5841,7 +5846,6 @@ RSpec.describe UsersController do end it "doesn't allow creating too many security keys" do - simulate_localhost_webauthn_challenge create_second_factor_security_key _response_parsed = response.parsed_body @@ -5859,7 +5863,6 @@ RSpec.describe UsersController do end it "doesn't allow the security key name to exceed the limit" do - simulate_localhost_webauthn_challenge create_second_factor_security_key _response_parsed = response.parsed_body @@ -6076,7 +6079,10 @@ RSpec.describe UsersController do end describe "#register_passkey" do - before { SiteSetting.enable_passkeys = true } + before do + SiteSetting.enable_passkeys = true + DiscourseWebauthn.stubs(:origin).returns("http://localhost:3000") + end it "fails if user is not logged in" do stub_secure_session_confirmed @@ -6445,8 +6451,12 @@ RSpec.describe UsersController do ) end - it "returns a successful response for the correct user" do + before do + DiscourseWebauthn.stubs(:origin).returns("http://localhost:3000") simulate_localhost_passkey_challenge + end + + it "returns a successful response for the correct user" do user1.create_or_fetch_secure_identifier post "/u/confirm-session.json", @@ -6463,7 +6473,6 @@ RSpec.describe UsersController do 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",