From 37ddccc612d6c72fd3d045d928f5b0b9d78fc0e0 Mon Sep 17 00:00:00 2001 From: John Niang Date: Thu, 28 Sep 2023 08:34:18 -0500 Subject: [PATCH] Update the lastUsed timestamp of PAT at least one minute apart (#4671) #### What type of PR is this? /kind bug /area core #### What this PR does / why we need it: After PAT mechanism implemented by , if we use the same PAT to request endpoints concurrently, we may encounter an error like the screenshot below: image This PR fixes the problem introduced by . We update the lastUsed timestamp of PAT at least one minute apart and with retry. #### Does this PR introduce a user-facing change? ```release-note None ``` --- .../pat/PatAuthenticationManager.java | 42 +++++++++++++++---- 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/application/src/main/java/run/halo/app/security/authentication/pat/PatAuthenticationManager.java b/application/src/main/java/run/halo/app/security/authentication/pat/PatAuthenticationManager.java index 02f8cee197..13f0db9b61 100644 --- a/application/src/main/java/run/halo/app/security/authentication/pat/PatAuthenticationManager.java +++ b/application/src/main/java/run/halo/app/security/authentication/pat/PatAuthenticationManager.java @@ -6,7 +6,9 @@ import com.nimbusds.jwt.JWTClaimNames; import java.time.Clock; +import java.time.Duration; import java.util.Objects; +import org.springframework.dao.OptimisticLockingFailureException; import org.springframework.security.authentication.DisabledException; import org.springframework.security.authentication.ReactiveAuthenticationManager; import org.springframework.security.core.Authentication; @@ -17,6 +19,7 @@ import org.springframework.security.oauth2.server.resource.authentication.ReactiveJwtAuthenticationConverter; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; +import reactor.util.retry.Retry; import run.halo.app.extension.ExtensionUtil; import run.halo.app.extension.ReactiveExtensionClient; import run.halo.app.security.PersonalAccessToken; @@ -24,6 +27,11 @@ public class PatAuthenticationManager implements ReactiveAuthenticationManager { + /** + * Minimal duration gap of personal access token update. + */ + private static final Duration MIN_UPDATE_GAP = Duration.ofMinutes(1); + private final ReactiveAuthenticationManager delegate; private final ReactiveExtensionClient client; @@ -78,17 +86,33 @@ private Mono checkAvailability(JwtAuthenticationToken jwtAuthToken) { return client.fetch(PersonalAccessToken.class, patName) .switchIfEmpty( Mono.error(() -> new DisabledException("Personal access token has been deleted."))) - .flatMap(pat -> patChecks(pat, jwtId) - .then(updateLastUsed(pat)) - .then() - ); + .flatMap(pat -> patChecks(pat, jwtId).and(updateLastUsed(patName))); } - private Mono updateLastUsed(PersonalAccessToken pat) { - return Mono.defer(() -> { - pat.getSpec().setLastUsed(clock.instant()); - return client.update(pat); - }); + private Mono updateLastUsed(String patName) { + // we try our best to update the last used timestamp. + + // the now should be outside the retry cycle because we don't want a fresh timestamp at + // every retry. + var now = clock.instant(); + return Mono.defer( + // we have to obtain a fresh PAT and retry the update. + () -> client.fetch(PersonalAccessToken.class, patName) + .filter(pat -> { + var lastUsed = pat.getSpec().getLastUsed(); + if (lastUsed == null) { + return true; + } + var diff = Duration.between(lastUsed, now); + return !diff.minus(MIN_UPDATE_GAP).isNegative(); + }) + .doOnNext(pat -> pat.getSpec().setLastUsed(now)) + .flatMap(client::update) + ) + .retryWhen(Retry.backoff(3, Duration.ofMillis(50)) + .filter(OptimisticLockingFailureException.class::isInstance)) + .onErrorComplete() + .then(); } private Mono patChecks(PersonalAccessToken pat, String tokenId) {