Skip to content

Commit ce6c163

Browse files
committed
add deferred account deletion logic with immediate soft deletes
1 parent b8eaae0 commit ce6c163

8 files changed

Lines changed: 58 additions & 35 deletions

File tree

src/integrationTest/java/com/trynoice/api/identity/AccountControllerTest.java

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import static org.junit.jupiter.api.Assertions.assertNotEquals;
4242
import static org.junit.jupiter.api.Assertions.assertNotNull;
4343
import static org.junit.jupiter.api.Assertions.assertNull;
44+
import static org.junit.jupiter.api.Assertions.assertTrue;
4445
import static org.junit.jupiter.params.provider.Arguments.arguments;
4546
import static org.mockito.ArgumentMatchers.any;
4647
import static org.mockito.ArgumentMatchers.eq;
@@ -357,11 +358,18 @@ void deleteAccount() throws Exception {
357358
.andExpect(status().is(HttpStatus.NO_CONTENT.value()));
358359

359360
// validate that all existing refresh tokens have been revoked.
360-
refreshTokens.forEach(t -> assertNull(entityManager.find(RefreshToken.class, t.getId())));
361+
refreshTokens.stream()
362+
.map(t -> entityManager.find(RefreshToken.class, t.getId()))
363+
.forEach(t -> assertTrue(t.getExpiresAt().isBefore(OffsetDateTime.now())));
361364

362-
// perform the request again to ensure access token no longer works
365+
// validate that account has been deactivated.'
366+
Stream.of(user)
367+
.map(u -> entityManager.find(AuthUser.class, u.getId()))
368+
.forEach(u -> assertNotNull(u.getDeactivatedAt()));
369+
370+
// perform a request to ensure access token no longer works
363371
mockMvc.perform(
364-
delete(urlFmt, anotherUser.getId())
372+
get("/v1/accounts/profile")
365373
.header("Authorization", "Bearer " + accessToken))
366374
.andExpect(status().is(HttpStatus.UNAUTHORIZED.value()));
367375
}

src/integrationTest/java/com/trynoice/api/identity/entities/RefreshTokenRepositoryTest.java

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import org.springframework.transaction.annotation.Transactional;
88

99
import javax.persistence.EntityManager;
10+
import java.time.OffsetDateTime;
1011
import java.util.stream.Collectors;
1112
import java.util.stream.IntStream;
1213

@@ -25,7 +26,7 @@ public class RefreshTokenRepositoryTest {
2526
private RefreshTokenRepository refreshTokenRepository;
2627

2728
@Test
28-
void deleteAllByOwnerId() {
29+
void expireAllByOwnerId() {
2930
val user = createAuthUser(entityManager);
3031

3132
val ownedRefreshTokens = IntStream.range(0, 5)
@@ -36,13 +37,11 @@ void deleteAllByOwnerId() {
3637
.mapToObj(i -> createRefreshToken(entityManager, createAuthUser(entityManager)))
3738
.collect(Collectors.toUnmodifiableList());
3839

39-
refreshTokenRepository.deleteAllByOwnerId(user.getId());
40-
assertTrue(
41-
ownedRefreshTokens.stream()
42-
.noneMatch(t -> refreshTokenRepository.existsById(t.getId())));
40+
refreshTokenRepository.expireAllByOwnerId(user.getId());
41+
ownedRefreshTokens.stream()
42+
.map(t -> entityManager.find(RefreshToken.class, t.getId()))
43+
.forEach(t -> assertTrue(t.getExpiresAt().isBefore(OffsetDateTime.now())));
4344

44-
assertTrue(
45-
unownedRefreshTokens.stream()
46-
.allMatch(t -> refreshTokenRepository.existsById(t.getId())));
45+
unownedRefreshTokens.forEach(t -> assertTrue(refreshTokenRepository.existsById(t.getId())));
4746
}
4847
}

src/main/java/com/trynoice/api/identity/AccountController.java

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -271,12 +271,18 @@ ResponseEntity<Void> updateProfile(
271271
}
272272

273273
/**
274-
* Deletes the account of an authenticated user. If the account with the given {@literal
275-
* accountId} does not belong to the authenticated user, it returns {@literal HTTP 400}.
274+
* <p>
275+
* Schedules an account for deletion. If the account with the given {@literal accountId} does
276+
* not belong to the authenticated user, it returns {@literal HTTP 400}.</p>
277+
*
278+
* <p>
279+
* Users will immediately lose access to their accounts and their account will be deactivated.
280+
* Deactivated accounts are permanently deleted after some time. Users can restore their
281+
* deactivated accounts before permanent deletion by completing a successful sign-in.</p>
276282
*
277283
* @param accountId must be the account id of the authenticated user.
278284
*/
279-
@Operation(summary = "Delete account of the auth user")
285+
@Operation(summary = "Schedule the account of the auth user for deletion")
280286
@ApiResponses({
281287
@ApiResponse(responseCode = "204", description = "account deleted successfully"),
282288
@ApiResponse(responseCode = "400", description = "request is not valid"),

src/main/java/com/trynoice/api/identity/AccountService.java

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -60,24 +60,24 @@ class AccountService implements AccountServiceContract {
6060
private final SignInTokenDispatchStrategy signInTokenDispatchStrategy;
6161
private final Algorithm jwtAlgorithm;
6262
private final JWTVerifier jwtVerifier;
63-
private final Cache<String, Boolean> revokedAccessJwtCache;
64-
private final Cache<Long, Boolean> deletedUserIdCache;
63+
private final Cache<String, Boolean> revokedAccessJwtsCache;
64+
private final Cache<Long, Boolean> deactivatedUserIdsCache;
6565

6666
@Autowired
6767
AccountService(
6868
@NonNull AuthUserRepository authUserRepository,
6969
@NonNull RefreshTokenRepository refreshTokenRepository,
7070
@NonNull AuthConfiguration authConfig,
7171
@NonNull SignInTokenDispatchStrategy signInTokenDispatchStrategy,
72-
@NonNull @Qualifier(AuthBeans.REVOKED_ACCESS_JWT_CACHE) Cache<String, Boolean> revokedAccessJwtCache,
73-
@NonNull @Qualifier(AuthBeans.DELETED_USER_ID_CACHE) Cache<Long, Boolean> deletedUserIdCache
72+
@NonNull @Qualifier(AuthBeans.REVOKED_ACCESS_JWTS_CACHE) Cache<String, Boolean> revokedAccessJwtsCache,
73+
@NonNull @Qualifier(AuthBeans.DEACTIVATED_USER_IDS_CACHE) Cache<Long, Boolean> deactivatedUserIdsCache
7474
) {
7575
this.authUserRepository = authUserRepository;
7676
this.refreshTokenRepository = refreshTokenRepository;
7777
this.authConfig = authConfig;
7878
this.signInTokenDispatchStrategy = signInTokenDispatchStrategy;
79-
this.revokedAccessJwtCache = revokedAccessJwtCache;
80-
this.deletedUserIdCache = deletedUserIdCache;
79+
this.revokedAccessJwtsCache = revokedAccessJwtsCache;
80+
this.deactivatedUserIdsCache = deactivatedUserIdsCache;
8181
this.jwtAlgorithm = Algorithm.HMAC256(authConfig.getHmacSecret());
8282
this.jwtVerifier = JWT.require(this.jwtAlgorithm).build();
8383
}
@@ -160,7 +160,7 @@ public void signOut(@NonNull String refreshJwt, @NonNull String accessJwt) throw
160160
val refreshToken = verifyRefreshJWT(refreshJwt);
161161
refreshToken.setExpiresAt(OffsetDateTime.now());
162162
refreshTokenRepository.save(refreshToken);
163-
revokedAccessJwtCache.put(accessJwt, Boolean.TRUE);
163+
revokedAccessJwtsCache.put(accessJwt, Boolean.TRUE);
164164
}
165165

166166
/**
@@ -178,6 +178,10 @@ public AuthCredentialsResponse issueAuthCredentials(@NonNull String refreshToken
178178
var token = verifyRefreshJWT(refreshToken);
179179
val owner = token.getOwner();
180180
owner.updateLastActiveTimestamp(); // update last active timestamp for the user and save.
181+
if (owner.getDeactivatedAt() != null) { // restore deactivated account on successful sign-in
182+
owner.setDeactivatedAt(null);
183+
deactivatedUserIdsCache.invalidate(owner.getId());
184+
}
181185

182186
// ordinal 0 implies that this refresh token is being used to sign in, so persist userAgent
183187
// and reset sign-in attempts.
@@ -283,9 +287,11 @@ public void updateProfile(@NonNull Long userId, @NonNull UpdateProfileParams par
283287
*/
284288
@Transactional(rollbackFor = Throwable.class)
285289
public void deleteAccount(@NonNull Long userId) {
286-
refreshTokenRepository.deleteAllByOwnerId(userId);
287-
authUserRepository.deleteById(userId);
288-
deletedUserIdCache.put(userId, Boolean.TRUE);
290+
val user = authUserRepository.findById(userId).orElseThrow();
291+
user.setDeactivatedAt(OffsetDateTime.now());
292+
authUserRepository.save(user);
293+
deactivatedUserIdsCache.put(userId, Boolean.TRUE);
294+
refreshTokenRepository.expireAllByOwnerId(userId);
289295
}
290296

291297
@Override
@@ -304,14 +310,14 @@ public Optional<String> findEmailByUser(@NonNull Long userId) {
304310
*/
305311
Authentication verifyAccessToken(@NonNull String token) {
306312
// check if the token was revoked during sign-out.
307-
if (requireNonNullElse(revokedAccessJwtCache.getIfPresent(token), false)) {
313+
if (requireNonNullElse(revokedAccessJwtsCache.getIfPresent(token), false)) {
308314
log.trace("attempted authentication with a revoked access token");
309315
return null;
310316
}
311317

312318
try {
313319
val auth = new BearerJWT(token);
314-
if (!requireNonNullElse(deletedUserIdCache.getIfPresent(auth.principalId), false)) {
320+
if (!requireNonNullElse(deactivatedUserIdsCache.getIfPresent(auth.principalId), false)) {
315321
return auth;
316322
}
317323

src/main/java/com/trynoice/api/identity/AuthBeans.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@
1616
@Configuration
1717
class AuthBeans {
1818

19-
static final String REVOKED_ACCESS_JWT_CACHE = "revoked_access_jwts";
20-
static final String DELETED_USER_ID_CACHE = "deleted_user_ids";
19+
static final String REVOKED_ACCESS_JWTS_CACHE = "revoked_access_jwts";
20+
static final String DEACTIVATED_USER_IDS_CACHE = "deleted_user_ids";
2121

2222
@NonNull
2323
@Bean
@@ -62,17 +62,17 @@ public FilterRegistrationBean<CookieAuthFilter> cookieAuthFilterRegistration(Coo
6262
}
6363

6464
@NonNull
65-
@Bean(REVOKED_ACCESS_JWT_CACHE)
66-
public Cache<String, Boolean> revokedAccessJwtCache(@NonNull AuthConfiguration authConfig) {
65+
@Bean(REVOKED_ACCESS_JWTS_CACHE)
66+
public Cache<String, Boolean> revokedAccessJwtsCache(@NonNull AuthConfiguration authConfig) {
6767
return Caffeine.newBuilder()
6868
.expireAfterWrite(authConfig.getAccessTokenExpiry())
6969
.maximumSize(1000) // an arbitrary upper-limit for sanity.
7070
.build();
7171
}
7272

7373
@NonNull
74-
@Bean(DELETED_USER_ID_CACHE)
75-
public Cache<Long, Boolean> deleteUserIdCache(@NonNull AuthConfiguration authConfig) {
74+
@Bean(DEACTIVATED_USER_IDS_CACHE)
75+
public Cache<Long, Boolean> deactivatedUserIdsCache(@NonNull AuthConfiguration authConfig) {
7676
return Caffeine.newBuilder()
7777
.expireAfterWrite(authConfig.getAccessTokenExpiry())
7878
.maximumSize(1000) // an arbitrary upper-limit for sanity.

src/main/java/com/trynoice/api/identity/entities/AuthUser.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ public class AuthUser {
5353

5454
private OffsetDateTime lastSignInAttemptAt;
5555

56+
private OffsetDateTime deactivatedAt;
57+
5658
public void updateLastActiveTimestamp() {
5759
this.lastActiveAt = OffsetDateTime.now();
5860
}

src/main/java/com/trynoice/api/identity/entities/RefreshTokenRepository.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,13 @@
1414
public interface RefreshTokenRepository extends CrudRepository<RefreshToken, Long> {
1515

1616
/**
17-
* Deletes all instances of the type {@link RefreshToken} for the given {@literal ownerId}.
17+
* Marks all instances of the type {@link RefreshToken} as expired for the given {@literal
18+
* ownerId}.
1819
*
1920
* @param ownerId must not be {@literal null}.
2021
*/
2122
@Modifying(flushAutomatically = true, clearAutomatically = true)
2223
@Transactional
23-
@Query("delete from RefreshToken e where e.owner.id = ?1")
24-
void deleteAllByOwnerId(@NonNull Long ownerId);
24+
@Query("update RefreshToken e set e.expiresAt = now() where e.owner.id = ?1")
25+
void expireAllByOwnerId(@NonNull Long ownerId);
2526
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ALTER TABLE auth_user ADD COLUMN deactivated_at timestamp with time zone;

0 commit comments

Comments
 (0)