Skip to content
Permalink
Browse files Browse the repository at this point in the history
Merge pull request from GHSA-hv9p-jfm4-gpr9
* SECURITY: Add confirmation screen when logging in via email link

* SECURITY: Add confirmation screen when logging in via user-api OTP

* FIX: Correct translation key in session controller specs

* FIX: Use .email-login class for page
  • Loading branch information
davidtaylorhq committed Jun 17, 2019
1 parent 89e0d8c commit b8340c6
Show file tree
Hide file tree
Showing 13 changed files with 271 additions and 120 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
72 changes: 51 additions & 21 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,49 +305,79 @@ 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
otp_username = $redis.get "otp_#{params[:token]}"
@otp_username = otp_username = $redis.get "otp_#{params[:token]}"

if otp_username && user = User.find_by_username(otp_username)
log_on_user(user)
$redis.del "otp_#{params[:token]}"
return redirect_to path("/")
if current_user&.username == otp_username
$redis.del "otp_#{params[:token]}"
return redirect_to path("/")
elsif request.post?
log_on_user(user)
$redis.del "otp_#{params[:token]}"
return redirect_to path("/")
else
# Display the form
end
else
@error = I18n.t('user_api_key.invalid_token')
end
Expand Down
45 changes: 0 additions & 45 deletions app/views/session/email_login.html.erb

This file was deleted.

6 changes: 6 additions & 0 deletions app/views/session/one_time_password.html.erb
Expand Up @@ -2,4 +2,10 @@
<div class='alert alert-error'>
<%= @error %>
</div>
<%else%>
<%= form_tag do%>
<h2><%= t("user_api_key.otp_confirmation.confirm_title", site_name: SiteSetting.title) %></h2>
<p><%= t("user_api_key.otp_confirmation.logging_in_as", username: @otp_username) %></p>
<%= submit_tag(t("user_api_key.otp_confirmation.confirm_button"), class: "btn btn-primary") %>
<%end%>
<%end%>
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
4 changes: 4 additions & 0 deletions config/locales/server.en.yml
Expand Up @@ -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:
Expand Down
3 changes: 2 additions & 1 deletion config/routes.rb
Expand Up @@ -337,9 +337,10 @@
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"

Expand Down

1 comment on commit b8340c6

@discoursereviewbot
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.