Skip to content

Commit

Permalink
Fixed a bug where the check for token/credentials expiration time was…
Browse files Browse the repository at this point in the history
… incorrect
  • Loading branch information
bhvkshah committed Oct 23, 2023
1 parent aafc726 commit f1f35b3
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 22 deletions.
3 changes: 2 additions & 1 deletion src/main/java/com/amazon/redshift/CredentialsHolder.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import java.util.Date;

import com.amazon.redshift.plugin.utils.RequestUtils;
import com.amazonaws.auth.AWSCredentials;
import com.amazonaws.auth.AWSSessionCredentials;

Expand Down Expand Up @@ -70,7 +71,7 @@ public String getAWSSecretKey()

public boolean isExpired()
{
return m_expiration != null && m_expiration.before(new Date(System.currentTimeMillis() - 60 * 1000 * 5));
return RequestUtils.isCredentialExpired(m_expiration);
}

public Date getExpiration()
Expand Down
5 changes: 3 additions & 2 deletions src/main/java/com/amazon/redshift/NativeTokenHolder.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package com.amazon.redshift;

import com.amazon.redshift.plugin.utils.RequestUtils;

import java.util.Date;


Expand Down Expand Up @@ -31,8 +33,7 @@ public static NativeTokenHolder newInstance(String accessToken, Date expiration)

public boolean isExpired()
{
return (m_expiration == null)
|| (m_expiration != null && m_expiration.before(new Date(System.currentTimeMillis() - 60 * 1000 * 5)));
return RequestUtils.isCredentialExpired(m_expiration);
}

public String getAccessToken()
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/com/amazon/redshift/core/IamHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ else if (RedshiftProperty.DB_GROUPS_FILTER.getName().equalsIgnoreCase(pluginArgK
}

if (credentials == null
|| credentials.getExpiration().before(new Date(System.currentTimeMillis() - 60 * 1000 * 5))) {
|| RequestUtils.isCredentialExpired(credentials.getExpiration())) {
// If not found or expired
// Get IDP token
if (providerType == CredentialProviderType.PLUGIN) {
Expand Down Expand Up @@ -806,7 +806,7 @@ private static synchronized GetClusterCredentialsResult getClusterCredentialsRes
}

if (credentials == null || (providerType == CredentialProviderType.PLUGIN && idpCredentialsRefresh)
|| credentials.getExpiration().before(new Date(System.currentTimeMillis() - 60 * 1000 * 5))) {
|| RequestUtils.isCredentialExpired(credentials.getExpiration())) {
if (RedshiftLogger.isEnable())
log.logInfo("GetClusterCredentials NOT from cache");

Expand Down Expand Up @@ -940,7 +940,7 @@ private static synchronized GetClusterCredentialsWithIAMResult getClusterCredent
}

if (credentials == null || (providerType == CredentialProviderType.PLUGIN && settings.m_idpToken != null)
|| credentials.getExpiration().before(new Date(System.currentTimeMillis() - 60 * 1000 * 5)))
|| RequestUtils.isCredentialExpired(credentials.getExpiration()))
{
if (RedshiftLogger.isEnable())
log.logInfo("GetClusterCredentialsV2 NOT from cache");
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.amazon.redshift.core;

import com.amazon.redshift.plugin.utils.RequestUtils;
import com.amazon.redshift.util.RedshiftProperties;
import com.amazonaws.util.StringUtils;
import com.amazon.redshift.INativePlugin;
Expand Down Expand Up @@ -142,12 +143,7 @@ private static String getNativeAuthPluginCredentials(RedshiftJDBCSettings settin
// here.
NativeTokenHolder credentials = provider.getCredentials();

if (credentials == null
||
(credentials.getExpiration() != null
&& credentials.getExpiration().before(new Date(System.currentTimeMillis() - 60 * 1000 * 5))
)
) {
if (credentials == null || RequestUtils.isCredentialExpired(credentials.getExpiration())) {
// If not found or expired
// Get IDP token
IPlugin plugin = (IPlugin) provider;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import com.amazon.redshift.core.IamHelper.CredentialProviderType;
import com.amazon.redshift.logger.RedshiftLogger;
import com.amazon.redshift.plugin.utils.RequestUtils;
import com.amazonaws.AmazonClientException;
import com.amazonaws.auth.AWSCredentialsProvider;
import com.amazonaws.services.redshiftserverless.AWSRedshiftServerlessClient;
Expand Down Expand Up @@ -78,7 +79,7 @@ synchronized void getCredentialsResult(RedshiftJDBCSettings settings,
if (credentials == null
|| (providerType == CredentialProviderType.PLUGIN
&& idpCredentialsRefresh)
|| credentials.getExpiration().before(new Date(System.currentTimeMillis() - 60 * 1000 * 5)))
|| RequestUtils.isCredentialExpired(credentials.getExpiration()))
{
if (RedshiftLogger.isEnable())
log.logInfo("GetCredentials NOT from cache");
Expand Down
18 changes: 9 additions & 9 deletions src/main/java/com/amazon/redshift/jdbc/RedshiftSQLXML.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import java.sql.SQLException;
import java.sql.SQLXML;

import javax.xml.XMLConstants;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.stream.XMLInputFactory;
Expand Down Expand Up @@ -205,11 +206,10 @@ public synchronized <T extends Result> T setResult(Class<T> resultClass) throws
(SAXTransformerFactory) SAXTransformerFactory.newInstance();

// https://www.aristotle.a2z.com/implementations/255
transformerFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
transformerFactory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
transformerFactory.setFeature("http://xml.org/sax/features/external-general-entities", false);
transformerFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd",false);


transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");

TransformerHandler transformerHandler = transformerFactory.newTransformerHandler();
stringWriter = new StringWriter();
transformerHandler.setResult(new StreamResult(stringWriter));
Expand Down Expand Up @@ -293,10 +293,10 @@ private void ensureInitialized() throws SQLException {

// Disable External Entities (XXE) parsing for Java
// https://www.aristotle.a2z.com/implementations/255
factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);

factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");

Transformer transformer = factory.newTransformer();
DOMSource domSource = new DOMSource(domResult.getNode());
StringWriter stringWriter = new StringWriter();
Expand Down
13 changes: 13 additions & 0 deletions src/main/java/com/amazon/redshift/plugin/utils/RequestUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import com.amazonaws.services.securitytoken.AWSSecurityTokenServiceClientBuilder;

import java.net.URL;
import java.util.Date;

/**
* Http Request utils.
Expand Down Expand Up @@ -112,4 +113,16 @@ private static boolean isCustomStsEndpointUrl(String stsEndpoint) throws Excepti

return isCustomStsEndPoint;
}

/*
* Checks expiry for credential.
* Note that this method returns true (i.e. credential is "expired") 1 minute before actual expiry time - This
* arbitrary buffer has been added to accommodate corner cases and allow enough time for retries if implemented.
*
* Returns true (i.e. credential is "expired") if expiry time is null.
*/
public static boolean isCredentialExpired(Date expiryTime) {
// We preemptively conclude the credential as expired 1 minute before actual expiry.
return expiryTime==null || expiryTime.before(new Date(System.currentTimeMillis() + 1000 * 60));
}
}

0 comments on commit f1f35b3

Please sign in to comment.