Merge pull request #15 from evgomes/14-refresh-token-allows-user-to-s…

…pecify-any-email-address Bug Fix for Security Issue - Take and Revoke Refresh Tokens
evgomes · Aug 1, 2022 · 5b7ad1b · 5b7ad1b
2 parents a1b061a + 1ddb66b
commit 5b7ad1b
Show file tree

Hide file tree

Showing 9 changed files with 69 additions and 34 deletions.
diff --git a/src/JWTAPI/JWTAPI/Controllers/LoginController.cs b/src/JWTAPI/JWTAPI/Controllers/LoginController.cs
@@ -58,14 +58,14 @@ public async Task<IActionResult> RefreshTokenAsync([FromBody] RefreshTokenResour
 
         [Route("/api/token/revoke")]
         [HttpPost]
-        public IActionResult RevokeToken([FromBody] RevokeTokenResource revokeTokenResource)
+        public IActionResult RevokeToken([FromBody] RevokeTokenResource resource)
         {
             if (!ModelState.IsValid)
             {
                 return BadRequest(ModelState);
             }
 
-            _authenticationService.RevokeRefreshToken(revokeTokenResource.Token);
+            _authenticationService.RevokeRefreshToken(resource.Token, resource.Email);
             return NoContent();
         }
     }

diff --git a/src/JWTAPI/JWTAPI/Controllers/Resources/RevokeTokenResource.cs b/src/JWTAPI/JWTAPI/Controllers/Resources/RevokeTokenResource.cs
@@ -6,5 +6,8 @@ public class RevokeTokenResource
     {
         [Required]
         public string Token { get; set; }
-    }
+
+		[Required]
+		public string Email { get; set; }
+	}
 }
diff --git a/src/JWTAPI/JWTAPI/Core/Security/Tokens/ITokenHandler.cs b/src/JWTAPI/JWTAPI/Core/Security/Tokens/ITokenHandler.cs
@@ -5,7 +5,7 @@ namespace JWTAPI.Core.Security.Tokens
     public interface ITokenHandler
     {
          AccessToken CreateAccessToken(User user);
-         RefreshToken TakeRefreshToken(string token);
-         void RevokeRefreshToken(string token);
+         RefreshToken TakeRefreshToken(string token, string userEmail);
+         void RevokeRefreshToken(string token, string userEmail);
     }
 }
diff --git a/src/JWTAPI/JWTAPI/Core/Security/Tokens/RefreshTokenWithEmail.cs b/src/JWTAPI/JWTAPI/Core/Security/Tokens/RefreshTokenWithEmail.cs
@@ -0,0 +1,8 @@
+namespace JWTAPI.Core.Security.Tokens
+{
+	public class RefreshTokenWithEmail
+	{
+		public string Email { get; set; }
+		public RefreshToken RefreshToken { get; set; }
+	}
+}
diff --git a/src/JWTAPI/JWTAPI/Core/Services/IAuthenticationService.cs b/src/JWTAPI/JWTAPI/Core/Services/IAuthenticationService.cs
@@ -6,6 +6,6 @@ public interface IAuthenticationService
     {
          Task<TokenResponse> CreateAccessTokenAsync(string email, string password);
          Task<TokenResponse> RefreshTokenAsync(string refreshToken, string userEmail);
-         void RevokeRefreshToken(string refreshToken);
+         void RevokeRefreshToken(string refreshToken, string userEmail);
     }
 }
diff --git a/src/JWTAPI/JWTAPI/Security/Tokens/TokenHandler.cs b/src/JWTAPI/JWTAPI/Security/Tokens/TokenHandler.cs
@@ -9,7 +9,7 @@ namespace JWTAPI.Security.Tokens
 {
     public class TokenHandler : ITokenHandler
     {
-        private readonly ISet<RefreshToken> _refreshTokens = new HashSet<RefreshToken>();
+        private readonly ISet<RefreshTokenWithEmail> _refreshTokens = new HashSet<RefreshTokenWithEmail>();
 
         private readonly TokenOptions _tokenOptions;
         private readonly SigningConfigurations _signingConfigurations;
@@ -26,26 +26,33 @@ public AccessToken CreateAccessToken(User user)
         {
             var refreshToken = BuildRefreshToken();
             var accessToken = BuildAccessToken(user, refreshToken);
-            _refreshTokens.Add(refreshToken);
+            _refreshTokens.Add(new RefreshTokenWithEmail 
+            {
+                Email = user.Email,
+                RefreshToken = refreshToken,
+            });
 
             return accessToken;
         }
 
-        public RefreshToken TakeRefreshToken(string token)
+        public RefreshToken TakeRefreshToken(string token, string userEmail)
         {
-            if (string.IsNullOrWhiteSpace(token))
+            if (string.IsNullOrWhiteSpace(token) || string.IsNullOrWhiteSpace(userEmail))
                 return null;
 
-            var refreshToken = _refreshTokens.SingleOrDefault(t => t.Token == token);
-            if (refreshToken != null)
-                _refreshTokens.Remove(refreshToken);
-
-            return refreshToken;
+            var refreshTokenWithEmail = _refreshTokens.SingleOrDefault(t => t.RefreshToken.Token == token && t.Email == userEmail);
+            if(refreshTokenWithEmail == null)
+			{
+                return null;
+			}
+
+            _refreshTokens.Remove(refreshTokenWithEmail);
+            return refreshTokenWithEmail.RefreshToken;
         }
 
-        public void RevokeRefreshToken(string token)
+        public void RevokeRefreshToken(string token, string userEmail)
         {
-            TakeRefreshToken(token);
+            TakeRefreshToken(token, userEmail);
         }
 
         private RefreshToken BuildRefreshToken()

diff --git a/src/JWTAPI/JWTAPI/Services/AuthenticationService.cs b/src/JWTAPI/JWTAPI/Services/AuthenticationService.cs
@@ -34,7 +34,7 @@ public async Task<TokenResponse> CreateAccessTokenAsync(string email, string pas
 
         public async Task<TokenResponse> RefreshTokenAsync(string refreshToken, string userEmail)
         {
-            var token = _tokenHandler.TakeRefreshToken(refreshToken);
+            var token = _tokenHandler.TakeRefreshToken(refreshToken, userEmail);
 
             if (token == null)
             {
@@ -56,9 +56,9 @@ public async Task<TokenResponse> RefreshTokenAsync(string refreshToken, string u
             return new TokenResponse(true, null, accessToken);
         }
 
-        public void RevokeRefreshToken(string refreshToken)
+        public void RevokeRefreshToken(string refreshToken, string userEmail)
         {
-            _tokenHandler.RevokeRefreshToken(refreshToken);
+            _tokenHandler.RevokeRefreshToken(refreshToken, userEmail);
         }
     }
 }
diff --git a/tests/JWTAPI.Tests/Security/Tokens/TokenHandlerTests.cs b/tests/JWTAPI.Tests/Security/Tokens/TokenHandlerTests.cs
@@ -79,33 +79,50 @@ public void Should_Create_Access_Token()
         public void Should_Take_Existing_Refresh_Token()
         {
             var accessToken = _tokenHandler.CreateAccessToken(_user);
-            var refreshToken = _tokenHandler.TakeRefreshToken(accessToken.RefreshToken.Token);
+            var refreshToken = _tokenHandler.TakeRefreshToken(accessToken.RefreshToken.Token, "test@test.com");
 
             Assert.NotNull(refreshToken);
             Assert.Equal(accessToken.RefreshToken.Token, refreshToken.Token);
             Assert.Equal(accessToken.RefreshToken.Expiration, refreshToken.Expiration);
         }
 
         [Fact]
-        public void Should_Return_Null_For_Empty_Refresh_Token_When_Trying_To_Take()
+        public void Should_Return_Null_For_Empty_Refresh_Token_When_Trying_To_Take_Refresh_Token()
         {
-            var refreshToken = _tokenHandler.TakeRefreshToken("");
+            var refreshToken = _tokenHandler.TakeRefreshToken(string.Empty, "test@test.com");
+            Assert.Null(refreshToken);
+        }
+
+		[Fact]
+        public void Should_Return_Null_For_Empty_Email_When_Trying_To_Take_Refresh_Token()
+		{
+            var accessToken = _tokenHandler.CreateAccessToken(_user);
+            var refreshToken = _tokenHandler.TakeRefreshToken(accessToken.RefreshToken.Token, string.Empty);
+
             Assert.Null(refreshToken);
         }
 
         [Fact]
-        public void Should_Return_Null_For_Invalid_Refresh_Token_When_Trying_To_Take()
+        public void Should_Return_Null_For_Invalid_Refresh_Token_When_Trying_To_Take_Refresh_oken()
         {
-            var refreshToken = _tokenHandler.TakeRefreshToken("invalid_token");
+            var refreshToken = _tokenHandler.TakeRefreshToken("invalid_token", "test@test.com");
+            Assert.Null(refreshToken);
+        }
+
+        [Fact]
+        public void Should_Return_Null_For_Invalid_Email_When_Trying_To_Take_Refresh_Token()
+        {
+            var accessToken = _tokenHandler.CreateAccessToken(_user);
+            var refreshToken = _tokenHandler.TakeRefreshToken(accessToken.RefreshToken.Token, "admin@admin.com");
             Assert.Null(refreshToken);
         }
 
         [Fact]
         public void Should_Not_Take_Refresh_Token_That_Was_Already_Taken()
         {
             var accessToken = _tokenHandler.CreateAccessToken(_user);
-            var refreshToken = _tokenHandler.TakeRefreshToken(accessToken.RefreshToken.Token);
-            var refreshTokenSecondTime = _tokenHandler.TakeRefreshToken(accessToken.RefreshToken.Token);
+            var refreshToken = _tokenHandler.TakeRefreshToken(accessToken.RefreshToken.Token, "test@test.com");
+            var refreshTokenSecondTime = _tokenHandler.TakeRefreshToken(accessToken.RefreshToken.Token, "test@test.com");
 
             Assert.NotNull(refreshToken);
             Assert.Null(refreshTokenSecondTime);
@@ -115,8 +132,8 @@ public void Should_Not_Take_Refresh_Token_That_Was_Already_Taken()
         public void Should_Revoke_Refresh_Token()
         {
             var accessToken = _tokenHandler.CreateAccessToken(_user);
-            _tokenHandler.RevokeRefreshToken(accessToken.RefreshToken.Token);
-            var refreshToken = _tokenHandler.TakeRefreshToken(accessToken.RefreshToken.Token);
+            _tokenHandler.RevokeRefreshToken(accessToken.RefreshToken.Token, "test@test.com");
+            var refreshToken = _tokenHandler.TakeRefreshToken(accessToken.RefreshToken.Token, "test@test.com");
 
             Assert.Null(refreshToken);
         }

diff --git a/tests/JWTAPI.Tests/Services/AuthenticationServiceTests.cs b/tests/JWTAPI.Tests/Services/AuthenticationServiceTests.cs
@@ -72,16 +72,16 @@ private void SetupMocks()
                                      )
                                  );
 
-            _tokenHandler.Setup(h => h.TakeRefreshToken("abc"))
+            _tokenHandler.Setup(h => h.TakeRefreshToken("abc", It.IsAny<string>()))
                          .Returns(new RefreshToken("abc", DateTime.UtcNow.AddSeconds(60).Ticks));
 
-            _tokenHandler.Setup(h => h.TakeRefreshToken("expired"))
+            _tokenHandler.Setup(h => h.TakeRefreshToken("expired", It.IsAny<string>()))
                          .Returns(new RefreshToken("expired", DateTime.UtcNow.AddSeconds(-60).Ticks));
 
-            _tokenHandler.Setup(h => h.TakeRefreshToken("invalid"))
+            _tokenHandler.Setup(h => h.TakeRefreshToken("invalid", It.IsAny<string>()))
                          .Returns<RefreshToken>(null);
 
-            _tokenHandler.Setup(h => h.RevokeRefreshToken("abc"))
+            _tokenHandler.Setup(h => h.RevokeRefreshToken("abc", It.IsAny<string>()))
                          .Callback(() => _calledRefreshToken = true);
         }
 
@@ -153,7 +153,7 @@ public async Task Should_Not_Refresh_Token_For_Invalid_User_Data()
         [Fact]
         public void Should_Revoke_Refresh_Token()
         {
-            _authenticationService.RevokeRefreshToken("abc");
+            _authenticationService.RevokeRefreshToken("abc", "test@test.com");
 
             Assert.True(_calledRefreshToken);
         }