From 2d23a2601e744719f0696db209275cd85fdbb140 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonne=20Ha=C3=9F?= Date: Thu, 9 May 2019 16:37:55 +0200 Subject: [PATCH 1/2] fix old photos without a remote_photo_path or remote_photo_name closes #8012 --- Changelog.md | 3 +++ .../20190509125709_fix_missing_remote_photo_fields.rb | 11 +++++++++++ 2 files changed, 14 insertions(+) create mode 100644 db/migrate/20190509125709_fix_missing_remote_photo_fields.rb diff --git a/Changelog.md b/Changelog.md index 2dbd4123d..90b3bff77 100644 --- a/Changelog.md +++ b/Changelog.md @@ -4,6 +4,9 @@ * Enable paranoid mode for devise [#8003](https://github.com/diaspora/diaspora/pull/8003) * Refactor likes cucumber test [#8002](https://github.com/diaspora/diaspora/pull/8002) +## Bug fixes +* Fix old photos without remote url for export [#8012](https://github.com/diaspora/diaspora/pull/8012) + ## Features * Add a manifest.json file as a first step to make diaspora\* a Progressive Web App [#7998](https://github.com/diaspora/diaspora/pull/7998) * Allow `web+diaspora://` links to link to a profile with only the diaspora ID [#8000](https://github.com/diaspora/diaspora/pull/8000) diff --git a/db/migrate/20190509125709_fix_missing_remote_photo_fields.rb b/db/migrate/20190509125709_fix_missing_remote_photo_fields.rb new file mode 100644 index 000000000..8d6462643 --- /dev/null +++ b/db/migrate/20190509125709_fix_missing_remote_photo_fields.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class FixMissingRemotePhotoFields < ActiveRecord::Migration[5.1] + def up + Photo.where(remote_photo_path: nil).each do |photo| + photo.write_attribute(:unprocessed_image, photo.read_attribute(:processed_image)) + photo.update_remote_path + photo.save! + end + end +end From 165b8f4f6e4e0a0c7a4577c1f3a9ffcb3d8a7a09 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Sat, 11 May 2019 19:06:42 +0200 Subject: [PATCH 2/2] Don't encrypt the OTP secret It doesn't add any security to have this encrypted, but it adds complexity for podmins, because they need to backup the key. closes #8014 --- .gitignore | 1 - app/models/user.rb | 3 +- ...29175654_add_devise_two_factor_to_users.rb | 12 +++-- ...0190511150503_decrypt_two_factor_secret.rb | 52 +++++++++++++++++++ lib/configuration_methods.rb | 18 ------- lib/tasks/generate_2fa_encription_key.rake | 24 --------- spec/models/user_spec.rb | 4 +- 7 files changed, 61 insertions(+), 53 deletions(-) create mode 100644 db/migrate/20190511150503_decrypt_two_factor_secret.rb delete mode 100644 lib/tasks/generate_2fa_encription_key.rake diff --git a/.gitignore b/.gitignore index 48f5537ad..65d3c2649 100644 --- a/.gitignore +++ b/.gitignore @@ -11,7 +11,6 @@ app/assets/images/custom/ # Configuration files config/diaspora.yml config/initializers/secret_token.rb -config/initializers/twofa_encryption_key.rb .bundle vendor/bundle/ vendor/cache/ diff --git a/app/models/user.rb b/app/models/user.rb index 0f9336eca..8d30efa97 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -19,11 +19,10 @@ class User < ApplicationRecord scope :halfyear_actives, ->(time = Time.now) { logged_in_since(time - 6.month) } scope :active, -> { joins(:person).where(people: {closed_account: false}) } - attribute :otp_secret + attr_encrypted :otp_secret, if: false, prefix: "plain_" devise :two_factor_authenticatable, :two_factor_backupable, - otp_secret_encryption_key: AppConfig.twofa_encryption_key, otp_backup_code_length: 16, otp_number_of_backup_codes: 10 diff --git a/db/migrate/20171229175654_add_devise_two_factor_to_users.rb b/db/migrate/20171229175654_add_devise_two_factor_to_users.rb index f66a4e77e..47280dd20 100644 --- a/db/migrate/20171229175654_add_devise_two_factor_to_users.rb +++ b/db/migrate/20171229175654_add_devise_two_factor_to_users.rb @@ -2,10 +2,12 @@ class AddDeviseTwoFactorToUsers < ActiveRecord::Migration[5.1] def change - add_column :users, :encrypted_otp_secret, :string - add_column :users, :encrypted_otp_secret_iv, :string - add_column :users, :encrypted_otp_secret_salt, :string - add_column :users, :consumed_timestep, :integer - add_column :users, :otp_required_for_login, :boolean + change_table :users, bulk: true do |t| + t.string :encrypted_otp_secret + t.string :encrypted_otp_secret_iv + t.string :encrypted_otp_secret_salt + t.integer :consumed_timestep + t.boolean :otp_required_for_login + end end end diff --git a/db/migrate/20190511150503_decrypt_two_factor_secret.rb b/db/migrate/20190511150503_decrypt_two_factor_secret.rb new file mode 100644 index 000000000..b1d21f5f6 --- /dev/null +++ b/db/migrate/20190511150503_decrypt_two_factor_secret.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +class DecryptTwoFactorSecret < ActiveRecord::Migration[5.1] + class User < ApplicationRecord + end + + def up + add_column :users, :plain_otp_secret, :string + + key = twofa_encryption_key + decrypt_existing_secrets(key) if key + + change_table :users, bulk: true do |t| + t.remove :encrypted_otp_secret + t.remove :encrypted_otp_secret_iv + t.remove :encrypted_otp_secret_salt + end + end + + def down + raise ActiveRecord::IrreversibleMigration + end + + private + + def twofa_encryption_key + if AppConfig.heroku? + ENV["TWOFA_ENCRYPTION_KEY"] + else + key_file = File.expand_path("../../config/initializers/twofa_encryption_key.rb", File.dirname(__FILE__)) + + if File.exist? key_file + require key_file + File.delete(key_file) + + return Diaspora::Application.config.twofa_encryption_key + end + end + end + + def decrypt_existing_secrets(key) + User.where.not(encrypted_otp_secret: nil).each do |user| + user.plain_otp_secret = Encryptor.decrypt( + value: user.encrypted_otp_secret.unpack("m").first, + key: key, + iv: user.encrypted_otp_secret_iv.unpack("m").first, + salt: user.encrypted_otp_secret_salt.slice(1..-1).unpack("m").first + ) + user.save! + end + end +end diff --git a/lib/configuration_methods.rb b/lib/configuration_methods.rb index 487b70672..58a050f8c 100644 --- a/lib/configuration_methods.rb +++ b/lib/configuration_methods.rb @@ -68,24 +68,6 @@ module Configuration end end - def twofa_encryption_key - if heroku? - return ENV["TWOFA_ENCRYPTION_KEY"] if ENV["TWOFA_ENCRYPTION_KEY"] - - warn "FATAL: Running on Heroku with TWOFA_ENCRYPTION_KEY unset" - warn " Run heroku config:add TWOFA_ENCRYPTION_KEY=#{SecureRandom.hex(32)}" - abort - else - key_file = File.expand_path( - "../config/initializers/twofa_encryption_key.rb", - File.dirname(__FILE__) - ) - system "DISABLE_SPRING=1 bin/rake generate:twofa_key" unless File.exist? key_file - require key_file - Diaspora::Application.config.twofa_encryption_key - end - end - def version_string return @version_string unless @version_string.nil? diff --git a/lib/tasks/generate_2fa_encription_key.rake b/lib/tasks/generate_2fa_encription_key.rake deleted file mode 100644 index 53572f51f..000000000 --- a/lib/tasks/generate_2fa_encription_key.rake +++ /dev/null @@ -1,24 +0,0 @@ -# frozen_string_literal: true - -namespace :generate do - desc "Generates a key for encrypting 2fa tokens" - task :twofa_key do - path = Rails.root.join("config", "initializers", "twofa_encryption_key.rb") - key = SecureRandom.hex(32) - File.open(path, "w") do |f| - f.write <<~CONF - # frozen_string_literal: true - - # The 2fa encryption key is used to encrypt users' OTP tokens in the database. - - # You can regenerate this key by running `rake generate:twofa_key` - - # If you change this key after a user has set up 2fa - # the users' tokens cannot be recovered - # and they will not be able to log in again! - - Diaspora::Application.config.twofa_encryption_key = "#{key}" - CONF - end - end -end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 13aa6fae8..560432927 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -984,9 +984,7 @@ describe User, :type => :model do exported_at exported_photos_at consumed_timestep - encrypted_otp_secret - encrypted_otp_secret_iv - encrypted_otp_secret_salt + plain_otp_secret otp_backup_codes otp_required_for_login otp_secret