From 25da480fab1da7d2a01ab68a56faf00cae0c23b1 Mon Sep 17 00:00:00 2001 From: Wadeck Follonier Date: Mon, 30 Jul 2018 21:22:46 +0200 Subject: [PATCH] [JENKINS-52441] Care about user with id = 'null' (#3558) * [JENKINS-52441] Care about user with id = 'null' * Add test case for the null id * Address Ramon's comment * Adjust whitespaces --- .../LegacyApiTokenAdministrativeMonitor.java | 20 ++++--- ...gacyApiTokenAdministrativeMonitorTest.java | 57 +++++++++++++++++-- 2 files changed, 64 insertions(+), 13 deletions(-) diff --git a/core/src/main/java/jenkins/security/apitoken/LegacyApiTokenAdministrativeMonitor.java b/core/src/main/java/jenkins/security/apitoken/LegacyApiTokenAdministrativeMonitor.java index 7c2a2ce1b0..8e56ceb40b 100644 --- a/core/src/main/java/jenkins/security/apitoken/LegacyApiTokenAdministrativeMonitor.java +++ b/core/src/main/java/jenkins/security/apitoken/LegacyApiTokenAdministrativeMonitor.java @@ -100,7 +100,7 @@ public class LegacyApiTokenAdministrativeMonitor extends AdministrativeMonitor { @Restricted(NoExternalUse.class) public @Nullable ApiTokenProperty.TokenInfoAndStats getLegacyStatsOf(@Nonnull User user, @Nullable ApiTokenStore.HashedToken legacyToken) { ApiTokenProperty apiTokenProperty = user.getProperty(ApiTokenProperty.class); - if(legacyToken != null){ + if (legacyToken != null) { ApiTokenStats.SingleTokenStats legacyStats = apiTokenProperty.getTokenStats().findTokenStatsById(legacyToken.getUuid()); ApiTokenProperty.TokenInfoAndStats tokenInfoAndStats = new ApiTokenProperty.TokenInfoAndStats(legacyToken, legacyStats); return tokenInfoAndStats; @@ -116,7 +116,7 @@ public class LegacyApiTokenAdministrativeMonitor extends AdministrativeMonitor { // used by Jelly view @Restricted(NoExternalUse.class) public boolean hasFreshToken(@Nonnull User user, @Nullable ApiTokenProperty.TokenInfoAndStats legacyStats) { - if(legacyStats == null){ + if (legacyStats == null) { return false; } @@ -140,12 +140,12 @@ public class LegacyApiTokenAdministrativeMonitor extends AdministrativeMonitor { // used by Jelly view @Restricted(NoExternalUse.class) public boolean hasMoreRecentlyUsedToken(@Nonnull User user, @Nullable ApiTokenProperty.TokenInfoAndStats legacyStats) { - if(legacyStats == null){ + if (legacyStats == null) { return false; } ApiTokenProperty apiTokenProperty = user.getProperty(ApiTokenProperty.class); - + return apiTokenProperty.getTokenList().stream() .filter(token -> !token.isLegacy) .anyMatch(token -> { @@ -161,18 +161,22 @@ public class LegacyApiTokenAdministrativeMonitor extends AdministrativeMonitor { @RequirePOST public HttpResponse doRevokeAllSelected(@JsonBody RevokeAllSelectedModel content) throws IOException { for (RevokeAllSelectedUserAndUuid value : content.values) { + if (value.userId == null) { + // special case not managed by JSONObject + value.userId = "null"; + } User user = User.getById(value.userId, false); if (user == null) { LOGGER.log(Level.INFO, "User not found id={0}", value.userId); } else { ApiTokenProperty apiTokenProperty = user.getProperty(ApiTokenProperty.class); - if(apiTokenProperty == null){ + if (apiTokenProperty == null) { LOGGER.log(Level.INFO, "User without apiTokenProperty found id={0}", value.userId); - }else{ + } else { ApiTokenStore.HashedToken revokedToken = apiTokenProperty.getTokenStore().revokeToken(value.uuid); - if(revokedToken == null){ + if (revokedToken == null) { LOGGER.log(Level.INFO, "User without selected token id={0}, tokenUuid={1}", new Object[]{value.userId, value.uuid}); - }else{ + } else { apiTokenProperty.deleteApiToken(); user.save(); LOGGER.log(Level.INFO, "Revocation success for user id={0}, tokenUuid={1}", new Object[]{value.userId, value.uuid}); diff --git a/test/src/test/java/jenkins/security/apitoken/LegacyApiTokenAdministrativeMonitorTest.java b/test/src/test/java/jenkins/security/apitoken/LegacyApiTokenAdministrativeMonitorTest.java index 80172d9ab7..7fbea41d0b 100644 --- a/test/src/test/java/jenkins/security/apitoken/LegacyApiTokenAdministrativeMonitorTest.java +++ b/test/src/test/java/jenkins/security/apitoken/LegacyApiTokenAdministrativeMonitorTest.java @@ -38,6 +38,7 @@ import org.apache.commons.lang.StringUtils; import org.hamcrest.Matchers; import org.junit.Rule; import org.junit.Test; +import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; import static org.junit.Assert.assertEquals; @@ -51,6 +52,18 @@ public class LegacyApiTokenAdministrativeMonitorTest { @Rule public JenkinsRule j = new JenkinsRule(); + private enum SelectFilter { + ALL(0), + ONLY_FRESH(1), + ONLY_RECENT(2); + + int index; + + SelectFilter(int index) { + this.index = index; + } + } + @Test public void isActive() throws Exception { ApiTokenPropertyConfiguration config = ApiTokenPropertyConfiguration.get(); @@ -76,6 +89,40 @@ public class LegacyApiTokenAdministrativeMonitorTest { assertTrue(monitor.isActivated()); } + @Test + @Issue("JENKINS-52441") + public void takeCareOfUserWithIdNull() throws Exception { + ApiTokenPropertyConfiguration config = ApiTokenPropertyConfiguration.get(); + config.setCreationOfLegacyTokenEnabled(true); + config.setTokenGenerationOnCreationEnabled(false); + + // user created without legacy token + User user = User.getById("null", true); + ApiTokenProperty apiTokenProperty = user.getProperty(ApiTokenProperty.class); + assertFalse(apiTokenProperty.hasLegacyToken()); + + LegacyApiTokenAdministrativeMonitor monitor = j.jenkins.getExtensionList(AdministrativeMonitor.class).get(LegacyApiTokenAdministrativeMonitor.class); + assertFalse(monitor.isActivated()); + + apiTokenProperty.changeApiToken(); + assertTrue(monitor.isActivated()); + + {//revoke the legacy token + JenkinsRule.WebClient wc = j.createWebClient(); + + HtmlPage page = wc.goTo(monitor.getUrl() + "/manage"); + {// select all (only one user normally) + HtmlAnchor filterAll = getFilterByIndex(page, SelectFilter.ALL); + HtmlElementUtil.click(filterAll); + } + // revoke them + HtmlButton revokeSelected = getRevokeSelected(page); + HtmlElementUtil.click(revokeSelected); + } + + assertFalse(monitor.isActivated()); + } + @Test public void listOfUserWithLegacyTokenIsCorrect() throws Exception { j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); @@ -254,7 +301,7 @@ public class LegacyApiTokenAdministrativeMonitorTest { checkUserWithLegacyTokenListHasSizeOf(page, 1 + 2 + 3 + 4, 2 + 4, 3 + 4); {// select 2 - HtmlAnchor filterOnlyFresh = getFilterByIndex(page, 1); + HtmlAnchor filterOnlyFresh = getFilterByIndex(page, SelectFilter.ONLY_FRESH); HtmlElementUtil.click(filterOnlyFresh); } // revoke them @@ -265,7 +312,7 @@ public class LegacyApiTokenAdministrativeMonitorTest { assertTrue(monitor.isActivated()); {// select 1 + 3 - HtmlAnchor filterAll = getFilterByIndex(newPage, 0); + HtmlAnchor filterAll = getFilterByIndex(newPage, SelectFilter.ALL); HtmlElementUtil.click(filterAll); } // revoke them @@ -275,13 +322,13 @@ public class LegacyApiTokenAdministrativeMonitorTest { assertFalse(monitor.isActivated()); } - private HtmlAnchor getFilterByIndex(HtmlPage page, int index) { + private HtmlAnchor getFilterByIndex(HtmlPage page, SelectFilter selectFilter) { HtmlElement document = page.getDocumentElement(); HtmlDivision filterDiv = document.getOneHtmlElementByAttribute("div", "class", "selection-panel"); DomNodeList filters = filterDiv.getElementsByTagName("a"); assertEquals(3, filters.size()); - HtmlAnchor filter = (HtmlAnchor) filters.get(index); + HtmlAnchor filter = (HtmlAnchor) filters.get(selectFilter.index); assertNotNull(filter); return filter; } @@ -341,7 +388,7 @@ public class LegacyApiTokenAdministrativeMonitorTest { private void createUserWithToken(boolean legacy, boolean fresh, boolean recent) throws Exception { User user = User.getById(String.format("user %b %b %b %d", legacy, fresh, recent, nextId++), true); if (!legacy) { - return ; + return; } ApiTokenProperty apiTokenProperty = user.getProperty(ApiTokenProperty.class); -- GitLab