Revert "Merge pull request from GHSA-hv9p-jfm4-gpr9"

This reverts commit b8340c6c8e.

app/assets/javascripts/discourse/controllers/email-login.js.es6

@@ -1,29 +0,0 @@
-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);
-    }
-  }
-});

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

@@ -177,7 +177,6 @@ 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" });

app/assets/javascripts/discourse/routes/email-login.js.es6

@@ -1,11 +0,0 @@
-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}`);
-  }
-});

app/assets/javascripts/discourse/templates/email-login.hbs

@@ -1,33 +0,0 @@
-<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>
-

app/assets/stylesheets/desktop/login.scss

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

app/assets/stylesheets/mobile/login.scss

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

app/controllers/session_controller.rb

@@ -11,10 +11,10 @@ class SessionController < ApplicationController
     render body: nil, status: 500
   end
 
-  before_action :check_local_login_allowed, only: %i(create forgot_password email_login email_login_info)
+  before_action :check_local_login_allowed, only: %i(create forgot_password email_login)
   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 one_time_password)
+  skip_before_action :preload_json, :check_xhr, only: %i(sso sso_login sso_provider destroy email_login one_time_password)
 
   ACTIVATE_USER_KEY = "activate_user"
 
@@ -305,79 +305,49 @@ 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]
-    matched_token = EmailToken.confirmable(token)
+    valid_token = !!EmailToken.valid_token_format?(token)
+    user = EmailToken.confirmable(token)&.user
 
-    if matched_token&.user&.totp_enabled?
+    if valid_token && user&.totp_enabled?
       if !second_factor_token.present?
-        return render json: { error: I18n.t('login.invalid_second_factor_code') }
+        @second_factor_required = true
-      elsif !matched_token.user.authenticate_second_factor(second_factor_token, second_factor_method)
+        @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)
         RateLimiter.new(nil, "second-factor-min-#{request.remote_ip}", 3, 1.minute).performed!
-        return render json: { error: I18n.t('login.invalid_second_factor_code') }
+        @error = I18n.t('login.invalid_second_factor_code')
+        return render layout: 'no_ember'
       end
     end
 
     if user = EmailToken.confirm(token)
       if login_not_approved_for?(user)
-        return render json: login_not_approved
+        @error = login_not_approved[:error]
       elsif payload = login_error_check(user)
-        return render json: payload
+        @error = payload[:error]
       else
         log_on_user(user)
-        return render json: success_json
+        return redirect_to path("/")
       end
+    else
+      @error = I18n.t('email_login.invalid_token')
     end
 
-    return render json: { error: I18n.t('email_login.invalid_token') }
+    render layout: 'no_ember'
   end
 
   def one_time_password
-    @otp_username = otp_username = $redis.get "otp_#{params[:token]}"
+    otp_username = $redis.get "otp_#{params[:token]}"
 
     if otp_username && user = User.find_by_username(otp_username)
-      if current_user&.username == otp_username
+      log_on_user(user)
-        $redis.del "otp_#{params[:token]}"
+      $redis.del "otp_#{params[:token]}"
-        return redirect_to path("/")
+      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

app/views/session/email_login.html.erb

@@ -0,0 +1,45 @@
+<%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 %>

app/views/session/one_time_password.html.erb

@@ -2,10 +2,4 @@
   <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%>

config/locales/client.en.yml

@@ -1387,9 +1387,6 @@ 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"

config/locales/server.en.yml

@@ -939,10 +939,6 @@ 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:

config/routes.rb

@@ -337,10 +337,9 @@ 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_info"
+  get "session/email-login/:token" => "session#email_login"
   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"
 

spec/requests/session_controller_spec.rb

@@ -14,7 +14,7 @@ RSpec.describe SessionController do
     end
   end
 
-  describe '#email_login_info' do
+  describe '#email_login' do
     before do
       SiteSetting.enable_local_logins_via_email = true
     end
@@ -26,66 +26,13 @@ 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
-        post "/session/email-login/adasdad.json"
+        get "/session/email-login/adasdad"
 
         expect(response.status).to eq(200)
-        expect(JSON.parse(response.body)["error"]).to eq(
+
+        expect(CGI.unescapeHTML(response.body)).to match(
           I18n.t('email_login.invalid_token')
         )
       end
@@ -94,11 +41,11 @@ RSpec.describe SessionController do
         it 'should return the right response' do
           email_token.update!(created_at: 999.years.ago)
 
-          post "/session/email-login/#{email_token.token}.json"
+          get "/session/email-login/#{email_token.token}"
 
           expect(response.status).to eq(200)
 
-          expect(JSON.parse(response.body)["error"]).to eq(
+          expect(CGI.unescapeHTML(response.body)).to match(
             I18n.t('email_login.invalid_token')
           )
         end
@@ -107,39 +54,37 @@ RSpec.describe SessionController do
 
     context 'valid token' do
       it 'returns success' do
-        post "/session/email-login/#{email_token.token}.json"
+        get "/session/email-login/#{email_token.token}"
 
-        expect(JSON.parse(response.body)["success"]).to eq("OK")
+        expect(response).to redirect_to("/")
-        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
 
-        post "/session/email-login/#{email_token.token}.json"
+        get "/session/email-login/#{email_token.token}"
 
         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
 
-        post "/session/email-login/#{email_token.token}.json"
+        get "/session/email-login/#{email_token.token}"
 
         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
 
-        post "/session/email-login/#{email_token.token}.json"
+        get "/session/email-login/#{email_token.token}"
 
         expect(response.status).to eq(200)
 
-        expect(JSON.parse(response.body)["error"]).to eq(I18n.t("login.not_approved"))
+        expect(CGI.unescapeHTML(response.body)).to include(
-        expect(session[:current_user_id]).to eq(nil)
+          I18n.t("login.not_approved")
+        )
       end
 
       context "when admin IP address is not valid" do
@@ -154,14 +99,13 @@ RSpec.describe SessionController do
         end
 
         it 'returns the right response' do
-          post "/session/email-login/#{email_token.token}.json"
+          get "/session/email-login/#{email_token.token}"
 
           expect(response.status).to eq(200)
 
-          expect(JSON.parse(response.body)["error"]).to eq(
+          expect(CGI.unescapeHTML(response.body)).to include(
             I18n.t("login.admin_not_allowed_from_ip_address", username: user.username)
           )
-          expect(session[:current_user_id]).to eq(nil)
         end
       end
 
@@ -178,14 +122,13 @@ RSpec.describe SessionController do
         it 'returns the right response' do
           ActionDispatch::Request.any_instance.stubs(:remote_ip).returns(permitted_ip_address)
 
-          post "/session/email-login/#{email_token.token}.json"
+          get "/session/email-login/#{email_token.token}"
 
           expect(response.status).to eq(200)
 
-          expect(JSON.parse(response.body)["error"]).to eq(
+          expect(CGI.unescapeHTML(response.body)).to include(
             I18n.t("login.not_allowed_from_ip_address", username: user.username)
           )
-          expect(session[:current_user_id]).to eq(nil)
         end
       end
 
@@ -195,48 +138,63 @@ RSpec.describe SessionController do
           suspended_at: Time.zone.now
         )
 
-        post "/session/email-login/#{email_token.token}.json"
+        get "/session/email-login/#{email_token.token}"
 
         expect(response.status).to eq(200)
 
-        expect(JSON.parse(response.body)["error"]).to eq(
+        expect(CGI.unescapeHTML(response.body)).to include(I18n.t("login.suspended",
-          I18n.t("login.suspended", date: I18n.l(user.suspended_till, format: :date_only)
+          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}.json", params: {
+              post "/session/email-login/#{email_token.token}", params: {
                 second_factor_token: "0000",
                 second_factor_method: UserSecondFactor.methods[:totp]
               }
 
               expect(response.status).to eq(200)
 
-              expect(JSON.parse(response.body)["error"]).to eq(
+              expect(CGI.unescapeHTML(response.body)).to include(I18n.t(
-                I18n.t("login.invalid_second_factor_code")
+                "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}.json", params: {
+              post "/session/email-login/#{email_token.token}", params: {
                 second_factor_token: "0000",
                 second_factor_method: UserSecondFactor.methods[:backup_codes]
               }
 
               expect(response.status).to eq(200)
-              expect(JSON.parse(response.body)["error"]).to eq(
+              expect(CGI.unescapeHTML(response.body)).to include(I18n.t(
-                I18n.t("login.invalid_second_factor_code")
+                "login.invalid_second_factor_code"
-              )
+              ))
-              expect(session[:current_user_id]).to eq(nil)
             end
           end
         end
@@ -244,24 +202,22 @@ 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}.json", params: {
+              post "/session/email-login/#{email_token.token}", params: {
                 second_factor_token: ROTP::TOTP.new(user_second_factor.data).now,
                 second_factor_method: UserSecondFactor.methods[:totp]
               }
 
-              expect(JSON.parse(response.body)["success"]).to eq("OK")
+              expect(response).to redirect_to("/")
-              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}.json", params: {
+              post "/session/email-login/#{email_token.token}", params: {
                 second_factor_token: "iAmValidBackupCode",
                 second_factor_method: UserSecondFactor.methods[:backup_codes]
               }
 
-              expect(JSON.parse(response.body)["success"]).to eq("OK")
+              expect(response).to redirect_to("/")
-              expect(session[:current_user_id]).to eq(user.id)
             end
           end
         end
@@ -1369,40 +1325,9 @@ RSpec.describe SessionController 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)
 
@@ -1412,7 +1337,7 @@ RSpec.describe SessionController do
           token = SecureRandom.hex
           $redis.setex "otp_#{token}", 10.minutes, user.username
 
-          post "/session/otp/#{token}"
+          get "/session/otp/#{token}"
 
           expect(response.status).to eq(302)
           expect(response).to redirect_to("/")