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.

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

Disconnected OAuth AuthenticationOption bug #24459

Merged
merged 7 commits into from Aug 28, 2018
8 changes: 4 additions & 4 deletions dashboard/app/models/user.rb
Expand Up @@ -507,17 +507,17 @@ def make_teachers_21
end

def normalize_email
return unless read_attribute(:email).present?
self.email = read_attribute(:email).strip.downcase
return unless email.present?
self.email = email.strip.downcase
end

def self.hash_email(email)
Digest::MD5.hexdigest(email.downcase)
end

def hash_email
return unless read_attribute(:email).present?
self.hashed_email = User.hash_email(read_attribute(:email))
return unless email.present?
self.hashed_email = User.hash_email(email)
end

# @return [Boolean, nil] Whether the the list of races stored in the `races` column represents an
Expand Down
2 changes: 0 additions & 2 deletions dashboard/lib/user_multi_auth_helper.rb
Expand Up @@ -66,8 +66,6 @@ def migrate_to_multi_auth

def clear_single_auth_fields
raise "Single auth fields may not be cleared on an unmigrated user" unless migrated?
self.email = ''
self.hashed_email = nil
self.uid = nil
self.oauth_token = nil
self.oauth_token_expiration = nil
Expand Down
7 changes: 4 additions & 3 deletions dashboard/test/lib/user_multi_auth_helper_test.rb
Expand Up @@ -354,7 +354,7 @@ def assert_convert_sso_user(user)
assert_raises {user.clear_single_auth_fields}
end

test 'clear_single_auth_fields clears all single-auth fields' do
test 'clear_single_auth_fields clears single-auth fields' do
user = create :teacher, :unmigrated_google_sso
user.migrate_to_multi_auth

Expand All @@ -374,8 +374,9 @@ def assert_convert_sso_user(user)
oauth_token: nil,
oauth_token_expiration: nil,
oauth_refresh_token: nil
assert_empty user.read_attribute(:email)
assert_nil user.read_attribute(:hashed_email)
# Does not clear email or hashed_email fields
refute_empty user.read_attribute(:email)
refute_nil user.read_attribute(:hashed_email)
end

test 'migrate and demigrate picture password student' do
Expand Down
19 changes: 17 additions & 2 deletions dashboard/test/models/user_test.rb
Expand Up @@ -84,17 +84,32 @@ class UserTest < ActiveSupport::TestCase
experiment.destroy
end

test 'normalize_email' do
test 'normalize_email for non-migrated user' do
teacher = create :teacher, email: 'CAPS@EXAMPLE.COM'
assert_equal 'caps@example.com', teacher.email
end

test 'hash_email' do
test 'normalize_email for migrated user' do
teacher = create :teacher, :with_migrated_email_authentication_option, email: 'OLD@EXAMPLE.COM'
teacher.update!(primary_contact_info: create(:authentication_option, user: teacher, email: 'NEW@EXAMPLE.COM'))
assert_equal 'new@example.com', teacher.primary_contact_info.email
assert_equal 'new@example.com', teacher.read_attribute(:email)
end

test 'hash_email for non-migrated user' do
@teacher.update!(email: 'hash_email@example.com')
assert_equal User.hash_email('hash_email@example.com'),
@teacher.hashed_email
end

test 'hash_email for migrated user' do
teacher = create :teacher, :with_migrated_email_authentication_option, email: 'OLD@EXAMPLE.COM'
teacher.update!(primary_contact_info: create(:authentication_option, user: teacher, email: 'NEW@EXAMPLE.COM'))
hashed_email = User.hash_email('new@example.com')
assert_equal hashed_email, teacher.primary_contact_info.hashed_email
assert_equal hashed_email, teacher.read_attribute(:hashed_email)
end

test "log in with password with pepper" do
assert Devise.pepper

Expand Down