From 3c6f0a885ce427b9fbeab0b57f3275cdc4bbe5f3 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Wed, 7 Oct 2026 16:48:41 +0200 Subject: [PATCH] saml session: compare timestamp according to the timezone it has been created with --- .../cloudstack/saml/SAMLTokenDaoImpl.java | 11 ++- .../cloudstack/saml/SAMLTokenDaoImplTest.java | 85 +++++++++++++++++++ 2 files changed, 94 insertions(+), 2 deletions(-) create mode 100644 plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/saml/SAMLTokenDaoImplTest.java diff --git a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAMLTokenDaoImpl.java b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAMLTokenDaoImpl.java index 20094c72e5a3..4c19fefd3db2 100644 --- a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAMLTokenDaoImpl.java +++ b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAMLTokenDaoImpl.java @@ -16,6 +16,7 @@ // under the License. package org.apache.cloudstack.saml; +import com.cloud.utils.DateUtil; import com.cloud.utils.db.DB; import com.cloud.utils.db.GenericDaoBase; import com.cloud.utils.db.TransactionLegacy; @@ -23,11 +24,16 @@ import org.springframework.stereotype.Component; import java.sql.PreparedStatement; +import java.sql.Timestamp; +import java.util.concurrent.TimeUnit; @DB @Component public class SAMLTokenDaoImpl extends GenericDaoBase implements SAMLTokenDao { + protected static final String EXPIRE_TOKENS_SQL = "DELETE FROM `saml_token` WHERE `created` < ?"; + protected static final long TOKEN_LIFETIME_MILLIS = TimeUnit.HOURS.toMillis(1); + public SAMLTokenDaoImpl() { super(); } @@ -37,8 +43,9 @@ public void expireTokens() { TransactionLegacy txn = TransactionLegacy.currentTxn(); try { txn.start(); - String sql = "DELETE FROM `saml_token` WHERE `created` < (NOW() - INTERVAL 1 HOUR)"; - PreparedStatement pstmt = txn.prepareAutoCloseStatement(sql); + Timestamp cutOff = new Timestamp(DateUtil.currentGMTTime().getTime() - TOKEN_LIFETIME_MILLIS); + PreparedStatement pstmt = txn.prepareAutoCloseStatement(EXPIRE_TOKENS_SQL); + pstmt.setTimestamp(1, cutOff, gmtCalendar()); pstmt.executeUpdate(); txn.commit(); } catch (Exception e) { diff --git a/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/saml/SAMLTokenDaoImplTest.java b/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/saml/SAMLTokenDaoImplTest.java new file mode 100644 index 000000000000..788c736fc811 --- /dev/null +++ b/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/saml/SAMLTokenDaoImplTest.java @@ -0,0 +1,85 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.saml; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.sql.PreparedStatement; +import java.sql.Timestamp; +import java.util.Calendar; +import java.util.TimeZone; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.MockedStatic; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnitRunner; + +import com.cloud.utils.db.TransactionLegacy; + +@RunWith(MockitoJUnitRunner.class) +public class SAMLTokenDaoImplTest { + + @Mock + private TransactionLegacy transactionMock; + + @Mock + private PreparedStatement preparedStatementMock; + + private final SAMLTokenDaoImpl samlTokenDao = new SAMLTokenDaoImpl(); + + @Test + public void testExpireTokensUsesBoundGmtCutOff() throws Exception { + TimeZone defaultTimeZone = TimeZone.getDefault(); + // a JVM/DB local zone ahead of UTC is where NOW() deleted fresh tokens + TimeZone.setDefault(TimeZone.getTimeZone("Europe/Amsterdam")); + try (MockedStatic ignored = Mockito.mockStatic(TransactionLegacy.class)) { + ignored.when(TransactionLegacy::currentTxn).thenReturn(transactionMock); + when(transactionMock.prepareAutoCloseStatement(anyString())).thenReturn(preparedStatementMock); + + long before = System.currentTimeMillis(); + samlTokenDao.expireTokens(); + long after = System.currentTimeMillis(); + + ArgumentCaptor sqlCaptor = ArgumentCaptor.forClass(String.class); + verify(transactionMock).prepareAutoCloseStatement(sqlCaptor.capture()); + assertFalse("cut-off must not depend on the DB session time zone", + sqlCaptor.getValue().toUpperCase().contains("NOW()")); + + ArgumentCaptor cutOffCaptor = ArgumentCaptor.forClass(Timestamp.class); + ArgumentCaptor calendarCaptor = ArgumentCaptor.forClass(Calendar.class); + verify(preparedStatementMock).setTimestamp(eq(1), cutOffCaptor.capture(), calendarCaptor.capture()); + verify(preparedStatementMock).executeUpdate(); + verify(transactionMock).commit(); + + long cutOff = cutOffCaptor.getValue().getTime(); + assertTrue(cutOff >= before - SAMLTokenDaoImpl.TOKEN_LIFETIME_MILLIS); + assertTrue(cutOff <= after - SAMLTokenDaoImpl.TOKEN_LIFETIME_MILLIS); + assertEquals(0, calendarCaptor.getValue().getTimeZone().getRawOffset()); + } finally { + TimeZone.setDefault(defaultTimeZone); + } + } +}