Delete invalid tokens in token cleanup process (bsc#1211874)

Signed-off-by: Ricardo Mateus <rmateus@suse.com>

java/code/src/com/redhat/rhn/common/db/datasource/xml/Task_queries.xml

@@ -303,6 +303,13 @@ SELECT id, channel_label, client, reason, force, bypass_filters, next_action
     </query>
 </write-mode>
 
+
+<write-mode name="taskomatic_token_cleanup">
+    <query>
+        DELETE FROM susechannelaccesstoken where expiration &lt; now() OR (valid = 'N' and minion_id is null)
+    </query>
+</write-mode>
+
 <mode name="taskomatic_task_status">
    <query params="">
    SELECT t.name, r.id, r.start_time, r.status

java/code/src/com/redhat/rhn/domain/channel/AccessToken.java

@@ -96,6 +96,9 @@ public void setMinion(MinionServer minionIn) {
      * @param validIn the new valid flag
      */
     public void setValid(boolean validIn) {
+        if (validIn && !this.valid && this.minion == null) {
+            throw new AccessTokenChangeException("Cannot set valid token when it's invalid and no minion is set");
+        }
         this.valid = validIn;
     }
 

java/code/src/com/redhat/rhn/domain/channel/AccessTokenChangeException.java

@@ -0,0 +1,47 @@
+/*
+ * Copyright (c) 2023 SUSE LLC
+ *
+ * This software is licensed to you under the GNU General Public License,
+ * version 2 (GPLv2). There is NO WARRANTY for this software, express or
+ * implied, including the implied warranties of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. You should have received a copy of GPLv2
+ * along with this software; if not, see
+ * http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt.
+ *
+ * Red Hat trademarks are not licensed under GPLv2. No permission is
+ * granted to use or replicate Red Hat trademarks that are incorporated
+ * in this software or its documentation.
+ */
+package com.redhat.rhn.domain.channel;
+
+import com.redhat.rhn.common.RhnRuntimeException;
+
+/**
+ * Invalid change to channel access token.
+ * <p>
+
+ *
+ */
+public class AccessTokenChangeException extends RhnRuntimeException  {
+
+
+     /**
+     * Constructor
+     * @param message exception message
+     */
+    public AccessTokenChangeException(String message) {
+        super(message);
+    }
+
+        /**
+     * Constructor
+     * @param message exception message
+     * @param cause the cause (which is saved for later retrieval
+     * by the Throwable.getCause() method). (A null value is
+     * permitted, and indicates that the cause is nonexistent or
+     * unknown.)
+     */
+    public AccessTokenChangeException(String message , Throwable cause) {
+        super(message, cause);
+    }
+}

java/code/src/com/redhat/rhn/domain/channel/AccessTokenFactory.java

@@ -14,8 +14,11 @@
  */
 package com.redhat.rhn.domain.channel;
 
+import com.redhat.rhn.common.db.datasource.ModeFactory;
+import com.redhat.rhn.common.db.datasource.WriteMode;
 import com.redhat.rhn.common.hibernate.HibernateFactory;
 import com.redhat.rhn.domain.server.MinionServer;
+import com.redhat.rhn.taskomatic.task.TaskConstants;
 
 import com.suse.manager.webui.utils.DownloadTokenBuilder;
 import com.suse.utils.Opt;
@@ -32,6 +35,7 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Date;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
@@ -213,13 +217,25 @@ public static boolean refreshTokens(MinionServer minion, Collection<AccessToken>
      * Deletes unassigned expired AccessTokens.
      */
     public static void cleanupUnusedExpired() {
-        Instant now = Instant.now();
-        all().forEach(token -> {
-            if (token.getMinion() == null &&
-                    now.isAfter(token.getExpiration().toInstant())) {
-                delete(token);
-            }
-        });
+        WriteMode m = ModeFactory.getWriteMode(TaskConstants.MODE_NAME,
+                TaskConstants.TASK_QUERY_TOKEN_CLEANUP);
+
+        if (log.isDebugEnabled()) {
+            log.debug("Executing WriteMode " + TaskConstants.MODE_NAME + "::" +
+                    TaskConstants.TASK_QUERY_TOKEN_CLEANUP);
+        }
+        int tokensDeleted = m.executeUpdate(new HashMap<>());
+        if (log.isDebugEnabled()) {
+            log.debug("WriteMode " + TaskConstants.MODE_NAME + "::" +
+                    TaskConstants.TASK_QUERY_TOKEN_CLEANUP + " returned");
+        }
+        //logs number of tokens deleted
+        if (tokensDeleted > 0) {
+            log.info("{} channel access tokens deleted", tokensDeleted);
+        }
+        else {
+            log.debug("No tokens to be deleted");
+        }
    }
 
     /**
@@ -295,6 +311,7 @@ public static AccessToken regenerate(AccessToken token) throws JoseException {
 
         // Unlink the old token
         token.setMinion(null);
+        token.setValid(false);
         AccessTokenFactory.save(token);
 
         return newToken;

java/code/src/com/redhat/rhn/domain/channel/test/AccessTokenFactoryTest.java

@@ -73,6 +73,41 @@ public void testCleanupNotDeletingChannels() throws Exception {
         assertEquals(initialChannelCount + 1, ChannelFactory.listAllBaseChannels().size());
     }
 
+
+    @Test
+    public void testCleanupNoMinionInvalid() throws Exception {
+
+
+        AccessToken valid = new AccessToken();
+        valid.setExpiration(Date.from(Instant.now().plus(Duration.ofDays(1))));
+        valid.setStart(Date.from(Instant.now()));
+        valid.setToken("valid1");
+        AccessTokenFactory.save(valid);
+
+        AccessToken withMinionInvalid = new AccessToken();
+        withMinionInvalid.setExpiration(Date.from(Instant.now().plus(Duration.ofDays(1))));
+        withMinionInvalid.setStart(Date.from(Instant.now()));
+        MinionServer minion = MinionServerFactoryTest.createTestMinionServer(user);
+        withMinionInvalid.setMinion(minion);
+        withMinionInvalid.setValid(false);
+        withMinionInvalid.setToken("valid2");
+        AccessTokenFactory.save(withMinionInvalid);
+
+        AccessToken noMinionInvalid = new AccessToken();
+        noMinionInvalid.setExpiration(Date.from(Instant.now().plus(Duration.ofDays(1))));
+        noMinionInvalid.setStart(Date.from(Instant.now()));
+        noMinionInvalid.setValid(false);
+        noMinionInvalid.setToken("valid3");
+        AccessTokenFactory.save(noMinionInvalid);
+
+        assertEquals(3, AccessTokenFactory.all().size());
+        AccessTokenFactory.cleanupUnusedExpired();
+        List<AccessToken> all = AccessTokenFactory.all();
+        assertEquals(2, all.size());
+        assertTrue(all.stream().anyMatch(t -> t.getToken().equals("valid1")));
+        assertTrue(all.stream().anyMatch(t -> t.getToken().equals("valid2")));
+    }
+
     @Test
     public void testCleanupExpired() {
         AccessToken valid = new AccessToken();

java/code/src/com/redhat/rhn/manager/system/SystemManager.java

@@ -756,6 +756,7 @@ public void deleteServer(User user, Long sid, boolean deleteSaltKey) {
 
         server.asMinionServer().ifPresent(minion -> minion.getAccessTokens().forEach(token -> {
             token.setValid(false);
+            token.setMinion(null);
             AccessTokenFactory.save(token);
         }));
 

java/code/src/com/redhat/rhn/taskomatic/task/TaskConstants.java

@@ -43,6 +43,9 @@ public class TaskConstants {
     public static final String TASK_QUERY_SESSION_CLEANUP =
         "taskomatic_session_cleanup";
 
+    public static final String TASK_QUERY_TOKEN_CLEANUP =
+        "taskomatic_token_cleanup";
+
     public static final String TASK_QUERY_PACKAGE_CHANGELOG_CLEANUP =
         "taskomatic_package_changelog_cleanup";
 

java/code/src/com/suse/manager/utils/SaltUtils.java

@@ -828,6 +828,7 @@ private void handleSubscribeChannels(ServerAction serverAction, JsonElement json
             SubscribeChannelsAction sca = (SubscribeChannelsAction)action;
             sca.getDetails().getAccessTokens().forEach(token -> {
                 token.setValid(false);
+                token.setMinion(null);
                 AccessTokenFactory.save(token);
             });
 

java/code/src/com/suse/manager/webui/controllers/DownloadController.java

@@ -51,6 +51,7 @@
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.security.Key;
+import java.time.Instant;
 import java.util.Arrays;
 import java.util.Base64;
 import java.util.List;
@@ -507,11 +508,15 @@ private static String getTokenForDebian(Request request) {
      */
     private static void validateToken(String token, String channel, String filename) {
 
-        AccessTokenFactory.lookupByToken(token).ifPresent(obj -> {
-            if (!obj.getValid()) {
+        AccessTokenFactory.lookupByToken(token).ifPresentOrElse(obj -> {
+            Instant now = Instant.now();
+            if (!obj.getValid() || now.isAfter(obj.getExpiration().toInstant())) {
                 LOG.info(String.format("Forbidden: invalid token %s to access %s", token, filename));
                 halt(HttpStatus.SC_FORBIDDEN, "This token is not valid");
             }
+        }, () -> {
+            LOG.info(String.format("Forbidden: token %s to access %s doesn't exists in the database", token, filename));
+            halt(HttpStatus.SC_FORBIDDEN, "This token is not valid");
         });
         try {
             JwtClaims claims = JWT_CONSUMER.processToClaims(token);

java/code/src/com/suse/manager/webui/controllers/test/DownloadControllerTest.java

@@ -52,12 +52,14 @@
 
 import com.suse.manager.webui.controllers.DownloadController;
 import com.suse.manager.webui.utils.DownloadTokenBuilder;
+import com.suse.manager.webui.utils.TokenBuilder;
 
 import com.mockobjects.servlet.MockHttpServletResponse;
 
 import org.apache.commons.codec.digest.DigestUtils;
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.FilenameUtils;
+import org.jose4j.lang.JoseException;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
@@ -67,10 +69,13 @@
 import java.io.IOException;
 import java.net.URI;
 import java.nio.file.Files;
+import java.time.Instant;
+import java.time.temporal.ChronoUnit;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Base64;
 import java.util.Collections;
+import java.util.Date;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -180,6 +185,24 @@ public void tearDown() throws Exception {
         Files.deleteIfExists(debPackageFile2.toPath());
     }
 
+    /**
+     * helper method to save a token to the database
+     * @param tokenBuilder
+     * @return the access token database object
+     * @throws JoseException if an error happens during token build
+     */
+    private AccessToken saveTokenToDataBase(TokenBuilder tokenBuilder) throws JoseException {
+        AccessToken newToken = new AccessToken();
+        newToken.setStart(Date.from(tokenBuilder.getIssuedAt()));
+        newToken.setToken(tokenBuilder.getToken());
+        Instant expiration = tokenBuilder.getIssuedAt()
+                .plus(tokenBuilder.getExpirationTimeMinutesInTheFuture(),
+                        ChronoUnit.MINUTES);
+        newToken.setExpiration(Date.from(expiration));
+        TestUtils.saveAndFlush(newToken);
+        return newToken;
+    }
+
     /**
      * Helper method for creating a Spark Request with parameters.
      *
@@ -531,7 +554,8 @@ private void testCorrectChannel(Supplier<File> pkgFile, Function<String, Request
         tokenBuilder.onlyChannels(
                 new HashSet<>(
                         Arrays.asList(channel.getLabel())));
-        String tokenChannel = tokenBuilder.getToken();
+        AccessToken accessToken = saveTokenToDataBase(tokenBuilder);
+        String tokenChannel = accessToken.getToken();
 
         Request request = requestFactory.apply(tokenChannel);
         try {
@@ -552,7 +576,8 @@ public void testDownloadPackageWithSpecialCharacters() throws Exception {
         DownloadTokenBuilder tokenBuilder = new DownloadTokenBuilder(user.getOrg().getId());
         tokenBuilder.useServerSecret();
         tokenBuilder.onlyChannels(new HashSet<>(Arrays.asList(channel.getLabel())));
-        String tokenChannel = tokenBuilder.getToken();
+        AccessToken accessToken = saveTokenToDataBase(tokenBuilder);
+        String tokenChannel = accessToken.getToken();
 
         Map<String, String> params = new HashMap<>();
         params.put(tokenChannel, "");
@@ -579,7 +604,8 @@ public void testDownloadPackageWithSpecialCharacters() throws Exception {
     public void testCorrectOrg() throws Exception {
         DownloadTokenBuilder tokenBuilder = new DownloadTokenBuilder(user.getOrg().getId());
         tokenBuilder.useServerSecret();
-        String tokenOrg = tokenBuilder.getToken();
+        AccessToken accessToken = saveTokenToDataBase(tokenBuilder);
+        String tokenOrg = accessToken.getToken();
 
         Map<String, String> params = new HashMap<>();
         params.put(tokenOrg, "");
@@ -609,7 +635,8 @@ public void testExpiredToken() throws Exception {
         tokenBuilder.useServerSecret();
         // already expired
         tokenBuilder.setExpirationTimeMinutesInTheFuture(-1);
-        String expiredToken = tokenBuilder.getToken();
+        AccessToken accessToken = saveTokenToDataBase(tokenBuilder);
+        String expiredToken = accessToken.getToken();
 
         Map<String, String> params = new HashMap<>();
         params.put(expiredToken, "");
@@ -622,7 +649,7 @@ public void testExpiredToken() throws Exception {
         }
         catch (spark.HaltException e) {
             assertEquals(403, e.getStatusCode());
-            assertTrue(e.getBody().contains("The JWT is no longer valid"));
+            assertTrue(e.getBody().contains("This token is not valid"));
             assertNull(response.raw().getHeader("X-Sendfile"));
         }
     }
@@ -636,7 +663,8 @@ public void testExpiredToken() throws Exception {
     public void testDownloadComps() throws Exception {
         DownloadTokenBuilder tokenBuilder = new DownloadTokenBuilder(user.getOrg().getId());
         tokenBuilder.useServerSecret();
-        String tokenOrg = tokenBuilder.getToken();
+        AccessToken accessToken = saveTokenToDataBase(tokenBuilder);
+        String tokenOrg = accessToken.getToken();
 
         Map<String, String> params = new HashMap<>();
         params.put(tokenOrg, "");
@@ -689,7 +717,8 @@ public void testDownloadComps() throws Exception {
     public void testDownloadModules() throws Exception {
         DownloadTokenBuilder tokenBuilder = new DownloadTokenBuilder(user.getOrg().getId());
         tokenBuilder.useServerSecret();
-        String tokenOrg = tokenBuilder.getToken();
+        AccessToken accessToken = saveTokenToDataBase(tokenBuilder);
+        String tokenOrg = accessToken.getToken();
 
         Map<String, String> params = new HashMap<>();
         params.put(tokenOrg, "");
@@ -741,7 +770,8 @@ public void testDownloadModules() throws Exception {
     public void testDownloadMediaProducts() throws Exception {
         DownloadTokenBuilder tokenBuilder = new DownloadTokenBuilder(user.getOrg().getId());
         tokenBuilder.useServerSecret();
-        String tokenOrg = tokenBuilder.getToken();
+        AccessToken accessToken = saveTokenToDataBase(tokenBuilder);
+        String tokenOrg = accessToken.getToken();
 
         Map<String, String> params = new HashMap<>();
         params.put(tokenOrg, "");
@@ -794,7 +824,8 @@ public void testDownloadMediaProducts() throws Exception {
     public void testDownloadMissingFile() throws Exception {
         DownloadTokenBuilder tokenBuilder = new DownloadTokenBuilder(user.getOrg().getId());
         tokenBuilder.useServerSecret();
-        String tokenOrg = tokenBuilder.getToken();
+        AccessToken accessToken = saveTokenToDataBase(tokenBuilder);
+        String tokenOrg = accessToken.getToken();
 
         Map<String, String> params = new HashMap<>();
         params.put(tokenOrg, "");

java/code/src/com/suse/manager/webui/services/impl/SaltService.java

@@ -1001,7 +1001,7 @@ public <T> Optional<LocalAsyncResult<T>> callAsync(LocalCall<T> callIn, Target<?
      */
     @Override
     public void deployChannels(List<String> minionIds) throws SaltException {
-        callSync(
+        callAsync(
                 com.suse.salt.netapi.calls.modules.State.apply(ApplyStatesEventMessage.CHANNELS),
                 new MinionList(minionIds));
     }

java/spacewalk-java.changes.rmateus.m43_token_clean_up

@@ -0,0 +1 @@
+- Token cleanup process removing invalid tokens using sql query (bsc#1211675)