Consistent expiration handling

src/IdentityServer4/Extensions/DateTimeExtensions.cs

@@ -36,7 +36,7 @@ public static bool HasExpired(this DateTime? expirationTime)
         [DebuggerStepThrough]
         public static bool HasExpired(this DateTime expirationTime)
         {
-            if (expirationTime < IdentityServerDateTime.UtcNow)
+            if (IdentityServerDateTime.UtcNow > expirationTime)
             {
                 return true;
             }

src/IdentityServer4/Services/DefaultConsentService.cs

@@ -75,7 +75,19 @@ public virtual async Task<bool> RequiresConsentAsync(ClaimsPrincipal subject, Cl
             }
 
             var consent = await _userConsentStore.GetUserConsentAsync(subject.GetSubjectId(), client.ClientId);
-            if (consent?.Scopes != null)
+
+            if (consent == null)
+            {
+                return true;
+            }
+
+            if (consent.Expiration.HasExpired())
+            {
+                await _userConsentStore.RemoveUserConsentAsync(consent.SubjectId, consent.ClientId);
+                return true;
+            }
+
+            if (consent.Scopes != null)
             {
                 var intersect = scopes.Intersect(consent.Scopes);
                 return !(scopes.Count() == intersect.Count());

src/IdentityServer4/Stores/Default/DefaultGrantStore.cs

@@ -72,20 +72,13 @@ protected async Task<T> GetItemAsync(string key)
             var grant = await _store.GetAsync(hashedKey);
             if (grant != null && grant.Type == _grantType)
             {
-                if (!grant.Expiration.HasExpired())
+                try
                 {
-                    try
-                    {
-                        return _serializer.Deserialize<T>(grant.Data);
-                    }
-                    catch (Exception ex)
-                    {
-                        _logger.LogError("Failed to deserailize JSON from grant store. Exception: {0}", ex.Message);
-                    }
+                    return _serializer.Deserialize<T>(grant.Data);
                 }
-                else
+                catch (Exception ex)
                 {
-                    _logger.LogDebug("{grantType} grant with value: {key} found in store, but has expired.", _grantType, key);
+                    _logger.LogError("Failed to deserailize JSON from grant store. Exception: {0}", ex.Message);
                 }
             }
             else

src/IdentityServer4/Validation/TokenValidator.cs

@@ -277,7 +277,7 @@ private async Task<TokenValidationResult> ValidateReferenceAccessTokenAsync(stri
                 return Invalid(OidcConstants.ProtectedResourceErrors.InvalidToken);
             }
 
-            if (IdentityServerDateTime.UtcNow >= token.CreationTime.AddSeconds(token.Lifetime))
+            if (token.CreationTime.HasExceeded(token.Lifetime))
             {
                 LogError("Token expired.");
 

test/IdentityServer.UnitTests/Services/Default/DefaultConsentServiceTests.cs

@@ -8,6 +8,7 @@
 using IdentityServer4.Models;
 using IdentityServer4.Services;
 using IdentityServer4.UnitTests.Common;
+using System;
 using System.Linq;
 using System.Security.Claims;
 using System.Threading.Tasks;
@@ -149,5 +150,22 @@ public async Task RequiresConsentAsync_prior_consent_with_too_few_scopes_should_
             result.Should().BeTrue();
         }
 
+        [Fact]
+        public async Task RequiresConsentAsync_expired_consent_should_require_consent()
+        {
+            var scopes = new string[] { "foo", "bar" };
+            await _userConsentStore.StoreUserConsentAsync(new Consent()
+            {
+                ClientId = _client.ClientId,
+                SubjectId = _user.GetSubjectId(),
+                Scopes = scopes,
+                CreationTime = DateTime.UtcNow.AddHours(-1),
+                Expiration = DateTime.UtcNow.AddSeconds(-1)
+            });
+
+            var result = await _subject.RequiresConsentAsync(_user, _client, scopes);
+
+            result.Should().BeTrue();
+        }
     }
 }

test/IdentityServer.UnitTests/Stores/Default/DefaultPersistedGrantStoreTests.cs

@@ -97,26 +97,6 @@ public async Task RemoveAuthorizationCodeAsync_should_remove_grant()
             code2.Should().BeNull();
         }
 
-        [Fact]
-        public async Task expired_code_should_not_load()
-        {
-            var code1 = new AuthorizationCode()
-            {
-                ClientId = "test",
-                CreationTime = DateTime.UtcNow.AddHours(-1),
-                Lifetime = 10,
-                Subject = _user,
-                CodeChallenge = "challenge",
-                RedirectUri = "http://client/cb",
-                Nonce = "nonce",
-                RequestedScopes = new string[] { "scope1", "scope2" }
-            };
-            var handle = await _codes.StoreAuthorizationCodeAsync(code1);
-
-            var code2 = await _codes.GetAuthorizationCodeAsync(handle);
-            code2.Should().BeNull();
-        }
-
         [Fact]
         public async Task StoreRefreshTokenAsync_should_persist_grant()
         {
@@ -183,34 +163,6 @@ public async Task RemoveRefreshTokenAsync_should_remove_grant()
             token2.Should().BeNull();
         }
 
-        [Fact]
-        public async Task expired_refresh_token_should_not_load()
-        {
-            var token1 = new RefreshToken()
-            {
-                CreationTime = DateTime.UtcNow.AddHours(-1),
-                Lifetime = 10,
-                AccessToken = new Token
-                {
-                    ClientId = "client",
-                    Audiences = { "aud" },
-                    CreationTime = DateTime.UtcNow,
-                    Type = "type",
-                    Claims = new List<Claim>
-                    {
-                        new Claim("sub", "123"),
-                        new Claim("scope", "foo")
-                    }
-                },
-                Version = 1
-            };
-
-            var handle = await _refreshTokens.StoreRefreshTokenAsync(token1);
-            var token2 = await _refreshTokens.GetRefreshTokenAsync(handle);
-
-            token2.Should().BeNull();
-        }
-
         [Fact]
         public async Task RemoveRefreshTokenAsync_by_sub_and_client_should_remove_grant()
         {
@@ -296,29 +248,6 @@ public async Task RemoveReferenceTokenAsync_should_remove_grant()
             token2.Should().BeNull();
         }
 
-        [Fact]
-        public async Task expired_reference_token_should_not_load()
-        {
-            var token1 = new Token()
-            {
-                ClientId = "client",
-                Audiences = { "aud" },
-                CreationTime = DateTime.UtcNow.AddHours(-1),
-                Type = "type",
-                Claims = new List<Claim>
-                {
-                    new Claim("sub", "123"),
-                    new Claim("scope", "foo")
-                },
-                Version = 1
-            };
-
-            var handle = await _referenceTokens.StoreReferenceTokenAsync(token1);
-
-            var token2 = await _referenceTokens.GetReferenceTokenAsync(handle);
-            token2.Should().BeNull();
-        }
-
         [Fact]
         public async Task RemoveReferenceTokenAsync_by_sub_and_client_should_remove_grant()
         {
@@ -380,24 +309,6 @@ public async Task RemoveUserConsentAsync_should_remove_grant()
             consent2.Should().BeNull();
         }
 
-        [Fact]
-        public async Task expired_user_consent_should_not_load()
-        {
-            var consent1 = new Consent()
-            {
-                ClientId = "client",
-                SubjectId = "123",
-                Scopes = new string[] { "foo", "bar" },
-                CreationTime = DateTime.UtcNow.AddHours(-1),
-                Expiration = DateTime.UtcNow.AddSeconds(-1)
-            };
-
-            await _userConsent.StoreUserConsentAsync(consent1);
-
-            var consent2 = await _userConsent.GetUserConsentAsync("123", "client");
-            consent2.Should().BeNull();
-        }
-
         [Fact]
         public async Task same_key_for_different_grant_types_should_not_interfere_with_each_other()
         {

test/IdentityServer.UnitTests/Validation/AccessTokenValidation.cs

@@ -143,8 +143,8 @@ public async Task Expired_Reference_Token()
             var token = TokenFactory.CreateAccessToken(new Client { ClientId = "roclient" }, "valid", 2, "read", "write");
 
             var handle = await store.StoreReferenceTokenAsync(token);
-            
-            now = now.AddMilliseconds(2000);
+
+            now = now.AddSeconds(3);
 
             var result = await validator.ValidateAccessTokenAsync(handle);