Skip to content

Commit

Permalink
[To Main] Add metadata management role (#2506)
Browse files Browse the repository at this point in the history
* Adding metadata management role

* fixing unit test for sidenav

* Removing metadata management role from engagement metadata
  • Loading branch information
VineetBala-AOT authored May 14, 2024
1 parent 1b990ba commit a94a65a
Show file tree
Hide file tree
Showing 10 changed files with 129 additions and 32 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.MD
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@
- Fixed the date formatting issue and using the end_date in the email preview
- Updated unit tests

- **Feature** Create role for metadata management [🎟️ DESENG-603](https://apps.itsm.gov.bc.ca/jira/browse/DESENG-603)
- Implemented a new role named "manage_metadata" within the Admin group to restrci access for metadata management
- Updated the frontend to restrict access to the "metadata management" link in the menu for users without the newly added role
- Backend changes to incorporate the new role for access control purposes, ensuring only authorized users can perform metadata management actions

## May 10, 2024

- **Bugfix** Language picker should only appear on engagements with more than one language [🎟️ DESENG-575](https://apps.itsm.gov.bc.ca/jira/browse/DESENG-575)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
"""add_manage_metadata_role
Revision ID: 1407e0ad88f6
Revises: 614b5376f19c
Create Date: 2024-05-09 20:58:49.586171
"""
from alembic import op
import sqlalchemy as sa


# revision identifiers, used by Alembic.
revision = '1407e0ad88f6'
down_revision = '614b5376f19c'
branch_labels = None
depends_on = None


def upgrade():
# ### commands auto generated by Alembic - please adjust! ###
op.execute("INSERT INTO user_role (created_date, updated_date, id, name, description) VALUES ('{0}', '{0}', 38, 'manage_metadata', 'Role to access metadata management')".format(sa.func.now()))
op.execute("INSERT INTO group_role_mapping (created_date, updated_date, id, role_id, group_id) VALUES ('{0}', '{0}', 60, 38, 1)".format(sa.func.now()))
# ### end Alembic commands ###


def downgrade():
# ### commands auto generated by Alembic - please adjust! ###
op.execute("DELETE FROM group_role_mapping WHERE id = 60")
op.execute("DELETE FROM user_role WHERE id = 38")
# ### end Alembic commands ###
70 changes: 50 additions & 20 deletions met-api/src/met_api/resources/engagement_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,14 @@
from flask_restx import Namespace, Resource, fields
from marshmallow import ValidationError
from met_api.auth import auth_methods
from met_api.auth import jwt as _jwt
from met_api.constants.membership_type import MembershipType
from met_api.services import authorization
from met_api.services.engagement_service import EngagementService
from met_api.services.engagement_metadata_service import EngagementMetadataService
from met_api.utils.roles import Role
from met_api.utils.tenant_validator import require_role
from met_api.utils.util import allowedorigins, cors_preflight

EDIT_ENGAGEMENT_ROLES = [Role.EDIT_ENGAGEMENT.value]
VIEW_ENGAGEMENT_ROLES = [
Role.VIEW_ENGAGEMENT.value, Role.EDIT_ENGAGEMENT.value]

API = Namespace('engagement_metadata',
path='/engagements/<engagement_id>/metadata',
Expand Down Expand Up @@ -75,9 +73,16 @@ class EngagementMetadata(Resource):
@cross_origin(origins=allowedorigins())
@API.doc(security='apikey')
@API.marshal_list_with(metadata_return_model)
@require_role(VIEW_ENGAGEMENT_ROLES)
@_jwt.requires_auth
def get(engagement_id):
"""Fetch engagement metadata entries by engagement id."""
authorization.check_auth(
one_of_roles=(
MembershipType.TEAM_MEMBER.name,
Role.VIEW_ENGAGEMENT.value
),
engagement_id=engagement_id
)
return metadata_service.get_by_engagement(engagement_id)

@staticmethod
Expand All @@ -86,11 +91,16 @@ def get(engagement_id):
@API.expect(metadata_create_model)
# type: ignore
@API.marshal_with(metadata_return_model, code=HTTPStatus.CREATED)
@require_role(EDIT_ENGAGEMENT_ROLES)
@_jwt.requires_auth
def post(engagement_id: int):
"""Create a new metadata entry for an engagement."""
authorization.check_auth(one_of_roles=EDIT_ENGAGEMENT_ROLES,
engagement_id=engagement_id)
authorization.check_auth(
one_of_roles=(
MembershipType.TEAM_MEMBER.name,
Role.EDIT_ENGAGEMENT.value
),
engagement_id=engagement_id
)
request_json = request.get_json(force=True)
try:
engagement_metadata = metadata_service.create(
Expand All @@ -107,11 +117,16 @@ def post(engagement_id: int):
@API.doc(security='apikey')
@API.expect(metadata_bulk_update_model, validate=True)
@API.marshal_list_with(metadata_return_model)
@require_role(EDIT_ENGAGEMENT_ROLES)
@_jwt.requires_auth
def patch(engagement_id):
"""Update the values of existing metadata entries for an engagement."""
authorization.check_auth(one_of_roles=EDIT_ENGAGEMENT_ROLES,
engagement_id=engagement_id)
authorization.check_auth(
one_of_roles=(
MembershipType.TEAM_MEMBER.name,
Role.EDIT_ENGAGEMENT.value
),
engagement_id=engagement_id
)
data = request.get_json(force=True)
taxon_id = data['taxon_id']
updated_values = data['values']
Expand All @@ -131,11 +146,16 @@ class EngagementMetadataById(Resource):

@staticmethod
@cross_origin(origins=allowedorigins())
@require_role(VIEW_ENGAGEMENT_ROLES)
@_jwt.requires_auth
def get(engagement_id, metadata_id):
"""Fetch an engagement metadata entry by id."""
authorization.check_auth(one_of_roles=VIEW_ENGAGEMENT_ROLES,
engagement_id=engagement_id)
authorization.check_auth(
one_of_roles=(
MembershipType.TEAM_MEMBER.name,
Role.VIEW_ENGAGEMENT.value
),
engagement_id=engagement_id
)
try:
metadata = metadata_service.get(metadata_id)
if str(metadata['engagement_id']) != str(engagement_id):
Expand All @@ -150,12 +170,17 @@ def get(engagement_id, metadata_id):

@staticmethod
@cross_origin(origins=allowedorigins())
@require_role(EDIT_ENGAGEMENT_ROLES)
@API.expect(metadata_update_model)
@_jwt.requires_auth
def patch(engagement_id, metadata_id):
"""Update the values of an existing metadata entry for an engagement."""
authorization.check_auth(one_of_roles=EDIT_ENGAGEMENT_ROLES,
engagement_id=engagement_id)
authorization.check_auth(
one_of_roles=(
MembershipType.TEAM_MEMBER.name,
Role.EDIT_ENGAGEMENT.value
),
engagement_id=engagement_id
)
request_json = request.get_json(force=True)
try:
value = request_json.get('value')
Expand All @@ -175,12 +200,17 @@ def patch(engagement_id, metadata_id):

@staticmethod
@cross_origin(origins=allowedorigins())
@require_role(EDIT_ENGAGEMENT_ROLES)
@_jwt.requires_auth
def delete(engagement_id, metadata_id):
"""Delete an existing metadata entry for an engagement."""
try:
authorization.check_auth(one_of_roles=EDIT_ENGAGEMENT_ROLES,
engagement_id=engagement_id)
authorization.check_auth(
one_of_roles=(
MembershipType.TEAM_MEMBER.name,
Role.EDIT_ENGAGEMENT.value
),
engagement_id=engagement_id
)
metadata = metadata_service.get(metadata_id)
if str(metadata['engagement_id']) != str(engagement_id):
raise ValidationError('Metadata does not belong to this '
Expand Down
14 changes: 6 additions & 8 deletions met-api/src/met_api/resources/metadata_taxon.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@
from met_api.utils.util import allowedorigins, cors_preflight


VIEW_TAXA_ROLES = [Role.VIEW_ENGAGEMENT.value, Role.CREATE_ENGAGEMENT.value]
MODIFY_TAXA_ROLES = [Role.EDIT_ENGAGEMENT.value]
MANAGE_METADATA_ROLE = [Role.MANAGE_METADATA.value]
TAXON_NOT_FOUND_MSG = 'Metadata taxon was not found'

API = Namespace('metadata_taxa', description='Endpoints for managing the taxa '
Expand Down Expand Up @@ -117,7 +116,6 @@ class MetadataTaxa(Resource):
@cross_origin(origins=allowedorigins())
@API.marshal_list_with(taxon_return_model)
@ensure_tenant_access()
@require_role(VIEW_TAXA_ROLES)
def get(tenant: Tenant):
"""Fetch a list of metadata taxa for the current tenant."""
tenant_taxa = taxon_service.get_by_tenant(tenant.id)
Expand All @@ -129,7 +127,7 @@ def get(tenant: Tenant):
# type: ignore
@API.marshal_with(taxon_return_model, code=HTTPStatus.CREATED)
@ensure_tenant_access()
@require_role(MODIFY_TAXA_ROLES)
@require_role(MANAGE_METADATA_ROLE)
def post(tenant: Tenant):
"""Create a new metadata taxon for a tenant and return it."""
request_json = request.get_json(force=True)
Expand All @@ -146,7 +144,7 @@ def post(tenant: Tenant):
@API.expect(taxon_ids_model)
@API.marshal_list_with(taxon_return_model)
@ensure_tenant_access()
@require_role(MODIFY_TAXA_ROLES)
@require_role(MANAGE_METADATA_ROLE)
def patch(tenant: Tenant):
"""Reorder the tenant's metadata taxa."""
request_json = request.get_json(force=True)
Expand All @@ -173,7 +171,7 @@ class MetadataTaxon(Resource):
@staticmethod
@cross_origin(origins=allowedorigins())
@ensure_tenant_access()
@require_role(VIEW_TAXA_ROLES)
@require_role(MANAGE_METADATA_ROLE)
@API.marshal_with(taxon_return_model)
def get(tenant: Tenant, taxon_id: int):
"""Fetch a single metadata taxon matching the provided id."""
Expand All @@ -187,7 +185,7 @@ def get(tenant: Tenant, taxon_id: int):
@API.expect(taxon_modify_model)
@API.marshal_with(taxon_return_model)
@ensure_tenant_access()
@require_role(MODIFY_TAXA_ROLES)
@require_role(MANAGE_METADATA_ROLE)
def patch(tenant: Tenant, taxon_id: int):
"""Update a metadata taxon."""
metadata_taxon = taxon_service.get_by_id(taxon_id)
Expand All @@ -199,7 +197,7 @@ def patch(tenant: Tenant, taxon_id: int):
@staticmethod
@cross_origin(origins=allowedorigins())
@ensure_tenant_access()
@require_role(MODIFY_TAXA_ROLES)
@require_role(MANAGE_METADATA_ROLE)
@API.doc(responses={**responses, HTTPStatus.NO_CONTENT.value: 'Taxon deleted'})
def delete(tenant: Tenant, taxon_id: int):
"""Delete a metadata taxon."""
Expand Down
1 change: 1 addition & 0 deletions met-api/src/met_api/utils/roles.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,4 @@ class Role(Enum):
EXPORT_INTERNAL_COMMENT_SHEET = 'export_internal_comment_sheet'
EXPORT_PROPONENT_COMMENT_SHEET = 'export_proponent_comment_sheet'
SUPER_ADMIN = 'super_admin'
MANAGE_METADATA = 'manage_metadata'
31 changes: 30 additions & 1 deletion met-api/tests/unit/api/test_engagement_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ def test_add_engagement_metadata_invalid_user(client, jwt, session):
headers=headers,
data=json.dumps(metadata_info),
content_type=ContentType.JSON.value)
assert response.status_code == HTTPStatus.UNAUTHORIZED
assert response.status_code == HTTPStatus.FORBIDDEN


def test_update_engagement_metadata(client, jwt, session):
Expand All @@ -131,6 +131,13 @@ def test_update_engagement_metadata(client, jwt, session):
assert response.json.get('engagement_id') == engagement.id
assert response.json.get('value') == 'new value'

# Test if Metadata does not belong to the engagement
engagement_id = fake.pyint()
response = client.get(f'/api/engagements/{engagement_id}/metadata/{metadata.id}',
headers=headers,
content_type=ContentType.JSON.value)
assert response.status_code == HTTPStatus.BAD_REQUEST


def test_bulk_update_engagement_metadata(client, jwt, session):
"""Test that metadata values can be updated in bulk."""
Expand Down Expand Up @@ -182,3 +189,25 @@ def test_delete_engagement_metadata(client, jwt, session):
content_type=ContentType.JSON.value)
assert response.status_code == HTTPStatus.NO_CONTENT, (f'Wrong response code; '
f'HTTP {response.status_code} -> {response.text}')


def test_get_engagement_metadata_by_id(client, jwt, session):
"""Test that metadata can be fetched by id."""
taxon, engagement, _, headers = factory_metadata_requirements(jwt)
metadata = factory_engagement_metadata_model({
'engagement_id': engagement.id,
'taxon_id': taxon.id,
'value': fake.sentence(),
})
response = client.get(f'/api/engagements/{engagement.id}/metadata/{metadata.id}',
headers=headers,
content_type=ContentType.JSON.value)
assert response.status_code == HTTPStatus.OK, (f'Wrong response code; '
f'HTTP {response.status_code} -> {response.text}')

# Test if Metadata does not belong to the engagement
engagement_id = fake.pyint()
response = client.get(f'/api/engagements/{engagement_id}/metadata/{metadata.id}',
headers=headers,
content_type=ContentType.JSON.value)
assert response.status_code == HTTPStatus.BAD_REQUEST
4 changes: 2 additions & 2 deletions met-web/src/components/layout/SideNav/SideNavElements.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ export const Routes: Route[] = [
name: 'Metadata Management',
path: '/metadatamanagement',
base: '/metadatamanagement',
authenticated: false,
allowedRoles: [],
authenticated: true,
allowedRoles: [USER_ROLES.MANAGE_METADATA],
},
{
name: 'User Management',
Expand Down
4 changes: 3 additions & 1 deletion met-web/src/routes/AuthenticatedRoutes.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,9 @@ const AuthenticatedRoutes = () => {
<Route path="/:slug/dashboard/:dashboardType" element={<PublicDashboard />} />
<Route path="/engagements/:engagementId/dashboard/:dashboardType" element={<PublicDashboard />} />
<Route path="/:slug/comments/:dashboardType" element={<EngagementComments />} />
<Route path="/metadatamanagement" element={<MetadataManagement />} />
<Route element={<AuthGate allowedRoles={[USER_ROLES.MANAGE_METADATA]} />}>
<Route path="/metadatamanagement" element={<MetadataManagement />} />
</Route>
<Route element={<AuthGate allowedRoles={[USER_ROLES.SUPER_ADMIN]} />}>
<Route path="/tenantadmin" element={<TenantManagement />} />
</Route>
Expand Down
1 change: 1 addition & 0 deletions met-web/src/services/userService/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export const USER_ROLES = {
EXPORT_ALL_TO_CSV: 'export_all_to_csv',
EXPORT_INTERNAL_COMMENT_SHEET: 'export_internal_comment_sheet',
EXPORT_PROPONENT_COMMENT_SHEET: 'export_proponent_comment_sheet',
MANAGE_METADATA: 'manage_metadata',
};

export type UserStatusName = 'ACTIVE' | 'INACTIVE';
Expand Down
1 change: 1 addition & 0 deletions met-web/tests/unit/components/sidenav.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ jest.mock('react-redux', () => ({
USER_ROLES.VIEW_USERS,
USER_ROLES.VIEW_FEEDBACKS,
USER_ROLES.SUPER_ADMIN,
USER_ROLES.MANAGE_METADATA,
];
}),
}));
Expand Down

0 comments on commit a94a65a

Please sign in to comment.