From aae39003095049f3a681b2ae4e20eeec4b60b7c8 Mon Sep 17 00:00:00 2001
From: Yetrina Battad <hello@yetti.io>
Date: Thu, 12 Oct 2023 16:04:40 +1100
Subject: [PATCH 1/8] feat: change patron login to Keycloak via OmniAuth

---
 .../concerns/auth_session_concern.rb          |  9 ++
 .../users/omniauth_callbacks_controller.rb    |  7 ++
 app/controllers/users/sessions_controller.rb  | 43 ++--------
 app/models/user.rb                            | 45 +++++++---
 app/views/users/sessions/new.html.erb         | 18 +---
 config/initializers/devise.rb                 | 62 ++------------
 config/initializers/omniauth.rb               |  8 ++
 config/locales/auth.en.yml                    |  1 +
 config/routes.rb                              |  2 +
 .../app/controllers/application_controller.rb |  1 +
 spec/dummy/app/views/pages/about.html.erb     |  4 +-
 spec/dummy/app/views/pages/home.html.erb      |  4 +
 spec/factories/users.rb                       |  7 +-
 spec/features/login_spec.rb                   | 84 ++++++++-----------
 spec/features/staff_login_spec.rb             | 59 -------------
 .../{staff_logout_spec.rb => logout_spec.rb}  | 12 +--
 16 files changed, 133 insertions(+), 233 deletions(-)
 create mode 100644 app/controllers/concerns/auth_session_concern.rb
 delete mode 100644 spec/features/staff_login_spec.rb
 rename spec/requests/{staff_logout_spec.rb => logout_spec.rb} (88%)

diff --git a/app/controllers/concerns/auth_session_concern.rb b/app/controllers/concerns/auth_session_concern.rb
new file mode 100644
index 0000000..8836075
--- /dev/null
+++ b/app/controllers/concerns/auth_session_concern.rb
@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+module AuthSessionConcern
+  extend ActiveSupport::Concern
+
+  def new_session_path(_scope)
+    new_user_session_path
+  end
+end
diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb
index 6ab37e2..ed93dfb 100644
--- a/app/controllers/users/omniauth_callbacks_controller.rb
+++ b/app/controllers/users/omniauth_callbacks_controller.rb
@@ -22,6 +22,13 @@ def catalogue_shared
     sign_in_and_redirect @user, event: :authentication
   end
 
+  def catalogue_patron
+    Rails.logger.debug(request.env["omniauth.auth"])
+    @user = User.from_keycloak_patron(request.env["omniauth.auth"])
+    store_keycloak_data(request.env["omniauth.auth"])
+    sign_in_and_redirect @user, event: :authentication
+  end
+
   # Keycloak will display its own error page when there is a failure to login.
   # :nocov:
   def failure
diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb
index 86beef6..61f580a 100644
--- a/app/controllers/users/sessions_controller.rb
+++ b/app/controllers/users/sessions_controller.rb
@@ -1,23 +1,17 @@
 # frozen_string_literal: true
 
 class Users::SessionsController < Devise::SessionsController
-  before_action :configure_sign_in_params, only: [:create]
-
   skip_before_action :verify_authenticity_token, only: [:backchannel_logout]
 
-  def create
-    super
-  end
-
   def destroy
-    if session[:iss].present?
-      # Keycloak logout. Keycloak will send a POST to "/devise_logout" to perform a
-      # backchannel logout that terminates the Devise session.
-      keycloak_logout
-    else
-      # There is no Keycloak session identifier, so destroy the Devise session.
-      devise_logout
-    end
+    # Keycloak logout. Keycloak will send a POST to "/backchannel_logout" to perform a
+    # backchannel logout that terminates the Devise session.
+    iss = session[:iss]
+    id_token = session[:id_token]
+
+    signed_out = (Devise.sign_out_all_scopes ? sign_out : sign_out(resource_name))
+    set_flash_message! :notice, :signed_out if signed_out
+    redirect_to("#{iss}/protocol/openid-connect/logout?id_token_hint=#{id_token}&post_logout_redirect_uri=#{root_url}", allow_other_host: true)
   end
 
   def backchannel_logout
@@ -30,25 +24,4 @@ def backchannel_logout
     user = User.find_by(session_token: session_id)
     user.update_column(:session_token, SecureRandom.hex)
   end
-
-  protected
-
-  def devise_logout
-    signed_out = (Devise.sign_out_all_scopes ? sign_out : sign_out(resource_name))
-    set_flash_message! :notice, :signed_out if signed_out
-    respond_to_on_destroy
-  end
-
-  def keycloak_logout
-    iss = session[:iss]
-    id_token = session[:id_token]
-
-    signed_out = (Devise.sign_out_all_scopes ? sign_out : sign_out(resource_name))
-    set_flash_message! :notice, :signed_out if signed_out
-    redirect_to("#{iss}/protocol/openid-connect/logout?id_token_hint=#{id_token}&post_logout_redirect_uri=#{root_url}", allow_other_host: true)
-  end
-
-  def configure_sign_in_params
-    devise_parameter_sanitizer.permit(:sign_in, keys: [user: [:username, :password]])
-  end
 end
diff --git a/app/models/user.rb b/app/models/user.rb
index 74c2492..b08ff6e 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -31,8 +31,7 @@ class User < PatronsRecord
 
   # Include default devise modules. Others available are:
   # :confirmable, :lockable, :timeoutable, :trackable and :omniauthable
-  devise :user_reg_authenticatable, :timeoutable,
-    :omniauthable, omniauth_providers: %i[catalogue_sol catalogue_spl catalogue_shared]
+  devise :timeoutable, :omniauthable, omniauth_providers: %i[catalogue_patron catalogue_sol catalogue_spl catalogue_shared]
 
   attr_accessor :username, :password, :session_token
 
@@ -68,20 +67,44 @@ def self.from_keycloak(auth)
     end
   end
 
+  def self.from_keycloak_patron(auth)
+    ActiveRecord::Base.transaction do
+      user = find_or_create_by!(folio_id: auth.extra.raw_info.preferred_username) do |user|
+        # We don't really care about the password since auth is via Keycloak, so we're just
+        # putting a dummy value here.
+        user.encrypted_password = SecureRandom.hex(14)
+      end
+      # set/update values from Keycloak in case they've changed
+      user.email = auth.info.email.present? ? auth.info.email : ""
+      user.provider = auth.provider
+      user.uid = auth.uid
+      user.name_given = auth.info.first_name
+      user.name_family = auth.info.last_name
+
+      # TODO: send request to catalogue services to determine active state of user account in FOLIO
+
+      # this is required for backchannel logout
+      user.session_token = auth.extra.raw_info.sid
+
+      # reload user with updated values from database
+      user.save!
+      user.reload
+    end
+  end
+
   # Method added by Blacklight; Blacklight uses #to_s on your
   # user class to get a user-displayable login/identifier for
   # the account.
   def to_s
     name = "#{name_given} #{name_family}"
-    if provider.present?
-      name = if provider == "catalogue_sol"
-        "#{name} (SOL)"
-      elsif provider == "catalogue_spl"
-        "#{name} (SPL)"
-      elsif provider == "catalogue_shared"
-        "#{name} (TOL)"
-      end
+    if provider == "catalogue_sol"
+      "#{name} (SOL)"
+    elsif provider == "catalogue_spl"
+      "#{name} (SPL)"
+    elsif provider == "catalogue_shared"
+      "#{name} (TOL)"
+    else
+      name
     end
-    name
   end
 end
diff --git a/app/views/users/sessions/new.html.erb b/app/views/users/sessions/new.html.erb
index 4c455c3..f4910ad 100644
--- a/app/views/users/sessions/new.html.erb
+++ b/app/views/users/sessions/new.html.erb
@@ -1,21 +1,5 @@
 <h1>Login</h1>
 
-<%= form_for(resource, as: resource_name, html: {'data-turbo' => "false"}, url: session_path(resource_name)) do |f| %>
-  <div class="field form-group">
-    <%= f.label :username, "User ID" %>
-    <%= f.text_field :username, autofocus: true, autocomplete: "username",class: "form-control col-md-4" %>
-  </div>
-
-  <div class="field form-group">
-    <%= f.label :password, "Family Name" %>
-    <%= f.text_field :password, autocomplete: "off",class: "form-control col-md-4" %>
-  </div>
-
-  <p><%= t("devise.registrations.register", url: ENV["NATIONAL_LIBRARY_CARD_URL"]).html_safe %></p>
-
-  <div class="actions">
-    <%= f.submit "Login", class: "btn btn-primary" %>
-  </div>
-<% end %>
+<%= button_to t("auth.patron_login"), user_catalogue_patron_omniauth_authorize_path, data: { turbo: false } %>
 
 <%= render "users/shared/links" %>
diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb
index bb5eaf5..e9bb118 100644
--- a/config/initializers/devise.rb
+++ b/config/initializers/devise.rb
@@ -1,49 +1,5 @@
 # frozen_string_literal: true
 
-class CatalogueFailureApp < Devise::FailureApp
-  def recall
-    header_info = if relative_url_root?
-      base_path = Pathname.new(relative_url_root)
-      full_path = Pathname.new(attempted_path)
-
-      {"SCRIPT_NAME" => relative_url_root,
-       "PATH_INFO" => "/" + full_path.relative_path_from(base_path).to_s}
-    else
-      {"PATH_INFO" => attempted_path}
-    end
-
-    header_info.each do |var, value|
-      if request.respond_to?(:set_header)
-        request.set_header(var, value)
-      else
-        request.env[var] = value
-      end
-    end
-
-    message = I18n.t("devise.failure.no_credentials", url: ENV["ASK_LIBRARIAN_URL"]).html_safe
-    flash.now[:alert] = i18n_message(message) if is_flashing_format?
-    self.response = recall_app(warden_options[:recall]).call(request.env).tap { |response|
-      response[0] = Rack::Utils.status_code(
-        response[0].in?(300..399) ? Devise.responder.redirect_status : Devise.responder.error_status
-      )
-    }
-  end
-end
-
-Devise.add_module(:getalibrarycard_authenticatable, {
-  strategy: true,
-  controller: :sessions,
-  model: "devise/models/getalibrarycard_authenticatable",
-  route: :session
-})
-
-Devise.add_module(:user_reg_authenticatable, {
-  strategy: true,
-  controller: :sessions,
-  model: "devise/models/user_reg_authenticatable",
-  route: :session
-})
-
 # Assuming you have not yet modified this file, each configuration option below
 # is set to its default value. Note that some are commented out while others
 # are not: uncommented lines are intended to protect your configuration from
@@ -58,7 +14,7 @@ def recall
   # confirmation, reset password and unlock tokens in the database.
   # Devise will use the `secret_key_base` as its `secret_key`
   # by default. You can change it below and use your own secret key.
-  # config.secret_key = '0c97e745c71f8e98a4795c38e08c1099147b591da8cd41ffcef1591b0d842f99e77f02b72e0cac0fe3c9d69bb9cbba43d4ccc6f6901089536ee1a48b953eba08'
+  # config.secret_key = 'b0381e2e65fbb4ae91e67940ada6f0798d37dfed9e0cc74f5aa16587f6c6f5a2cc41d7ab86d5908f5eb67e5c481bc9ab86b13f01fc63bf02803526a5268ede3b'
 
   # ==> Controller configuration
   # Configure the parent class to the devise controllers.
@@ -90,7 +46,7 @@ def recall
   # session. If you need permissions, you should implement that in a before filter.
   # You can also supply a hash where the value is a boolean determining whether
   # or not authentication should be aborted when the value is not present.
-  config.authentication_keys = {username: true, password: true}
+  # config.authentication_keys = [:email]
 
   # Configure parameters from the request object used for authentication. Each entry
   # given should be a request method and it will automatically be passed to the
@@ -107,7 +63,7 @@ def recall
   # Configure which authentication keys should have whitespace stripped.
   # These keys will have whitespace before and after removed upon creating or
   # modifying a user and when used to authenticate or find a user. Default is :email.
-  config.strip_whitespace_keys = [:username, :password]
+  config.strip_whitespace_keys = [:email]
 
   # Tell if authentication through request.params is enabled. True by default.
   # It can be set to an array that will enable params authentication only for the
@@ -170,7 +126,7 @@ def recall
   config.stretches = Rails.env.test? ? 1 : 12
 
   # Set up a pepper to generate the hashed password.
-  # config.pepper = 'c9ee4c0658e1c6d2b8d347958245cfe5040389fcf78ed6a71db76ee4be52802ee17f3d6e18d4d431522ba2330b5d25a4a16b13680b5689ad61902c4b7b0fd000'
+  # config.pepper = 'a4d2d0b4d065678ac151abcc34342a4af2086f5b45ff704dc2bcef23efe9bc6b5a1bb8b85acffbadaafef827eec324e9748a6d0b811980a29b870c79ac7b7bd1'
 
   # Send a notification to the original email when the user's email is changed.
   # config.send_email_changed_notification = false
@@ -310,7 +266,7 @@ def recall
   # config.navigational_formats = ['*/*', :html, :turbo_stream]
 
   # The default HTTP method used to sign out a resource. Default is :delete.
-  # config.sign_out_via = :delete
+  config.sign_out_via = :delete
 
   # ==> OmniAuth
   # Add a new OmniAuth provider. Check the wiki for more information on setting
@@ -321,10 +277,10 @@ def recall
   # If you want to use other strategies, that are not supported by Devise, or
   # change the failure app, you can configure them inside the config.warden block.
   #
-  config.warden do |manager|
-    manager.failure_app = CatalogueFailureApp
-    manager.default_strategies(scope: :user).unshift :user_reg_authenticatable
-  end
+  # config.warden do |manager|
+  #   manager.intercept_401 = false
+  #   manager.default_strategies(scope: :user).unshift :some_external_strategy
+  # end
 
   # ==> Mountable engine configurations
   # When using Devise inside an engine, let's call it `MyEngine`, and this engine
diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb
index b47638d..876bb45 100644
--- a/config/initializers/omniauth.rb
+++ b/config/initializers/omniauth.rb
@@ -22,4 +22,12 @@
     strategy_class: OmniAuth::Strategies::KeycloakOpenId,
     authorize_params: {scope: "openid"},
     name: "catalogue_shared"
+
+  provider :keycloak_openid,
+    ENV.fetch("KC_PATRON_CLIENT", "patron"),
+    ENV.fetch("KC_PATRON_SECRET", "default secret"),
+    client_options: {site: ENV.fetch("KEYCLOAK_URL", "http://localhost:9090"), realm: ENV.fetch("KC_PATRON_REALM", "nla-patron")},
+    strategy_class: OmniAuth::Strategies::KeycloakOpenId,
+    authorize_params: {scope: "openid"},
+    name: "catalogue_patron"
 end
diff --git a/config/locales/auth.en.yml b/config/locales/auth.en.yml
index ff75b21..74ea6e4 100644
--- a/config/locales/auth.en.yml
+++ b/config/locales/auth.en.yml
@@ -6,3 +6,4 @@ en:
       sol_login: "Staff Official Loan"
       spl_login: "Staff Personal Loan"
       shared_login: "Team Official Loan"
+    patron_login: "Patron Login"
diff --git a/config/routes.rb b/config/routes.rb
index 8d14baf..03f8b4e 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -51,6 +51,8 @@
   }
 
   devise_scope(:user) do
+    get "sign_in", to: "users/sessions#new", as: :new_user_session
+    delete "sign_out", to: "users/sessions#destroy", as: :destroy_user_session
     post "/backchannel_logout", to: "users/sessions#backchannel_logout", as: :backchannel_logout
   end
 end
diff --git a/spec/dummy/app/controllers/application_controller.rb b/spec/dummy/app/controllers/application_controller.rb
index 1652e22..d249f90 100644
--- a/spec/dummy/app/controllers/application_controller.rb
+++ b/spec/dummy/app/controllers/application_controller.rb
@@ -1,3 +1,4 @@
 class ApplicationController < ActionController::Base
   # Attempt to find the mapped route for devise based on request path
+  include AuthSessionConcern
 end
diff --git a/spec/dummy/app/views/pages/about.html.erb b/spec/dummy/app/views/pages/about.html.erb
index 3453cf2..56bfe1f 100644
--- a/spec/dummy/app/views/pages/about.html.erb
+++ b/spec/dummy/app/views/pages/about.html.erb
@@ -1,2 +1,2 @@
-<h1>Pages#home</h1>
-<p>Find me in app/views/pages/home.html.erb</p>
+<h1>Pages#about</h1>
+<p>Find me in app/views/pages/about.html.erb</p>
diff --git a/spec/dummy/app/views/pages/home.html.erb b/spec/dummy/app/views/pages/home.html.erb
index 3453cf2..f9d09c4 100644
--- a/spec/dummy/app/views/pages/home.html.erb
+++ b/spec/dummy/app/views/pages/home.html.erb
@@ -1,2 +1,6 @@
 <h1>Pages#home</h1>
 <p>Find me in app/views/pages/home.html.erb</p>
+
+<% if current_user.present? %>
+Logged in as <%= current_user %>
+<% end %>
diff --git a/spec/factories/users.rb b/spec/factories/users.rb
index 9701389..20ae1eb 100644
--- a/spec/factories/users.rb
+++ b/spec/factories/users.rb
@@ -28,17 +28,18 @@
   factory :user do
     sequence(:email) { |n| "test-#{n.to_s.rjust(3, "0")}@example.com" }
     password { "123456" }
-    folio_id { SecureRandom.uuid }
+    uid { SecureRandom.uuid }
+    provider { "catalogue_patron" }
     name_given { "Test" }
     name_family { "User" }
+    session_token { SecureRandom.hex }
 
     trait :staff do
       provider { "catalogue_sol" }
       uid { "603e26dd-b2d4-4a88-ad9d-406eaec31463" }
       name_given { "Staff" }
       name_family { "User" }
-      email { "staff@nla.gov.au" }
-      session_token { SecureRandom.hex }
+      sequence(:email) { |n| "staff-#{n.to_s.rjust(3, "0")}@nla.gov.au" }
     end
 
     created_at { Time.current }
diff --git a/spec/features/login_spec.rb b/spec/features/login_spec.rb
index 5dffe14..82a2d93 100644
--- a/spec/features/login_spec.rb
+++ b/spec/features/login_spec.rb
@@ -3,73 +3,63 @@
 require "rails_helper"
 
 RSpec.describe "Login" do
-  before do
-    # rubocop:disable RSpec/AnyInstance
-    allow_any_instance_of(PatronHelper).to receive(:user_location).and_return(:offsite)
-    # rubocop:enable RSpec/AnyInstance
-  end
-
-  it "creates a new session for the user" do
+  it "displays the patron login link" do
     visit root_path
     click_link "Login"
-    expect(page).to have_content("Login")
-
-    fill_in "user_username", with: "bltest"
-    fill_in "user_password", with: "test"
-    click_button "Login"
-
-    expect(page).to have_content(I18n.t("devise.sessions.signed_in"))
-    expect(page).to have_content("blacklight test")
+    expect(page).to have_content(I18n.t("auth.patron_login"))
   end
 
-  it "displays a registration link" do
-    visit new_user_session_path
-
-    expect(page).to have_link("here", href: ENV["NATIONAL_LIBRARY_CARD_URL"])
-  end
-
-  it "disables Turbo" do
-    visit new_user_session_path
-
-    expect(page).to have_css("form[data-turbo]")
-  end
+  context "when in public network" do
+    # rubocop:disable RSpec/AnyInstance
+    before do
+      allow_any_instance_of(PatronHelper).to receive(:user_location).and_return(:offsite)
+    end
+    # rubocop:enable RSpec/AnyInstance
 
-  context "when user is inactive" do
-    it "displays an error message" do
+    it "does not display staff login links" do
       visit root_path
       click_link "Login"
       expect(page).to have_content("Login")
 
-      fill_in "user_username", with: "bltest"
-      fill_in "user_password", with: "blacklight"
-      click_button "Login"
-
-      expect(page).to have_content(I18n.t("devise.failure.expired"))
-      expect(page).to have_content("Login")
+      expect(page).not_to have_content(I18n.t("auth.staff.sol_login"))
+      expect(page).not_to have_content(I18n.t("auth.staff.spl_login"))
+      expect(page).not_to have_content(I18n.t("auth.staff.shared_login"))
     end
   end
 
-  context "when user has the wrong credentials" do
-    it "displays an error message" do
+  context "when inside local network" do
+    # rubocop:disable RSpec/AnyInstance
+    before do
+      allow_any_instance_of(PatronHelper).to receive(:user_location).and_return(:onsite)
+    end
+    # rubocop:enable RSpec/AnyInstance
+
+    it "does not display staff login links" do
       visit root_path
       click_link "Login"
       expect(page).to have_content("Login")
 
-      fill_in "user_username", with: "0000"
-      fill_in "user_password", with: "failure"
-      click_button "Login"
-
-      expect(page).to have_link("Ask a Librarian", href: ENV["ASK_LIBRARIAN_URL"])
-      expect(page).to have_content("Login")
+      expect(page).not_to have_content(I18n.t("auth.staff.sol_login"))
+      expect(page).not_to have_content(I18n.t("auth.staff.spl_login"))
+      expect(page).not_to have_content(I18n.t("auth.staff.shared_login"))
     end
   end
 
-  context "when user does not enter a username or password" do
-    it "displays an error message" do
-      visit new_user_session_path
-      click_button "Login"
+  context "when inside staff network" do
+    # rubocop:disable RSpec/AnyInstance
+    before do
+      allow_any_instance_of(PatronHelper).to receive(:user_location).and_return(:staff)
+    end
+    # rubocop:enable RSpec/AnyInstance
+
+    it "displays staff login links" do
+      visit root_path
+      click_link "Login"
+      expect(page).to have_content("Login")
 
-      expect(page).to have_link("Ask a Librarian", href: ENV["ASK_LIBRARIAN_URL"])
+      expect(page).to have_content(I18n.t("auth.staff.sol_login"))
+      expect(page).to have_content(I18n.t("auth.staff.spl_login"))
+      expect(page).to have_content(I18n.t("auth.staff.shared_login"))
     end
   end
 end
diff --git a/spec/features/staff_login_spec.rb b/spec/features/staff_login_spec.rb
deleted file mode 100644
index 51a6618..0000000
--- a/spec/features/staff_login_spec.rb
+++ /dev/null
@@ -1,59 +0,0 @@
-# frozen_string_literal: true
-
-require "rails_helper"
-
-RSpec.describe "Staff login" do
-  context "when in public network" do
-    # rubocop:disable RSpec/AnyInstance
-    before do
-      allow_any_instance_of(PatronHelper).to receive(:user_location).and_return(:offsite)
-    end
-    # rubocop:enable RSpec/AnyInstance
-
-    it "display login links" do
-      visit root_path
-      click_link "Login"
-      expect(page).to have_content("Login")
-
-      expect(page).not_to have_content(I18n.t("auth.staff.sol_login"))
-      expect(page).not_to have_content(I18n.t("auth.staff.spl_login"))
-      expect(page).not_to have_content(I18n.t("auth.staff.shared_login"))
-    end
-  end
-
-  context "when inside local network" do
-    # rubocop:disable RSpec/AnyInstance
-    before do
-      allow_any_instance_of(PatronHelper).to receive(:user_location).and_return(:onsite)
-    end
-    # rubocop:enable RSpec/AnyInstance
-
-    it "display login links" do
-      visit root_path
-      click_link "Login"
-      expect(page).to have_content("Login")
-
-      expect(page).not_to have_content(I18n.t("auth.staff.sol_login"))
-      expect(page).not_to have_content(I18n.t("auth.staff.spl_login"))
-      expect(page).not_to have_content(I18n.t("auth.staff.shared_login"))
-    end
-  end
-
-  context "when inside staff network" do
-    # rubocop:disable RSpec/AnyInstance
-    before do
-      allow_any_instance_of(PatronHelper).to receive(:user_location).and_return(:staff)
-    end
-    # rubocop:enable RSpec/AnyInstance
-
-    it "display login links" do
-      visit root_path
-      click_link "Login"
-      expect(page).to have_content("Login")
-
-      expect(page).to have_content(I18n.t("auth.staff.sol_login"))
-      expect(page).to have_content(I18n.t("auth.staff.spl_login"))
-      expect(page).to have_content(I18n.t("auth.staff.shared_login"))
-    end
-  end
-end
diff --git a/spec/requests/staff_logout_spec.rb b/spec/requests/logout_spec.rb
similarity index 88%
rename from spec/requests/staff_logout_spec.rb
rename to spec/requests/logout_spec.rb
index 6335a32..012ad8e 100644
--- a/spec/requests/staff_logout_spec.rb
+++ b/spec/requests/logout_spec.rb
@@ -2,15 +2,15 @@
 
 require "rails_helper"
 
-RSpec.describe "Staff logout" do
+RSpec.describe "Logout" do
   include Devise::Test::IntegrationHelpers
 
   let(:file) { IO.read("spec/files/auth/staff_auth_hash.json") }
   let(:auth_hash) { OmniAuth::AuthHash.new(JSON.parse(file)) }
 
   before do
-    staff = create(:user, :staff)
-    sign_in staff
+    patron = create(:user, :staff)
+    sign_in patron
   end
 
   it "redirects to Keycloak's logout URL" do
@@ -34,19 +34,19 @@
       jwt = JWT.decode(logout_token, nil, false)
       jwt[0]["sid"]
     end
-    let(:staff) do
+    let(:patron) do
       user = create(:user, :staff)
       user.update_column(:session_token, session_id)
       user.reload
     end
 
     before do
-      sign_in staff
+      sign_in patron
     end
 
     it "updates the session_token for the logged out user" do
       post "/backchannel_logout", params: {logout_token: logout_token}
-      expect(staff.session_token).not_to eq session_id
+      expect(patron.session_token).not_to eq session_id
     end
   end
 end

From 25e5a8f924ffbc743cd102aa9b82c7e4b6dcdc43 Mon Sep 17 00:00:00 2001
From: Yetrina Battad <hello@yetti.io>
Date: Thu, 12 Oct 2023 16:09:57 +1100
Subject: [PATCH 2/8] chore: update dependency versions

---
 Gemfile.lock | 48 ++++++++++++++++++++++++------------------------
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/Gemfile.lock b/Gemfile.lock
index 7f815c7..11b24e0 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -98,7 +98,7 @@ GEM
     base64 (0.1.1)
     bcrypt (3.1.19)
     bindata (2.4.15)
-    blacklight (7.33.1)
+    blacklight (7.34.0)
       deprecation
       globalid
       hashdiff
@@ -107,7 +107,7 @@ GEM
       kaminari (>= 0.15)
       ostruct (>= 0.3.2)
       rails (>= 5.1, < 7.1)
-      view_component (~> 2.66)
+      view_component (>= 2.66, < 4)
     brakeman (6.0.1)
     builder (3.2.4)
     bundler-audit (0.9.1)
@@ -134,7 +134,7 @@ GEM
       reline (>= 0.3.1)
     deprecation (1.1.0)
       activesupport
-    devise (4.9.2)
+    devise (4.9.3)
       bcrypt (~> 3.0)
       orm_adapter (~> 0.1)
       railties (>= 4.1.0)
@@ -199,7 +199,7 @@ GEM
     kaminari-core (1.2.2)
     language_server-protocol (3.17.0.3)
     lint_roller (1.1.0)
-    loofah (2.21.3)
+    loofah (2.21.4)
       crass (~> 1.0.2)
       nokogiri (>= 1.12.0)
     mail (2.8.1)
@@ -215,14 +215,14 @@ GEM
     multi_json (1.15.0)
     multi_xml (0.6.0)
     mysql2 (0.5.5)
-    net-imap (0.3.7)
+    net-imap (0.4.1)
       date
       net-protocol
     net-pop (0.1.2)
       net-protocol
     net-protocol (0.2.1)
       timeout
-    net-smtp (0.3.3)
+    net-smtp (0.4.0)
       net-protocol
     nio4r (2.5.9)
     nokogiri (1.15.4-arm64-darwin)
@@ -254,13 +254,13 @@ GEM
     orm_adapter (0.5.0)
     ostruct (0.5.5)
     parallel (1.23.0)
-    parser (3.2.2.3)
+    parser (3.2.2.4)
       ast (~> 2.4.1)
       racc
-    psych (5.1.0)
+    psych (5.1.1)
       stringio
     public_suffix (5.0.3)
-    puma (6.3.1)
+    puma (6.4.0)
       nio4r (~> 2.0)
     racc (1.7.1)
     rack (2.2.8)
@@ -306,10 +306,10 @@ GEM
     rake (13.0.6)
     rdoc (6.5.0)
       psych (>= 4.0.0)
-    regexp_parser (2.8.1)
-    reline (0.3.8)
+    regexp_parser (2.8.2)
+    reline (0.3.9)
       io-console (~> 0.5)
-    responders (3.1.0)
+    responders (3.1.1)
       actionpack (>= 5.2)
       railties (>= 5.2)
     rexml (3.2.6)
@@ -333,7 +333,7 @@ GEM
       rspec-mocks (~> 3.12)
       rspec-support (~> 3.12)
     rspec-support (3.12.1)
-    rubocop (1.56.3)
+    rubocop (1.56.4)
       base64 (~> 0.1.1)
       json (~> 2.3)
       language_server-protocol (>= 3.17.0)
@@ -354,18 +354,18 @@ GEM
     rubocop-performance (1.19.1)
       rubocop (>= 1.7.0, < 2.0)
       rubocop-ast (>= 0.4.0)
-    rubocop-rails (2.21.1)
+    rubocop-rails (2.21.2)
       activesupport (>= 4.2.0)
       rack (>= 1.1)
       rubocop (>= 1.33.0, < 2.0)
-    rubocop-rspec (2.24.0)
+    rubocop-rspec (2.24.1)
       rubocop (~> 1.33)
       rubocop-capybara (~> 2.17)
       rubocop-factory_bot (~> 2.22)
     ruby-progressbar (1.13.0)
     ruby2_keywords (0.0.5)
     rubyzip (2.3.2)
-    selenium-webdriver (4.12.0)
+    selenium-webdriver (4.14.0)
       rexml (~> 3.2, >= 3.2.5)
       rubyzip (>= 1.2.2, < 3.0)
       websocket (~> 1.0)
@@ -390,28 +390,28 @@ GEM
       actionpack (>= 5.2)
       activesupport (>= 5.2)
       sprockets (>= 3.0.0)
-    standard (1.31.1)
+    standard (1.31.2)
       language_server-protocol (~> 3.17.0.2)
       lint_roller (~> 1.0)
-      rubocop (~> 1.56.2)
+      rubocop (~> 1.56.4)
       standard-custom (~> 1.0.0)
       standard-performance (~> 1.2)
     standard-custom (1.0.2)
       lint_roller (~> 1.0)
       rubocop (~> 1.50)
-    standard-performance (1.2.0)
+    standard-performance (1.2.1)
       lint_roller (~> 1.1)
-      rubocop-performance (~> 1.19.0)
+      rubocop-performance (~> 1.19.1)
     stringio (3.0.8)
-    strong_migrations (1.6.2)
+    strong_migrations (1.6.3)
       activerecord (>= 5.2)
     thor (1.2.2)
     timeout (0.4.0)
     tzinfo (2.0.6)
       concurrent-ruby (~> 1.0)
-    unicode-display_width (2.4.2)
+    unicode-display_width (2.5.0)
     version_gem (1.1.3)
-    view_component (2.82.0)
+    view_component (3.6.0)
       activesupport (>= 5.2.0, < 8.0)
       concurrent-ruby (~> 1.0)
       method_source (~> 1.0)
@@ -427,7 +427,7 @@ GEM
     websocket-extensions (0.1.5)
     xpath (3.2.0)
       nokogiri (~> 1.8)
-    zeitwerk (2.6.11)
+    zeitwerk (2.6.12)
     zk (1.10.0)
       zookeeper (~> 1.5.0)
     zookeeper (1.5.5)

From b9f38b299625800db6d87e765847b19ccb9494a8 Mon Sep 17 00:00:00 2001
From: Yetrina Battad <hello@yetti.io>
Date: Thu, 12 Oct 2023 16:22:48 +1100
Subject: [PATCH 3/8] fix: restyle patron login button

---
 app/views/users/sessions/new.html.erb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/views/users/sessions/new.html.erb b/app/views/users/sessions/new.html.erb
index f4910ad..9bc2ed1 100644
--- a/app/views/users/sessions/new.html.erb
+++ b/app/views/users/sessions/new.html.erb
@@ -1,5 +1,5 @@
 <h1>Login</h1>
 
-<%= button_to t("auth.patron_login"), user_catalogue_patron_omniauth_authorize_path, data: { turbo: false } %>
+<%= button_to t("auth.patron_login"), user_catalogue_patron_omniauth_authorize_path, class: "btn btn-primary", data: { turbo: false } %>
 
 <%= render "users/shared/links" %>

From 71eea98f4dd051800d2875b0f39f91236647f89a Mon Sep 17 00:00:00 2001
From: Yetrina Battad <hello@yetti.io>
Date: Mon, 16 Oct 2023 08:17:00 +1100
Subject: [PATCH 4/8] chore: update dependencies

---
 Gemfile.lock | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Gemfile.lock b/Gemfile.lock
index 11b24e0..1aa2a8e 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -171,7 +171,7 @@ GEM
     i18n (1.14.1)
       concurrent-ruby (~> 1.0)
     io-console (0.6.0)
-    irb (1.8.1)
+    irb (1.8.3)
       rdoc
       reline (>= 0.3.8)
     jbuilder (2.11.5)

From 24c53d8e924338c94f0499481d4b4810c0de0313 Mon Sep 17 00:00:00 2001
From: Yetrina Battad <hello@yetti.io>
Date: Mon, 16 Oct 2023 10:38:04 +1100
Subject: [PATCH 5/8] test: fix tests config for OmniAuth

---
 .../app/views/shared/_user_links.html.erb     |  2 +-
 spec/features/active_link_spec.rb             |  6 ++--
 spec/features/logout_spec.rb                  | 24 ---------------
 spec/files/auth/auth_hash.json                | 30 +++++++++----------
 spec/simplecov_helper.rb                      |  2 ++
 spec/support/omniauth.rb                      |  9 ++++++
 6 files changed, 29 insertions(+), 44 deletions(-)
 delete mode 100644 spec/features/logout_spec.rb
 create mode 100644 spec/support/omniauth.rb

diff --git a/spec/dummy/app/views/shared/_user_links.html.erb b/spec/dummy/app/views/shared/_user_links.html.erb
index b9a7ca2..ff6ca8e 100644
--- a/spec/dummy/app/views/shared/_user_links.html.erb
+++ b/spec/dummy/app/views/shared/_user_links.html.erb
@@ -3,7 +3,7 @@
   <li><%= link_to "About", about_path, class: "#{active_link_class_controller(%w[pages])}" %></li>
 <% if current_user %>
   <li>
-    <%= link_to t('blacklight.header_links.logout'), destroy_user_session_path, method: :delete, data: { turbo_method: :delete } %>
+    <%= link_to t('blacklight.header_links.logout'), destroy_user_session_url, method: :delete, data: { turbo_method: :delete } %>
   </li>
   <% unless current_user.to_s.blank? %>
     <li>
diff --git a/spec/features/active_link_spec.rb b/spec/features/active_link_spec.rb
index 2322ad4..a8d17f6 100644
--- a/spec/features/active_link_spec.rb
+++ b/spec/features/active_link_spec.rb
@@ -25,13 +25,11 @@
       click_link "Login"
       expect(page).to have_content("Login")
 
-      fill_in "user_username", with: "bltest"
-      fill_in "user_password", with: "test"
-      click_button "Login"
+      click_button "Patron Login"
 
       visit account_path
 
-      expect(page).to have_css("a.active", text: "blacklight test")
+      expect(page).to have_css("a.active", text: "Blacklight Test")
     end
   end
 end
diff --git a/spec/features/logout_spec.rb b/spec/features/logout_spec.rb
deleted file mode 100644
index f54bb1d..0000000
--- a/spec/features/logout_spec.rb
+++ /dev/null
@@ -1,24 +0,0 @@
-# frozen_state_literal: true
-
-require "rails_helper"
-
-RSpec.describe "Logout" do
-  before do
-    # rubocop:disable RSpec/AnyInstance
-    allow_any_instance_of(PatronHelper).to receive(:user_location).and_return(:offsite)
-    # rubocop:enable RSpec/AnyInstance
-  end
-
-  it "destroys the user session" do
-    visit new_user_session_path
-    fill_in "user_username", with: "bltest"
-    fill_in "user_password", with: "test"
-    click_button "Login"
-
-    visit root_path
-    click_link "Log Out"
-
-    expect(page).to have_content(I18n.t("devise.sessions.signed_out"))
-    expect(page).not_to have_content("blacklight test")
-  end
-end
diff --git a/spec/files/auth/auth_hash.json b/spec/files/auth/auth_hash.json
index 1babc18..81198d4 100644
--- a/spec/files/auth/auth_hash.json
+++ b/spec/files/auth/auth_hash.json
@@ -2,28 +2,28 @@
   "provider": "keycloakopenid",
   "uid": "603e26dd-b2d4-4a88-ad9d-406eaec31463",
   "info": {
-    "name": "Staff User",
-    "email": "staff@nla.gov.au",
-    "first_name": "Staff",
-    "last_name": "User"
+    "name": "Blacklight Test",
+    "email": "test@example.com",
+    "first_name": "Blacklight",
+    "last_name": "Test"
   },
   "credentials": {
-    "token": "eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiIsImtpZCI6ImYyZjVmYjc3OWY4YjhhNjM2NDVlYjcxMTEzOWJhN2VhIn0.eyJleHAiOjE2NzM4NDAyOTYsImlhdCI6MTY3MzgzNjY5NiwiYXV0aF90aW1lIjoxNjczODM2Njk2LCJqdGkiOiI4ZGJmMDg4NS1kNzI3LTQ1YzQtODYwYS05ODE2YWUxOWZmNWYiLCJpc3MiOiJodHRwczovL2xvZ2luLmV4YW1wbGUuY29tL2F1dGgvcmVhbG1zL2V4YW1wbGUiLCJhdWQiOiJhY2NvdW50Iiwic3ViIjoiZjVhZmFkODEtMDZjNC00OWE2LTgyN2UtMmVmMmRkM2I4ZmRhIiwidHlwIjoiQmVhcmVyIiwiYXpwIjoiY2F0YWxvZ3VlIiwic2Vzc2lvbl9zdGF0ZSI6IjgwMjQyNDYwLWVlMjEtNGUyYy1iZmY2LTNjZmM1M2JjOGVmYiIsImFsbG93ZWQtb3JpZ2lucyI6WyJsb2NhbGhvc3QiLCIwLjAuMC4wIiwiY2F0YWxvZ3VlLm5sYS5nb3YuYXUiXSwicmVhbG1fYWNjZXNzIjp7InJvbGVzIjpbIm9mZmxpbmVfYWNjZXNzIiwidW1hX2F1dGhvcml6YXRpb24iXX0sInJlc291cmNlX2FjY2VzcyI6eyJhY2NvdW50Ijp7InJvbGVzIjpbIm1hbmFnZS1hY2NvdW50IiwibWFuYWdlLWFjY291bnQtbGlua3MiLCJ2aWV3LXByb2ZpbGUiXX19LCJzY29wZSI6InByb2ZpbGUgZW1haWwgcm9sZXMiLCJzaWQiOiI4MDI0MjQ2MC1lZTIxLTRlMmMtYmZmNi0zY2ZjNTNiYzhlZmIiLCJlbWFpbF92ZXJpZmllZCI6ZmFsc2UsInJvbGVzIjpbIm9mZmxpbmVfYWNjZXNzIiwidW1hX2F1dGhvcml6YXRpb24iXSwibmFtZSI6IlN0YWZmIFVzZXIiLCJwcmVmZXJyZWRfdXNlcm5hbWUiOiJzdGFmZiIsImdpdmVuX25hbWUiOiJTdGFmZiIsImZhbWlseV9uYW1lIjoiVXNlciIsImVtYWlsIjoic3RhZmZAbmxhLmdvdi5hdSJ9.cjKwwSfeFvujsEduYtzDfoZSBnPHVb3vvc7mYwOoa2Ji_LYd7UNRz7iXzCqQG149_UlyrBfk8f9-JU6AVp_izTYt58H9p8FjgkE6p1U-C-SXKk8TdbM0Lf4GMbZs2spQNK4LTZYTjduGS8nU3ao30O8RQvtdd1XvucCTmEBFLlKG8U6j1IfxVEtNzHBtqJFyWvKQdfruVfgWfi2QpgkH-X-Hjr6PS5OKNsZhx6N6d9iTS8MMzZGZtIuHd9Ho-A3iL8dj6ReUAxXLAF_qFXCnp5xZZ0RtUsmr38XDl6A-iqhfD19vh3L1JZUboRgDaX85ViOkTkU5s-JJi7oC3e39OrAk2o_brDT5FM8AvkJjOB_d9xIZaMScgVgPHI3cueP0yiXgmYNxaJ8NtF0LlV4qrec3DB2y4FvF7kYGnXjof5_P3urC8MrT4c1xigfM6alXmkNUuKTTooDL9l9SpivV7c7WUrJ1nNmH3511ObmKjJWZIggJphfOpGBq9CqJ8nCr",
+    "token": "eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiIsImtpZCI6ImYyZjVmYjc3OWY4YjhhNjM2NDVlYjcxMTEzOWJhN2VhIn0.eyJleHAiOjE2NzM4NDAyOTYsImlhdCI6MTY3MzgzNjY5NiwiYXV0aF90aW1lIjoxNjczODM2Njk2LCJqdGkiOiI4ZGJmMDg4NS1kNzI3LTQ1YzQtODYwYS05ODE2YWUxOWZmNWYiLCJpc3MiOiJodHRwczovL2xvZ2luLmV4YW1wbGUuY29tL2F1dGgvcmVhbG1zL2V4YW1wbGUiLCJhdWQiOiJhY2NvdW50Iiwic3ViIjoiZjVhZmFkODEtMDZjNC00OWE2LTgyN2UtMmVmMmRkM2I4ZmRhIiwidHlwIjoiQmVhcmVyIiwiYXpwIjoiY2F0YWxvZ3VlIiwic2Vzc2lvbl9zdGF0ZSI6IjgwMjQyNDYwLWVlMjEtNGUyYy1iZmY2LTNjZmM1M2JjOGVmYiIsImFsbG93ZWQtb3JpZ2lucyI6WyJsb2NhbGhvc3QiLCIwLjAuMC4wIiwiY2F0YWxvZ3VlLm5sYS5nb3YuYXUiXSwicmVhbG1fYWNjZXNzIjp7InJvbGVzIjpbIm9mZmxpbmVfYWNjZXNzIiwidW1hX2F1dGhvcml6YXRpb24iXX0sInJlc291cmNlX2FjY2VzcyI6eyJhY2NvdW50Ijp7InJvbGVzIjpbIm1hbmFnZS1hY2NvdW50IiwibWFuYWdlLWFjY291bnQtbGlua3MiLCJ2aWV3LXByb2ZpbGUiXX19LCJzY29wZSI6InByb2ZpbGUgZW1haWwgcm9sZXMiLCJzaWQiOiI4MDI0MjQ2MC1lZTIxLTRlMmMtYmZmNi0zY2ZjNTNiYzhlZmIiLCJlbWFpbF92ZXJpZmllZCI6ZmFsc2UsInJvbGVzIjpbIm9mZmxpbmVfYWNjZXNzIiwidW1hX2F1dGhvcml6YXRpb24iXSwibmFtZSI6IkJsYWNrbGlnaHQgVGVzdCIsInByZWZlcnJlZF91c2VybmFtZSI6ImMyMDJhN2Q1LTNkMzAtNDExYS1iMGE3LTUzNzUzODAxYmY5OCIsImdpdmVuX25hbWUiOiJCbGFja2xpZ2h0IiwiZmFtaWx5X25hbWUiOiJUZXN0IiwiZW1haWwiOiJ0ZXN0QGV4YW1wbGUuY29tIn0.lMGOYGiMroCMtmDF9LORZoPMzkj40mKB5Ee1c_cCdCYdzZCXygeED4eSAewtCexID-Dfc8-F-CaKr4I-mXjjtn2G5FKjfhvQGLmDvQ9pnpSsfHrK7V4c3AQgpgeEHw_aiW6uXicbp9yNLhuAOWJ0nKwGjFPQRMP9MOK1uQ_KWHBmdniz5XJ-E6C_KmFib-_Tzo2QVlGZsjIcankJ-cUEkyvrVmJ0y6JfaC6Nguf2YbwBELrpJgSATrWNQLicQ-yzA_pkKjCLQwRIOqkQU7s25Ty3a2CtVAJnjxMTOgpMbKoiy79Pzw8QVC_HJyL0vXEP9fDNNA9uHJIGxcHIjdcpBw",
     "refresh_token": "eyJhbGciOiJIUzI1NiIsInR5cCIgOiAiSldUIiwia2lkIiA6ICJjNjRhZTM5OS1hYmFmLTQ0MDQtYTUwNS04MDgwZmFkM2UzZTAifQ.eyJleHAiOjE2NzQwMDk0OTYsImlhdCI6MTY3MzgzNjY5NiwianRpIjoiZDE4ODViNzctMGUxNi00MzBjLWEyOTctNGE3NTA1ZWY0ZDFmIiwiaXNzIjoiaHR0cHM6Ly9sb2dpbi1kZXZlbC5ubGEuZ292LmF1L2F1dGgvcmVhbG1zL3NoaXJlIiwiYXVkIjoiaHR0cHM6Ly9sb2dpbi1kZXZlbC5ubGEuZ292LmF1L2F1dGgvcmVhbG1zL3NoaXJlIiwic3ViIjoiZjVhZmFkODEtMDZjNC00OWE2LTgyN2UtMmVmMmRkM2I4ZmRhIiwidHlwIjoiUmVmcmVzaCIsImF6cCI6ImNhdGFsb2d1ZSIsInNlc3Npb25fc3RhdGUiOiI4MDI0MjQ2MC1lZTIxLTRlMmMtYmZmNi0zY2ZjNTNiYzhlZmIiLCJzY29wZSI6InByb2ZpbGUgZW1haWwgcm9sZXMiLCJzaWQiOiI4MDI0MjQ2MC1lZTIxLTRlMmMtYmZmNi0zY2ZjNTNiYzhlZmIifQ.q_P4zHKyeEHnhlu0FFENX9c6n_YimkaokX7AV9MC-Ag",
-    "expires_at": 1673840296,
+    "expires_at": 1728943716,
     "expires": true
   },
   "extra": {
     "raw_info": {
-      "exp": 1673840296,
-      "iat": 1673836696,
+      "exp": 1728943716,
+      "iat": 1697407716,
       "auth_time": 1673836696,
       "jti": "8dbf0885-d727-45c4-860a-9816ae19ff5f",
-      "iss": "https://login.example.com/auth/realms/staff",
+      "iss": "https://login.example.com/auth/realms/example",
       "aud": "account",
       "sub": "603e26dd-b2d4-4a88-ad9d-406eaec31463",
       "typ": "Bearer",
-      "azp": "catalogue",
+      "azp": "join-us",
       "session_state": "80242460-ee21-4e2c-bff6-3cfc53bc8efb",
       "allowed-origins": [
         "localhost",
@@ -52,11 +52,11 @@
         "offline_access",
         "uma_authorization"
       ],
-      "name": "Staff User",
-      "preferred_username": "staff",
-      "given_name": "Staff",
-      "family_name": "User",
-      "email": "staff@nla.gov.au"
+      "name": "Blacklight Test",
+      "preferred_username": "c202a7d5-3d30-411a-b0a7-53753801bf98",
+      "given_name": "Blacklight",
+      "family_name": "Test",
+      "email": "test@example.com"
     },
     "id_token": null
   }
diff --git a/spec/simplecov_helper.rb b/spec/simplecov_helper.rb
index 19e769d..dfeb33b 100644
--- a/spec/simplecov_helper.rb
+++ b/spec/simplecov_helper.rb
@@ -23,4 +23,6 @@
   # these will be removed soon
   add_filter "lib/devise/models/getalibrarycard_authenticatable.rb"
   add_filter "lib/devise/strategies/getalibrarycard_authenticatable.rb"
+  add_filter "lib/devise/models/user_reg_authenticatable.rb"
+  add_filter "lib/devise/strategies/user_reg_authenticatable.rb"
 end
diff --git a/spec/support/omniauth.rb b/spec/support/omniauth.rb
new file mode 100644
index 0000000..14ec3fd
--- /dev/null
+++ b/spec/support/omniauth.rb
@@ -0,0 +1,9 @@
+require "omniauth"
+
+OmniAuth.config.test_mode = true
+
+RSpec.configure do |config|
+  config.before do
+    OmniAuth.config.add_mock(:catalogue_patron, JSON.parse(IO.read("spec/files/auth/auth_hash.json")))
+  end
+end

From f66cf1666ea8624adea734ad96b2d3daf5a3b74c Mon Sep 17 00:00:00 2001
From: Yetrina Battad <hello@yetti.io>
Date: Tue, 17 Oct 2023 13:25:28 +1100
Subject: [PATCH 6/8] refactor: refactor to use KC_PATRON_REALM as a feature
 flag

---
 .../concerns/auth_session_concern.rb          |   9 ++
 .../users/omniauth_callbacks_controller.rb    |   7 ++
 app/models/user.rb                            |  35 +++++-
 app/views/users/sessions/new.html.erb         |  30 ++---
 config/initializers/devise.rb                 |  16 ++-
 config/initializers/omniauth.rb               |   8 ++
 config/locales/auth.en.yml                    |   1 +
 config/routes.rb                              |   4 +
 .../app/controllers/application_controller.rb |   4 +
 spec/dummy/app/views/pages/about.html.erb     |   4 +-
 spec/dummy/app/views/pages/home.html.erb      |   4 +
 spec/factories/users.rb                       |   8 +-
 spec/features/active_link_spec.rb             |  19 ++++
 spec/features/login_spec.rb                   | 105 +++++++++++-------
 spec/files/auth/auth_hash.json                |  30 ++---
 spec/requests/staff_logout_spec.rb            |  10 +-
 spec/simplecov_helper.rb                      |   2 +
 spec/support/omniauth.rb                      |   9 ++
 18 files changed, 220 insertions(+), 85 deletions(-)
 create mode 100644 app/controllers/concerns/auth_session_concern.rb
 create mode 100644 spec/support/omniauth.rb

diff --git a/app/controllers/concerns/auth_session_concern.rb b/app/controllers/concerns/auth_session_concern.rb
new file mode 100644
index 0000000..8836075
--- /dev/null
+++ b/app/controllers/concerns/auth_session_concern.rb
@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+module AuthSessionConcern
+  extend ActiveSupport::Concern
+
+  def new_session_path(_scope)
+    new_user_session_path
+  end
+end
diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb
index 6ab37e2..ed93dfb 100644
--- a/app/controllers/users/omniauth_callbacks_controller.rb
+++ b/app/controllers/users/omniauth_callbacks_controller.rb
@@ -22,6 +22,13 @@ def catalogue_shared
     sign_in_and_redirect @user, event: :authentication
   end
 
+  def catalogue_patron
+    Rails.logger.debug(request.env["omniauth.auth"])
+    @user = User.from_keycloak_patron(request.env["omniauth.auth"])
+    store_keycloak_data(request.env["omniauth.auth"])
+    sign_in_and_redirect @user, event: :authentication
+  end
+
   # Keycloak will display its own error page when there is a failure to login.
   # :nocov:
   def failure
diff --git a/app/models/user.rb b/app/models/user.rb
index 74c2492..3da843d 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -31,8 +31,12 @@ class User < PatronsRecord
 
   # Include default devise modules. Others available are:
   # :confirmable, :lockable, :timeoutable, :trackable and :omniauthable
-  devise :user_reg_authenticatable, :timeoutable,
-    :omniauthable, omniauth_providers: %i[catalogue_sol catalogue_spl catalogue_shared]
+  if ENV["KC_PATRON_REALM"]
+    devise :timeoutable, :omniauthable, omniauth_providers: %i[catalogue_patron catalogue_sol catalogue_spl catalogue_shared]
+  else
+    devise :user_reg_authenticatable, :timeoutable,
+      :omniauthable, omniauth_providers: %i[catalogue_patron catalogue_sol catalogue_spl catalogue_shared]
+  end
 
   attr_accessor :username, :password, :session_token
 
@@ -68,6 +72,31 @@ def self.from_keycloak(auth)
     end
   end
 
+  def self.from_keycloak_patron(auth)
+    ActiveRecord::Base.transaction do
+      user = find_or_create_by!(folio_id: auth.extra.raw_info.preferred_username) do |user|
+        # We don't really care about the password since auth is via Keycloak, so we're just
+        # putting a dummy value here.
+        user.encrypted_password = SecureRandom.hex(14)
+      end
+      # set/update values from Keycloak in case they've changed
+      user.email = auth.info.email.present? ? auth.info.email : ""
+      user.provider = auth.provider
+      user.uid = auth.uid
+      user.name_given = auth.info.first_name
+      user.name_family = auth.info.last_name
+
+      # TODO: send request to catalogue services to determine active state of user account in FOLIO
+
+      # this is required for backchannel logout
+      user.session_token = auth.extra.raw_info.sid
+
+      # reload user with updated values from database
+      user.save!
+      user.reload
+    end
+  end
+
   # Method added by Blacklight; Blacklight uses #to_s on your
   # user class to get a user-displayable login/identifier for
   # the account.
@@ -80,6 +109,8 @@ def to_s
         "#{name} (SPL)"
       elsif provider == "catalogue_shared"
         "#{name} (TOL)"
+      else
+        name
       end
     end
     name
diff --git a/app/views/users/sessions/new.html.erb b/app/views/users/sessions/new.html.erb
index 4c455c3..2d985b6 100644
--- a/app/views/users/sessions/new.html.erb
+++ b/app/views/users/sessions/new.html.erb
@@ -1,21 +1,25 @@
 <h1>Login</h1>
 
-<%= form_for(resource, as: resource_name, html: {'data-turbo' => "false"}, url: session_path(resource_name)) do |f| %>
-  <div class="field form-group">
-    <%= f.label :username, "User ID" %>
-    <%= f.text_field :username, autofocus: true, autocomplete: "username",class: "form-control col-md-4" %>
-  </div>
+<% if ENV["KC_PATRON_REALM"] %>
+  <%= button_to t("auth.patron_login"), user_catalogue_patron_omniauth_authorize_path, data: { turbo: false } %>
+<% else %>
+  <%= form_for(resource, as: resource_name, html: {'data-turbo' => "false"}, url: session_path(resource_name)) do |f| %>
+    <div class="field form-group">
+      <%= f.label :username, "User ID" %>
+      <%= f.text_field :username, autofocus: true, autocomplete: "username",class: "form-control col-md-4" %>
+    </div>
 
-  <div class="field form-group">
-    <%= f.label :password, "Family Name" %>
-    <%= f.text_field :password, autocomplete: "off",class: "form-control col-md-4" %>
-  </div>
+    <div class="field form-group">
+      <%= f.label :password, "Family Name" %>
+      <%= f.text_field :password, autocomplete: "off",class: "form-control col-md-4" %>
+    </div>
 
-  <p><%= t("devise.registrations.register", url: ENV["NATIONAL_LIBRARY_CARD_URL"]).html_safe %></p>
+    <p><%= t("devise.registrations.register", url: ENV["NATIONAL_LIBRARY_CARD_URL"]).html_safe %></p>
 
-  <div class="actions">
-    <%= f.submit "Login", class: "btn btn-primary" %>
-  </div>
+    <div class="actions">
+      <%= f.submit "Login", class: "btn btn-primary" %>
+    </div>
+  <% end %>
 <% end %>
 
 <%= render "users/shared/links" %>
diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb
index bb5eaf5..5c81b89 100644
--- a/config/initializers/devise.rb
+++ b/config/initializers/devise.rb
@@ -90,7 +90,9 @@ def recall
   # session. If you need permissions, you should implement that in a before filter.
   # You can also supply a hash where the value is a boolean determining whether
   # or not authentication should be aborted when the value is not present.
-  config.authentication_keys = {username: true, password: true}
+  unless ENV["KC_PATRON_REALM"]
+    config.authentication_keys = {username: true, password: true}
+  end
 
   # Configure parameters from the request object used for authentication. Each entry
   # given should be a request method and it will automatically be passed to the
@@ -107,7 +109,9 @@ def recall
   # Configure which authentication keys should have whitespace stripped.
   # These keys will have whitespace before and after removed upon creating or
   # modifying a user and when used to authenticate or find a user. Default is :email.
-  config.strip_whitespace_keys = [:username, :password]
+  unless ENV["KC_PATRON_REALM"]
+    config.strip_whitespace_keys = [:username, :password]
+  end
 
   # Tell if authentication through request.params is enabled. True by default.
   # It can be set to an array that will enable params authentication only for the
@@ -321,9 +325,11 @@ def recall
   # If you want to use other strategies, that are not supported by Devise, or
   # change the failure app, you can configure them inside the config.warden block.
   #
-  config.warden do |manager|
-    manager.failure_app = CatalogueFailureApp
-    manager.default_strategies(scope: :user).unshift :user_reg_authenticatable
+  unless ENV["KC_PATRON_REALM"]
+    config.warden do |manager|
+      manager.failure_app = CatalogueFailureApp
+      manager.default_strategies(scope: :user).unshift :user_reg_authenticatable
+    end
   end
 
   # ==> Mountable engine configurations
diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb
index b47638d..876bb45 100644
--- a/config/initializers/omniauth.rb
+++ b/config/initializers/omniauth.rb
@@ -22,4 +22,12 @@
     strategy_class: OmniAuth::Strategies::KeycloakOpenId,
     authorize_params: {scope: "openid"},
     name: "catalogue_shared"
+
+  provider :keycloak_openid,
+    ENV.fetch("KC_PATRON_CLIENT", "patron"),
+    ENV.fetch("KC_PATRON_SECRET", "default secret"),
+    client_options: {site: ENV.fetch("KEYCLOAK_URL", "http://localhost:9090"), realm: ENV.fetch("KC_PATRON_REALM", "nla-patron")},
+    strategy_class: OmniAuth::Strategies::KeycloakOpenId,
+    authorize_params: {scope: "openid"},
+    name: "catalogue_patron"
 end
diff --git a/config/locales/auth.en.yml b/config/locales/auth.en.yml
index ff75b21..74ea6e4 100644
--- a/config/locales/auth.en.yml
+++ b/config/locales/auth.en.yml
@@ -6,3 +6,4 @@ en:
       sol_login: "Staff Official Loan"
       spl_login: "Staff Personal Loan"
       shared_login: "Team Official Loan"
+    patron_login: "Patron Login"
diff --git a/config/routes.rb b/config/routes.rb
index 8d14baf..2551fce 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -51,6 +51,10 @@
   }
 
   devise_scope(:user) do
+    if ENV["KC_PATRON_REALM"]
+      get "sign_in", to: "users/sessions#new", as: :new_user_session
+      delete "sign_out", to: "users/sessions#destroy", as: :destroy_user_session
+    end
     post "/backchannel_logout", to: "users/sessions#backchannel_logout", as: :backchannel_logout
   end
 end
diff --git a/spec/dummy/app/controllers/application_controller.rb b/spec/dummy/app/controllers/application_controller.rb
index 1652e22..d45ad21 100644
--- a/spec/dummy/app/controllers/application_controller.rb
+++ b/spec/dummy/app/controllers/application_controller.rb
@@ -1,3 +1,7 @@
 class ApplicationController < ActionController::Base
   # Attempt to find the mapped route for devise based on request path
+
+  if ENV["KC_PATRON_REALM"]
+    include AuthSessionConcern
+  end
 end
diff --git a/spec/dummy/app/views/pages/about.html.erb b/spec/dummy/app/views/pages/about.html.erb
index 3453cf2..56bfe1f 100644
--- a/spec/dummy/app/views/pages/about.html.erb
+++ b/spec/dummy/app/views/pages/about.html.erb
@@ -1,2 +1,2 @@
-<h1>Pages#home</h1>
-<p>Find me in app/views/pages/home.html.erb</p>
+<h1>Pages#about</h1>
+<p>Find me in app/views/pages/about.html.erb</p>
diff --git a/spec/dummy/app/views/pages/home.html.erb b/spec/dummy/app/views/pages/home.html.erb
index 3453cf2..b528a86 100644
--- a/spec/dummy/app/views/pages/home.html.erb
+++ b/spec/dummy/app/views/pages/home.html.erb
@@ -1,2 +1,6 @@
 <h1>Pages#home</h1>
 <p>Find me in app/views/pages/home.html.erb</p>
+
+<% if current_user.present? %>
+  Logged in as <%= current_user %>
+<% end %>
diff --git a/spec/factories/users.rb b/spec/factories/users.rb
index 9701389..95ea94d 100644
--- a/spec/factories/users.rb
+++ b/spec/factories/users.rb
@@ -32,12 +32,18 @@
     name_given { "Test" }
     name_family { "User" }
 
+    if ENV["KC_PATRON_REALM"]
+      uid { SecureRandom.uuid }
+      provider { "catalogue_patron" }
+      session_token { SecureRandom.hex }
+    end
+
     trait :staff do
       provider { "catalogue_sol" }
       uid { "603e26dd-b2d4-4a88-ad9d-406eaec31463" }
       name_given { "Staff" }
       name_family { "User" }
-      email { "staff@nla.gov.au" }
+      sequence(:email) { |n| "staff-#{n.to_s.rjust(3, "0")}@nla.gov.au" }
       session_token { SecureRandom.hex }
     end
 
diff --git a/spec/features/active_link_spec.rb b/spec/features/active_link_spec.rb
index 2322ad4..6b09478 100644
--- a/spec/features/active_link_spec.rb
+++ b/spec/features/active_link_spec.rb
@@ -33,5 +33,24 @@
 
       expect(page).to have_css("a.active", text: "blacklight test")
     end
+
+    context "when Keycloak patron authentication is enabled" do
+      before do
+        allow(ENV).to receive(:[]).and_call_original
+        allow(ENV).to receive(:[]).with("KC_PATRON_REALM").and_return("patron_realm")
+      end
+
+      it "returns active given the controller and action" do
+        visit root_path
+        click_link "Login"
+        expect(page).to have_content("Login")
+
+        click_button "Patron Login"
+
+        visit account_path
+
+        expect(page).to have_css("a.active", text: "Blacklight Test")
+      end
+    end
   end
 end
diff --git a/spec/features/login_spec.rb b/spec/features/login_spec.rb
index 5dffe14..80f893e 100644
--- a/spec/features/login_spec.rb
+++ b/spec/features/login_spec.rb
@@ -9,67 +9,88 @@
     # rubocop:enable RSpec/AnyInstance
   end
 
-  it "creates a new session for the user" do
-    visit root_path
-    click_link "Login"
-    expect(page).to have_content("Login")
-
-    fill_in "user_username", with: "bltest"
-    fill_in "user_password", with: "test"
-    click_button "Login"
-
-    expect(page).to have_content(I18n.t("devise.sessions.signed_in"))
-    expect(page).to have_content("blacklight test")
-  end
-
-  it "displays a registration link" do
-    visit new_user_session_path
+  context "when Keycloak patron authentication is enabled" do
+    before do
+      allow(ENV).to receive(:[]).and_call_original
+      allow(ENV).to receive(:[]).with("KC_PATRON_REALM").and_return("patron_realm")
+    end
 
-    expect(page).to have_link("here", href: ENV["NATIONAL_LIBRARY_CARD_URL"])
-  end
+    it "displays a login link" do
+      visit root_path
 
-  it "disables Turbo" do
-    visit new_user_session_path
+      expect(page).to have_link("Login", href: new_user_session_path)
+    end
 
-    expect(page).to have_css("form[data-turbo]")
+    it "displays the patron login button" do
+      visit root_path
+      click_link "Login"
+      expect(page).to have_content(I18n.t("auth.patron_login"))
+    end
   end
 
-  context "when user is inactive" do
-    it "displays an error message" do
+  context "when Keycloak patron authentication is disabled" do
+    it "creates a new session for the user" do
       visit root_path
       click_link "Login"
       expect(page).to have_content("Login")
 
       fill_in "user_username", with: "bltest"
-      fill_in "user_password", with: "blacklight"
+      fill_in "user_password", with: "test"
       click_button "Login"
 
-      expect(page).to have_content(I18n.t("devise.failure.expired"))
-      expect(page).to have_content("Login")
+      expect(page).to have_content(I18n.t("devise.sessions.signed_in"))
+      expect(page).to have_content("blacklight test")
     end
-  end
-
-  context "when user has the wrong credentials" do
-    it "displays an error message" do
-      visit root_path
-      click_link "Login"
-      expect(page).to have_content("Login")
 
-      fill_in "user_username", with: "0000"
-      fill_in "user_password", with: "failure"
-      click_button "Login"
+    it "displays a registration link" do
+      visit new_user_session_path
 
-      expect(page).to have_link("Ask a Librarian", href: ENV["ASK_LIBRARIAN_URL"])
-      expect(page).to have_content("Login")
+      expect(page).to have_link("here", href: ENV["NATIONAL_LIBRARY_CARD_URL"])
     end
-  end
 
-  context "when user does not enter a username or password" do
-    it "displays an error message" do
+    it "disables Turbo" do
       visit new_user_session_path
-      click_button "Login"
 
-      expect(page).to have_link("Ask a Librarian", href: ENV["ASK_LIBRARIAN_URL"])
+      expect(page).to have_css("form[data-turbo]")
+    end
+
+    context "when user is inactive" do
+      it "displays an error message" do
+        visit root_path
+        click_link "Login"
+        expect(page).to have_content("Login")
+
+        fill_in "user_username", with: "bltest"
+        fill_in "user_password", with: "blacklight"
+        click_button "Login"
+
+        expect(page).to have_content(I18n.t("devise.failure.expired"))
+        expect(page).to have_content("Login")
+      end
+    end
+
+    context "when user has the wrong credentials" do
+      it "displays an error message" do
+        visit root_path
+        click_link "Login"
+        expect(page).to have_content("Login")
+
+        fill_in "user_username", with: "0000"
+        fill_in "user_password", with: "failure"
+        click_button "Login"
+
+        expect(page).to have_link("Ask a Librarian", href: ENV["ASK_LIBRARIAN_URL"])
+        expect(page).to have_content("Login")
+      end
+    end
+
+    context "when user does not enter a username or password" do
+      it "displays an error message" do
+        visit new_user_session_path
+        click_button "Login"
+
+        expect(page).to have_link("Ask a Librarian", href: ENV["ASK_LIBRARIAN_URL"])
+      end
     end
   end
 end
diff --git a/spec/files/auth/auth_hash.json b/spec/files/auth/auth_hash.json
index 1babc18..81198d4 100644
--- a/spec/files/auth/auth_hash.json
+++ b/spec/files/auth/auth_hash.json
@@ -2,28 +2,28 @@
   "provider": "keycloakopenid",
   "uid": "603e26dd-b2d4-4a88-ad9d-406eaec31463",
   "info": {
-    "name": "Staff User",
-    "email": "staff@nla.gov.au",
-    "first_name": "Staff",
-    "last_name": "User"
+    "name": "Blacklight Test",
+    "email": "test@example.com",
+    "first_name": "Blacklight",
+    "last_name": "Test"
   },
   "credentials": {
-    "token": "eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiIsImtpZCI6ImYyZjVmYjc3OWY4YjhhNjM2NDVlYjcxMTEzOWJhN2VhIn0.eyJleHAiOjE2NzM4NDAyOTYsImlhdCI6MTY3MzgzNjY5NiwiYXV0aF90aW1lIjoxNjczODM2Njk2LCJqdGkiOiI4ZGJmMDg4NS1kNzI3LTQ1YzQtODYwYS05ODE2YWUxOWZmNWYiLCJpc3MiOiJodHRwczovL2xvZ2luLmV4YW1wbGUuY29tL2F1dGgvcmVhbG1zL2V4YW1wbGUiLCJhdWQiOiJhY2NvdW50Iiwic3ViIjoiZjVhZmFkODEtMDZjNC00OWE2LTgyN2UtMmVmMmRkM2I4ZmRhIiwidHlwIjoiQmVhcmVyIiwiYXpwIjoiY2F0YWxvZ3VlIiwic2Vzc2lvbl9zdGF0ZSI6IjgwMjQyNDYwLWVlMjEtNGUyYy1iZmY2LTNjZmM1M2JjOGVmYiIsImFsbG93ZWQtb3JpZ2lucyI6WyJsb2NhbGhvc3QiLCIwLjAuMC4wIiwiY2F0YWxvZ3VlLm5sYS5nb3YuYXUiXSwicmVhbG1fYWNjZXNzIjp7InJvbGVzIjpbIm9mZmxpbmVfYWNjZXNzIiwidW1hX2F1dGhvcml6YXRpb24iXX0sInJlc291cmNlX2FjY2VzcyI6eyJhY2NvdW50Ijp7InJvbGVzIjpbIm1hbmFnZS1hY2NvdW50IiwibWFuYWdlLWFjY291bnQtbGlua3MiLCJ2aWV3LXByb2ZpbGUiXX19LCJzY29wZSI6InByb2ZpbGUgZW1haWwgcm9sZXMiLCJzaWQiOiI4MDI0MjQ2MC1lZTIxLTRlMmMtYmZmNi0zY2ZjNTNiYzhlZmIiLCJlbWFpbF92ZXJpZmllZCI6ZmFsc2UsInJvbGVzIjpbIm9mZmxpbmVfYWNjZXNzIiwidW1hX2F1dGhvcml6YXRpb24iXSwibmFtZSI6IlN0YWZmIFVzZXIiLCJwcmVmZXJyZWRfdXNlcm5hbWUiOiJzdGFmZiIsImdpdmVuX25hbWUiOiJTdGFmZiIsImZhbWlseV9uYW1lIjoiVXNlciIsImVtYWlsIjoic3RhZmZAbmxhLmdvdi5hdSJ9.cjKwwSfeFvujsEduYtzDfoZSBnPHVb3vvc7mYwOoa2Ji_LYd7UNRz7iXzCqQG149_UlyrBfk8f9-JU6AVp_izTYt58H9p8FjgkE6p1U-C-SXKk8TdbM0Lf4GMbZs2spQNK4LTZYTjduGS8nU3ao30O8RQvtdd1XvucCTmEBFLlKG8U6j1IfxVEtNzHBtqJFyWvKQdfruVfgWfi2QpgkH-X-Hjr6PS5OKNsZhx6N6d9iTS8MMzZGZtIuHd9Ho-A3iL8dj6ReUAxXLAF_qFXCnp5xZZ0RtUsmr38XDl6A-iqhfD19vh3L1JZUboRgDaX85ViOkTkU5s-JJi7oC3e39OrAk2o_brDT5FM8AvkJjOB_d9xIZaMScgVgPHI3cueP0yiXgmYNxaJ8NtF0LlV4qrec3DB2y4FvF7kYGnXjof5_P3urC8MrT4c1xigfM6alXmkNUuKTTooDL9l9SpivV7c7WUrJ1nNmH3511ObmKjJWZIggJphfOpGBq9CqJ8nCr",
+    "token": "eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiIsImtpZCI6ImYyZjVmYjc3OWY4YjhhNjM2NDVlYjcxMTEzOWJhN2VhIn0.eyJleHAiOjE2NzM4NDAyOTYsImlhdCI6MTY3MzgzNjY5NiwiYXV0aF90aW1lIjoxNjczODM2Njk2LCJqdGkiOiI4ZGJmMDg4NS1kNzI3LTQ1YzQtODYwYS05ODE2YWUxOWZmNWYiLCJpc3MiOiJodHRwczovL2xvZ2luLmV4YW1wbGUuY29tL2F1dGgvcmVhbG1zL2V4YW1wbGUiLCJhdWQiOiJhY2NvdW50Iiwic3ViIjoiZjVhZmFkODEtMDZjNC00OWE2LTgyN2UtMmVmMmRkM2I4ZmRhIiwidHlwIjoiQmVhcmVyIiwiYXpwIjoiY2F0YWxvZ3VlIiwic2Vzc2lvbl9zdGF0ZSI6IjgwMjQyNDYwLWVlMjEtNGUyYy1iZmY2LTNjZmM1M2JjOGVmYiIsImFsbG93ZWQtb3JpZ2lucyI6WyJsb2NhbGhvc3QiLCIwLjAuMC4wIiwiY2F0YWxvZ3VlLm5sYS5nb3YuYXUiXSwicmVhbG1fYWNjZXNzIjp7InJvbGVzIjpbIm9mZmxpbmVfYWNjZXNzIiwidW1hX2F1dGhvcml6YXRpb24iXX0sInJlc291cmNlX2FjY2VzcyI6eyJhY2NvdW50Ijp7InJvbGVzIjpbIm1hbmFnZS1hY2NvdW50IiwibWFuYWdlLWFjY291bnQtbGlua3MiLCJ2aWV3LXByb2ZpbGUiXX19LCJzY29wZSI6InByb2ZpbGUgZW1haWwgcm9sZXMiLCJzaWQiOiI4MDI0MjQ2MC1lZTIxLTRlMmMtYmZmNi0zY2ZjNTNiYzhlZmIiLCJlbWFpbF92ZXJpZmllZCI6ZmFsc2UsInJvbGVzIjpbIm9mZmxpbmVfYWNjZXNzIiwidW1hX2F1dGhvcml6YXRpb24iXSwibmFtZSI6IkJsYWNrbGlnaHQgVGVzdCIsInByZWZlcnJlZF91c2VybmFtZSI6ImMyMDJhN2Q1LTNkMzAtNDExYS1iMGE3LTUzNzUzODAxYmY5OCIsImdpdmVuX25hbWUiOiJCbGFja2xpZ2h0IiwiZmFtaWx5X25hbWUiOiJUZXN0IiwiZW1haWwiOiJ0ZXN0QGV4YW1wbGUuY29tIn0.lMGOYGiMroCMtmDF9LORZoPMzkj40mKB5Ee1c_cCdCYdzZCXygeED4eSAewtCexID-Dfc8-F-CaKr4I-mXjjtn2G5FKjfhvQGLmDvQ9pnpSsfHrK7V4c3AQgpgeEHw_aiW6uXicbp9yNLhuAOWJ0nKwGjFPQRMP9MOK1uQ_KWHBmdniz5XJ-E6C_KmFib-_Tzo2QVlGZsjIcankJ-cUEkyvrVmJ0y6JfaC6Nguf2YbwBELrpJgSATrWNQLicQ-yzA_pkKjCLQwRIOqkQU7s25Ty3a2CtVAJnjxMTOgpMbKoiy79Pzw8QVC_HJyL0vXEP9fDNNA9uHJIGxcHIjdcpBw",
     "refresh_token": "eyJhbGciOiJIUzI1NiIsInR5cCIgOiAiSldUIiwia2lkIiA6ICJjNjRhZTM5OS1hYmFmLTQ0MDQtYTUwNS04MDgwZmFkM2UzZTAifQ.eyJleHAiOjE2NzQwMDk0OTYsImlhdCI6MTY3MzgzNjY5NiwianRpIjoiZDE4ODViNzctMGUxNi00MzBjLWEyOTctNGE3NTA1ZWY0ZDFmIiwiaXNzIjoiaHR0cHM6Ly9sb2dpbi1kZXZlbC5ubGEuZ292LmF1L2F1dGgvcmVhbG1zL3NoaXJlIiwiYXVkIjoiaHR0cHM6Ly9sb2dpbi1kZXZlbC5ubGEuZ292LmF1L2F1dGgvcmVhbG1zL3NoaXJlIiwic3ViIjoiZjVhZmFkODEtMDZjNC00OWE2LTgyN2UtMmVmMmRkM2I4ZmRhIiwidHlwIjoiUmVmcmVzaCIsImF6cCI6ImNhdGFsb2d1ZSIsInNlc3Npb25fc3RhdGUiOiI4MDI0MjQ2MC1lZTIxLTRlMmMtYmZmNi0zY2ZjNTNiYzhlZmIiLCJzY29wZSI6InByb2ZpbGUgZW1haWwgcm9sZXMiLCJzaWQiOiI4MDI0MjQ2MC1lZTIxLTRlMmMtYmZmNi0zY2ZjNTNiYzhlZmIifQ.q_P4zHKyeEHnhlu0FFENX9c6n_YimkaokX7AV9MC-Ag",
-    "expires_at": 1673840296,
+    "expires_at": 1728943716,
     "expires": true
   },
   "extra": {
     "raw_info": {
-      "exp": 1673840296,
-      "iat": 1673836696,
+      "exp": 1728943716,
+      "iat": 1697407716,
       "auth_time": 1673836696,
       "jti": "8dbf0885-d727-45c4-860a-9816ae19ff5f",
-      "iss": "https://login.example.com/auth/realms/staff",
+      "iss": "https://login.example.com/auth/realms/example",
       "aud": "account",
       "sub": "603e26dd-b2d4-4a88-ad9d-406eaec31463",
       "typ": "Bearer",
-      "azp": "catalogue",
+      "azp": "join-us",
       "session_state": "80242460-ee21-4e2c-bff6-3cfc53bc8efb",
       "allowed-origins": [
         "localhost",
@@ -52,11 +52,11 @@
         "offline_access",
         "uma_authorization"
       ],
-      "name": "Staff User",
-      "preferred_username": "staff",
-      "given_name": "Staff",
-      "family_name": "User",
-      "email": "staff@nla.gov.au"
+      "name": "Blacklight Test",
+      "preferred_username": "c202a7d5-3d30-411a-b0a7-53753801bf98",
+      "given_name": "Blacklight",
+      "family_name": "Test",
+      "email": "test@example.com"
     },
     "id_token": null
   }
diff --git a/spec/requests/staff_logout_spec.rb b/spec/requests/staff_logout_spec.rb
index 6335a32..963390f 100644
--- a/spec/requests/staff_logout_spec.rb
+++ b/spec/requests/staff_logout_spec.rb
@@ -9,8 +9,8 @@
   let(:auth_hash) { OmniAuth::AuthHash.new(JSON.parse(file)) }
 
   before do
-    staff = create(:user, :staff)
-    sign_in staff
+    patron = create(:user, :staff)
+    sign_in patron
   end
 
   it "redirects to Keycloak's logout URL" do
@@ -34,19 +34,19 @@
       jwt = JWT.decode(logout_token, nil, false)
       jwt[0]["sid"]
     end
-    let(:staff) do
+    let(:patron) do
       user = create(:user, :staff)
       user.update_column(:session_token, session_id)
       user.reload
     end
 
     before do
-      sign_in staff
+      sign_in patron
     end
 
     it "updates the session_token for the logged out user" do
       post "/backchannel_logout", params: {logout_token: logout_token}
-      expect(staff.session_token).not_to eq session_id
+      expect(patron.session_token).not_to eq session_id
     end
   end
 end
diff --git a/spec/simplecov_helper.rb b/spec/simplecov_helper.rb
index 19e769d..dfeb33b 100644
--- a/spec/simplecov_helper.rb
+++ b/spec/simplecov_helper.rb
@@ -23,4 +23,6 @@
   # these will be removed soon
   add_filter "lib/devise/models/getalibrarycard_authenticatable.rb"
   add_filter "lib/devise/strategies/getalibrarycard_authenticatable.rb"
+  add_filter "lib/devise/models/user_reg_authenticatable.rb"
+  add_filter "lib/devise/strategies/user_reg_authenticatable.rb"
 end
diff --git a/spec/support/omniauth.rb b/spec/support/omniauth.rb
new file mode 100644
index 0000000..14ec3fd
--- /dev/null
+++ b/spec/support/omniauth.rb
@@ -0,0 +1,9 @@
+require "omniauth"
+
+OmniAuth.config.test_mode = true
+
+RSpec.configure do |config|
+  config.before do
+    OmniAuth.config.add_mock(:catalogue_patron, JSON.parse(IO.read("spec/files/auth/auth_hash.json")))
+  end
+end

From d44e16937c57ff8698440bd7dcb16e281d6ee37b Mon Sep 17 00:00:00 2001
From: Yetrina Battad <hello@yetti.io>
Date: Tue, 17 Oct 2023 13:33:26 +1100
Subject: [PATCH 7/8] test: fix test broken by merge

---
 spec/features/active_link_spec.rb | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/spec/features/active_link_spec.rb b/spec/features/active_link_spec.rb
index dba5513..6b09478 100644
--- a/spec/features/active_link_spec.rb
+++ b/spec/features/active_link_spec.rb
@@ -25,11 +25,13 @@
       click_link "Login"
       expect(page).to have_content("Login")
 
-      click_button "Patron Login"
+      fill_in "user_username", with: "bltest"
+      fill_in "user_password", with: "test"
+      click_button "Login"
 
       visit account_path
 
-      expect(page).to have_css("a.active", text: "Blacklight Test")
+      expect(page).to have_css("a.active", text: "blacklight test")
     end
 
     context "when Keycloak patron authentication is enabled" do

From bdc42a9d7ec42e53bbd577939b224b9586a59cdb Mon Sep 17 00:00:00 2001
From: Yetrina Battad <hello@yetti.io>
Date: Tue, 17 Oct 2023 13:46:08 +1100
Subject: [PATCH 8/8] fix: fix session destruction and patron login style

---
 app/controllers/users/sessions_controller.rb | 43 ++++++++++++++++----
 app/views/users/sessions/new.html.erb        |  2 +-
 2 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb
index 61f580a..86beef6 100644
--- a/app/controllers/users/sessions_controller.rb
+++ b/app/controllers/users/sessions_controller.rb
@@ -1,17 +1,23 @@
 # frozen_string_literal: true
 
 class Users::SessionsController < Devise::SessionsController
+  before_action :configure_sign_in_params, only: [:create]
+
   skip_before_action :verify_authenticity_token, only: [:backchannel_logout]
 
-  def destroy
-    # Keycloak logout. Keycloak will send a POST to "/backchannel_logout" to perform a
-    # backchannel logout that terminates the Devise session.
-    iss = session[:iss]
-    id_token = session[:id_token]
+  def create
+    super
+  end
 
-    signed_out = (Devise.sign_out_all_scopes ? sign_out : sign_out(resource_name))
-    set_flash_message! :notice, :signed_out if signed_out
-    redirect_to("#{iss}/protocol/openid-connect/logout?id_token_hint=#{id_token}&post_logout_redirect_uri=#{root_url}", allow_other_host: true)
+  def destroy
+    if session[:iss].present?
+      # Keycloak logout. Keycloak will send a POST to "/devise_logout" to perform a
+      # backchannel logout that terminates the Devise session.
+      keycloak_logout
+    else
+      # There is no Keycloak session identifier, so destroy the Devise session.
+      devise_logout
+    end
   end
 
   def backchannel_logout
@@ -24,4 +30,25 @@ def backchannel_logout
     user = User.find_by(session_token: session_id)
     user.update_column(:session_token, SecureRandom.hex)
   end
+
+  protected
+
+  def devise_logout
+    signed_out = (Devise.sign_out_all_scopes ? sign_out : sign_out(resource_name))
+    set_flash_message! :notice, :signed_out if signed_out
+    respond_to_on_destroy
+  end
+
+  def keycloak_logout
+    iss = session[:iss]
+    id_token = session[:id_token]
+
+    signed_out = (Devise.sign_out_all_scopes ? sign_out : sign_out(resource_name))
+    set_flash_message! :notice, :signed_out if signed_out
+    redirect_to("#{iss}/protocol/openid-connect/logout?id_token_hint=#{id_token}&post_logout_redirect_uri=#{root_url}", allow_other_host: true)
+  end
+
+  def configure_sign_in_params
+    devise_parameter_sanitizer.permit(:sign_in, keys: [user: [:username, :password]])
+  end
 end
diff --git a/app/views/users/sessions/new.html.erb b/app/views/users/sessions/new.html.erb
index 2d985b6..daa910b 100644
--- a/app/views/users/sessions/new.html.erb
+++ b/app/views/users/sessions/new.html.erb
@@ -1,7 +1,7 @@
 <h1>Login</h1>
 
 <% if ENV["KC_PATRON_REALM"] %>
-  <%= button_to t("auth.patron_login"), user_catalogue_patron_omniauth_authorize_path, data: { turbo: false } %>
+  <%= button_to t("auth.patron_login"), user_catalogue_patron_omniauth_authorize_path, class: "btn btn-primary", data: { turbo: false } %>
 <% else %>
   <%= form_for(resource, as: resource_name, html: {'data-turbo' => "false"}, url: session_path(resource_name)) do |f| %>
     <div class="field form-group">