diff --git a/app/models/user.py b/app/models/user.py index 90866767..492d187c 100644 --- a/app/models/user.py +++ b/app/models/user.py @@ -419,15 +419,19 @@ class User(UserMixin, db.Model): # Permission and role helpers def has_permission(self, permission_name): """Check if user has a specific permission through any of their roles""" + # Legacy admin bypass: a user with role="admin" and no explicit + # roles assigned should have all permissions for backward + # compatibility. Check this BEFORE _auto_assign_role_from_legacy + # — that helper appends a Role row to self.roles as a side effect, + # and an empty seeded "admin" role would otherwise mask the + # bypass and leave the user with no effective permissions. + if self.role == "admin" and not self.roles: + return True + # Auto-assign role from legacy role field if user has no roles assigned if not self.roles and self.role: self._auto_assign_role_from_legacy() - # Super admin users have all permissions - if self.role == "admin" and not self.roles: - # Legacy admin users without roles have all permissions - return True - # Check if any of the user's roles have this permission for role in self.roles: if role.has_permission(permission_name): diff --git a/tests/test_permissions.py b/tests/test_permissions.py index 26cdc46a..53b434f9 100644 --- a/tests/test_permissions.py +++ b/tests/test_permissions.py @@ -256,9 +256,12 @@ def test_admin_role_user(app): user = User(username="testuser", role="user") db.session.add(user) - # Create admin role - admin_role = Role(name="admin") - db.session.add(admin_role) + # The conftest fixture seeds an "admin" role; re-use it if present + # so this test doesn't trip the unique constraint on roles.name. + admin_role = Role.query.filter_by(name="admin").first() + if admin_role is None: + admin_role = Role(name="admin") + db.session.add(admin_role) db.session.commit() # User is not admin initially diff --git a/tests/test_routes/test_api_v1_expenses_complete.py b/tests/test_routes/test_api_v1_expenses_complete.py index 73f0ddd9..e06b7926 100644 --- a/tests/test_routes/test_api_v1_expenses_complete.py +++ b/tests/test_routes/test_api_v1_expenses_complete.py @@ -82,7 +82,7 @@ class TestAPIExpensesComplete: data = response.get_json() assert "expense" in data assert data["expense"]["title"] == "Complete Test Expense" - assert data["expense"]["amount"] == "250.75" + assert data["expense"]["amount"] == 250.75 def test_update_expense_uses_service_layer(self, app, client_with_token, expense): """Test that update_expense uses service layer"""