From 5a3adba60391ae16321941f0c73f28c33b12f59f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Sun, 17 Mar 2024 11:04:11 +0100 Subject: [PATCH] Move nostr pubkeys to LDAP attribute closes #173 --- app/controllers/settings_controller.rb | 17 ++- app/models/user.rb | 16 +-- .../ldap_manager/fetch_user_by_nostr_key.rb | 18 +++ app/services/ldap_manager/update_nostr_key.rb | 16 +++ app/services/ldap_manager_service.rb | 3 - app/services/ldap_service.rb | 28 ++--- ...16153558_remove_nostr_pubkey_from_users.rb | 5 + db/schema.rb | 3 +- spec/features/settings/nostr_spec.rb | 17 ++- spec/models/user_spec.rb | 14 ++- spec/requests/settings_spec.rb | 115 +++++++++++++----- spec/requests/well_known_spec.rb | 13 +- 12 files changed, 188 insertions(+), 77 deletions(-) create mode 100644 app/services/ldap_manager/fetch_user_by_nostr_key.rb create mode 100644 app/services/ldap_manager/update_nostr_key.rb create mode 100644 db/migrate/20240316153558_remove_nostr_pubkey_from_users.rb diff --git a/app/controllers/settings_controller.rb b/app/controllers/settings_controller.rb index 841d0f2..b736c7f 100644 --- a/app/controllers/settings_controller.rb +++ b/app/controllers/settings_controller.rb @@ -88,6 +88,7 @@ class SettingsController < ApplicationController def set_nostr_pubkey signed_event = nostr_event_params[:signed_event].to_h.symbolize_keys + is_valid_id = NostrManager::ValidateId.call(event: signed_event) is_valid_sig = NostrManager::VerifySignature.call(event: signed_event) is_correct_content = signed_event[:content] == "Connect my public key to #{current_user.address} (confirmation #{session[:shared_secret]})" @@ -97,28 +98,24 @@ class SettingsController < ApplicationController http_status :unprocessable_entity and return end - pubkey_taken = User.all_except(current_user).where( - ou: current_user.ou, nostr_pubkey: signed_event[:pubkey] - ).any? + user_with_pubkey = LdapManager::FetchUserByNostrKey.call(pubkey: signed_event[:pubkey]) - if pubkey_taken + if user_with_pubkey.present? && (user_with_pubkey != current_user) flash[:alert] = "Public key already in use for a different account" http_status :unprocessable_entity and return end - current_user.update! nostr_pubkey: signed_event[:pubkey] + LdapManager::UpdateNostrKey.call(dn: current_user.dn, pubkey: signed_event[:pubkey]) session[:shared_secret] = nil flash[:success] = "Public key verification successful" http_status :ok - rescue - flash[:alert] = "Public key could not be verified" - http_status :unprocessable_entity and return end # DELETE /settings/nostr_pubkey def remove_nostr_pubkey - current_user.update! nostr_pubkey: nil + # TODO require current pubkey or password to delete + LdapManager::UpdateNostrKey.call(dn: current_user.dn, pubkey: nil) redirect_to setting_path(:nostr), flash: { success: 'Public key removed from account' @@ -165,7 +162,7 @@ class SettingsController < ApplicationController def nostr_event_params params.permit(signed_event: [ - :id, :pubkey, :created_at, :kind, :tags, :content, :sig + :id, :pubkey, :created_at, :kind, :content, :sig, tags: [] ]) end diff --git a/app/models/user.rb b/app/models/user.rb index fb8d95a..f9bd0ae 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -50,8 +50,6 @@ class User < ApplicationRecord validates_length_of :display_name, minimum: 3, maximum: 35, allow_blank: true, if: -> { defined?(@display_name) } - validates_uniqueness_of :nostr_pubkey, allow_blank: true - validate :acceptable_avatar # @@ -163,6 +161,15 @@ class User < ApplicationRecord @display_name ||= ldap_entry[:display_name] end + def nostr_pubkey + @nostr_pubkey ||= ldap_entry[:nostr_key] + end + + def nostr_pubkey_bech32 + return nil unless nostr_pubkey.present? + Nostr::PublicKey.new(nostr_pubkey).to_bech32 + end + def avatar @avatar_base64 ||= LdapManager::FetchAvatar.call(cn: cn) end @@ -189,11 +196,6 @@ class User < ApplicationRecord ldap.delete_attribute(dn,:service) end - def nostr_pubkey_bech32 - return nil unless nostr_pubkey.present? - Nostr::PublicKey.new(nostr_pubkey).to_bech32 - end - private def ldap diff --git a/app/services/ldap_manager/fetch_user_by_nostr_key.rb b/app/services/ldap_manager/fetch_user_by_nostr_key.rb new file mode 100644 index 0000000..a808a1c --- /dev/null +++ b/app/services/ldap_manager/fetch_user_by_nostr_key.rb @@ -0,0 +1,18 @@ +module LdapManager + class FetchUserByNostrKey < LdapManagerService + def initialize(pubkey:) + @ou = Setting.primary_domain + @pubkey = pubkey + end + + def call + treebase = "ou=#{@ou},cn=users,#{ldap_suffix}" + attributes = %w{ cn } + filter = Net::LDAP::Filter.eq("nostrKey", @pubkey) + + entry = client.search(base: treebase, filter: filter, attributes: attributes).first + + User.find_by cn: entry.cn, ou: @ou unless entry.nil? + end + end +end diff --git a/app/services/ldap_manager/update_nostr_key.rb b/app/services/ldap_manager/update_nostr_key.rb new file mode 100644 index 0000000..0e0b8a9 --- /dev/null +++ b/app/services/ldap_manager/update_nostr_key.rb @@ -0,0 +1,16 @@ +module LdapManager + class UpdateNostrKey < LdapManagerService + def initialize(dn:, pubkey:) + @dn = dn + @pubkey = pubkey + end + + def call + if @pubkey.present? + replace_attribute @dn, :nostrKey, @pubkey + else + delete_attribute @dn, :nostrKey + end + end + end +end diff --git a/app/services/ldap_manager_service.rb b/app/services/ldap_manager_service.rb index c7a2599..0f43e32 100644 --- a/app/services/ldap_manager_service.rb +++ b/app/services/ldap_manager_service.rb @@ -1,5 +1,2 @@ class LdapManagerService < LdapService - def suffix - @suffix ||= ENV["LDAP_SUFFIX"] || "dc=kosmos,dc=org" - end end diff --git a/app/services/ldap_service.rb b/app/services/ldap_service.rb index 371f9a9..8e78616 100644 --- a/app/services/ldap_service.rb +++ b/app/services/ldap_service.rb @@ -1,8 +1,4 @@ class LdapService < ApplicationService - def initialize - @suffix = ENV["LDAP_SUFFIX"] || "dc=kosmos,dc=org" - end - def modify(dn, operations=[]) client.modify dn: dn, operations: operations client.get_operation_result.code @@ -45,7 +41,7 @@ class LdapService < ApplicationService end filter = Net::LDAP::Filter.eq("objectClass", objectclass) - entries = client.search(base: @suffix, filter: filter, attributes: %w{dn}) + entries = client.search(base: ldap_suffix, filter: filter, attributes: %w{dn}) entries.sort_by!{ |e| e.dn.length }.reverse! entries.each do |e| @@ -55,14 +51,14 @@ class LdapService < ApplicationService def fetch_users(args={}) if args[:ou] - treebase = "ou=#{args[:ou]},cn=users,#{@suffix}" + treebase = "ou=#{args[:ou]},cn=users,#{ldap_suffix}" else treebase = ldap_config["base"] end attributes = %w[ dn cn uid mail displayName admin service - mailRoutingAddress mailpassword + mailRoutingAddress mailpassword nostrKey ] filter = Net::LDAP::Filter.eq("uid", args[:uid] || "*") @@ -77,7 +73,7 @@ class LdapService < ApplicationService services_enabled: e.try(:serviceEnabled), email_maildrop: e.try(:mailRoutingAddress), email_password: e.try(:mailpassword), - nostr_key: e.try(:nostrKey) + nostr_key: e.try(:nostrKey) ? e.nostrKey.first : nil } end end @@ -86,7 +82,7 @@ class LdapService < ApplicationService attributes = %w{dn ou description} filter = Net::LDAP::Filter.eq("objectClass", "organizationalUnit") # filter = Net::LDAP::Filter.eq("objectClass", "*") - treebase = "cn=users,#{@suffix}" + treebase = "cn=users,#{ldap_suffix}" entries = client.search(base: treebase, filter: filter, attributes: attributes) @@ -102,10 +98,10 @@ class LdapService < ApplicationService end def add_organization(ou, description, interactive=false) - dn = "ou=#{ou},cn=users,#{@suffix}" + dn = "ou=#{ou},cn=users,#{ldap_suffix}" aci = <<-EOS -(target="ldap:///cn=*,ou=#{ou},cn=users,#{@suffix}")(targetattr="cn || sn || uid || mail || userPassword || nsRole || objectClass") (version 3.0; acl "service-#{ou.gsub(".", "-")}-read-search"; allow (read,search) userdn="ldap:///uid=service,ou=#{ou},cn=applications,#{@suffix}";) +(target="ldap:///cn=*,ou=#{ou},cn=users,#{ldap_suffix}")(targetattr="cn || sn || uid || mail || userPassword || nsRole || objectClass") (version 3.0; acl "service-#{ou.gsub(".", "-")}-read-search"; allow (read,search) userdn="ldap:///uid=service,ou=#{ou},cn=applications,#{ldap_suffix}";) EOS attrs = { @@ -126,14 +122,14 @@ class LdapService < ApplicationService delete_all_entries! user_read_aci = <<-EOS -(target="ldap:///#{@suffix}")(targetattr="*") (version 3.0; acl "user-read-search-own-attributes"; allow (read,search) userdn="ldap:///self";) +(target="ldap:///#{ldap_suffix}")(targetattr="*") (version 3.0; acl "user-read-search-own-attributes"; allow (read,search) userdn="ldap:///self";) EOS - add_entry @suffix, { + add_entry ldap_suffix, { dc: "kosmos", objectClass: ["top", "domain"], aci: user_read_aci }, true - add_entry "cn=users,#{@suffix}", { + add_entry "cn=users,#{ldap_suffix}", { cn: "users", objectClass: ["top", "organizationalRole"] }, true end @@ -155,4 +151,8 @@ class LdapService < ApplicationService def ldap_config ldap_config ||= YAML.load(ERB.new(File.read("#{Rails.root}/config/ldap.yml")).result)[Rails.env] end + + def ldap_suffix + @ldap_suffix ||= ENV["LDAP_SUFFIX"] || "dc=kosmos,dc=org" + end end diff --git a/db/migrate/20240316153558_remove_nostr_pubkey_from_users.rb b/db/migrate/20240316153558_remove_nostr_pubkey_from_users.rb new file mode 100644 index 0000000..bed06f3 --- /dev/null +++ b/db/migrate/20240316153558_remove_nostr_pubkey_from_users.rb @@ -0,0 +1,5 @@ +class RemoveNostrPubkeyFromUsers < ActiveRecord::Migration[7.1] + def change + remove_column :users, :nostr_pubkey, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index 934c5f3..0810c7c 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_02_16_124640) do +ActiveRecord::Schema[7.1].define(version: 2024_03_16_153558) do create_table "active_storage_attachments", force: :cascade do |t| t.string "name", null: false t.string "record_type", null: false @@ -129,7 +129,6 @@ ActiveRecord::Schema[7.1].define(version: 2024_02_16_124640) do t.string "unconfirmed_email" t.text "ln_password_ciphertext" t.string "ln_account" - t.string "nostr_pubkey" t.datetime "remember_created_at" t.string "remember_token" t.text "preferences" diff --git a/spec/features/settings/nostr_spec.rb b/spec/features/settings/nostr_spec.rb index c2178fc..3de7e12 100644 --- a/spec/features/settings/nostr_spec.rb +++ b/spec/features/settings/nostr_spec.rb @@ -5,6 +5,10 @@ RSpec.describe 'Nostr Settings', type: :feature do before do login_as user, scope: :user + + allow_any_instance_of(User).to receive(:dn) + .and_return("cn=#{user.cn},ou=kosmos.org,cn=users,dc=kosmos,dc=org") + allow_any_instance_of(User).to receive(:nostr_pubkey).and_return(nil) end describe 'Adding a nostr pubkey' do @@ -22,19 +26,22 @@ RSpec.describe 'Nostr Settings', type: :feature do context "With pubkey configured" do before do - user.update! nostr_pubkey: "07e188a1ff87ce171d517b8ed2bb7a31b1d3453a0db3b15379ec07b724d232f3" + allow_any_instance_of(User).to receive(:nostr_pubkey) + .and_return("ce273cbfb0d4e3e06930773a337c1459e4849efd4cb4c751b906a561c98a6d09") end scenario 'Remove nostr pubkey from account' do visit setting_path(:nostr) expect(page).to have_field("nostr_public_key", - with: "npub1qlsc3g0lsl8pw8230w8d9wm6xxcax3f6pkemz5measrmwfxjxteslf2hac", + with: "npub1ecnne0as6n37q6fswuarxlq5t8jgf8hafj6vw5deq6jkrjv2d5ysnehu73", disabled: true) + expect(LdapManager::UpdateNostrKey).to receive(:call).with( + dn: "cn=jimmy,ou=kosmos.org,cn=users,dc=kosmos,dc=org", + pubkey: nil + ) + click_link "Remove" - expect(page).to_not have_field("nostr_public_key") - expect(page).to have_content("verify your public key") - expect(user.reload.nostr_pubkey).to be_nil end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 3758d3b..249ac61 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -206,9 +206,21 @@ RSpec.describe User, type: :model do end end + describe "#nostr_pubkey" do + before do + allow_any_instance_of(User).to receive(:ldap_entry) + .and_return({ nostr_key: "07e188a1ff87ce171d517b8ed2bb7a31b1d3453a0db3b15379ec07b724d232f3" }) + end + + it "returns the raw pubkey from LDAP" do + expect(user.nostr_pubkey).to eq("07e188a1ff87ce171d517b8ed2bb7a31b1d3453a0db3b15379ec07b724d232f3") + end + end + describe "#nostr_pubkey_bech32" do before do - user.update! nostr_pubkey: "07e188a1ff87ce171d517b8ed2bb7a31b1d3453a0db3b15379ec07b724d232f3" + allow_any_instance_of(User).to receive(:ldap_entry) + .and_return({ nostr_key: "07e188a1ff87ce171d517b8ed2bb7a31b1d3453a0db3b15379ec07b724d232f3" }) end it "encodes the hexadecimal pubkey to bech32" do diff --git a/spec/requests/settings_spec.rb b/spec/requests/settings_spec.rb index 5d2ba17..76c1bca 100644 --- a/spec/requests/settings_spec.rb +++ b/spec/requests/settings_spec.rb @@ -2,9 +2,18 @@ require 'rails_helper' RSpec.describe "Settings", type: :request do let(:user) { create :user, cn: 'mark', ou: 'kosmos.org' } + let(:other_user) { create :user, id: 2, cn: 'markymark', ou: 'kosmos.org', email: 'markymark@interscope.com' } before do login_as user, :scope => :user + + allow_any_instance_of(User).to receive(:dn) + .and_return("cn=#{user.cn},ou=kosmos.org,cn=users,dc=kosmos,dc=org") + allow_any_instance_of(User).to receive(:nostr_pubkey).and_return(nil) + + allow(LdapManager::FetchUserByNostrKey).to receive(:call).with( + pubkey: "07e188a1ff87ce171d517b8ed2bb7a31b1d3453a0db3b15379ec07b724d232f3" + ).and_return(nil) end describe "GET /settings/nostr" do @@ -22,6 +31,11 @@ RSpec.describe "Settings", type: :request do context "With valid data" do before do + expect(LdapManager::UpdateNostrKey).to receive(:call).with( + dn: "cn=mark,ou=kosmos.org,cn=users,dc=kosmos,dc=org", + pubkey: "07e188a1ff87ce171d517b8ed2bb7a31b1d3453a0db3b15379ec07b724d232f3" + ).and_return(0) + post set_nostr_pubkey_settings_path, params: { signed_event: { id: "84f266bbd784551aaa9e35cb0aceb4ee59182a1dab9ab279d9e40dd56ecbbdd3", @@ -41,41 +55,17 @@ RSpec.describe "Settings", type: :request do expect(response).to have_http_status(200) end - it "saves the pubkey" do - expect(user.nostr_pubkey).to eq("07e188a1ff87ce171d517b8ed2bb7a31b1d3453a0db3b15379ec07b724d232f3") + it "informs the user about the success" do + expect(flash[:success]).to eq("Public key verification successful") end end - context "With wrong username" do + context "With key already in use by someone else" do before do - post set_nostr_pubkey_settings_path, params: { - signed_event: { - id: "2e1e20ee762d6a5b5b30835eda9ca03146e4baf82490e53fd75794c08de08ac0", - pubkey: "07e188a1ff87ce171d517b8ed2bb7a31b1d3453a0db3b15379ec07b724d232f3", - created_at: 1678255391, - kind: 1, - content: "Connect my public key to admin@kosmos.org (confirmation rMjWEmvcvtTlQkMd)", - sig: "2ace19c9db892ac6383848721a3e08b13d90d689fdeac60d9633a623d3f08eb7e0d468f1b3e928d1ea979477c2ec46ee6cdb2d053ef2e4ed3c0630a51d249029" - } - }.to_json, headers: { - "CONTENT_TYPE" => "application/json", - "HTTP_ACCEPT" => "application/json" - } - end - - it "returns a 422 status" do - expect(response).to have_http_status(422) - end - - it "does not save the pubkey" do - expect(user.nostr_pubkey).to be_nil - end - end - - context "With wrong shared secret" do - before do - session_stub = { shared_secret: "ho-chi-minh" } - allow_any_instance_of(SettingsController).to receive(:session).and_return(session_stub) + expect(LdapManager::FetchUserByNostrKey).to receive(:call).with( + pubkey: "07e188a1ff87ce171d517b8ed2bb7a31b1d3453a0db3b15379ec07b724d232f3" + ).and_return(other_user) + expect(LdapManager::UpdateNostrKey).not_to receive(:call) post set_nostr_pubkey_settings_path, params: { signed_event: { @@ -96,8 +86,67 @@ RSpec.describe "Settings", type: :request do expect(response).to have_http_status(422) end - it "does not save the pubkey" do - expect(user.nostr_pubkey).to be_nil + it "informs the user about the failure" do + expect(flash[:alert]).to eq("Public key already in use for a different account") + end + end + + context "With wrong username" do + before do + expect(LdapManager::UpdateNostrKey).not_to receive(:call) + + post set_nostr_pubkey_settings_path, params: { + signed_event: { + id: "2e1e20ee762d6a5b5b30835eda9ca03146e4baf82490e53fd75794c08de08ac0", + pubkey: "07e188a1ff87ce171d517b8ed2bb7a31b1d3453a0db3b15379ec07b724d232f3", + created_at: 1678255391, + kind: 1, + content: "Connect my public key to admin@kosmos.org (confirmation rMjWEmvcvtTlQkMd)", + sig: "2ace19c9db892ac6383848721a3e08b13d90d689fdeac60d9633a623d3f08eb7e0d468f1b3e928d1ea979477c2ec46ee6cdb2d053ef2e4ed3c0630a51d249029" + } + }.to_json, headers: { + "CONTENT_TYPE" => "application/json", + "HTTP_ACCEPT" => "application/json" + } + end + + it "returns a 422 status" do + expect(response).to have_http_status(422) + end + + it "informs the user about the failure" do + expect(flash[:alert]).to eq("Public key could not be verified") + end + end + + context "With wrong shared secret" do + before do + session_stub = { shared_secret: "ho-chi-minh" } + allow_any_instance_of(SettingsController).to receive(:session).and_return(session_stub) + + expect(LdapManager::UpdateNostrKey).not_to receive(:call) + + post set_nostr_pubkey_settings_path, params: { + signed_event: { + id: "84f266bbd784551aaa9e35cb0aceb4ee59182a1dab9ab279d9e40dd56ecbbdd3", + pubkey: "07e188a1ff87ce171d517b8ed2bb7a31b1d3453a0db3b15379ec07b724d232f3", + created_at: 1678254161, + kind: 1, + content: "Connect my public key to mark@kosmos.org (confirmation rMjWEmvcvtTlQkMd)", + sig: "96796d420547d6e2c7be5de82a2ce7a48be99aac6415464a6081859ac1a9017305accc0228c630466a57d45ec1c3b456376eb538b76dfdaa2397e3258be02fdd" + } + }.to_json, headers: { + "CONTENT_TYPE" => "application/json", + "HTTP_ACCEPT" => "application/json" + } + end + + it "returns a 422 status" do + expect(response).to have_http_status(422) + end + + it "informs the user about the failure" do + expect(flash[:alert]).to eq("Public key could not be verified") end end end diff --git a/spec/requests/well_known_spec.rb b/spec/requests/well_known_spec.rb index 1a24a12..9862b79 100644 --- a/spec/requests/well_known_spec.rb +++ b/spec/requests/well_known_spec.rb @@ -19,6 +19,11 @@ RSpec.describe "Well-known URLs", type: :request do context "user does not have a nostr pubkey configured" do let(:user) { create :user, cn: 'spongebob', ou: 'kosmos.org' } + before do + allow_any_instance_of(User).to receive(:ldap_entry) + .and_return({ nostr_key: nil }) + end + it "returns a 404 status" do get "/.well-known/nostr.json?name=spongebob" expect(response).to have_http_status(:not_found) @@ -26,8 +31,12 @@ RSpec.describe "Well-known URLs", type: :request do end context "user with nostr pubkey" do - let(:user) { create :user, cn: 'bobdylan', ou: 'kosmos.org', nostr_pubkey: '438d35a6750d0dd6b75d032af8a768aad76b62f0c70ecb45f9c4d9e63540f7f4' } - before { user.save! } + let(:user) { create :user, cn: 'bobdylan', ou: 'kosmos.org' } + before do + user.save! + allow_any_instance_of(User).to receive(:nostr_pubkey) + .and_return('438d35a6750d0dd6b75d032af8a768aad76b62f0c70ecb45f9c4d9e63540f7f4') + end it "returns a NIP-05 response" do get "/.well-known/nostr.json?name=bobdylan"