Skip to content

WIP: Spike remove gds sso #769

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,8 @@ gem "pg", "~> 1.5"
gem "puma", "~> 6.4.0"

# Used for handling authentication
gem "gds-sso"
gem "omniauth"
gem "omniauth-auth0"
gem "omniauth_openid_connect"
gem "warden"

# Used for handling authorisation policies
Expand Down
59 changes: 0 additions & 59 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,7 @@ GEM
tzinfo (~> 2.0)
addressable (2.8.5)
public_suffix (>= 2.0.2, < 6.0)
aes_key_wrap (1.1.0)
ast (2.4.2)
attr_required (1.0.1)
aws-eventstream (1.2.0)
aws-partitions (1.834.0)
aws-sdk-cloudwatch (1.81.0)
Expand Down Expand Up @@ -129,7 +127,6 @@ GEM
erubi (~> 1.4)
parser (>= 2.4)
smart_properties
bindata (2.4.15)
bootsnap (1.17.0)
msgpack (~> 1.2)
brakeman (6.0.1)
Expand Down Expand Up @@ -209,17 +206,7 @@ GEM
faraday (2.7.10)
faraday-net_http (>= 2.0, < 3.1)
ruby2_keywords (>= 0.0.4)
faraday-follow_redirects (0.3.0)
faraday (>= 1, < 3)
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)
Expand Down Expand Up @@ -259,12 +246,6 @@ GEM
reline (>= 0.3.8)
jmespath (1.6.2)
json (2.6.3)
json-jwt (1.16.3)
activesupport (>= 4.2)
aes_key_wrap
bindata
faraday (~> 2.0)
faraday-follow_redirects
jwt (2.7.1)
language_server-protocol (3.17.0.3)
lograge (0.14.0)
Expand Down Expand Up @@ -324,22 +305,6 @@ GEM
omniauth-oauth2 (1.8.0)
oauth2 (>= 1.4, < 3)
omniauth (~> 2.0)
omniauth_openid_connect (0.7.1)
omniauth (>= 1.9, < 3)
openid_connect (~> 2.2)
openid_connect (2.2.0)
activemodel
attr_required (>= 1.0.0)
faraday (~> 2.0)
faraday-follow_redirects
json-jwt (>= 1.16)
net-smtp
rack-oauth2 (~> 2.2)
swd (~> 2.0)
tzinfo
validate_email
validate_url
webfinger (~> 2.0)
pagy (6.1.0)
paper_trail (15.1.0)
activerecord (>= 6.1)
Expand All @@ -349,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)
Expand All @@ -364,13 +328,6 @@ GEM
rspec-support (~> 3.12)
racc (1.7.3)
rack (2.2.8)
rack-oauth2 (2.2.0)
activesupport
attr_required
faraday (~> 2.0)
faraday-follow_redirects
json-jwt (>= 1.11.0)
rack (>= 2.1.0)
rack-protection (3.1.0)
rack (~> 2.2, >= 2.2.4)
rack-proxy (0.7.7)
Expand Down Expand Up @@ -496,11 +453,6 @@ GEM
hashie
version_gem (~> 1.1, >= 1.1.1)
stringio (3.0.8)
swd (2.0.2)
activesupport (>= 3)
attr_required (>= 0.0.5)
faraday (~> 2.0)
faraday-follow_redirects
terminal-table (3.0.2)
unicode-display_width (>= 1.1.1, < 3)
thor (1.3.0)
Expand All @@ -511,9 +463,6 @@ GEM
tzinfo-data (1.2023.3)
tzinfo (>= 1.0.0)
unicode-display_width (2.5.0)
validate_email (0.1.6)
activemodel (>= 3.0)
mail (>= 2.2.5)
validate_url (1.0.15)
activemodel (>= 3.0.0)
public_suffix
Expand All @@ -535,12 +484,6 @@ GEM
zeitwerk (~> 2.2)
warden (1.2.9)
rack (>= 2.0.9)
warden-oauth2 (0.0.1)
warden
webfinger (2.1.2)
activesupport
faraday (~> 2.0)
faraday-follow_redirects
webmock (3.19.1)
addressable (>= 2.8.0)
crack (>= 0.3.2)
Expand Down Expand Up @@ -575,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)
Expand All @@ -584,7 +526,6 @@ DEPENDENCIES
lograge
omniauth
omniauth-auth0
omniauth_openid_connect
paper_trail
pg (~> 1.5)
puma (~> 6.4.0)
Expand Down
12 changes: 4 additions & 8 deletions app/controllers/authentication_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ def sign_out
end
end

def failure
render "authentications/failure", layout: "application"
end

private

def attempted_path
Expand Down Expand Up @@ -83,14 +87,6 @@ def auth0_sign_out_url
URI::HTTPS.build(host: Settings.auth0.domain, path: "/v2/logout", query: request_params.to_query).to_s
end

def cddo_sso_sign_out_url
request_params = {
from_app: Settings.cddo_sso.identifier,
}

URI::HTTPS.build(host: "sso.service.security.gov.uk", path: "/sign-out", query: request_params.to_query).to_s
end

def mock_gds_sso_sign_out_url
"https://signon.integration.publishing.service.gov.uk/users/sign_out"
end
Expand Down
13 changes: 12 additions & 1 deletion app/models/user.rb
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure why update_attribute is used here - maybe to make sure it happens despite the state the user is in?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need the remotely_signed_out! stuff? The existing stuff is reliant on the GOV.UK Signon servers calling an API exposed by gds_sso gem.

# 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?
Expand Down
15 changes: 2 additions & 13 deletions app/service/navigation_items_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -70,22 +70,11 @@ def user_provider
end

def signout_url
if user_provider == :gds
gds_sign_out_path
elsif %i[auth0 cddo_sso mock_gds_sso].include? user_provider
if %i[auth0 mock_gds_sso].include? user_provider
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be updated

sign_out_path
end
end

def user_profile_url
case user_provider
when :cddo_sso
"https://sso.service.security.gov.uk/profile"
when :gds
GDS::SSO::Config.oauth_root_url
end
end

def should_show_user_profile_link?
Pundit.policy(user, :user).can_manage_user?
end
Expand Down
75 changes: 40 additions & 35 deletions config/initializers/authentication.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,42 @@
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)|
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could add more tests around these parts lifted from gds_sso

# 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.middleware.use OmniAuth::Builder do
provider(
:auth0,
Settings.auth0.client_id,
Settings.auth0.client_secret,
Settings.auth0.domain,
Expand All @@ -12,35 +46,6 @@
connection: "email", # default to using the passwordless flow
},
)

# add CDDO SSO provider
Rails.application.config.app_middleware.use(
OmniAuth::Strategies::OpenIDConnect,
name: :cddo_sso,
issuer: "https://sso.service.security.gov.uk",
discovery: true,
require_state: true,

scope: %i[openid email profile],
client_options: {
identifier: Settings.cddo_sso.identifier,
secret: Settings.cddo_sso.secret,
},
)

# 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

GDS::SSO::Config.auth_valid_for = Settings.auth_valid_for
end

# Monkeypatch omniauth_openid_connect
class OmniAuth::Strategies::OpenIDConnect
def redirect_uri
callback_url
end
end
OmniAuth.config.allowed_request_methods = %i[post get]
6 changes: 6 additions & 0 deletions config/initializers/warden/strategies/basic_auth.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,10 @@ def authenticate!
custom! [@status, headers, [@message]]
end
end

private

def logger
Rails.logger || env["rack.logger"]
end
end
16 changes: 0 additions & 16 deletions config/initializers/warden/strategies/cddo_sso.rb

This file was deleted.

15 changes: 15 additions & 0 deletions config/initializers/warden/strategies/mock_user.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
Warden::Strategies.add(:mock_user) do
def authenticate!
logger.warn("Authenticating with mock_gds_sso strategy")

test_user ||= User.first

if test_user
success!(test_user)
elsif Rails.env.test? && ENV["GDS_SSO_MOCK_INVALID"].present?
fail!(:invalid)
else
raise "Mock_user running in mock mode and no test user found. Normally we'd load the first user in the database. Create a user in the database."
end
end
end
1 change: 1 addition & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
5 changes: 0 additions & 5 deletions config/settings.yml
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,4 @@ basic_auth:
slug: gds-user-research
govuk_content_id: "00000000-0000-0000-0000-000000000000"

# CDDO SSO at sso.service.security.gov.uk via OpenID Connect
cddo_sso:
identifier: changeme
secret:

forms_env: local
2 changes: 1 addition & 1 deletion config/settings/development.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,4 @@ forms_api:
auth_key: changeme

# Use mock authentication
auth_provider: mock_gds_sso
auth_provider: mock_user
Loading