From bcb4b2b9647d4d11111bd04d7448d7718e5f6a08 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 3 Jun 2026 17:45:52 +0000 Subject: [PATCH] fix(quartz): roll back authentication when the post-auth hook rejects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audit finding: FullAuthPolicy.accept(AuthCmd) added the pubkey to the authenticated set before onAuthenticated ran, so a bridge that threw from onAuthenticated (its whole point — reject when e.g. a JWT exchange fails) produced an OK false while the connection stayed authenticated server-side. Subsequent REQ/EVENT/COUNT were then allowed despite the failed login — an auth bypass. - Add IRelayPolicy.onAuthenticationFailed(pubKey) (default no-op), forwarded by PolicyStack and overridden by FullAuthPolicy to drop the pubkey. - RelaySession.handleAuth calls it when onAuthenticated throws, restoring the invariant that a client treated as authenticated is exactly one that got OK true. The rollback is itself guarded so a misbehaving policy can't also swallow the failing OK. - Tests: failed-hook now asserts the connection is NOT authenticated and that a follow-up REQ is rejected with auth-required. - RELAY.md: note that throwing from onAuthenticated rolls auth back. https://claude.ai/code/session_016YBS2pWCBSDgAMthHzfCTr --- quartz/RELAY.md | 2 +- .../nip01Core/relay/server/IRelayPolicy.kt | 11 ++++++ .../nip01Core/relay/server/RelaySession.kt | 8 +++++ .../relay/server/policies/FullAuthPolicy.kt | 4 +++ .../relay/server/policies/PolicyStack.kt | 4 +++ .../relay/server/NostrServerAuthTest.kt | 36 +++++++++++++++++-- 6 files changed, 61 insertions(+), 4 deletions(-) diff --git a/quartz/RELAY.md b/quartz/RELAY.md index df6f9b17c4..e9be348a05 100644 --- a/quartz/RELAY.md +++ b/quartz/RELAY.md @@ -169,7 +169,7 @@ val server = NostrServer( ) ``` -`FullAuthPolicy` already implements the full NIP-42 challenge/verify handshake — you should not re-implement it. To bridge auth to an external system (e.g. exchange the verified event for a backend JWT), override the `suspend` `onAuthenticated` hook. It runs after the NIP-42 checks pass but before the success `OK` is sent, so it can do network/disk I/O; throwing from it turns the AUTH into a failing `OK false`: +`FullAuthPolicy` already implements the full NIP-42 challenge/verify handshake — you should not re-implement it. To bridge auth to an external system (e.g. exchange the verified event for a backend JWT), override the `suspend` `onAuthenticated` hook. It runs after the NIP-42 checks pass but before the success `OK` is sent, so it can do network/disk I/O; throwing from it turns the AUTH into a failing `OK false` **and rolls the authentication back** (the connection is not left logged in behind a false `OK`): ```kotlin class JwtAuthPolicy( diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/IRelayPolicy.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/IRelayPolicy.kt index af4ee38eb6..f698577b6f 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/IRelayPolicy.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/IRelayPolicy.kt @@ -91,6 +91,17 @@ interface IRelayPolicy { event: RelayAuthEvent, ) {} + /** + * Rolls back an authentication that [accept] granted but [onAuthenticated] + * then rejected by throwing. The engine calls this so the connection does + * not stay authenticated after a failing `OK` — preserving the invariant + * that a client treated as logged in is exactly one that received `OK true`. + * + * The default implementation does nothing; [com.vitorpamplona.quartz.nip01Core.relay.server.policies.FullAuthPolicy] + * overrides it to drop the pubkey from its authenticated set. + */ + fun onAuthenticationFailed(pubKey: HexKey) {} + /** * Filters a live event before it is forwarded to a subscriber. * diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/RelaySession.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/RelaySession.kt index 394cfe806d..e7cce64b4b 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/RelaySession.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/RelaySession.kt @@ -201,6 +201,14 @@ class RelaySession( } catch (e: CancellationException) { throw e } catch (e: Exception) { + // The hook rejected the login: undo whatever accept() committed + // so the connection isn't left authenticated behind a false OK. + // Guard the rollback so a misbehaving policy can't also swallow + // the failing OK and leave the client without a reply. + try { + policy.onAuthenticationFailed(cmd.event.pubKey) + } catch (_: Exception) { + } send(OkMessage(cmd.event.id, false, "error: ${e.message ?: "authentication failed"}")) return } diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/policies/FullAuthPolicy.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/policies/FullAuthPolicy.kt index e1fc896baf..0b2dd72e1b 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/policies/FullAuthPolicy.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/policies/FullAuthPolicy.kt @@ -109,5 +109,9 @@ open class FullAuthPolicy( PolicyResult.Rejected("auth-required: this relay requires authentication") } + override fun onAuthenticationFailed(pubKey: HexKey) { + authenticatedUsers.remove(pubKey) + } + override fun canSendToSession(event: Event) = true } diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/policies/PolicyStack.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/policies/PolicyStack.kt index 21a0e7adaa..4e3d76ffd2 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/policies/PolicyStack.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/policies/PolicyStack.kt @@ -56,6 +56,10 @@ class PolicyStack( policies.forEach { it.onAuthenticated(pubKey, event) } } + override fun onAuthenticationFailed(pubKey: HexKey) { + policies.forEach { it.onAuthenticationFailed(pubKey) } + } + private inline fun runPolicies( initialCmd: T, operation: (IRelayPolicy, T) -> PolicyResult, diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/NostrServerAuthTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/NostrServerAuthTest.kt index aeaa885758..9f136e903c 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/NostrServerAuthTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/NostrServerAuthTest.kt @@ -589,9 +589,39 @@ class NostrServerAuthTest { assertEquals(1, okMessages.size) assertTrue(okMessages[0].contains(",false,")) assertTrue(okMessages[0].contains("backend rejected user")) - // The pubkey is still recorded by accept(); the hook governs the OK, - // not the authenticated-set membership. - assertTrue((session.policy as FullAuthPolicy).authenticatedUsers.contains(pubkey)) + // A failing hook must roll back the authentication: a false OK and a + // still-authenticated connection would be an auth bypass. + val authPolicy = session.policy as FullAuthPolicy + assertFalse(authPolicy.isAuthenticated()) + assertFalse(authPolicy.authenticatedUsers.contains(pubkey)) + + server.close() + } + + @Test + fun commandsRejectedAfterFailedAuthHook() = + runTest { + val policy = + object : FullAuthPolicy(relayUrl) { + override suspend fun onAuthenticated( + pubKey: String, + event: RelayAuthEvent, + ): Unit = throw IllegalStateException("backend rejected user") + } + + val dispatcher = UnconfinedTestDispatcher(testScheduler) + val server = createServer(dispatcher = dispatcher, policyBuilder = { policy }) + val collector = MessageCollector() + val session = server.connect(collector.sendCallback) + + val msg = OptimizedJsonMapper.fromJsonToMessage(collector.messages[0]) as AuthMessage + session.receive(authJson(authEvent(challenge = msg.challenge))) + + // After a failed auth hook, a privileged REQ must still be gated. + session.receive("""["REQ","sub1",{"kinds":[1]}]""") + val closed = collector.rawMessagesContaining("CLOSED") + assertEquals(1, closed.size) + assertTrue(closed[0].contains("auth-required:")) server.close() }