Allow users to update their OpenPGP pubkey

app/controllers/settings_controller.rb

@@ -25,7 +25,8 @@ class SettingsController < ApplicationController
   def update
     @user.preferences.merge!(user_params[:preferences] || {})
     @user.display_name = user_params[:display_name]
-    @user.avatar_new = user_params[:avatar]
+    @user.avatar_new   = user_params[:avatar]
+    @user.pgp_pubkey   = user_params[:pgp_pubkey]
 
     if @user.save
       if @user.display_name && (@user.display_name != @user.ldap_entry[:display_name])
@@ -36,6 +37,10 @@ class SettingsController < ApplicationController
         LdapManager::UpdateAvatar.call(dn: @user.dn, file: @user.avatar_new)
       end
 
+      if @user.pgp_pubkey && (@user.pgp_pubkey != @user.ldap_entry[:pgp_key])
+        UserManager::UpdatePgpKey.call(user: @user)
+      end
+
       redirect_to setting_path(@settings_section), flash: {
         success: 'Settings saved.'
       }
@@ -157,7 +162,8 @@ class SettingsController < ApplicationController
 
     def user_params
       params.require(:user).permit(
-        :display_name, :avatar, preferences: UserPreferences.pref_keys
+        :display_name, :avatar, :pgp_pubkey,
+        preferences: UserPreferences.pref_keys
       )
     end
 

app/models/user.rb

@@ -52,7 +52,7 @@ class User < ApplicationRecord
 
   validate :acceptable_avatar
 
-  validate :acceptable_pgp_key_format, if: -> { defined?(@pgp_pubkey) && @pgp_pubkey != "" }
+  validate :acceptable_pgp_key_format, if: -> { defined?(@pgp_pubkey) && @pgp_pubkey.present? }
 
   #
   # Scopes

app/services/ldap_manager/update_pgp_key.rb

@@ -0,0 +1,16 @@
+module LdapManager
+  class UpdatePgpKey < LdapManagerService
+    def initialize(dn:, pubkey:)
+      @dn = dn
+      @pubkey = pubkey
+    end
+
+    def call
+      if @pubkey.present?
+        replace_attribute @dn, :pgpKey, @pubkey
+      else
+        delete_attribute @dn, :pgpKey
+      end
+    end
+  end
+end

app/services/user_manager/update_pgp_key.rb

@@ -0,0 +1,24 @@
+module UserManager
+  class UpdatePgpKey < UserManagerService
+    def initialize(user:)
+      @user = user
+    end
+
+    def call
+      if @user.pgp_pubkey.blank?
+        @user.update! pgp_fpr: nil
+      else
+        result = GPGME::Key.import(@user.pgp_pubkey)
+
+        if result.imports.present?
+          @user.update! pgp_fpr: result.imports.first.fpr
+        else
+          # TODO notify Sentry, user
+          raise "Failed to import OpenPGP pubkey"
+        end
+      end
+
+      LdapManager::UpdatePgpKey.call(dn: @user.dn, pubkey: @user.pgp_pubkey)
+    end
+  end
+end

app/views/settings/_account.html.erb

@@ -1,6 +1,6 @@
 <%= tag.section data: {
       controller: "settings--account--email",
-      "settings--account--email-validation-failed-value": @validation_errors.present?
+      "settings--account--email-validation-failed-value": @validation_errors&.[](:email)&.present?
     } do %>
   <h3>E-Mail</h3>
   <%= form_for(@user, url: update_email_settings_path, method: "post") do |f| %>
@@ -23,7 +23,7 @@
         </span>
       </button>
     </p>
-    <% if @validation_errors.present? && @validation_errors[:email].present? %>
+    <% if @validation_errors&.[](:email)&.present? %>
       <p class="error-msg"><%= @validation_errors[:email].first %></p>
     <% end %>
     <div class="initial-hidden">
@@ -41,10 +41,33 @@
 <% end %>
 <section>
   <h3>Password</h3>
-  <p class="mb-8">Use the following button to request an email with a password reset link:</p>
+  <p class="mb-6">Use the following button to request an email with a password reset link:</p>
   <%= form_with(url: reset_password_settings_path, method: :post) do %>
     <p>
       <%= submit_tag("Send me a password reset link", class: 'btn-md btn-gray w-full sm:w-auto') %>
     </p>
   <% end %>
 </section>
+<%= form_for(@user, url: setting_path(:account), html: { :method => :put }) do |f| %>
+  <section class="!pt-8 sm:!pt-12">
+    <h3>OpenPGP</h3>
+    <ul role="list">
+      <%= render FormElements::FieldsetComponent.new(
+            title: "Public key",
+            description: "Your OpenPGP public key in ASCII Armor format ([example])"
+          ) do %>
+        <%= f.text_area :pgp_pubkey,
+          value: @user.pgp_pubkey,
+          class: "h-24 w-full" %>
+        <% if @validation_errors&.[](:pgp_pubkey)&.present? %>
+          <p class="error-msg">This <%= @validation_errors[:pgp_pubkey].first %></p>
+        <% end %>
+      <% end %>
+    </ul>
+  </section>
+  <section>
+    <p class="pt-6 border-t border-gray-200 text-right">
+      <%= f.submit 'Save', class: "btn-md btn-blue w-full md:w-auto" %>
+    </p>
+  </section>
+<% end %>

spec/features/settings/account_spec.rb

@@ -14,6 +14,7 @@ RSpec.describe 'Account settings', type: :feature do
         .with("invalid password").and_return(false)
       allow_any_instance_of(User).to receive(:valid_ldap_authentication?)
         .with("valid password").and_return(true)
+      allow_any_instance_of(User).to receive(:pgp_pubkey).and_return(nil)
     end
 
     scenario 'fails with invalid password' do
@@ -55,4 +56,44 @@ RSpec.describe 'Account settings', type: :feature do
       end
     end
   end
+
+  feature "Update OpenPGP key" do
+    let(:invalid_key) { File.read("#{Rails.root}/spec/fixtures/files/pgp_key_invalid.asc") }
+    let(:valid_key_alice) { File.read("#{Rails.root}/spec/fixtures/files/pgp_key_valid_alice.asc") }
+    let(:fingerprint_alice) { "EB85BB5FA33A75E15E944E63F231550C4F47E38E" }
+
+    before do
+      login_as user, :scope => :user
+      allow_any_instance_of(User).to receive(:ldap_entry).and_return({
+        uid: user.cn, ou: user.ou, display_name: nil, pgp_key: nil
+      })
+    end
+
+    scenario 'rejects an invalid key' do
+      expect(UserManager::UpdatePgpKey).not_to receive(:call)
+
+      visit setting_path(:account)
+      fill_in 'Public key', with: invalid_key
+      click_button "Save"
+
+      expect(current_url).to eq(setting_url(:account))
+      within ".error-msg" do
+        expect(page).to have_content("This is not a valid armored PGP public key block")
+      end
+    end
+
+    scenario 'stores a valid key' do
+      expect(UserManager::UpdatePgpKey).to receive(:call)
+        .with(user: user).and_return(true)
+
+      visit setting_path(:account)
+      fill_in 'Public key', with: valid_key_alice
+      click_button "Save"
+
+      expect(current_url).to eq(setting_url(:account))
+      within ".flash-msg" do
+        expect(page).to have_content("Settings saved")
+      end
+    end
+  end
 end

spec/features/settings/profile_spec.rb

@@ -9,7 +9,7 @@ RSpec.describe 'Profile settings', type: :feature do
     allow(user).to receive(:display_name).and_return("Mark")
     allow_any_instance_of(User).to receive(:dn).and_return("cn=mwahlberg,ou=kosmos.org,cn=users,dc=kosmos,dc=org")
     allow_any_instance_of(User).to receive(:ldap_entry).and_return({
-      uid: user.cn, ou: user.ou, display_name: "Mark"
+      uid: user.cn, ou: user.ou, display_name: "Mark", pgp_key: nil
     })
     allow_any_instance_of(User).to receive(:avatar).and_return(avatar_base64)
 

spec/models/user_spec.rb

@@ -262,7 +262,6 @@ RSpec.describe User, type: :model do
     let(:valid_key_jimmy) { File.read("#{Rails.root}/spec/fixtures/files/pgp_key_valid_jimmy.asc") }
     let(:fingerprint_alice) { "EB85BB5FA33A75E15E944E63F231550C4F47E38E" }
     let(:fingerprint_jimmy) { "316BF516236DAF77236B15F6057D93972FB862C3" }
-    let(:gnupg_key_alice) {  }
     let(:invalid_key) { File.read("#{Rails.root}/spec/fixtures/files/pgp_key_invalid.asc") }
 
     before do
@@ -275,8 +274,8 @@ RSpec.describe User, type: :model do
     end
 
     after do
-      GPGME::Key.get(fingerprint_alice).delete!
+      alice.gnupg_key.delete!
-      GPGME::Key.get(fingerprint_jimmy).delete!
+      jimmy.gnupg_key.delete!
     end
 
     describe "#acceptable_pgp_key_format" do

spec/services/user_manager/update_pgp_key_spec.rb

@@ -0,0 +1,74 @@
+require 'rails_helper'
+
+RSpec.describe UserManager::UpdatePgpKey, type: :model do
+  include ActiveJob::TestHelper
+
+  let(:alice) { create :user, cn: "alice" }
+  let(:dn)    { "cn=alice,ou=kosmos.org,cn=users,dc=kosmos,dc=org" }
+  let(:pubkey_asc) { File.read("#{Rails.root}/spec/fixtures/files/pgp_key_valid_alice.asc") }
+  let(:fingerprint) { "EB85BB5FA33A75E15E944E63F231550C4F47E38E" }
+
+  before do
+    allow(alice).to receive(:dn).and_return(dn)
+    allow(alice).to receive(:ldap_entry).and_return({
+      uid: alice.cn, ou: alice.ou, pgp_key: nil
+    })
+  end
+
+  describe "#call" do
+    context "with valid key" do
+      before do
+        alice.pgp_pubkey = pubkey_asc
+
+        allow(LdapManager::UpdatePgpKey).to receive(:call)
+          .with(dn: alice.dn, pubkey: pubkey_asc)
+      end
+
+      after do
+        alice.gnupg_key.delete!
+      end
+
+      it "imports the key into the GnuPG keychain" do
+        described_class.call(user: alice)
+        expect(alice.gnupg_key).to be_present
+      end
+
+      it "stores the key's fingerprint on the user record" do
+        described_class.call(user: alice)
+        expect(alice.pgp_fpr).to eq(fingerprint)
+      end
+
+      it "updates the user's LDAP entry with the new key" do
+        expect(LdapManager::UpdatePgpKey).to receive(:call)
+          .with(dn: alice.dn, pubkey: pubkey_asc)
+        described_class.call(user: alice)
+      end
+    end
+
+    context "with empty key" do
+      before do
+        alice.update pgp_fpr: fingerprint
+        alice.pgp_pubkey = ""
+
+        allow(LdapManager::UpdatePgpKey).to receive(:call)
+          .with(dn: alice.dn, pubkey: "")
+      end
+
+      it "does not attempt to import the key" do
+        expect(GPGME::Key).not_to receive(:import)
+        described_class.call(user: alice)
+      end
+
+      it "removes the key's fingerprint from the user record" do
+        described_class.call(user: alice)
+        expect(alice.pgp_fpr).to be_nil
+      end
+
+      it "removes the key from the user's LDAP entry" do
+        expect(LdapManager::UpdatePgpKey).to receive(:call)
+          .with(dn: alice.dn, pubkey: "")
+        described_class.call(user: alice)
+      end
+    end
+  end
+end