Skip to content
Permalink
Browse files Browse the repository at this point in the history
SECURITY: Add confirmation screen when logging in via email link
  • Loading branch information
davidtaylorhq committed Jun 17, 2019
1 parent 5f6f707 commit 52387be
Show file tree
Hide file tree
Showing 11 changed files with 217 additions and 115 deletions.
29 changes: 29 additions & 0 deletions app/assets/javascripts/discourse/controllers/email-login.js.es6
@@ -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);
}
}
});
Expand Up @@ -177,6 +177,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" });
Expand Down
11 changes: 11 additions & 0 deletions app/assets/javascripts/discourse/routes/email-login.js.es6
@@ -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 changes: 33 additions & 0 deletions app/assets/javascripts/discourse/templates/email-login.hbs
@@ -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 changes: 3 additions & 1 deletion app/assets/stylesheets/desktop/login.scss
Expand Up @@ -267,6 +267,7 @@
}

.password-reset,
.email-login,
.invites-show {
.col-form {
padding-left: 20px;
Expand All @@ -282,7 +283,8 @@
}
}

.password-reset {
.password-reset,
.email-login {
.col-form {
padding-top: 40px;
}
Expand Down
1 change: 1 addition & 0 deletions app/assets/stylesheets/mobile/login.scss
Expand Up @@ -182,6 +182,7 @@
}

.password-reset,
.email-login,
.invites-show {
margin-top: 30px;
.col-image {
Expand Down
57 changes: 40 additions & 17 deletions app/controllers/session_controller.rb
Expand Up @@ -11,10 +11,10 @@ class LocalLoginNotAllowed < StandardError; end
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 one_time_password)
skip_before_action :preload_json, :check_xhr, only: %i(sso sso_login sso_provider destroy one_time_password)

ACTIVATE_USER_KEY = "activate_user"

Expand Down Expand Up @@ -305,40 +305,63 @@ def create
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 one_time_password
Expand Down
45 changes: 0 additions & 45 deletions app/views/session/email_login.html.erb

This file was deleted.

3 changes: 3 additions & 0 deletions config/locales/client.en.yml
Expand Up @@ -1387,6 +1387,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"
Expand Down
2 changes: 1 addition & 1 deletion config/routes.rb
Expand Up @@ -337,7 +337,7 @@
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]+/ }
get "composer_messages" => "composer_messages#index"
Expand Down

0 comments on commit 52387be

Please sign in to comment.