Streamline login for single provider instances (#2720)

app/controllers/admin_controller.rb

@@ -75,6 +75,7 @@ def update_features_enabled
     end
 
     Seek::Config.omniauth_enabled = string_to_boolean params[:omniauth_enabled]
+    Seek::Config.omniauth_skip_login_page = string_to_boolean params[:omniauth_skip_login_page]
     Seek::Config.standard_login_enabled = string_to_boolean params[:standard_login_enabled]
     Seek::Config.omniauth_user_create = string_to_boolean params[:omniauth_user_create]
     Seek::Config.omniauth_user_activate = string_to_boolean params[:omniauth_user_activate]

app/controllers/users_controller.rb

@@ -1,6 +1,7 @@
 class UsersController < ApplicationController
   before_action :is_current_user_auth, only: %i[edit update]
   before_action :is_user_admin_auth, only: %i[impersonate resend_activation_email destroy activate_other]
+  before_action :redirect_if_registration_disabled, only: :create
 
   skip_before_action :project_membership_required
 
@@ -197,6 +198,13 @@ def user_params
     params.require(:user).permit(permitted_params)
   end
 
+  def redirect_if_registration_disabled
+    return unless Seek::Config.registration_disabled
+
+    flash[:error] = Seek::Config.registration_disabled_description
+    redirect_to main_app.root_path
+  end
+
   def check_registration
     if @user.save
       successful_registration

app/helpers/sessions_helper.rb

@@ -1,4 +1,11 @@
 module SessionsHelper
+  LOGIN_STRATEGIES = %i[password elixir_aai ldap github oidc].freeze
+
+  # strategies with a form to fill in, rather than sending the user out to the provider
+  FORM_LOGIN_STRATEGIES = %i[password ldap].freeze
+
+  REDIRECTING_LOGIN_STRATEGIES = (LOGIN_STRATEGIES - FORM_LOGIN_STRATEGIES).freeze
+
   # a person can be logged in but not fully registered during
   # the registration process whilst selecting or creating a profile
   def logged_in_and_registered?
@@ -11,13 +18,27 @@ def admin_logged_in?
   end
 
   def detect_default_login_strategy
-    return 'password' if show_standard_password_login?
-    return 'elixir_aai' if show_elixir_aai_login?
-    return 'ldap' if show_ldap_login?
-    return 'github' if show_github_login?
-    return 'oidc' if show_oidc_login?
+    available_login_strategies.first&.to_s
+  end
+
+  # the login strategies currently available, in order of preference
+  def available_login_strategies
+    LOGIN_STRATEGIES.select { |strategy| send("show_#{strategy}_login?") }
+  end
+
+  # the only way to log in, when that is a provider the user can be sent straight to
+  def sole_redirecting_login_strategy
+    strategies = available_login_strategies
+    strategies.first if strategies.one? && REDIRECTING_LOGIN_STRATEGIES.include?(strategies.first)
+  end
+
+  # the provider to send the user straight to, skipping the login page altogether.
+  # the strategy and error checks stop a failed login bouncing straight back to the provider.
+  def auto_login_strategy
+    return unless Seek::Config.omniauth_skip_login_page
+    return if params[:strategy].present? || flash[:error].present?
 
-    nil
+    sole_redirecting_login_strategy
   end
 
   # returns true if there is somebody logged in and they are an project manager
@@ -42,7 +63,7 @@ def logged_in_and_member?
     User.logged_in_and_member?
   end
 
-  def show_standard_password_login?
+  def show_password_login?
     # always show if omniauth options aren't available, regardless of standard_login_enabled setting
     params[:show_standard_login].present? || Seek::Config.standard_login_enabled || !show_omniauth_login?
   end

app/views/admin/_omniauth.html.erb

@@ -6,6 +6,10 @@
                              "Standard login enabled", "If disabled the standard username and password login will be hidden, forcing users to use an alternative provider.
                              It is only hidden if an alternative provider has been configured and enabled below.
                              <strong>WARNING:</strong>To avoid an administrator being locked out, the standard login option can always be displayed by including the special parameter <i>show_standard_login=true</i>, i.e: #{login_url(show_standard_login: true, host: Seek::Config.site_base_host)}") %>
+  <%= admin_checkbox_setting(:omniauth_skip_login_page, 1, Seek::Config.omniauth_skip_login_page,
+                             "Skip the login page", "When a single provider is the only way to log in, send the user straight to it rather than
+                             showing a login page containing one button. It has no effect while there is more than one option, or when the only
+                             option needs credentials entering in #{Seek::Config.instance_name} (LDAP, or the standard login).") %>
   <%= admin_checkbox_setting(:omniauth_user_create, 1, Seek::Config.omniauth_user_create,
                              "Omniauth user creation on login", "When a user logs in through an omniauth provider and does not exist as a #{Seek::Config.instance_name} user, they will be created using the information given by the provider.") %>
   <%= admin_checkbox_setting(:omniauth_user_activate, 1, Seek::Config.omniauth_user_activate,

app/views/gadgets/_sign_in.html.erb

@@ -13,7 +13,7 @@
         <%# tabs if omniauth authentication with providers is enabled %>
         <% if show_omniauth_login? %>
           <ul class="nav nav-tabs" role="tablist">
-            <% if show_standard_password_login? %>
+            <% if show_password_login? %>
               <%= content_tag(:li, role: 'presentation', class: strategy == 'password' ? 'active' : nil) do %>
                 <a href="#password_login" aria-controls="password_login" role="tab" data-toggle="tab">SEEK login</a>
               <% end %>
@@ -42,7 +42,7 @@
         <% end #omniauth enabled and providers available %>
 
         <div class="tab-content">
-          <% if show_standard_password_login? %>
+          <% if show_password_login? %>
             <%= content_tag(:div, id: 'password_login', role: 'tabpanel', class: strategy == 'password' ? 'tab-pane active' : 'tab-pane') do %>
               <%= form_tag main_app.session_path do %>
                 <%= hidden_field_tag "called_from[path]", original_path -%>
@@ -103,10 +103,15 @@
         </div>
       </div>
 
-      <div class="panel-footer">
-        <%= link_to "Register an account", signup_path %> or
-        <%= link_to "Forgotten your password?", forgot_password_path %>
-      </div>
+      <% show_registration_link = !Seek::Config.registration_disabled %>
+      <% show_forgotten_password_link = show_password_login? %>
+      <% if show_registration_link || show_forgotten_password_link %>
+        <div class="panel-footer">
+          <%= link_to "Register an account", signup_path if show_registration_link %>
+          <%= "or" if show_registration_link && show_forgotten_password_link %>
+          <%= link_to "Forgotten your password?", forgot_password_path if show_forgotten_password_link %>
+        </div>
+      <% end %>
 
     </div>
 <% end -%>

app/views/homes/_home_features.html.erb

@@ -28,6 +28,8 @@
   </div>
   <div class="clearfix">
     <%= link_to 'Learn more', Seek::Config.instance_link, target: '_blank', rel: 'noopener', class: 'btn btn-primary btn-lg pull-left' %>
-    <%= link_to 'Register', signup_path, class: 'btn btn-primary btn-lg pull-right' %>
+    <% unless Seek::Config.registration_disabled %>
+      <%= link_to 'Register', signup_path, class: 'btn btn-primary btn-lg pull-right' %>
+    <% end %>
   </div>
 </div>

app/views/layouts/navbar/_navbar.html.erb

@@ -33,7 +33,9 @@
         <% if logged_in_and_registered? %>
             <%= render :partial => "layouts/navbar/user_menu" %>
         <% else %>
-            <li><%= link_to 'Register', signup_path %></li>
+            <% unless Seek::Config.registration_disabled %>
+                <li><%= link_to 'Register', signup_path %></li>
+            <% end %>
             <li><%= link_to 'Log in', login_path(:return_to => params[:return_to] || request.original_fullpath) %></li>
         <% end %>
       </ul>

app/views/sessions/_auto_login.html.erb

@@ -0,0 +1,11 @@
+<% original_path ||= request.original_fullpath %>
+<div class="text-center" id="auto-login">
+  <p>Redirecting to <%= omniauth_method_name(strategy) %>&hellip;</p>
+  <%= form_tag omniauth_authorize_path(strategy, state: "return_to:#{original_path}"), id: 'auto-login-form' do %>
+    <%= submit_tag "Sign in with #{omniauth_method_name(strategy)}", class: 'btn btn-primary', id: 'auto_login_button' %>
+  <% end %>
+</div>
+
+<script type="text/javascript">
+    document.getElementById('auto-login-form').submit();
+</script>

app/views/sessions/new.html.erb

@@ -4,6 +4,11 @@
     <p class="text-center"><%= link_to "Sign out", logout_url %></p>
 <% else %>
     <div class="container">
-        <%= render :partial => "gadgets/sign_in", :locals => {:original_path => params[:return_to]} %>
+      <% if (auto_strategy = auto_login_strategy) %>
+          <%= render partial: 'sessions/auto_login',
+                     locals: { strategy: auto_strategy, original_path: params[:return_to] } %>
+      <% else %>
+          <%= render :partial => "gadgets/sign_in", :locals => {:original_path => params[:return_to]} %>
+      <% end %>
     </div>
 <% end %>

app/views/users/new.html.erb

@@ -29,7 +29,7 @@
     <div class="panel-body">
       <% if show_omniauth_login? %>
         <ul class="nav nav-tabs" role="tablist">
-          <% if show_standard_password_login? %>
+          <% if show_password_login? %>
             <%= content_tag(:li, role: 'presentation', class: strategy == 'password' ? 'active' : nil) do %>
               <a href="#password_registration" aria-controls="password_registration" role="tab" data-toggle="tab">SEEK login</a>
             <% end %>
@@ -58,7 +58,7 @@
       <% end #omniauth enabled and providers available %>
 
       <div class="tab-content">
-        <% if show_standard_password_login? %>
+        <% if show_password_login? %>
           <%= content_tag(:div, id: 'password_registration', role: 'tabpanel', class: strategy == 'password' ? 'tab-pane active' : 'tab-pane') do %>
             <%= form_for @user do |f| -%>
 

config/initializers/seek_configuration.rb

@@ -229,6 +229,7 @@ def load_seek_config_defaults!
 
   # omniauth settings and behaviour
   Seek::Config.default :omniauth_enabled, false
+  Seek::Config.default :omniauth_skip_login_page, false
   Seek::Config.default :omniauth_user_create, true
   Seek::Config.default :omniauth_user_activate, true
   Seek::Config.default :omniauth_elixir_aai_enabled, false

lib/seek/config_setting_attributes.yml

@@ -225,6 +225,7 @@ placeholders_enabled:
 standard_login_enabled:
 # Omniauth
 omniauth_enabled:
+omniauth_skip_login_page:
 omniauth_user_create:
 omniauth_user_activate:
 omniauth_elixir_aai_enabled:

test/functional/homes_controller_test.rb

@@ -813,4 +813,19 @@ def reddit_feed_url
     end
   end
 
+
+  test 'should hide the register button when registration is disabled' do
+    with_config_value(:home_show_features, true) do
+      get :index
+      assert_response :success
+      assert_select '#home-features a[href=?]', signup_path, 1
+
+      with_config_value(:registration_disabled, true) do
+        get :index
+        assert_response :success
+        assert_select '#home-features a[href=?]', signup_path, count: 0
+      end
+    end
+  end
+
 end

test/functional/sessions_controller_test.rb

@@ -305,6 +305,114 @@ class SessionsControllerTest < ActionController::TestCase
     end
   end
 
+  test 'should skip the login page when there is a single omniauth provider' do
+    with_config_values(omniauth_skip_login_page: true, standard_login_enabled: false, omniauth_enabled: true,
+                       omniauth_ldap_enabled: false, omniauth_github_enabled: false,
+                       omniauth_elixir_aai_enabled: false, omniauth_oidc_enabled: true) do
+      get :new
+      assert_response :success
+      assert_select '#login-panel', count: 0
+      assert_select 'form#auto-login-form[action^=?]', '/auth/oidc'
+      assert_select 'form#auto-login-form input#auto_login_button[value=?]', 'Sign in with SEEK Testing OIDC'
+    end
+  end
+
+  test 'should not skip the login page by default' do
+    with_config_values(standard_login_enabled: false, omniauth_enabled: true,
+                       omniauth_ldap_enabled: false, omniauth_github_enabled: false,
+                       omniauth_elixir_aai_enabled: false, omniauth_oidc_enabled: true) do
+      refute Seek::Config.omniauth_skip_login_page
+      get :new
+      assert_response :success
+      assert_select 'form#auto-login-form', count: 0
+      assert_select '#oidc_login a', 1
+    end
+  end
+
+  test 'should not skip the login page when there is more than one login option' do
+    with_config_values(omniauth_skip_login_page: true, standard_login_enabled: true, omniauth_enabled: true,
+                       omniauth_ldap_enabled: false, omniauth_github_enabled: false,
+                       omniauth_elixir_aai_enabled: false, omniauth_oidc_enabled: true) do
+      get :new
+      assert_response :success
+      assert_select 'form#auto-login-form', count: 0
+      assert_select '#login-panel', 1
+      assert_select '#oidc_login a', 1
+    end
+  end
+
+  test 'should not skip the login page for a provider with its own form' do
+    with_config_values(omniauth_skip_login_page: true, standard_login_enabled: false, omniauth_enabled: true,
+                       omniauth_ldap_enabled: true, omniauth_github_enabled: false,
+                       omniauth_elixir_aai_enabled: false, omniauth_oidc_enabled: false) do
+      get :new
+      assert_response :success
+      assert_select 'form#auto-login-form', count: 0
+      assert_select '#ldap_login input[name="username"]', 1
+    end
+  end
+
+  test 'should not skip the login page when a strategy is requested' do
+    with_config_values(omniauth_skip_login_page: true, standard_login_enabled: false, omniauth_enabled: true,
+                       omniauth_ldap_enabled: false, omniauth_github_enabled: false,
+                       omniauth_elixir_aai_enabled: false, omniauth_oidc_enabled: true) do
+      get :new, params: { strategy: 'oidc' }
+      assert_response :success
+      assert_select 'form#auto-login-form', count: 0
+      assert_select '#oidc_login a', 1
+    end
+  end
+
+  test 'should not skip the login page after a failed login' do
+    with_config_values(omniauth_skip_login_page: true, standard_login_enabled: false, omniauth_enabled: true,
+                       omniauth_ldap_enabled: false, omniauth_github_enabled: false,
+                       omniauth_elixir_aai_enabled: false, omniauth_oidc_enabled: true) do
+      get :new, flash: { error: 'Something went wrong' }
+      assert_response :success
+      assert_select 'form#auto-login-form', count: 0
+      assert_select '#oidc_login a', 1
+    end
+  end
+
+  test 'should still reach the password form when skipping the login page' do
+    with_config_values(omniauth_skip_login_page: true, standard_login_enabled: false, omniauth_enabled: true,
+                       omniauth_ldap_enabled: false, omniauth_github_enabled: false,
+                       omniauth_elixir_aai_enabled: false, omniauth_oidc_enabled: true) do
+      get :new, params: { show_standard_login: true }
+      assert_response :success
+      assert_select 'form#auto-login-form', count: 0
+      assert_select 'div.tab-content div#password_login', 1
+    end
+  end
+
+  test 'should hide registration and password reset links when unavailable' do
+    FactoryBot.create(:user)
+
+    get :new
+    assert_select '.panel-footer a[href=?]', signup_path, 1
+    assert_select '.panel-footer a[href=?]', forgot_password_path, 1
+
+    with_config_value(:registration_disabled, true) do
+      get :new
+      assert_select '.panel-footer a[href=?]', signup_path, count: 0
+      assert_select '.panel-footer a[href=?]', forgot_password_path, 1
+    end
+
+    # no SEEK password to reset when standard login is unavailable
+    with_config_values(standard_login_enabled: false, omniauth_enabled: true,
+                       omniauth_ldap_enabled: true, omniauth_github_enabled: false,
+                       omniauth_elixir_aai_enabled: false, omniauth_oidc_enabled: true) do
+      get :new
+      assert_select '.panel-footer a[href=?]', signup_path, 1
+      assert_select '.panel-footer a[href=?]', forgot_password_path, count: 0
+
+      with_config_value(:registration_disabled, true) do
+        get :new
+        assert_select '.panel-footer', count: 0
+      end
+    end
+  end
+
   protected
 
   def cookie_for(user)