CVE-2019-1020018: Improper Authentication (CWE-287)
value: Discourse before 2.3.0 and 2.4.x before 2.4.0.beta3 lacks a confirmation screen when logging in via an email link.
CWE description: When an actor claims to have a given identity, the product does not prove or insufficiently proves that the claim is correct.
CVSS v3 base score: 7.3
Published: 2019-07-29T14:15Z
Repository: https://github.com/discourse/discourse
Language: Other
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 date: 2019-06-17 15:59:41 +0100
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

Diff: diff --git a/app/assets/javascripts/discourse/controllers/email-login.js.es6 b/app/assets/javascripts/discourse/controllers/email-login.js.es6
new file mode 100644
index 0000000000..5a025ce7a6
--- /dev/null
+++ b/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);
+    }
+  }
+});
diff --git a/app/assets/javascripts/discourse/routes/app-route-map.js.es6 b/app/assets/javascripts/discourse/routes/app-route-map.js.es6
index fe84822b07..bfec70df96 100644
--- a/app/assets/javascripts/discourse/routes/app-route-map.js.es6
+++ b/app/assets/javascripts/discourse/routes/app-route-map.js.es6
@@ -175,10 +175,11 @@ export default function() {
     this.route("topics", { path: "/topics" });
     this.route("settings", { path: "/settings" });
   });
   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" });
   this.route("tos", { path: "/tos" });
   this.route("privacy", { path: "/privacy" });
diff --git a/app/assets/javascripts/discourse/routes/email-login.js.es6 b/app/assets/javascripts/discourse/routes/email-login.js.es6
new file mode 100644
index 0000000000..617de051cd
--- /dev/null
+++ b/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}`);
+  }
+});
diff --git a/app/assets/javascripts/discourse/templates/email-login.hbs b/app/assets/javascripts/discourse/templates/email-login.hbs
new file mode 100644
index 0000000000..1556a212a2
--- /dev/null
+++ b/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>
+
diff --git a/app/assets/stylesheets/desktop/login.scss b/app/assets/stylesheets/desktop/login.scss
index 86a46bf4c2..122d95e500 100644
--- a/app/assets/stylesheets/desktop/login.scss
+++ b/app/assets/stylesheets/desktop/login.scss
@@ -265,10 +265,11 @@
     }
   }
 }
 
 .password-reset,
+.email-login,
 .invites-show {
   .col-form {
     padding-left: 20px;
   }
   .col-image {
@@ -280,10 +281,11 @@
   h2 {
     margin-bottom: 12px;
   }
 }
 
-.password-reset {
+.password-reset,
+.email-login {
   .col-form {
     padding-top: 40px;
   }
 }
diff --git a/app/assets/stylesheets/mobile/login.scss b/app/assets/stylesheets/mobile/login.scss
index 4334699b71..ef6b808065 100644
--- a/app/assets/stylesheets/mobile/login.scss
+++ b/app/assets/stylesheets/mobile/login.scss
@@ -180,10 +180,11 @@
     }
   }
 }
 
 .password-reset,
+.email-login,
 .invites-show {
   margin-top: 30px;
   .col-image {
     padding-top: 12px;
   }
diff --git a/app/controllers/session_controller.rb b/app/controllers/session_controller.rb
index 3ea48ad1d8..b48ee8a8f1 100644
--- a/app/controllers/session_controller.rb
+++ b/app/controllers/session_controller.rb
@@ -9,14 +9,14 @@ class SessionController < ApplicationController
   class LocalLoginNotAllowed < StandardError; end
   rescue_from LocalLoginNotAllowed do
     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"
 
   def csrf
     render json: { csrf: form_authenticity_token }
@@ -303,53 +303,83 @@ class SessionController < ApplicationController
 
       (user.active && user.email_confirmed?) ? login(user) : not_activated(user)
     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
 
     render layout: 'no_ember'
diff --git a/app/views/session/email_login.html.erb b/app/views/session/email_login.html.erb
deleted file mode 100644
index 1929995e01..0000000000
--- a/app/views/session/email_login.html.erb
+++ /dev/null
@@ -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 %>
diff --git a/app/views/session/one_time_password.html.erb b/app/views/session/one_time_password.html.erb
index a0faef79d9..e86e371298 100644
--- a/app/views/session/one_time_password.html.erb
+++ b/app/views/session/one_time_password.html.erb
@@ -1,5 +1,11 @@
 <%if @error%>
   <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%>
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index 6471cf66e0..9b5b3feba8 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -1385,10 +1385,13 @@ en:
       complete_email: "If an account matches <b>%{email}</b>, you should receive an email with a login link shortly."
       complete_username_found: "We found an account that matches the username <b>%{username}</b>, you should receive an email with a login link shortly."
       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"
       username: "User"
       password: "Password"
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index 04de4f106b..2140b1f407 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -937,10 +937,14 @@ en:
     read: "read"
     read_write: "read/write"
     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:
       message_bus: "Live updates"
       notifications: "Read and clear notifications"
diff --git a/config/routes.rb b/config/routes.rb
index c996544c2e..d6132b9da2 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -335,13 +335,14 @@ Discourse::Application.routes.draw do
   get "session/sso" => "session#sso"
   get "session/sso_login" => "session#sso_login"
   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"
 
   resources :static
   post "login" => "static#enter", constraints: { format: /(json|html)/ }
diff --git a/spec/requests/session_controller_spec.rb b/spec/requests/session_controller_spec.rb
index a7b71a39c4..f3491ef5bd 100644
--- a/spec/requests/session_controller_spec.rb
+++ b/spec/requests/session_controller_spec.rb
@@ -12,11 +12,11 @@ RSpec.describe SessionController do
       expect(response).not_to be_successful
       expect(response.status).to eq(500)
     end
   end
 
-  describe '#email_login' do
+  describe '#email_login_info' do
     before do
       SiteSetting.enable_local_logins_via_email = true
     end
 
     context 'missing token' do
@@ -24,69 +24,124 @@ RSpec.describe SessionController do
         get "/session/email-login"
         expect(response.status).to eq(404)
       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
 
       context 'when token has expired' 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
       end
     end
 
     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
         before do
           Fabricate(:screened_ip_address,
@@ -97,17 +152,18 @@ RSpec.describe SessionController do
           SiteSetting.use_admin_ip_whitelist = true
           user.update!(admin: true)
         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
 
       context "when IP address is blocked" do
         let(:permitted_ip_address) { '111.234.23.11' }
@@ -120,106 +176,94 @@ RSpec.describe SessionController do
         end
 
         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
 
       it "fails when user is suspended" do
         user.update!(
           suspended_till: 2.days.from_now,
           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
 
         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
       end
     end
@@ -1323,23 +1367,54 @@ RSpec.describe SessionController do
     context 'invalid token' do
       it 'returns the right response' 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)
 
           get "/session/current.json"
           expect(response.status).to eq(404)
 
           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("/")
           expect($redis.get("otp_#{token}")).to eq(nil)
 

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}"