Skip to content

Commit

Permalink
Update the authentication pages to use the GOV.UK Helpers.
Browse files Browse the repository at this point in the history
The pages in question relate to:
- Passwords
- Registrations
- Sessions
- Users

I’ve also tidied up some of the models/services/controllers to help with this
  • Loading branch information
tim-s-ccs committed Nov 16, 2023
1 parent 105bed2 commit 73cd50c
Show file tree
Hide file tree
Showing 22 changed files with 450 additions and 415 deletions.
26 changes: 20 additions & 6 deletions app/controllers/base/passwords_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,28 +16,42 @@ def edit
end

def create
@response = Cognito::ForgotPassword.call(params[:email])
@response = Cognito::ForgotPassword.call(create_params[:email])

if @response.success?
cookies[:crown_marketplace_reset_email] = { value: params[:email], expires: 20.minutes, httponly: true }
cookies[:crown_marketplace_reset_email] = { value: create_params[:email], expires: 20.minutes, httponly: true }

redirect_to edit_password_path
else
flash[:error] = @response.error
redirect_to new_user_password_path
render :new
end
end

def update
@response = Cognito::ConfirmPasswordReset.call(params[:email], params[:password], params[:password_confirmation], params[:confirmation_code])
@response = Cognito::ConfirmPasswordReset.call(update_params[:email], update_params[:password], update_params[:password_confirmation], update_params[:confirmation_code])

if @response.success?
cookies.delete :crown_marketplace_reset_email

redirect_to password_reset_success_path
else
render :edit, erorr: @response.error
render :edit
end
end

def password_reset_success; end

def create_params
params.require(:cognito_forgot_password).permit(:email)
end

def update_params
params.require(:cognito_confirm_password_reset).permit(
:email,
:password,
:password_confirmation,
:confirmation_code
)
end
end
end
20 changes: 10 additions & 10 deletions app/controllers/base/sessions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def new

def create
self.resource ||= User.new
@result = Cognito::SignInUser.new(params[:user][:email], params[:user][:password], request.cookies.blank?)
@result = Cognito::SignInUser.new(sign_in_params[:email], sign_in_params[:password], request.cookies.blank?)
@result.call

if @result.success?
Expand Down Expand Up @@ -53,26 +53,26 @@ def after_sign_out_path_for(_resource)
sign_in_url
end

def challenge_path
cookies[:crown_marketplace_challenge_session] = { value: @result.session, expires: 20.minutes, httponly: true }
cookies[:crown_marketplace_challenge_username] = { value: @result.cognito_uuid, expires: 20.minutes, httponly: true }

service_challenge_path
end

def result_unsuccessful_path
sign_out
if @result.needs_password_reset
cookies[:crown_marketplace_reset_email] = { value: params[:user][:email], expires: 20.minutes, httponly: true }
cookies[:crown_marketplace_reset_email] = { value: sign_in_params[:email], expires: 20.minutes, httponly: true }

redirect_to edit_password_path
elsif @result.needs_confirmation
cookies[:crown_marketplace_confirmation_email] = { value: params[:user][:email], expires: 20.minutes, httponly: true }
cookies[:crown_marketplace_confirmation_email] = { value: sign_in_params[:email], expires: 20.minutes, httponly: true }

redirect_to confirm_email_path
else
render :new
end
end

def challenge_path
cookies[:crown_marketplace_challenge_session] = { value: @result.session, expires: 20.minutes, httponly: true }
cookies[:crown_marketplace_challenge_username] = { value: @result.cognito_uuid, expires: 20.minutes, httponly: true }

service_challenge_path
end
end
end
39 changes: 31 additions & 8 deletions app/controllers/base/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,16 @@ class UsersController < ApplicationController

before_action :authenticate_user!, except: %i[confirm_new confirm resend_confirmation_email challenge_new challenge]
before_action :authorize_user, except: %i[confirm_new confirm resend_confirmation_email challenge_new challenge]
before_action :set_user_phone, only: %i[challenge_new challenge]
before_action :set_user_phone, only: %i[challenge_new challenge], if: -> { params[:challenge_name] == 'SMS_MFA' }

helper_method :confirm_email_path, :resend_confirmation_email_path, :challenge_user_path, :new_user_session_path

def confirm_new
@result = Cognito::ConfirmSignUp.new(nil, nil)
@result = Cognito::ConfirmSignUp.new(cookies[:crown_marketplace_confirmation_email], nil)
end

def confirm
@result = Cognito::ConfirmSignUp.call(params[:email], params[:confirmation_code])
@result = Cognito::ConfirmSignUp.call(confirm_params[:email], confirm_params[:confirmation_code])
if @result.success?
cookies.delete :crown_marketplace_confirmation_email

Expand All @@ -24,17 +24,17 @@ def confirm
end

def resend_confirmation_email
result = Cognito::ResendConfirmationCode.call(params[:email])
Cognito::ResendConfirmationCode.call(confirm_params[:email])

redirect_to confirm_email_path, error: result.error
redirect_to confirm_email_path
end

def challenge_new
@challenge = Cognito::RespondToChallenge.new(params[:challenge_name], params[:username], params[:session])
@challenge = Cognito::RespondToChallenge.new(params[:challenge_name], cookies[:crown_marketplace_challenge_username], cookies[:crown_marketplace_challenge_session])
end

def challenge
@challenge = Cognito::RespondToChallenge.new(params[:challenge_name], params[:username], params[:session], new_password: params[:new_password], new_password_confirmation: params[:new_password_confirmation], access_code: params[:access_code])
@challenge = Cognito::RespondToChallenge.new(params[:challenge_name], challenge_params[:username], challenge_params[:session], new_password: challenge_params[:new_password], new_password_confirmation: challenge_params[:new_password_confirmation], access_code: challenge_params[:access_code])
@challenge.call
if @challenge.success?
@challenge.challenge? ? redirect_to(new_challenge_path) : find_and_sign_in_user
Expand All @@ -56,7 +56,7 @@ def find_and_sign_in_user
cookies.delete :crown_marketplace_challenge_session
cookies.delete :crown_marketplace_challenge_username

user = Cognito::CreateUserFromCognito.call(params[:username]).user
user = Cognito::CreateUserFromCognito.call(challenge_params[:username]).user
sign_in_user(user)
end

Expand All @@ -69,5 +69,28 @@ def set_user_phone
user_phone_full = User.find_by(cognito_uuid: cookies[:crown_marketplace_challenge_username]).try(:phone_number) || Cognito::CreateUserFromCognito.call(cookies[:crown_marketplace_challenge_username]).try(:user).try(:phone_number)
@user_phone = ('*' * 9) + user_phone_full.last(3) if user_phone_full
end

def confirm_params
params.require(:cognito_confirm_sign_up).permit(
:confirmation_code,
:email
)
end

def challenge_params
params.require(:cognito_respond_to_challenge).permit(
%i[session username] + CHALLANGE_PARAMS[params[:challenge_name]]
)
end

CHALLANGE_PARAMS = {
'NEW_PASSWORD_REQUIRED' => %i[
new_password
new_password_confirmation
],
'SMS_MFA' => %i[
access_code
]
}.freeze
end
end
9 changes: 0 additions & 9 deletions app/helpers/application_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,6 @@ def support_telephone_number
Marketplace.support_telephone_number
end

def display_error(journey, attribute, margin = true, id_prefix = '')
error = journey.errors[attribute].first
return if error.blank?

tag.span(id: "#{id_prefix}#{error_id(attribute)}", class: "govuk-error-message #{'govuk-!-margin-top-3' if margin}") do
error.to_s
end
end

def error_id(attribute)
"#{attribute}-error"
end
Expand Down
2 changes: 1 addition & 1 deletion app/services/cognito/confirm_sign_up.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class ConfirmSignUp < BaseService
attr_reader :email, :confirmation_code
attr_accessor :user

validates_presence_of :email, :confirmation_code
validates_presence_of :confirmation_code, :email
validates :confirmation_code,
presence: true,
format: { with: /\A\d+\z/, message: :invalid_format },
Expand Down
16 changes: 6 additions & 10 deletions app/services/cognito/forgot_password.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,29 +3,25 @@ class ForgotPassword < BaseService
include ActiveModel::Validations
attr_reader :email, :error

validates :email, format: { with: /\A([\w+\d-].?)+@[a-z\d-]+(\.[a-z]+)*\.[a-z]+\z/i }
validates :email, format: { with: /\A([\w+-].?)+@[a-z\d-]+(\.[a-z]+)*\.[a-z]+\z/i }

def initialize(email)
@email = email.try(:downcase)
@error = nil
end

def call
if valid?
forgot_password
else
@error = I18n.t('cognito/cog_forgot_password_request.attributes.please_enter_a_valid_email_address')
end
forgot_password if valid?
rescue Aws::CognitoIdentityProvider::Errors::UserNotFoundException
@error = nil
# To prevent user enumeration, continue if the email does not exist
rescue Aws::CognitoIdentityProvider::Errors::InvalidParameterException
@error = I18n.t('cognito/cog_forgot_password_request.attributes.please_enter_a_valid_email_address')
errors.add(:email, :invalid)
rescue Aws::CognitoIdentityProvider::Errors::ServiceError => e
@error = e.message
errors.add(:base, e.message)
end

def success?
error.nil?
errors.none?
end

private
Expand Down
7 changes: 2 additions & 5 deletions app/services/cognito/sign_in_user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,17 @@ def initialize(email, password, cookies_disabled)
def call
initiate_auth if valid?
rescue Aws::CognitoIdentityProvider::Errors::PasswordResetRequiredException => e
@error = e.message
errors.add(:base, e.message)
@needs_password_reset = true
rescue Aws::CognitoIdentityProvider::Errors::UserNotConfirmedException => e
@error = e.message
errors.add(:base, e.message)
@needs_confirmation = true
rescue Aws::CognitoIdentityProvider::Errors::ServiceError
@error = I18n.t('facilities_management.users.sign_in_error')
errors.add(:base, @error)
errors.add(:base, :sign_in_error)
end

def success?
@auth_response.present? && @error.nil?
@auth_response.present? && errors.none?
end

def challenge?
Expand Down
Loading

0 comments on commit 73cd50c

Please sign in to comment.