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

@@ -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."""

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:

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)

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,22 +96,23 @@ 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:
             EmailVerificationService().verify(
                 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)

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

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