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

There is no way to determine the user ID when connecting through the API #5490

Closed
paulmueller opened this issue Jul 6, 2020 · 9 comments · Fixed by #6338
Closed

There is no way to determine the user ID when connecting through the API #5490

paulmueller opened this issue Jul 6, 2020 · 9 comments · Fixed by #6338
Assignees
Projects

Comments

@paulmueller
Copy link
Contributor

CKAN version
2.8

Describe the bug
It is not possible to determine a users ID when connecting through the API. In my opinion, this should be the default behavior of user_show, but the documentation states that either the id or the user_obj parameter must be given. I think it would be user-convenient to change this default behavior.

Steps to reproduce
Connect via API using an API key and call user_show without arguments.

Expected behavior
The information about the currently logged-in user is shown.

Actual behavior

{"help": "https://dmoain.tld/api/3/action/help_show?name=user_show", "success": false, "error": {"message": "Not found", "__type": "Not Found Error"}}
@paulmueller
Copy link
Contributor Author

Note: A workaround is to iterate over user_list and check whether the API key matches the apikey value (which is only set if authorized).

@wardi
Copy link
Contributor

wardi commented Jul 7, 2020

Instead of changing user_show let's add a new action id_for_current_user_show or similar that returns just the user id when someone is logged in. IIUC this should be harder to abuse with XSS attacks because it requires another round-trip to get actual user details (security experts please weigh in if you know better)

@shubham-mahajan shubham-mahajan added this to In progress in Working On Jul 15, 2020
shubham-mahajan added a commit to shubham-mahajan/ckan that referenced this issue Jul 16, 2020
shubham-mahajan added a commit to shubham-mahajan/ckan that referenced this issue Jul 16, 2020
@shubham-mahajan shubham-mahajan moved this from In progress to Review in progress in Working On Jul 16, 2020
@jqnatividad
Copy link
Contributor

@shubham-mahajan @wardi just pinging to see if PR #5504 is still WIP.

@shubham-mahajan
Copy link
Member

@jqnatividad From my end PR is done, let's wait @wardi to review.

@paulmueller
Copy link
Contributor Author

Any updates on this one? The workaround I mentioned before does not anymore work with API tokens. A new workaround is to check whether "email" is set for a user in the user list.

@shubham-mahajan
Copy link
Member

@paulmueller It is agreed in (#5504 (comment)) that we will not be using a separate method but will create a flag in the request for that.

I will create a PR soon for that

shubham-mahajan added a commit to shubham-mahajan/ckan that referenced this issue Aug 24, 2021
Currently, we cannot determine the user details of the user
which is logged in without passing the id, so method is modified
to fetch the details of the current user if the user is logged in.
shubham-mahajan added a commit to shubham-mahajan/ckan that referenced this issue Aug 24, 2021
shubham-mahajan added a commit to shubham-mahajan/ckan that referenced this issue Aug 24, 2021
shubham-mahajan added a commit to shubham-mahajan/ckan that referenced this issue Aug 27, 2021
This param is not needed anymore and may cause issue
shubham-mahajan added a commit to shubham-mahajan/ckan that referenced this issue Aug 27, 2021
Working On automation moved this from Review in progress to Done Sep 27, 2021
@paulmueller
Copy link
Contributor Author

paulmueller commented Feb 26, 2022

@wardi I just updated from CKAN 2.9.4 to CKAN 2.9.5 and realized that #6338 did not get merged into the 2.9 branch. Is there any particular reason for that?

I checked

The docs say that during a patch release, developers should cherry-pick commits which are labeled as "backports". How is this done? Or do I have to wait until 2.10 for this to arrive in production?

http://docs.ckan.org/en/latest/contributing/release-process.html#preparing-patch-releases

@wardi
Copy link
Contributor

wardi commented Feb 26, 2022

@paulmueller new features aren't backported in case they cause issues for current users.
If you need this feature in 2.9 the simplest option would be to create a branch and backport just the feature(s) you need. Once you upgrade to 2.10 this feature will be available without running your own branch.

@paulmueller
Copy link
Contributor Author

👍 Thank you for the clarification.

mtearle pushed a commit to BioplatformsAustralia/ckan that referenced this issue Oct 13, 2022
Currently, we cannot determine the user details of the user
which is logged in without passing the id, so method is modified
to fetch the details of the current user if the user is logged in.
mtearle pushed a commit to BioplatformsAustralia/ckan that referenced this issue Oct 13, 2022
mtearle pushed a commit to BioplatformsAustralia/ckan that referenced this issue Oct 13, 2022
mtearle pushed a commit to BioplatformsAustralia/ckan that referenced this issue Oct 13, 2022
mtearle pushed a commit to BioplatformsAustralia/ckan that referenced this issue Oct 13, 2022
This param is not needed anymore and may cause issue
mtearle pushed a commit to BioplatformsAustralia/ckan that referenced this issue Oct 13, 2022
spwoodcock pushed a commit to EnviDat/ckan-forked that referenced this issue Dec 16, 2022
Currently, we cannot determine the user details of the user
which is logged in without passing the id, so method is modified
to fetch the details of the current user if the user is logged in.

(cherry picked from commit 2e78748)
spwoodcock pushed a commit to EnviDat/ckan-forked that referenced this issue Dec 16, 2022
spwoodcock pushed a commit to EnviDat/ckan-forked that referenced this issue Dec 16, 2022
This param is not needed anymore and may cause issue

(cherry picked from commit 11d707f)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Working On
  
Done
4 participants