Skip to content

Commit

Permalink
[#283] Check context has user in related_update
Browse files Browse the repository at this point in the history 
* Check that the context has user key.
* Check that a user object exists corresponding to
  value of 'user' in context.
  • Loading branch information
@nigelbabu
nigelbabu committed Mar 26, 2013
1 parent 991488f commit f6ed9b6
Showing 1 changed file with 8 additions and 0 deletions.
8 changes: 8 additions & 0 deletions ckan/logic/action/update.py
View file
Expand Up @@ -126,10 +126,18 @@ def related_update(context, data_dict):
:rtype: dictionary
'''
if not context.has_key('user'):
raise logic.NotAuthorized(
_("You must be logged in to update a related item."))

This comment has been minimized.

Sign in to view

Copy link
@seanh

seanh Apr 10, 2013

Contributor

Hmm. Is this necessary? I think you can assume that 'user' is always in context. If no one's logged in, then 'user' will be an IP address, you can still do model.User.get() with it like below but you will get None back, then you only need to raise NotAuthorized in one place not two.

has_key() is deprecated in Python use if 'user' not in context.

I know that several other action functions do the authorization themselves and then raise NotAuthorized, but I think this is actually incorrect, authorization should be done in a ckan.logic.auth.related_update() function, and then ckan.logic.action.related_update() should use check_access(). See for example organization_create() for the Right Way to do this. The reason is that if you don't use a separate auth function, then the IAuthFunctions plugin interface does not work. @tobes Do you agree?

This comment has been minimized.

Sign in to view

Copy link
@seanh

seanh Apr 10, 2013

Contributor

I think this is the method that ensures that user is always in context: https://github.com/okfn/ckan/blob/master/ckan/lib/base.py#L224 It's called by the __before__ method of BaseController. BaseController is a base class that all controller classes in CKAN inherit from (including the api controller), and __before__() is a Pylons special method that gets called for each request, before any of the normal methods associated with the request get called.

model = context['model']
user = context['user']
id = _get_or_bust(data_dict, "id")
userobj = model.User.get(user)
if not userobj:
raise logic.NotAuthorized(
_("You must be logged in to update a related item."))

session = context['session']

schema = context.get('schema') or ckan.logic.schema.default_related_schema()
Expand Down

0 comments on commit f6ed9b6

Please sign in to comment.