Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
1791594
update status
mdiarra3 Apr 7, 2023
956b54d
Merge remote-tracking branch 'origin/main' into LG-8534-reset-url-sto…
mdiarra3 Apr 11, 2023
006f63b
changelog: User-Facing Improvements, Authentication, Forgot Password …
mdiarra3 Apr 13, 2023
2f12bb6
Merge remote-tracking branch 'origin/main' into LG-8534-reset-url-sto…
mdiarra3 Apr 14, 2023
81636c8
remove unneded schema
mdiarra3 Apr 14, 2023
c17df60
add spec on reset controller
mdiarra3 Apr 17, 2023
7d8db96
reset password controller update
mdiarra3 Apr 18, 2023
a232753
fix tests
mdiarra3 Apr 18, 2023
bd2e072
update url
mdiarra3 Apr 19, 2023
4ed8e88
rubocop
mdiarra3 Apr 19, 2023
6c1d01b
rspec for new logic
mdiarra3 Apr 19, 2023
e45d2de
rubocop
mdiarra3 Apr 20, 2023
d5307ef
remove unneeded route
mdiarra3 Apr 24, 2023
2b6245f
update application.yml
mdiarra3 Apr 24, 2023
2610f1b
add test for both toggle on and off
mdiarra3 Apr 24, 2023
38f6a7a
update spec
mdiarra3 Apr 24, 2023
687eb49
refactor to move edit and storing session only on toggle
mdiarra3 Apr 25, 2023
a5fe284
only delete on success or redirect
mdiarra3 Apr 25, 2023
4744ab8
rspec and rubocop fixes
mdiarra3 Apr 25, 2023
6d9ed30
changelog: Internal, Authentication, remove Feature toggle
mdiarra3 Apr 26, 2023
cac194c
remove toggle
mdiarra3 Apr 26, 2023
43c2b14
Merge remote-tracking branch 'origin/main' into LG-8534-remove-toggle
mdiarra3 Apr 28, 2023
c03fbb5
fix merge issues
mdiarra3 Apr 28, 2023
3d3bb25
fix password token
mdiarra3 May 1, 2023
f7931a4
Merge remote-tracking branch 'origin/main' into LG-8534-remove-toggle
mdiarra3 Jun 13, 2023
26ed1fa
remove toggle references
mdiarra3 Jun 13, 2023
f6d5204
fix reset password token
mdiarra3 Jun 13, 2023
b08f869
fix password
mdiarra3 Jun 13, 2023
572fa4c
fix rspec
mdiarra3 Jun 13, 2023
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
12 changes: 4 additions & 8 deletions app/controllers/users/reset_passwords_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ module Users
class ResetPasswordsController < Devise::PasswordsController
include AuthorizationCountConcern
before_action :store_sp_metadata_in_session, only: [:edit]
before_action :store_token_in_session, only: [:edit], if: :clean_edit_url_toggle_on?
before_action :store_token_in_session, only: [:edit]

def new
analytics.password_reset_visit
Expand All @@ -23,7 +23,7 @@ def create
end

def edit
if params[:reset_password_token] && clean_edit_url_toggle_on?
if params[:reset_password_token]
redirect_to edit_user_password_url
else
result = PasswordResetTokenValidator.new(token_user).submit
Expand Down Expand Up @@ -56,7 +56,7 @@ def update
)

if result.success?
session.delete(:reset_password_token) if session[:reset_password_token]
session.delete(:reset_password_token)
handle_successful_password_reset
else
handle_unsuccessful_password_reset(result)
Expand Down Expand Up @@ -103,10 +103,6 @@ def store_token_in_session
session[:reset_password_token] = params[:reset_password_token]
end

def clean_edit_url_toggle_on?
FeatureManagement.redirect_to_clean_edit_password_url?
end

def create_account_if_email_not_found
user, result = RequestPasswordReset.new(
email: email,
Expand Down Expand Up @@ -167,7 +163,7 @@ def send_password_reset_risc_event
def handle_unsuccessful_password_reset(result)
reset_password_token_errors = result.errors[:reset_password_token]
if reset_password_token_errors.present?
session.delete(:reset_password_token) if session[:reset_password_token]
session.delete(:reset_password_token)
flash[:error] = t("devise.passwords.#{reset_password_token_errors.first}")
redirect_to new_user_password_url
return
Expand Down
3 changes: 0 additions & 3 deletions config/application.yml.default
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,6 @@ verify_gpo_key_max_attempts: 3
verify_personal_key_attempt_window_in_minutes: 15
verify_personal_key_max_attempts: 5
use_dashboard_service_providers: false
use_clean_edit_password_url: true
use_kms: false
usps_confirmation_max_days: 10
usps_ipp_password: ''
Expand Down Expand Up @@ -393,7 +392,6 @@ development:
doc_auth_vendor_randomize_alternate_vendor: ''
domain_name: localhost:3000
enable_rate_limiting: false
use_clean_edit_password_url: true
hmac_fingerprinter_key: a2c813d4dca919340866ba58063e4072adc459b767a74cf2666d5c1eef3861db26708e7437abde1755eb24f4034386b0fea1850a1cb7e56bff8fae3cc6ade96c
hmac_fingerprinter_key_queue: '["11111111111111111111111111111111", "22222222222222222222222222222222"]'
identity_pki_local_dev: true
Expand Down Expand Up @@ -510,7 +508,6 @@ production:
skip_encryption_allowed_list: '["urn:gov:gsa:SAML:2.0.profiles:sp:sso:dev", "urn:gov:gsa:SAML:2.0.profiles:sp:sso:int"]'
state_tracking_enabled: false
telephony_adapter: pinpoint
use_clean_edit_password_url: false
use_kms: true
usps_confirmation_max_days: 30
usps_upload_sftp_directory: ''
Expand Down
4 changes: 0 additions & 4 deletions lib/feature_management.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,6 @@ def self.use_kms?
IdentityConfig.store.use_kms
end

def self.redirect_to_clean_edit_password_url?
IdentityConfig.store.use_clean_edit_password_url
end

def self.kms_multi_region_enabled?
IdentityConfig.store.aws_kms_multi_region_enabled
end
Expand Down
1 change: 0 additions & 1 deletion lib/identity_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,6 @@ def self.build_store(config_map)
config.add(:totp_code_interval, type: :integer)
config.add(:unauthorized_scope_enabled, type: :boolean)
config.add(:use_dashboard_service_providers, type: :boolean)
config.add(:use_clean_edit_password_url, type: :boolean)
config.add(:use_kms, type: :boolean)
config.add(:usps_mock_fallback, type: :boolean)
config.add(:usps_confirmation_max_days, type: :integer)
Expand Down
181 changes: 80 additions & 101 deletions spec/controllers/users/reset_passwords_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,72 @@
let(:success_properties) { { success: true, failure_reason: nil } }
let(:token_expired_error) { 'token_expired' }
describe '#edit' do
let(:user) { instance_double('User', uuid: '123') }
let(:email_address) { instance_double('EmailAddress') }
before do
stub_analytics
stub_attempts_tracker
allow(@analytics).to receive(:track_event)
end

context 'when clean url feature toggle is set to false' do
context 'when token isnt stored in session' do
it 'redirects to the clean edit password url with token stored in session' do
get :edit, params: { reset_password_token: 'foo' }
expect(response).to redirect_to(edit_user_password_url)
expect(session[:reset_password_token]).to eq('foo')
end
end

context 'no user matches token' do
let(:user_blank_error) { { user: [:blank] } }
let(:token) { 'foo' }
before do
session[:reset_password_token] = token
end
let(:analytics_hash) do
{
success: false,
errors: { user: ['invalid_token'] },
error_details: user_blank_error,
user_id: nil,
}
end

it 'redirects to page where user enters email for password reset token' do
expect(@irs_attempts_api_tracker).to receive(:forgot_password_email_confirmed).with(
success: false,
failure_reason: user_blank_error,
)

get :edit

expect(@analytics).to have_received(:track_event).
with('Password Reset: Token Submitted', analytics_hash)
expect(response).to redirect_to new_user_password_path
expect(flash[:error]).to eq t('devise.passwords.invalid_token')
end
end

context 'token expired' do
let(:user_token_error) { { user: [token_expired_error] } }
let(:token) { 'foo' }
before do
session[:reset_password_token] = token
end
let(:analytics_hash) do
{
success: false,
errors: user_token_error,
error_details: user_token_error,
user_id: '123',
}
end
let(:user) { instance_double('User', uuid: '123') }

before do
allow(FeatureManagement).to receive(:redirect_to_clean_edit_password_url?).and_return(false)
allow(User).to receive(:with_reset_password_token).with(token).and_return(user)
allow(User).to receive(:with_reset_password_token).with('bar').and_return(nil)
allow(user).to receive(:reset_password_period_valid?).and_return(false)
end

context 'no user matches token' do
Expand All @@ -29,13 +86,17 @@
}
end

before do
session[:reset_password_token] = 'bar'
end

it 'redirects to page where user enters email for password reset token' do
expect(@irs_attempts_api_tracker).to receive(:forgot_password_email_confirmed).with(
success: false,
failure_reason: user_blank_error,
)

get :edit, params: { reset_password_token: 'foo' }
get :edit

expect(@analytics).to have_received(:track_event).
with('Password Reset: Token Submitted', analytics_hash)
Expand Down Expand Up @@ -67,7 +128,7 @@
failure_reason: user_token_error,
)

get :edit, params: { reset_password_token: 'foo' }
get :edit

expect(@analytics).to have_received(:track_event).
with('Password Reset: Token Submitted', analytics_hash)
Expand Down Expand Up @@ -99,7 +160,7 @@
success_properties,
)

get :edit, params: { reset_password_token: 'foo' }
get :edit

expect(response).to render_template :edit
expect(flash.keys).to be_empty
Expand All @@ -108,106 +169,24 @@
end
end

context 'when clean url feature toggle is set to true' do
let(:user) { instance_double('User', uuid: '123') }
let(:email_address) { instance_double('EmailAddress') }
context 'when token is valid' do
before do
allow(FeatureManagement).to receive(:redirect_to_clean_edit_password_url?).and_return(true)
allow(User).to receive(:with_reset_password_token).with('foo').and_return(user)
allow(user).to receive(:reset_password_period_valid?).and_return(true)
allow(user).to receive(:email_addresses).and_return([email_address])
session[:reset_password_token] = 'foo'
end
it 'renders the template to the clean edit password url with token stored in session' do
expect(email_address).to receive(:email).twice

context 'when token isnt stored in session' do
it 'redirects to the clean edit password url with token stored in session' do
get :edit, params: { reset_password_token: 'foo' }
expect(response).to redirect_to(edit_user_password_url)
expect(session[:reset_password_token]).to eq('foo')
end
end
forbidden = instance_double(ForbiddenPasswords)
allow(ForbiddenPasswords).to receive(:new).
with(email_address.email).and_return(forbidden)
expect(forbidden).to receive(:call)

context 'no user matches token' do
let(:user_blank_error) { { user: [:blank] } }
let(:token) { 'foo' }
before do
session[:reset_password_token] = token
end
let(:analytics_hash) do
{
success: false,
errors: { user: ['invalid_token'] },
error_details: user_blank_error,
user_id: nil,
}
end

it 'redirects to page where user enters email for password reset token' do
expect(@irs_attempts_api_tracker).to receive(:forgot_password_email_confirmed).with(
success: false,
failure_reason: user_blank_error,
)

get :edit

expect(@analytics).to have_received(:track_event).
with('Password Reset: Token Submitted', analytics_hash)
expect(response).to redirect_to new_user_password_path
expect(flash[:error]).to eq t('devise.passwords.invalid_token')
end
end

context 'token expired' do
let(:user_token_error) { { user: [token_expired_error] } }
let(:token) { 'foo' }
before do
session[:reset_password_token] = token
end
let(:analytics_hash) do
{
success: false,
errors: user_token_error,
error_details: user_token_error,
user_id: '123',
}
end
let(:user) { instance_double('User', uuid: '123') }

before do
allow(User).to receive(:with_reset_password_token).with(token).and_return(user)
allow(user).to receive(:reset_password_period_valid?).and_return(false)
end

it 'redirects to page where user enters email for password reset token' do
expect(@irs_attempts_api_tracker).to receive(:forgot_password_email_confirmed).with(
success: false,
failure_reason: user_token_error,
)

get :edit

expect(@analytics).to have_received(:track_event).
with('Password Reset: Token Submitted', analytics_hash)
expect(response).to redirect_to new_user_password_path
expect(flash[:error]).to eq t('devise.passwords.token_expired')
end
end

context 'when token is valid' do
before do
allow(User).to receive(:with_reset_password_token).with('foo').and_return(user)
allow(user).to receive(:reset_password_period_valid?).and_return(true)
allow(user).to receive(:email_addresses).and_return([email_address])
session[:reset_password_token] = 'foo'
end
it 'renders the template to the clean edit password url with token stored in session' do
expect(email_address).to receive(:email).twice

forbidden = instance_double(ForbiddenPasswords)
allow(ForbiddenPasswords).to receive(:new).
with(email_address.email).and_return(forbidden)
expect(forbidden).to receive(:call)

get :edit
expect(response).to render_template :edit
expect(flash.keys).to be_empty
end
get :edit
expect(response).to render_template :edit
expect(flash.keys).to be_empty
end
end
end
Expand Down