diff --git a/java/code/src/com/redhat/rhn/common/db/datasource/xml/Task_queries.xml b/java/code/src/com/redhat/rhn/common/db/datasource/xml/Task_queries.xml index 0bae946bf196..cb7843a7e1af 100644 --- a/java/code/src/com/redhat/rhn/common/db/datasource/xml/Task_queries.xml +++ b/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 + + + + DELETE FROM susechannelaccesstoken where expiration < now() OR (valid = 'N' and minion_id is null) + + + SELECT t.name, r.id, r.start_time, r.status diff --git a/java/code/src/com/redhat/rhn/domain/channel/AccessToken.java b/java/code/src/com/redhat/rhn/domain/channel/AccessToken.java index 7dfd87036cfe..a689a897d8b6 100644 --- a/java/code/src/com/redhat/rhn/domain/channel/AccessToken.java +++ b/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; } diff --git a/java/code/src/com/redhat/rhn/domain/channel/AccessTokenChangeException.java b/java/code/src/com/redhat/rhn/domain/channel/AccessTokenChangeException.java new file mode 100644 index 000000000000..66ba668ce299 --- /dev/null +++ b/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. + *

+ + * + */ +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); + } +} diff --git a/java/code/src/com/redhat/rhn/domain/channel/AccessTokenFactory.java b/java/code/src/com/redhat/rhn/domain/channel/AccessTokenFactory.java index 26f3e731d9e1..b0973e164d92 100644 --- a/java/code/src/com/redhat/rhn/domain/channel/AccessTokenFactory.java +++ b/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 * 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; diff --git a/java/code/src/com/redhat/rhn/domain/channel/test/AccessTokenFactoryTest.java b/java/code/src/com/redhat/rhn/domain/channel/test/AccessTokenFactoryTest.java index 812d866b83cf..5706c2871c78 100644 --- a/java/code/src/com/redhat/rhn/domain/channel/test/AccessTokenFactoryTest.java +++ b/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 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(); diff --git a/java/code/src/com/redhat/rhn/manager/system/SystemManager.java b/java/code/src/com/redhat/rhn/manager/system/SystemManager.java index 0309e22e27c9..448cd59a8685 100644 --- a/java/code/src/com/redhat/rhn/manager/system/SystemManager.java +++ b/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); })); diff --git a/java/code/src/com/redhat/rhn/taskomatic/task/TaskConstants.java b/java/code/src/com/redhat/rhn/taskomatic/task/TaskConstants.java index d8008b0c4f09..c76821874be3 100644 --- a/java/code/src/com/redhat/rhn/taskomatic/task/TaskConstants.java +++ b/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"; diff --git a/java/code/src/com/suse/manager/utils/SaltUtils.java b/java/code/src/com/suse/manager/utils/SaltUtils.java index a70718c1c2e4..06c649eb54de 100644 --- a/java/code/src/com/suse/manager/utils/SaltUtils.java +++ b/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); }); diff --git a/java/code/src/com/suse/manager/webui/controllers/DownloadController.java b/java/code/src/com/suse/manager/webui/controllers/DownloadController.java index e7dda1ef8960..4e997e8ba55c 100644 --- a/java/code/src/com/suse/manager/webui/controllers/DownloadController.java +++ b/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); diff --git a/java/code/src/com/suse/manager/webui/controllers/test/DownloadControllerTest.java b/java/code/src/com/suse/manager/webui/controllers/test/DownloadControllerTest.java index e09ac0a4e01a..0f9659f2b36f 100644 --- a/java/code/src/com/suse/manager/webui/controllers/test/DownloadControllerTest.java +++ b/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 pkgFile, Function( 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 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 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 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 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 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 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 params = new HashMap<>(); params.put(tokenOrg, ""); diff --git a/java/code/src/com/suse/manager/webui/services/impl/SaltService.java b/java/code/src/com/suse/manager/webui/services/impl/SaltService.java index 1d7cf1bb3238..ac53a1b3c7a1 100644 --- a/java/code/src/com/suse/manager/webui/services/impl/SaltService.java +++ b/java/code/src/com/suse/manager/webui/services/impl/SaltService.java @@ -1001,7 +1001,7 @@ public Optional> callAsync(LocalCall callIn, Target minionIds) throws SaltException { - callSync( + callAsync( com.suse.salt.netapi.calls.modules.State.apply(ApplyStatesEventMessage.CHANNELS), new MinionList(minionIds)); } diff --git a/java/spacewalk-java.changes.rmateus.m43_token_clean_up b/java/spacewalk-java.changes.rmateus.m43_token_clean_up new file mode 100644 index 000000000000..e3df23fe6d69 --- /dev/null +++ b/java/spacewalk-java.changes.rmateus.m43_token_clean_up @@ -0,0 +1 @@ +- Token cleanup process removing invalid tokens using sql query (bsc#1213376)