Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove legacy sign in method migration for office 365 and smartschool #5791

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Remove legacy sign in method migration for office 365 and smartschool #5791

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 1 addition & 34 deletions app/controllers/auth/omniauth_callbacks_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,40 +222,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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,224 +450,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