CKAN: Ticket #1044: Sysadmins locked-out of API without Right: (visitor, SITE_READ, System)

rgrp — Thu, 17 Mar 2011 10:59:22 GMT

Think this would be resolved by refactoring implied in ticket:1001 (specifically checks for api key and remote user should lead to availability of same set of identification information).

cc set

dread — Thu, 17 Mar 2011 17:19:27 GMT

cc pudo added

I agree with leaving #1001 solving this issue. Centralising this is good. This ticket can instead focus on:

returning the correct 403 error (rather than the 302 currently)
creating some tests
documenting
consider renaming the SITE_READ variable and rethinking purpose

owner changed

dread — Fri, 18 Mar 2011 12:05:05 GMT

owner changed from dread to pudo

I've added in tests for SITE_READ and corrected the 403 response - see branch bug-1044-site-read.

The tests have uncovered some inconsistencies in the use of SITE_READ between reading/editing a package in the Web interface vs the REST API. Friedrich, do you want to take a look at ckan/tests/functional/test_authz.py:TestSiteRead.test_pkggroupadmin_read for details and have a think about how this should best work. How about making SITE_READ more widespread in the WUI, to protect read/editing/searching things, in addition to the READ and EDIT RoleActions?.

Once that is decided, it should be pretty easy to refer to the tests and document SITE_READ.

pudo — Sun, 20 Mar 2011 13:16:47 GMT

David, thanks for writing those tests - perhaps we should combine them with the ones below ("TestLockedDownUsage?") which attempt to basically do the same.

As for the inconsistency introduced by the global check in the REST API you're right: Of course it is strange that WUI access checks are more granular than the API checks. The alternative is that we either move authz checks into all relevant REST places (a major refactoring I would be suspicious of) or that we introduce duplicate checks on the WUI actions (I actually have performance concerns about that, authz is incredibly slow - and it introduces another level of special authz that I think we really should not have). Given the choice I'd opt for the REST refactor - there is no good reason to make SITE_READ a duplicate check where authz already applies.

On the other hand, this is a function we really don't want to enable or even have and that was only added to satisfy some very specific user demands. Given that these are fulfilled, I'm actually OK keeping the inconsistency for now - nobody will see it in normal operations and in a locked-down environment, users will need to have API keys anyway.

Regarding the naming, I'm pretty opionion-less: SITE_READ was proposed by rgrp and I think its pretty fitting, while OTHER_READ would just confuse me. PUBLIC_READ might work, though.

dread — Mon, 21 Mar 2011 12:57:03 GMT

I'm happy to leave the inconsistencies in SITE_READ for now, since this will be resolved when authz is centralised between WUI and API in the dictization refactor.

I'm also happy to leave both TestSiteRead? and TestLockedDownUsage? as they complement each other nicely actually.

So I think all that remains for this ticket is to add a paragraph to the authz documentation about SITE_READ. Pudo would you mind doing this? It's well worth noting where it applies and where it doesn't.

status changed; resolution set

pudo — Mon, 28 Mar 2011 09:25:54 GMT

status changed from new to closed
resolution set to worksforme

status changed; resolution deleted

dread — Mon, 28 Mar 2011 09:34:31 GMT

status changed from closed to reopened
resolution worksforme deleted

I think all that remains for this ticket is to add a paragraph to the authz documentation about SITE_READ. Pudo would you mind doing this? It's well worth noting where it applies and where it doesn't.

status changed; resolution set

dread — Wed, 06 Apr 2011 13:22:35 GMT

status changed from reopened to closed
resolution set to fixed

I've added in the docs for this in cset:013da53052d1 ready for release 1.3.3.