diff --git a/client/src/vue_components/user/CreateUser.vue b/client/src/vue_components/user/CreateUser.vue index 1783d9c4..fcdf2f19 100644 --- a/client/src/vue_components/user/CreateUser.vue +++ b/client/src/vue_components/user/CreateUser.vue @@ -115,10 +115,16 @@ export default { event.preventDefault(); } else { await this.CREATE_USER(this.state); + if (this.isFirstAdmin) { + await this.USER_LOGIN({ + username: this.state.username, + password: this.state.password, + }); + } this.$emit('created_user'); } }, - ...mapActions(['CREATE_USER']), + ...mapActions(['CREATE_USER', 'USER_LOGIN']), }, }; diff --git a/server/controllers/api/auth/user.py b/server/controllers/api/auth/user.py index 4af56f50..8049ed9d 100644 --- a/server/controllers/api/auth/user.py +++ b/server/controllers/api/auth/user.py @@ -15,62 +15,71 @@ api_authenticated, no_live_session, require_admin, - requires_show, ) @ApiRoute("auth/create", ApiVersion.V1) class UserCreateController(BaseAPIController): async def post(self): - data = escape.json_decode(self.request.body) + with self.make_session() as session: + # If there are no users, allow creation without authentication, otherwise require admin. + has_any_users = session.scalars(select(User)).first() is not None + if has_any_users: + self.requires_admin() - username = data.get("username", "") - if not username: - self.set_status(400) - await self.finish({"message": "Username missing"}) - return + data = escape.json_decode(self.request.body) - password = data.get("password", "") - if not password: - self.set_status(400) - await self.finish({"message": "Password missing"}) - return + username = data.get("username", "") + if not username: + self.set_status(400) + await self.finish({"message": "Username missing"}) + return - # Validate password strength - is_valid, error_msg = PasswordService.validate_password_strength(password) - if not is_valid: - self.set_status(400) - await self.finish({"message": error_msg}) - return + password = data.get("password", "") + if not password: + self.set_status(400) + await self.finish({"message": "Password missing"}) + return - is_admin = data.get("is_admin", False) + is_admin = data.get("is_admin", False) + if not has_any_users and not is_admin: + self.set_status(400) + await self.finish({"message": "First user must be an admin"}) + return - with self.make_session() as session: - conflict_user = session.scalars( - select(User).where(User.username == username) - ).first() - if conflict_user: + # Validate password strength + is_valid, error_msg = PasswordService.validate_password_strength(password) + if not is_valid: self.set_status(400) - await self.finish({"message": "Username already taken"}) + await self.finish({"message": error_msg}) return - hashed_password = await PasswordService.hash_password(password) + async with NamedLockRegistry.acquire(f"UserLock::{username}"): + conflict_user = session.scalars( + select(User).where(User.username == username) + ).first() + if conflict_user: + self.set_status(400) + await self.finish({"message": "Username already taken"}) + return - session.add( - User( - username=username, - password=hashed_password, - is_admin=is_admin, + hashed_password = await PasswordService.hash_password(password) + + session.add( + User( + username=username, + password=hashed_password, + is_admin=is_admin, + ) ) - ) - session.commit() + session.commit() - if is_admin: - await self.application.digi_settings.set("has_admin_user", True) + if is_admin: + await self.application.digi_settings.set("has_admin_user", True) - self.set_status(200) - await self.application.ws_send_to_all("NOOP", "GET_USERS", {}) - await self.finish({"message": "Successfully created user"}) + self.set_status(200) + await self.application.ws_send_to_all("NOOP", "GET_USERS", {}) + await self.finish({"message": "Successfully created user"}) @ApiRoute("auth/delete", ApiVersion.V1) @@ -233,7 +242,6 @@ async def post(self): class UsersHandler(BaseAPIController): @api_authenticated @require_admin - @requires_show def get(self): user_schema = UserSchema() with self.make_session() as session: diff --git a/server/test/controllers/api/test_auth.py b/server/test/controllers/api/test_auth.py index 825cba51..af589061 100644 --- a/server/test/controllers/api/test_auth.py +++ b/server/test/controllers/api/test_auth.py @@ -1,7 +1,6 @@ from sqlalchemy import select from tornado import escape -from models.show import Show, ShowScriptType from models.user import User from test.conftest import DigiScriptTestCase @@ -51,6 +50,21 @@ def test_create_admin(self): self.assertTrue("message" in response_body) self.assertEqual("Successfully created user", response_body["message"]) + def test_create_first_user_must_be_admin(self): + """Test that the first user created in the system must be an admin.""" + response = self.fetch( + "/api/v1/auth/create", + method="POST", + body=escape.json_encode( + {"username": "firstuser", "password": "password", "is_admin": False} + ), + ) + response_body = escape.json_decode(response.body) + + self.assertEqual(400, response.code) + self.assertTrue("message" in response_body) + self.assertEqual("First user must be an admin", response_body["message"]) + def test_create_user_duplicate_username(self): """Test POST /api/v1/auth/create with duplicate username. @@ -60,11 +74,28 @@ def test_create_user_duplicate_username(self): When a user with the same username already exists, the query should return that user and the endpoint should return a 400 error. """ - # Create an existing user directly in the database - with self._app.get_db().sessionmaker() as session: - existing_user = User(username="duplicate_test", password="hashed_pw") - session.add(existing_user) - session.commit() + # Create initial admin user + self.fetch( + "/api/v1/auth/create", + method="POST", + body=escape.json_encode( + { + "username": "duplicate_test", + "password": "adminpass", + "is_admin": True, + } + ), + ) + + # Login as admin + response = self.fetch( + "/api/v1/auth/login", + method="POST", + body=escape.json_encode( + {"username": "duplicate_test", "password": "adminpass"} + ), + ) + admin_token = escape.json_decode(response.body)["access_token"] # Try to create a user with the same username response = self.fetch( @@ -77,6 +108,7 @@ def test_create_user_duplicate_username(self): "is_admin": False, } ), + headers={"Authorization": f"Bearer {admin_token}"}, ) response_body = escape.json_decode(response.body) @@ -407,16 +439,6 @@ def test_delete_user(self): ), ) - # Create a test show (required by @requires_show decorator) - with self._app.get_db().sessionmaker() as session: - show = Show(name="Test Show", script_mode=ShowScriptType.FULL) - session.add(show) - session.flush() - show_id = show.id - session.commit() - - self._app.digi_settings.settings["current_show"].set_value(show_id) - # Login as admin to get token response = self.fetch( "/api/v1/auth/login", @@ -433,6 +455,7 @@ def test_delete_user(self): body=escape.json_encode( {"username": "userToDelete", "password": "password", "is_admin": False} ), + headers={"Authorization": f"Bearer {admin_token}"}, ) # Get the user ID @@ -479,16 +502,6 @@ def test_get_users(self): ), ) - # Create a test show (required by @requires_show decorator) - with self._app.get_db().sessionmaker() as session: - show = Show(name="Test Show", script_mode=ShowScriptType.FULL) - session.add(show) - session.flush() - show_id = show.id - session.commit() - - self._app.digi_settings.settings["current_show"].set_value(show_id) - # Login as admin response = self.fetch( "/api/v1/auth/login", @@ -505,6 +518,7 @@ def test_get_users(self): body=escape.json_encode( {"username": "user1", "password": "password", "is_admin": False} ), + headers={"Authorization": f"Bearer {admin_token}"}, ) self.fetch( "/api/v1/auth/create", @@ -512,6 +526,7 @@ def test_get_users(self): body=escape.json_encode( {"username": "user2", "password": "password", "is_admin": False} ), + headers={"Authorization": f"Bearer {admin_token}"}, ) # Get all users @@ -535,6 +550,23 @@ def test_get_users(self): def test_change_password_success(self): """Test successful password change with valid old password""" + # Create admin user + self.fetch( + "/api/v1/auth/create", + method="POST", + body=escape.json_encode( + {"username": "admin", "password": "adminpass", "is_admin": True} + ), + ) + + # Login as admin + response = self.fetch( + "/api/v1/auth/login", + method="POST", + body=escape.json_encode({"username": "admin", "password": "adminpass"}), + ) + admin_token = escape.json_decode(response.body)["access_token"] + # Create and login a user self.fetch( "/api/v1/auth/create", @@ -542,6 +574,7 @@ def test_change_password_success(self): body=escape.json_encode( {"username": "testuser", "password": "oldpass123", "is_admin": False} ), + headers={"Authorization": f"Bearer {admin_token}"}, ) response = self.fetch( @@ -593,6 +626,23 @@ def test_change_password_success(self): def test_change_password_incorrect_old_password(self): """Test password change fails with incorrect old password""" + # Create admin user + self.fetch( + "/api/v1/auth/create", + method="POST", + body=escape.json_encode( + {"username": "admin", "password": "adminpass", "is_admin": True} + ), + ) + + # Login as admin + response = self.fetch( + "/api/v1/auth/login", + method="POST", + body=escape.json_encode({"username": "admin", "password": "adminpass"}), + ) + admin_token = escape.json_decode(response.body)["access_token"] + # Create and login a user self.fetch( "/api/v1/auth/create", @@ -600,6 +650,7 @@ def test_change_password_incorrect_old_password(self): body=escape.json_encode( {"username": "testuser", "password": "oldpass123", "is_admin": False} ), + headers={"Authorization": f"Bearer {admin_token}"}, ) response = self.fetch( @@ -625,6 +676,23 @@ def test_change_password_incorrect_old_password(self): def test_change_password_missing_new_password(self): """Test password change fails when new password is missing""" + # Create admin user + self.fetch( + "/api/v1/auth/create", + method="POST", + body=escape.json_encode( + {"username": "admin", "password": "adminpass", "is_admin": True} + ), + ) + + # Login as admin + response = self.fetch( + "/api/v1/auth/login", + method="POST", + body=escape.json_encode({"username": "admin", "password": "adminpass"}), + ) + admin_token = escape.json_decode(response.body)["access_token"] + # Create and login a user self.fetch( "/api/v1/auth/create", @@ -632,6 +700,7 @@ def test_change_password_missing_new_password(self): body=escape.json_encode( {"username": "testuser", "password": "oldpass123", "is_admin": False} ), + headers={"Authorization": f"Bearer {admin_token}"}, ) response = self.fetch( @@ -655,6 +724,23 @@ def test_change_password_missing_new_password(self): def test_change_password_weak_password(self): """Test password change fails with weak new password""" + # Create admin user + self.fetch( + "/api/v1/auth/create", + method="POST", + body=escape.json_encode( + {"username": "admin", "password": "adminpass", "is_admin": True} + ), + ) + + # Login as admin + response = self.fetch( + "/api/v1/auth/login", + method="POST", + body=escape.json_encode({"username": "admin", "password": "adminpass"}), + ) + admin_token = escape.json_decode(response.body)["access_token"] + # Create and login a user self.fetch( "/api/v1/auth/create", @@ -662,6 +748,7 @@ def test_change_password_weak_password(self): body=escape.json_encode( {"username": "testuser", "password": "oldpass123", "is_admin": False} ), + headers={"Authorization": f"Bearer {admin_token}"}, ) response = self.fetch( @@ -699,6 +786,23 @@ def test_change_password_requires_authentication(self): def test_change_password_with_requires_password_change_flag(self): """Test password change works without old password when requires_password_change=True""" + # Create admin user + self.fetch( + "/api/v1/auth/create", + method="POST", + body=escape.json_encode( + {"username": "admin", "password": "adminpass", "is_admin": True} + ), + ) + + # Login as admin + response = self.fetch( + "/api/v1/auth/login", + method="POST", + body=escape.json_encode({"username": "admin", "password": "adminpass"}), + ) + admin_token = escape.json_decode(response.body)["access_token"] + # Create user via API to ensure password is properly hashed self.fetch( "/api/v1/auth/create", @@ -710,6 +814,7 @@ def test_change_password_with_requires_password_change_flag(self): "is_admin": False, } ), + headers={"Authorization": f"Bearer {admin_token}"}, ) # Set requires_password_change flag @@ -748,6 +853,23 @@ def test_change_password_with_requires_password_change_flag(self): def test_password_enforcement_blocks_regular_endpoints(self): """Test that requires_password_change blocks access to regular endpoints""" + # Create admin user + self.fetch( + "/api/v1/auth/create", + method="POST", + body=escape.json_encode( + {"username": "admin", "password": "adminpass", "is_admin": True} + ), + ) + + # Login as admin + response = self.fetch( + "/api/v1/auth/login", + method="POST", + body=escape.json_encode({"username": "admin", "password": "adminpass"}), + ) + admin_token = escape.json_decode(response.body)["access_token"] + # Create user with requires_password_change=True self.fetch( "/api/v1/auth/create", @@ -759,6 +881,7 @@ def test_password_enforcement_blocks_regular_endpoints(self): "is_admin": False, } ), + headers={"Authorization": f"Bearer {admin_token}"}, ) # Log in to get JWT @@ -790,6 +913,23 @@ def test_password_enforcement_blocks_regular_endpoints(self): def test_password_enforcement_allows_change_password_endpoint(self): """Test that requires_password_change allows access to change-password""" + # Create admin user + self.fetch( + "/api/v1/auth/create", + method="POST", + body=escape.json_encode( + {"username": "admin", "password": "adminpass", "is_admin": True} + ), + ) + + # Login as admin + response = self.fetch( + "/api/v1/auth/login", + method="POST", + body=escape.json_encode({"username": "admin", "password": "adminpass"}), + ) + admin_token = escape.json_decode(response.body)["access_token"] + # Create user with requires_password_change=True self.fetch( "/api/v1/auth/create", @@ -801,6 +941,7 @@ def test_password_enforcement_allows_change_password_endpoint(self): "is_admin": False, } ), + headers={"Authorization": f"Bearer {admin_token}"}, ) # Log in to get JWT @@ -833,6 +974,23 @@ def test_password_enforcement_allows_change_password_endpoint(self): def test_password_enforcement_allows_logout_endpoint(self): """Test that requires_password_change allows access to logout""" + # Create admin user + self.fetch( + "/api/v1/auth/create", + method="POST", + body=escape.json_encode( + {"username": "admin", "password": "adminpass", "is_admin": True} + ), + ) + + # Login as admin + response = self.fetch( + "/api/v1/auth/login", + method="POST", + body=escape.json_encode({"username": "admin", "password": "adminpass"}), + ) + admin_token = escape.json_decode(response.body)["access_token"] + # Create user with requires_password_change=True self.fetch( "/api/v1/auth/create", @@ -844,6 +1002,7 @@ def test_password_enforcement_allows_logout_endpoint(self): "is_admin": False, } ), + headers={"Authorization": f"Bearer {admin_token}"}, ) # Log in to get JWT @@ -900,6 +1059,7 @@ def test_admin_reset_password_success(self): body=escape.json_encode( {"username": "regularuser", "password": "userpass", "is_admin": False} ), + headers={"Authorization": f"Bearer {admin_token}"}, ) # Get user ID @@ -907,6 +1067,8 @@ def test_admin_reset_password_success(self): user = session.scalars( select(User).where(User.username == "regularuser") ).first() + if not user: + self.fail("User 'regularuser' not found in database") user_id = user.id # Admin resets user password @@ -979,6 +1141,23 @@ def test_admin_reset_password_cannot_reset_own(self): def test_admin_reset_password_requires_admin(self): """Test password reset requires admin privileges""" + # Create admin user + self.fetch( + "/api/v1/auth/create", + method="POST", + body=escape.json_encode( + {"username": "admin", "password": "adminpass", "is_admin": True} + ), + ) + + # Login as admin + response = self.fetch( + "/api/v1/auth/login", + method="POST", + body=escape.json_encode({"username": "admin", "password": "adminpass"}), + ) + admin_token = escape.json_decode(response.body)["access_token"] + # Create regular user self.fetch( "/api/v1/auth/create", @@ -986,6 +1165,7 @@ def test_admin_reset_password_requires_admin(self): body=escape.json_encode( {"username": "regularuser", "password": "userpass", "is_admin": False} ), + headers={"Authorization": f"Bearer {admin_token}"}, ) # Login as regular user diff --git a/server/utils/web/base_controller.py b/server/utils/web/base_controller.py index 5aa3b422..22a04e2f 100644 --- a/server/utils/web/base_controller.py +++ b/server/utils/web/base_controller.py @@ -121,6 +121,12 @@ async def prepare( self.current_show = show_schema.dump(show) return + def requires_admin(self): + if not self.current_user: + raise HTTPError(401, log_message="Not logged in") + if not self.current_user["is_admin"]: + raise HTTPError(403, log_message="Admin access required") + def requires_role(self, resource: db.Model, role: Role): if not self.current_user: raise HTTPError(401, log_message="Not logged in")