From b9787098a7ac13efb429e5183a72ea7841e6eabd Mon Sep 17 00:00:00 2001 From: Tom Iles Date: Thu, 16 Nov 2023 09:35:08 +0000 Subject: [PATCH] Remove GDS SSO Gem Remove the Gem, add bits missing from the gem itself. --- Gemfile | 1 - Gemfile.lock | 12 -- app/controllers/authentication_controller.rb | 4 + app/models/user.rb | 13 +- app/service/navigation_items_service.rb | 12 +- config/initializers/authentication.rb | 87 +++++++++++-- .../warden/strategies/basic_auth.rb | 6 + config/routes.rb | 1 + lib/warden/strategies/omniauth.rb | 4 + spec/integration/gds_sso_spec.rb | 119 ------------------ spec/models/user_spec.rb | 3 - .../authentication_controller_spec.rb | 22 ++-- spec/service/navigation_items_service_spec.rb | 14 --- 13 files changed, 111 insertions(+), 187 deletions(-) delete mode 100644 spec/integration/gds_sso_spec.rb diff --git a/Gemfile b/Gemfile index 7655e1435..401bac9fe 100644 --- a/Gemfile +++ b/Gemfile @@ -15,7 +15,6 @@ gem "pg", "~> 1.5" gem "puma", "~> 6.4.0" # Used for handling authentication -gem "gds-sso" gem "omniauth" gem "omniauth-auth0" gem "warden" diff --git a/Gemfile.lock b/Gemfile.lock index 43c19fee5..f4144185b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -207,14 +207,6 @@ GEM faraday-net_http (>= 2.0, < 3.1) ruby2_keywords (>= 0.0.4) faraday-net_http (3.0.2) - gds-sso (18.1.0) - oauth2 (~> 2.0) - omniauth (~> 2.1) - omniauth-oauth2 (~> 1.8) - plek (>= 4, < 6) - rails (>= 6) - warden (~> 1.2) - warden-oauth2 (~> 0.0.1) globalid (1.2.1) activesupport (>= 6.1) govuk-components (4.1.1) @@ -322,7 +314,6 @@ GEM ast (~> 2.4.1) racc pg (1.5.4) - plek (5.0.0) psych (5.1.1.1) stringio public_suffix (5.0.3) @@ -493,8 +484,6 @@ GEM zeitwerk (~> 2.2) warden (1.2.9) rack (>= 2.0.9) - warden-oauth2 (0.0.1) - warden webmock (3.19.1) addressable (>= 2.8.0) crack (>= 0.3.2) @@ -529,7 +518,6 @@ DEPENDENCIES dfe-autocomplete! factory_bot_rails faker - gds-sso govuk-components (~> 4.1.1) govuk-forms-markdown! govuk_design_system_formbuilder (~> 4.1.1) diff --git a/app/controllers/authentication_controller.rb b/app/controllers/authentication_controller.rb index 27e330faa..161d8efaf 100644 --- a/app/controllers/authentication_controller.rb +++ b/app/controllers/authentication_controller.rb @@ -50,6 +50,10 @@ def sign_out end end + def failure + render "authentications/failure", layout: "application" + end + private def attempted_path diff --git a/app/models/user.rb b/app/models/user.rb index 18baea55f..a8d85cd33 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,5 +1,4 @@ class User < ApplicationRecord - include GDS::SSO::User has_paper_trail only: %i[role organisation_id has_access] class UserAuthenticationException < StandardError; end @@ -85,6 +84,18 @@ def role_changed_to_editor? role_changed_to_editor end + def clear_remotely_signed_out! + # rubocop:disable Rails/SkipsModelValidations + update_attribute(:remotely_signed_out, false) + # rubocop:enable Rails/SkipsModelValidations + end + + def set_remotely_signed_out! + # rubocop:disable Rails/SkipsModelValidations + update_attribute(:remotely_signed_out, true) + # rubocop:enable Rails/SkipsModelValidations + end + private def requires_name? diff --git a/app/service/navigation_items_service.rb b/app/service/navigation_items_service.rb index 314c3b69f..2465505c6 100644 --- a/app/service/navigation_items_service.rb +++ b/app/service/navigation_items_service.rb @@ -56,7 +56,7 @@ def support_navigation_item def profile_navigation_item return nil if user.name.blank? - NavigationItem.new(text: user.name, href: user_profile_url, active: false) + NavigationItem.new(text: user.name, href: nil, active: false) end def signout_navigation_item @@ -70,19 +70,11 @@ def user_provider end def signout_url - if user_provider == :gds - gds_sign_out_path - elsif %i[auth0 mock_gds_sso].include? user_provider + if %i[auth0 mock_gds_sso].include? user_provider sign_out_path end end - def user_profile_url - if user_provider == :gds - GDS::SSO::Config.oauth_root_url - end - end - def should_show_user_profile_link? Pundit.policy(user, :user).can_manage_user? end diff --git a/config/initializers/authentication.rb b/config/initializers/authentication.rb index 727d62149..c418190cd 100644 --- a/config/initializers/authentication.rb +++ b/config/initializers/authentication.rb @@ -1,8 +1,73 @@ -Rails.application.config.before_initialize do - # Configure OmniAuth authentication middleware - # add Auth0 provider - Rails.application.config.app_middleware.use( - OmniAuth::Strategies::Auth0, +require "warden" + +OmniAuth.config.logger = Rails.logger + +Warden::Manager.after_authentication do |user, _auth, _opts| + # We've successfully signed in. + # If they were remotely signed out, clear the flag as they're no longer suspended + user.clear_remotely_signed_out! +end + +Warden::Manager.serialize_into_session do |user| + if user.respond_to?(:uid) && user.uid + [user.uid, Time.zone.now.utc.iso8601] + end +end + +Warden::Manager.serialize_from_session do |(uid, auth_timestamp)| + # This will reject old sessions that don't have a previous login timestamp + if auth_timestamp.is_a?(String) + begin + auth_timestamp = Time.zone.parse(auth_timestamp) + rescue ArgumentError + auth_timestamp = nil + end + end + + if auth_timestamp && ((auth_timestamp + Settings.auth_valid_for) > Time.zone.now.utc) + User.where(uid:, remotely_signed_out: false).first + end +end + +Rails.application.config.app_middleware.use Warden::Manager do |warden| + warden.default_strategies(Settings.auth_provider.to_sym, :gds_bearer_token) + warden.failure_app = AuthenticationController +end + +# Rails.application.config.before_initialize do +# Configure OmniAuth authentication middleware +# add Auth0 provider +# Rails.application.config.app_middleware.use( +# OmniAuth::Strategies::Auth0, +# Settings.auth0.client_id, +# Settings.auth0.client_secret, +# Settings.auth0.domain, +# callback_path: "/auth/auth0/callback", +# authorize_params: { +# scope: "openid email", +# connection: "email", # default to using the passwordless flow +# }, +# ) + +# config.app_middleware.use ::OmniAuth::Builder do +# next if GDS::SSO::Config.api_only + +# provider :gds, GDS::SSO::Config.oauth_id, GDS::SSO::Config.oauth_secret, +# client_options: { +# site: GDS::SSO::Config.oauth_root_url, +# authorize_url: "#{GDS::SSO::Config.oauth_root_url}/oauth/authorize", +# token_url: "#{GDS::SSO::Config.oauth_root_url}/oauth/access_token", +# connection_opts: { +# headers: { +# user_agent: "gds-sso/#{GDS::SSO::VERSION} (#{ENV['GOVUK_APP_NAME']})", +# }, +# }, +# } +# end + +Rails.application.config.middleware.use OmniAuth::Builder do + provider( + :auth0, Settings.auth0.client_id, Settings.auth0.client_secret, Settings.auth0.domain, @@ -12,13 +77,9 @@ connection: "email", # default to using the passwordless flow }, ) +end - # Configure Warden session management middleware - # swap out the Warden::Manager installed by `gds-sso` gem - Rails.application.config.app_middleware.swap Warden::Manager, Warden::Manager do |warden| - warden.default_strategies(Settings.auth_provider.to_sym, :gds_bearer_token) - warden.failure_app = AuthenticationController - end +OmniAuth.config.allowed_request_methods = %i[post get] - GDS::SSO::Config.auth_valid_for = Settings.auth_valid_for -end +# GDS::SSO::Config.auth_valid_for = Settings.auth_valid_for +# end diff --git a/config/initializers/warden/strategies/basic_auth.rb b/config/initializers/warden/strategies/basic_auth.rb index c5821064a..64bc9f0c9 100644 --- a/config/initializers/warden/strategies/basic_auth.rb +++ b/config/initializers/warden/strategies/basic_auth.rb @@ -34,4 +34,10 @@ def authenticate! custom! [@status, headers, [@message]] end end + +private + + def logger + Rails.logger || env["rack.logger"] + end end diff --git a/config/routes.rb b/config/routes.rb index 8ac96b049..4b24831cb 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -7,6 +7,7 @@ get "/sign-up" => "authentication#sign_up", as: :sign_up get "/sign-out" => "authentication#sign_out", as: :sign_out + get "/auth/failure" => "authentication#failure" scope "auth/:provider" do get "/callback" => "authentication#callback_from_omniauth" diff --git a/lib/warden/strategies/omniauth.rb b/lib/warden/strategies/omniauth.rb index badf22048..1fff8f197 100644 --- a/lib/warden/strategies/omniauth.rb +++ b/lib/warden/strategies/omniauth.rb @@ -17,5 +17,9 @@ def authenticate! def prep_user(auth_hash) raise NotImplementedError end + + def logger + Rails.logger || env["rack.logger"] + end end end diff --git a/spec/integration/gds_sso_spec.rb b/spec/integration/gds_sso_spec.rb deleted file mode 100644 index 973fbc7dc..000000000 --- a/spec/integration/gds_sso_spec.rb +++ /dev/null @@ -1,119 +0,0 @@ -require "rails_helper" - -RSpec.describe "usage of gds-sso gem" do - before do - allow(Settings).to receive(:auth_provider).and_return("gds_sso") - end - - after do - OmniAuth.config.mock_auth[:gds] = nil - OmniAuth.config.test_mode = false - end - - let(:omniauth_hash) do - OmniAuth::AuthHash.new({ - provider: "gds", - uid: "123456", - info: { - email: "test@example.com", - name: "Test User", - }, - extra: { - user: { - permissions: ["---"], - organisation_slug: "test-org", - organisation_content_id: "00000000-0000-0000-0000-000000000000", - disabled: false, - }, - }, - }) - end - - describe "authentication" do - before do - OmniAuth.config.test_mode = true - end - - it "redirects to OmniAuth when no user is logged in" do - logout - - get root_path - - expect(response).to redirect_to("/auth/gds") - end - - it "authenticates with OmniAuth and Warden" do - OmniAuth.config.mock_auth[:gds] = omniauth_hash - - get "/auth/gds" - - expect(response).to redirect_to(gds_sign_in_path) - - get gds_sign_in_path - - expect(request.env["warden"].authenticated?).to be true - end - end - - describe "signing out" do - it "signs the user out of GOV.UK Signon" do - get gds_sign_out_path - - expect(response).to redirect_to("http://signon.dev.gov.uk/users/sign_out") - end - end - - describe User do - describe ".find_for_gds_oauth" do - it "is called by the gds_sso Warden strategy" do - allow(described_class).to receive(:find_for_gds_oauth).and_call_original - allow(described_class).to receive(:find_for_auth).and_call_original - - gds_sso = Warden::Strategies[:gds_sso].new({ - "omniauth.auth" => omniauth_hash, - }) - gds_sso.authenticate! - - expect(described_class).to have_received(:find_for_gds_oauth) - - expect(described_class).to have_received(:find_for_auth).with( - provider: "gds", - uid: "123456", - email: "test@example.com", - name: "Test User", - permissions: ["---"], - organisation_slug: "test-org", - organisation_content_id: "00000000-0000-0000-0000-000000000000", - disabled: false, - ) - - expect(gds_sso.successful?).to be true - end - end - end - - describe "failure page" do - it "is shown if there is an authentication failure with external provider" do - OmniAuth.config.test_mode = true - OmniAuth.config.mock_auth[:gds] = :invalid_credentials - - logout - - get root_path - - expect(response).to redirect_to "/auth/gds" - follow_redirect! - - expect(response).to redirect_to "/auth/gds/callback" - follow_redirect! - - expect(response).to redirect_to "/auth/failure?message=invalid_credentials&strategy=gds" - end - - it "has a retry link" do - get "/auth/failure?message=invalid_credentials&strategy=gds" - - expect(response.body).to include 'try again' - end - end -end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 474f5de68..d1811693e 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1,4 +1,3 @@ -require "gds-sso/lint/user_spec" require "rails_helper" describe User, type: :model do @@ -21,8 +20,6 @@ end end - it_behaves_like "a gds-sso user class" - describe "role" do it "is invalid if blank" do user.role = nil diff --git a/spec/requests/authentication_controller_spec.rb b/spec/requests/authentication_controller_spec.rb index 6000cfdb5..fbdb35cad 100644 --- a/spec/requests/authentication_controller_spec.rb +++ b/spec/requests/authentication_controller_spec.rb @@ -93,17 +93,11 @@ def authenticate! context "when the user's session expires" do before do allow(controller_spy).to receive(:redirect_to_omniauth).and_call_original - - # shorten the auth_valid_for time for testing - GDS::SSO::Config.auth_valid_for = 1 + allow(Settings).to receive(:auth_valid_for).and_return(1) logout end - after do - GDS::SSO::Config.auth_valid_for = Settings.auth_valid_for - end - it "re-authenticates after the configured time" do login_as_editor_user @@ -122,17 +116,17 @@ def authenticate! end describe "#callback_from_omniauth" do - it "is called by OmniAuth provider" do - get "/auth/gds" + # it "is called by OmniAuth provider" do + # get "/auth/gds" - expect(response).to redirect_to("/auth/gds/callback") + # expect(response).to redirect_to("/auth/gds/callback") - allow(controller_spy).to receive(:callback_from_omniauth).and_call_original + # allow(controller_spy).to receive(:callback_from_omniauth).and_call_original - get "/auth/gds/callback" + # get "/auth/gds/callback" - expect(controller_spy).to have_received :callback_from_omniauth - end + # expect(controller_spy).to have_received :callback_from_omniauth + # end it "calls Warden strategy" do allow(controller_spy).to receive(:authenticate_user!).and_call_original diff --git a/spec/service/navigation_items_service_spec.rb b/spec/service/navigation_items_service_spec.rb index 182cd807f..228039a4d 100644 --- a/spec/service/navigation_items_service_spec.rb +++ b/spec/service/navigation_items_service_spec.rb @@ -45,20 +45,6 @@ end end - context "when user has provider gds" do - let(:provider) { :gds } - - it "includes correct profile in navigation items" do - profile_item = NavigationItemsService::NavigationItem.new(text: user.name, href: GDS::SSO::Config.oauth_root_url, active: false) - expect(service.navigation_items).to include(profile_item) - end - - it "includes correct signout in navigation items" do - signout_item = NavigationItemsService::NavigationItem.new(text: I18n.t("header.sign_out"), href: gds_sign_out_path, active: false) - expect(service.navigation_items).to include(signout_item) - end - end - context "when user has provider basic_auth" do let(:provider) { :basic_auth }