Ticket #1001 (closed enhancement: fixed)
API should use normal user credentials if available
| Reported by: | rgrp | Owned by: | rgrp |
|---|---|---|---|
| Priority: | critical | Milestone: | ckan-v1.4-sprint-4 |
| Component: | ckan | Keywords: | bitesize core |
| Cc: | Repository: | ckan | |
| Theme: | none |
Description (last modified by rgrp) (diff)
When using the API 'locally' i.e. from the CKAN instance (as would be the case with an ajax interface) the API, especially that allowing READ requests should use the normal user credentials if they are available prior to looking for an API key.
The key change appears to be to change _get_user_for_apikey method in lib/base.py BaseController? to check the c.user attribute (may wish to rename as the name may now be a bit misleading ...).
This is critical to incorporating any ajax editing into the frontend.
As part of this ticket we should do a general consolidation of the identification system in lib/base.py so that both api_key and normal user auth lead to the same set of auth-related objects being available (suggest c.user and c.userobj and c.author).
Change History
comment:1 Changed 3 years ago by rgrp
- Milestone changed from ckan-v1.4-sprint-2 to ckan-v1.4-sprint-3
comment:2 Changed 3 years ago by rgrp
- Repository set to ckan
- Theme set to none
- Description modified (diff)
comment:3 Changed 3 years ago by thejimmyg
Yes, I think the two should be consolidated, but as was noted elsewhere (I think) we don't want the API to ever be authenticated via a cookie, otherwise we open ourselves to CSRF attacks. Instead we could support the API key as a query parameter as an alternative to via the HTTP header if it is hard to set an HTTP header in JS libraries?
comment:4 Changed 3 years ago by pudo
My vote on this would be to enable "httpbasic" auth via repoze.who and to add an API key challenger to ckan/lib/authenticator.py that accepts API keys, from within repoze.who. This way we won't have to work around the framework at all and can retain compatibility to the existing mechanisms.
comment:5 Changed 3 years ago by rgrp
- Milestone changed from ckan-v1.4-sprint-3 to ckan-v1.4-sprint-4
First work in cset:6b8cbe465b61
comment:6 Changed 3 years ago by pudo
Wrote a WDMMG version of this yesterday: https://bitbucket.org/okfn/wdmmg/changeset/398ce0eb3901#chg-wdmmg/lib/authenticator.py
comment:7 Changed 3 years ago by rgrp
- Status changed from new to closed
- Resolution set to fixed
Completed and merged into default in cset:cb200f339dbb. At the moment have done the very simple option but pudo's approach seems better and we could refactor towards that going forward.
comment:8 Changed 3 years ago by dread
- Status changed from closed to reopened
- Resolution fixed deleted
You mentioned writing tests? Also the CSRF question from James hasn't been addressed.
comment:9 Changed 3 years ago by rgrp
- Status changed from reopened to closed
- Resolution set to fixed
This ticket did not include CSRF matters - please raise in a separate ticket if you wish. I looked in some detail at the testing options and nothing seemed simple to do that didn't duplicate code we already have (since no way to access the base controller halfway through a request). This can be tested as necessary as part of the development of the new js work.
