<%= t("user_api_key.otp_confirmation.logging_in_as", username: @otp_username) %>
+ <%= submit_tag(t("user_api_key.otp_confirmation.confirm_button"), class: "btn btn-primary") %> + <%end%> <%end%> diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 6471cf66e0a..9b5b3feba8f 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1387,6 +1387,9 @@ en: complete_email_found: "We found an account that matches %{email}, you should receive an email with a login link shortly." complete_username_not_found: "No account matches the username %{username}" complete_email_not_found: "No account matches %{email}" + confirm_title: Continue to %{site_name} + logging_in_as: Logging in as %{email} + confirm_button: Finish Login login: title: "Log In" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 04de4f106b2..2140b1f407a 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -939,6 +939,10 @@ en: description: '"%{application_name}" is requesting the following access to your account:' instructions: 'We just generated a new user API key for you to use with "%{application_name}", please paste the following key into your application:' otp_description: 'Would you like to allow "%{application_name}" to access this site?' + otp_confirmation: + confirm_title: Continue to %{site_name} + logging_in_as: Logging in as %{username} + confirm_button: Finish Login no_trust_level: "Sorry, you do not have the required trust level to access the user API" generic_error: "Sorry, we are unable to issue user API keys, this feature may be disabled by the site admin" scopes: diff --git a/config/routes.rb b/config/routes.rb index c996544c2e0..d6132b9da29 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -337,9 +337,10 @@ Discourse::Application.routes.draw do get "session/sso_provider" => "session#sso_provider" get "session/current" => "session#current" get "session/csrf" => "session#csrf" - get "session/email-login/:token" => "session#email_login" + get "session/email-login/:token" => "session#email_login_info" post "session/email-login/:token" => "session#email_login" get "session/otp/:token" => "session#one_time_password", constraints: { token: /[0-9a-f]+/ } + post "session/otp/:token" => "session#one_time_password", constraints: { token: /[0-9a-f]+/ } get "composer_messages" => "composer_messages#index" post "composer/parse_html" => "composer#parse_html" diff --git a/spec/requests/session_controller_spec.rb b/spec/requests/session_controller_spec.rb index a7b71a39c4d..f3491ef5bdc 100644 --- a/spec/requests/session_controller_spec.rb +++ b/spec/requests/session_controller_spec.rb @@ -14,7 +14,7 @@ RSpec.describe SessionController do end end - describe '#email_login' do + describe '#email_login_info' do before do SiteSetting.enable_local_logins_via_email = true end @@ -26,13 +26,66 @@ RSpec.describe SessionController do end end + context 'valid token' do + it 'returns information' do + get "/session/email-login/#{email_token.token}.json" + + expect(JSON.parse(response.body)["can_login"]).to eq(true) + expect(JSON.parse(response.body)["second_factor_required"]).to eq(nil) + + # Does not log in the user + expect(session[:current_user_id]).to be_nil + end + + it 'fails when local logins via email is disabled' do + SiteSetting.enable_local_logins_via_email = false + + get "/session/email-login/#{email_token.token}.json" + + expect(response.status).to eq(404) + end + + it 'fails when local logins is disabled' do + SiteSetting.enable_local_logins = false + + get "/session/email-login/#{email_token.token}.json" + + expect(response.status).to eq(500) + end + + context 'user has 2-factor logins' do + let!(:user_second_factor) { Fabricate(:user_second_factor_totp, user: user) } + let!(:user_second_factor_backup) { Fabricate(:user_second_factor_backup, user: user) } + + it "includes that information in the response" do + get "/session/email-login/#{email_token.token}.json" + + expect(JSON.parse(response.body)["can_login"]).to eq(true) + expect(JSON.parse(response.body)["second_factor_required"]).to eq(true) + expect(JSON.parse(response.body)["backup_codes_enabled"]).to eq(true) + end + end + end + end + + describe '#email_login' do + before do + SiteSetting.enable_local_logins_via_email = true + end + + context 'missing token' do + it 'returns the right response' do + post "/session/email-login" + expect(response.status).to eq(404) + end + end + context 'invalid token' do it 'returns the right response' do - get "/session/email-login/adasdad" + post "/session/email-login/adasdad.json" expect(response.status).to eq(200) - - expect(CGI.unescapeHTML(response.body)).to match( + expect(JSON.parse(response.body)["error"]).to eq( I18n.t('email_login.invalid_token') ) end @@ -41,11 +94,11 @@ RSpec.describe SessionController do it 'should return the right response' do email_token.update!(created_at: 999.years.ago) - get "/session/email-login/#{email_token.token}" + post "/session/email-login/#{email_token.token}.json" expect(response.status).to eq(200) - expect(CGI.unescapeHTML(response.body)).to match( + expect(JSON.parse(response.body)["error"]).to eq( I18n.t('email_login.invalid_token') ) end @@ -54,37 +107,39 @@ RSpec.describe SessionController do context 'valid token' do it 'returns success' do - get "/session/email-login/#{email_token.token}" + post "/session/email-login/#{email_token.token}.json" - expect(response).to redirect_to("/") + expect(JSON.parse(response.body)["success"]).to eq("OK") + expect(session[:current_user_id]).to eq(user.id) end it 'fails when local logins via email is disabled' do SiteSetting.enable_local_logins_via_email = false - get "/session/email-login/#{email_token.token}" + post "/session/email-login/#{email_token.token}.json" expect(response.status).to eq(404) + expect(session[:current_user_id]).to eq(nil) end it 'fails when local logins is disabled' do SiteSetting.enable_local_logins = false - get "/session/email-login/#{email_token.token}" + post "/session/email-login/#{email_token.token}.json" expect(response.status).to eq(500) + expect(session[:current_user_id]).to eq(nil) end it "doesn't log in the user when not approved" do SiteSetting.must_approve_users = true - get "/session/email-login/#{email_token.token}" + post "/session/email-login/#{email_token.token}.json" expect(response.status).to eq(200) - expect(CGI.unescapeHTML(response.body)).to include( - I18n.t("login.not_approved") - ) + expect(JSON.parse(response.body)["error"]).to eq(I18n.t("login.not_approved")) + expect(session[:current_user_id]).to eq(nil) end context "when admin IP address is not valid" do @@ -99,13 +154,14 @@ RSpec.describe SessionController do end it 'returns the right response' do - get "/session/email-login/#{email_token.token}" + post "/session/email-login/#{email_token.token}.json" expect(response.status).to eq(200) - expect(CGI.unescapeHTML(response.body)).to include( + expect(JSON.parse(response.body)["error"]).to eq( I18n.t("login.admin_not_allowed_from_ip_address", username: user.username) ) + expect(session[:current_user_id]).to eq(nil) end end @@ -122,13 +178,14 @@ RSpec.describe SessionController do it 'returns the right response' do ActionDispatch::Request.any_instance.stubs(:remote_ip).returns(permitted_ip_address) - get "/session/email-login/#{email_token.token}" + post "/session/email-login/#{email_token.token}.json" expect(response.status).to eq(200) - expect(CGI.unescapeHTML(response.body)).to include( + expect(JSON.parse(response.body)["error"]).to eq( I18n.t("login.not_allowed_from_ip_address", username: user.username) ) + expect(session[:current_user_id]).to eq(nil) end end @@ -138,63 +195,48 @@ RSpec.describe SessionController do suspended_at: Time.zone.now ) - get "/session/email-login/#{email_token.token}" + post "/session/email-login/#{email_token.token}.json" expect(response.status).to eq(200) - expect(CGI.unescapeHTML(response.body)).to include(I18n.t("login.suspended", - date: I18n.l(user.suspended_till, format: :date_only) + expect(JSON.parse(response.body)["error"]).to eq( + I18n.t("login.suspended", date: I18n.l(user.suspended_till, format: :date_only) )) + expect(session[:current_user_id]).to eq(nil) end context 'user has 2-factor logins' do let!(:user_second_factor) { Fabricate(:user_second_factor_totp, user: user) } let!(:user_second_factor_backup) { Fabricate(:user_second_factor_backup, user: user) } - describe 'requires second factor' do - it 'should return a second factor prompt' do - get "/session/email-login/#{email_token.token}" - - expect(response.status).to eq(200) - - response_body = CGI.unescapeHTML(response.body) - - expect(response_body).to include(I18n.t( - "login.second_factor_title" - )) - - expect(response_body).to_not include(I18n.t( - "login.invalid_second_factor_code" - )) - end - end - describe 'errors on incorrect 2-factor' do context 'when using totp method' do it 'does not log in with incorrect two factor' do - post "/session/email-login/#{email_token.token}", params: { + post "/session/email-login/#{email_token.token}.json", params: { second_factor_token: "0000", second_factor_method: UserSecondFactor.methods[:totp] } expect(response.status).to eq(200) - expect(CGI.unescapeHTML(response.body)).to include(I18n.t( - "login.invalid_second_factor_code" - )) + expect(JSON.parse(response.body)["error"]).to eq( + I18n.t("login.invalid_second_factor_code") + ) + expect(session[:current_user_id]).to eq(nil) end end context 'when using backup code method' do it 'does not log in with incorrect backup code' do - post "/session/email-login/#{email_token.token}", params: { + post "/session/email-login/#{email_token.token}.json", params: { second_factor_token: "0000", second_factor_method: UserSecondFactor.methods[:backup_codes] } expect(response.status).to eq(200) - expect(CGI.unescapeHTML(response.body)).to include(I18n.t( - "login.invalid_second_factor_code" - )) + expect(JSON.parse(response.body)["error"]).to eq( + I18n.t("login.invalid_second_factor_code") + ) + expect(session[:current_user_id]).to eq(nil) end end end @@ -202,22 +244,24 @@ RSpec.describe SessionController do describe 'allows successful 2-factor' do context 'when using totp method' do it 'logs in correctly' do - post "/session/email-login/#{email_token.token}", params: { + post "/session/email-login/#{email_token.token}.json", params: { second_factor_token: ROTP::TOTP.new(user_second_factor.data).now, second_factor_method: UserSecondFactor.methods[:totp] } - expect(response).to redirect_to("/") + expect(JSON.parse(response.body)["success"]).to eq("OK") + expect(session[:current_user_id]).to eq(user.id) end end context 'when using backup code method' do it 'logs in correctly' do - post "/session/email-login/#{email_token.token}", params: { + post "/session/email-login/#{email_token.token}.json", params: { second_factor_token: "iAmValidBackupCode", second_factor_method: UserSecondFactor.methods[:backup_codes] } - expect(response).to redirect_to("/") + expect(JSON.parse(response.body)["success"]).to eq("OK") + expect(session[:current_user_id]).to eq(user.id) end end end @@ -1325,9 +1369,40 @@ RSpec.describe SessionController do get "/session/otp/asd1231dasd123" expect(response.status).to eq(404) + + post "/session/otp/asd1231dasd123" + + expect(response.status).to eq(404) end context 'when token is valid' do + it "should display the form for GET" do + token = SecureRandom.hex + $redis.setex "otp_#{token}", 10.minutes, user.username + + get "/session/otp/#{token}" + + expect(response.status).to eq(200) + expect(response.body).to include( + I18n.t("user_api_key.otp_confirmation.logging_in_as", username: user.username) + ) + expect($redis.get("otp_#{token}")).to eq(user.username) + + expect(session[:current_user_id]).to eq(nil) + end + + it "should redirect on GET if already logged in" do + sign_in(user) + token = SecureRandom.hex + $redis.setex "otp_#{token}", 10.minutes, user.username + + get "/session/otp/#{token}" + expect(response.status).to eq(302) + + expect($redis.get("otp_#{token}")).to eq(nil) + expect(session[:current_user_id]).to eq(user.id) + end + it 'should authenticate user and delete token' do user = Fabricate(:user) @@ -1337,7 +1412,7 @@ RSpec.describe SessionController do token = SecureRandom.hex $redis.setex "otp_#{token}", 10.minutes, user.username - get "/session/otp/#{token}" + post "/session/otp/#{token}" expect(response.status).to eq(302) expect(response).to redirect_to("/")