Skip to content

Commit

Permalink
Remove legacy sign in method migration for office 365 and smartschool
Browse files Browse the repository at this point in the history
  • Loading branch information
jorg-vr committed Sep 11, 2024
1 parent 5caeb65 commit 760eaf5
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 252 deletions.
35 changes: 1 addition & 34 deletions app/controllers/auth/omniauth_callbacks_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -219,40 +219,7 @@ def find_identity_by_uid
# In case of provider without uids, don't return any identity (As it won't be matching a unique user)
return nil if auth_uid.nil?

identity = Identity.find_by(identifier: auth_uid, provider: provider)

return identity unless identity.nil?

if provider.class.sym == :office365 && auth_email.present?
# This code supports a migration of the office365 oauth api from v1 to v2
# Try to find the user by the legacy identifier
identity = Identity.find_by(identifier: auth_email.split('@').first, provider: provider, identifier_based_on_email: true)

# Try to find user by preferred username
identity = Identity.find_by(identifier: auth_hash.extra.preferred_username.split('@').first, provider: provider, identifier_based_on_email: true) if identity.nil? && auth_hash&.extra&.preferred_username.present?

# Try to find user by name
identity = Identity.joins(:user).find_by(user: { first_name: auth_hash.info.first_name, last_name: auth_hash.info.last_name }, provider: provider, identifier_based_on_email: true) if identity.nil?
return nil if identity.nil?

# Update the identifier to the new uid
identity.update(identifier: auth_uid, identifier_based_on_email: false)
elsif provider.class.sym == :smartschool && auth_username.present?
# This code supports a migration of smartschool usernames to userID as identifier
# Try to find user by username
identity = Identity.find_by(identifier: auth_username, provider: provider, identifier_based_on_username: true)

# Try to find user by email
identity = Identity.joins(:user).find_by(user: { email: auth_email }, provider: provider, identifier_based_on_username: true) if identity.nil?

# Try to find user by name
identity = Identity.joins(:user).find_by(user: { first_name: auth_hash.info.first_name, last_name: auth_hash.info.last_name }, provider: provider, identifier_based_on_username: true) if identity.nil?
return nil if identity.nil?

# Update the identifier to the new uid
identity.update(identifier: auth_uid, identifier_based_on_username: false)
end
identity
Identity.find_by(identifier: auth_uid, provider: provider)
end

def find_identity_by_user(user)
Expand Down
218 changes: 0 additions & 218 deletions test/controllers/auth/omniauth_callbacks_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -449,224 +449,6 @@ def omniauth_path(provider)
end
end

test 'Office 365 identifier should be updated upon login if the identifier still used the old format' do
# Setup.
provider = create :office365_provider
user = create :user, institution: provider.institution
identity = create :identity, provider: provider, user: user, identifier: 'Foo.Bar', identifier_based_on_email: true
omniauth_mock_identity identity,
info: {
email: '[email protected]'
},
uid: 'NEW-UID'

post omniauth_url(provider)
follow_redirect!

assert_equal @controller.current_user, user
identity.reload

assert_equal('NEW-UID', identity.identifier)

# Cleanup.
sign_out user

# sign in should still work with changed identifier
omniauth_mock_identity identity,
info: {
email: '[email protected]'
},
uid: 'NEW-UID'

post omniauth_url(provider)
follow_redirect!

# Assert successful authentication.
assert_redirected_to root_path
assert_equal @controller.current_user, user

# Cleanup.
sign_out user

# Should not be able to change it again
omniauth_mock_identity identity,
info: {
email: '[email protected]'
},
uid: 'NEWER-UID'

post omniauth_url(provider)
follow_redirect!

# Assert successful authentication.
assert_redirected_to root_path
assert_not_equal @controller.current_user, user
identity.reload

assert_equal('NEW-UID', identity.identifier)

# Cleanup.
sign_out user
end

test 'Office 365 legacy sign in works with prefered username' do
# Setup.
provider = create :office365_provider
user = create :user, institution: provider.institution
identity = create :identity, provider: provider, user: user, identifier: 'Foo.Bar', identifier_based_on_email: true
omniauth_mock_identity identity,
info: {
email: '[email protected]'
},
extra: {
preferred_username: '[email protected]'
},
uid: 'NEW-UID'

post omniauth_url(provider)
follow_redirect!

assert_equal @controller.current_user, user
identity.reload

assert_equal('NEW-UID', identity.identifier)

# Cleanup.
sign_out user
end

test 'Office 365 legacy sign in works with name' do
# Setup.
provider = create :office365_provider
user = create :user, institution: provider.institution, first_name: 'Foo', last_name: 'Bar'
identity = create :identity, provider: provider, user: user, identifier: 'X.Y', identifier_based_on_email: true
omniauth_mock_identity identity,
info: {
email: '[email protected]'
},
uid: 'NEW-UID'

post omniauth_url(provider)
follow_redirect!

assert_equal @controller.current_user, user
identity.reload

assert_equal('NEW-UID', identity.identifier)

# Cleanup.
sign_out user
end

test 'Smartschool identifier should be updated upon login if the identifier still used the old format' do
# Setup.
provider = create :smartschool_provider
user = create :user, institution: provider.institution
identity = create :identity, provider: provider, user: user, identifier: 'OLD-UID', identifier_based_on_username: true
omniauth_mock_identity identity,
info: {
username: 'OLD-UID'
},
uid: 'NEW-UID'

post omniauth_url(provider)
follow_redirect!

assert_equal @controller.current_user, user
identity.reload

assert_equal('NEW-UID', identity.identifier)

# Cleanup.
sign_out user

# sign in should still work with changed identifier
omniauth_mock_identity identity,
info: {
username: 'OLD-UID'
},
uid: 'NEW-UID'

post omniauth_url(provider)
follow_redirect!

# Assert successful authentication.
assert_redirected_to root_path
assert_equal @controller.current_user, user

# Cleanup.
sign_out user

# Should not be able to change it again
omniauth_mock_identity identity,
info: {
username: 'NEW-UID'
},
uid: 'NEWER-UID'

post omniauth_url(provider)
follow_redirect!

# Assert successful authentication.
assert_redirected_to root_path
assert_not_equal @controller.current_user, user
identity.reload

assert_equal('NEW-UID', identity.identifier)

# Cleanup.
sign_out user
end

test 'Smartschool legacy sign in works with email' do
# Setup.
provider = create :smartschool_provider
user = create :user, institution: provider.institution, email: '[email protected]'
identity = create :identity, provider: provider, user: user, identifier: 'OLD-UID', identifier_based_on_username: true
omniauth_mock_identity identity,
info: {
email: '[email protected]',
username: 'NEW-USERNAME'
},
uid: 'NEW-UID'

post omniauth_url(provider)
follow_redirect!

assert_equal @controller.current_user, user
identity.reload

assert_equal('NEW-UID', identity.identifier)

# Cleanup.
sign_out user
end

test 'Smartschool legacy sign in works with name' do
# Setup.
provider = create :smartschool_provider
user = create :user, institution: provider.institution, first_name: 'Foo', last_name: 'Bar'
identity = create :identity, provider: provider, user: user, identifier: 'OLD-UID', identifier_based_on_username: true
omniauth_mock_identity identity,
info: {
first_name: 'Foo',
last_name: 'Bar',
username: 'NEW-USERNAME'
},
uid: 'NEW-UID'

post omniauth_url(provider)
follow_redirect!

assert_equal @controller.current_user, user
identity.reload

assert_equal('NEW-UID', identity.identifier)

# Cleanup.
sign_out user
end

test 'Smartschool co-accounts should be blocked' do
# Setup.
provider = create :smartschool_provider
Expand Down

0 comments on commit 760eaf5

Please sign in to comment.