[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

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});

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<HtmlElement> 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);