discourse/spec/lib/second_factor/auth_manager_spec.rb

Ignoring revisions in .git-blame-ignore-revs. Click here to bypass and see the normal blame view.

225 lines
8.2 KiB
Ruby
Raw Normal View History

FEATURE: Centralized 2FA page (#15377) 2FA support in Discourse was added and grown gradually over the years: we first added support for TOTP for logins, then we implemented backup codes, and last but not least, security keys. 2FA usage was initially limited to logging in, but it has been expanded and we now require 2FA for risky actions such as adding a new admin to the site. As a result of this gradual growth of the 2FA system, technical debt has accumulated to the point where it has become difficult to require 2FA for more actions. We now have 5 different 2FA UI implementations and each one has to support all 3 2FA methods (TOTP, backup codes, and security keys) which makes it difficult to maintain a consistent UX for these different implementations. Moreover, there is a lot of repeated logic in the server-side code behind these 5 UI implementations which hinders maintainability even more. This commit is the first step towards repaying the technical debt: it builds a system that centralizes as much as possible of the 2FA server-side logic and UI. The 2 main components of this system are: 1. A dedicated page for 2FA with support for all 3 methods. 2. A reusable server-side class that centralizes the 2FA logic (the `SecondFactor::AuthManager` class). From a top-level view, the 2FA flow in this new system looks like this: 1. User initiates an action that requires 2FA; 2. Server is aware that 2FA is required for this action, so it redirects the user to the 2FA page if the user has a 2FA method, otherwise the action is performed. 3. User submits the 2FA form on the page; 4. Server validates the 2FA and if it's successful, the action is performed and the user is redirected to the previous page. A more technically-detailed explanation/documentation of the new system is available as a comment at the top of the `lib/second_factor/auth_manager.rb` file. Please note that the details are not set in stone and will likely change in the future, so please don't use the system in your plugins yet. Since this is a new system that needs to be tested, we've decided to migrate only the 2FA for adding a new admin to the new system at this time (in this commit). Our plan is to gradually migrate the remaining 2FA implementations to the new system. For screenshots of the 2FA page, see PR #15377 on GitHub.
2022-02-17 04:12:59 -05:00
# frozen_string_literal: true
describe SecondFactor::AuthManager do
fab!(:user) { Fabricate(:user) }
fab!(:guardian) { Guardian.new(user) }
fab!(:user_totp) { Fabricate(:user_second_factor_totp, user: user) }
def create_request(request_method: "GET", path: "/")
ActionDispatch::TestRequest.create({
"REQUEST_METHOD" => request_method,
"PATH_INFO" => path
})
end
def create_manager(action)
SecondFactor::AuthManager.new(guardian, action)
end
def create_action
TestSecondFactorAction.new(guardian)
end
def stage_challenge(successful:)
action = create_action
action.expects(:no_second_factors_enabled!).never
action
.expects(:second_factor_auth_required!)
.with({ random_param: 'hello' })
.returns({ callback_params: { call_me_back: 4314 } })
.once
manager = create_manager(action)
request = create_request(
request_method: "POST",
path: "/abc/xyz"
)
secure_session = {}
begin
manager.run!(request, { random_param: 'hello' }, secure_session)
rescue SecondFactor::AuthManager::SecondFactorRequired
# expected
end
challenge = JSON
.parse(secure_session["current_second_factor_auth_challenge"])
.deep_symbolize_keys
challenge[:successful] = successful
secure_session["current_second_factor_auth_challenge"] = challenge.to_json
[challenge[:nonce], secure_session]
end
describe '#allow_backup_codes!' do
it 'adds the backup codes method to the allowed methods set' do
manager = create_manager(create_action)
expect(manager.allowed_methods).not_to include(
UserSecondFactor.methods[:backup_codes]
)
manager.allow_backup_codes!
expect(manager.allowed_methods).to include(
UserSecondFactor.methods[:backup_codes]
)
end
end
describe '#run!' do
context 'when the user does not have a suitable 2FA method' do
before do
user_totp.destroy!
end
it 'calls the no_second_factors_enabled! method of the action' do
action = create_action
action.expects(:no_second_factors_enabled!).with({ hello_world: 331 }).once
action.expects(:second_factor_auth_required!).never
action.expects(:second_factor_auth_completed!).never
manager = create_manager(action)
manager.run!(create_request, { hello_world: 331 }, {})
end
it 'calls the no_second_factors_enabled! method of the action even if a nonce is present in the params' do
action = create_action
params = { second_factor_nonce: SecureRandom.hex }
action.expects(:no_second_factors_enabled!).with(params).once
action.expects(:second_factor_auth_required!).never
action.expects(:second_factor_auth_completed!).never
manager = create_manager(action)
manager.run!(create_request, params, {})
end
end
it "initiates the 2FA process and stages a challenge in secure session when there is no nonce in params" do
action = create_action
action.expects(:no_second_factors_enabled!).never
action
.expects(:second_factor_auth_required!)
.with({ expect_me: 131 })
.returns({ callback_params: { call_me_back: 4314 }, redirect_path: "/gg", description: "hello world!" })
FEATURE: Centralized 2FA page (#15377) 2FA support in Discourse was added and grown gradually over the years: we first added support for TOTP for logins, then we implemented backup codes, and last but not least, security keys. 2FA usage was initially limited to logging in, but it has been expanded and we now require 2FA for risky actions such as adding a new admin to the site. As a result of this gradual growth of the 2FA system, technical debt has accumulated to the point where it has become difficult to require 2FA for more actions. We now have 5 different 2FA UI implementations and each one has to support all 3 2FA methods (TOTP, backup codes, and security keys) which makes it difficult to maintain a consistent UX for these different implementations. Moreover, there is a lot of repeated logic in the server-side code behind these 5 UI implementations which hinders maintainability even more. This commit is the first step towards repaying the technical debt: it builds a system that centralizes as much as possible of the 2FA server-side logic and UI. The 2 main components of this system are: 1. A dedicated page for 2FA with support for all 3 methods. 2. A reusable server-side class that centralizes the 2FA logic (the `SecondFactor::AuthManager` class). From a top-level view, the 2FA flow in this new system looks like this: 1. User initiates an action that requires 2FA; 2. Server is aware that 2FA is required for this action, so it redirects the user to the 2FA page if the user has a 2FA method, otherwise the action is performed. 3. User submits the 2FA form on the page; 4. Server validates the 2FA and if it's successful, the action is performed and the user is redirected to the previous page. A more technically-detailed explanation/documentation of the new system is available as a comment at the top of the `lib/second_factor/auth_manager.rb` file. Please note that the details are not set in stone and will likely change in the future, so please don't use the system in your plugins yet. Since this is a new system that needs to be tested, we've decided to migrate only the 2FA for adding a new admin to the new system at this time (in this commit). Our plan is to gradually migrate the remaining 2FA implementations to the new system. For screenshots of the 2FA page, see PR #15377 on GitHub.
2022-02-17 04:12:59 -05:00
.once
action.expects(:second_factor_auth_completed!).never
manager = create_manager(action)
request = create_request(
request_method: "POST",
path: "/abc/xyz"
)
secure_session = {}
expect {
manager.run!(request, { expect_me: 131 }, secure_session)
}.to raise_error(SecondFactor::AuthManager::SecondFactorRequired)
json = secure_session["current_second_factor_auth_challenge"]
challenge = JSON.parse(json).deep_symbolize_keys
expect(challenge[:nonce]).to be_present
expect(challenge[:callback_method]).to eq("POST")
expect(challenge[:callback_path]).to eq("/abc/xyz")
expect(challenge[:redirect_path]).to eq("/gg")
expect(challenge[:allowed_methods]).to eq(manager.allowed_methods.to_a)
expect(challenge[:callback_params]).to eq({ call_me_back: 4314 })
expect(challenge[:description]).to eq("hello world!")
FEATURE: Centralized 2FA page (#15377) 2FA support in Discourse was added and grown gradually over the years: we first added support for TOTP for logins, then we implemented backup codes, and last but not least, security keys. 2FA usage was initially limited to logging in, but it has been expanded and we now require 2FA for risky actions such as adding a new admin to the site. As a result of this gradual growth of the 2FA system, technical debt has accumulated to the point where it has become difficult to require 2FA for more actions. We now have 5 different 2FA UI implementations and each one has to support all 3 2FA methods (TOTP, backup codes, and security keys) which makes it difficult to maintain a consistent UX for these different implementations. Moreover, there is a lot of repeated logic in the server-side code behind these 5 UI implementations which hinders maintainability even more. This commit is the first step towards repaying the technical debt: it builds a system that centralizes as much as possible of the 2FA server-side logic and UI. The 2 main components of this system are: 1. A dedicated page for 2FA with support for all 3 methods. 2. A reusable server-side class that centralizes the 2FA logic (the `SecondFactor::AuthManager` class). From a top-level view, the 2FA flow in this new system looks like this: 1. User initiates an action that requires 2FA; 2. Server is aware that 2FA is required for this action, so it redirects the user to the 2FA page if the user has a 2FA method, otherwise the action is performed. 3. User submits the 2FA form on the page; 4. Server validates the 2FA and if it's successful, the action is performed and the user is redirected to the previous page. A more technically-detailed explanation/documentation of the new system is available as a comment at the top of the `lib/second_factor/auth_manager.rb` file. Please note that the details are not set in stone and will likely change in the future, so please don't use the system in your plugins yet. Since this is a new system that needs to be tested, we've decided to migrate only the 2FA for adding a new admin to the new system at this time (in this commit). Our plan is to gradually migrate the remaining 2FA implementations to the new system. For screenshots of the 2FA page, see PR #15377 on GitHub.
2022-02-17 04:12:59 -05:00
end
it "sets the redirect_path to the root path if second_factor_auth_required! doesn't specify a redirect_path" do
action = create_action
action.expects(:no_second_factors_enabled!).never
action
.expects(:second_factor_auth_required!)
.with({ expect_me: 131 })
.returns({ callback_params: { call_me_back: 4314 } })
.once
action.expects(:second_factor_auth_completed!).never
manager = create_manager(action)
request = create_request(
request_method: "POST",
path: "/abc/xyz"
)
secure_session = {}
expect {
manager.run!(request, { expect_me: 131 }, secure_session)
}.to raise_error(SecondFactor::AuthManager::SecondFactorRequired)
json = secure_session["current_second_factor_auth_challenge"]
challenge = JSON.parse(json).deep_symbolize_keys
expect(challenge[:redirect_path]).to eq("/")
set_subfolder("/community")
action = create_action
action.expects(:no_second_factors_enabled!).never
action
.expects(:second_factor_auth_required!)
.with({ expect_me: 131 })
.returns({ callback_params: { call_me_back: 4314 } })
.once
action.expects(:second_factor_auth_completed!).never
manager = create_manager(action)
request = create_request(
request_method: "POST",
path: "/abc/xyz"
)
secure_session = {}
expect {
manager.run!(request, { expect_me: 131 }, secure_session)
}.to raise_error(SecondFactor::AuthManager::SecondFactorRequired)
json = secure_session["current_second_factor_auth_challenge"]
challenge = JSON.parse(json).deep_symbolize_keys
expect(challenge[:redirect_path]).to eq("/community")
end
it "calls the second_factor_auth_completed! method of the action if the challenge is successful and not expired" do
nonce, secure_session = stage_challenge(successful: true)
action = create_action
action.expects(:no_second_factors_enabled!).never
action.expects(:second_factor_auth_required!).never
action
.expects(:second_factor_auth_completed!)
.with({ call_me_back: 4314 })
.once
manager = create_manager(action)
request = create_request(
request_method: "POST",
path: "/abc/xyz"
)
manager.run!(request, { second_factor_nonce: nonce }, secure_session)
end
it "does not call the second_factor_auth_completed! method of the action if the challenge is not marked successful" do
nonce, secure_session = stage_challenge(successful: false)
action = create_action
action.expects(:no_second_factors_enabled!).never
action.expects(:second_factor_auth_required!).never
action.expects(:second_factor_auth_completed!).never
manager = create_manager(action)
request = create_request(
request_method: "POST",
path: "/abc/xyz"
)
expect {
manager.run!(request, { second_factor_nonce: nonce }, secure_session)
}.to raise_error(SecondFactor::BadChallenge) do |ex|
expect(ex.error_translation_key).to eq("second_factor_auth.challenge_not_completed")
end
end
it "does not call the second_factor_auth_completed! method of the action if the challenge is expired" do
nonce, secure_session = stage_challenge(successful: true)
action = create_action
action.expects(:no_second_factors_enabled!).never
action.expects(:second_factor_auth_required!).never
action.expects(:second_factor_auth_completed!).never
manager = create_manager(action)
request = create_request(
request_method: "POST",
path: "/abc/xyz"
)
freeze_time (SecondFactor::AuthManager::MAX_CHALLENGE_AGE + 1.minute).from_now
expect {
manager.run!(request, { second_factor_nonce: nonce }, secure_session)
}.to raise_error(SecondFactor::BadChallenge) do |ex|
expect(ex.error_translation_key).to eq("second_factor_auth.challenge_expired")
end
end
end
end