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

via REST API uploaded documents to not get proper permissions #12616

Closed
gannebamm opened this issue Sep 25, 2024 · 11 comments · Fixed by #12707
Closed

via REST API uploaded documents to not get proper permissions #12616

gannebamm opened this issue Sep 25, 2024 · 11 comments · Fixed by #12707
Assignees
Labels
4.4.x bug master regression Issues related to regressions.

Comments

@gannebamm
Copy link
Contributor

gannebamm commented Sep 25, 2024

Expected Behavior

edit: clarifications

If I as a normal registered user upload a document via the REST API it belongs to my user and I can delete it. It should show this:

firefox_RrCcekaJKe

Actual Behavior

I as a normal registered user am unable to delete it. It does not show delete:
grafik

as admin I am able to delete the ressource

Steps to Reproduce the Problem

  • create a new user (registered user)
  • use this new user account to upload a document via REST API like:
import requests
url = "https://stable.demo.geonode.org/api/v2/documents"
payload = { 'title' : 'test upload of document via rest api' }
files = [ ('doc_file', ('test.txt', open('./test.txt', 'rb'), 'text/plain')) ]
headers = { 'Authorization' : 'Basic ...' } # replaced "..." with the output of $(echo -n user:password | base64) with my credentials
response = requests.request ("POST", url, data=payload, headers=headers, files=files)

Open https://stable.demo.geonode.org/catalogue/#/documents

log in as user (not admin!)

See the delete option is missing on the new document:

Specifications

  • GeoNode version: stable.demo (4.4.0.dev0)
  • Additional details:
@gannebamm gannebamm added regression Issues related to regressions. bug labels Sep 25, 2024
@gannebamm
Copy link
Contributor Author

gannebamm commented Sep 25, 2024

@ahmdthr please take a look at this. Since I tested both development and stable demo it should not be linked to your latest PR #11872

edit: or better send this to @kilichenko-pixida since he has more free capacity.

@ridoo
Copy link
Contributor

ridoo commented Oct 16, 2024

@ridoo Test REST API upload v4.3.1

@gannebamm
Copy link
Contributor Author

additional specification to reproduce: You shall use a 'normal' user to upload. Admin /superuser will work.

@mattiagiupponi
Copy link
Contributor

Hi @gannebamm did your team had time to check this issue? otherwise i can give a check on it

@gannebamm
Copy link
Contributor Author

gannebamm commented Oct 28, 2024

@ridoo , @kilichenko-pixida please check this. As stated a normal registered user shall be used NOT an admin user.

edit: I did some clarification in the how to reproduce section above

@kilichenko-pixida
Copy link
Contributor

@gannebamm yes, I was able to reproduce it and ran into the same problem on the non-admin user after an API upload

@giohappy
Copy link
Contributor

@gannebamm are you going to take care of this?

@gannebamm
Copy link
Contributor Author

@gannebamm yes, I was able to reproduce it and ran into the same problem on the non-admin user after an API upload

@kilichenko-pixida please debug into the process and take a look at the permission granting process to identify the issue.

@giohappy we will try

@kilichenko-pixida
Copy link
Contributor

The issue was indeed due to improper permissions handling. Both in DocumentUploadView that handles UI requests and in DocumentViewSet that handles API there is a call to set_permissions method and in both cases all permissions are being removed.

The difference, however, is that in UI code, after all permissions have been removed, there is also a resource_manager.update call which actually includes calling set_default_permissions method which restores default values.

See PR #10 for the suggested fix.

I see setting permissions to None on the API side was first introduced in 7f56ebe, though I don't know whether it was a simple oversight or there quite possibly was a good reason for it that I do not understand.

On the UI side, this permission workflow was implemented as part of big changes in f760e0f from 2021.

@gannebamm
Copy link
Contributor Author

@kilichenko-pixida please provide a PR for master, too. @giohappy please use some of our booked time to answer the questions above to make sure the PR fits the quality critera.

@kilichenko-pixida
Copy link
Contributor

kilichenko-pixida commented Nov 8, 2024

@giohappy here is the PR: #12707 , I already sent the signed license agreement to [email protected], but I guess approval is pending. Thank you.

kilichenko-pixida added a commit to kilichenko-pixida/geonode that referenced this issue Nov 12, 2024
kilichenko-pixida added a commit to kilichenko-pixida/geonode that referenced this issue Nov 12, 2024
kilichenko-pixida added a commit to kilichenko-pixida/geonode that referenced this issue Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4.4.x bug master regression Issues related to regressions.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants