Merge pull request #15318 from hennevogel/bugfix/15315

Redirect to login page or root if anonymous

src/api/app/controllers/webui/webui_controller.rb

@@ -172,14 +172,20 @@ def require_admin
 
   # before filter to only show the frontpage to anonymous users
   def check_anonymous
-    if User.session
-      false
-    else
-      unless ::Configuration.anonymous
-        flash[:error] = 'No anonymous access. Please log in!'
-        redirect_back(fallback_location: root_path)
-      end
-    end
+    return if User.session.present?
+    return if ::Configuration.anonymous
+
+    login_page = case CONFIG['proxy_auth_mode']
+                 when :mellon
+                   add_return_to_parameter_to_query(url: CONFIG['proxy_auth_login_page'], parameter_name: 'ReturnTo')
+                 when :ichain
+                   add_return_to_parameter_to_query(url: CONFIG['proxy_auth_login_page'], parameter_name: 'url')
+                 else
+                   root_path
+                 end
+
+    flash[:error] = 'No anonymous access. Please log in!' unless ::Configuration.proxy_auth_mode_enabled?
+    redirect_to login_page
   end
 
   # After filter to clean up caches
@@ -240,4 +246,15 @@ def set_influxdb_data
       interface: :webui
     }
   end
+
+  def add_return_to_parameter_to_query(url:, parameter_name:)
+    uri = URI(url)
+    return_to = {}
+    return_to[parameter_name] = request.fullpath
+    query_array = uri.query.to_s.split('&')
+    query_array << return_to.to_query # for URL encoding
+    uri.query = query_array.join('&')
+
+    uri.to_s
+  end
 end

src/api/app/helpers/webui/webui_helper.rb

@@ -292,7 +292,11 @@ def sign_up_link(css_class: nil)
   end
 
   def log_in_link(css_class: nil)
-    if kerberos_mode?
+    if CONFIG['proxy_auth_mode'] == :mellon
+      link_to(CONFIG['proxy_auth_login_page'], class: css_class) do
+        link_content('Log In', css_class, 'fa-sign-in-alt')
+      end
+    elsif kerberos_mode?
       link_to(new_session_path, class: css_class) do
         link_content('Log In', css_class, 'fa-sign-in-alt')
       end

src/api/app/models/concerns/configuration_constants.rb

@@ -0,0 +1,46 @@
+module ConfigurationConstants
+  extend ActiveSupport::Concern
+
+  # note: do not add defaults here. It must be either the options.yml content or nil
+  # rubocop:disable Style/MutableConstant
+  # FIXME: The hash keys are outputted in the error message of the PUT endpoint in the configurations_controller.
+  #        To remove the confusion, the hash keys should have the same name as the config options from which they take their value.
+  OPTIONS_YML = {
+    title: nil,
+    description: nil,
+    name: nil, # from BSConfig.pm
+    download_on_demand: nil, # from BSConfig.pm
+    enforce_project_keys: nil, # from BSConfig.pm
+    anonymous: CONFIG['allow_anonymous'],
+    registration: CONFIG['new_user_registration'],
+    default_access_disabled: CONFIG['default_access_disabled'],
+    allow_user_to_create_home_project: CONFIG['allow_user_to_create_home_project'],
+    disallow_group_creation: CONFIG['disallow_group_creation_with_api'],
+    change_password: CONFIG['change_passwd'],
+    obs_url: nil, # inital setup may happen in webui api controller
+    api_url: nil,
+    hide_private_options: CONFIG['hide_private_options'],
+    gravatar: CONFIG['use_gravatar'],
+    download_url: CONFIG['download_url'],
+    ymp_url: CONFIG['ymp_url'],
+    bugzilla_url: CONFIG['bugzilla_host'],
+    http_proxy: CONFIG['http_proxy'],
+    no_proxy: nil,
+    cleanup_after_days: nil,
+    theme: CONFIG['theme'],
+    cleanup_empty_projects: nil,
+    disable_publish_for_branches: nil,
+    admin_email: nil,
+    unlisted_projects_filter: nil,
+    unlisted_projects_filter_description: nil,
+    tos_url: nil,
+    code_of_conduct: nil,
+    default_tracker: nil
+  }
+  # rubocop:enable Style/MutableConstant
+
+  ON_OFF_OPTIONS = [:anonymous, :default_access_disabled, :allow_user_to_create_home_project, :disallow_group_creation,
+                    :change_password, :hide_private_options, :gravatar, :download_on_demand, :enforce_project_keys,
+                    :cleanup_empty_projects, :disable_publish_for_branches].freeze
+  PROXY_MODE_ENABLED_VALUES = [:on, :ichain, :mellon].freeze
+end

src/api/app/models/configuration.rb

@@ -3,58 +3,17 @@ class Configuration < ApplicationRecord
   after_save :delayed_write_to_backend
 
   include CanRenderModel
+  include ConfigurationConstants
 
   validates :name, :title, :description, presence: true
   validates :admin_email, :api_url, :bugzilla_url, :default_tracker, :download_url, :http_proxy, :name, :no_proxy, :obs_url, :theme, :title, :tos_url, :unlisted_projects_filter,
             :unlisted_projects_filter_description, :ymp_url, length: { maximum: 255 }
   validates :description, :code_of_conduct, length: { maximum: 65_535 }
 
-  # note: do not add defaults here. It must be either the options.yml content or nil
-  # rubocop:disable Style/MutableConstant
-  # FIXME: The hash keys are outputted in the error message of the PUT endpoint in the configurations_controller.
-  #        To remove the confusion, the hash keys should have the same name as the config options from which they take their value.
-  OPTIONS_YML = {
-    title: nil,
-    description: nil,
-    name: nil, # from BSConfig.pm
-    download_on_demand: nil, # from BSConfig.pm
-    enforce_project_keys: nil, # from BSConfig.pm
-    anonymous: CONFIG['allow_anonymous'],
-    registration: CONFIG['new_user_registration'],
-    default_access_disabled: CONFIG['default_access_disabled'],
-    allow_user_to_create_home_project: CONFIG['allow_user_to_create_home_project'],
-    disallow_group_creation: CONFIG['disallow_group_creation_with_api'],
-    change_password: CONFIG['change_passwd'],
-    obs_url: nil, # inital setup may happen in webui api controller
-    api_url: nil,
-    hide_private_options: CONFIG['hide_private_options'],
-    gravatar: CONFIG['use_gravatar'],
-    download_url: CONFIG['download_url'],
-    ymp_url: CONFIG['ymp_url'],
-    bugzilla_url: CONFIG['bugzilla_host'],
-    http_proxy: CONFIG['http_proxy'],
-    no_proxy: nil,
-    cleanup_after_days: nil,
-    theme: CONFIG['theme'],
-    cleanup_empty_projects: nil,
-    disable_publish_for_branches: nil,
-    admin_email: nil,
-    unlisted_projects_filter: nil,
-    unlisted_projects_filter_description: nil,
-    tos_url: nil,
-    code_of_conduct: nil,
-    default_tracker: nil
-  }
-  # rubocop:enable Style/MutableConstant
-
-  ON_OFF_OPTIONS = [:anonymous, :default_access_disabled, :allow_user_to_create_home_project, :disallow_group_creation,
-                    :change_password, :hide_private_options, :gravatar, :download_on_demand, :enforce_project_keys,
-                    :cleanup_empty_projects, :disable_publish_for_branches].freeze
-
   class << self
     def map_value(key, value)
       # make them boolean
-      return value.in?([:on, ':on', 'on', 'true', true]) if key.in?(ON_OFF_OPTIONS)
+      return value.in?([:on, ':on', 'on', 'true', true]) if key.in?(::Configuration::ON_OFF_OPTIONS)
 
       value
     end
@@ -86,7 +45,16 @@ def ldap_enabled?
   end
 
   def proxy_auth_mode_enabled?
-    CONFIG['proxy_auth_mode'] == :on || CONFIG['ichain_mode'] == :on
+    logger.info 'Warning: You are using the deprecated ichain_mode setting in config/options.yml' if CONFIG['ichain_mode'].present?
+
+    return false unless PROXY_MODE_ENABLED_VALUES.include?(CONFIG['proxy_auth_mode']) || CONFIG['ichain_mode'] == :on
+
+    unless CONFIG['proxy_auth_login_page'].present? && CONFIG['proxy_auth_logout_page'].present?
+      logger.info 'Warning: You enabled proxy_auth_mode in config/options.yml but did not set the required proxy_auth_login_page/proxy_auth_logout_page options'
+      return false
+    end
+
+    true
   end
 
   def amqp_namespace

src/api/config/options.yml.example

@@ -29,22 +29,47 @@ default: &default
 
   extended_backend_log: false
 
-  ### Proxy Auth Mode configuration
+  ### Public Access Configuration
+  # Configure if this OBS is allowing people to browse it's content anonymously or not.
+  # Disabling this also disables the interconnect feature.
+  # Boolean: true or false
+  # Default: true
+  #
+  # allow_anonymous: false
 
-  # Enable proxy_auth_mode. Can be :off or :on
-  proxy_auth_mode: :off
+  ### Proxy Auth Configuration
+  # Configure if this OBS is behind an Identity And Access Proxy (IAP). A
+  # reverse proxy in front of this OBS that authenticates the user making the
+  # request using an Identity Provider (IDP) and modifies the request headers
+  # to include information about the authenticated user.
   #
-  # The URL we send people to sign up
-  # proxy_auth_register_page: https://my-idp.example.com/signup
+  # Request Headers used by OBS:
+  # - HTTP_X_USERNAME -> User.login
+  # - HTTP_X_EMAIL -> User.email
+  # - HTTP_X_FIRSTNAME + HTTP_X_LASTNAME -> User.realname
   #
-  # The URL we send people to log in
-  # proxy_auth_login_page: https://my-idp.example.com/login
+  # The mode. Can be:
+  # - :off to disable it
+  # - :on to enable generic mode
+  # - :mellon to enable apache mod_auth_mellon (SAML) mode
+  # - :ichain to enable legacy ichain mode
+  proxy_auth_mode: :off
   #
-  # The URL we send people to view their account
-  # proxy_auth_account_page: https://my-idp.example.com/myaccount
+  # The URL we send people to log in.
+  # *This setting is required for proxy auth to work.
+  # proxy_auth_login_page: https://my-idp.example.com/login
   #
-  # The URL we send people to log out
+  # The URL we send people to log out.
+  # *This setting is required for proxy auth to work.
   # proxy_auth_logout_page: https://my-idp.example.com/logout
+  #
+  # The URL we send people to sign up.
+  # This setting is optional, if not set sign up is disabled.
+  # proxy_auth_register_page: https://my-idp.example.com/signup
+  #
+  # The URL we send people to view their account.
+  # This setting is optional, if not set account links are not shown.
+  # proxy_auth_account_page: https://my-idp.example.com/myaccount
 
   ### Kerberos configuration
 

src/api/spec/controllers/webui/webui_controller_spec.rb

@@ -7,9 +7,10 @@
     before_action :require_login, only: :show
     before_action :set_project, only: [:edit, :create]
     before_action :require_package, only: :create
+    before_action :check_anonymous, only: :index
 
     def index
-      render plain: 'anonymous controller'
+      render plain: 'anonymous controller  - check_anonymous'
     end
 
     # RSpec anonymous controller only support RESTful routes
@@ -31,37 +32,6 @@ def create
     end
   end
 
-  describe 'GET index as nobody' do
-    it 'is allowed when Configuration.anonymous is true' do
-      Configuration.update(anonymous: true)
-
-      get :index
-      expect(response).to have_http_status(:success)
-    end
-
-    it 'is not allowed when Configuration.anonymous is false' do
-      Configuration.update(anonymous: false)
-
-      get :index
-      expect(response).to redirect_to(root_path)
-    end
-  end
-
-  describe 'GET index as a user' do
-    it 'is always allowed' do
-      login(create(:confirmed_user))
-      Configuration.update(anonymous: true)
-
-      get :index
-      expect(response).to have_http_status(:success)
-
-      Configuration.update(anonymous: false)
-
-      get :index
-      expect(response).to have_http_status(:success)
-    end
-  end
-
   describe 'require_admin before filter' do
     it 'redirects to main page for non privileged user' do
       login(create(:confirmed_user, login: 'confirmed_user'))
@@ -138,4 +108,29 @@ def create
       end
     end
   end
+
+  describe 'check_anonymous before filter' do
+    subject { get :index }
+
+    before do
+      allow(Configuration).to receive(:anonymous).and_return(false)
+    end
+
+    context 'with proxy_auth_mode enabled' do
+      before do
+        allow(Configuration).to receive(:proxy_auth_mode_enabled?).and_return(true)
+        stub_const('CONFIG', { proxy_auth_login_page: '/', proxy_auth_logout_page: '/', proxy_auth_mode: :mellon }.with_indifferent_access)
+      end
+
+      it { is_expected.to redirect_to('/?ReturnTo=%2Fwebui%2Fwebui') }
+    end
+
+    context 'with proxy_auth_mode disabled' do
+      before do
+        allow(Configuration).to receive(:proxy_auth_mode_enabled?).and_return(false)
+      end
+
+      it { is_expected.to redirect_to(root_path) }
+    end
+  end
 end

src/api/spec/models/configuration_spec.rb

@@ -41,9 +41,14 @@
     end
 
     context 'external authentication services' do
-      it 'returns false if config option `proxy_auth_mode` is set to :on' do
-        stub_const('CONFIG', CONFIG.merge('proxy_auth_mode' => :on))
-        expect(config.passwords_changable?).to be(false)
+      context 'in proxy auth mode' do
+        before do
+          stub_const('CONFIG', { proxy_auth_mode: :on, proxy_auth_login_page: '/', proxy_auth_logout_page: '/' }.with_indifferent_access)
+        end
+
+        it 'returns false if config option `proxy_auth_mode` is set to :on' do
+          expect(config.passwords_changable?).to be(false)
+        end
       end
 
       context 'in LDAP mode' do
@@ -79,11 +84,8 @@
     let(:config) { Configuration.first }
 
     context 'proxy_auth_mode is enabled' do
-      before do
-        stub_const('CONFIG', CONFIG.merge('proxy_auth_mode' => :on))
-      end
-
       it 'returns false if proxy_auth_account_page is not present' do
+        stub_const('CONFIG', { proxy_auth_mode: :on, proxy_auth_login_page: '/', proxy_auth_logout_page: '/' }.with_indifferent_access)
         expect(config.accounts_editable?).to be(false)
       end