Skip to content

Commit 39479f9

Browse files
author
tcheeric
committed
fix: handle null passphrase in key management methods
- Normalize null or blank passphrases to empty string for create, unlock, and rotate key operations to allow unencrypted keys. - Remove `requirePassphrase` validation and add `normalizePassphrase` method. - Fix MockWebServer lifecycle issues in `RelayConnectionChaosTest` by implementing proper WebSocket event handling.
1 parent db4e29c commit 39479f9

File tree

3 files changed

+85
-13
lines changed

3 files changed

+85
-13
lines changed

nsecbunker-admin/src/main/java/xyz/tcheeric/nsecbunker/admin/key/DefaultKeyManager.java

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -58,20 +58,22 @@ public DefaultKeyManager(NsecBunkerAdminClient adminClient, ObjectMapper objectM
5858
@Override
5959
public CompletableFuture<BunkerKey> createKey(String name, String nsec, String passphrase) {
6060
validateName(name);
61-
requirePassphrase(passphrase);
6261
if (nsec == null || nsec.isBlank()) {
6362
throw new IllegalArgumentException("nsec must not be null or blank");
6463
}
6564

66-
return sendForKey(METHOD_CREATE_NEW_KEY, List.of(name, passphrase, nsec), name);
65+
// Empty passphrase is allowed - nsecbunkerd will store the key unencrypted
66+
String normalizedPassphrase = normalizePassphrase(passphrase);
67+
return sendForKey(METHOD_CREATE_NEW_KEY, List.of(name, normalizedPassphrase, nsec), name);
6768
}
6869

6970
@Override
7071
public CompletableFuture<BunkerKey> createKey(String name, String passphrase) {
7172
validateName(name);
72-
requirePassphrase(passphrase);
7373

74-
return sendForKey(METHOD_CREATE_NEW_KEY, List.of(name, passphrase), name);
74+
// Empty passphrase is allowed - nsecbunkerd will store the key unencrypted
75+
String normalizedPassphrase = normalizePassphrase(passphrase);
76+
return sendForKey(METHOD_CREATE_NEW_KEY, List.of(name, normalizedPassphrase), name);
7577
}
7678

7779
@Override
@@ -83,9 +85,10 @@ public CompletableFuture<List<BunkerKey>> listKeys() {
8385
@Override
8486
public CompletableFuture<Boolean> unlockKey(String name, String passphrase) {
8587
validateName(name);
86-
requirePassphrase(passphrase);
8788

88-
return sendForResult(METHOD_UNLOCK_KEY, List.of(name, passphrase), "unlock key " + name)
89+
// Empty passphrase is allowed - for keys stored without encryption
90+
String normalizedPassphrase = normalizePassphrase(passphrase);
91+
return sendForResult(METHOD_UNLOCK_KEY, List.of(name, normalizedPassphrase), "unlock key " + name)
8992
.thenApply(ignored -> Boolean.TRUE);
9093
}
9194

@@ -108,9 +111,9 @@ public CompletableFuture<BunkerKey> getKeyDetails(String name) {
108111
public CompletableFuture<BunkerKey> rotateKey(String oldName, String newName, String passphrase) {
109112
validateName(oldName);
110113
validateName(newName);
111-
requirePassphrase(passphrase);
112114

113-
return sendForKey(METHOD_ROTATE_KEY, List.of(oldName, newName, passphrase), newName);
115+
String normalizedPassphrase = normalizePassphrase(passphrase);
116+
return sendForKey(METHOD_ROTATE_KEY, List.of(oldName, newName, normalizedPassphrase), newName);
114117
}
115118

116119
private CompletableFuture<String> sendForResult(String method, List<String> params, String description) {
@@ -185,9 +188,14 @@ private void validateName(String name) {
185188
}
186189
}
187190

188-
private void requirePassphrase(String passphrase) {
189-
if (passphrase == null || passphrase.isBlank()) {
190-
throw new IllegalArgumentException("Passphrase must not be null or blank");
191-
}
191+
/**
192+
* Normalizes a passphrase to an empty string if null or blank.
193+
* This allows creating/unlocking keys without encryption.
194+
*
195+
* @param passphrase the passphrase to normalize
196+
* @return the passphrase or empty string if null/blank
197+
*/
198+
private String normalizePassphrase(String passphrase) {
199+
return passphrase != null && !passphrase.isBlank() ? passphrase : "";
192200
}
193201
}

nsecbunker-admin/src/test/java/xyz/tcheeric/nsecbunker/admin/key/DefaultKeyManagerTest.java

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,45 @@ void shouldRotateKey() throws Exception {
213213
assertThat(request.getParams()).containsExactlyElementsOf(List.of("cashu-old", "cashu-rotated", TEST_PASSPHRASE));
214214
}
215215

216+
/**
217+
* Ensures creating a key with null passphrase normalizes it to empty string.
218+
*/
219+
@Test
220+
void shouldCreateKeyWithNullPassphraseNormalizedToEmpty() {
221+
// Arrange
222+
String npub = "npub1nopwd";
223+
ArgumentCaptor<Nip46Request> requestCaptor = ArgumentCaptor.forClass(Nip46Request.class);
224+
when(adminClient.sendRequest(requestCaptor.capture()))
225+
.thenReturn(CompletableFuture.completedFuture(Nip46Response.success("1", npub)));
226+
227+
// Act
228+
BunkerKey result = keyManager.createKey("key-no-passphrase", null).join();
229+
230+
// Assert
231+
assertThat(result.getName()).isEqualTo("key-no-passphrase");
232+
Nip46Request request = requestCaptor.getValue();
233+
assertThat(request.getParams()).containsExactlyElementsOf(List.of("key-no-passphrase", ""));
234+
}
235+
236+
/**
237+
* Ensures unlocking a key with null passphrase normalizes it to empty string.
238+
*/
239+
@Test
240+
void shouldUnlockKeyWithNullPassphraseNormalizedToEmpty() {
241+
// Arrange
242+
ArgumentCaptor<Nip46Request> requestCaptor = ArgumentCaptor.forClass(Nip46Request.class);
243+
when(adminClient.sendRequest(requestCaptor.capture()))
244+
.thenReturn(CompletableFuture.completedFuture(Nip46Response.success("1", "ok")));
245+
246+
// Act
247+
boolean result = keyManager.unlockKey("key-unencrypted", null).join();
248+
249+
// Assert
250+
assertThat(result).isTrue();
251+
Nip46Request request = requestCaptor.getValue();
252+
assertThat(request.getParams()).containsExactlyElementsOf(List.of("key-unencrypted", ""));
253+
}
254+
216255
/**
217256
* Ensures NIP-46 errors are surfaced as AdminException through the future.
218257
*/

nsecbunker-tests/nsecbunker-chaos/src/test/java/xyz/tcheeric/nsecbunker/chaos/RelayConnectionChaosTest.java

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,9 +89,34 @@ void shouldStopAfterMaxReconnectAttempts() throws Exception {
8989
}
9090

9191
/**
92-
* No-op WebSocket listener for MockWebServer upgrades.
92+
* WebSocket listener for MockWebServer upgrades that properly handles lifecycle.
93+
*
94+
* <p>A completely empty listener causes NPE in OkHttp's RealWebSocket.loopReader
95+
* because the reader loop expects proper lifecycle handling. This implementation
96+
* handles onOpen, onMessage, and onClosing to prevent the MockWebServer crash.
9397
*/
9498
private static final class NoopWebSocketListener extends okhttp3.WebSocketListener {
99+
100+
@Override
101+
public void onOpen(okhttp3.WebSocket webSocket, okhttp3.Response response) {
102+
// Connection opened - no action needed for test
103+
}
104+
105+
@Override
106+
public void onMessage(okhttp3.WebSocket webSocket, String text) {
107+
// Message received - no action needed for test
108+
}
109+
110+
@Override
111+
public void onClosing(okhttp3.WebSocket webSocket, int code, String reason) {
112+
// Server closing - echo back close to complete handshake
113+
webSocket.close(code, reason);
114+
}
115+
116+
@Override
117+
public void onFailure(okhttp3.WebSocket webSocket, Throwable t, okhttp3.Response response) {
118+
// Connection failed - no action needed for test
119+
}
95120
}
96121

97122
/**

0 commit comments

Comments
 (0)