Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
54 commits
Select commit Hold shift + click to select a range
2ea9387
temp commit
mdiarra3 Jul 11, 2024
0a2080e
edit downloader
mdiarra3 Jul 11, 2024
d17d324
fed email domain
mdiarra3 Jul 16, 2024
254c34a
fix
mdiarra3 Jul 17, 2024
d6cb9b8
changelog: Upcoming Features, Authentication, Piv filtering via domains
mdiarra3 Jul 18, 2024
62df787
rubocop fix
mdiarra3 Jul 19, 2024
c112136
use fed email check
mdiarra3 Jul 22, 2024
fbdceda
fix email address
mdiarra3 Jul 22, 2024
006df88
fed email presenter spec
mdiarra3 Jul 22, 2024
7f0f986
Merge remote-tracking branch 'origin/main' into LG-13355-filter-piv-u…
mdiarra3 Jul 22, 2024
81f881b
set up piv cac selection presenter
mdiarra3 Jul 22, 2024
788eb83
add file to gitlab ci and setup
mdiarra3 Jul 23, 2024
86a789b
change to is fed email
mdiarra3 Jul 23, 2024
9c9d636
update specs and sample files
mdiarra3 Jul 23, 2024
7ace915
address failing spec
mdiarra3 Jul 23, 2024
3287200
remove comment
mdiarra3 Jul 23, 2024
423fd19
rename fed email domains spec
mdiarra3 Jul 24, 2024
936d934
add feature flag
mdiarra3 Jul 25, 2024
7cdbb5b
change user
mdiarra3 Jul 25, 2024
29bc68b
use store for config
mdiarra3 Jul 25, 2024
a5c90fa
fix specs errors
mdiarra3 Jul 25, 2024
e8f0833
sign in spec fix
mdiarra3 Jul 25, 2024
279b31c
remove is predicate
mdiarra3 Jul 25, 2024
2e09681
refactor to do db instead of reading a file
mdiarra3 Jul 30, 2024
f4909de
rubocop
mdiarra3 Jul 30, 2024
26faf64
remove downloader and just use a rake task
mdiarra3 Jul 31, 2024
1c8b180
edit domainsg
mdiarra3 Jul 31, 2024
75fd8d7
remove downloader
mdiarra3 Jul 31, 2024
53d8294
edit rake task
mdiarra3 Jul 31, 2024
b842baf
remove unneeded config
mdiarra3 Jul 31, 2024
9074684
edit email domain
mdiarra3 Jul 31, 2024
738308d
fix schema
mdiarra3 Aug 1, 2024
5f8e510
add additional fed email specs
mdiarra3 Aug 2, 2024
026a15d
change name
mdiarra3 Aug 5, 2024
11552b0
move piv cac recommended to separate spec for ease of readibility
mdiarra3 Aug 5, 2024
e946f2b
fix rubocop
mdiarra3 Aug 5, 2024
f6d7b8a
fix specs
mdiarra3 Aug 5, 2024
1b3e04e
Merge remote-tracking branch 'origin/main' into LG-13355-filter-piv-u…
mdiarra3 Aug 5, 2024
f5e6d5f
Merge remote-tracking branch 'origin/main' into LG-13355-filter-piv-u…
mdiarra3 Aug 5, 2024
20bf08d
fix specs to use false and true
mdiarra3 Aug 6, 2024
5f002a9
address comments
mdiarra3 Aug 7, 2024
819b6f7
remove line
mdiarra3 Aug 8, 2024
71eadc5
Merge remote-tracking branch 'origin/main' into LG-13355-filter-piv-u…
mdiarra3 Aug 8, 2024
24a7714
make it simple assignment
mdiarra3 Aug 8, 2024
ce69370
Merge remote-tracking branch 'origin/main' into LG-13355-filter-piv-u…
mdiarra3 Aug 8, 2024
8430b3c
update application yaml default
mdiarra3 Aug 8, 2024
790b615
update rake and application yml default
mdiarra3 Aug 9, 2024
29fb273
update schema
mdiarra3 Aug 9, 2024
49bf127
include default loading of default domain values
mdiarra3 Aug 12, 2024
8753d51
address comments
mdiarra3 Aug 14, 2024
42055fb
Merge remote-tracking branch 'origin/main' into LG-13355-filter-piv-u…
mdiarra3 Aug 26, 2024
d32b774
fix merge conflict
mdiarra3 Aug 26, 2024
9077fa1
update application config
mdiarra3 Aug 26, 2024
06e4152
fix schema
mdiarra3 Aug 26, 2024
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: 1 addition & 1 deletion app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ def user_needs_to_reactivate_account?
end

def user_recommended_for_piv_cac?
current_user.piv_cac_recommended_dismissed_at.nil? && current_user.has_gov_or_mil_email? &&
current_user.piv_cac_recommended_dismissed_at.nil? && current_user.has_fed_or_mil_email? &&
!user_already_has_piv?
end

Expand Down
2 changes: 1 addition & 1 deletion app/controllers/concerns/mfa_setup_concern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def show_skip_additional_mfa_link?
end

def check_if_possible_piv_user
if current_user.has_gov_or_mil_email? && current_user.piv_cac_recommended_dismissed_at.nil?
if current_user.has_fed_or_mil_email? && current_user.piv_cac_recommended_dismissed_at.nil?
redirect_to login_piv_cac_recommended_path
end
end
Expand Down
6 changes: 3 additions & 3 deletions app/controllers/users/piv_cac_recommended_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class PivCacRecommendedController < ApplicationController

before_action :confirm_user_authenticated_for_2fa_setup
before_action :apply_secure_headers_override
before_action :redirect_unless_user_email_is_gov_or_mil
before_action :redirect_unless_user_email_is_fed_or_mil

def show
@recommended_presenter = PivCacRecommendedPresenter.new(current_user)
Expand All @@ -30,8 +30,8 @@ def skip

private

def redirect_unless_user_email_is_gov_or_mil
redirect_to after_sign_in_path_for(current_user) unless current_user.has_gov_or_mil_email?
def redirect_unless_user_email_is_fed_or_mil
redirect_to after_sign_in_path_for(current_user) unless current_user.has_fed_or_mil_email?
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def index
@presenter = two_factor_options_presenter
analytics.user_registration_2fa_setup_visit(
enabled_mfa_methods_count:,
gov_or_mil_email: has_gov_or_mil_email?,
gov_or_mil_email: fed_or_mil_email?,
)
end

Expand Down Expand Up @@ -44,8 +44,8 @@ def two_factor_options_form

private

def has_gov_or_mil_email?
current_user.confirmed_email_addresses.any?(&:gov_or_mil?)
def fed_or_mil_email?
current_user.confirmed_email_addresses.any?(&:fed_or_mil_email?)
end

def mfa_context
Expand Down
21 changes: 19 additions & 2 deletions app/models/email_address.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,25 @@ def confirmation_period_expired?
Time.zone.now > expiration_time
end

def gov_or_mil?
email.end_with?('.gov', '.mil')
def domain
Mail::Address.new(email).domain
end

def fed_or_mil_email?
fed_email? || mil_email?
end

def fed_email?
if IdentityConfig.store.use_fed_domain_class
Comment thread
aduth marked this conversation as resolved.
return false unless domain
FederalEmailDomain.fed_domain?(domain)
else
email.end_with?('.gov')
end
end

def mil_email?
email.end_with?('.mil')
end

class << self
Expand Down
7 changes: 7 additions & 0 deletions app/models/federal_email_domain.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# frozen_string_literal: true

class FederalEmailDomain < ApplicationRecord
def self.fed_domain?(domain)
exists?(name: domain)
end
end
4 changes: 2 additions & 2 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ def confirmed?
email_addresses.where.not(confirmed_at: nil).any?
end

def has_gov_or_mil_email?
confirmed_email_addresses.any?(&:gov_or_mil?)
def has_fed_or_mil_email?
confirmed_email_addresses.any?(&:fed_or_mil_email?)
end

def accepted_rules_of_use_still_valid?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def phishing_resistant?
end

def recommended?
user.confirmed_email_addresses.any?(&:gov_or_mil?)
user.confirmed_email_addresses.any?(&:fed_or_mil_email?)
end

def desktop_only?
Expand Down
1 change: 0 additions & 1 deletion app/presenters/two_factor_options_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ class TwoFactorOptionsPresenter
:user_agent

delegate :two_factor_enabled?, to: :mfa_policy

def initialize(
user_agent:,
user: nil,
Expand Down
3 changes: 3 additions & 0 deletions config/application.yml.default
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,7 @@ test_ssn_allowed_list: ''
totp_code_interval: 30
unauthorized_scope_enabled: false
use_dashboard_service_providers: false
use_fed_domain_class: false
use_kms: false
use_vot_in_sp_requests: true
usps_auth_token_refresh_job_enabled: false
Expand Down Expand Up @@ -432,6 +433,7 @@ development:
state_tracking_enabled: true
telephony_adapter: test
use_dashboard_service_providers: true
use_fed_domain_class: true
usps_eipp_sponsor_id: '222222222222222'
usps_ipp_sponsor_id: '111111111111111'
usps_ipp_transliteration_enabled: true
Expand Down Expand Up @@ -563,6 +565,7 @@ test:
telephony_adapter: test
test_ssn_allowed_list: '999999999'
totp_code_interval: 3
use_fed_domain_class: true
usps_eipp_sponsor_id: '222222222222222'
usps_ipp_root_url: 'http://localhost:1000'
usps_ipp_sponsor_id: '111111111111111'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
class CreateFederalEmailDomain < ActiveRecord::Migration[7.1]
def change
create_table :federal_email_domains do |t|
t.citext :name, null: false
end

add_index :federal_email_domains, :name, unique: true
end
end
5 changes: 5 additions & 0 deletions db/schema.rb
Comment thread
aduth marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,11 @@
t.index ["user_id", "created_at"], name: "index_events_on_user_id_and_created_at"
end

create_table "federal_email_domains", force: :cascade do |t|
t.citext "name", null: false
t.index ["name"], name: "index_federal_email_domains_on_name", unique: true
end

create_table "fraud_review_requests", force: :cascade do |t|
t.integer "user_id"
t.string "uuid"
Expand Down
1 change: 1 addition & 0 deletions lib/identity_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,7 @@ def self.store
config.add(:usps_auth_token_refresh_job_enabled, type: :boolean)
config.add(:usps_confirmation_max_days, type: :integer)
config.add(:usps_eipp_sponsor_id, type: :string)
config.add(:use_fed_domain_class, type: :boolean)
config.add(:usps_ipp_client_id, type: :string)
config.add(:usps_ipp_password, type: :string)
config.add(:usps_ipp_request_timeout, type: :integer)
Expand Down
17 changes: 17 additions & 0 deletions lib/tasks/federal_email_domains.rake
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# frozen_string_literal: true

require 'faraday'
require 'csv'

DOT_GOV_DOWNLOAD_PATH = 'https://raw.githubusercontent.com/cisagov/dotgov-data/main/current-federal.csv'
namespace :federal_email_domains do
task load_to_db: :environment do |_task, _args|
response = Faraday.get(DOT_GOV_DOWNLOAD_PATH)

csv = CSV.parse(response.body, col_sep: ',', headers: true)
csv.each do |row|
FederalEmailDomain.find_or_create_by(name: row['Domain name'])
end
Comment on lines +11 to +14
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is probably fine since we're only dealing with about 1400 entries, but I'd wonder if this should work a little more like the disposable emails task using insert_all. I think upsert_all could be an option too if we wanted to gracefully handle duplicates. I think the "find" element here is kinda wasted since we don't care to do anything with the result, so we could also just insert and rescue and discard on a ActiveRecord::RecordNotUnique exception.

If this was a bigger file, I'd also suggest using CSV.foreach instead of CSV.parse + Array#each, since it'd process row-by-row instead of loading the whole thing into memory first.

end
end
# rake "federal_email_domains:load_to_db"
13 changes: 8 additions & 5 deletions spec/controllers/users/piv_cac_recommended_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

RSpec.describe Users::PivCacRecommendedController do
describe 'New user' do
let(:user) { create(:user, email: 'example@example.gov') }
let(:user) { create(:user, email: 'example@gsa.gov') }
let!(:federal_domain) { create(:federal_email_domain, name: 'gsa.gov') }
before do
stub_sign_in_before_2fa(user)
stub_analytics
Expand All @@ -28,10 +29,10 @@
end

describe 'Sign in flow' do
let(:user) { create(:user, :with_phone, { email: 'example@example.gov' }) }
let(:user) { create(:user, :with_phone, { email: 'example@gsa.gov' }) }
let!(:federal_domain) { create(:federal_email_domain, name: 'gsa.gov') }
before do
stub_analytics

stub_sign_in(user)
user.reload
end
Expand All @@ -49,7 +50,8 @@
end

context '#confirm' do
let(:user) { create(:user, email: 'example@example.gov') }
let(:user) { create(:user, email: 'example@gsa.gov') }
let!(:federal_domain) { create(:federal_email_domain, name: 'gsa.gov') }
before do
stub_sign_in_before_2fa(user)
stub_analytics
Expand Down Expand Up @@ -77,7 +79,8 @@
end

context '#skip' do
let(:user) { create(:user, email: 'example@example.gov') }
let(:user) { create(:user, email: 'example@gsa.gov') }
let!(:federal_domain) { create(:federal_email_domain, name: 'gsa.gov') }
before do
stub_sign_in_before_2fa(user)
stub_analytics
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,16 @@
)
end

context 'with user having gov or mil email' do
context 'with user having gov or mil email and use_fed_domain_class set to false' do
let(:user) do
create(:user, email: 'example@example.gov', piv_cac_recommended_dismissed_at: Time.zone.now)
end
let!(:federal_domain) { create(:federal_email_domain, name: 'gsa.gov') }

before do
allow(IdentityConfig.store).to receive(:use_fed_domain_class).and_return(false)
end

context 'having already visited the PIV interstitial page' do
it 'tracks the visit in analytics' do
get :index
Expand All @@ -48,6 +54,40 @@
end
end

context 'with user having gov or mil email and use_fed_domain_class set to true' do
before do
allow(IdentityConfig.store).to receive(:use_fed_domain_class).and_return(true)
end

let!(:federal_domain) { create(:federal_email_domain, name: 'gsa.gov') }
let(:user) do
create(:user, email: 'example@gsa.gov', piv_cac_recommended_dismissed_at: Time.zone.now)
end
context 'having already visited the PIV interstitial page' do
it 'tracks the visit in analytics' do
get :index

expect(@analytics).to have_logged_event(
'User Registration: 2FA Setup visited',
enabled_mfa_methods_count: 0,
gov_or_mil_email: true,
)
end
end

context 'directed to page without having visited PIV interstitial page' do
let(:user) do
create(:user, email: 'example@gsa.gov')
end

it 'redirects user to piv_recommended_path' do
get :index

expect(response).to redirect_to(login_piv_cac_recommended_url)
end
end
end

context 'when signed out' do
let(:user) { nil }

Expand Down
4 changes: 4 additions & 0 deletions spec/factories/federal_email_domain.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
FactoryBot.define do
factory :federal_email_domain do
end
end
Loading