From ddded17535400d2fabacedcb7af0546d2c89a55f Mon Sep 17 00:00:00 2001 From: saravanpa-aot Date: Thu, 10 Aug 2023 09:16:03 -0700 Subject: [PATCH] Added auth checks on the comments (#1984) * Added auth checks on the comments * Added auth checks on the comments * Added auth checks on the comments --- met-api/src/met_api/models/submission.py | 5 -- met-api/src/met_api/resources/submission.py | 5 +- .../src/met_api/services/comment_service.py | 4 +- .../met_api/services/submission_service.py | 59 ++++++++++++------- met-api/src/met_api/utils/roles.py | 3 +- met-api/tests/unit/api/test_comment.py | 59 ++++++++++++++++++- 6 files changed, 99 insertions(+), 36 deletions(-) diff --git a/met-api/src/met_api/models/submission.py b/met-api/src/met_api/models/submission.py index 1ae1fde29..40a0cb559 100644 --- a/met-api/src/met_api/models/submission.py +++ b/met-api/src/met_api/models/submission.py @@ -41,11 +41,6 @@ class Submission(BaseModel): # pylint: disable=too-few-public-methods comments = db.relationship('Comment', backref='submission', cascade='all, delete') staff_note = db.relationship('StaffNote', backref='submission', cascade='all, delete') - @classmethod - def get(cls, submission_id) -> Submission: - """Get a submission by id.""" - return db.session.query(Submission).filter_by(id=submission_id).first() - @classmethod def get_by_survey_id(cls, survey_id) -> List[SubmissionSchema]: """Get submissions by survey id.""" diff --git a/met-api/src/met_api/resources/submission.py b/met-api/src/met_api/resources/submission.py index 6a5e2f3d2..4a700259a 100644 --- a/met-api/src/met_api/resources/submission.py +++ b/met-api/src/met_api/resources/submission.py @@ -25,7 +25,6 @@ from met_api.schemas.submission import SubmissionSchema from met_api.services.submission_service import SubmissionService from met_api.utils.token_info import TokenInfo -from met_api.utils.roles import Role from met_api.utils.util import allowedorigins, cors_preflight @@ -41,7 +40,7 @@ class Submission(Resource): @staticmethod @cross_origin(origins=allowedorigins()) - @_jwt.has_one_of_roles([Role.VIEW_UNAPPROVED_COMMENTS.value]) + @_jwt.requires_auth def get(submission_id): """Fetch a single submission.""" try: @@ -54,7 +53,7 @@ def get(submission_id): @staticmethod @cross_origin(origins=allowedorigins()) - @_jwt.has_one_of_roles([Role.REVIEW_COMMENTS.value]) + @_jwt.requires_auth def put(submission_id): """Update comment status by submission id.""" try: diff --git a/met-api/src/met_api/services/comment_service.py b/met-api/src/met_api/services/comment_service.py index 91bc4d39c..2422543cf 100644 --- a/met-api/src/met_api/services/comment_service.py +++ b/met-api/src/met_api/services/comment_service.py @@ -37,7 +37,7 @@ def get_comment(comment_id) -> CommentSchema: def get_comments_by_submission(submission_id) -> CommentSchema: """Get Comment by the id.""" comments = Comment.get_by_submission(submission_id) - submission = SubmissionModel.get(submission_id) + submission = SubmissionModel.find_by_id(submission_id) if submission.comment_status_id != Status.Approved.value: can_view_unapproved_comments = CommentService.can_view_unapproved_comments(submission.survey_id) if not can_view_unapproved_comments: @@ -52,7 +52,7 @@ def can_view_unapproved_comments(survey_id: int) -> bool: return False user_roles = TokenInfo.get_user_roles() - if Role.VIEW_UNAPPROVED_COMMENTS.value in user_roles: + if Role.REVIEW_COMMENTS.value in user_roles: return True engagement = SurveyModel.find_by_id(survey_id) diff --git a/met-api/src/met_api/services/submission_service.py b/met-api/src/met_api/services/submission_service.py index 4ea0d994f..a801dcbaf 100644 --- a/met-api/src/met_api/services/submission_service.py +++ b/met-api/src/met_api/services/submission_service.py @@ -12,13 +12,14 @@ from met_api.exceptions.business_exception import BusinessException from met_api.models import Engagement as EngagementModel from met_api.models import Survey as SurveyModel +from met_api.models import Tenant as TenantModel from met_api.models.comment import Comment from met_api.models.comment_status import CommentStatus from met_api.models.db import session_scope from met_api.models.pagination_options import PaginationOptions from met_api.models.participant import Participant as ParticipantModel from met_api.models.staff_note import StaffNote -from met_api.models.submission import Submission +from met_api.models.submission import Submission as SubmissionModel from met_api.schemas.submission import PublicSubmissionSchema, SubmissionSchema from met_api.services import authorization from met_api.services.comment_service import CommentService @@ -28,7 +29,6 @@ from met_api.utils import notification from met_api.utils.roles import Role from met_api.utils.template import Template -from met_api.models import Tenant as TenantModel class SubmissionService: @@ -39,15 +39,34 @@ class SubmissionService: @classmethod def get(cls, submission_id): """Get submission by the id.""" - db_data = Submission.get(submission_id) - return SubmissionSchema(exclude=['submission_json']).dump(db_data) + submission: SubmissionModel = SubmissionModel.find_by_id(submission_id) + + # if submission is approved , anyone can see .No need to do auth + if submission.comment_status_id != Status.Approved.value: + cls._check_comment_auth(submission) + + return SubmissionSchema(exclude=['submission_json']).dump(submission) + + @classmethod + def _check_comment_auth(cls, submission): + """Verify comment auth.""" + survey: SurveyModel = SurveyModel.find_by_id(submission.survey_id) + engagement: EngagementModel = EngagementModel.find_by_id( + survey.engagement_id) + # TM can see all comments if assigned + # who ever has REVIEW_COMMENTS + one_of_roles = ( + MembershipType.TEAM_MEMBER.name, + Role.REVIEW_COMMENTS.value + ) + authorization.check_auth(one_of_roles=one_of_roles, engagement_id=engagement.id) @classmethod def get_by_token(cls, token): """Get submission by the verification token.""" email_verification = EmailVerificationService().get_active(token) submission_id = email_verification.get('submission_id') - submission = Submission.get(submission_id) + submission = SubmissionModel.find_by_id(submission_id) return PublicSubmissionSchema().dump(submission) @classmethod @@ -66,7 +85,7 @@ def create(cls, token, submission: SubmissionSchema): submission['created_by'] = participant_id submission['engagement_id'] = survey.get('engagement_id') - submission_result = Submission.create(submission, session) + submission_result = SubmissionModel.create(submission, session) submission['id'] = submission_result.id comments = CommentService.extract_comments_from_survey( submission, survey) @@ -77,14 +96,15 @@ def create(cls, token, submission: SubmissionSchema): def update(cls, data: SubmissionSchema): """Update submission.""" cls._validate_fields(data) - return Submission.update(data) + return SubmissionModel.update(data) @classmethod def update_comments(cls, token, data: PublicSubmissionSchema): """Update submission comments.""" email_verification = EmailVerificationService().get_active(token) submission_id = email_verification.get('submission_id') - submission = Submission.get(submission_id) + submission = SubmissionModel.find_by_id(submission_id) + submission.comment_status_id = Status.Pending with session_scope() as session: @@ -92,7 +112,7 @@ def update_comments(cls, token, data: PublicSubmissionSchema): token, submission.survey_id, submission.id, session) comments_result = [Comment.update( submission.id, comment, session) for comment in data.get('comments', [])] - Submission.update(SubmissionSchema().dump(submission), session) + SubmissionModel.update(SubmissionSchema().dump(submission), session) return comments_result @staticmethod @@ -113,8 +133,9 @@ def _validate_fields(submission): def review_comment(cls, submission_id, staff_review_details: dict, external_user_id) -> SubmissionSchema: """Review comment.""" user = StaffUserService.get_user_by_external_id(external_user_id) - - cls.validate_review(staff_review_details, user, submission_id) + submission = SubmissionModel.find_by_id(submission_id) + cls._check_comment_auth(submission) + cls.validate_review(staff_review_details, user, submission) reviewed_by = ' '.join( [user.get('first_name', ''), user.get('last_name', '')]) @@ -124,7 +145,7 @@ def review_comment(cls, submission_id, staff_review_details: dict, external_user with session_scope() as session: should_send_email = SubmissionService._should_send_email( submission_id, staff_review_details) - submission = Submission.update_comment_status( + submission = SubmissionModel.update_comment_status( submission_id, staff_review_details, session) if staff_notes := staff_review_details.get('staff_note', []): cls.add_or_update_staff_note( @@ -150,7 +171,7 @@ def _trigger_email(review_note, session, submission): submission, review_note, email_verification.get('verification_token')) @classmethod - def validate_review(cls, values: dict, user, submission_id): + def validate_review(cls, values: dict, user, submission): """Validate a review comment request.""" status_id = values.get('status_id', None) has_personal_info = values.get('has_personal_info', None) @@ -172,14 +193,8 @@ def validate_review(cls, values: dict, user, submission_id): not rejected_reason_other: raise ValueError('A rejection reason is required.') - submission = Submission.get(submission_id) if not submission: raise ValueError('Invalid submission.') - authorization.check_auth( - one_of_roles=(MembershipType.TEAM_MEMBER.name, - Role.REVIEW_ALL_COMMENTS.value), - engagement_id=submission.engagement_id - ) @classmethod def add_or_update_staff_note(cls, survey_id, submission_id, staff_notes): @@ -238,7 +253,7 @@ def _should_send_email(submission_id: int, staff_comment_details: dict) -> bool: @staticmethod def is_rejection_reason_changed(submission_id, values: dict): """Check if rejection reason has changed.""" - submission = Submission.get(submission_id) + submission = SubmissionModel.find_by_id(submission_id) if submission.has_personal_info == values.get('has_personal_info') and \ submission.has_profanity == values.get('has_profanity') and \ submission.has_threat == values.get('has_threat') and \ @@ -290,7 +305,7 @@ def get_paginated( } @staticmethod - def _send_rejected_email(submission: Submission, review_note, token) -> None: + def _send_rejected_email(submission: SubmissionModel, review_note, token) -> None: """Send an verification email.Throws error if fails.""" participant_id = submission.participant_id participant = ParticipantModel.find_by_id(participant_id) @@ -314,7 +329,7 @@ def _send_rejected_email(submission: Submission, review_note, token) -> None: status_code=HTTPStatus.INTERNAL_SERVER_ERROR) from exc @staticmethod - def _render_email_template(submission: Submission, review_note, token): + def _render_email_template(submission: SubmissionModel, review_note, token): template = Template.get_template('email_rejected_comment.html') engagement: EngagementModel = EngagementModel.find_by_id( submission.engagement_id) diff --git a/met-api/src/met_api/utils/roles.py b/met-api/src/met_api/utils/roles.py index b7240ccb8..cd27a2ed5 100644 --- a/met-api/src/met_api/utils/roles.py +++ b/met-api/src/met_api/utils/roles.py @@ -49,8 +49,7 @@ class Role(Enum): EDIT_UPCOMING_ENGAGEMENT = 'edit_upcoming_engagement' EDIT_OPEN_ENGAGEMENT = 'edit_open_engagement' EDIT_CLOSED_ENGAGEMENT = 'edit_closed_engagement' - VIEW_APPROVED_COMMENTS = 'view_approved_comments' - VIEW_UNAPPROVED_COMMENTS = 'view_unapproved_comments' + VIEW_APPROVED_COMMENTS = 'view_approved_comments' # used just in the front end to show the comment page VIEW_FEEDBACKS = 'view_feedbacks' VIEW_ALL_ENGAGEMENTS = 'view_all_engagements' # Allows user access to all engagements including draft SHOW_ALL_COMMENT_STATUS = 'show_all_comment_status' # Allows user to see all comment status diff --git a/met-api/tests/unit/api/test_comment.py b/met-api/tests/unit/api/test_comment.py index 26d01aedd..dd3954f7a 100644 --- a/met-api/tests/unit/api/test_comment.py +++ b/met-api/tests/unit/api/test_comment.py @@ -17,17 +17,20 @@ Test-Suite to ensure that the /Comment endpoint is working as expected. """ import json +from http import HTTPStatus from unittest.mock import MagicMock, patch from faker import Faker +from met_api.constants.membership_type import MembershipType from met_api.constants.staff_note_type import StaffNoteType from met_api.utils import notification from met_api.utils.enums import ContentType from tests.utilities.factory_scenarios import TestJwtClaims from tests.utilities.factory_utils import ( - factory_auth_header, factory_comment_model, factory_participant_model, factory_staff_user_model, - factory_submission_model, factory_survey_and_eng_model, set_global_tenant) + factory_auth_header, factory_comment_model, factory_membership_model, factory_participant_model, + factory_staff_user_model, factory_submission_model, factory_survey_and_eng_model, set_global_tenant) + fake = Faker() @@ -63,6 +66,58 @@ def test_review_comment(client, jwt, session): # pylint:disable=unused-argument assert rv.status_code == 200 +def test_review_comment_by_team_member(client, jwt, session): # pylint:disable=unused-argument + """Assert that a comment can be reviewed.""" + claims = TestJwtClaims.team_member_role + + user = factory_staff_user_model(TestJwtClaims.staff_admin_role.get('sub')) + + participant = factory_participant_model() + survey, eng = factory_survey_and_eng_model() + factory_membership_model(user_id=user.id, engagement_id=eng.id, member_type=MembershipType.TEAM_MEMBER.name) + submission = factory_submission_model(survey.id, eng.id, participant.id) + factory_comment_model(survey.id, submission.id) + to_dict = { + 'status_id': 2, + } + headers = factory_auth_header(jwt=jwt, claims=claims) + rv = client.put(f'/api/submissions/{submission.id}', + data=json.dumps(to_dict), headers=headers, content_type=ContentType.JSON.value) + assert rv.status_code == HTTPStatus.OK.value + + # Trying to put an engagement which he is a not a team member of + survey, eng = factory_survey_and_eng_model() + submission = factory_submission_model(survey.id, eng.id, participant.id) + factory_comment_model(survey.id, submission.id) + to_dict = { + 'status_id': 2, + } + headers = factory_auth_header(jwt=jwt, claims=claims) + rv = client.put(f'/api/submissions/{submission.id}', + data=json.dumps(to_dict), headers=headers, content_type=ContentType.JSON.value) + assert rv.status_code == HTTPStatus.FORBIDDEN.value + + +def test_review_comment_by_reviewer(client, jwt, session): # pylint:disable=unused-argument + """Assert that a comment can be reviewed.""" + claims = TestJwtClaims.reviewer_role + + user = factory_staff_user_model(TestJwtClaims.staff_admin_role.get('sub')) + + participant = factory_participant_model() + survey, eng = factory_survey_and_eng_model() + factory_membership_model(user_id=user.id, engagement_id=eng.id, member_type=MembershipType.REVIEWER.name) + submission = factory_submission_model(survey.id, eng.id, participant.id) + factory_comment_model(survey.id, submission.id) + to_dict = { + 'status_id': 2, + } + headers = factory_auth_header(jwt=jwt, claims=claims) + rv = client.put(f'/api/submissions/{submission.id}', + data=json.dumps(to_dict), headers=headers, content_type=ContentType.JSON.value) + assert rv.status_code == HTTPStatus.FORBIDDEN.value + + def test_review_comment_internal_note(client, jwt, session): # pylint:disable=unused-argument """Assert that a comment can be reviewed.""" claims = TestJwtClaims.staff_admin_role