From 22d362e1a0cbf96fcb2e7f9bc104ef73aeadc608 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Mon, 1 Apr 2024 18:22:32 +0300 Subject: [PATCH] Refactor Nostr settings/connect * Use NIP-42 auth event instead of short text note * Verify event ID and signature using the nostr gem instead of custom code --- .env.test | 1 + app/controllers/application_controller.rb | 5 ++ app/controllers/settings_controller.rb | 22 ++++---- .../settings/nostr_pubkey_controller.js | 17 ++++-- app/services/nostr_manager/validate_id.rb | 11 ---- app/services/nostr_manager/verify_auth.rb | 17 ++++++ .../nostr_manager/verify_signature.rb | 17 ------ app/views/settings/_nostr.html.erb | 1 + spec/requests/settings_spec.rb | 53 +++++++++++-------- 9 files changed, 77 insertions(+), 67 deletions(-) delete mode 100644 app/services/nostr_manager/validate_id.rb create mode 100644 app/services/nostr_manager/verify_auth.rb delete mode 100644 app/services/nostr_manager/verify_signature.rb diff --git a/.env.test b/.env.test index aadec95..fae879b 100644 --- a/.env.test +++ b/.env.test @@ -1,4 +1,5 @@ PRIMARY_DOMAIN=kosmos.org +AKKOUNTS_DOMAIN=accounts.kosmos.org REDIS_URL='redis://localhost:6379/0' diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index a0da71a..4eb4820 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -63,4 +63,9 @@ class ApplicationController < ActionController::Base @fetch_balance_retried = true lndhub_fetch_balance end + + def nostr_event_from_params + params.permit! + params[:signed_event].to_h.symbolize_keys + end end diff --git a/app/controllers/settings_controller.rb b/app/controllers/settings_controller.rb index b736c7f..cb7de77 100644 --- a/app/controllers/settings_controller.rb +++ b/app/controllers/settings_controller.rb @@ -87,25 +87,27 @@ class SettingsController < ApplicationController end def set_nostr_pubkey - signed_event = nostr_event_params[:signed_event].to_h.symbolize_keys + signed_event = Nostr::Event.new(**nostr_event_from_params) - 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]})" + is_valid_sig = signed_event.verify_signature + is_valid_auth = NostrManager::VerifyAuth.call( + event: signed_event, + challenge: session[:shared_secret] + ) - unless is_valid_id && is_valid_sig && is_correct_content + unless is_valid_sig && is_valid_auth flash[:alert] = "Public key could not be verified" http_status :unprocessable_entity and return end - user_with_pubkey = LdapManager::FetchUserByNostrKey.call(pubkey: signed_event[:pubkey]) + user_with_pubkey = LdapManager::FetchUserByNostrKey.call(pubkey: signed_event.pubkey) 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 - LdapManager::UpdateNostrKey.call(dn: current_user.dn, 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" @@ -160,12 +162,6 @@ class SettingsController < ApplicationController params.require(:user).permit(:current_password) end - def nostr_event_params - params.permit(signed_event: [ - :id, :pubkey, :created_at, :kind, :content, :sig, tags: [] - ]) - end - def generate_email_password characters = [('a'..'z'), ('A'..'Z'), (0..9)].map(&:to_a).flatten SecureRandom.random_bytes(16).each_byte.map { |b| characters[b % characters.length] }.join diff --git a/app/javascript/controllers/settings/nostr_pubkey_controller.js b/app/javascript/controllers/settings/nostr_pubkey_controller.js index 9a3875c..4de30e5 100644 --- a/app/javascript/controllers/settings/nostr_pubkey_controller.js +++ b/app/javascript/controllers/settings/nostr_pubkey_controller.js @@ -3,7 +3,12 @@ import { Controller } from "@hotwired/stimulus" // Connects to data-controller="settings--nostr-pubkey" export default class extends Controller { static targets = [ "noExtension", "setPubkey", "pubkeyBech32Input" ] - static values = { userAddress: String, pubkeyHex: String, sharedSecret: String } + static values = { + userAddress: String, + pubkeyHex: String, + site: String, + sharedSecret: String + } connect () { if (window.nostr) { @@ -19,11 +24,15 @@ export default class extends Controller { this.setPubkeyTarget.disabled = true try { + // Auth based on NIP-42 const signedEvent = await window.nostr.signEvent({ created_at: Math.floor(Date.now() / 1000), - kind: 1, - tags: [], - content: `Connect my public key to ${this.userAddressValue} (confirmation ${this.sharedSecretValue})` + kind: 22242, + tags: [ + ["site", this.siteValue], + ["challenge", this.sharedSecretValue] + ], + content: "" }) const res = await fetch("/settings/set_nostr_pubkey", { diff --git a/app/services/nostr_manager/validate_id.rb b/app/services/nostr_manager/validate_id.rb deleted file mode 100644 index e838441..0000000 --- a/app/services/nostr_manager/validate_id.rb +++ /dev/null @@ -1,11 +0,0 @@ -module NostrManager - class ValidateId < NostrManagerService - def initialize(event:) - @event = Nostr::Event.new(**event) - end - - def call - @event.id == Digest::SHA256.hexdigest(JSON.generate(@event.serialize)) - end - end -end diff --git a/app/services/nostr_manager/verify_auth.rb b/app/services/nostr_manager/verify_auth.rb new file mode 100644 index 0000000..fdcef2d --- /dev/null +++ b/app/services/nostr_manager/verify_auth.rb @@ -0,0 +1,17 @@ +module NostrManager + class VerifyAuth < NostrManagerService + def initialize(event:, challenge:) + @event = event + @challenge_expected = challenge + @site_expected = Setting.accounts_domain + end + + def call + site_given = @event.tags.find{|t| t[0] == "site"}[1] + challenge_given = @event.tags.find{|t| t[0] == "challenge"}[1] + + site_given == @site_expected && + challenge_given == @challenge_expected + end + end +end diff --git a/app/services/nostr_manager/verify_signature.rb b/app/services/nostr_manager/verify_signature.rb deleted file mode 100644 index 9c62234..0000000 --- a/app/services/nostr_manager/verify_signature.rb +++ /dev/null @@ -1,17 +0,0 @@ -module NostrManager - class VerifySignature < NostrManagerService - def initialize(event:) - @event = Nostr::Event.new(**event) - end - - def call - Schnorr.check_sig!( - [@event.id].pack('H*'), - [@event.pubkey].pack('H*'), - [@event.sig].pack('H*') - ) - rescue Schnorr::InvalidSignatureError - false - end - end -end diff --git a/app/views/settings/_nostr.html.erb b/app/views/settings/_nostr.html.erb index 3c1a5b2..1251dd5 100644 --- a/app/views/settings/_nostr.html.erb +++ b/app/views/settings/_nostr.html.erb @@ -3,6 +3,7 @@

Public Key

diff --git a/spec/requests/settings_spec.rb b/spec/requests/settings_spec.rb index 76c1bca..f2e8302 100644 --- a/spec/requests/settings_spec.rb +++ b/spec/requests/settings_spec.rb @@ -25,7 +25,7 @@ RSpec.describe "Settings", type: :request do describe "POST /settings/set_nostr_pubkey" do before do - session_stub = { shared_secret: "rMjWEmvcvtTlQkMd" } + session_stub = { shared_secret: "YMeTyOxIEJcfe6vd" } allow_any_instance_of(SettingsController).to receive(:session).and_return(session_stub) end @@ -38,12 +38,13 @@ RSpec.describe "Settings", type: :request do post set_nostr_pubkey_settings_path, params: { signed_event: { - id: "84f266bbd784551aaa9e35cb0aceb4ee59182a1dab9ab279d9e40dd56ecbbdd3", + id: "7cc165c4fe4c9ec3f2b859cb422f01b38beaf6bbd228fea928ea1400ec254a89", pubkey: "07e188a1ff87ce171d517b8ed2bb7a31b1d3453a0db3b15379ec07b724d232f3", - created_at: 1678254161, - kind: 1, - content: "Connect my public key to mark@kosmos.org (confirmation rMjWEmvcvtTlQkMd)", - sig: "96796d420547d6e2c7be5de82a2ce7a48be99aac6415464a6081859ac1a9017305accc0228c630466a57d45ec1c3b456376eb538b76dfdaa2397e3258be02fdd" + created_at: 1711963922, + kind: 22242, + tags: [["site","accounts.kosmos.org"],["challenge","YMeTyOxIEJcfe6vd"]], + content: "", + sig: "b484a28cd9c92facca0eba80e8ef5303d25ed044c3815e3a068b9887f91d3546ad209a0dd674c59b48cf8057aecd75df5416973d17ed58f68195030af09c28d1" } }.to_json, headers: { "CONTENT_TYPE" => "application/json", @@ -69,12 +70,13 @@ RSpec.describe "Settings", type: :request do post set_nostr_pubkey_settings_path, params: { signed_event: { - id: "84f266bbd784551aaa9e35cb0aceb4ee59182a1dab9ab279d9e40dd56ecbbdd3", + id: "7cc165c4fe4c9ec3f2b859cb422f01b38beaf6bbd228fea928ea1400ec254a89", pubkey: "07e188a1ff87ce171d517b8ed2bb7a31b1d3453a0db3b15379ec07b724d232f3", - created_at: 1678254161, - kind: 1, - content: "Connect my public key to mark@kosmos.org (confirmation rMjWEmvcvtTlQkMd)", - sig: "96796d420547d6e2c7be5de82a2ce7a48be99aac6415464a6081859ac1a9017305accc0228c630466a57d45ec1c3b456376eb538b76dfdaa2397e3258be02fdd" + created_at: 1711963922, + kind: 22242, + tags: [["site","accounts.kosmos.org"],["challenge","YMeTyOxIEJcfe6vd"]], + content: "", + sig: "b484a28cd9c92facca0eba80e8ef5303d25ed044c3815e3a068b9887f91d3546ad209a0dd674c59b48cf8057aecd75df5416973d17ed58f68195030af09c28d1" } }.to_json, headers: { "CONTENT_TYPE" => "application/json", @@ -91,18 +93,20 @@ RSpec.describe "Settings", type: :request do end end - context "With wrong username" do + context "With wrong site tag" do before do + Setting.accounts_domain = "accounts.wikipedia.org" expect(LdapManager::UpdateNostrKey).not_to receive(:call) post set_nostr_pubkey_settings_path, params: { signed_event: { - id: "2e1e20ee762d6a5b5b30835eda9ca03146e4baf82490e53fd75794c08de08ac0", + id: "7cc165c4fe4c9ec3f2b859cb422f01b38beaf6bbd228fea928ea1400ec254a89", pubkey: "07e188a1ff87ce171d517b8ed2bb7a31b1d3453a0db3b15379ec07b724d232f3", - created_at: 1678255391, - kind: 1, - content: "Connect my public key to admin@kosmos.org (confirmation rMjWEmvcvtTlQkMd)", - sig: "2ace19c9db892ac6383848721a3e08b13d90d689fdeac60d9633a623d3f08eb7e0d468f1b3e928d1ea979477c2ec46ee6cdb2d053ef2e4ed3c0630a51d249029" + created_at: 1711963922, + kind: 22242, + tags: [["site","accounts.kosmos.org"],["challenge","YMeTyOxIEJcfe6vd"]], + content: "", + sig: "b484a28cd9c92facca0eba80e8ef5303d25ed044c3815e3a068b9887f91d3546ad209a0dd674c59b48cf8057aecd75df5416973d17ed58f68195030af09c28d1" } }.to_json, headers: { "CONTENT_TYPE" => "application/json", @@ -110,6 +114,10 @@ RSpec.describe "Settings", type: :request do } end + after do + Setting.accounts_domain = "accounts.kosmos.org" + end + it "returns a 422 status" do expect(response).to have_http_status(422) end @@ -128,12 +136,13 @@ RSpec.describe "Settings", type: :request do post set_nostr_pubkey_settings_path, params: { signed_event: { - id: "84f266bbd784551aaa9e35cb0aceb4ee59182a1dab9ab279d9e40dd56ecbbdd3", + id: "7cc165c4fe4c9ec3f2b859cb422f01b38beaf6bbd228fea928ea1400ec254a89", pubkey: "07e188a1ff87ce171d517b8ed2bb7a31b1d3453a0db3b15379ec07b724d232f3", - created_at: 1678254161, - kind: 1, - content: "Connect my public key to mark@kosmos.org (confirmation rMjWEmvcvtTlQkMd)", - sig: "96796d420547d6e2c7be5de82a2ce7a48be99aac6415464a6081859ac1a9017305accc0228c630466a57d45ec1c3b456376eb538b76dfdaa2397e3258be02fdd" + created_at: 1711963922, + kind: 22242, + tags: [["site","accounts.kosmos.org"],["challenge","YMeTyOxIEJcfe6vd"]], + content: "", + sig: "b484a28cd9c92facca0eba80e8ef5303d25ed044c3815e3a068b9887f91d3546ad209a0dd674c59b48cf8057aecd75df5416973d17ed58f68195030af09c28d1" } }.to_json, headers: { "CONTENT_TYPE" => "application/json",