mirror/Signal-Server
0
0
Fork 0
mirror of https://github.com/signalapp/Signal-Server.git synced 2024-09-19 19:42:18 +02:00
Code

simplify profile auth

Browse Source
This commit is contained in:
Jonathan Klabunde Tomer 2024-04-23 14:49:04 -07:00 committed by GitHub
parent fba7686390
commit b3bd4ccc17
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 35 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
service/src/main/java/org/whispersystems/textsecuregcm/auth/OptionalAccess.java
View File

@ -50,25 +50,31 @@ public class OptionalAccess {
Optional<Anonymous> accessKey,
Optional<Account> targetAccount)
{
if (requestAccount.isPresent() && targetAccount.isPresent() && targetAccount.get().isEnabled()) {
if (requestAccount.isPresent()) {
// Authenticated requests are never unauthorized; if the target exists and is enabled, return OK, otherwise throw not-found.
if (targetAccount.isPresent() && targetAccount.get().isEnabled()) {
return;
} else {
throw new NotFoundException();
}
}
// Anything past this point can only be authenticated by an access key. Even when the target
// has unrestricted unidentified access, callers need to supply a fake access key. Likewise, if
// the target account does not exist, we *also* report unauthorized here (*not* not-found,
// since that would provide a free exists check).
if (accessKey.isEmpty() || !targetAccount.map(Account::isEnabled).orElse(false)) {
throw new NotAuthorizedException(Response.Status.UNAUTHORIZED);
}
// Unrestricted unidentified access does what it says on the tin: we don't check if the key the
// caller provided is right or not.
if (targetAccount.get().isUnrestrictedUnidentifiedAccess()) {
return;
}
//noinspection ConstantConditions
if (requestAccount.isPresent() && (targetAccount.isEmpty() || (targetAccount.isPresent() && !targetAccount.get().isEnabled()))) {
throw new NotFoundException();
}
if (accessKey.isPresent() && targetAccount.isPresent() && targetAccount.get().isEnabled() && targetAccount.get().isUnrestrictedUnidentifiedAccess()) {
return;
}
if (accessKey.isPresent() &&
targetAccount.isPresent() &&
targetAccount.get().getUnidentifiedAccessKey().isPresent() &&
targetAccount.get().isEnabled() &&
MessageDigest.isEqual(accessKey.get().getAccessKey(), targetAccount.get().getUnidentifiedAccessKey().get()))
{
// Otherwise, access is gated by the caller having the unidentified-access key matching the target account.
if (MessageDigest.isEqual(accessKey.get().getAccessKey(), targetAccount.get().getUnidentifiedAccessKey().get())) {
return;
}

36
service/src/main/java/org/whispersystems/textsecuregcm/controllers/ProfileController.java
View File

@ -237,7 +237,7 @@ public class ProfileController {
throws RateLimitExceededException {
final Optional<Account> maybeRequester = auth.map(AuthenticatedAccount::getAccount);
final Account targetAccount = verifyPermissionToReceiveAccountIdentityProfile(maybeRequester, accessKey, accountIdentifier);
final Account targetAccount = verifyPermissionToReceiveProfile(maybeRequester, accessKey, accountIdentifier);
return buildVersionedProfileResponse(targetAccount,
version,
@ -263,7 +263,7 @@ public class ProfileController {
}
final Optional<Account> maybeRequester = auth.map(AuthenticatedAccount::getAccount);
final Account targetAccount = verifyPermissionToReceiveAccountIdentityProfile(maybeRequester, accessKey, accountIdentifier);
final Account targetAccount = verifyPermissionToReceiveProfile(maybeRequester, accessKey, accountIdentifier);
final boolean isSelf = maybeRequester.map(requester -> ProfileHelper.isSelfProfileRequest(requester.getUuid(), accountIdentifier)).orElse(false);
return buildExpiringProfileKeyCredentialProfileResponse(targetAccount,
@ -290,30 +290,16 @@ public class ProfileController {
final Optional<Account> maybeRequester = auth.map(AuthenticatedAccount::getAccount);
final Account targetAccount = verifyPermissionToReceiveProfile(
maybeRequester, accessKey.filter(ignored -> identifier.identityType() == IdentityType.ACI), identifier);
return switch (identifier.identityType()) {
case ACI -> {
final AciServiceIdentifier aciServiceIdentifier = (AciServiceIdentifier) identifier;
final Account targetAccount =
verifyPermissionToReceiveAccountIdentityProfile(maybeRequester, accessKey, aciServiceIdentifier);
yield buildBaseProfileResponseForAccountIdentity(targetAccount,
maybeRequester.map(requester -> ProfileHelper.isSelfProfileRequest(requester.getUuid(), aciServiceIdentifier)).orElse(false),
maybeRequester.map(requester -> ProfileHelper.isSelfProfileRequest(requester.getUuid(), identifier)).orElse(false),
containerRequestContext);
}
case PNI -> {
final Optional<Account> maybeAccountByPni = accountsManager.getByPhoneNumberIdentifier(identifier.uuid());
if (maybeRequester.isEmpty()) {
throw new WebApplicationException(Response.Status.UNAUTHORIZED);
} else {
rateLimiters.getProfileLimiter().validate(maybeRequester.get().getUuid());
}
OptionalAccess.verify(maybeRequester, Optional.empty(), maybeAccountByPni);
assert maybeAccountByPni.isPresent();
yield buildBaseProfileResponseForPhoneNumberIdentity(maybeAccountByPni.get());
yield buildBaseProfileResponseForPhoneNumberIdentity(targetAccount);
}
};
}
@ -485,19 +471,15 @@ public class ProfileController {
* @throws NotAuthorizedException if the requester is not authorized to receive the target account's profile or if the
* requester was not authenticated and did not present an anonymous access key
*/
private Account verifyPermissionToReceiveAccountIdentityProfile(final Optional<Account> maybeRequester,
private Account verifyPermissionToReceiveProfile(final Optional<Account> maybeRequester,
final Optional<Anonymous> maybeAccessKey,
final AciServiceIdentifier accountIdentifier) throws RateLimitExceededException {
if (maybeRequester.isEmpty() && maybeAccessKey.isEmpty()) {
throw new WebApplicationException(Response.Status.UNAUTHORIZED);
}
final ServiceIdentifier accountIdentifier) throws RateLimitExceededException {
if (maybeRequester.isPresent()) {
rateLimiters.getProfileLimiter().validate(maybeRequester.get().getUuid());
}
final Optional<Account> maybeTargetAccount = accountsManager.getByAccountIdentifier(accountIdentifier.uuid());
final Optional<Account> maybeTargetAccount = accountsManager.getByServiceIdentifier(accountIdentifier);
OptionalAccess.verify(maybeRequester, maybeAccessKey, maybeTargetAccount);
assert maybeTargetAccount.isPresent();

4
service/src/main/java/org/whispersystems/textsecuregcm/util/ProfileHelper.java
View File

@ -9,7 +9,7 @@ import org.signal.libsignal.zkgroup.profiles.ProfileKeyCommitment;
import org.signal.libsignal.zkgroup.profiles.ProfileKeyCredentialRequest;
import org.signal.libsignal.zkgroup.profiles.ServerZkProfileOperations;
import org.whispersystems.textsecuregcm.configuration.BadgeConfiguration;
import org.whispersystems.textsecuregcm.identity.AciServiceIdentifier;
import org.whispersystems.textsecuregcm.identity.ServiceIdentifier;
import org.whispersystems.textsecuregcm.storage.AccountBadge;
import org.whispersystems.textsecuregcm.storage.VersionedProfile;
import javax.annotation.Nullable;
@ -81,7 +81,7 @@ public class ProfileHelper {
return "profiles/" + Base64.getUrlEncoder().encodeToString(object);
}
public static boolean isSelfProfileRequest(@Nullable final UUID requesterUuid, final AciServiceIdentifier targetIdentifier) {
public static boolean isSelfProfileRequest(@Nullable final UUID requesterUuid, final ServiceIdentifier targetIdentifier) {
return targetIdentifier.uuid().equals(requesterUuid);
}

11
service/src/test/java/org/whispersystems/textsecuregcm/controllers/ProfileControllerTest.java
View File

@ -256,7 +256,6 @@ class ProfileControllerTest {
assertThat(profile.getBadges()).hasSize(1).element(0).has(new Condition<>(
badge -> "Test Badge".equals(badge.getName()), "has badge with expected name"));
verify(accountsManager).getByAccountIdentifier(AuthHelper.VALID_UUID_TWO);
verify(rateLimiter, times(1)).validate(AuthHelper.VALID_UUID);
}
@ -287,7 +286,6 @@ class ProfileControllerTest {
assertThat(profile.getBadges()).hasSize(1).element(0).has(new Condition<>(
badge -> "Test Badge".equals(badge.getName()), "has badge with expected name"));
verify(accountsManager).getByAccountIdentifier(AuthHelper.VALID_UUID_TWO);
verify(rateLimiter, never()).validate(AuthHelper.VALID_UUID);
}
@ -328,7 +326,6 @@ class ProfileControllerTest {
assertThat(profile.isUnrestrictedUnidentifiedAccess()).isFalse();
assertThat(profile.getUnidentifiedAccess()).isNull();
verify(accountsManager).getByPhoneNumberIdentifier(AuthHelper.VALID_PNI_TWO);
verify(rateLimiter, times(1)).validate(AuthHelper.VALID_UUID);
}
@ -357,7 +354,6 @@ class ProfileControllerTest {
assertThat(response.getStatus()).isEqualTo(401);
verify(accountsManager).getByPhoneNumberIdentifier(AuthHelper.VALID_PNI_TWO);
verify(rateLimiter, never()).validate(AuthHelper.VALID_UUID);
}
@ -879,9 +875,6 @@ class ProfileControllerTest {
assertThat(profile.getBaseProfileResponse().getBadges()).hasSize(1).element(0).has(new Condition<>(
badge -> "Test Badge".equals(badge.getName()), "has badge with expected name"));
verify(accountsManager, times(1)).getByAccountIdentifier(eq(AuthHelper.VALID_UUID_TWO));
verify(profilesManager, times(1)).get(eq(AuthHelper.VALID_UUID_TWO), eq(version));
verify(rateLimiter, times(1)).validate(AuthHelper.VALID_UUID);
}
@ -1122,7 +1115,7 @@ class ProfileControllerTest {
final ExpiringProfileKeyCredentialResponse credentialResponse =
serverZkProfile.issueExpiringProfileKeyCredential(credentialRequest, new ServiceId.Aci(AuthHelper.VALID_UUID), profileKeyCommitment, expiration);
when(accountsManager.getByAccountIdentifier(AuthHelper.VALID_UUID)).thenReturn(Optional.of(account));
when(accountsManager.getByServiceIdentifier(new AciServiceIdentifier(AuthHelper.VALID_UUID))).thenReturn(Optional.of(account));
when(profilesManager.get(AuthHelper.VALID_UUID, version)).thenReturn(Optional.of(versionedProfile));
when(zkProfileOperations.issueExpiringProfileKeyCredential(eq(credentialRequest), eq(new ServiceId.Aci(AuthHelper.VALID_UUID)), eq(profileKeyCommitment), any()))
.thenReturn(credentialResponse);
@ -1182,7 +1175,7 @@ class ProfileControllerTest {
when(account.isEnabled()).thenReturn(true);
when(account.getUnidentifiedAccessKey()).thenReturn(Optional.of(UNIDENTIFIED_ACCESS_KEY));
when(accountsManager.getByAccountIdentifier(AuthHelper.VALID_UUID)).thenReturn(Optional.of(account));
when(accountsManager.getByServiceIdentifier(new AciServiceIdentifier(AuthHelper.VALID_UUID))).thenReturn(Optional.of(account));
when(profilesManager.get(AuthHelper.VALID_UUID, version)).thenReturn(Optional.of(versionedProfile));
when(zkProfileOperations.issueExpiringProfileKeyCredential(any(), any(), any(), any()))
.thenThrow(new VerificationFailedException());