diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java index d7f10bf22..5185d6610 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java @@ -252,9 +252,14 @@ public class AccountController { Tag.of(SCORE_TAG_NAME, result.score()))) .increment()); + boolean pushChallengeMatch = pushChallengeMatches(number, pushChallenge, storedChallenge); + if (pushChallenge.isPresent() && !pushChallengeMatch) { + throw new WebApplicationException(Response.status(403).build()); + } + final boolean requiresCaptcha = assessmentResult .map(result -> !result.valid()) - .orElseGet(() -> requiresCaptcha(number, transport, forwardedFor, sourceHost, storedChallenge, pushChallenge)); + .orElseGet(() -> requiresCaptcha(number, transport, forwardedFor, sourceHost, pushChallengeMatch)); if (requiresCaptcha) { captchaRequiredMeter.mark(); @@ -821,43 +826,35 @@ public class AccountController { } } - private boolean requiresCaptcha(String number, String transport, String forwardedFor, - String sourceHost, - Optional storedVerificationCode, - Optional pushChallenge) { + private boolean pushChallengeMatches( + final String number, + final Optional pushChallenge, + final Optional storedVerificationCode) { + final String countryCode = Util.getCountryCode(number); + final String region = Util.getRegion(number); + + final List tags = new ArrayList<>(); + tags.add(Tag.of(COUNTRY_CODE_TAG_NAME, countryCode)); + tags.add(Tag.of(REGION_TAG_NAME, region)); + tags.add(Tag.of(CHALLENGE_PRESENT_TAG_NAME, Boolean.toString(pushChallenge.isPresent()))); + Optional storedPushChallenge = storedVerificationCode.map(StoredVerificationCode::getPushCode); + boolean match = pushChallenge.isPresent() && storedPushChallenge.isPresent() && pushChallenge.get().equals(storedPushChallenge.get()); + tags.add(Tag.of(CHALLENGE_MATCH_TAG_NAME, Boolean.toString(match))); + return match; + } + + private boolean requiresCaptcha(String number, String transport, String forwardedFor, String sourceHost, boolean pushChallengeMatch) { if (testDevices.containsKey(number)) { return false; } + if (!pushChallengeMatch) { + return true; + } + final String countryCode = Util.getCountryCode(number); final String region = Util.getRegion(number); - { - final List tags = new ArrayList<>(); - tags.add(Tag.of(COUNTRY_CODE_TAG_NAME, countryCode)); - tags.add(Tag.of(REGION_TAG_NAME, region)); - - try { - if (pushChallenge.isPresent()) { - tags.add(Tag.of(CHALLENGE_PRESENT_TAG_NAME, "true")); - - Optional storedPushChallenge = storedVerificationCode.map(StoredVerificationCode::getPushCode); - - if (!pushChallenge.get().equals(storedPushChallenge.orElse(null))) { - tags.add(Tag.of(CHALLENGE_MATCH_TAG_NAME, "false")); - return true; - } else { - tags.add(Tag.of(CHALLENGE_MATCH_TAG_NAME, "true")); - } - } else { - tags.add(Tag.of(CHALLENGE_PRESENT_TAG_NAME, "false")); - return true; - } - } finally { - Metrics.counter(PUSH_CHALLENGE_COUNTER_NAME, tags).increment(); - } - } - DynamicCaptchaConfiguration captchaConfig = dynamicConfigurationManager.getConfiguration() .getCaptchaConfiguration(); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java index 79f74b554..06e0544e7 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java @@ -705,7 +705,7 @@ class AccountControllerTest { .header("X-Forwarded-For", NICE_HOST) .get(); - assertThat(response.getStatus()).isEqualTo(402); + assertThat(response.getStatus()).isEqualTo(403); verifyNoMoreInteractions(smsSender); verifyNoMoreInteractions(abusiveHostRules);