Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Limit QGIS project modifications to managers and admins #958

Merged
merged 6 commits into from
Jun 20, 2024

Conversation

boardend
Copy link
Contributor

@boardend boardend commented Jun 4, 2024

Add a has_restricted_projectfiles flag to the project in order to restrict modifications of the QGIS project file to users that have the "Manager" or "Administrator" role

Screenshot from 2024-06-17 15-59-53

@duke-nyuki
Copy link
Collaborator

Task linked: QF-3822 Limit .qgs file edit to admins

@boardend boardend marked this pull request as draft June 4, 2024 13:03
boardend and others added 3 commits June 13, 2024 12:04
@nirvn
Copy link
Member

nirvn commented Jun 17, 2024

Open question: should we also limit modification of project plugins to admins? I think so

@boardend
Copy link
Contributor Author

Open question: should we also limit modification of project plugins to admins? I think so

Sounds reasonable, but the question is up to @suricactus

If so, let's add a new task for that; This probably doesn't have to be configurable on the project like the is_projectfile_restricted flag. I would just add a new rule on the POST of DownloadPushDeleteFileView

@boardend
Copy link
Contributor Author

Strange that qfieldcloud.core.tests.test_delta fails here... (Mark this as "Ready for review")

@boardend boardend marked this pull request as ready for review June 17, 2024 14:10
@boardend boardend changed the title WIP: Limit QGIS project modifications to admins Limit QGIS project modifications to managers and admins Jun 17, 2024
@nirvn
Copy link
Member

nirvn commented Jun 17, 2024

@boardend , I would tie this to is_projectfile_restricted. I can't see need for further permission granularity to cover the project plugin .qml vs the project file .qgz

@nirvn
Copy link
Member

nirvn commented Jun 18, 2024

@boardend , you'll also need to take into consideration another project sidecar file. When saved as .qgs, QGIS saves an _attacments.zip file too. Logic should be updated here to catch all project sidecar files:

https://github.com/opengisch/qfieldcloud/blob/330348bc54e18e272441b1a96cf8525aa624e4af/docker-app/qfieldcloud/core/utils.py#L238

@suricactus
Copy link
Collaborator

Let's leave the plugins out of the scope of this PR.

As for the _attachments.zip, we had a dev discussion last week we can ignore it for now, as there are a few other places where this should be updated. We also need to verify what is the QGIS convention for the naming, I think it is <qgs_file_basename>_attachhments.zip but need to verify in QGIS source code. We should take this into consideration and not only look for the suffix, as I might have family_photos_attachments.zip as a valid file which is not a sidecar for .qgs.

IMO we should create a function is_protected_project_file, instead of raw path.suffix.lower() in (".qgs", ".qgz"). This way in the future we can add more sensitive files to that function.

@nirvn
Copy link
Member

nirvn commented Jun 18, 2024

@suricactus , you are correct about the convention. And yeah, same logic to identify the plugin would be used here.

  • Step 1: identify the one project file in the directory
  • Step 2: identify _attachments.zip as protected
  • Step 3: identify .qml as protected

The function your suggesting would allow for emancipation of that logic too, which is useful.

@boardend
Copy link
Contributor Author

@nirvn & @suricactus I just created the follow up task to refactor the QGIS/QField project file detection: https://app.clickup.com/t/2192114/QF-4375

@boardend boardend requested a review from suricactus June 18, 2024 14:46
@boardend
Copy link
Contributor Author

Did a final updated based on the discussion with @suricactus

Let's accept this one and I will create a new PR to respect all QGIS/QField related projectfiles

Copy link
Collaborator

@suricactus suricactus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! In a follow-up would be great to add a test.

@suricactus suricactus merged commit 61288ce into master Jun 20, 2024
6 of 8 checks passed
@suricactus suricactus deleted the QF-3822-limit-project-edit-to-admin branch June 20, 2024 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants