Skip to content

Commit

Permalink
fix(nbp): let user know if required attributes are missing
Browse files Browse the repository at this point in the history
- test(enmeshed): add test for invalid free text input in the status group

Part of XI-6523
  • Loading branch information
nenock committed Feb 5, 2025
1 parent 68ddf44 commit ec816b1
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 23 deletions.
31 changes: 21 additions & 10 deletions app/controllers/users/nbp_wallet_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,8 @@ def finalize

private

def accept_and_create_user(relationship) # rubocop:disable Metrics/AbcSize, Metrics/MethodLength
# Enmeshed::Relationship checks for the presence of all the requested attributes first and later parses the
# provided status group. If it is not a synonym of the valid ones, the attribute is cleared so that a fitting
# alert can be passed on to the user here.
if relationship.userdata[:status_group].blank?
abort_and_refresh(
relationship,
t('common.errors.model_not_created', model: User.model_name.human, errors: t('users.nbp_wallet.unrecognized_role'))
) and return
end
def accept_and_create_user(relationship) # rubocop:disable Metrics/AbcSize
return unless user_data_valid?(relationship)

user = User.new_from_omniauth(relationship.userdata, 'nbp', @provider_uid)
user.identities << UserIdentity.new(omniauth_provider: 'enmeshed', provider_uid: relationship.peer)
Expand Down Expand Up @@ -92,6 +84,25 @@ def abort_and_refresh(relationship, reason = t('common.errors.generic'))
redirect_to nbp_wallet_connect_users_path, alert: reason
end

def user_data_valid?(relationship)
# Enmeshed::Relationship checks for the presence of all the requested attributes first and later parses the
# provided status group. If it is not a synonym of the valid ones, the attribute is cleared so that a fitting
# alert can be passed on to the user here.
if relationship.userdata[:status_group].blank?
abort_and_refresh(relationship, t('common.errors.model_not_created', model: User.model_name.human,
errors: t('users.nbp_wallet.unrecognized_role'))) and return false
end

true
rescue Enmeshed::ConnectorError => e
# If the error is due to missing attributes, pass on the error message to the user.
if e.message.include?('must not be empty')
abort_and_refresh(relationship, e.message) and return false
else
abort_and_refresh(relationship) and return false
end
end

def require_user!
@provider_uid = session[:saml_uid]
raise Pundit::NotAuthorizedError unless @provider_uid.present? && session[:omniauth_provider] == 'nbp'
Expand Down
11 changes: 11 additions & 0 deletions spec/lib/enmeshed/relationship_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,17 @@
end
end

context 'with gibberish as status group' do
before do
response_items.last[:attribute][:value][:value] = 'gibberish'
end

it 'returns `nil` as the status group' do
expect(userdata).to eq({email: '[email protected]', first_name: 'John', last_name: 'Oliver',
status_group: nil})
end
end

context 'with a blank attribute' do
before do
json[:@type]
Expand Down
38 changes: 25 additions & 13 deletions spec/requests/users/nbp_wallet/finalize_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -83,15 +83,6 @@
end
end

shared_examples 'a documented erroneous request' do |error|
it 'passes the error reason to Sentry' do
expect(Sentry).to receive(:capture_exception) do |e|
expect(e).to be_a error
end
finalize_request
end
end

context 'without the session' do
let(:session_params) { {} }

Expand All @@ -116,16 +107,15 @@
require 'enmeshed/connector'

let(:relationship) do
instance_double(Enmeshed::Relationship, accept!: false, reject!: true)
instance_double(Enmeshed::Relationship, reject!: true)
end

before do
allow(Enmeshed::Relationship).to receive(:pending_for).with(uid).and_return(relationship)
allow(relationship).to receive(:userdata).and_raise(Enmeshed::ConnectorError, 'EMailAddress must not be empty')
end

it_behaves_like 'a handled erroneous request', I18n.t('common.errors.generic')
it_behaves_like 'a documented erroneous request', Enmeshed::ConnectorError
it_behaves_like 'a handled erroneous request', 'EMailAddress must not be empty'
end

context 'with an invalid status group' do
Expand All @@ -148,6 +138,22 @@
'"Teacher" or "Student" as your role.'
end

context 'with an unexpected format in the response items' do
# `Enmeshed::ConnectorError` is unknown until 'lib/enmeshed/connector.rb' is loaded, because it's defined there
require 'enmeshed/connector'

let(:relationship) do
instance_double(Enmeshed::Relationship, reject!: true)
end

before do
allow(Enmeshed::Relationship).to receive(:pending_for).with(uid).and_return(relationship)
allow(relationship).to receive(:userdata).and_raise(Enmeshed::ConnectorError, 'Could not parse userdata in the response items')
end

it_behaves_like 'a handled erroneous request', I18n.t('common.errors.generic')
end

context 'when the User cannot be saved' do
let(:relationship) do
instance_double(Enmeshed::Relationship,
Expand Down Expand Up @@ -198,7 +204,6 @@
before { allow(relationship).to receive(:reject!).and_raise(Faraday::ConnectionFailed) }

it_behaves_like 'a handled erroneous request', I18n.t('common.errors.generic')
it_behaves_like 'a documented erroneous request', Faraday::ConnectionFailed

it 'does not create a user' do
expect { finalize_request }.not_to change(User, :count)
Expand All @@ -212,6 +217,13 @@
expect(relationship).to receive(:reject!).once
finalize_request
end

it 'passes the error reason to Sentry' do
expect(Sentry).to receive(:capture_exception) do |e|
expect(e).to be_a Faraday::ConnectionFailed
end
finalize_request
end
end
end
end
Expand Down

0 comments on commit ec816b1

Please sign in to comment.