Skip to content

Commit

Permalink
Perform extended status group validation for NBP sign-up
Browse files Browse the repository at this point in the history
For all users signing up through the NBP wallet, we want to validate their role. Only those registrations providing a specific role (learner / educator) should be allowed. Any other registrations should not be affected by this change.
  • Loading branch information
MrSerth committed Feb 12, 2025
1 parent aa987d6 commit 6323e63
Show file tree
Hide file tree
Showing 7 changed files with 70 additions and 12 deletions.
9 changes: 1 addition & 8 deletions app/controllers/users/nbp_wallet_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,7 @@ def finalize

private

def accept_and_create_user(relationship) # rubocop:disable Metrics/AbcSize, Metrics/MethodLength
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
user = User.new_from_omniauth(relationship.userdata, 'nbp', @provider_uid)
user.identities << UserIdentity.new(omniauth_provider: 'enmeshed', provider_uid: relationship.peer)
user.skip_confirmation_notification!
Expand Down
4 changes: 4 additions & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ class User < ApplicationRecord

# `Other` is a catch-all for any status that doesn't fit into the other categories and should be *last*.
enum :status_group, {unknown: 0, learner: 2, educator: 3, other: 1}, default: :unknown, prefix: true
# When a user is created through the NBP wallet connection, we want to ensure a valid status group.
validates :status_group, inclusion: {in: %s[learner educator], message: :unrecognized_role}, on: :create, if: lambda {

Check warning on line 51 in app/models/user.rb

View workflow job for this annotation

GitHub Actions / lint

[rubocop] reported by reviewdog 🐶 `%s`-literals should be delimited by `(` and `)`. Raw Output: app/models/user.rb:51:44: C: Style/PercentLiteralDelimiters: `%s`-literals should be delimited by `(` and `)`.
identities.loaded? && identities.any? {|identity| identity.omniauth_provider == 'nbp' }
}

# Called by Devise and overwritten for soft-deletion
def destroy
Expand Down
2 changes: 0 additions & 2 deletions config/locales/de/controllers/users.yml
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
---
de:
users:
nbp_wallet:
unrecognized_role: Unbekannte Rolle. Bitte wählen Sie entweder "Lehrer:in" oder "Schüler:in" als Ihre Rolle.
omniauth_callbacks:
failure_deauthorize: 'Das Trennen von Ihrem %{kind}-Account war aufgrund des folgenden Grundes nicht möglich: "%{reason}".'
failure_deauthorize_last_identity: Sie können sich nicht von Ihrem %{kind}-Account trennen, da dies Ihr letzter verbleibender Account ist. Bitte erstellen Sie ein Passwort, um sich in Zukunft anmelden zu können.
Expand Down
2 changes: 2 additions & 0 deletions config/locales/de/models.yml
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ de:
avatar:
not_an_image: muss ein Bild sein
size_over_10_mb: Größe muss kleiner als 10MB sein
status_group:
unrecognized_role: ist unbekannt. Bitte wählen Sie entweder "Lehrer:in" oder "Schüler:in" als Ihre Rolle.
invalid_api_key: Der API-Schlüssel in Ihrem Profil ist ungültig. Bitte fügen Sie den entsprechenden API-Schlüssel in Ihrem Profil hinzu, um die KI-Funktionen zu nutzen.
models:
account_link:
Expand Down
2 changes: 0 additions & 2 deletions config/locales/en/controllers/users.yml
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
---
en:
users:
nbp_wallet:
unrecognized_role: Unknown role. Please select either "Teacher" or "Student" as your role.
omniauth_callbacks:
failure_deauthorize: Could not remove authorization from %{kind} because "%{reason}".
failure_deauthorize_last_identity: You cannot deauthorize your %{kind} account because it is your last remaining account. Please create a password to log in in the future.
Expand Down
2 changes: 2 additions & 0 deletions config/locales/en/models.yml
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ en:
avatar:
not_an_image: needs to be an image
size_over_10_mb: size needs to be less than 10MB
status_group:
unrecognized_role: is unknown. Please select either "Teacher" or "Student" as your role.
invalid_api_key: The API key in your profile is invalid. Please add the appropriate API key in your profile to use the AI features.
models:
account_link:
Expand Down
61 changes: 61 additions & 0 deletions spec/models/user_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,67 @@
expect(user).to be_valid
end
end

context 'when creating a new user' do
let(:user_info) { {first_name: 'John', last_name: 'Oliver', email: '[email protected]', status_group:} }

shared_examples 'a valid user' do |group|
let(:status_group) { group }

it 'is valid' do

Check failure on line 109 in spec/models/user_spec.rb

View workflow job for this annotation

GitHub Actions / test

User#valid? when creating a new user when using #new_from_omniauth when registered through NBP behaves like a valid user is valid Failure/Error: expect(user).to be_valid NoMethodError: undefined method 'learner educator' for an instance of User Shared Example Group: "a valid user" called from ./spec/models/user_spec.rb:147

Check failure on line 109 in spec/models/user_spec.rb

View workflow job for this annotation

GitHub Actions / test

User#valid? when creating a new user when using #new_from_omniauth when registered through NBP behaves like a valid user is valid Failure/Error: expect(user).to be_valid NoMethodError: undefined method 'learner educator' for an instance of User Shared Example Group: "a valid user" called from ./spec/models/user_spec.rb:146
expect(user).to be_valid
end
end

shared_examples 'an invalid user' do |group|
let(:status_group) { group }

it 'is not valid' do

Check failure on line 117 in spec/models/user_spec.rb

View workflow job for this annotation

GitHub Actions / test

User#valid? when creating a new user when using #new_from_omniauth when registered through NBP behaves like an invalid user is not valid Failure/Error: expect(user).not_to be_valid NoMethodError: undefined method 'learner educator' for an instance of User Shared Example Group: "an invalid user" called from ./spec/models/user_spec.rb:149

Check failure on line 117 in spec/models/user_spec.rb

View workflow job for this annotation

GitHub Actions / test

User#valid? when creating a new user when using #new_from_omniauth when registered through NBP behaves like an invalid user is not valid Failure/Error: expect(user).not_to be_valid NoMethodError: undefined method 'learner educator' for an instance of User Shared Example Group: "an invalid user" called from ./spec/models/user_spec.rb:150
expect(user).not_to be_valid
end

it 'adds an error for status_group' do

Check failure on line 121 in spec/models/user_spec.rb

View workflow job for this annotation

GitHub Actions / test

User#valid? when creating a new user when using #new_from_omniauth when registered through NBP behaves like an invalid user adds an error for status_group Failure/Error: user.valid? NoMethodError: undefined method 'learner educator' for an instance of User Shared Example Group: "an invalid user" called from ./spec/models/user_spec.rb:149

Check failure on line 121 in spec/models/user_spec.rb

View workflow job for this annotation

GitHub Actions / test

User#valid? when creating a new user when using #new_from_omniauth when registered through NBP behaves like an invalid user adds an error for status_group Failure/Error: user.valid? NoMethodError: undefined method 'learner educator' for an instance of User Shared Example Group: "an invalid user" called from ./spec/models/user_spec.rb:150
user.valid?
expect(user.errors[:status_group]).to include(I18n.t('activerecord.errors.models.user.attributes.status_group.unrecognized_role'))
end
end

context 'when using #new' do
subject(:user) { described_class.new(user_info.merge(password:)) }

let(:password) { 'password' }

it_behaves_like 'a valid user', :unknown
it_behaves_like 'a valid user', :learner
it_behaves_like 'a valid user', :educator
it_behaves_like 'a valid user', :other
end

context 'when using #new_from_omniauth' do
subject(:user) { described_class.new_from_omniauth(user_info, provider, provider_uid) }

let(:provider_uid) { '12345' }

context 'when registered through NBP' do
let(:provider) { 'nbp' }

it_behaves_like 'a valid user', :learner
it_behaves_like 'a valid user', :educator

it_behaves_like 'an invalid user', :unknown
it_behaves_like 'an invalid user', :other
end

context 'when registering through BIRD' do
let(:provider) { 'bird' }

it_behaves_like 'a valid user', :unknown
it_behaves_like 'a valid user', :learner
it_behaves_like 'a valid user', :educator
it_behaves_like 'a valid user', :other
end
end
end
end

describe '#destroy' do
Expand Down

0 comments on commit 6323e63

Please sign in to comment.