diff --git a/wavefront/server/modules/auth_module/auth_module/controllers/superset_controller.py b/wavefront/server/modules/auth_module/auth_module/controllers/superset_controller.py index 0c53599e..e12f2fa9 100644 --- a/wavefront/server/modules/auth_module/auth_module/controllers/superset_controller.py +++ b/wavefront/server/modules/auth_module/auth_module/controllers/superset_controller.py @@ -56,13 +56,13 @@ async def superset_authenticator( user_id=user_id, scope=ResourceScope.DATA ) - if data_filters and len(data_filters) < 1: - return JSONResponse( - status_code=status.HTTP_403_FORBIDDEN, - content=response_formatter.buildErrorResponse( - 'Data access not set for user' - ), - ) + if not data_filters: + return JSONResponse( + status_code=status.HTTP_403_FORBIDDEN, + content=response_formatter.buildErrorResponse( + 'Data access not set for user' + ), + ) guest_token = superset_service.generate_guest_token( user_id, dashboards, data_filters, filter diff --git a/wavefront/server/modules/user_management_module/tests/test_access_controller.py b/wavefront/server/modules/user_management_module/tests/test_access_controller.py index d27b8539..d99644cc 100644 --- a/wavefront/server/modules/user_management_module/tests/test_access_controller.py +++ b/wavefront/server/modules/user_management_module/tests/test_access_controller.py @@ -479,6 +479,236 @@ async def test_get_roles_scope_filter_excludes_cross_scope_composite( assert role_names == {'route_only_role'} +@pytest.mark.asyncio +async def test_patch_role_resources_pure_console_role_rejected( + test_client, + test_session: AsyncSession, + mock_auth_admin_functions, + test_user_id, + test_session_id, + auth_token, +): + """A single-resource role whose only resource is console-scoped is a console + role (UI identity marker) and its resources must not be editable.""" + await create_session(test_session, test_user_id, test_session_id) + + console_resource = Resource( + id=str(uuid.uuid4()), + key='console_resource', + value='viewer_resource', + description='Console resource', + scope=ResourceScope.CONSOLE, + ) + data_resource = Resource( + id=str(uuid.uuid4()), + key='branch', + value='mumbai', + description='Data resource', + scope=ResourceScope.DATA, + ) + console_role = Role( + id=str(uuid.uuid4()), + name='console_role', + description='Pure console role', + ) + + async with test_session() as session: + session.add_all([console_resource, data_resource, console_role]) + await session.commit() + + async with test_session() as session: + session.add( + RoleResource(role_id=console_role.id, resource_id=console_resource.id) + ) + await session.commit() + + response = test_client.patch( + f'/floware/v1/access/roles/{console_role.id}', + json={'resources': [data_resource.id]}, + headers={'Authorization': f'Bearer {auth_token}'}, + ) + assert response.status_code == 400 + assert ( + 'cannot update resources of a console role' + in response.json()['meta']['error'].lower() + ) + + +@pytest.mark.asyncio +async def test_patch_role_resources_composite_with_console_allowed( + test_client, + test_session: AsyncSession, + mock_auth_admin_functions, + test_user_id, + test_session_id, + auth_token, +): + """A composite role (2+ resources) that merely includes a console resource is + not a console role and must remain editable.""" + await create_session(test_session, test_user_id, test_session_id) + + console_resource = Resource( + id=str(uuid.uuid4()), + key='console_resource', + value='viewer_resource', + description='Console resource', + scope=ResourceScope.CONSOLE, + ) + data_resource = Resource( + id=str(uuid.uuid4()), + key='branch', + value='mumbai', + description='Data resource', + scope=ResourceScope.DATA, + ) + route_resource = Resource( + id=str(uuid.uuid4()), + key='route', + value='agents', + description='Route resource', + scope=ResourceScope.ROUTE, + ) + composite_role = Role( + id=str(uuid.uuid4()), + name='composite_role', + description='Composite role including a console resource', + ) + + async with test_session() as session: + session.add_all( + [console_resource, data_resource, route_resource, composite_role] + ) + await session.commit() + + async with test_session() as session: + session.add_all( + [ + RoleResource( + role_id=composite_role.id, resource_id=console_resource.id + ), + RoleResource(role_id=composite_role.id, resource_id=data_resource.id), + ] + ) + await session.commit() + + response = test_client.patch( + f'/floware/v1/access/roles/{composite_role.id}', + json={'resources': [data_resource.id, route_resource.id]}, + headers={'Authorization': f'Bearer {auth_token}'}, + ) + assert response.status_code == 200 + + async with test_session() as session: + result = await session.execute( + select(RoleResource.resource_id).where( + RoleResource.role_id == composite_role.id + ) + ) + resource_ids = {row[0] for row in result.all()} + assert resource_ids == {data_resource.id, route_resource.id} + + +@pytest.mark.asyncio +async def test_delete_role_pure_console_role_rejected( + test_client, + test_session: AsyncSession, + mock_auth_admin_functions, + test_user_id, + test_session_id, + auth_token, +): + """A pure single console-resource role must not be deletable here.""" + await create_session(test_session, test_user_id, test_session_id) + + console_resource = Resource( + id=str(uuid.uuid4()), + key='console_resource', + value='viewer_resource', + description='Console resource', + scope=ResourceScope.CONSOLE, + ) + console_role = Role( + id=str(uuid.uuid4()), + name='console_role', + description='Pure console role', + ) + + async with test_session() as session: + session.add_all([console_resource, console_role]) + await session.commit() + + async with test_session() as session: + session.add( + RoleResource(role_id=console_role.id, resource_id=console_resource.id) + ) + await session.commit() + + response = test_client.delete( + f'/floware/v1/access/roles/{console_role.id}', + headers={'Authorization': f'Bearer {auth_token}'}, + ) + assert response.status_code == 400 + assert 'cannot delete a console role' in response.json()['meta']['error'].lower() + + +@pytest.mark.asyncio +async def test_delete_role_composite_with_console_allowed( + test_client, + test_session: AsyncSession, + mock_auth_admin_functions, + test_user_id, + test_session_id, + auth_token, +): + """A composite role that includes a console resource must remain deletable.""" + await create_session(test_session, test_user_id, test_session_id) + + console_resource = Resource( + id=str(uuid.uuid4()), + key='console_resource', + value='viewer_resource', + description='Console resource', + scope=ResourceScope.CONSOLE, + ) + data_resource = Resource( + id=str(uuid.uuid4()), + key='branch', + value='mumbai', + description='Data resource', + scope=ResourceScope.DATA, + ) + composite_role = Role( + id=str(uuid.uuid4()), + name='composite_role', + description='Composite role including a console resource', + ) + + async with test_session() as session: + session.add_all([console_resource, data_resource, composite_role]) + await session.commit() + + async with test_session() as session: + session.add_all( + [ + RoleResource( + role_id=composite_role.id, resource_id=console_resource.id + ), + RoleResource(role_id=composite_role.id, resource_id=data_resource.id), + ] + ) + await session.commit() + + response = test_client.delete( + f'/floware/v1/access/roles/{composite_role.id}', + headers={'Authorization': f'Bearer {auth_token}'}, + ) + assert response.status_code == 200 + + async with test_session() as session: + result = await session.execute(select(Role).where(Role.id == composite_role.id)) + assert result.scalar_one_or_none() is None + + @pytest.mark.asyncio async def test_create_role_invalid_resources( test_client, diff --git a/wavefront/server/modules/user_management_module/user_management_module/controllers/access_controller.py b/wavefront/server/modules/user_management_module/user_management_module/controllers/access_controller.py index f966c4f0..0ee100ce 100644 --- a/wavefront/server/modules/user_management_module/user_management_module/controllers/access_controller.py +++ b/wavefront/server/modules/user_management_module/user_management_module/controllers/access_controller.py @@ -11,6 +11,7 @@ from fastapi import Request from fastapi import status from fastapi.responses import JSONResponse +from sqlalchemy import delete from sqlalchemy import func from sqlalchemy import or_ from sqlalchemy import Result @@ -193,16 +194,25 @@ async def create_role( ) role_id = None - # Check if a role already exists for the given resources + # Check if a role already exists with the EXACT same resource set. Grouping + # spans every resource of each role (no WHERE filter) so a superset role — + # one containing all the payload resources plus others — is not treated as a + # duplicate. A role matches only when its total resource count equals the + # payload size and every one of those resources is in the payload. async with role_resource_repository.session() as session: stmt = ( select(RoleResource.role_id) - .where(RoleResource.resource_id.in_(payload.resources)) .group_by(RoleResource.role_id) .having( func.count(func.distinct(RoleResource.resource_id)) == len(payload.resources) ) + .having( + func.count(func.distinct(RoleResource.resource_id)).filter( + RoleResource.resource_id.in_(payload.resources) + ) + == len(payload.resources) + ) ) result: Result = await session.execute(stmt) role_id = result.scalar() @@ -522,7 +532,40 @@ async def delete_resources( 'Resource not found with the given ID.' ), ) - await resource_repository.delete_all(id=resource_id) + + async with resource_repository.session() as session: + async with session.begin(): + linked_role_ids = ( + await session.scalars( + select(RoleResource.role_id).where( + RoleResource.resource_id == resource_id + ) + ) + ).all() + + # Deleting the resource cascades its role_resource links. + await resource_repository.delete_all(id=resource_id, session=session) + + if linked_role_ids: + remaining_role_ids = set( + ( + await session.scalars( + select(RoleResource.role_id).where( + RoleResource.role_id.in_(linked_role_ids) + ) + ) + ).all() + ) + orphaned_role_ids = [ + role_id + for role_id in linked_role_ids + if role_id not in remaining_role_ids + ] + if orphaned_role_ids: + await session.execute( + delete(Role).where(Role.id.in_(orphaned_role_ids)) + ) + return JSONResponse( status_code=status.HTTP_200_OK, content=response_formatter.buildSuccessResponse( @@ -560,18 +603,26 @@ async def patch_role_resources( ) # Console roles are the user's UI identity marker and are managed separately, - # so their resource assignments must not be edited here. + # so their resource assignments must not be edited here. A console role is a + # single-resource role whose only resource is console-scoped; a composite role + # (2+ resources) that merely includes a console resource is still editable. async with role_resource_repository.session() as session: - console_stmt = ( - select(func.count()) + counts_stmt = ( + select( + func.count(RoleResource.resource_id), + func.count(RoleResource.resource_id).filter( + Resource.scope == ResourceScope.CONSOLE + ), + ) .select_from(RoleResource) .join(Resource, Resource.id == RoleResource.resource_id) .where(RoleResource.role_id == role_id) - .where(Resource.scope == ResourceScope.CONSOLE) ) - console_resource_count = (await session.execute(console_stmt)).scalar() or 0 + total_resource_count, console_resource_count = ( + await session.execute(counts_stmt) + ).one() - if console_resource_count > 0: + if console_resource_count > 0 and total_resource_count == 1: return JSONResponse( status_code=status.HTTP_400_BAD_REQUEST, content=response_formatter.buildErrorResponse( @@ -634,18 +685,26 @@ async def delete_role( ) # Console roles are the user's UI identity marker and are managed separately, - # so they must not be deleted here. + # so they must not be deleted here. A console role is a single-resource role + # whose only resource is console-scoped; a composite role (2+ resources) that + # merely includes a console resource is still deletable. async with role_resource_repository.session() as session: - console_stmt = ( - select(func.count()) + counts_stmt = ( + select( + func.count(RoleResource.resource_id), + func.count(RoleResource.resource_id).filter( + Resource.scope == ResourceScope.CONSOLE + ), + ) .select_from(RoleResource) .join(Resource, Resource.id == RoleResource.resource_id) .where(RoleResource.role_id == role_id) - .where(Resource.scope == ResourceScope.CONSOLE) ) - console_resource_count = (await session.execute(console_stmt)).scalar() or 0 + total_resource_count, console_resource_count = ( + await session.execute(counts_stmt) + ).one() - if console_resource_count > 0: + if console_resource_count > 0 and total_resource_count == 1: return JSONResponse( status_code=status.HTTP_400_BAD_REQUEST, content=response_formatter.buildErrorResponse(