mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-08-10 08:27:04 +00:00
fix(quartz): roll back authentication when the post-auth hook rejects
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
This commit is contained in:
+1
-1
@@ -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(
|
||||
|
||||
+11
@@ -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.
|
||||
*
|
||||
|
||||
+8
@@ -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
|
||||
}
|
||||
|
||||
+4
@@ -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
|
||||
}
|
||||
|
||||
+4
@@ -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 <T : Command> runPolicies(
|
||||
initialCmd: T,
|
||||
operation: (IRelayPolicy, T) -> PolicyResult<T>,
|
||||
|
||||
+33
-3
@@ -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()
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user