diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 72729222b8f..f41cccbc3f9 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -407,4 +407,16 @@ def add_sp_cost(token) def mobile? BrowserCache.parse(request.user_agent).mobile? end + + def user_is_banned? + return false unless user_signed_in? + BannedUserResolver.new(current_user).banned_for_sp?(issuer: current_sp&.issuer) + end + + def handle_banned_user + return unless user_is_banned? + analytics.track_event(Analytics::BANNED_USER_REDIRECT) + sign_out + redirect_to banned_user_url + end end diff --git a/app/controllers/banned_user_controller.rb b/app/controllers/banned_user_controller.rb new file mode 100644 index 00000000000..287219e07f7 --- /dev/null +++ b/app/controllers/banned_user_controller.rb @@ -0,0 +1,5 @@ +class BannedUserController < ApplicationController + def show + analytics.track_event(Analytics::BANNED_USER_VISITED) + end +end diff --git a/app/controllers/openid_connect/authorization_controller.rb b/app/controllers/openid_connect/authorization_controller.rb index eda2da11c66..a7e182d3e2b 100644 --- a/app/controllers/openid_connect/authorization_controller.rb +++ b/app/controllers/openid_connect/authorization_controller.rb @@ -13,6 +13,7 @@ class AuthorizationController < ApplicationController before_action :store_request, only: [:index] before_action :check_sp_active, only: [:index] before_action :apply_secure_headers_override, only: [:index] + before_action :handle_banned_user before_action :confirm_user_is_authenticated_with_fresh_mfa, only: :index before_action :prompt_for_password_if_ial2_request_and_pii_locked, only: [:index] before_action :bump_auth_count, only: [:index] diff --git a/app/controllers/saml_idp_controller.rb b/app/controllers/saml_idp_controller.rb index d6d4d0d1565..9b78e36685f 100644 --- a/app/controllers/saml_idp_controller.rb +++ b/app/controllers/saml_idp_controller.rb @@ -17,6 +17,7 @@ class SamlIdpController < ApplicationController prepend_before_action :skip_session_expiration, only: :metadata skip_before_action :verify_authenticity_token + before_action :handle_banned_user before_action :confirm_user_is_authenticated_with_fresh_mfa, only: :auth before_action :bump_auth_count, only: [:auth] diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index 8f5782f7118..a4e8a71ef2a 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -199,7 +199,11 @@ def request_id end def next_url_after_valid_authentication - if pending_account_reset_request.present? + if user_is_banned? + analytics.track_event(Analytics::BANNED_USER_REDIRECT) + sign_out + banned_user_url + elsif pending_account_reset_request.present? account_reset_pending_url elsif current_user.accepted_rules_of_use_still_valid? user_two_factor_authentication_url diff --git a/app/models/sign_in_restriction.rb b/app/models/sign_in_restriction.rb new file mode 100644 index 00000000000..24fc8866ce1 --- /dev/null +++ b/app/models/sign_in_restriction.rb @@ -0,0 +1,3 @@ +class SignInRestriction < ApplicationRecord + belongs_to :user +end diff --git a/app/models/user.rb b/app/models/user.rb index 2cb395a33ab..5260b7d00f4 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -49,6 +49,7 @@ class User < ApplicationRecord has_many :service_providers, through: :identities, source: :service_provider_record + has_many :sign_in_restrictions, dependent: :destroy attr_accessor :asserted_attributes diff --git a/app/services/analytics.rb b/app/services/analytics.rb index 28bb6f9843a..f0f37e76a48 100644 --- a/app/services/analytics.rb +++ b/app/services/analytics.rb @@ -133,6 +133,8 @@ def browser_attributes AUTHENTICATION_CONFIRMATION = 'Authentication Confirmation' AUTHENTICATION_CONFIRMATION_CONTINUE = 'Authentication Confirmation: Continue selected' AUTHENTICATION_CONFIRMATION_RESET = 'Authentication Confirmation: Reset selected' + BANNED_USER_REDIRECT = 'Banned User redirected' + BANNED_USER_VISITED = 'Banned User visited' DOC_AUTH = 'Doc Auth' # visited or submitted is appended DOC_AUTH_ASYNC = 'Doc Auth Async' DOC_AUTH_WARNING = 'Doc Auth Warning' diff --git a/app/services/banned_user_resolver.rb b/app/services/banned_user_resolver.rb new file mode 100644 index 00000000000..df903618bb7 --- /dev/null +++ b/app/services/banned_user_resolver.rb @@ -0,0 +1,11 @@ +class BannedUserResolver + attr_reader :user + + def initialize(user) + @user = user + end + + def banned_for_sp?(issuer:) + user.sign_in_restrictions.where(service_provider: [nil, issuer]).any? + end +end diff --git a/app/views/banned_user/show.html.erb b/app/views/banned_user/show.html.erb new file mode 100644 index 00000000000..fefe2847901 --- /dev/null +++ b/app/views/banned_user/show.html.erb @@ -0,0 +1,7 @@ +<% title t('banned_user.title') %> + +<%= image_tag('alert/fail-x.svg', width: 54, alt: '', class: 'display-block margin-bottom-4') %> +<%= render PageHeadingComponent.new.with_content(t('banned_user.title')) %> +

+ <%= t('banned_user.details') %> +

diff --git a/config/locales/banned_user/en.yml b/config/locales/banned_user/en.yml new file mode 100644 index 00000000000..22ee04cfc76 --- /dev/null +++ b/config/locales/banned_user/en.yml @@ -0,0 +1,4 @@ +en: + banned_user: + details: We are unable to authenticate you at this time. + title: Access is restricted diff --git a/config/locales/banned_user/es.yml b/config/locales/banned_user/es.yml new file mode 100644 index 00000000000..30048bf1887 --- /dev/null +++ b/config/locales/banned_user/es.yml @@ -0,0 +1,4 @@ +es: + banned_user: + details: No podemos autenticarlo en este momento. + title: El acceso está restringido diff --git a/config/locales/banned_user/fr.yml b/config/locales/banned_user/fr.yml new file mode 100644 index 00000000000..a401a1a6355 --- /dev/null +++ b/config/locales/banned_user/fr.yml @@ -0,0 +1,4 @@ +fr: + banned_user: + details: Nous ne sommes pas en mesure de vous authentifier pour le moment. + title: L’accès est restreint diff --git a/config/routes.rb b/config/routes.rb index f2ee15d744f..738cec47974 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -259,6 +259,8 @@ delete '/users' => 'users#destroy', as: :destroy_user + get '/restricted' => 'banned_user#show', as: :banned_user + scope '/verify', as: 'idv' do get '/' => 'idv#index' get '/activated' => 'idv#activated' diff --git a/db/primary_migrate/20220129181752_create_sign_in_restrictions.rb b/db/primary_migrate/20220129181752_create_sign_in_restrictions.rb new file mode 100644 index 00000000000..b192be905bb --- /dev/null +++ b/db/primary_migrate/20220129181752_create_sign_in_restrictions.rb @@ -0,0 +1,17 @@ +class CreateSignInRestrictions < ActiveRecord::Migration[6.1] + def change + create_table :sign_in_restrictions do |t| + t.integer :user_id, null: false + t.string :service_provider + + t.timestamps + end + + add_index( + :sign_in_restrictions, + [:user_id, :service_provider], + unique: true, + name: :index_sign_in_restrictions_on_user_id_and_service_provider, + ) + end +end diff --git a/db/schema.rb b/db/schema.rb index 228a9b9210f..65198bbd79f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2022_01_14_183203) do +ActiveRecord::Schema.define(version: 2022_01_29_181752) do # These are extensions that must be enabled in order to support this database enable_extension "pg_stat_statements" @@ -534,6 +534,14 @@ t.index ["issuer"], name: "index_service_providers_on_issuer", unique: true end + create_table "sign_in_restrictions", force: :cascade do |t| + t.integer "user_id", null: false + t.string "service_provider" + t.datetime "created_at", precision: 6, null: false + t.datetime "updated_at", precision: 6, null: false + t.index ["user_id", "service_provider"], name: "index_sign_in_restrictions_on_user_id_and_service_provider", unique: true + end + create_table "sp_costs", force: :cascade do |t| t.string "issuer", null: false t.integer "agency_id", null: false diff --git a/spec/features/sign_in/banned_users_spec.rb b/spec/features/sign_in/banned_users_spec.rb new file mode 100644 index 00000000000..f2d75d985a8 --- /dev/null +++ b/spec/features/sign_in/banned_users_spec.rb @@ -0,0 +1,70 @@ +require 'rails_helper' + +feature 'Banning users for an SP' do + include SamlAuthHelper + + context 'a user is banned from all SPs' do + it 'does not let the user sign in to any SP' do + user = create(:user, :signed_up) + + SignInRestriction.create(user: user) + + sign_in_user(user) + expect_user_to_be_banned + + visit_idp_from_sp_with_ial1(:saml) + sign_in_user(user) + expect_user_to_be_banned + + visit_idp_from_sp_with_ial1(:oidc) + sign_in_user(user) + expect_user_to_be_banned + end + end + + context 'a user is banned for a SAML SP' do + it 'bans the user from signing in to the banned SP but allows other sign ins' do + user = create(:user, :signed_up) + + SignInRestriction.create(user: user, service_provider: 'http://localhost:3000') + + sign_in_live_with_2fa(user) + expect(current_path).to eq(account_path) + + visit_idp_from_sp_with_ial1(:saml) + expect_user_to_be_banned + + visit_idp_from_sp_with_ial1(:oidc) + sign_in_live_with_2fa(user) + click_agree_and_continue + expect(current_url).to start_with('http://localhost:7654/auth/result') + end + end + + context 'a user is banner for an OIDC SP' do + it 'bans the user from signing in to the banned SP but allows other sign ins' do + user = create(:user, :signed_up) + + SignInRestriction.create(user: user, service_provider: 'urn:gov:gsa:openidconnect:sp:server') + + sign_in_live_with_2fa(user) + expect(current_path).to eq(account_path) + + visit_idp_from_sp_with_ial1(:oidc) + expect_user_to_be_banned + + visit_idp_from_sp_with_ial1(:saml) + sign_in_live_with_2fa(user) + click_agree_and_continue + expect(current_path).to eq(api_saml_auth2022_path) + end + end + + def expect_user_to_be_banned + expect(current_path).to eq(banned_user_path) + expect(page).to have_content(I18n.t('banned_user.title')) + + visit account_path + expect(current_path).to eq(new_user_session_path) + end +end diff --git a/spec/services/banned_user_resolver_spec.rb b/spec/services/banned_user_resolver_spec.rb new file mode 100644 index 00000000000..1a233ea9585 --- /dev/null +++ b/spec/services/banned_user_resolver_spec.rb @@ -0,0 +1,47 @@ +require 'rails_helper' + +describe BannedUserResolver do + context 'the user is not banned' do + it 'returns false' do + user = create(:user) + sp = create(:service_provider) + + expect(described_class.new(user).banned_for_sp?(issuer: sp.issuer)).to eq(false) + end + end + + context 'the user is banned for a single SP' do + it 'returns true for the banned SP' do + user = create(:user) + sp = create(:service_provider) + + SignInRestriction.create(user: user, service_provider: sp.issuer) + + expect(described_class.new(user).banned_for_sp?(issuer: sp.issuer)).to eq(true) + end + + it 'returns false for the not banned SP' do + user = create(:user) + sp = create(:service_provider) + banned_sp = create(:service_provider) + + SignInRestriction.create(user: user, service_provider: banned_sp.issuer) + + expect(described_class.new(user).banned_for_sp?(issuer: sp.issuer)).to eq(false) + end + end + + context 'the user is banned for all SPs' do + it 'returns true for all SPs' do + user = create(:user) + sp1 = create(:service_provider) + sp2 = create(:service_provider) + + SignInRestriction.create(user: user, service_provider: nil) + + expect(described_class.new(user).banned_for_sp?(issuer: sp1.issuer)).to eq(true) + expect(described_class.new(user).banned_for_sp?(issuer: sp2.issuer)).to eq(true) + expect(described_class.new(user).banned_for_sp?(issuer: nil)).to eq(true) + end + end +end