[UserController] Update security, attributes, tests (#3850)

Author: imnasnainaec

Backend.Tests/Controllers/UserControllerTests.cs

@@ -12,6 +12,7 @@ namespace Backend.Tests.Controllers
     public class UserControllerTests : IDisposable
     {
         private IUserRepository _userRepo = null!;
+        private IPasswordResetService _passwordResetService = null!;
         private IPermissionService _permissionService = null!;
         private UserController _userController = null!;
 
@@ -33,9 +34,10 @@ protected virtual void Dispose(bool disposing)
         public void Setup()
         {
             _userRepo = new UserRepositoryMock();
+            _passwordResetService = new PasswordResetServiceMock();
             _permissionService = new PermissionServiceMock(_userRepo);
             _userController = new UserController(_userRepo, _permissionService,
-                new CaptchaServiceMock(), new EmailServiceMock(), new PasswordResetServiceMock());
+                new CaptchaServiceMock(), new EmailServiceMock(), _passwordResetService);
         }
 
         private static User RandomUser()
@@ -49,6 +51,62 @@ private static User RandomUser()
             return user;
         }
 
+        [Test]
+        public void TestVerifyCaptchaToken()
+        {
+            // No permissions should be required to verify CAPTCHA.
+            _userController.ControllerContext.HttpContext = PermissionServiceMock.UnauthorizedHttpContext();
+
+            var result = _userController.VerifyCaptchaToken("token").Result;
+            Assert.That(result, Is.TypeOf<OkResult>());
+        }
+
+        [Test]
+        public void TestResetPasswordRequest()
+        {
+            // No permissions should be required to request a password reset.
+            _userController.ControllerContext.HttpContext = PermissionServiceMock.UnauthorizedHttpContext();
+
+            // Returns Ok regardless of if user exists.
+            var noUserResult = _userController.ResetPasswordRequest(new()).Result;
+            Assert.That(noUserResult, Is.TypeOf<OkResult>());
+            var username = (_userRepo.Create(new() { Username = "Imarealboy" }).Result)!.Username;
+            var yesUserResult = _userController.ResetPasswordRequest(new() { EmailOrUsername = username }).Result;
+            Assert.That(yesUserResult, Is.TypeOf<OkResult>());
+        }
+
+        [Test]
+        public void TestValidateResetToken()
+        {
+            // No permissions should be required to validate a password reset token.
+            _userController.ControllerContext.HttpContext = PermissionServiceMock.UnauthorizedHttpContext();
+
+            ((PasswordResetServiceMock)_passwordResetService).SetNextBoolResponse(false);
+            var falseResult = _userController.ValidateResetToken("token").Result;
+            Assert.That(falseResult, Is.TypeOf<OkObjectResult>());
+            Assert.That(((OkObjectResult)falseResult).Value, Is.EqualTo(false));
+
+            ((PasswordResetServiceMock)_passwordResetService).SetNextBoolResponse(true);
+            var trueResult = _userController.ValidateResetToken("token").Result;
+            Assert.That(trueResult, Is.TypeOf<OkObjectResult>());
+            Assert.That(((OkObjectResult)trueResult).Value, Is.EqualTo(true));
+        }
+
+        [Test]
+        public void TestResetPassword()
+        {
+            // No permissions should be required to reset password via a token.
+            _userController.ControllerContext.HttpContext = PermissionServiceMock.UnauthorizedHttpContext();
+
+            ((PasswordResetServiceMock)_passwordResetService).SetNextBoolResponse(false);
+            var falseResult = _userController.ResetPassword(new()).Result;
+            Assert.That(falseResult, Is.TypeOf<ForbidResult>());
+
+            ((PasswordResetServiceMock)_passwordResetService).SetNextBoolResponse(true);
+            var trueResult = _userController.ResetPassword(new()).Result;
+            Assert.That(trueResult, Is.TypeOf<OkResult>());
+        }
+
         [Test]
         public void TestGetAllUsers()
         {
@@ -62,6 +120,37 @@ public void TestGetAllUsers()
                 user => Assert.That(users, Does.Contain(user).UsingPropertiesComparer()));
         }
 
+        [Test]
+        public void TestGetAllUsersNoPermission()
+        {
+            _userController.ControllerContext.HttpContext = PermissionServiceMock.UnauthorizedHttpContext();
+            var result = _userController.GetAllUsers().Result;
+            Assert.That(result, Is.InstanceOf<ForbidResult>());
+        }
+
+        [Test]
+        public void TestAuthenticateBadCredentials()
+        {
+            _userController.ControllerContext.HttpContext = PermissionServiceMock.UnauthorizedHttpContext();
+            var result = _userController.Authenticate(new() { EmailOrUsername = "no", Password = "no" }).Result;
+            Assert.That(result, Is.InstanceOf<UnauthorizedObjectResult>());
+        }
+
+        [Test]
+        public void TestGetCurrentUserNoneAuthenticated()
+        {
+            var result = _userController.GetCurrentUser().Result;
+            Assert.That(result, Is.InstanceOf<ForbidResult>());
+        }
+
+        [Test]
+        public void TestGetUserNoPermission()
+        {
+            _userController.ControllerContext.HttpContext = PermissionServiceMock.UnauthorizedHttpContext();
+            var result = _userController.GetUser("any-user").Result;
+            Assert.That(result, Is.InstanceOf<ForbidResult>());
+        }
+
         [Test]
         public void TestGetUser()
         {
@@ -72,14 +161,14 @@ public void TestGetUser()
 
             var result = _userController.GetUser(user.Id).Result;
             Assert.That(result, Is.InstanceOf<ObjectResult>());
-            Assert.That(((ObjectResult)result).Value, Is.EqualTo(user).UsingPropertiesComparer());
+            Assert.That(((ObjectResult)result).Value, Is.EqualTo(new UserStub(user)).UsingPropertiesComparer());
         }
 
         [Test]
         public void TestGetMissingUser()
         {
             var result = _userController.GetUser("INVALID_USER_ID").Result;
-            Assert.That(result, Is.InstanceOf<NotFoundObjectResult>());
+            Assert.That(result, Is.InstanceOf<NotFoundResult>());
         }
 
         [Test]
@@ -120,7 +209,7 @@ public void TestGetUserIdByEmailOrUsernameNoPermission()
         {
             _userController.ControllerContext.HttpContext = PermissionServiceMock.UnauthorizedHttpContext();
             const string email = "[email protected]";
-            var _ = _userRepo.Create(
+            _ = _userRepo.Create(
                 new User { Email = email, Username = Util.RandString(10), Password = Util.RandString(10) }
             ).Result ?? throw new UserCreationException();
 
@@ -142,30 +231,29 @@ public void TestCreateUserBadUsername()
         {
             var user = RandomUser();
             _userRepo.Create(user);
+
             var user2 = RandomUser();
             user2.Username = " ";
-            var result = _userController.CreateUser(user).Result;
-            Assert.That(result, Is.TypeOf<BadRequestObjectResult>());
+            Assert.That(_userController.CreateUser(user2).Result, Is.TypeOf<BadRequestObjectResult>());
             user2.Username = user.Username;
-            result = _userController.CreateUser(user).Result;
-            Assert.That(result, Is.TypeOf<BadRequestObjectResult>());
+            Assert.That(_userController.CreateUser(user2).Result, Is.TypeOf<BadRequestObjectResult>());
             user2.Username = user.Email;
-            result = _userController.CreateUser(user).Result;
-            Assert.That(result, Is.TypeOf<BadRequestObjectResult>());
+            Assert.That(_userController.CreateUser(user2).Result, Is.TypeOf<BadRequestObjectResult>());
         }
 
         [Test]
         public void TestCreateUserBadEmail()
         {
             var user = RandomUser();
             _userRepo.Create(user);
+
             var user2 = RandomUser();
             user2.Email = " ";
-            var result = _userController.CreateUser(user).Result;
-            Assert.That(result, Is.TypeOf<BadRequestObjectResult>());
+            Assert.That(_userController.CreateUser(user2).Result, Is.TypeOf<BadRequestObjectResult>());
             user2.Email = user.Email;
-            result = _userController.CreateUser(user).Result;
-            Assert.That(result, Is.TypeOf<BadRequestObjectResult>());
+            Assert.That(_userController.CreateUser(user2).Result, Is.TypeOf<BadRequestObjectResult>());
+            user2.Email = user.Username;
+            Assert.That(_userController.CreateUser(user2).Result, Is.TypeOf<BadRequestObjectResult>());
         }
 
         [Test]
@@ -196,6 +284,14 @@ public void TestUpdateUserCantUpdateIsAdmin()
             Assert.That(users, Does.Contain(modUser).UsingPropertiesComparer());
         }
 
+        [Test]
+        public void TestUpdateUserNoPermission()
+        {
+            _userController.ControllerContext.HttpContext = PermissionServiceMock.UnauthorizedHttpContext();
+            var result = _userController.UpdateUser("any-user", new()).Result;
+            Assert.That(result, Is.InstanceOf<ForbidResult>());
+        }
+
         [Test]
         public void TestDeleteUser()
         {
@@ -206,6 +302,21 @@ public void TestDeleteUser()
             Assert.That(_userRepo.GetAllUsers().Result, Is.Empty);
         }
 
+        [Test]
+        public void TestDeleteUserNoUser()
+        {
+            var result = _userController.DeleteUser("not-a-user").Result;
+            Assert.That(result, Is.InstanceOf<NotFoundResult>());
+        }
+
+        [Test]
+        public void TestDeleteUserNoPermission()
+        {
+            _userController.ControllerContext.HttpContext = PermissionServiceMock.UnauthorizedHttpContext();
+            var result = _userController.DeleteUser("anything").Result;
+            Assert.That(result, Is.InstanceOf<ForbidResult>());
+        }
+
         [Test]
         public void TestIsEmailOrUsernameAvailable()
         {
@@ -233,7 +344,7 @@ public void TestIsEmailOrUsernameAvailable()
         }
 
         [Test]
-        public void TestIsUserSiteAdminNotAuthorized()
+        public void TestIsUserSiteAdminNoPermission()
         {
             _userController.ControllerContext.HttpContext = PermissionServiceMock.UnauthorizedHttpContext();
             var result = _userController.IsUserSiteAdmin().Result;

Backend.Tests/Mocks/PasswordResetServiceMock.cs

@@ -6,6 +6,12 @@ namespace Backend.Tests.Mocks
 {
     internal sealed class PasswordResetServiceMock : IPasswordResetService
     {
+        private bool _boolResponse;
+        internal void SetNextBoolResponse(bool response)
+        {
+            _boolResponse = response;
+        }
+
         public Task<PasswordReset> CreatePasswordReset(string email)
         {
             return Task.FromResult(new PasswordReset(15, email));
@@ -18,14 +24,12 @@ public Task ExpirePasswordReset(string email)
 
         public Task<bool> ValidateToken(string token)
         {
-            // TODO: More sophisticated mock
-            return Task.FromResult(true);
+            return Task.FromResult(_boolResponse);
         }
 
         public Task<bool> ResetPassword(string token, string password)
         {
-            // TODO: More sophisticated mock
-            return Task.FromResult(true);
+            return Task.FromResult(_boolResponse);
         }
     }
 }