From ee58baf14e4e02afecd15d717671997d7e34ab9e Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Mon, 29 Jul 2024 13:19:07 +0200 Subject: [PATCH 1/3] Move account private key to dedicated table --- app/models/account.rb | 13 +++++++++++++ app/models/account_secret.rb | 17 +++++++++++++++++ app/models/concerns/account/associations.rb | 3 +++ .../20240726143215_create_account_secrets.rb | 12 ++++++++++++ db/schema.rb | 11 ++++++++++- spec/models/account_spec.rb | 11 +++++++++++ 6 files changed, 66 insertions(+), 1 deletion(-) create mode 100644 app/models/account_secret.rb create mode 100644 db/migrate/20240726143215_create_account_secrets.rb diff --git a/app/models/account.rb b/app/models/account.rb index 23ff07c7696..ad2d3899828 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -501,6 +501,19 @@ class Account < ApplicationRecord save! end + def private_key + if account_secret + account_secret.private_key + else + super + end + end + + def private_key=(value) + self.account_secret ||= AccountSecret.new + account_secret.private_key = value + end + private def prepare_contents diff --git a/app/models/account_secret.rb b/app/models/account_secret.rb new file mode 100644 index 00000000000..772bc8562d0 --- /dev/null +++ b/app/models/account_secret.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +# == Schema Information +# +# Table name: account_secrets +# +# id :bigint(8) not null, primary key +# private_key :text +# account_id :bigint(8) not null +# created_at :datetime not null +# updated_at :datetime not null +# +class AccountSecret < ApplicationRecord + belongs_to :account + + encrypts :private_key +end diff --git a/app/models/concerns/account/associations.rb b/app/models/concerns/account/associations.rb index 1c67b07e511..19a7ec25409 100644 --- a/app/models/concerns/account/associations.rb +++ b/app/models/concerns/account/associations.rb @@ -77,5 +77,8 @@ module Account::Associations # Imports has_many :bulk_imports, inverse_of: :account, dependent: :delete_all + + # Secrets + has_one :account_secret, inverse_of: :account, dependent: :destroy, autosave: true end end diff --git a/db/migrate/20240726143215_create_account_secrets.rb b/db/migrate/20240726143215_create_account_secrets.rb new file mode 100644 index 00000000000..30cda823e6d --- /dev/null +++ b/db/migrate/20240726143215_create_account_secrets.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +class CreateAccountSecrets < ActiveRecord::Migration[7.1] + def change + create_table :account_secrets do |t| + t.text :private_key + t.references :account, null: false, foreign_key: true + + t.timestamps + end + end +end diff --git a/db/schema.rb b/db/schema.rb index d4796079cae..618a29baf2d 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2024_07_24_181224) do +ActiveRecord::Schema[7.1].define(version: 2024_07_26_143215) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -103,6 +103,14 @@ ActiveRecord::Schema[7.1].define(version: 2024_07_24_181224) do t.index ["relationship_severance_event_id"], name: "idx_on_relationship_severance_event_id_403f53e707" end + create_table "account_secrets", force: :cascade do |t| + t.text "private_key" + t.bigint "account_id", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["account_id"], name: "index_account_secrets_on_account_id" + end + create_table "account_stats", force: :cascade do |t| t.bigint "account_id", null: false t.bigint "statuses_count", default: 0, null: false @@ -1274,6 +1282,7 @@ ActiveRecord::Schema[7.1].define(version: 2024_07_24_181224) do add_foreign_key "account_pins", "accounts", on_delete: :cascade add_foreign_key "account_relationship_severance_events", "accounts", on_delete: :cascade add_foreign_key "account_relationship_severance_events", "relationship_severance_events", on_delete: :cascade + add_foreign_key "account_secrets", "accounts" add_foreign_key "account_stats", "accounts", on_delete: :cascade add_foreign_key "account_statuses_cleanup_policies", "accounts", on_delete: :cascade add_foreign_key "account_warnings", "accounts", column: "target_account_id", on_delete: :cascade diff --git a/spec/models/account_spec.rb b/spec/models/account_spec.rb index 8e5648a0b0e..f0bf23dcc14 100644 --- a/spec/models/account_spec.rb +++ b/spec/models/account_spec.rb @@ -1067,4 +1067,15 @@ RSpec.describe Account do expect(subject.reload.followers_count).to eq 15 end end + + describe 'private key' do + it 'encrypts and decrypts the key' do + account = Fabricate(:account) + + account.private_key = 'secret' + account.save! + + expect(account.reload.private_key).to eq 'secret' + end + end end From bab725fc31eca8a53bf3beca4e297f77e625e921 Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Mon, 29 Jul 2024 13:50:38 +0200 Subject: [PATCH 2/3] Add on_delete: :cascade --- db/migrate/20240726143215_create_account_secrets.rb | 2 +- db/schema.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/db/migrate/20240726143215_create_account_secrets.rb b/db/migrate/20240726143215_create_account_secrets.rb index 30cda823e6d..1cdc17a60ca 100644 --- a/db/migrate/20240726143215_create_account_secrets.rb +++ b/db/migrate/20240726143215_create_account_secrets.rb @@ -4,7 +4,7 @@ class CreateAccountSecrets < ActiveRecord::Migration[7.1] def change create_table :account_secrets do |t| t.text :private_key - t.references :account, null: false, foreign_key: true + t.references :account, null: false, foreign_key: { on_delete: :cascade } t.timestamps end diff --git a/db/schema.rb b/db/schema.rb index 618a29baf2d..5188537d573 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1282,7 +1282,7 @@ ActiveRecord::Schema[7.1].define(version: 2024_07_26_143215) do add_foreign_key "account_pins", "accounts", on_delete: :cascade add_foreign_key "account_relationship_severance_events", "accounts", on_delete: :cascade add_foreign_key "account_relationship_severance_events", "relationship_severance_events", on_delete: :cascade - add_foreign_key "account_secrets", "accounts" + add_foreign_key "account_secrets", "accounts", on_delete: :cascade add_foreign_key "account_stats", "accounts", on_delete: :cascade add_foreign_key "account_statuses_cleanup_policies", "accounts", on_delete: :cascade add_foreign_key "account_warnings", "accounts", column: "target_account_id", on_delete: :cascade From 7ac305367360f256d2d679f7e215aeb0c4d94130 Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Mon, 29 Jul 2024 14:54:41 +0200 Subject: [PATCH 3/3] Fix failing specs --- spec/models/concerns/account/counters_spec.rb | 14 ++++++-------- spec/requests/api/v1/directories_spec.rb | 18 +++++++++--------- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/spec/models/concerns/account/counters_spec.rb b/spec/models/concerns/account/counters_spec.rb index ccac9e95de6..036b7b99e93 100644 --- a/spec/models/concerns/account/counters_spec.rb +++ b/spec/models/concerns/account/counters_spec.rb @@ -46,16 +46,14 @@ describe Account::Counters do end it 'preserves last_status_at when decrementing statuses_count' do - account_stat = Fabricate( - :account_stat, - account: account, - last_status_at: 3.days.ago, - statuses_count: 10 - ) + account.statuses_count = 10 + account.save! + account.account_stat.last_status_at = 3.days.ago + account.account_stat.save! expect { account.decrement_count!(:statuses_count) } - .to change(account_stat.reload, :statuses_count).by(-1) - .and not_change(account_stat.reload, :last_status_at) + .to change(account.account_stat.reload, :statuses_count).by(-1) + .and not_change(account.account_stat.reload, :last_status_at) end end end diff --git a/spec/requests/api/v1/directories_spec.rb b/spec/requests/api/v1/directories_spec.rb index 0a1864d136c..286fcbecc5a 100644 --- a/spec/requests/api/v1/directories_spec.rb +++ b/spec/requests/api/v1/directories_spec.rb @@ -17,7 +17,7 @@ describe 'Directories API' do user: Fabricate(:user, confirmed_at: nil, approved: true), username: 'local_unconfirmed' ) - local_unconfirmed_account.create_account_stat! + local_unconfirmed_account.account_stat.save! local_unapproved_account = Fabricate( :account, @@ -25,7 +25,7 @@ describe 'Directories API' do user: Fabricate(:user, confirmed_at: 10.days.ago), username: 'local_unapproved' ) - local_unapproved_account.create_account_stat! + local_unapproved_account.account_stat.save! local_unapproved_account.user.update(approved: false) local_undiscoverable_account = Fabricate( @@ -35,7 +35,7 @@ describe 'Directories API' do discoverable: false, username: 'local_undiscoverable' ) - local_undiscoverable_account.create_account_stat! + local_undiscoverable_account.account_stat.save! excluded_from_timeline_account = Fabricate( :account, @@ -43,7 +43,7 @@ describe 'Directories API' do discoverable: true, username: 'remote_excluded_from_timeline' ) - excluded_from_timeline_account.create_account_stat! + excluded_from_timeline_account.account_stat.save! Fabricate(:block, account: user.account, target_account: excluded_from_timeline_account) domain_blocked_account = Fabricate( @@ -52,11 +52,11 @@ describe 'Directories API' do discoverable: true, username: 'remote_domain_blocked' ) - domain_blocked_account.create_account_stat! + domain_blocked_account.account_stat.save! Fabricate(:account_domain_block, account: user.account, domain: 'test.example') - local_discoverable_account.create_account_stat! - eligible_remote_account.create_account_stat! + local_discoverable_account.account_stat.save! + eligible_remote_account.account_stat.save! end let(:local_discoverable_account) do @@ -93,8 +93,8 @@ describe 'Directories API' do let(:remote_account) { Fabricate(:account, domain: 'host.example') } before do - local_account.create_account_stat! - remote_account.create_account_stat! + local_account.account_stat.save! + remote_account.account_stat.save! end it 'returns only the local accounts' do