feat: implement complete RBAC access control with tests
Add 4-layer access control stack (subscription → module → menu → permissions): - P1: Wire requires_permission into menu sidebar filtering - P2: Expose window.USER_PERMISSIONS for Alpine.js client-side gating - P3: Add page-level permission guards on store routes - P4: Role CRUD API endpoints and role editor UI - P5: Audit trail for all role/permission changes Includes unit tests (menu permission filtering, role CRUD service) and integration tests (role API endpoints). All 404 core+tenancy tests pass. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -26,9 +26,11 @@ def get_preset_permissions(preset_name: str) -> set[str]:
|
||||
"""Get permissions for a preset role."""
|
||||
return permission_discovery_service.get_preset_permissions(preset_name)
|
||||
from app.modules.billing.exceptions import TierLimitExceededException
|
||||
from app.modules.core.services.audit_aggregator import audit_aggregator
|
||||
from app.modules.tenancy.exceptions import (
|
||||
CannotRemoveOwnerException,
|
||||
InvalidInvitationTokenException,
|
||||
InvalidRoleException,
|
||||
TeamInvitationAlreadyAcceptedException,
|
||||
TeamMemberAlreadyExistsException,
|
||||
UserNotFoundException,
|
||||
@@ -174,6 +176,20 @@ class StoreTeamService:
|
||||
# TODO: Send invitation email
|
||||
# self._send_invitation_email(email, store, invitation_token)
|
||||
|
||||
audit_aggregator.log(
|
||||
db=db,
|
||||
admin_user_id=inviter.id,
|
||||
action="member.invite",
|
||||
target_type="store_user",
|
||||
target_id=str(store_user.id),
|
||||
details={
|
||||
"email": email,
|
||||
"role": role_name,
|
||||
"store_id": store.id,
|
||||
"store_code": store.store_code,
|
||||
},
|
||||
)
|
||||
|
||||
return {
|
||||
"invitation_token": invitation_token,
|
||||
"email": email,
|
||||
@@ -274,6 +290,7 @@ class StoreTeamService:
|
||||
db: Session,
|
||||
store: Store,
|
||||
user_id: int,
|
||||
actor_user_id: int | None = None,
|
||||
) -> bool:
|
||||
"""
|
||||
Remove a team member from a store.
|
||||
@@ -309,6 +326,21 @@ class StoreTeamService:
|
||||
store_user.is_active = False
|
||||
|
||||
logger.info(f"Removed user {user_id} from store {store.store_code}")
|
||||
|
||||
if actor_user_id is not None:
|
||||
audit_aggregator.log(
|
||||
db=db,
|
||||
admin_user_id=actor_user_id,
|
||||
action="member.remove",
|
||||
target_type="store_user",
|
||||
target_id=str(store_user.id),
|
||||
details={
|
||||
"user_id": user_id,
|
||||
"store_id": store.id,
|
||||
"store_code": store.store_code,
|
||||
},
|
||||
)
|
||||
|
||||
return True
|
||||
|
||||
except (UserNotFoundException, CannotRemoveOwnerException):
|
||||
@@ -324,6 +356,7 @@ class StoreTeamService:
|
||||
user_id: int,
|
||||
new_role_name: str,
|
||||
custom_permissions: list[str] | None = None,
|
||||
actor_user_id: int | None = None,
|
||||
) -> StoreUser:
|
||||
"""
|
||||
Update a team member's role.
|
||||
@@ -363,14 +396,31 @@ class StoreTeamService:
|
||||
custom_permissions=custom_permissions,
|
||||
)
|
||||
|
||||
old_role_name = store_user.role.name if store_user.role else "none"
|
||||
store_user.role_id = new_role.id
|
||||
db.flush()
|
||||
db.refresh(store_user)
|
||||
|
||||
logger.info(
|
||||
f"Updated role for user {user_id} in store {store.store_code} "
|
||||
f"to {new_role_name}"
|
||||
)
|
||||
|
||||
if actor_user_id is not None:
|
||||
audit_aggregator.log(
|
||||
db=db,
|
||||
admin_user_id=actor_user_id,
|
||||
action="member.role_change",
|
||||
target_type="store_user",
|
||||
target_id=str(store_user.id),
|
||||
details={
|
||||
"user_id": user_id,
|
||||
"store_id": store.id,
|
||||
"old_role": old_role_name,
|
||||
"new_role": new_role_name,
|
||||
},
|
||||
)
|
||||
|
||||
return store_user
|
||||
|
||||
except (UserNotFoundException, CannotRemoveOwnerException):
|
||||
@@ -470,6 +520,210 @@ class StoreTeamService:
|
||||
for role in roles
|
||||
]
|
||||
|
||||
# ========================================================================
|
||||
# Role CRUD
|
||||
# ========================================================================
|
||||
|
||||
PRESET_ROLE_NAMES = {"manager", "staff", "support", "viewer", "marketing"}
|
||||
|
||||
def create_custom_role(
|
||||
self,
|
||||
db: Session,
|
||||
store_id: int,
|
||||
name: str,
|
||||
permissions: list[str],
|
||||
actor_user_id: int | None = None,
|
||||
) -> Role:
|
||||
"""
|
||||
Create a custom role for a store.
|
||||
|
||||
Args:
|
||||
db: Database session
|
||||
store_id: Store ID
|
||||
name: Role name
|
||||
permissions: List of permission IDs
|
||||
actor_user_id: ID of user performing the action (for audit)
|
||||
|
||||
Returns:
|
||||
Created Role object
|
||||
|
||||
Raises:
|
||||
InvalidRoleException: If role name conflicts with a preset
|
||||
"""
|
||||
if name.lower() in self.PRESET_ROLE_NAMES:
|
||||
raise InvalidRoleException(f"Cannot create role with preset name: {name}")
|
||||
|
||||
# Check for duplicate name
|
||||
existing = (
|
||||
db.query(Role)
|
||||
.filter(Role.store_id == store_id, Role.name == name)
|
||||
.first()
|
||||
)
|
||||
if existing:
|
||||
raise InvalidRoleException(f"Role '{name}' already exists for this store")
|
||||
|
||||
# Validate permissions exist
|
||||
valid_ids = permission_discovery_service.get_all_permission_ids()
|
||||
invalid = set(permissions) - valid_ids
|
||||
if invalid:
|
||||
raise InvalidRoleException(f"Invalid permission IDs: {', '.join(sorted(invalid))}")
|
||||
|
||||
role = Role(
|
||||
store_id=store_id,
|
||||
name=name,
|
||||
permissions=permissions,
|
||||
)
|
||||
db.add(role)
|
||||
db.flush()
|
||||
|
||||
audit_aggregator.log(
|
||||
db=db,
|
||||
admin_user_id=actor_user_id,
|
||||
action="role.create",
|
||||
target_type="role",
|
||||
target_id=str(role.id),
|
||||
details={"name": name, "permissions_count": len(permissions), "store_id": store_id},
|
||||
)
|
||||
|
||||
return role
|
||||
|
||||
def update_role(
|
||||
self,
|
||||
db: Session,
|
||||
store_id: int,
|
||||
role_id: int,
|
||||
name: str | None = None,
|
||||
permissions: list[str] | None = None,
|
||||
actor_user_id: int | None = None,
|
||||
) -> Role:
|
||||
"""
|
||||
Update a role's name and/or permissions.
|
||||
|
||||
Args:
|
||||
db: Database session
|
||||
store_id: Store ID (for ownership check)
|
||||
role_id: Role ID to update
|
||||
name: New role name (optional)
|
||||
permissions: New permission list (optional)
|
||||
|
||||
Returns:
|
||||
Updated Role object
|
||||
|
||||
Raises:
|
||||
InvalidRoleException: If role not found or name conflicts
|
||||
"""
|
||||
role = (
|
||||
db.query(Role)
|
||||
.filter(Role.id == role_id, Role.store_id == store_id)
|
||||
.first()
|
||||
)
|
||||
if not role:
|
||||
raise InvalidRoleException(f"Role {role_id} not found for this store")
|
||||
|
||||
if name is not None:
|
||||
if name.lower() in self.PRESET_ROLE_NAMES and role.name.lower() != name.lower():
|
||||
raise InvalidRoleException(f"Cannot rename to preset name: {name}")
|
||||
# Check duplicate
|
||||
duplicate = (
|
||||
db.query(Role)
|
||||
.filter(
|
||||
Role.store_id == store_id,
|
||||
Role.name == name,
|
||||
Role.id != role_id,
|
||||
)
|
||||
.first()
|
||||
)
|
||||
if duplicate:
|
||||
raise InvalidRoleException(f"Role '{name}' already exists for this store")
|
||||
role.name = name
|
||||
|
||||
if permissions is not None:
|
||||
valid_ids = permission_discovery_service.get_all_permission_ids()
|
||||
invalid = set(permissions) - valid_ids
|
||||
if invalid:
|
||||
raise InvalidRoleException(
|
||||
f"Invalid permission IDs: {', '.join(sorted(invalid))}"
|
||||
)
|
||||
old_permissions = role.permissions or []
|
||||
role.permissions = permissions
|
||||
|
||||
db.flush()
|
||||
|
||||
details = {"role_name": role.name, "store_id": store_id}
|
||||
if name is not None:
|
||||
details["new_name"] = name
|
||||
if permissions is not None:
|
||||
added = set(permissions) - set(old_permissions)
|
||||
removed = set(old_permissions) - set(permissions)
|
||||
if added:
|
||||
details["permissions_added"] = sorted(added)
|
||||
if removed:
|
||||
details["permissions_removed"] = sorted(removed)
|
||||
|
||||
audit_aggregator.log(
|
||||
db=db,
|
||||
admin_user_id=actor_user_id,
|
||||
action="role.update",
|
||||
target_type="role",
|
||||
target_id=str(role.id),
|
||||
details=details,
|
||||
)
|
||||
|
||||
return role
|
||||
|
||||
def delete_role(
|
||||
self,
|
||||
db: Session,
|
||||
store_id: int,
|
||||
role_id: int,
|
||||
actor_user_id: int | None = None,
|
||||
) -> None:
|
||||
"""
|
||||
Delete a custom role. Preset roles cannot be deleted.
|
||||
|
||||
Args:
|
||||
db: Database session
|
||||
store_id: Store ID (for ownership check)
|
||||
role_id: Role ID to delete
|
||||
|
||||
Raises:
|
||||
InvalidRoleException: If role not found or is a preset role
|
||||
"""
|
||||
role = (
|
||||
db.query(Role)
|
||||
.filter(Role.id == role_id, Role.store_id == store_id)
|
||||
.first()
|
||||
)
|
||||
if not role:
|
||||
raise InvalidRoleException(f"Role {role_id} not found for this store")
|
||||
|
||||
if role.name.lower() in self.PRESET_ROLE_NAMES:
|
||||
raise InvalidRoleException(f"Cannot delete preset role: {role.name}")
|
||||
|
||||
# Check if any team members use this role
|
||||
members_with_role = (
|
||||
db.query(StoreUser)
|
||||
.filter(StoreUser.store_id == store_id, StoreUser.role_id == role_id)
|
||||
.count()
|
||||
)
|
||||
if members_with_role > 0:
|
||||
raise InvalidRoleException(
|
||||
f"Cannot delete role: {members_with_role} team member(s) still assigned"
|
||||
)
|
||||
|
||||
role_name = role.name
|
||||
db.delete(role)
|
||||
db.flush()
|
||||
|
||||
audit_aggregator.log(
|
||||
db=db,
|
||||
admin_user_id=actor_user_id,
|
||||
action="role.delete",
|
||||
target_type="role",
|
||||
target_id=str(role_id),
|
||||
details={"role_name": role_name, "store_id": store_id},
|
||||
)
|
||||
|
||||
# Private helper methods
|
||||
|
||||
def _generate_invitation_token(self) -> str:
|
||||
|
||||
Reference in New Issue
Block a user