From 425643bbd8d1729be5b92e5f12b9457687dc85b8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9gis=20Hanol?= <regis@hanol.fr>
Date: Mon, 21 Oct 2024 09:29:37 +0200
Subject: [PATCH] FIX: staff only mode blocks admin password resets (#29289)

When staff only mode is enabled - Discourse.enable_readonly_mode(Discourse::STAFF_WRITES_ONLY_MODE_KEY)

Staff members couldn't reset their password via the "forgot password" link.

This fixes it.

Internal ref. t/133990
---
 app/controllers/session_controller.rb    |  4 ++--
 app/controllers/users_controller.rb      |  5 +++--
 spec/requests/session_controller_spec.rb | 17 +++++++++++++++++
 spec/requests/users_controller_spec.rb   | 24 ++++++++++++++++++++++++
 4 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/app/controllers/session_controller.rb b/app/controllers/session_controller.rb
index a63f8aed1c0..ac600d05c9a 100644
--- a/app/controllers/session_controller.rb
+++ b/app/controllers/session_controller.rb
@@ -12,8 +12,7 @@ class SessionController < ApplicationController
 
   skip_before_action :check_xhr, only: %i[second_factor_auth_show]
 
-  allow_in_staff_writes_only_mode :create
-  allow_in_staff_writes_only_mode :email_login
+  allow_in_staff_writes_only_mode :create, :email_login, :forgot_password
 
   ACTIVATE_USER_KEY = "activate_user"
   FORGOT_PASSWORD_EMAIL_LIMIT_PER_DAY = 6
@@ -634,6 +633,7 @@ class SessionController < ApplicationController
       end
 
     if user
+      raise Discourse::ReadOnly if staff_writes_only_mode? && !user.staff?
       enqueue_password_reset_for_user(user)
     else
       RateLimiter.new(
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 984f4ada620..bc6ae120633 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -107,8 +107,7 @@ class UsersController < ApplicationController
 
   before_action :add_noindex_header, only: %i[show my_redirect]
 
-  allow_in_staff_writes_only_mode :admin_login
-  allow_in_staff_writes_only_mode :email_login
+  allow_in_staff_writes_only_mode :admin_login, :email_login, :password_reset_update
 
   MAX_RECENT_SEARCHES = 5
 
@@ -866,6 +865,8 @@ class UsersController < ApplicationController
     # no point doing anything else if we can't even find
     # a user from the token
     if @user
+      raise Discourse::ReadOnly if staff_writes_only_mode? && !@user.staff?
+
       if !secure_session["second-factor-#{token}"]
         second_factor_authentication_result =
           @user.authenticate_second_factor(params, secure_session)
diff --git a/spec/requests/session_controller_spec.rb b/spec/requests/session_controller_spec.rb
index 933a4450167..492a1a340ea 100644
--- a/spec/requests/session_controller_spec.rb
+++ b/spec/requests/session_controller_spec.rb
@@ -2841,6 +2841,23 @@ RSpec.describe SessionController do
         expect(Jobs::CriticalUserEmail.jobs.size).to eq(0)
       end
     end
+
+    context "when in staff writes only mode" do
+      before { Discourse.enable_readonly_mode(Discourse::STAFF_WRITES_ONLY_MODE_KEY) }
+
+      it "allows staff to forget their password" do
+        post "/session/forgot_password.json", params: { login: admin.username }
+        expect(response.status).to eq(200)
+        expect(response.parsed_body["error"]).not_to be_present
+        expect(Jobs::CriticalUserEmail.jobs.size).to eq(1)
+      end
+
+      it "doesn't allow non-staff to forget their password" do
+        post "/session/forgot_password.json", params: { login: user.username }
+        expect(response.status).to eq(503)
+        expect(Jobs::CriticalUserEmail.jobs.size).to eq(0)
+      end
+    end
   end
 
   describe "#current" do
diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb
index 84c2d690182..cb0a20ea72c 100644
--- a/spec/requests/users_controller_spec.rb
+++ b/spec/requests/users_controller_spec.rb
@@ -600,6 +600,30 @@ RSpec.describe UsersController do
         expect(response.parsed_body["errors"]).to be_blank
         expect(session[:current_user_id]).to be_blank
       end
+
+      context "when in staff writes only mode" do
+        before { Discourse.enable_readonly_mode(Discourse::STAFF_WRITES_ONLY_MODE_KEY) }
+
+        it "allows staff to reset their password" do
+          admin = Fabricate(:admin)
+          email_token =
+            Fabricate(:email_token, user: admin, scope: EmailToken.scopes[:password_reset])
+
+          put "/u/password-reset/#{email_token.token}.json",
+              params: {
+                password: "hg9ow8yhg98oadminlonger",
+              }
+
+          expect(response.parsed_body["errors"]).to be_blank
+          expect(session[:current_user_id]).to eq(admin.id)
+        end
+
+        it "doesn't allow non-staff to reset their password" do
+          put "/u/password-reset/#{email_token.token}.json", params: { password: "ksjafh928r" }
+          expect(response.parsed_body["errors"]).to_not be_blank
+          expect(session[:current_user_id]).to be_blank
+        end
+      end
     end
   end