Remove Setting.protocol in favor of static config

app/helpers/warning_bar_helper.rb

@@ -40,7 +40,7 @@ module WarningBarHelper
   end
 
   def setting_protocol_mismatched?
-    (request.ssl? && Setting.protocol == 'http') || (!request.ssl? && Setting.protocol == 'https')
+    request.ssl? != OpenProject::Configuration.secure_connection?
   end
 
   def setting_hostname_mismatched?

app/models/setting/aliases.rb

@@ -31,10 +31,13 @@ class Setting
   # Shorthand to common setting aliases to avoid checking values
   module Aliases
     ##
-    # Whether the application is configured to use or force SSL output
-    # for cookie storage et al.
-    def https?
-      Setting.protocol == 'https' || Rails.configuration.force_ssl
+    # Restore the previous Setting.protocol now replaced by https?
+    def protocol
+      if OpenProject::Configuration.secure_connection?
+        'https'
+      else
+        'http'
+      end
     end
   end
 end

app/models/setting/mail_settings.rb

@@ -44,6 +44,7 @@ class Setting
 
     private
 
+    # rubocop:disable Metrics/AbcSize
     def reload_smtp_settings!
       # Correct smtp settings when using authentication :none
       authentication = Setting.smtp_authentication.try(:to_sym)
@@ -74,5 +75,6 @@ class Setting
         ActionMailer::Base.smtp_settings[:openssl_verify_mode] = mode unless mode.nil?
       end
     end
+    # rubocop:enable Metrics/AbcSize
   end
 end

app/views/admin/settings/general_settings/show.html.erb

@@ -47,7 +47,6 @@ See COPYRIGHT and LICENSE files for more details.
         <%= t(:label_example) %>: <%= @guessed_host %>
       </span>
     </div>
-    <div class="form--field"><%= setting_select :protocol, [['HTTP', 'http'], ['HTTPS', 'https']], container_class: '-xslim' %></div>
     <div class="form--field"><%= setting_check_box :cache_formatted_text %></div>
     <div class="form--field"><%= setting_check_box :feeds_enabled, size: 6 %></div>
     <div class="form--field"><%= setting_text_field :feeds_limit, size: 6, container_class: '-xslim' %></div>

config/constants/settings/definitions.rb

@@ -638,10 +638,6 @@ Settings::Definition.define do
   add :plain_text_mail,
       default: false
 
-  add :protocol,
-      default: "http",
-      allowed: %w[http https]
-
   add :project_gantt_query,
       default: nil,
       format: :string
@@ -662,8 +658,17 @@ Settings::Definition.define do
       default: '',
       writable: false
 
+  # Assume we're running in an TLS terminated connection.
+  # This does not affect HSTS, use +rails_force_ssl+ for that.
+  add :https,
+      format: :boolean,
+      default: Rails.env.production?,
+      writable: false
+
+  # Enable HTTPS and HSTS
   add :rails_force_ssl,
-      default: false,
+      format: :boolean,
+      default: Rails.env.production?,
       writable: false
 
   add :registration_footer,

config/initializers/session_store.rb

@@ -54,7 +54,7 @@ module OpenProject
     rescue StandardError => e
       Rails.logger.error(
         "Failed to determine new `after_expire` value. " +
-        "Falling back to original value. (#{e.message} at #{caller.first})"
+          "Falling back to original value. (#{e.message} at #{caller.first})"
       )
 
       options[:expire_after]
@@ -66,52 +66,52 @@ module OpenProject
   end
 end
 
-Rails.application.config.after_initialize do
-  config = OpenProject::Configuration
+config = OpenProject::Configuration
 
-  # Enforce session storage for testing
-  if Rails.env.test?
-    config['session_store'] = :active_record_store
-  end
+# Enforce session storage for testing
+if Rails.env.test?
+  config['session_store'] = :active_record_store
+end
 
-  session_store     = config['session_store'].to_sym
-  relative_url_root = config['rails_relative_url_root'].presence
+session_store = config['session_store'].to_sym
+relative_url_root = config['rails_relative_url_root'].presence
 
-  session_options = {
-    key: config['session_cookie_name'],
-    httponly: true,
-    secure: Setting.https?,
-    path: relative_url_root
-  }
+session_options = {
+  key: config['session_cookie_name'],
+  httponly: true,
+  secure: config.secure_connection?,
+  path: relative_url_root
+}
 
-  if session_store == :cache_store
-    # env OPENPROJECT_CACHE__STORE__SESSION__USER__TTL__DAYS
-    session_ttl = config['cache_store_session_user_ttl_days']&.to_i&.days || 3.days
+if session_store == :cache_store
+  # env OPENPROJECT_CACHE__STORE__SESSION__USER__TTL__DAYS
+  session_ttl = config['cache_store_session_user_ttl_days']&.to_i&.days || 3.days
 
-    # Extend session cache entry TTL so that they can stay logged in when their
-    # session ID cookie's TTL is 'session' where usually the session entry in the
-    # cache would expire before the session in the browser by default.
-    session_options[:expire_store_after] = lambda do |session, expire_after|
-      if session.include? "user_id" # logged-in user
-        [session_ttl, expire_after].compact.max
-      else
-        expire_after # anonymous user
-      end
+  # Extend session cache entry TTL so that they can stay logged in when their
+  # session ID cookie's TTL is 'session' where usually the session entry in the
+  # cache would expire before the session in the browser by default.
+  session_options[:expire_store_after] = lambda do |session, expire_after|
+    if session.include? "user_id" # logged-in user
+      [session_ttl, expire_after].compact.max
+    else
+      expire_after # anonymous user
     end
+  end
+end
 
-    method = ActionDispatch::Session::CacheStore.instance_method(:write_session)
-    unless method.to_s.include?("write_session(env, sid, session, options)")
-      raise(
-        "The signature for `ActionDispatch::Session::CacheStore.write_session` " +
+OpenProject::Application.config.session_store session_store, **session_options
+
+Rails.application.reloader.to_prepare do
+  method = ActionDispatch::Session::CacheStore.instance_method(:write_session)
+  unless method.to_s.include?("write_session(env, sid, session, options)")
+    raise(
+      "The signature for `ActionDispatch::Session::CacheStore.write_session` " +
         "seems to have changed. Please update the " +
         "`ExpireStoreAfterOption` module (and this check) in #{__FILE__}"
-      )
-    end
-
-    ActionDispatch::Session::CacheStore.prepend OpenProject::ExpireStoreAfterOption
+    )
   end
 
-  OpenProject::Application.config.session_store session_store, **session_options
+  ActionDispatch::Session::CacheStore.prepend OpenProject::ExpireStoreAfterOption
 
   ##
   # We use our own decorated session model to note the user_id
@@ -120,4 +120,3 @@ Rails.application.config.after_initialize do
   # Continue to use marshal serialization to retain symbols and whatnot
   ActiveRecord::SessionStore::Session.serializer = :marshal
 end
-

docs/development/localhost-ssl/README.md

@@ -102,17 +102,13 @@ Rails.application.config.hosts << 'openproject.example.com'
 
 
 
-Then, you will start a REPL console for OpenProject with: `RAILS_ENV=development ./bin/rails console`
-
-Update the settings for host name and protocol:
+Then, you will to set the following settings
 
 ```ruby
-Setting.protocol = 'https'
-Setting.host_name = 'openproject.example.com'
+export OPENPROJECT_HTTPS = true
+export OPENPROJECT_HOST__NAME = 'openproject.example.com'
 ```
 
-
-
 Finally, start your OpenProject development server and Frontend server and access `https://openproject.example.com` in your browser.
 
 

docs/installation-and-operations/configuration/ssl/README.md

@@ -79,7 +79,7 @@ If you're terminating SSL on the outer server, you need to set the `X-Forwarded-
 
 
 
-Finally, to let OpenProject know that it should create links with 'https' when no request is available (for example, when sending emails), you need to set the Protocol setting of OpenProject to `https`. You will find this setting on your system settings or via the rails console with `Setting.protocol = 'https'`
+Finally, to let OpenProject know that it should create links with 'https' when no request is available (for example, when sending emails), you need to set the Protocol setting of OpenProject to `https`. You can set this configuration by setting the ENV `OPENPROJECT_HTTPS=true`.
 
 
 _<sup>1</sup> In the packaged installation this means you selected "no" when asked for SSL in the configuration wizard but at the same time take care of SSL termination elsewhere. This can be a manual Apache setup on the same server (not recommended) or an external server, for instance._

lib/tasks/packager.rake

@@ -66,17 +66,17 @@ namespace :packager do
     Setting.host_name = ENV.fetch('SERVER_HOSTNAME', Setting.host_name)
 
     if ENV['SERVER_PROTOCOL_HTTPS_NO_HSTS']
-      # Allow setting only the Setting.protocol without enabling FORCE__SSL
-      # due to external proxy configuration
-      Setting.protocol = 'https'
+      # Allow setting only HTTPS setting without enabling FORCE__SSL
+      # due to external proxy configuration. This avoids activation of HSTS headers.
+      shell_setup(['config:set', "OPENPROJECT_HTTPS=true"])
       shell_setup(['config:unset', "OPENPROJECT_RAILS__FORCE__SSL"])
     elsif ENV['SERVER_PROTOCOL_FORCE_HTTPS'] || ENV.fetch('SERVER_PROTOCOL', Setting.protocol) == 'https'
       # Allow overriding the protocol setting from ENV
       # to allow instances where SSL is terminated earlier to respect that setting
-      Setting.protocol = 'https'
+      shell_setup(['config:set', "OPENPROJECT_HTTPS=true"])
       shell_setup(['config:set', "OPENPROJECT_RAILS__FORCE__SSL=true"])
     else
-      Setting.protocol = 'http'
+      shell_setup(['config:set', "OPENPROJECT_HTTPS=false"])
       shell_setup(['config:unset', "OPENPROJECT_RAILS__FORCE__SSL"])
     end
 

lib_static/open_project/configuration/helpers.rb

@@ -32,6 +32,12 @@ module OpenProject
     # To be included into OpenProject::Configuration in order to provide
     # helper methods for easier access to certain configuration options.
     module Helpers
+      ##
+      # Are we behind a TLS terminated session?
+      def secure_connection?
+        https? || rails_force_ssl?
+      end
+
       def direct_uploads
         return false unless direct_uploads_supported?
 

modules/avatars/lib/open_project/avatars/patches/avatar_helper_patch.rb

@@ -112,10 +112,10 @@ AvatarHelper.class_eval do
     end
 
     def default_gravatar_options
-      options = { secure: Setting.protocol == 'https' }
-      options[:default] = OpenProject::Configuration.gravatar_fallback_image
-
-      options
+      {
+        secure: OpenProject::Configuration.secure_connection?,
+        default: OpenProject::Configuration.gravatar_fallback_image
+      }
     end
 
     ##

modules/avatars/spec/helpers/avatar_helper_spec.rb

@@ -60,11 +60,12 @@ describe AvatarHelper, type: :helper, with_settings: { protocol: 'http' } do
     context 'when enabled' do
       let(:enable_gravatars) { true }
       let(:enable_local_avatars) { true }
-      it "should return the image attached to the user" do
+
+      it "returns the image attached to the user" do
         expect(helper.avatar(user)).to be_html_eql(expected_user_avatar_tag(user))
       end
 
-      it "should return the gravatar image if no image uploaded for the user" do
+      it "returns the gravatar image if no image uploaded for the user" do
         allow(user).to receive(:local_avatar_attachment).and_return nil
 
         expect(helper.avatar(user)).to be_html_eql(expected_user_avatar_tag(user))
@@ -74,7 +75,8 @@ describe AvatarHelper, type: :helper, with_settings: { protocol: 'http' } do
     context 'when gravatar disabled' do
       let(:enable_gravatars) { false }
       let(:enable_local_avatars) { true }
-      it "should return blank if image attached to the user but gravatars disabled" do
+
+      it "returns blank if image attached to the user but gravatars disabled" do
         expect(helper.avatar(user)).to be_html_eql(expected_user_avatar_tag(user))
       end
     end
@@ -83,7 +85,7 @@ describe AvatarHelper, type: :helper, with_settings: { protocol: 'http' } do
       let(:enable_gravatars) { false }
       let(:enable_local_avatars) { false }
 
-      it "should return blank" do
+      it "returns blank" do
         expect(helper.avatar(user)).to be_html_eql(expected_user_avatar_tag(user))
       end
     end
@@ -93,11 +95,12 @@ describe AvatarHelper, type: :helper, with_settings: { protocol: 'http' } do
     context 'when enabled' do
       let(:enable_gravatars) { true }
       let(:enable_local_avatars) { true }
-      it "should return the url to the image attached to the user" do
+
+      it "returns the url to the image attached to the user" do
         expect(helper.avatar_url(user)).to eq(local_expected_url(user))
       end
 
-      it "should return the gravatar url if no image uploaded for the user" do
+      it "returns the gravatar url if no image uploaded for the user" do
         allow(user).to receive(:local_avatar_attachment).and_return nil
 
         expect(helper.avatar_url(user)).to eq(gravatar_expected_url(mail_digest))
@@ -107,7 +110,8 @@ describe AvatarHelper, type: :helper, with_settings: { protocol: 'http' } do
     context 'when gravatar disabled' do
       let(:enable_gravatars) { false }
       let(:enable_local_avatars) { true }
-      it "should return the url if image attached to the user but gravatars disabled" do
+
+      it "returns the url if image attached to the user but gravatars disabled" do
         expect(helper.avatar_url(user)).to eq(local_expected_url(user))
       end
     end
@@ -116,71 +120,70 @@ describe AvatarHelper, type: :helper, with_settings: { protocol: 'http' } do
       let(:enable_gravatars) { false }
       let(:enable_local_avatars) { false }
 
-      it "should return blank" do
+      it "returns blank" do
         expect(helper.avatar_url(user)).to eq ''
       end
     end
   end
 
-  describe 'gravatar' do
-    context 'when enabled' do
-      let(:enable_gravatars) { true }
-      let(:enable_local_avatars) { false }
-      describe 'ssl dependent on protocol settings' do
-        context 'with https protocol', with_settings: { protocol: 'https' } do
-          it "should be set to secure if protocol is 'https'" do
-            expect(helper.default_gravatar_options[:secure]).to be true
-          end
-        end
+  context 'when gravatar enabled' do
+    let(:enable_gravatars) { true }
+    let(:enable_local_avatars) { false }
 
-        context 'with http protocol', with_settings: { protocol: 'http' } do
-          it "should be set to unsecure if protocol is 'http'" do
-            expect(helper.default_gravatar_options[:secure]).to be false
-          end
+    describe 'ssl dependent on protocol settings' do
+      context 'with https protocol', with_config: { https: true } do
+        it "is set to secure if protocol is 'https'" do
+          expect(helper.default_gravatar_options[:secure]).to be true
         end
       end
 
-      context 'with http', with_settings: { protocol: 'http' } do
-        it 'should return a gravatar image tag if a user is provided' do
-          expect(helper.avatar(user)).to be_html_eql(expected_user_avatar_tag(user))
-        end
-
-        it 'should return a gravatar url if a user is provided' do
-          expect(helper.avatar_url(user)).to eq(gravatar_expected_url(mail_digest))
+      context 'with http protocol', with_config: { https: false } do
+        it "is set to unsecure if protocol is 'http'" do
+          expect(helper.default_gravatar_options[:secure]).to be false
         end
       end
+    end
 
-      context 'with https', with_settings: { protocol: 'https' } do
-        it 'should return a gravatar image tag with ssl if the request was ssl required' do
-          expect(helper.avatar(user)).to be_html_eql(expected_user_avatar_tag(user))
-        end
-
-        it 'should return a gravatar image tag with ssl if the request was ssl required' do
-          expect(helper.avatar_url(user)).to eq(gravatar_expected_url(mail_digest, ssl: true))
-        end
+    context 'with http', with_config: { https: false } do
+      it 'returns a gravatar image tag if a user is provided' do
+        expect(helper.avatar(user)).to be_html_eql(expected_user_avatar_tag(user))
       end
 
-      it 'should return an empty string if a non parsable (e-mail) string is provided' do
-        expect(helper.avatar('just the name')).to eq('')
+      it 'returns a gravatar url if a user is provided' do
+        expect(helper.avatar_url(user)).to eq(gravatar_expected_url(mail_digest))
+      end
+    end
+
+    context 'with https', with_config: { https: true } do
+      it 'returns a gravatar image tag without ssl if the request was no ssl required' do
+        expect(helper.avatar(user)).to be_html_eql(expected_user_avatar_tag(user))
       end
 
-      it 'should return an empty string if nil is provided' do
-        expect(helper.avatar(nil)).to eq('')
+      it 'returns a gravatar image tag with ssl if the request was ssl required' do
+        expect(helper.avatar_url(user)).to eq(gravatar_expected_url(mail_digest, ssl: true))
       end
+    end
 
-      it 'should return an empty string if a parsable e-mail with default avatar is provided' do
-        mail = '<e-mail@mail.de>'
+    it 'returns an empty string if a non parsable (e-mail) string is provided' do
+      expect(helper.avatar('just the name')).to eq('')
+    end
 
-        expect(helper.avatar(mail)).to eq('')
-      end
+    it 'returns an empty string if nil is provided' do
+      expect(helper.avatar(nil)).to eq('')
+    end
 
-      it 'should return an empty string if a non parsable (e-mail) string is provided' do
-        expect(helper.avatar_url('just the name')).to eq('')
-      end
+    it 'returns an empty string if a parsable e-mail with default avatar is provided' do
+      mail = '<e-mail@mail.de>'
 
-      it 'should return an empty string if nil is provided' do
-        expect(helper.avatar_url(nil)).to eq('')
-      end
+      expect(helper.avatar(mail)).to eq('')
+    end
+
+    it 'returns an empty string if a non parsable (e-mail) string url is provided' do
+      expect(helper.avatar_url('just the name')).to eq('')
+    end
+
+    it 'returns an empty string if nil url is provided' do
+      expect(helper.avatar_url(nil)).to eq('')
     end
   end
 
@@ -188,11 +191,11 @@ describe AvatarHelper, type: :helper, with_settings: { protocol: 'http' } do
     let(:enable_gravatars) { false }
     let(:enable_local_avatars) { false }
 
-    it 'should return an empty string if gravatar is disabled' do
+    it 'returns an empty string for avatar if gravatar is disabled' do
       expect(helper.avatar(user)).to be_html_eql(expected_user_avatar_tag(user))
     end
 
-    it 'should return an empty string if gravatar is disabled' do
+    it 'returns an empty string for avatar_url if gravatar is disabled' do
       expect(helper.avatar_url(user)).to eq('')
     end
   end

modules/bim/lib/open_project/bim/engine.rb

@@ -203,9 +203,11 @@ module OpenProject::Bim
     config.to_prepare do
       Doorkeeper.configuration.scopes.add(:bcf_v2_1)
 
+      # rubocop:disable Lint/ConstantDefinitionInBlock
       module OpenProject::Authentication::Scope
-        BCF_V2_1 = :bcf_v2_1 # rubocop:disable Lint/ConstantDefinitionInBlock
+        BCF_V2_1 = :bcf_v2_1
       end
+      # rubocop:enable Lint/ConstantDefinitionInBlock
 
       OpenProject::Authentication.update_strategies(OpenProject::Authentication::Scope::BCF_V2_1,
                                                     store: false) do |_strategies|

modules/two_factor_authentication/app/controllers/concerns/two_factor_authentication/remember_token.rb

@@ -29,7 +29,7 @@ module ::TwoFactorAuthentication
         value: new_token!(@authenticated_user),
         httponly: true,
         expires: remember_2fa_days.days.from_now,
-        secure: Setting.protocol == 'https'
+        secure: OpenProject::Configuration.secure_connection?
       }
     end