Add `@NotNull` to controller args where appropriate

Notably, `@Valid` doesn't imply `@NotNull`
This commit is contained in:
Ravi Khadiwala 2022-02-08 18:30:58 -06:00 committed by ravi-signal
parent ed398aa7b9
commit baaae6cd9f
10 changed files with 50 additions and 20 deletions

View File

@ -29,6 +29,7 @@ import java.util.concurrent.TimeUnit;
import javax.annotation.Nullable;
import javax.servlet.http.HttpServletRequest;
import javax.validation.Valid;
import javax.validation.constraints.NotNull;
import javax.ws.rs.BadRequestException;
import javax.ws.rs.Consumes;
import javax.ws.rs.DELETE;
@ -339,7 +340,7 @@ public class AccountController {
@HeaderParam("X-Signal-Agent") String signalAgent,
@HeaderParam("User-Agent") String userAgent,
@QueryParam("transfer") Optional<Boolean> availableForTransfer,
@Valid AccountAttributes accountAttributes)
@NotNull @Valid AccountAttributes accountAttributes)
throws RateLimitExceededException, InterruptedException {
String number = authorizationHeader.getUsername();
@ -399,7 +400,7 @@ public class AccountController {
@PUT
@Path("/number")
@Produces(MediaType.APPLICATION_JSON)
public AccountIdentityResponse changeNumber(@Auth final AuthenticatedAccount authenticatedAccount, @Valid final ChangePhoneNumberRequest request)
public AccountIdentityResponse changeNumber(@Auth final AuthenticatedAccount authenticatedAccount, @NotNull @Valid final ChangePhoneNumberRequest request)
throws RateLimitExceededException, InterruptedException, ImpossiblePhoneNumberException, NonNormalizedPhoneNumberException {
final Account updatedAccount;
@ -457,7 +458,7 @@ public class AccountController {
@Consumes(MediaType.APPLICATION_JSON)
@ChangesDeviceEnabledState
public void setGcmRegistrationId(@Auth DisabledPermittedAuthenticatedAccount disabledPermittedAuth,
@Valid GcmRegistrationId registrationId) {
@NotNull @Valid GcmRegistrationId registrationId) {
Account account = disabledPermittedAuth.getAccount();
Device device = disabledPermittedAuth.getAuthenticatedDevice();
@ -495,7 +496,7 @@ public class AccountController {
@Consumes(MediaType.APPLICATION_JSON)
@ChangesDeviceEnabledState
public void setApnRegistrationId(@Auth DisabledPermittedAuthenticatedAccount disabledPermittedAuth,
@Valid ApnRegistrationId registrationId) {
@NotNull @Valid ApnRegistrationId registrationId) {
Account account = disabledPermittedAuth.getAccount();
Device device = disabledPermittedAuth.getAuthenticatedDevice();
@ -530,7 +531,7 @@ public class AccountController {
@PUT
@Produces(MediaType.APPLICATION_JSON)
@Path("/registration_lock")
public void setRegistrationLock(@Auth AuthenticatedAccount auth, @Valid RegistrationLock accountLock) {
public void setRegistrationLock(@Auth AuthenticatedAccount auth, @NotNull @Valid RegistrationLock accountLock) {
AuthenticationCredentials credentials = new AuthenticationCredentials(accountLock.getRegistrationLock());
accounts.update(auth.getAccount(),
@ -547,7 +548,7 @@ public class AccountController {
@Timed
@PUT
@Path("/name/")
public void setName(@Auth DisabledPermittedAuthenticatedAccount disabledPermittedAuth, @Valid DeviceName deviceName) {
public void setName(@Auth DisabledPermittedAuthenticatedAccount disabledPermittedAuth, @NotNull @Valid DeviceName deviceName) {
Account account = disabledPermittedAuth.getAccount();
Device device = disabledPermittedAuth.getAuthenticatedDevice();
accounts.updateDevice(account, device.getId(), d -> d.setName(deviceName.getDeviceName()));
@ -567,7 +568,7 @@ public class AccountController {
@ChangesDeviceEnabledState
public void setAccountAttributes(@Auth DisabledPermittedAuthenticatedAccount disabledPermittedAuth,
@HeaderParam("X-Signal-Agent") String userAgent,
@Valid AccountAttributes attributes) {
@NotNull @Valid AccountAttributes attributes) {
Account account = disabledPermittedAuth.getAccount();
long deviceId = disabledPermittedAuth.getAuthenticatedDevice().getId();

View File

@ -224,7 +224,7 @@ public class DeviceController {
@Timed
@PUT
@Path("/capabilities")
public void setCapabiltities(@Auth AuthenticatedAccount auth, @Valid DeviceCapabilities capabilities) {
public void setCapabiltities(@Auth AuthenticatedAccount auth, @NotNull @Valid DeviceCapabilities capabilities) {
assert (auth.getAuthenticatedDevice() != null);
final long deviceId = auth.getAuthenticatedDevice().getId();
accounts.updateDevice(auth.getAccount(), deviceId, d -> d.setCapabilities(capabilities));

View File

@ -35,6 +35,7 @@ import java.util.concurrent.ForkJoinPool.ManagedBlocker;
import java.util.function.Function;
import javax.annotation.Nonnull;
import javax.validation.Valid;
import javax.validation.constraints.NotNull;
import javax.ws.rs.Consumes;
import javax.ws.rs.POST;
import javax.ws.rs.Path;
@ -124,7 +125,7 @@ public class DonationController {
@Produces({MediaType.APPLICATION_JSON, MediaType.TEXT_PLAIN})
public CompletionStage<Response> redeemReceipt(
@Auth final AuthenticatedAccount auth,
@Valid final RedeemReceiptRequest request) {
@NotNull @Valid final RedeemReceiptRequest request) {
return CompletableFuture.supplyAsync(() -> {
ReceiptCredentialPresentation receiptCredentialPresentation;
try {
@ -192,7 +193,7 @@ public class DonationController {
@Path("/authorize-apple-pay")
@Consumes(MediaType.APPLICATION_JSON)
@Produces(MediaType.APPLICATION_JSON)
public CompletableFuture<Response> getApplePayAuthorization(@Auth AuthenticatedAccount auth, @Valid ApplePayAuthorizationRequest request) {
public CompletableFuture<Response> getApplePayAuthorization(@Auth AuthenticatedAccount auth, @NotNull @Valid ApplePayAuthorizationRequest request) {
if (!supportedCurrencies.contains(request.getCurrency())) {
return CompletableFuture.completedFuture(Response.status(422).build());
}

View File

@ -18,6 +18,7 @@ import java.util.Map;
import java.util.Optional;
import java.util.UUID;
import javax.validation.Valid;
import javax.validation.constraints.NotNull;
import javax.ws.rs.Consumes;
import javax.ws.rs.GET;
import javax.ws.rs.HeaderParam;
@ -85,7 +86,7 @@ public class KeysController {
@Consumes(MediaType.APPLICATION_JSON)
@ChangesDeviceEnabledState
public void setKeys(@Auth final DisabledPermittedAuthenticatedAccount disabledPermittedAuth,
@Valid final PreKeyState preKeys,
@NotNull @Valid final PreKeyState preKeys,
@QueryParam("identity") final Optional<String> identityType) {
Account account = disabledPermittedAuth.getAccount();
Device device = disabledPermittedAuth.getAuthenticatedDevice();

View File

@ -38,6 +38,7 @@ import java.util.stream.Collectors;
import java.util.stream.Stream;
import javax.annotation.Nonnull;
import javax.validation.Valid;
import javax.validation.constraints.NotNull;
import javax.ws.rs.BadRequestException;
import javax.ws.rs.Consumes;
import javax.ws.rs.DELETE;
@ -167,7 +168,7 @@ public class MessageController {
@HeaderParam("User-Agent") String userAgent,
@HeaderParam("X-Forwarded-For") String forwardedFor,
@PathParam("destination") UUID destinationUuid,
@Valid IncomingMessageList messages)
@NotNull @Valid IncomingMessageList messages)
throws RateLimitExceededException, RateLimitChallengeException {
if (source.isEmpty() && accessKey.isEmpty()) {
@ -274,7 +275,7 @@ public class MessageController {
@PathParam("destination") UUID destinationUuid,
@QueryParam("online") boolean online,
@QueryParam("ts") long timestamp,
@Valid IncomingDeviceMessage[] messages)
@NotNull @Valid IncomingDeviceMessage[] messages)
throws RateLimitExceededException, RateLimitChallengeException {
if (source.isEmpty() && accessKey.isEmpty()) {
@ -374,7 +375,7 @@ public class MessageController {
@HeaderParam("X-Forwarded-For") String forwardedFor,
@QueryParam("online") boolean online,
@QueryParam("ts") long timestamp,
@Valid MultiRecipientMessage multiRecipientMessage) {
@NotNull @Valid MultiRecipientMessage multiRecipientMessage) {
Map<UUID, Account> uuidToAccountMap = Arrays.stream(multiRecipientMessage.getRecipients())
.map(Recipient::getUuid)

View File

@ -23,6 +23,7 @@ import java.util.UUID;
import java.util.function.Function;
import java.util.stream.Collectors;
import javax.validation.Valid;
import javax.validation.constraints.NotNull;
import javax.ws.rs.BadRequestException;
import javax.ws.rs.Consumes;
import javax.ws.rs.DefaultValue;
@ -148,7 +149,7 @@ public class ProfileController {
@PUT
@Produces(MediaType.APPLICATION_JSON)
@Consumes(MediaType.APPLICATION_JSON)
public Response setProfile(@Auth AuthenticatedAccount auth, @Valid CreateProfileRequest request) {
public Response setProfile(@Auth AuthenticatedAccount auth, @NotNull @Valid CreateProfileRequest request) {
if (StringUtils.isNotBlank(request.getPaymentAddress())) {
final boolean hasDisallowedPrefix =
dynamicConfigurationManager.getConfiguration().getPaymentsConfiguration().getDisallowedPrefixes().stream()

View File

@ -9,6 +9,7 @@ import com.codahale.metrics.annotation.Timed;
import io.dropwizard.auth.Auth;
import java.util.Base64;
import javax.validation.Valid;
import javax.validation.constraints.NotNull;
import javax.ws.rs.Consumes;
import javax.ws.rs.PUT;
import javax.ws.rs.Path;
@ -41,7 +42,7 @@ public class ProvisioningController {
@Produces(MediaType.APPLICATION_JSON)
public void sendProvisioningMessage(@Auth AuthenticatedAccount auth,
@PathParam("destination") String destinationName,
@Valid ProvisioningMessage message)
@NotNull @Valid ProvisioningMessage message)
throws RateLimitExceededException {
rateLimiters.getMessagesLimiter().validate(auth.getAccount().getUuid());

View File

@ -19,6 +19,7 @@ import java.util.UUID;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import javax.validation.Valid;
import javax.validation.constraints.NotNull;
import javax.ws.rs.Consumes;
import javax.ws.rs.DELETE;
import javax.ws.rs.GET;
@ -79,7 +80,7 @@ public class RemoteConfigController {
@PUT
@Consumes(MediaType.APPLICATION_JSON)
@Produces(MediaType.APPLICATION_JSON)
public void set(@HeaderParam("Config-Token") String configToken, @Valid RemoteConfig config) {
public void set(@HeaderParam("Config-Token") String configToken, @NotNull @Valid RemoteConfig config) {
if (!isAuthorized(configToken)) {
throw new WebApplicationException(Response.Status.UNAUTHORIZED);
}

View File

@ -35,6 +35,7 @@ import javax.crypto.spec.SecretKeySpec;
import javax.validation.Valid;
import javax.validation.constraints.Min;
import javax.validation.constraints.NotEmpty;
import javax.validation.constraints.NotNull;
import javax.ws.rs.BadRequestException;
import javax.ws.rs.Consumes;
import javax.ws.rs.DELETE;
@ -512,7 +513,7 @@ public class SubscriptionController {
@Path("/boost/create")
@Consumes(MediaType.APPLICATION_JSON)
@Produces(MediaType.APPLICATION_JSON)
public CompletableFuture<Response> createBoostPaymentIntent(CreateBoostRequest request) {
public CompletableFuture<Response> createBoostPaymentIntent(@NotNull @Valid CreateBoostRequest request) {
return stripeManager.createPaymentIntent(request.getCurrency(), request.getAmount())
.thenApply(paymentIntent -> Response.ok(new CreateBoostResponse(paymentIntent.getClientSecret())).build());
}
@ -530,10 +531,12 @@ public class SubscriptionController {
this.receiptCredentialRequest = receiptCredentialRequest;
}
@NotNull
public String getPaymentIntentId() {
return paymentIntentId;
}
@NotNull
public byte[] getReceiptCredentialRequest() {
return receiptCredentialRequest;
}
@ -559,7 +562,7 @@ public class SubscriptionController {
@Path("/boost/receipt_credentials")
@Consumes(MediaType.APPLICATION_JSON)
@Produces(MediaType.APPLICATION_JSON)
public CompletableFuture<Response> createBoostReceiptCredentials(CreateBoostReceiptCredentialsRequest request) {
public CompletableFuture<Response> createBoostReceiptCredentials(@NotNull @Valid CreateBoostReceiptCredentialsRequest request) {
return stripeManager.getPaymentIntent(request.getPaymentIntentId())
.thenCompose(paymentIntent -> {
if (paymentIntent == null) {
@ -745,7 +748,7 @@ public class SubscriptionController {
public CompletableFuture<Response> createSubscriptionReceiptCredentials(
@Auth Optional<AuthenticatedAccount> authenticatedAccount,
@PathParam("subscriberId") String subscriberId,
@Valid GetReceiptCredentialsRequest request) {
@NotNull @Valid GetReceiptCredentialsRequest request) {
RequestData requestData = RequestData.process(authenticatedAccount, subscriberId, clock);
return subscriptionManager.get(requestData.subscriberUser, requestData.hmac)
.thenApply(this::requireRecordFromGetResult)

View File

@ -34,6 +34,7 @@ import org.whispersystems.textsecuregcm.configuration.BoostConfiguration;
import org.whispersystems.textsecuregcm.configuration.SubscriptionConfiguration;
import org.whispersystems.textsecuregcm.configuration.SubscriptionLevelConfiguration;
import org.whispersystems.textsecuregcm.configuration.SubscriptionPriceConfiguration;
import org.whispersystems.textsecuregcm.controllers.SubscriptionController.CreateBoostReceiptCredentialsRequest;
import org.whispersystems.textsecuregcm.controllers.SubscriptionController.GetLevelsResponse;
import org.whispersystems.textsecuregcm.entities.Badge;
import org.whispersystems.textsecuregcm.entities.BadgeSvg;
@ -42,6 +43,8 @@ import org.whispersystems.textsecuregcm.storage.SubscriptionManager;
import org.whispersystems.textsecuregcm.stripe.StripeManager;
import org.whispersystems.textsecuregcm.tests.util.AuthHelper;
import org.whispersystems.textsecuregcm.util.SystemMapper;
import javax.ws.rs.client.Entity;
import javax.ws.rs.core.Response;
@ExtendWith(DropwizardExtensionsSupport.class)
class SubscriptionControllerTest {
@ -74,6 +77,23 @@ class SubscriptionControllerTest {
BADGE_TRANSLATOR, LEVEL_TRANSLATOR);
}
@Test
void createBoostReceiptInvalid() {
final Response response = RESOURCE_EXTENSION.target("/v1/subscription/boost/receipt_credentials")
.request()
// invalid, request body should have receiptCredentialRequest
.post(Entity.json("{\"paymentIntentId\": \"foo\"}"));
assertThat(response.getStatus()).isEqualTo(422);
}
@Test
void createBoostReceiptNoRequest() {
final Response response = RESOURCE_EXTENSION.target("/v1/subscription/boost/receipt_credentials")
.request()
.post(Entity.json(""));
assertThat(response.getStatus()).isEqualTo(422);
}
@Test
void getLevels() {
when(SUBSCRIPTION_CONFIG.getLevels()).thenReturn(Map.of(