From 5f7c50cd0d034198a1fc0b34af5f8a82c35f446c Mon Sep 17 00:00:00 2001 From: Wadeck Follonier Date: Tue, 26 Jun 2018 13:06:16 +0200 Subject: [PATCH] [JENKINS-52161] Improve creation-date value determination - in case of migration we put null instead of now - improve also the difference in days for the Xxx day(s) ago to take calendar day in difference and not the total duration (<24h = 0 days, which seems wrong in UI) --- core/src/main/java/hudson/Util.java | 26 +++++++++++++- .../security/apitoken/ApiTokenStats.java | 13 ++++--- .../security/apitoken/ApiTokenStore.java | 36 ++++++++++++------- .../manage.jelly | 20 +++++++---- .../manage.properties | 2 ++ core/src/test/java/hudson/UtilTest.java | 17 +++++++++ 6 files changed, 86 insertions(+), 28 deletions(-) diff --git a/core/src/main/java/hudson/Util.java b/core/src/main/java/hudson/Util.java index 4053ce915e..87c78c62ce 100644 --- a/core/src/main/java/hudson/Util.java +++ b/core/src/main/java/hudson/Util.java @@ -23,6 +23,7 @@ */ package hudson; +import com.sun.istack.internal.NotNull; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import hudson.model.TaskListener; @@ -74,6 +75,9 @@ import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.text.NumberFormat; import java.text.ParseException; +import java.time.LocalDate; +import java.time.Period; +import java.time.ZoneId; import java.util.*; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; @@ -1662,7 +1666,27 @@ public class Util { throw new IOException(e); } } - + + /** + * Compute the number of calendar days elapsed since the given date. + * As it's only the calendar days difference that matter, "11.00pm" to "2.00am the day after" returns 1, + * even if there are only 3 hours between. As well as "10am" to "2pm" both on the same day, returns 0. + */ + @Restricted(NoExternalUse.class) + public static int differenceInCalendarDay(@Nonnull Date a, @Nonnull Date b){ + LocalDate aLocal = a.toInstant().atZone(ZoneId.systemDefault()).toLocalDate(); + LocalDate bLocal = b.toInstant().atZone(ZoneId.systemDefault()).toLocalDate(); + return Period.between(aLocal, bLocal).getDays(); + } + + /** + * @see #differenceInCalendarDay(Date, Date) + */ + @Restricted(NoExternalUse.class) + public static int numOfCalendarDayToNow(@Nonnull Date date){ + return differenceInCalendarDay(date, new Date()); + } + public static final FastDateFormat XS_DATETIME_FORMATTER = FastDateFormat.getInstance("yyyy-MM-dd'T'HH:mm:ss'Z'",new SimpleTimeZone(0,"GMT")); // Note: RFC822 dates must not be localized! diff --git a/core/src/main/java/jenkins/security/apitoken/ApiTokenStats.java b/core/src/main/java/jenkins/security/apitoken/ApiTokenStats.java index bebf9db68d..91594b18e7 100644 --- a/core/src/main/java/jenkins/security/apitoken/ApiTokenStats.java +++ b/core/src/main/java/jenkins/security/apitoken/ApiTokenStats.java @@ -24,6 +24,7 @@ package jenkins.security.apitoken; import hudson.BulkChange; +import hudson.Util; import hudson.XmlFile; import hudson.model.Saveable; import hudson.model.listeners.SaveableListener; @@ -33,7 +34,11 @@ import org.kohsuke.accmod.restrictions.NoExternalUse; import javax.annotation.Nonnull; import java.io.File; import java.io.IOException; +import java.time.Duration; import java.time.Instant; +import java.time.LocalDate; +import java.time.Period; +import java.time.ZoneId; import java.time.temporal.ChronoUnit; import java.util.ArrayList; import java.util.Comparator; @@ -251,13 +256,7 @@ public class ApiTokenStats implements Saveable { * Relevant only if the lastUseDate is not null */ public long getNumDaysUse() { - return lastUseDate == null ? 0 : computeDeltaDays(lastUseDate.toInstant(), Instant.now()); - } - - private long computeDeltaDays(Instant a, Instant b) { - long deltaDays = ChronoUnit.DAYS.between(a, b); - deltaDays = Math.max(0, deltaDays); - return deltaDays; + return lastUseDate == null ? 0 : Math.max(0, Util.numOfCalendarDayToNow(lastUseDate)); } } } diff --git a/core/src/main/java/jenkins/security/apitoken/ApiTokenStore.java b/core/src/main/java/jenkins/security/apitoken/ApiTokenStore.java index bf05aa8be3..c4d2dc2cae 100644 --- a/core/src/main/java/jenkins/security/apitoken/ApiTokenStore.java +++ b/core/src/main/java/jenkins/security/apitoken/ApiTokenStore.java @@ -124,7 +124,7 @@ public class ApiTokenStore { * Remove the legacy token present and generate a new one using the given secret. */ public synchronized void regenerateTokenFromLegacy(@Nonnull Secret newLegacyApiToken) { - deleteAllLegacyAndGenerateNewOne(newLegacyApiToken); + deleteAllLegacyAndGenerateNewOne(newLegacyApiToken, false); } /** @@ -134,13 +134,13 @@ public class ApiTokenStore { */ public synchronized void regenerateTokenFromLegacyIfRequired(@Nonnull Secret newLegacyApiToken) { if(tokenList.stream().noneMatch(HashedToken::isLegacy)){ - deleteAllLegacyAndGenerateNewOne(newLegacyApiToken); + deleteAllLegacyAndGenerateNewOne(newLegacyApiToken, true); } } - private void deleteAllLegacyAndGenerateNewOne(@Nonnull Secret newLegacyApiToken) { + private void deleteAllLegacyAndGenerateNewOne(@Nonnull Secret newLegacyApiToken, boolean migrationFromExistingLegacy) { deleteAllLegacyTokens(); - addLegacyToken(newLegacyApiToken); + addLegacyToken(newLegacyApiToken, migrationFromExistingLegacy); } private void deleteAllLegacyTokens() { @@ -148,13 +148,13 @@ public class ApiTokenStore { tokenList.removeIf(HashedToken::isLegacy); } - private void addLegacyToken(@Nonnull Secret legacyToken) { + private void addLegacyToken(@Nonnull Secret legacyToken, boolean migrationFromExistingLegacy) { String tokenUserUseNormally = Util.getDigestOf(legacyToken.getPlainText()); String secretValueHashed = this.plainSecretToHashInHex(tokenUserUseNormally); HashValue hashValue = new HashValue(LEGACY_VERSION, secretValueHashed); - HashedToken token = HashedToken.buildNew(Messages.ApiTokenProperty_LegacyTokenName(), hashValue); + HashedToken token = HashedToken.buildNewFromLegacy(hashValue, migrationFromExistingLegacy); this.addToken(token); } @@ -371,6 +371,22 @@ public class ApiTokenStore { return result; } + + public static @Nonnull HashedToken buildNewFromLegacy(@Nonnull HashValue value, boolean migrationFromExistingLegacy) { + HashedToken result = new HashedToken(); + result.name = Messages.ApiTokenProperty_LegacyTokenName(); + if(migrationFromExistingLegacy){ + // we do not know when the legacy token was created + result.creationDate = null; + }else{ + // it comes from a manual action, so we set the creation date to now + result.creationDate = new Date(); + } + + result.value = value; + + return result; + } public void rename(String newName) { this.name = newName; @@ -404,7 +420,7 @@ public class ApiTokenStore { * Relevant only if the lastUseDate is not null */ public long getNumDaysCreation() { - return creationDate == null ? 0 : computeDeltaDays(creationDate.toInstant(), Instant.now()); + return creationDate == null ? 0 : Math.max(0, Util.numOfCalendarDayToNow(creationDate)); } // used by Jelly view @@ -412,12 +428,6 @@ public class ApiTokenStore { return this.uuid; } - private long computeDeltaDays(Instant a, Instant b) { - long deltaDays = ChronoUnit.DAYS.between(a, b); - deltaDays = Math.max(0, deltaDays); - return deltaDays; - } - public boolean isLegacy() { return this.value.version.equals(LEGACY_VERSION); } diff --git a/core/src/main/resources/jenkins/security/apitoken/LegacyApiTokenAdministrativeMonitor/manage.jelly b/core/src/main/resources/jenkins/security/apitoken/LegacyApiTokenAdministrativeMonitor/manage.jelly index d385f62d03..550cb12d92 100644 --- a/core/src/main/resources/jenkins/security/apitoken/LegacyApiTokenAdministrativeMonitor/manage.jelly +++ b/core/src/main/resources/jenkins/security/apitoken/LegacyApiTokenAdministrativeMonitor/manage.jelly @@ -58,20 +58,26 @@ THE SOFTWARE. - + + + + + + + - + - + @@ -94,13 +100,13 @@ THE SOFTWARE. ${legacyToken.name} - ${legacyStats.numDaysCreation} + ${creationDateValue} ${legacyStats.useCounter} - ${legacyStats.numDaysUse} + ${lastUseDateValue} @@ -138,14 +144,14 @@ THE SOFTWARE. - +
Select: all only fresh only recent
- +