github-mirror/discourse
2
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2024-12-13 19:53:51 +08:00
Code Issues Actions 1 Packages Projects Releases Wiki Activity

SECURITY: Add confirmation screen when logging in via email link

Browse Source
This commit is contained in:
David Taylor 2019-06-12 15:37:26 +01:00
parent 809a544786
commit 40cbcc7720
11 changed files with 217 additions and 115 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
app/assets/javascripts/discourse/controllers/email-login.js.es6 Normal file
View File

@ -0,0 +1,29 @@
import { SECOND_FACTOR_METHODS } from "discourse/models/user";
import { ajax } from "discourse/lib/ajax";
import DiscourseURL from "discourse/lib/url";
import { popupAjaxError } from "discourse/lib/ajax-error";
export default Ember.Controller.extend({
secondFactorMethod: SECOND_FACTOR_METHODS.TOTP,
lockImageUrl: Discourse.getURL("/images/lock.svg"),
actions: {
finishLogin() {
ajax({
url: `/session/email-login/${this.model.token}`,
type: "POST",
data: {
second_factor_token: this.secondFactorToken,
second_factor_method: this.secondFactorMethod
}
})
.then(result => {
if (result.success) {
DiscourseURL.redirectTo("/");
} else {
this.set("model.error", result.error);
}
})
.catch(popupAjaxError);
}
}
});

1
app/assets/javascripts/discourse/routes/app-route-map.js.es6
View File

@ -174,6 +174,7 @@ export default function() {
this.route("signup", { path: "/signup" });
this.route("login", { path: "/login" });
this.route("email-login", { path: "/session/email-login/:token" });
this.route("login-preferences");
this.route("forgot-password", { path: "/password-reset" });
this.route("faq", { path: "/faq" });

11
app/assets/javascripts/discourse/routes/email-login.js.es6 Normal file
View File

@ -0,0 +1,11 @@
import { ajax } from "discourse/lib/ajax";
export default Discourse.Route.extend({
titleToken() {
return I18n.t("login.title");
},
model(params) {
return ajax(`/session/email-login/${params.token}`);
}
});

33
app/assets/javascripts/discourse/templates/email-login.hbs Normal file
View File

@ -0,0 +1,33 @@
<div class="container email-login clearfix">
<div class="pull-left col-image">
<img src={{lockImageUrl}} class="password-reset-img">
</div>
<div class="pull-left col-form">
<form>
{{#if model.error}}
<div class='alert alert-error'>
{{model.error}}
</div>
{{/if}}
{{#if model.can_login}}
{{#if model.second_factor_required}}
{{#second-factor-form
secondFactorMethod=secondFactorMethod
secondFactorToken=secondFactorToken
backupEnabled=model.backup_codes_enabled
isLogin=true}}
{{second-factor-input value=secondFactorToken secondFactorMethod=secondFactorMethod backupEnabled=backupEnabled}}
{{/second-factor-form}}
{{else}}
<h2>{{i18n "email_login.confirm_title" site_name=siteSettings.title}}</h2>
<p>{{i18n "email_login.logging_in_as" email=model.token_email}}</p>
{{/if}}
{{d-button label="email_login.confirm_button" action=(action "finishLogin") class="btn-primary"}}
{{/if}}
</form>
</div>
</div>

4
app/assets/stylesheets/desktop/login.scss
View File

@ -227,6 +227,7 @@
}
.password-reset,
.email-login,
.invites-show {
.col-form {
padding-left: 20px;
@ -242,7 +243,8 @@
}
}
.password-reset {
.password-reset,
.email-login {
.col-form {
padding-top: 40px;
}

1
app/assets/stylesheets/mobile/login.scss
View File

@ -175,6 +175,7 @@
}
.password-reset,
.email-login,
.invites-show {
margin-top: 30px;
.col-image {

57
app/controllers/session_controller.rb
View File

@ -9,10 +9,10 @@ class SessionController < ApplicationController
render body: nil, status: 500
end
before_action :check_local_login_allowed, only: %i(create forgot_password email_login)
before_action :check_local_login_allowed, only: %i(create forgot_password email_login email_login_info)
before_action :rate_limit_login, only: %i(create email_login)
skip_before_action :redirect_to_login_if_required
skip_before_action :preload_json, :check_xhr, only: %i(sso sso_login sso_provider destroy email_login)
skip_before_action :preload_json, :check_xhr, only: %i(sso sso_login sso_provider destroy)
ACTIVATE_USER_KEY = "activate_user"
@ -285,40 +285,63 @@ class SessionController < ApplicationController
end
end
def email_login_info
raise Discourse::NotFound if !SiteSetting.enable_local_logins_via_email
token = params[:token]
matched_token = EmailToken.confirmable(token)
if matched_token
response = {
can_login: true,
token: token,
token_email: matched_token.email
}
if matched_token.user&.totp_enabled?
response.merge!(
second_factor_required: true,
backup_codes_enabled: matched_token.user&.backup_codes_enabled?
)
end
render json: response
else
render json: {
can_login: false,
error: I18n.t('email_login.invalid_token')
}
end
end
def email_login
raise Discourse::NotFound if !SiteSetting.enable_local_logins_via_email
second_factor_token = params[:second_factor_token]
second_factor_method = params[:second_factor_method].to_i
token = params[:token]
valid_token = !!EmailToken.valid_token_format?(token)
user = EmailToken.confirmable(token)&.user
matched_token = EmailToken.confirmable(token)
if valid_token && user&.totp_enabled?
if matched_token&.user&.totp_enabled?
if !second_factor_token.present?
@second_factor_required = true
@backup_codes_enabled = true if user&.backup_codes_enabled?
return render layout: 'no_ember'
elsif !user.authenticate_second_factor(second_factor_token, second_factor_method)
return render json: { error: I18n.t('login.invalid_second_factor_code') }
elsif !matched_token.user.authenticate_second_factor(second_factor_token, second_factor_method)
RateLimiter.new(nil, "second-factor-min-#{request.remote_ip}", 3, 1.minute).performed!
@error = I18n.t('login.invalid_second_factor_code')
return render layout: 'no_ember'
return render json: { error: I18n.t('login.invalid_second_factor_code') }
end
end
if user = EmailToken.confirm(token)
if login_not_approved_for?(user)
@error = login_not_approved[:error]
return render json: login_not_approved
elsif payload = login_error_check(user)
@error = payload[:error]
return render json: payload
else
log_on_user(user)
return redirect_to path("/")
return render json: success_json
end
else
@error = I18n.t('email_login.invalid_token')
end
render layout: 'no_ember'
return render json: { error: I18n.t('email_login.invalid_token') }
end
def forgot_password

45
app/views/session/email_login.html.erb
View File

@ -1,45 +0,0 @@
<%if @error%>
<div class='alert alert-error'>
<%= @error %>
</div>
<%end%>
<%if @second_factor_required%>
<div id="simple-container">
<div id="primary-second-factor-form">
<%= form_tag(method: "post") do%>
<h2><%=t "login.second_factor_title" %></h2>
<%= label_tag(:second_factor_token, t("login.second_factor_description")) %>
<div><%= render 'common/second_factor_text_field' %></div>
<%= submit_tag(t("submit"), class: "btn btn-large btn-primary") %>
<%end%>
</div>
<%if @backup_codes_enabled%>
<div id="backup-second-factor-form" style="display: none">
<%= form_tag(method: "post") do%>
<h2><%=t "login.second_factor_backup_title" %></h2>
<%= label_tag(:second_factor_token, t("login.second_factor_backup_description")) %>
<div><%= render 'common/second_factor_backup_input' %></div>
<%= submit_tag(t("submit"), class: "btn btn-large btn-primary") %>
<%end%>
</div>
<a href id="toggle-form"><%=t "login.second_factor_toggle.backup_code" %></a>
<%= render 'common/second_factor_form_script' %>
<%end%>
</div>
<%end%>
<% content_for :title do %><%=t "email_login.title" %><% end %>
<%- content_for(:no_ember_head) do %>
<meta name="referrer" content="no-referrer">
<%= preload_script "ember_jquery" %>
<%= render_google_universal_analytics_code %>
<%- end %>
<%- content_for(:head) do %>
<meta name="referrer" content="no-referrer">
<%- end %>

3
config/locales/client.en.yml
View File

@ -1220,6 +1220,9 @@ en:
complete_email_found: "We found an account that matches <b>%{email}</b>, you should receive an email with a login link shortly."
complete_username_not_found: "No account matches the username <b>%{username}</b>"
complete_email_not_found: "No account matches <b>%{email}</b>"
confirm_title: Continue to %{site_name}
logging_in_as: Logging in as %{email}
confirm_button: Finish Login
login:
title: "Log In"

2
config/routes.rb
View File

@ -321,7 +321,7 @@ 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 "composer_messages" => "composer_messages#index"
post "composer/parse_html" => "composer#parse_html"

146
spec/requests/session_controller_spec.rb
View File

@ -12,7 +12,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
@ -24,13 +24,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
@ -39,11 +92,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
@ -52,37 +105,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
@ -97,13 +152,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
@ -120,13 +176,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
@ -136,63 +193,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
@ -200,22 +242,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