Skip to content

Commit

Permalink
LG-225 Remove Agency Based UUID feature flags to prevent reverting to…
Browse files Browse the repository at this point in the history
… old SP UUIDs

**Why**: To prevent reverting to old sp based uuids which would break agencies. This feature gave us the flexibility to turn the agency based uuids on for slow roll or rollback in the event of failures. However once the agencies migrated to the new uuid structure and the rollout is error free it should not be turned off again because users would then be associated with the wrong uuids.

**How**: Assume every logic path for the two feature flags is now true and remove unneccessary code in addition to removing the actual feature flags.
  • Loading branch information
stevegsa committed Apr 27, 2018
1 parent 1e366f4 commit 3dd6ade
Show file tree
Hide file tree
Showing 20 changed files with 9 additions and 154 deletions.
6 changes: 1 addition & 5 deletions app/controllers/concerns/saml_idp_logout_concern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,7 @@ def name_id_user

def sp_slo_identity
@_sp_slo_identity ||= begin
if FeatureManagement.enable_agency_based_uuids?
AgencyIdentityLinker.sp_identity_from_uuid(name_id)
else
Identity.includes(:user).find_by(uuid: name_id)
end
AgencyIdentityLinker.sp_identity_from_uuid(name_id)
end
end

Expand Down
6 changes: 1 addition & 5 deletions app/forms/openid_connect_logout_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,7 @@ def load_identity
def identity_from_payload(payload)
uuid = payload[:sub]
sp = payload[:aud]
if FeatureManagement.enable_agency_based_uuids?
AgencyIdentityLinker.sp_identity_from_uuid_and_sp(uuid, sp)
else
Identity.where(uuid: uuid, service_provider: sp).first
end
AgencyIdentityLinker.sp_identity_from_uuid_and_sp(uuid, sp)
end

def build_openid_connect_redirector
Expand Down
4 changes: 0 additions & 4 deletions app/models/agency_identity.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,4 @@ class AgencyIdentity < ApplicationRecord
belongs_to :user
belongs_to :agency
validates :uuid, presence: true

def agency_enabled?
!FeatureManagement.agencies_with_agency_based_uuids.index(agency_id).nil?
end
end
6 changes: 1 addition & 5 deletions app/presenters/openid_connect_user_info_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,7 @@ def user_info
private

def uuid_from_sp_identity(identity)
if FeatureManagement.enable_agency_based_uuids?
AgencyIdentityLinker.new(identity).link_identity.uuid
else
identity.uuid
end
AgencyIdentityLinker.new(identity).link_identity.uuid
end

# rubocop:disable Metrics/AbcSize
Expand Down
11 changes: 4 additions & 7 deletions app/services/agency_identity_linker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,13 @@ def initialize(sp_identity)
end

def link_identity
ai = find_or_create_agency_identity
return ai if ai&.agency_enabled?
AgencyIdentity.new(user_id: @sp_identity.user_id, uuid: @sp_identity.uuid)
find_or_create_agency_identity ||
AgencyIdentity.new(user_id: @sp_identity.user_id, uuid: @sp_identity.uuid)
end

def self.sp_identity_from_uuid_and_sp(uuid, service_provider)
ai = AgencyIdentity.where(uuid: uuid).first
criteria = if ai&.agency_enabled?
criteria = if ai
{ user_id: ai.user_id, service_provider: service_provider }
else
{ uuid: uuid, service_provider: service_provider }
Expand All @@ -31,9 +30,7 @@ def self.sp_identity_from_uuid(uuid)
private

def find_or_create_agency_identity
ai = agency_identity
return ai if ai
create_agency_identity_for_sp
agency_identity || create_agency_identity_for_sp
end

def create_agency_identity_for_sp
Expand Down
6 changes: 1 addition & 5 deletions app/services/attribute_asserter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,7 @@ def add_bundle(attrs)
def uuid_getter_function
lambda do |principal|
identity = principal.decorate.active_identity_for(service_provider)
if FeatureManagement.enable_agency_based_uuids?
AgencyIdentityLinker.new(identity).link_identity.uuid
else
identity.uuid
end
AgencyIdentityLinker.new(identity).link_identity.uuid
end
end

Expand Down
2 changes: 1 addition & 1 deletion app/services/identity_linker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ def initialize(user, provider)
def link_identity(**extra_attrs)
attributes = merged_attributes(extra_attrs)
identity.update!(attributes)
AgencyIdentityLinker.new(identity).link_identity if FeatureManagement.enable_agency_based_uuids?
AgencyIdentityLinker.new(identity).link_identity
identity
end

Expand Down
6 changes: 0 additions & 6 deletions config/application.yml.example
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ development:
aamva_public_key: '123abc'
aamva_private_key: '123abc'
aamva_verification_url: 'https://example.org:12345/verification/url'
agencies_with_agency_based_uuids: '1,2,3,4,5'
async_job_refresh_interval_seconds: '5'
async_job_refresh_max_wait_seconds: '15'
attribute_cost: '4000$8$4$' # SCrypt::Engine.calibrate(max_time: 0.5)
Expand All @@ -87,7 +86,6 @@ development:
database_timeout: '5000'
database_username: ''
domain_name: 'localhost:3000'
enable_agency_based_uuids: 'true'
enable_identity_verification: 'true'
enable_rate_limiting: 'false'
enable_test_routes: 'true'
Expand Down Expand Up @@ -161,7 +159,6 @@ production:
aamva_public_key: # Base64 encoded public key for AAMVA
aamva_private_key: # Base64 encoded private key for AAMVA
aamva_verification_url: # DLDV Verification URL
agencies_with_agency_based_uuids: '1,2,3,4,5'
async_job_refresh_interval_seconds: '5'
async_job_refresh_max_wait_seconds: '15'
attribute_cost: '4000$8$4$' # SCrypt::Engine.calibrate(max_time: 0.5)
Expand All @@ -176,7 +173,6 @@ production:
disable_email_sending: 'false'
dashboard_api_token:
domain_name: 'login.gov'
enable_agency_based_uuids: 'true'
enable_identity_verification: 'false'
enable_rate_limiting: 'true'
enable_test_routes: 'false'
Expand Down Expand Up @@ -248,7 +244,6 @@ test:
aamva_public_key: '123abc'
aamva_private_key: '123abc'
aamva_verification_url: 'https://example.org:12345/verification/url'
agencies_with_agency_based_uuids: '1,2,3,4,5'
async_job_refresh_interval_seconds: '1'
async_job_refresh_max_wait_seconds: '15'
attribute_cost: '800$8$1$' # SCrypt::Engine.calibrate(max_time: 0.01)
Expand All @@ -271,7 +266,6 @@ test:
database_timeout: '5000'
database_username: ''
dashboard_api_token: '123ABC'
enable_agency_based_uuids: 'true'
enable_identity_verification: 'true'
enable_rate_limiting: 'true'
enable_test_routes: 'true'
Expand Down
1 change: 0 additions & 1 deletion config/initializers/figaro.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
'attribute_cost',
'attribute_encryption_key',
'domain_name',
'enable_agency_based_uuids',
'enable_identity_verification',
'enable_rate_limiting',
'enable_test_routes',
Expand Down
8 changes: 0 additions & 8 deletions lib/feature_management.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,6 @@ def self.no_pii_mode?
enable_identity_verification? && Figaro.env.profile_proofing_vendor == :mock
end

def self.enable_agency_based_uuids?
Figaro.env.enable_agency_based_uuids == 'true'
end

def self.agencies_with_agency_based_uuids
(Figaro.env.agencies_with_agency_based_uuids || '').split(',').map(&:to_i)
end

def self.enable_saml_cert_rotation?
Figaro.env.saml_secret_rotation_enabled == 'true'
end
Expand Down
5 changes: 0 additions & 5 deletions spec/features/openid_connect/openid_connect_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,6 @@
oidc_end_client_secret_jwt(prompt: 'select_account')
end

it 'succeeds with new agency based uuids' do
allow(FeatureManagement).to receive(:enable_agency_based_uuids?).and_return(true)
oidc_end_client_secret_jwt(prompt: 'select_account')
end

it 'succeeds in returning back to sp with prompt select_account and prior session' do
user = oidc_end_client_secret_jwt(prompt: 'select_account')
oidc_end_client_secret_jwt(prompt: 'select_account', user: user, redirs_to: '/auth/result')
Expand Down
13 changes: 0 additions & 13 deletions spec/features/saml/loa1_sso_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -175,19 +175,6 @@
end
end

context 'fully signed up user is signed in with email/pwd and new agency based uuids' do
it 'prompts to enter OTP' do
allow(FeatureManagement).to receive(:enable_agency_based_uuids?).and_return(true)
user = create(:user, :signed_up)
sign_in_user(user)

saml_authn_request = auth_request.create(saml_settings)
visit saml_authn_request

expect(current_path).to eq login_two_factor_path(otp_delivery_preference: 'sms')
end
end

context 'user that has not yet set up 2FA is signed in with email and password only' do
it 'prompts to set up 2FA' do
sign_in_user
Expand Down
1 change: 0 additions & 1 deletion spec/features/saml/sp_initiated_slo_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@
let(:user) { create(:user, :signed_up) }

before do
allow(FeatureManagement).to receive(:enable_agency_based_uuids?).and_return(true)
sign_in_and_2fa_user(user)
visit sp1_authnrequest

Expand Down
12 changes: 0 additions & 12 deletions spec/forms/openid_connect_logout_form_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,6 @@
it 'has a successful response' do
expect(result).to be_success
end

it 'has a successful response when agency based uuids are enabled' do
allow(FeatureManagement).to receive(:enable_agency_based_uuids?).and_return(true)
expect(result).to be_success
end
end

context 'with an invalid form' do
Expand Down Expand Up @@ -118,13 +113,6 @@
expect(form.errors[:id_token_hint]).
to include(t('openid_connect.logout.errors.id_token_hint'))
end

it 'is not valid when agency based uuids are enabled' do
allow(FeatureManagement).to receive(:enable_agency_based_uuids?).and_return(true)
expect(valid?).to eq(false)
expect(form.errors[:id_token_hint]).
to include(t('openid_connect.logout.errors.id_token_hint'))
end
end

context 'with an expired, but otherwise valid id_token_hint' do
Expand Down
54 changes: 0 additions & 54 deletions spec/lib/feature_management_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -226,58 +226,4 @@
end
end
end

describe '#enable_agency_based_uuids?' do
context 'when enabled' do
before do
allow(Figaro.env).to receive(:enable_agency_based_uuids).and_return('true')
end

it 'enables the feature' do
expect(FeatureManagement.enable_agency_based_uuids?).to eq(true)
end
end

context 'when disabled' do
before do
allow(Figaro.env).to receive(:enable_agency_based_uuids).and_return('false')
end

it 'disables the feature' do
expect(FeatureManagement.enable_agency_based_uuids?).to eq(false)
end
end
end

describe 'agencies_with_agency_based_uuids' do
context 'when multiple agencies are enabled' do
before do
allow(Figaro.env).to receive(:agencies_with_agency_based_uuids).and_return('1,2,3')
end

it 'it returns an array of agencies' do
expect(FeatureManagement.agencies_with_agency_based_uuids).to eq([1, 2, 3])
end
end

context 'when one agency is enabled' do
before do
allow(Figaro.env).to receive(:agencies_with_agency_based_uuids).and_return('1')
end

it 'returns an array containing a single agency' do
expect(FeatureManagement.agencies_with_agency_based_uuids).to eq([1])
end
end

context 'when blank' do
before do
allow(Figaro.env).to receive(:agencies_with_agency_based_uuids).and_return('')
end

it 'returns an empty array' do
expect(FeatureManagement.agencies_with_agency_based_uuids).to eq([])
end
end
end
end
14 changes: 0 additions & 14 deletions spec/models/agency_identity_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,4 @@

it { is_expected.to validate_presence_of(:uuid) }
end

describe '#agency_enabled?' do
it 'returns true if the agency is enabled' do
allow(Figaro.env).to receive(:agencies_with_agency_based_uuids).and_return('1')
ai = AgencyIdentity.new(agency_id: 1, user_id: 1, uuid: 'UUID1')
expect(ai.agency_enabled?).to eq(true)
end

it 'returns false if the agency is disabled' do
allow(Figaro.env).to receive(:agencies_with_agency_based_uuids).and_return('')
ai = AgencyIdentity.new(agency_id: 1, user_id: 1, uuid: 'UUID1')
expect(ai.agency_enabled?).to eq(false)
end
end
end
2 changes: 0 additions & 2 deletions spec/services/agency_identity_linker_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,6 @@
end

def init_env(user)
allow(Figaro.env).to receive(:enable_agency_based_uuids).and_return('true')
allow(Figaro.env).to receive(:agencies_with_agency_based_uuids).and_return('1,2,3')
Identity.where(user_id: user.id).delete_all
AgencyIdentity.where(user_id: user.id).delete_all
end
Expand Down
2 changes: 0 additions & 2 deletions spec/services/link_agency_identities_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,6 @@
end

def init_env(user)
allow(Figaro.env).to receive(:enable_agency_based_uuids).and_return('true')
allow(Figaro.env).to receive(:agencies_with_agency_based_uuids).and_return('1,2,3')
AgencySeeder.new(rails_env: Rails.env, deploy_env: Rails.env).run
Identity.where(user_id: user.id).delete_all
AgencyIdentity.where(user_id: user.id).delete_all
Expand Down
2 changes: 0 additions & 2 deletions spec/support/idv_examples/sp_handoff.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
include IdvHelper

before do
allow(Figaro.env).to receive(:enable_agency_based_uuids).and_return('true')
allow(Figaro.env).to receive(:agencies_with_agency_based_uuids).and_return('1,2,3')
allow(FeatureManagement).to receive(:prefill_otp_codes?).and_return(true)
end

Expand Down
2 changes: 0 additions & 2 deletions spec/support/idv_examples/sp_requested_attributes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
include IdvHelper

before do
allow(Figaro.env).to receive(:enable_agency_based_uuids).and_return('true')
allow(Figaro.env).to receive(:agencies_with_agency_based_uuids).and_return('1,2,3')
allow(FeatureManagement).to receive(:prefill_otp_codes?).and_return(true)
end

Expand Down

0 comments on commit 3dd6ade

Please sign in to comment.