Skip to content

Commit

Permalink
Show attributes shared with sp's after sign up
Browse files Browse the repository at this point in the history
**Why**:
Currently we only show the shared attributes once
after the initial sign up. This change shows
sign up completed with a new "First time
logging in" message along with the new
attributes being sent. Page is only shown
when new attributes are being shared with an
SP.
  • Loading branch information
Nick Bristow authored and Nick Bristow committed Feb 5, 2018
1 parent adab037 commit 4b58322
Show file tree
Hide file tree
Showing 28 changed files with 390 additions and 64 deletions.
9 changes: 6 additions & 3 deletions app/controllers/concerns/saml_idp_auth_concern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,17 +49,20 @@ def default_authn_context

def link_identity_from_session_data
IdentityLinker.new(
current_user,
current_issuer
current_user, current_issuer
).link_identity(
ial: loa3_requested? ? 3 : 1
ial: ial_level
)
end

def identity_needs_verification?
loa3_requested? && current_user.decorate.identity_not_verified?
end

def ial_level
loa3_requested? ? 3 : 1
end

def loa3_requested?
requested_authn_context == Saml::Idp::Constants::LOA3_AUTHN_CONTEXT_CLASSREF
end
Expand Down
51 changes: 51 additions & 0 deletions app/controllers/concerns/verify_sp_attributes_concern.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
module VerifySPAttributesConcern
def needs_sp_attribute_verification?
if sp_session_identity.nil? || !requested_attributes_verified?
set_verify_shared_attributes_session
true
else
clear_verify_attributes_sessions
false
end
end

def update_verified_attributes
IdentityLinker.new(
current_user,
sp_session[:issuer]
).link_identity(
ial: sp_session_ial,
verified_attributes: sp_session[:requested_attributes]
)
end

def set_verify_shared_attributes_session
user_session[:verify_shared_attributes] = true
end

def new_service_provider_attributes
user_session[:verify_shared_attributes] if
user_session.class == ActiveSupport::HashWithIndifferentAccess
end

def clear_verify_attributes_sessions
user_session[:verify_shared_attributes] = false
end

private

def sp_session_identity
@sp_session_identity =
current_user&.identities&.find_by(service_provider: sp_session[:issuer])
end

def requested_attributes_verified?
@sp_session_identity && (
sp_session[:requested_attributes] - @sp_session_identity.verified_attributes.to_a
).empty?
end

def sp_session_ial
sp_session[:loa3] ? 3 : 1
end
end
15 changes: 10 additions & 5 deletions app/controllers/openid_connect/authorization_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ module OpenidConnect
class AuthorizationController < ApplicationController
include FullyAuthenticatable
include VerifyProfileConcern
include VerifySPAttributesConcern

before_action :build_authorize_form_from_params, only: [:index]
before_action :validate_authorize_form, only: [:index]
Expand All @@ -12,18 +13,22 @@ class AuthorizationController < ApplicationController

def index
return confirm_two_factor_authenticated(request_id) unless user_fully_authenticated?
@authorize_form.link_identity_to_service_provider(current_user, session.id)
return redirect_to_account_or_verify_profile_url if profile_or_identity_needs_verification?
return redirect_to(sign_up_completed_url) if needs_sp_attribute_verification?
handle_successful_handoff
end

@authorize_form.link_identity_to_service_provider(current_user, session.id)
private

def handle_successful_handoff
redirect_to @authorize_form.success_redirect_uri
delete_branded_experience
end

private

def redirect_to_account_or_verify_profile_url
return redirect_to account_or_verify_profile_url if profile_needs_verification?
redirect_to verify_url if identity_needs_verification?
return redirect_to(account_or_verify_profile_url) if profile_needs_verification?
redirect_to(verify_url) if identity_needs_verification?
end

def profile_or_identity_needs_verification?
Expand Down
36 changes: 23 additions & 13 deletions app/controllers/saml_idp_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,17 @@ class SamlIdpController < ApplicationController
include SamlIdpLogoutConcern
include FullyAuthenticatable
include VerifyProfileConcern
include VerifySPAttributesConcern

skip_before_action :verify_authenticity_token

def auth
return confirm_two_factor_authenticated(request_id) unless user_fully_authenticated?
process_fully_authenticated_user do |needs_idv, needs_profile_finish|
return redirect_to(verify_url) if needs_idv && !needs_profile_finish
return redirect_to(account_or_verify_profile_url) if needs_profile_finish
end
delete_branded_experience

render_template_for(saml_response, saml_request.response_url, 'SAMLResponse')
link_identity_from_session_data
capture_analytics
return redirect_to_account_or_verify_profile_url if profile_or_identity_needs_verification?
return redirect_to(sign_up_completed_url) if needs_sp_attribute_verification?
handle_successful_handoff
end

def metadata
Expand All @@ -39,15 +38,26 @@ def logout

private

def process_fully_authenticated_user
link_identity_from_session_data
def redirect_to_account_or_verify_profile_url
return redirect_to(account_or_verify_profile_url) if profile_needs_verification?
redirect_to(verify_url) if identity_needs_verification?
end

needs_idv = identity_needs_verification?
needs_profile_finish = profile_needs_verification?
analytics_payload = @result.to_h.merge(idv: needs_idv, finish_profile: needs_profile_finish)
def profile_or_identity_needs_verification?
profile_needs_verification? || identity_needs_verification?
end

def capture_analytics
analytics_payload = @result.to_h.merge(
idv: identity_needs_verification?,
finish_profile: profile_needs_verification?
)
analytics.track_event(Analytics::SAML_AUTH, analytics_payload)
end

yield needs_idv, needs_profile_finish
def handle_successful_handoff
delete_branded_experience
render_template_for(saml_response, saml_request.response_url, 'SAMLResponse')
end

def render_template_for(message, action_url, type)
Expand Down
7 changes: 5 additions & 2 deletions app/controllers/sign_up/completions_controller.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
module SignUp
class CompletionsController < ApplicationController
include SecureHeadersConcern
include VerifySPAttributesConcern

before_action :confirm_two_factor_authenticated
before_action :verify_confirmed, if: :loa3?
Expand All @@ -21,7 +22,8 @@ def update
track_agency_handoff(
Analytics::USER_REGISTRATION_AGENCY_HANDOFF_COMPLETE
)

update_verified_attributes
clear_verify_attributes_sessions
if decider.go_back_to_mobile_app?
sign_user_out_and_instruct_to_go_back_to_mobile_app
else
Expand All @@ -40,7 +42,8 @@ def view_model
SignUpCompletionsShow.new(
loa3_requested: loa3?,
decorated_session: decorated_session,
current_user: current_user
current_user: current_user,
handoff: new_service_provider_attributes
)
end

Expand Down
2 changes: 1 addition & 1 deletion app/decorators/service_provider_session_decorator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ def idv_hardfail4_partial
end

def requested_attributes
sp_session[:requested_attributes]
sp_session[:requested_attributes].sort
end

def sp_name
Expand Down
13 changes: 11 additions & 2 deletions app/forms/openid_connect_authorize_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def submit
end

def loa3_requested?
ial == 3
loa == 3
end

def sp_redirect_uri
Expand Down Expand Up @@ -87,6 +87,11 @@ def success_redirect_uri

attr_reader :identity, :success, :openid_connect_redirector, :already_linked

def requested_attributes
@requested_attributes ||=
OpenidConnectAuthorizeDecorator.new(scopes: scope).requested_attributes
end

def parse_to_values(param_value, possible_values)
return [] if param_value.blank?
param_value.split(' ').compact & possible_values
Expand All @@ -111,7 +116,7 @@ def validate_scope
errors.add(:scope, t('openid_connect.authorization.errors.no_valid_scope'))
end

def ial
def loa
case acr_values.sort.max
when Saml::Idp::Constants::LOA1_AUTHN_CONTEXT_CLASSREF
1
Expand All @@ -120,6 +125,10 @@ def ial
end
end

def ial
loa == 3 ? 3 : 1
end

def extra_analytics_attributes
{
client_id: client_id,
Expand Down
9 changes: 8 additions & 1 deletion app/services/identity_linker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,21 @@ def optional_attributes(
ial: nil,
nonce: nil,
rails_session_id: nil,
scope: nil
scope: nil,
verified_attributes: nil
)
{
code_challenge: code_challenge,
ial: ial,
nonce: nonce,
rails_session_id: rails_session_id,
scope: scope,
verified_attributes: merge_attributes(verified_attributes),
}
end

def merge_attributes(verified_attributes)
verified_attributes = verified_attributes.to_a.map(&:to_s)
(identity.verified_attributes.to_a + verified_attributes).uniq.sort
end
end
22 changes: 14 additions & 8 deletions app/view_models/sign_up_completions_show.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
class SignUpCompletionsShow
include ActionView::Helpers::TagHelper

def initialize(loa3_requested:, decorated_session:, current_user:)
def initialize(loa3_requested:, decorated_session:, current_user:, handoff:)
@loa3_requested = loa3_requested
@decorated_session = decorated_session
@current_user = current_user
@handoff = handoff
end

attr_reader :loa3_requested, :decorated_session
Expand All @@ -22,15 +23,16 @@ def initialize(loa3_requested:, decorated_session:, current_user:)

# rubocop:disable Rails/OutputSafety
def heading
return content_tag(:strong, I18n.t('titles.sign_up.new_sp')) if handoff?
if requested_loa == 'loa3'
content_tag(:strong, I18n.t('titles.sign_up.verified', app: APP_NAME))
else
safe_join([I18n.t(
'titles.sign_up.completion_html',
accent: content_tag(:strong, I18n.t('titles.sign_up.loa1')),
app: APP_NAME
).html_safe])
return content_tag(:strong, I18n.t('titles.sign_up.verified', app: APP_NAME))
end

safe_join([I18n.t(
'titles.sign_up.completion_html',
accent: content_tag(:strong, I18n.t('titles.sign_up.loa1')),
app: APP_NAME
).html_safe])
end
# rubocop:enable Rails/OutputSafety

Expand Down Expand Up @@ -92,6 +94,10 @@ def user_has_identities?

private

def handoff?
@handoff
end

def requested_attributes
decorated_session.requested_attributes.map(&:to_sym)
end
Expand Down
1 change: 1 addition & 0 deletions config/locales/titles/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ en:
completion_html: You have %{accent} with %{app}
loa1: created your account
verified: You have verified your identity with %{app}
new_sp: You are now logging in for the first time
totp_setup:
new: Set up two-factor authentication
two_factor_setup: Two-factor authentication setup
Expand Down
1 change: 1 addition & 0 deletions config/locales/titles/es.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ es:
completion_html: Tiene %{accent} con %{app}
loa1: creó su cuenta
verified: Tiene verificó su identidad con %{app}
new_sp: NOT TRANSLATED YET
totp_setup:
new: Configure la autenticación de dos factores
two_factor_setup: Configuración de autenticación de dos factores
Expand Down
1 change: 1 addition & 0 deletions config/locales/titles/fr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ fr:
completion_html: Vous avez %{accent} avec %{app}
loa1: créé votre compte
verified: Vous avez verifié votre identité avec %{app}
new_sp: NOT TRANSLATED YET
totp_setup:
new: Configurer l'authentification à deux facteurs
two_factor_setup: Configuration de l'authentification à deux facteurs
Expand Down
26 changes: 25 additions & 1 deletion spec/controllers/openid_connect/authorization_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@

context 'with valid params' do
it 'redirects back to the client app with a code' do
IdentityLinker.new(user, client_id).link_identity(ial: 1)
user.identities.last.update!(verified_attributes: ["given_name", "family_name", "birthdate"])
action

expect(response).to redirect_to(/^#{params[:redirect_uri]}/)
Expand Down Expand Up @@ -54,6 +56,8 @@
let(:user) { create(:profile, :active, :verified).user }

it 'redirects to the redirect_uri immediately' do
IdentityLinker.new(user, client_id).link_identity(ial: 3)
user.identities.last.update!(verified_attributes: ["given_name", "family_name", "birthdate"])
action

expect(response).to redirect_to(/^#{params[:redirect_uri]}/)
Expand All @@ -68,15 +72,35 @@
end
end

context 'user has not approved this application' do
it 'redirects verify shared attributes page' do
action

expect(response).to redirect_to(sign_up_completed_url)
end

it 'links identity to the user' do
action
sp = user.identities.last.service_provider
expect(sp).to eq(params[:client_id])
end
end

context 'user has already approved this application' do
before do
IdentityLinker.new(user, client_id).link_identity
user.identities.last.update!(verified_attributes: ["given_name", "family_name", "birthdate"])
end

it 'redirects to the redirect_uri immediately' do
it 'redirects back to the client app with a code' do
action

expect(response).to redirect_to(/^#{params[:redirect_uri]}/)

redirect_params = URIService.params(response.location)

expect(redirect_params[:code]).to be_present
expect(redirect_params[:state]).to eq(params[:state])
end
end
end
Expand Down
Loading

0 comments on commit 4b58322

Please sign in to comment.