From 6e53f4d9133f70659a3cf1c82cf27d29815afbf8 Mon Sep 17 00:00:00 2001 From: Daniel Waterworth Date: Tue, 17 May 2022 13:06:08 -0500 Subject: [PATCH] DEV: New readonly mode. Only applies to non-staff (#16243) --- .../discourse/app/controllers/login.js | 6 ++ .../javascripts/discourse/app/models/site.js | 1 + .../discourse/app/routes/application.js | 14 ++- app/controllers/application_controller.rb | 8 +- app/controllers/forums_controller.rb | 4 +- app/controllers/session_controller.rb | 14 ++- .../users/omniauth_callbacks_controller.rb | 6 +- app/controllers/users_controller.rb | 2 + lib/discourse.rb | 9 +- lib/read_only_header.rb | 13 --- lib/read_only_mixin.rb | 57 ++++++++++++ spec/requests/forums_controller_spec.rb | 7 ++ .../omniauth_callbacks_controller_spec.rb | 27 ++++++ spec/requests/session_controller_spec.rb | 87 +++++++++++++++++++ 14 files changed, 228 insertions(+), 27 deletions(-) delete mode 100644 lib/read_only_header.rb create mode 100644 lib/read_only_mixin.rb diff --git a/app/assets/javascripts/discourse/app/controllers/login.js b/app/assets/javascripts/discourse/app/controllers/login.js index ef43315ea96..4371268d09b 100644 --- a/app/assets/javascripts/discourse/app/controllers/login.js +++ b/app/assets/javascripts/discourse/app/controllers/login.js @@ -256,6 +256,12 @@ export default Controller.extend(ModalFunctionality, { // Failed to login if (e.jqXHR && e.jqXHR.status === 429) { this.flash(I18n.t("login.rate_limit"), "error"); + } else if ( + e.jqXHR && + e.jqXHR.status === 503 && + e.jqXHR.responseJSON.error_type === "read_only" + ) { + this.flash(I18n.t("read_only_mode.login_disabled"), "error"); } else if (!areCookiesEnabled()) { this.flash(I18n.t("login.cookies_error"), "error"); } else { diff --git a/app/assets/javascripts/discourse/app/models/site.js b/app/assets/javascripts/discourse/app/models/site.js index 4a9725ad377..72666a3623b 100644 --- a/app/assets/javascripts/discourse/app/models/site.js +++ b/app/assets/javascripts/discourse/app/models/site.js @@ -129,6 +129,7 @@ Site.reopenClass(Singleton, { const store = getOwner(this).lookup("service:store"); const siteAttributes = PreloadStore.get("site"); siteAttributes["isReadOnly"] = PreloadStore.get("isReadOnly"); + siteAttributes["isStaffWritesOnly"] = PreloadStore.get("isStaffWritesOnly"); return store.createRecord("site", siteAttributes); }, diff --git a/app/assets/javascripts/discourse/app/routes/application.js b/app/assets/javascripts/discourse/app/routes/application.js index ea6da0a5c76..0b02e6417fa 100644 --- a/app/assets/javascripts/discourse/app/routes/application.js +++ b/app/assets/javascripts/discourse/app/routes/application.js @@ -17,7 +17,17 @@ import showModal from "discourse/lib/show-modal"; function unlessReadOnly(method, message) { return function () { - if (this.site.get("isReadOnly")) { + if (this.site.isReadOnly) { + bootbox.alert(message); + } else { + this[method](); + } + }; +} + +function unlessStrictlyReadOnly(method, message) { + return function () { + if (this.site.isReadOnly && !this.site.isStaffWritesOnly) { bootbox.alert(message); } else { this[method](); @@ -114,7 +124,7 @@ const ApplicationRoute = DiscourseRoute.extend(OpenComposer, { return true; }, - showLogin: unlessReadOnly( + showLogin: unlessStrictlyReadOnly( "handleShowLogin", I18n.t("read_only_mode.login_disabled") ), diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 0a87a245baf..6fff04294fa 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -8,7 +8,7 @@ class ApplicationController < ActionController::Base include JsonError include GlobalPath include Hijack - include ReadOnlyHeader + include ReadOnlyMixin include VaryHeader attr_reader :theme_id @@ -631,6 +631,7 @@ class ApplicationController < ActionController::Base store_preloaded("banner", banner_json) store_preloaded("customEmoji", custom_emoji) store_preloaded("isReadOnly", @readonly_mode.to_s) + store_preloaded("isStaffWritesOnly", @staff_writes_only_mode.to_s) store_preloaded("activatedThemes", activated_themes_json) end @@ -876,11 +877,6 @@ class ApplicationController < ActionController::Base !disqualified_from_2fa_enforcement && enforcing_2fa && !current_user.has_any_second_factor_methods_enabled? end - def block_if_readonly_mode - return if request.fullpath.start_with?(path "/admin/backups") - raise Discourse::ReadOnly.new if !(request.get? || request.head?) && @readonly_mode - end - def build_not_found_page(opts = {}) if SiteSetting.bootstrap_error_pages? preload_json diff --git a/app/controllers/forums_controller.rb b/app/controllers/forums_controller.rb index 88d2879e9b1..ac172d54b3a 100644 --- a/app/controllers/forums_controller.rb +++ b/app/controllers/forums_controller.rb @@ -1,9 +1,9 @@ # frozen_string_literal: true -require "read_only_header" +require "read_only_mixin" class ForumsController < ActionController::Base - include ReadOnlyHeader + include ReadOnlyMixin before_action :check_readonly_mode after_action :add_readonly_header diff --git a/app/controllers/session_controller.rb b/app/controllers/session_controller.rb index d1222a0728f..26299bcb8fd 100644 --- a/app/controllers/session_controller.rb +++ b/app/controllers/session_controller.rb @@ -10,6 +10,8 @@ class SessionController < ApplicationController requires_login only: [:second_factor_auth_show, :second_factor_auth_perform] + allow_in_staff_writes_only_mode :create + ACTIVATE_USER_KEY = "activate_user" def csrf @@ -116,7 +118,7 @@ class SessionController < ApplicationController def sso_login raise Discourse::NotFound unless SiteSetting.enable_discourse_connect - raise Discourse::ReadOnly if @readonly_mode + raise Discourse::ReadOnly if @readonly_mode && !staff_writes_only_mode? params.require(:sso) params.require(:sig) @@ -147,6 +149,7 @@ class SessionController < ApplicationController invite = validate_invitiation!(sso) if user = sso.lookup_or_create_user(request.remote_ip) + raise Discourse::ReadOnly if staff_writes_only_mode? && !user&.staff? if user.suspended? render_sso_error(text: failed_to_login(user)[:error], status: 403) @@ -270,6 +273,9 @@ class SessionController < ApplicationController return invalid_credentials if params[:password].length > User.max_password_length user = User.find_by_username_or_email(normalized_login_param) + + raise Discourse::ReadOnly if staff_writes_only_mode? && !user&.staff? + rate_limit_second_factor!(user) if user.present? @@ -303,7 +309,11 @@ class SessionController < ApplicationController return render(json: @second_factor_failure_payload) end - (user.active && user.email_confirmed?) ? login(user, second_factor_auth_result) : not_activated(user) + if user.active && user.email_confirmed? + login(user, second_factor_auth_result) + else + not_activated(user) + end end def email_login_info diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb index 49ca8919e25..3634d2f4c0d 100644 --- a/app/controllers/users/omniauth_callbacks_controller.rb +++ b/app/controllers/users/omniauth_callbacks_controller.rb @@ -14,6 +14,8 @@ class Users::OmniauthCallbacksController < ApplicationController # will not have a CSRF token, however the payload is all validated so its safe skip_before_action :verify_authenticity_token, only: :complete + allow_in_staff_writes_only_mode :complete + def confirm_request self.class.find_authenticator(params[:provider]) render locals: { hide_auth_buttons: true } @@ -22,7 +24,7 @@ class Users::OmniauthCallbacksController < ApplicationController def complete auth = request.env["omniauth.auth"] raise Discourse::NotFound unless request.env["omniauth.auth"] - raise Discourse::ReadOnly if @readonly_mode + raise Discourse::ReadOnly if @readonly_mode && !staff_writes_only_mode? auth[:session] = session @@ -71,6 +73,8 @@ class Users::OmniauthCallbacksController < ApplicationController return render_auth_result_failure if @auth_result.failed? + raise Discourse::ReadOnly if staff_writes_only_mode? && !@auth_result.user&.staff? + complete_response_data return render_auth_result_failure if @auth_result.failed? diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 53ecf2f9baf..20e90ae126e 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -51,6 +51,8 @@ class UsersController < ApplicationController after_action :add_noindex_header, only: [:show, :my_redirect] + allow_in_staff_writes_only_mode :admin_login + MAX_RECENT_SEARCHES = 5 def index diff --git a/lib/discourse.rb b/lib/discourse.rb index e4a5c49c12c..dc0a93447ee 100644 --- a/lib/discourse.rb +++ b/lib/discourse.rb @@ -504,6 +504,9 @@ module Discourse USER_READONLY_MODE_KEY ||= 'readonly_mode:user' PG_FORCE_READONLY_MODE_KEY ||= 'readonly_mode:postgres_force' + # Psuedo readonly mode, where staff can still write + STAFF_WRITES_ONLY_MODE_KEY ||= 'readonly_mode:staff_writes_only' + READONLY_KEYS ||= [ READONLY_MODE_KEY, PG_READONLY_MODE_KEY, @@ -516,7 +519,7 @@ module Discourse Sidekiq.pause!("pg_failover") if !Sidekiq.paused? end - if key == USER_READONLY_MODE_KEY || key == PG_FORCE_READONLY_MODE_KEY + if [USER_READONLY_MODE_KEY, PG_FORCE_READONLY_MODE_KEY, STAFF_WRITES_ONLY_MODE_KEY].include?(key) Discourse.redis.set(key, 1) else ttl = @@ -594,6 +597,10 @@ module Discourse recently_readonly? || Discourse.redis.exists?(*keys) end + def self.staff_writes_only_mode? + Discourse.redis.get(STAFF_WRITES_ONLY_MODE_KEY).present? + end + def self.pg_readonly_mode? Discourse.redis.get(PG_READONLY_MODE_KEY).present? end diff --git a/lib/read_only_header.rb b/lib/read_only_header.rb deleted file mode 100644 index 51cb2447baa..00000000000 --- a/lib/read_only_header.rb +++ /dev/null @@ -1,13 +0,0 @@ -# frozen_string_literal: true - -module ReadOnlyHeader - - def check_readonly_mode - @readonly_mode = Discourse.readonly_mode? - end - - def add_readonly_header - response.headers['Discourse-Readonly'] = 'true' if @readonly_mode - end - -end diff --git a/lib/read_only_mixin.rb b/lib/read_only_mixin.rb new file mode 100644 index 00000000000..3fa05604f02 --- /dev/null +++ b/lib/read_only_mixin.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +module ReadOnlyMixin + module ClassMethods + def actions_allowed_in_staff_writes_only_mode + @actions_allowed_in_staff_writes_only_mode ||= [] + end + + def allow_in_staff_writes_only_mode(*actions) + actions_allowed_in_staff_writes_only_mode.concat(actions.map(&:to_sym)) + end + + def allowed_in_staff_writes_only_mode?(action_name) + actions_allowed_in_staff_writes_only_mode.include?(action_name.to_sym) + end + end + + def staff_writes_only_mode? + @staff_writes_only_mode + end + + def check_readonly_mode + if Discourse.readonly_mode? + @readonly_mode = true + @staff_writes_only_mode = false + elsif Discourse.staff_writes_only_mode? + @readonly_mode = true + @staff_writes_only_mode = true + else + @readonly_mode = false + @staff_writes_only_mode = false + end + end + + def add_readonly_header + response.headers['Discourse-Readonly'] = 'true' if @readonly_mode + end + + def allowed_in_staff_writes_only_mode? + self.class.allowed_in_staff_writes_only_mode?(action_name) + end + + def block_if_readonly_mode + return if request.fullpath.start_with?(path "/admin/backups") + return if request.get? || request.head? + + if @staff_writes_only_mode + raise Discourse::ReadOnly.new if !current_user&.staff? && !allowed_in_staff_writes_only_mode? + elsif @readonly_mode + raise Discourse::ReadOnly.new + end + end + + def self.included(base) + base.extend(ClassMethods) + end +end diff --git a/spec/requests/forums_controller_spec.rb b/spec/requests/forums_controller_spec.rb index 188f216f868..fc94c012023 100644 --- a/spec/requests/forums_controller_spec.rb +++ b/spec/requests/forums_controller_spec.rb @@ -15,6 +15,13 @@ RSpec.describe ForumsController do expect(response.status).to eq(200) expect(response.headers['Discourse-Readonly']).to eq('true') end + + it "returns a readonly header if the site is in staff-writes-only mode" do + Discourse.stubs(:staff_writes_only_mode?).returns(true) + get "/srv/status" + expect(response.status).to eq(200) + expect(response.headers['Discourse-Readonly']).to eq('true') + end end describe "cluster parameter" do diff --git a/spec/requests/omniauth_callbacks_controller_spec.rb b/spec/requests/omniauth_callbacks_controller_spec.rb index ffccc1e3f04..032c539c13c 100644 --- a/spec/requests/omniauth_callbacks_controller_spec.rb +++ b/spec/requests/omniauth_callbacks_controller_spec.rb @@ -168,6 +168,33 @@ RSpec.describe Users::OmniauthCallbacksController do end end + context "in staff writes only mode" do + use_redis_snapshotting + + before do + Discourse.enable_readonly_mode(Discourse::STAFF_WRITES_ONLY_MODE_KEY) + end + + it "returns a 503 for non-staff" do + mock_auth(user.email, user.username, user.name) + get "/auth/google_oauth2/callback.json" + expect(response.status).to eq(503) + logged_on_user = Discourse.current_user_provider.new(request.env).current_user + + expect(logged_on_user).to eq(nil) + end + + it "completes for staff" do + user.update!(admin: true) + mock_auth(user.email, user.username, user.name) + get "/auth/google_oauth2/callback.json" + expect(response.status).to eq(302) + logged_on_user = Discourse.current_user_provider.new(request.env).current_user + + expect(logged_on_user).not_to eq(nil) + end + end + context "without an `omniauth.auth` env" do it "should return a 404" do get "/auth/eviltrout/callback" diff --git a/spec/requests/session_controller_spec.rb b/spec/requests/session_controller_spec.rb index 39681e63679..d40472dd8d2 100644 --- a/spec/requests/session_controller_spec.rb +++ b/spec/requests/session_controller_spec.rb @@ -6,6 +6,9 @@ describe SessionController do let(:user) { Fabricate(:user) } let(:email_token) { Fabricate(:email_token, user: user) } + fab!(:admin) { Fabricate(:admin) } + let(:admin_email_token) { Fabricate(:email_token, user: admin) } + shared_examples 'failed to continue local login' do it 'should return the right response' do expect(response).not_to be_successful @@ -549,6 +552,41 @@ describe SessionController do sso end + context 'in staff writes only mode' do + use_redis_snapshotting + + before do + Discourse.enable_readonly_mode(Discourse::STAFF_WRITES_ONLY_MODE_KEY) + end + + it 'allows staff to login' do + sso = get_sso('/a/') + sso.external_id = '666' + sso.email = 'bob@bob.com' + sso.name = 'Bob Bobson' + sso.username = 'bob' + sso.admin = true + + get "/session/sso_login", params: Rack::Utils.parse_query(sso.payload), headers: headers + + logged_on_user = Discourse.current_user_provider.new(request.env).current_user + expect(logged_on_user).not_to eq(nil) + end + + it 'doesn\'t allow non-staff to login' do + sso = get_sso('/a/') + sso.external_id = '666' + sso.email = 'bob@bob.com' + sso.name = 'Bob Bobson' + sso.username = 'bob' + + get "/session/sso_login", params: Rack::Utils.parse_query(sso.payload), headers: headers + + logged_on_user = Discourse.current_user_provider.new(request.env).current_user + expect(logged_on_user).to eq(nil) + end + end + it 'does not create superfluous auth tokens when already logged in' do user = Fabricate(:user) sign_in(user) @@ -1494,6 +1532,55 @@ describe SessionController do end describe '#create' do + context 'read only mode' do + use_redis_snapshotting + + before do + Discourse.enable_readonly_mode + EmailToken.confirm(email_token.token) + EmailToken.confirm(admin_email_token.token) + end + + it 'prevents login by regular users' do + post "/session.json", params: { + login: user.username, password: 'myawesomepassword' + } + expect(response.status).not_to eq(200) + end + + it 'prevents login by admins' do + post "/session.json", params: { + login: user.username, password: 'myawesomepassword' + } + expect(response.status).not_to eq(200) + end + end + + context 'staff writes only mode' do + use_redis_snapshotting + + before do + Discourse.enable_readonly_mode(Discourse::STAFF_WRITES_ONLY_MODE_KEY) + EmailToken.confirm(email_token.token) + EmailToken.confirm(admin_email_token.token) + end + + it 'allows admin login' do + post "/session.json", params: { + login: admin.username, password: 'myawesomepassword' + } + expect(response.status).to eq(200) + expect(response.parsed_body['error']).not_to be_present + end + + it 'prevents login by regular users' do + post "/session.json", params: { + login: user.username, password: 'myawesomepassword' + } + expect(response.status).not_to eq(200) + end + end + context 'local login is disabled' do before do SiteSetting.enable_local_logins = false