An unanswered invite from a project can have the invite revoked

[rv0lt]

1. Description / Summary

Added changes in the api to allow for revoking access to unacepted invites. Before that there was an error because the users werent considered to be in the system.

2. Jira task / GitHub issue

DDS-1192

3. Type of change

Check the relevant boxes below. For an explanation of the different sections, enter edit mode of this PR description template.

• [x] New feature
  • [ ] Breaking: Why / How? Add info here.
  • [x] Non-breaking
• [ ] Database change: Remember the to include a new migration version, or explain here why it's not needed.
• [ ] Bug fix
• [ ] Security Alert fix
• [ ] Documentation
• [ ] Workflow
• [ ] Tests only

4. Additional information

• [x] Sprintlog
• [ ] Blocking PRs
  • [ ] Merged
• [ ] PR to master branch: _If checked, read the release instructions
  • [ ] I have followed steps 1-8.

5. Actions / Scans

Check the boxes when the specified checks have passed.

For information on what the different checks do and how to fix it if they're failing, enter edit mode of this description or go to the PR template.

• [x] Black
• [x] Prettier
• [x] Yamllint
• [x] Tests
• [x] CodeQL
• [x] Trivy
• [x] Snyk

[codecov[bot]]

Codecov Report

Merging #1468 (e5bfb52) into dev (5a8f956) will increase coverage by 0.03%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##              dev    #1468      +/-   ##
==========================================
+ Coverage   90.15%   90.18%   +0.03%     
==========================================
  Files          29       29              
  Lines        4408     4424      +16     
==========================================
+ Hits         3974     3990      +16     
  Misses        434      434              

Files	Coverage Δ
dds_web/api/user.py	90.90% <100.00%> (+0.27%)	:arrow_up:

[rv0lt]

• Only operration error not incluyed in codebot
• I am not sure if the tests are placed in the correct test files

[rv0lt]

@i-oden The thing is that Units Users/Admins cannot be granted access to individual projects right?

Then, the only condition to check is that a PO is not deleted by a Researcher. If Researchers are removed of the functionality to remove by the auth.login_required. Then there is no need to check right?

i.e Unit Admins and Personal can delete any user they want in their unit

PO can only delete within their project which is already checked that the user to delete share a project

TODO to do the test for this last functionality I think

[rv0lt]

I was adding some more tests, but with this new comments I will need to change them. ON monday I will finish it

[i-oden]

@i-oden The thing is that Units Users/Admins cannot be granted access to individual projects right?

Then, the only condition to check is that a PO is not deleted by a Researcher. If Researchers are removed of the functionality to remove by the auth.login_required. Then there is no need to check right?

i.e Unit Admins and Personal can delete any user they want in their unit

PO can only delete within their project which is already checked that the user to delete share a project

TODO to do the test for this last functionality I think

@rv0lt So yes this is true when it comes to a registered user, because the code checks for project.researchusers (line 919 to 935), but this is the table models.ProjectUsers, which only contains Researchers with access to the project. When it comes to invites though, if a Unit Personnel / Admin invites another Unit Personnel / Admin, they get access to all projects within that unit, so the Invite row is connected to a number of different ProjectInviteKey rows since they need to get access to all the project invite keys. So in this code for example, it checks if there are any ProjectInviteKeys for the specific invite and project, which there will be independent on which user role the invite has. So this means that this code:

        if unanswered_invite:
            invite_id = unanswered_invite.id

            # Check if the unanswered invite is associated with the project
            project_invite_key = models.ProjectInviteKeys.query.filter_by(
                invite_id=invite_id, project_id=project.id
            ).one_or_none()

            if project_invite_key:
                # Remove the association if it exists
                db.session.delete(project_invite_key)

                # Check if the invite is associated with only one project, then delete it
                project_invite_key = models.ProjectInviteKeys.query.filter_by(
                    invite_id=invite_id
                ).one_or_none()

                if not project_invite_key:
                    db.session.delete(unanswered_invite)

will remove the access for a Unit Personnel / Admin, which we don't want to be possible.

Let me know if I'm misunderstanding the code somewhere (or you).

[rv0lt]

@i-oden The thing is that Units Users/Admins cannot be granted access to individual projects right? Then, the only condition to check is that a PO is not deleted by a Researcher. If Researchers are removed of the functionality to remove by the auth.login_required. Then there is no need to check right? i.e Unit Admins and Personal can delete any user they want in their unit PO can only delete within their project which is already checked that the user to delete share a project TODO to do the test for this last functionality I think

@rv0lt So yes this is true when it comes to a registered user, because the code checks for project.researchusers (line 919 to 935), but this is the table models.ProjectUsers, which only contains Researchers with access to the project. When it comes to invites though, if a Unit Personnel / Admin invites another Unit Personnel / Admin, they get access to all projects within that unit, so the Invite row is connected to a number of different ProjectInviteKey rows since they need to get access to all the project invite keys. So in this code for example, it checks if there are any ProjectInviteKeys for the specific invite and project, which there will be independent on which user role the invite has. So this means that this code:

        if unanswered_invite:
            invite_id = unanswered_invite.id

            # Check if the unanswered invite is associated with the project
            project_invite_key = models.ProjectInviteKeys.query.filter_by(
                invite_id=invite_id, project_id=project.id
            ).one_or_none()

            if project_invite_key:
                # Remove the association if it exists
                db.session.delete(project_invite_key)

                # Check if the invite is associated with only one project, then delete it
                project_invite_key = models.ProjectInviteKeys.query.filter_by(
                    invite_id=invite_id
                ).one_or_none()

                if not project_invite_key:
                    db.session.delete(unanswered_invite)

will remove the access for a Unit Personnel / Admin, which we don't want to be possible.

Let me know if I'm misunderstanding the code somewhere (or you).

Ok, yeah I see this issue. I was testing and removing an unanswered invite to be a Unit Admin returns a Response code: 500. Definitely should return a proper error message. So then, we should just check if the invite row has a _unitid and if it does don't proceed with the code?

[i-oden]

@i-oden The thing is that Units Users/Admins cannot be granted access to individual projects right? Then, the only condition to check is that a PO is not deleted by a Researcher. If Researchers are removed of the functionality to remove by the auth.login_required. Then there is no need to check right? i.e Unit Admins and Personal can delete any user they want in their unit PO can only delete within their project which is already checked that the user to delete share a project TODO to do the test for this last functionality I think

@rv0lt So yes this is true when it comes to a registered user, because the code checks for project.researchusers (line 919 to 935), but this is the table models.ProjectUsers, which only contains Researchers with access to the project. When it comes to invites though, if a Unit Personnel / Admin invites another Unit Personnel / Admin, they get access to all projects within that unit, so the Invite row is connected to a number of different ProjectInviteKey rows since they need to get access to all the project invite keys. So in this code for example, it checks if there are any ProjectInviteKeys for the specific invite and project, which there will be independent on which user role the invite has. So this means that this code:

        if unanswered_invite:
            invite_id = unanswered_invite.id

            # Check if the unanswered invite is associated with the project
            project_invite_key = models.ProjectInviteKeys.query.filter_by(
                invite_id=invite_id, project_id=project.id
            ).one_or_none()

            if project_invite_key:
                # Remove the association if it exists
                db.session.delete(project_invite_key)

                # Check if the invite is associated with only one project, then delete it
                project_invite_key = models.ProjectInviteKeys.query.filter_by(
                    invite_id=invite_id
                ).one_or_none()

                if not project_invite_key:
                    db.session.delete(unanswered_invite)

will remove the access for a Unit Personnel / Admin, which we don't want to be possible. Let me know if I'm misunderstanding the code somewhere (or you).

Ok, yeah I see this issue. I was testing and removing an unanswered invite to be a Unit Admin returns a Response code: 500. Definitely should return a proper error message. So then, we should just check if the invite row has a _unitid and if it does don't proceed with the code?

Just adding response here as well. Discussed during daily. Yes, that's also definitely an option and that could be simpler.