CVE-2019-1020017 (Other, published 2019-07-29T13:15Z)
Description: value: Discourse before 2.3.0 and 2.4.x before 2.4.0.beta3 lacks a confirmation screen when logging in via a user-api OTP.
Severity: MEDIUM (CVSS v3 base score: 5.3)
Weakness: NVD-CWE-noinfo \u2014 Insufficient Information
There is insufficient information about the issue to classify it; details are unkown or unspecified.
Repository: https://github.com/discourse/discourse
Files changed:
app/assets/javascripts/discourse/controllers/email-login.js.es6
app/assets/javascripts/discourse/routes/app-route-map.js.es6
app/assets/javascripts/discourse/routes/email-login.js.es6
app/assets/javascripts/discourse/templates/email-login.hbs
app/assets/stylesheets/desktop/login.scss
app/assets/stylesheets/mobile/login.scss
app/controllers/session_controller.rb
app/views/session/one_time_password.html.erb
config/locales/client.en.yml
config/locales/server.en.yml
config/routes.rb
spec/requests/session_controller_spec.rb
Commit message: 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


Vulnerable code:
.password-reset {
  before_action :check_local_login_allowed, only: %i(create forgot_password email_login)
  skip_before_action :preload_json, :check_xhr, only: %i(sso sso_login sso_provider destroy email_login one_time_password)
    valid_token = !!EmailToken.valid_token_format?(token)
    user = EmailToken.confirmable(token)&.user
    if valid_token && user&.totp_enabled?
        @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)
        @error = I18n.t('login.invalid_second_factor_code')
        return render layout: 'no_ember'
        @error = login_not_approved[:error]
        @error = payload[:error]
        return redirect_to path("/")
    else
      @error = I18n.t('email_login.invalid_token')
    render layout: 'no_ember'
    otp_username = $redis.get "otp_#{params[:token]}"
      log_on_user(user)
      $redis.del "otp_#{params[:token]}"
      return redirect_to path("/")
<%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 %>
  get "session/email-login/:token" => "session#email_login"
  describe '#email_login' do
        get "/session/email-login/adasdad"

        expect(CGI.unescapeHTML(response.body)).to match(
          get "/session/email-login/#{email_token.token}"
          expect(CGI.unescapeHTML(response.body)).to match(
        get "/session/email-login/#{email_token.token}"
        expect(response).to redirect_to("/")
        get "/session/email-login/#{email_token.token}"
        get "/session/email-login/#{email_token.token}"
        get "/session/email-login/#{email_token.token}"
        expect(CGI.unescapeHTML(response.body)).to include(
          I18n.t("login.not_approved")
        )
          get "/session/email-login/#{email_token.token}"
          expect(CGI.unescapeHTML(response.body)).to include(
          get "/session/email-login/#{email_token.token}"
          expect(CGI.unescapeHTML(response.body)).to include(
        get "/session/email-login/#{email_token.token}"
        expect(CGI.unescapeHTML(response.body)).to include(I18n.t("login.suspended",
          date: I18n.l(user.suspended_till, format: :date_only)
        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

              post "/session/email-login/#{email_token.token}", params: {
              expect(CGI.unescapeHTML(response.body)).to include(I18n.t(
                "login.invalid_second_factor_code"
              ))
              post "/session/email-login/#{email_token.token}", params: {
              expect(CGI.unescapeHTML(response.body)).to include(I18n.t(
                "login.invalid_second_factor_code"
              ))
              post "/session/email-login/#{email_token.token}", params: {
              expect(response).to redirect_to("/")
              post "/session/email-login/#{email_token.token}", params: {
              expect(response).to redirect_to("/")
          get "/session/otp/#{token}"

Fixed code:
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);
    }
  }
});
  this.route("email-login", { path: "/session/email-login/:token" });
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}`);
  }
});
<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>

.email-login,
.password-reset,
.email-login {
.email-login,
  before_action :check_local_login_allowed, only: %i(create forgot_password email_login email_login_info)
  skip_before_action :preload_json, :check_xhr, only: %i(sso sso_login sso_provider destroy one_time_password)
  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

    matched_token = EmailToken.confirmable(token)
    if matched_token&.user&.totp_enabled?
        return render json: { error: I18n.t('login.invalid_second_factor_code') }
      elsif !matched_token.user.authenticate_second_factor(second_factor_token, second_factor_method)
        return render json: { error: I18n.t('login.invalid_second_factor_code') }
        return render json: login_not_approved
        return render json: payload
        return render json: success_json
    return render json: { error: I18n.t('email_login.invalid_token') }
    @otp_username = otp_username = $redis.get "otp_#{params[:token]}"
      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%>
  <%= 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%>
      confirm_title: Continue to %{site_name}
      logging_in_as: Logging in as %{email}
      confirm_button: Finish Login
    otp_confirmation: 
      confirm_title: Continue to %{site_name}
      logging_in_as: Logging in as %{username}
      confirm_button: Finish Login
  get "session/email-login/:token" => "session#email_login_info"
  post "session/otp/:token" => "session#one_time_password", constraints: { token: /[0-9a-f]+/ }
  describe '#email_login_info' do
    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

        post "/session/email-login/adasdad.json"
        expect(JSON.parse(response.body)["error"]).to eq(
          post "/session/email-login/#{email_token.token}.json"
          expect(JSON.parse(response.body)["error"]).to eq(
        post "/session/email-login/#{email_token.token}.json"
        expect(JSON.parse(response.body)["success"]).to eq("OK")
        expect(session[:current_user_id]).to eq(user.id)
        post "/session/email-login/#{email_token.token}.json"
        expect(session[:current_user_id]).to eq(nil)
        post "/session/email-login/#{email_token.token}.json"
        expect(session[:current_user_id]).to eq(nil)
        post "/session/email-login/#{email_token.token}.json"
        expect(JSON.parse(response.body)["error"]).to eq(I18n.t("login.not_approved"))
        expect(session[:current_user_id]).to eq(nil)
          post "/session/email-login/#{email_token.token}.json"
          expect(JSON.parse(response.body)["error"]).to eq(
          expect(session[:current_user_id]).to eq(nil)
          post "/session/email-login/#{email_token.token}.json"
          expect(JSON.parse(response.body)["error"]).to eq(
          expect(session[:current_user_id]).to eq(nil)
        post "/session/email-login/#{email_token.token}.json"
        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)
              post "/session/email-login/#{email_token.token}.json", params: {
              expect(JSON.parse(response.body)["error"]).to eq(
                I18n.t("login.invalid_second_factor_code")
              )
              expect(session[:current_user_id]).to eq(nil)
              post "/session/email-login/#{email_token.token}.json", params: {
              expect(JSON.parse(response.body)["error"]).to eq(
                I18n.t("login.invalid_second_factor_code")
              )
              expect(session[:current_user_id]).to eq(nil)
              post "/session/email-login/#{email_token.token}.json", params: {
              expect(JSON.parse(response.body)["success"]).to eq("OK")
              expect(session[:current_user_id]).to eq(user.id)
              post "/session/email-login/#{email_token.token}.json", params: {
              expect(JSON.parse(response.body)["success"]).to eq("OK")
              expect(session[:current_user_id]).to eq(user.id)

        post "/session/otp/asd1231dasd123"

        expect(response.status).to eq(404)
        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

          post "/session/otp/#{token}"