Skip to content

Commit eaf7d9f

Browse files
committed
Addressing comment from Stoyan
1 parent 81c0db1 commit eaf7d9f

2 files changed

Lines changed: 42 additions & 6 deletions

File tree

forgerock-authenticator/src/main/java/org/forgerock/android/auth/PushResponder.java

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -356,9 +356,18 @@ public void onResponse(@NotNull Call call, @NotNull Response response) {
356356
}
357357
listener.onSuccess(null);
358358
} else if (response.code() == 400 && pushNotification.getPushType() == PushType.CHALLENGE) {
359-
// Number challenge failed — notification stays pending so the user can retry.
360-
listener.onException(new PushNumberChallengeException(
361-
"Number challenge failed: the selected number was incorrect."));
359+
String challengeMessage = "Number challenge failed.";
360+
try {
361+
if (response.body() != null) {
362+
JSONObject json = new JSONObject(response.body().string());
363+
String amMessage = json.optString("message", null);
364+
if (amMessage != null && !amMessage.isEmpty()) {
365+
challengeMessage = amMessage;
366+
}
367+
}
368+
} catch (IOException | JSONException ignored) {}
369+
Logger.warn(TAG, "Push 400 response for CHALLENGE notification: %s", challengeMessage);
370+
listener.onException(new PushNumberChallengeException(challengeMessage));
362371
} else {
363372
listener.onException(new PushMechanismException("Communication with " +
364373
"server returned " + response.code() + " code."));

forgerock-authenticator/src/test/java/org/forgerock/android/auth/PushResponderTest.java

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -275,8 +275,35 @@ public void testShouldSignJWTCorrectly() throws Exception {
275275

276276
@Test
277277
public void testReplyAuthenticationMessageNumberChallenge400() {
278-
// AC (a): 400 on a CHALLENGE notification → PushNumberChallengeException with "incorrect"
279-
// message; notification stays pending=true, approved=false.
278+
// AC (a): 400 on a CHALLENGE notification with AM JSON body → PushNumberChallengeException
279+
// surfaces AM's message; notification stays pending=true, approved=false.
280+
server.enqueue(new MockResponse()
281+
.setResponseCode(400)
282+
.setBody("{\"code\":400,\"reason\":\"Bad Request\",\"message\":\"Number challenge predicate not met.\"}"));
283+
284+
PushNotification notification = null;
285+
try {
286+
notification = newChallengePushNotification();
287+
} catch (Exception e) {
288+
Assert.fail("Failed to build challenge notification: " + e.getMessage());
289+
}
290+
291+
try {
292+
PushResponder.getInstance(storageClient).authentication(notification, true, pushListenerFuture);
293+
pushListenerFuture.get();
294+
Assert.fail("Should throw PushNumberChallengeException");
295+
} catch (Exception e) {
296+
assertTrue(e.getCause() instanceof PushNumberChallengeException);
297+
assertTrue(e.getLocalizedMessage().contains("Number challenge predicate not met."));
298+
assertTrue(notification.isPending());
299+
assertFalse(notification.isApproved());
300+
}
301+
}
302+
303+
@Test
304+
public void testReplyAuthenticationMessageNumberChallenge400NoAmMessage() {
305+
// AC (a2): 400 on a CHALLENGE notification with no body → fallback message used;
306+
// notification stays pending=true, approved=false.
280307
server.enqueue(new MockResponse().setResponseCode(400));
281308

282309
PushNotification notification = null;
@@ -292,7 +319,7 @@ public void testReplyAuthenticationMessageNumberChallenge400() {
292319
Assert.fail("Should throw PushNumberChallengeException");
293320
} catch (Exception e) {
294321
assertTrue(e.getCause() instanceof PushNumberChallengeException);
295-
assertTrue(e.getLocalizedMessage().contains("incorrect"));
322+
assertTrue(e.getLocalizedMessage().contains("Number challenge failed."));
296323
assertTrue(notification.isPending());
297324
assertFalse(notification.isApproved());
298325
}

0 commit comments

Comments
 (0)