From 22e76bee84e9796bd4026430a2bf43d6e9fa1bb6 Mon Sep 17 00:00:00 2001 From: Andy Zhao Date: Wed, 3 Jul 2019 12:01:16 -0400 Subject: [PATCH] Add functionality to remove and recover identity (#3377) * Lint some quotation marks * Add BackupData table * Add identity and removal functionality * Test additional functionality * Remove dependent destroy for backup data * Add auth_data_dump column * Add challenge to reserved words * Add more shoulda matchers --- app/controllers/internal/users_controller.rb | 28 ++++++++++- app/models/backup_data.rb | 15 ++++++ app/models/identity.rb | 1 + app/models/user.rb | 1 + app/views/internal/users/edit.html.erb | 48 +++++++++++++++++++ app/views/users/_account.html.erb | 16 +++---- config/initializers/reserved_words.rb | 1 + config/routes.rb | 2 + ...20190703003817_create_backup_data_table.rb | 12 +++++ db/schema.rb | 11 ++++- spec/factories/identities.rb | 7 +++ spec/models/backup_data_spec.rb | 11 +++++ spec/requests/internal_users_spec.rb | 32 +++++++++++++ 13 files changed, 175 insertions(+), 10 deletions(-) create mode 100644 app/models/backup_data.rb create mode 100644 db/migrate/20190703003817_create_backup_data_table.rb create mode 100644 spec/models/backup_data_spec.rb diff --git a/app/controllers/internal/users_controller.rb b/app/controllers/internal/users_controller.rb index f292b6bba..55cd27542 100644 --- a/app/controllers/internal/users_controller.rb +++ b/app/controllers/internal/users_controller.rb @@ -75,6 +75,32 @@ class Internal::UsersController < Internal::ApplicationController redirect_to "/internal/users/#{@user.id}/edit" end + def remove_identity + identity = Identity.find(user_params[:identity_id]) + @user = identity.user + begin + BackupData.backup!(identity) + identity.delete + @user.update("#{identity.provider}_username" => nil) + flash[:success] = "The #{identity.provider.capitalize} identity was successfully deleted and backed up." + rescue StandardError => e + flash[:error] = e.message + end + redirect_to "/internal/users/#{@user.id}/edit" + end + + def recover_identity + backup = BackupData.find(user_params[:backup_data_id]) + @user = backup.instance_user + begin + identity = backup.recover! + flash[:success] = "The #{identity.provider} identity was successfully recovered, and the backup was removed." + rescue StandardError => e + flash[:error] = e.message + end + redirect_to "/internal/users/#{@user.id}/edit" + end + private def manage_credits @@ -121,7 +147,7 @@ class Internal::UsersController < Internal::ApplicationController new_note note_for_current_role user_status pro merge_user_id add_credits remove_credits add_org_credits remove_org_credits ghostify - organization_id + organization_id identity_id backup_data_id ] params.require(:user).permit(allowed_params) end diff --git a/app/models/backup_data.rb b/app/models/backup_data.rb new file mode 100644 index 000000000..3665bbf5a --- /dev/null +++ b/app/models/backup_data.rb @@ -0,0 +1,15 @@ +class BackupData < ApplicationRecord + belongs_to :instance, polymorphic: true + belongs_to :instance_user, class_name: "User" + validates :instance_id, :instance_type, :json_data, presence: true + + def self.backup!(instance) + BackupData.create!(instance_type: instance.class.name, instance_id: instance.id, instance_user_id: instance.user_id, json_data: instance.attributes) + end + + def recover! + instance = instance_type.constantize.create!(json_data.to_h) + destroy! + instance + end +end diff --git a/app/models/identity.rb b/app/models/identity.rb index ed5a2b082..02746769f 100644 --- a/app/models/identity.rb +++ b/app/models/identity.rb @@ -1,5 +1,6 @@ class Identity < ApplicationRecord belongs_to :user + has_many :backup_data, as: :instance, class_name: "BackupData", dependent: :destroy validates :uid, :provider, presence: true validates :uid, uniqueness: { scope: :provider }, if: proc { |i| i.uid_changed? || i.provider_changed? } validates :user_id, uniqueness: { scope: :provider }, if: proc { |i| i.user_id_changed? || i.provider_changed? } diff --git a/app/models/user.rb b/app/models/user.rb index 2072e5ec6..ff7535669 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -44,6 +44,7 @@ class User < ApplicationRecord has_many :classified_listings has_many :poll_votes has_many :poll_skips + has_many :backup_data, foreign_key: "instance_user_id", inverse_of: :instance_user, class_name: "BackupData" mount_uploader :profile_image, ProfileImageUploader diff --git a/app/views/internal/users/edit.html.erb b/app/views/internal/users/edit.html.erb index ea4f0380c..e56eda72a 100644 --- a/app/views/internal/users/edit.html.erb +++ b/app/views/internal/users/edit.html.erb @@ -39,6 +39,54 @@ <% end %> <%= render "notes" %> +
+

Remove Identity

+

Removing a social account identity can solve certain sign in issues, for example:

+ +

Merge User

To merge a duplicate account, make sure you are currently on the page of the user you want to KEEP. Below, add the user id of the account to merge information from, and delete.

diff --git a/app/views/users/_account.html.erb b/app/views/users/_account.html.erb index bc9193e27..b5a9e1816 100644 --- a/app/views/users/_account.html.erb +++ b/app/views/users/_account.html.erb @@ -1,4 +1,4 @@ -<% unless @user.identities.exists?(provider: 'github') %> +<% unless @user.identities.exists?(provider: "github") %>
" alt="github logo"> CONNECT GITHUB ACCOUNT @@ -7,7 +7,7 @@
<% end %> -<% unless @user.identities.exists?(provider: 'twitter') %> +<% unless @user.identities.exists?(provider: "twitter") %>
<%= ApplicationConfig["APP_DOMAIN"] %>/users/auth/twitter/callback" class="big-button cta" data-no-instant> " alt="twitter logo"> CONNECT TWITTER ACCOUNT @@ -37,14 +37,14 @@ <% end %> <% @user.api_secrets.order(created_at: :desc).each do |api_secret| %> -
-
-

<%= api_secret.description %>

-

<%= api_secret.secret %>

-

Created <%= api_secret.created_at.to_date.to_s %>

+
+
+

<%= api_secret.description %>

+

<%= api_secret.secret %>

+

Created <%= api_secret.created_at.to_date.to_s %>

-
+
<%= form_tag users_api_secret_path(api_secret.id), method: :delete do %> <%= button_tag "Revoke", class: "big-action danger-button" %> <% end %> diff --git a/config/initializers/reserved_words.rb b/config/initializers/reserved_words.rb index 8e8783d95..7cc977a63 100644 --- a/config/initializers/reserved_words.rb +++ b/config/initializers/reserved_words.rb @@ -32,6 +32,7 @@ class ReservedWords rly connect chat_channels + challenge email_subscriptions social_previews code-of-conduct diff --git a/config/routes.rb b/config/routes.rb index fc1b8ca68..a93bcaa7c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -51,6 +51,8 @@ Rails.application.routes.draw do post "full_delete" patch "user_status" post "merge" + delete "remove_identity" + post "recover_identity" end end resources :welcome, only: %i[index create] diff --git a/db/migrate/20190703003817_create_backup_data_table.rb b/db/migrate/20190703003817_create_backup_data_table.rb new file mode 100644 index 000000000..b806cda5e --- /dev/null +++ b/db/migrate/20190703003817_create_backup_data_table.rb @@ -0,0 +1,12 @@ +class CreateBackupDataTable < ActiveRecord::Migration[5.2] + def change + create_table :backup_data do |t| + t.bigint :instance_id, null: false + t.string :instance_type, null: false + t.bigint :instance_user_id + t.jsonb :json_data, null: false + + t.timestamps null: false + end + end +end diff --git a/db/schema.rb b/db/schema.rb index adf402aa2..95da46377 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -12,7 +12,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2019_06_28_123548) do +ActiveRecord::Schema.define(version: 2019_07_03_003817) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -144,6 +144,15 @@ ActiveRecord::Schema.define(version: 2019_06_28_123548) do t.index ["user_id"], name: "index_articles_on_user_id" end + create_table "backup_data", force: :cascade do |t| + t.datetime "created_at", null: false + t.bigint "instance_id", null: false + t.string "instance_type", null: false + t.bigint "instance_user_id" + t.jsonb "json_data", null: false + t.datetime "updated_at", null: false + end + create_table "badge_achievements", force: :cascade do |t| t.bigint "badge_id", null: false t.datetime "created_at", null: false diff --git a/spec/factories/identities.rb b/spec/factories/identities.rb index 5dc5f2e4d..b6e7cd57b 100644 --- a/spec/factories/identities.rb +++ b/spec/factories/identities.rb @@ -6,5 +6,12 @@ FactoryBot.define do provider { "github" } token { rand(100_000) } secret { rand(100_000) } + auth_data_dump do + { + "info" => { + "nickname" => "something" + } + } + end end end diff --git a/spec/models/backup_data_spec.rb b/spec/models/backup_data_spec.rb new file mode 100644 index 000000000..1c4cb4088 --- /dev/null +++ b/spec/models/backup_data_spec.rb @@ -0,0 +1,11 @@ +require "rails_helper" + +RSpec.describe BackupData, type: :model do + describe "validations" do + it { is_expected.to validate_presence_of(:instance_type) } + it { is_expected.to validate_presence_of(:instance_id) } + it { is_expected.to validate_presence_of(:json_data) } + it { is_expected.to belong_to(:instance) } + it { is_expected.to belong_to(:instance_user).class_name("User") } + end +end diff --git a/spec/requests/internal_users_spec.rb b/spec/requests/internal_users_spec.rb index 820c54bfa..9f54c08cf 100644 --- a/spec/requests/internal_users_spec.rb +++ b/spec/requests/internal_users_spec.rb @@ -45,4 +45,36 @@ RSpec.describe "internal/users", type: :request do expect(user.reload.username).to include("spam") end end + + describe "DELETE internal/users/:id/remove_identity" do + it "removes the given identity" do + identity = user.identities.first + delete "/internal/users/#{user.id}/remove_identity", params: { user: { identity_id: identity.id } } + expect { identity.reload }.to raise_error ActiveRecord::RecordNotFound + end + + it "updates their social account's username to nil" do + identity = user.identities.first + delete "/internal/users/#{user.id}/remove_identity", params: { user: { identity_id: identity.id } } + expect(user.reload.github_username).to eq nil + end + end + + describe "POST internal/users/:id/recover_identity" do + it "recovers a deleted identity" do + identity = user.identities.first + backup = BackupData.backup!(identity) + identity.delete + post "/internal/users/#{user.id}/recover_identity", params: { user: { backup_data_id: backup.id } } + expect(identity).to eq Identity.first + end + + it "deletes the backup data" do + identity = user.identities.first + backup = BackupData.backup!(identity) + identity.delete + post "/internal/users/#{user.id}/recover_identity", params: { user: { backup_data_id: backup.id } } + expect { backup.reload }.to raise_error ActiveRecord::RecordNotFound + end + end end