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

Explicitly add provider to user account soft delete redshift logging #25048

Merged
merged 1 commit into from
Sep 26, 2018
Merged

Explicitly add provider to user account soft delete redshift logging #25048

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 8 additions & 5 deletions dashboard/app/controllers/omniauth_callbacks_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ def connect_provider
move_sections_and_destroy_source_user \
source_user: existing_credential_holder,
destination_user: current_user,
takeover_type: 'connect_provider'
takeover_type: 'connect_provider',
provider: provider
end

# TODO: some of this won't work right for non-Google providers, because info comes in differently
Expand Down Expand Up @@ -351,13 +352,14 @@ def allows_google_classroom_takeover(user)
def silent_takeover(oauth_user, auth_hash)
lookup_email = oauth_user.email.presence || auth_hash.info.email
lookup_user = User.find_by_email_or_hashed_email(lookup_email)
provider = auth_hash.provider.to_s

unless lookup_user.present?
# Even if silent takeover is not available for student imported from Google Classroom, we still want
# to attach the email received from Google login to the student's account since GC imports do not provide emails.
if allows_google_classroom_takeover(oauth_user)
oauth_user.update_email_for(
provider: auth_hash.provider.to_s,
provider: provider,
uid: auth_hash.uid,
email: lookup_email
)
Expand All @@ -370,7 +372,8 @@ def silent_takeover(oauth_user, auth_hash)
return unless move_sections_and_destroy_source_user(
source_user: oauth_user,
destination_user: lookup_user,
takeover_type: 'silent'
takeover_type: 'silent',
provider: provider,
)
end

Expand All @@ -379,7 +382,7 @@ def silent_takeover(oauth_user, auth_hash)
AuthenticationOption.create!(
user: lookup_user,
email: lookup_email,
credential_type: auth_hash.provider.to_s,
credential_type: provider,
authentication_id: auth_hash.uid,
data: {
oauth_token: auth_hash.credentials&.token,
Expand All @@ -390,7 +393,7 @@ def silent_takeover(oauth_user, auth_hash)
else
lookup_user.update!(
email: lookup_email,
provider: auth_hash.provider.to_s,
provider: provider,
uid: auth_hash.uid,
oauth_token: auth_hash.credentials&.token,
oauth_token_expiration: auth_hash.credentials&.expires_at,
Expand Down
4 changes: 2 additions & 2 deletions dashboard/app/controllers/registrations_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ def email_preference_params(source, form_kind)
def log_account_deletion_to_firehose(current_user, dependent_users)
# Log event for user initiating account deletion.
FirehoseClient.instance.put_record(
study: 'user-soft-delete-audit',
study: 'user-soft-delete-audit-v2',
event: 'initiated-account-deletion',
user_id: current_user.id,
data_json: {
Expand All @@ -400,7 +400,7 @@ def log_account_deletion_to_firehose(current_user, dependent_users)
# This should only happen for teachers.
dependent_users.each do |user|
FirehoseClient.instance.put_record(
study: 'user-soft-delete-audit',
study: 'user-soft-delete-audit-v2',
event: 'dependent-account-deletion',
user_id: user[:id],
data_json: {
Expand Down
9 changes: 5 additions & 4 deletions dashboard/app/helpers/users_helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@ module UsersHelper

# Move followed sections from source_user to destination_user and destroy source_user.
# Returns a boolean - true if all steps were successful, false otherwise.
def move_sections_and_destroy_source_user(source_user:, destination_user:, takeover_type:)
def move_sections_and_destroy_source_user(source_user:, destination_user:, takeover_type:, provider:)
# No-op if source_user is nil
return true unless source_user.present?

firehose_params = {
source_user: source_user,
destination_user: destination_user,
type: takeover_type,
provider: destination_user.provider,
provider: provider,
}

if source_user.has_activity?
Expand Down Expand Up @@ -65,7 +65,8 @@ def check_and_apply_oauth_takeover(user)
return unless move_sections_and_destroy_source_user(
source_user: existing_account,
destination_user: user,
takeover_type: 'oauth'
takeover_type: 'oauth',
provider: provider,
)

if user.migrated?
Expand Down Expand Up @@ -96,7 +97,7 @@ def check_and_apply_oauth_takeover(user)

def log_account_takeover_to_firehose(source_user:, destination_user:, type:, provider:, error: nil)
FirehoseClient.instance.put_record(
study: 'user-soft-delete-audit',
study: 'user-soft-delete-audit-v2',
event: "#{type}-account-takeover", # Silent or OAuth takeover
user_id: source_user.id, # User account being "taken over" (deleted)
data_int: destination_user.id, # User account after takeover
Expand Down