Flaw in token storing logic when configured to allow token renewal after expiry

There seems to be a fundamental flaw in the STS logic for token renewal when configuration allows for renewal beyond token expiration.

The flaw in logic seems to be in both the SAMLTokenProvider and the SAMLTokenRenewer. In brief, when tokens are added to the default token store, and when allowRenewalAfterExipiry is set to true, the tokens are stored in a way that causes them to expire before you can perform a second token renewal.

This reason why this flaw does not show up in your system tests is because you are only testing one renewal and the logic for storing a token in SAMLTokenProvider is different than in SAMLTokenRenewer. If you would change your renewal system test to renew the renewed token you would catch this error.

Here is the relevant code. I will start with SAMLTokenProvider. In the method createToken:

if (tokenParameters.getTokenStore() != null && signatureValue != null
&& signatureValue.length > 0) {
Date expires = new Date();
long currentTime = expires.getTime();
expires.setTime(currentTime + (conditionsProvider.getLifetime() * 1000L));

SecurityToken securityToken = new SecurityToken(assertion.getId(), null, expires);

The token is stored with an expiration that is essentially the current time + the lifetime parameter. This means the token is cleared out of the cache after a period of time equal to the "lifetime" setting. If the lifetime setting is larger than token TTL in the request submitted by the STSClient (like in your system test) then renewal will be possible because the token will not get cleared out of the cache before you renew.

However, consider the logic for storing the token in SAMLRenewer.storeTokenInCache():

if (tokenStore != null && signatureValue != null && signatureValue.length > 0) {
DateTime validTill = null;
if (assertion.getSamlVersion().equals(SAMLVersion.VERSION_20))

{ validTill = assertion.getSaml2().getConditions().getNotOnOrAfter(); }

else

{ validTill = assertion.getSaml1().getConditions().getNotOnOrAfter(); }

SecurityToken securityToken = new SecurityToken(assertion.getId(), null, validTill.toDate());

In this case you are setting the token expiration to be exactly equal to the TTL in the request for security token. Which means that when the token expires and you try to renew the token it's no longer in the cache!

I believe what should be happening is that you should add the value of the SAMLTokenRenewer maxExpiry field to the "validTill" date. Also the logic in SAMLTokenRenewer should be the same as the logic in SAMLTokenProvider. Right now the logic is duplicated in two different places and that can't be good for maintainability.

I hope this is clear enough. The bottom line is that the token cache is using the expiry that you are setting when creating the SecurityToken object to decide when to clear the token out of the cache. If you don't set this value appropriately the token will not be in the cache when you try to renew it.