Skip to content

Commit

Permalink
[#2846] use 403 or 404 when forbidden, not 401
Browse files Browse the repository at this point in the history
  • Loading branch information
wardi committed Jan 26, 2016
1 parent 63b2af1 commit 69c6314
Show file tree
Hide file tree
Showing 9 changed files with 83 additions and 139 deletions.
6 changes: 3 additions & 3 deletions ckan/config/middleware.py
Expand Up @@ -108,12 +108,12 @@ def make_app(conf, full_stack=True, static_files=True, **app_conf):
# Handle Python exceptions
app = ErrorHandler(app, conf, **config['pylons.errorware'])

# Display error documents for 401, 403, 404 status codes (and
# Display error documents for 400, 403, 404 status codes (and
# 500 when debug is disabled)
if asbool(config['debug']):
app = StatusCodeRedirect(app, [400, 404])
app = StatusCodeRedirect(app, [400, 403, 404])
else:
app = StatusCodeRedirect(app, [400, 404, 500])
app = StatusCodeRedirect(app, [400, 403, 404, 500])

# Initialize repoze.who
who_parser = WhoConfig(conf['here'])
Expand Down
2 changes: 1 addition & 1 deletion ckan/controllers/admin.py
Expand Up @@ -28,7 +28,7 @@ def __before__(self, action, **params):
try:
logic.check_access('sysadmin', context, {})
except logic.NotAuthorized:
base.abort(401, _('Need to be system administrator to administer'))
base.abort(403, _('Need to be system administrator to administer'))
c.revision_change_state_allowed = True

def _get_config_form_items(self):
Expand Down
60 changes: 20 additions & 40 deletions ckan/controllers/group.py
Expand Up @@ -160,7 +160,7 @@ def index(self):
try:
self._check_access('site_read', context)
except NotAuthorized:
abort(401, _('Not authorized to see this page'))
abort(403, _('Not authorized to see this page'))

# pass user info to context as needed to view private datasets of
# orgs correctly
Expand Down Expand Up @@ -218,10 +218,8 @@ def read(self, id, limit=20):
data_dict['include_datasets'] = False
c.group_dict = self._action('group_show')(context, data_dict)
c.group = context['group']
except NotFound:
except (NotFound, NotAuthorized):
abort(404, _('Group not found'))
except NotAuthorized:
abort(401, _('Unauthorized to read group %s') % id)

self._read(id, limit, group_type)
return render(self._read_template(c.group_dict['type']),
Expand Down Expand Up @@ -400,10 +398,8 @@ def bulk_process(self, id):
data_dict['include_datasets'] = False
c.group_dict = self._action('group_show')(context, data_dict)
c.group = context['group']
except NotFound:
except (NotFound, NotAuthorized):
abort(404, _('Group not found'))
except NotAuthorized:
abort(401, _('Unauthorized to read group %s') % id)

#use different form names so that ie7 can be detected
form_names = set(["bulk_action.public", "bulk_action.delete",
Expand Down Expand Up @@ -446,7 +442,7 @@ def bulk_process(self, id):
try:
get_action(action_functions[action])(context, data_dict)
except NotAuthorized:
abort(401, _('Not authorized to perform bulk update'))
abort(403, _('Not authorized to perform bulk update'))
base.redirect(h.url_for(controller='organization',
action='bulk_process',
id=id))
Expand All @@ -466,7 +462,7 @@ def new(self, data=None, errors=None, error_summary=None):
try:
self._check_access('group_create', context)
except NotAuthorized:
abort(401, _('Unauthorized to create a group'))
abort(403, _('Unauthorized to create a group'))

if context['save'] and not data:
return self._save_new(context, group_type)
Expand Down Expand Up @@ -508,10 +504,8 @@ def edit(self, id, data=None, errors=None, error_summary=None):
c.grouptitle = old_data.get('title')
c.groupname = old_data.get('name')
data = data or old_data
except NotFound:
except (NotFound, NotAuthorized):
abort(404, _('Group not found'))
except NotAuthorized:
abort(401, _('Unauthorized to read group %s') % '')

group = context.get("group")
c.group = group
Expand All @@ -520,7 +514,7 @@ def edit(self, id, data=None, errors=None, error_summary=None):
try:
self._check_access('group_update', context)
except NotAuthorized:
abort(401, _('User %r not authorized to edit %s') % (c.user, id))
abort(403, _('User %r not authorized to edit %s') % (c.user, id))

errors = errors or {}
vars = {'data': data, 'errors': errors,
Expand Down Expand Up @@ -553,9 +547,7 @@ def _save_new(self, context, group_type=None):

# Redirect to the appropriate _read route for the type of group
h.redirect_to(group['type'] + '_read', id=group['name'])
except NotAuthorized:
abort(401, _('Unauthorized to read group %s') % '')
except NotFound, e:
except (NotFound, NotAuthorized), e:
abort(404, _('Group not found'))
except dict_fns.DataError:
abort(400, _(u'Integrity Error'))
Expand Down Expand Up @@ -585,9 +577,7 @@ def _save_edit(self, id, context):
self._force_reindex(group)

h.redirect_to('%s_read' % group['type'], id=group['name'])
except NotAuthorized:
abort(401, _('Unauthorized to read group %s') % id)
except NotFound, e:
except (NotFound, NotAuthorized), e:
abort(404, _('Group not found'))
except dict_fns.DataError:
abort(400, _(u'Integrity Error'))
Expand Down Expand Up @@ -615,7 +605,7 @@ def authz(self, id):
except NotAuthorized:
c.authz_editable = False
if not c.authz_editable:
abort(401,
abort(403,
_('User %r not authorized to edit %s authorizations') %
(c.user, id))

Expand All @@ -636,7 +626,7 @@ def delete(self, id):
try:
self._check_access('group_delete', context, {'id': id})
except NotAuthorized:
abort(401, _('Unauthorized to delete group %s') % '')
abort(403, _('Unauthorized to delete group %s') % '')

try:
if request.method == 'POST':
Expand All @@ -651,7 +641,7 @@ def delete(self, id):
self._redirect_to_this_controller(action='index')
c.group_dict = self._action('group_show')(context, {'id': id})
except NotAuthorized:
abort(401, _('Unauthorized to delete group %s') % '')
abort(403, _('Unauthorized to delete group %s') % '')
except NotFound:
abort(404, _('Group not found'))
return self._render_template('group/confirm_delete.html', group_type)
Expand All @@ -669,9 +659,7 @@ def members(self, id):
data_dict = {'id': id}
data_dict['include_datasets'] = False
c.group_dict = self._action('group_show')(context, data_dict)
except NotAuthorized:
abort(401, _('Unauthorized to delete group %s') % '')
except NotFound:
except (NotFound, NotAuthorized):
abort(404, _('Group not found'))
return self._render_template('group/members.html', group_type)

Expand Down Expand Up @@ -722,7 +710,7 @@ def member_new(self, id):
else:
c.user_role = 'member'
except NotAuthorized:
abort(401, _('Unauthorized to add member to group %s') % '')
abort(403, _('Unauthorized to add member to group %s') % '')
except NotFound:
abort(404, _('Group not found'))
except ValidationError, e:
Expand All @@ -741,7 +729,7 @@ def member_delete(self, id):
try:
self._check_access('group_member_delete', context, {'id': id})
except NotAuthorized:
abort(401, _('Unauthorized to delete group %s members') % '')
abort(403, _('Unauthorized to delete group %s members') % '')

try:
user_id = request.params.get('user')
Expand All @@ -754,7 +742,7 @@ def member_delete(self, id):
c.user_id = user_id
c.group_id = id
except NotAuthorized:
abort(401, _('Unauthorized to delete group %s') % '')
abort(403, _('Unauthorized to delete group %s members') % '')
except NotFound:
abort(404, _('Group not found'))
return self._render_template('group/confirm_delete_member.html',
Expand Down Expand Up @@ -788,10 +776,8 @@ def history(self, id):
#TODO: remove
# Still necessary for the authz check in group/layout.html
c.group = context['group']
except NotFound:
except (NotFound, NotAuthorized):
abort(404, _('Group not found'))
except NotAuthorized:
abort(401, _('User %r not authorized to edit %r') % (c.user, id))

format = request.params.get('format', '')
if format == 'atom':
Expand Down Expand Up @@ -847,12 +833,8 @@ def activity(self, id, offset=0):
'user': c.user, 'for_view': True}
try:
c.group_dict = self._get_group_dict(id)
except NotFound:
except (NotFound, NotAuthorized):
abort(404, _('Group not found'))
except NotAuthorized:
abort(401,
_('Unauthorized to read group {group_id}').format(
group_id=id))

# Add the group's activity stream (already rendered to HTML) to the
# template context for the group/read.html template to retrieve later.
Expand Down Expand Up @@ -912,7 +894,7 @@ def followers(self, id):
c.followers = \
get_action('group_follower_list')(context, {'id': id})
except NotAuthorized:
abort(401, _('Unauthorized to view followers %s') % '')
abort(403, _('Unauthorized to view followers %s') % '')
return render('group/followers.html',
extra_vars={'group_type': group_type})

Expand Down Expand Up @@ -943,7 +925,5 @@ def _get_group_dict(self, id):
try:
return self._action('group_show')(
context, {'id': id, 'include_datasets': False})
except NotFound:
except (NotFound, NotAuthorized):
abort(404, _('Group not found'))
except NotAuthorized:
abort(401, _('Unauthorized to read group %s') % id)
2 changes: 1 addition & 1 deletion ckan/controllers/home.py
Expand Up @@ -23,7 +23,7 @@ def __before__(self, action, **env):
'auth_user_obj': c.userobj}
logic.check_access('site_read', context)
except logic.NotAuthorized:
base.abort(401, _('Not authorized to see this page'))
base.abort(403, _('Not authorized to see this page'))
except (sqlalchemy.exc.ProgrammingError,
sqlalchemy.exc.OperationalError), e:
# postgres and sqlite errors for missing tables
Expand Down

0 comments on commit 69c6314

Please sign in to comment.