Merge pull request ckan#1224 from okfn/1224-not-auth-except-msg

propogate NotAuthorized exception message

ckan/controllers/api.py

@@ -114,8 +114,10 @@ def _finish_ok(self, response_data=None,
 
         return self._finish(status_int, response_data, content_type)
 
-    def _finish_not_authz(self):
+    def _finish_not_authz(self, extra_msg=None):
         response_data = _('Access denied')
+        if extra_msg:
+            response_data = '%s - %s' % (response_data, extra_msg)
         return self._finish(403, response_data, 'json')
 
     def _finish_not_found(self, extra_msg=None):
@@ -194,10 +196,14 @@ def action(self, logic_function, ver=None):
                                     'data': request_data}
             return_dict['success'] = False
             return self._finish(400, return_dict, content_type='json')
-        except NotAuthorized:
+        except NotAuthorized, e:
             return_dict['error'] = {'__type': 'Authorization Error',
                                     'message': _('Access denied')}
             return_dict['success'] = False
+
+            if e.extra_msg:
+                return_dict['error']['message'] += ': %s' % e.extra_msg
+
             return self._finish(403, return_dict, content_type='json')
         except NotFound, e:
             return_dict['error'] = {'__type': 'Not Found Error',
@@ -277,8 +283,9 @@ def list(self, ver=None, register=None, subregister=None, id=None):
         except NotFound, e:
             extra_msg = e.extra_msg
             return self._finish_not_found(extra_msg)
-        except NotAuthorized:
-            return self._finish_not_authz()
+        except NotAuthorized, e:
+            extra_msg = e.extra_msg
+            return self._finish_not_authz(extra_msg)
 
     def show(self, ver=None, register=None, subregister=None,
              id=None, id2=None):
@@ -308,8 +315,9 @@ def show(self, ver=None, register=None, subregister=None,
         except NotFound, e:
             extra_msg = e.extra_msg
             return self._finish_not_found(extra_msg)
-        except NotAuthorized:
-            return self._finish_not_authz()
+        except NotAuthorized, e:
+            extra_msg = e.extra_msg
+            return self._finish_not_authz(extra_msg)
 
     def _represent_package(self, package):
         return package.as_dict(ref_package_by=self.ref_package_by,
@@ -354,8 +362,9 @@ def create(self, ver=None, register=None, subregister=None,
                                           data_dict.get("id")))
             return self._finish_ok(response_data,
                                    resource_location=location)
-        except NotAuthorized:
-            return self._finish_not_authz()
+        except NotAuthorized, e:
+            extra_msg = e.extra_msg
+            return self._finish_not_authz(extra_msg)
         except NotFound, e:
             extra_msg = e.extra_msg
             return self._finish_not_found(extra_msg)
@@ -410,8 +419,9 @@ def update(self, ver=None, register=None, subregister=None,
         try:
             response_data = action(context, data_dict)
             return self._finish_ok(response_data)
-        except NotAuthorized:
-            return self._finish_not_authz()
+        except NotAuthorized, e:
+            extra_msg = e.extra_msg
+            return self._finish_not_authz(extra_msg)
         except NotFound, e:
             extra_msg = e.extra_msg
             return self._finish_not_found(extra_msg)
@@ -458,8 +468,9 @@ def delete(self, ver=None, register=None, subregister=None,
         try:
             response_data = action(context, data_dict)
             return self._finish_ok(response_data)
-        except NotAuthorized:
-            return self._finish_not_authz()
+        except NotAuthorized, e:
+            extra_msg = e.extra_msg
+            return self._finish_not_authz(extra_msg)
         except NotFound, e:
             extra_msg = e.extra_msg
             return self._finish_not_found(extra_msg)

ckan/tests/functional/api/model/test_group_and_organization_purge.py

@@ -266,8 +266,7 @@ def _test_visitors_cannot_purge_groups_or_orgs(self, is_org):
         result = tests.call_action_api(self.app, action, id=group_or_org['id'],
                                        status=403,
                                        )
-        assert result == {'__type': 'Authorization Error',
-                          'message': 'Access denied'}
+        assert result['__type'] == 'Authorization Error'
 
     def test_visitors_cannot_purge_organizations(self):
         '''Visitors (who aren't logged in) should not be authorized to purge

ckan/tests/functional/api/model/test_vocabulary.py

@@ -399,7 +399,7 @@ def test_vocabulary_create_not_logged_in(self):
                 params=param_string,
                 status=403)
         assert response.json['success'] is False
-        assert response.json['error']['message'] == 'Access denied'
+        assert response.json['error']['__type'] == 'Authorization Error'
 
     def test_vocabulary_create_not_authorized(self):
         '''Test that users who are not authorized cannot create vocabs.'''
@@ -412,7 +412,7 @@ def test_vocabulary_create_not_authorized(self):
                     str(self.normal_user.apikey)},
                 status=403)
         assert response.json['success'] is False
-        assert response.json['error']['message'] == 'Access denied'
+        assert response.json['error']['__type'] == 'Authorization Error'
 
     def test_vocabulary_update_id_only(self):
         self._update_vocabulary({'id': self.genre_vocab['id']},
@@ -494,7 +494,7 @@ def test_vocabulary_update_not_logged_in(self):
                 params=param_string,
                 status=403)
         assert response.json['success'] is False
-        assert response.json['error']['message'] == 'Access denied'
+        assert response.json['error']['__type'] == 'Authorization Error'
 
     def test_vocabulary_update_with_tags(self):
         tags = [
@@ -630,7 +630,7 @@ def test_vocabulary_delete_not_logged_in(self):
                 params=param_string,
                 status=403)
         assert response.json['success'] is False
-        assert response.json['error']['message'] == 'Access denied'
+        assert response.json['error']['__type'] == 'Authorization Error'
 
     def test_vocabulary_delete_not_authorized(self):
         '''Test that users who are not authorized cannot delete vocabs.'''
@@ -642,7 +642,7 @@ def test_vocabulary_delete_not_authorized(self):
                     str(self.normal_user.apikey)},
                 status=403)
         assert response.json['success'] is False
-        assert response.json['error']['message'] == 'Access denied'
+        assert response.json['error']['__type'] == 'Authorization Error'
 
     def test_add_tag_to_vocab(self):
         '''Test that a tag can be added to and then retrieved from a vocab.'''
@@ -781,7 +781,7 @@ def test_add_tag_not_logged_in(self):
                 params=tag_string,
                 status=403)
         assert response.json['success'] is False
-        assert response.json['error']['message'] == 'Access denied'
+        assert response.json['error']['__type'] == 'Authorization Error'
 
     def test_add_tag_not_authorized(self):
         tag_dict = {
@@ -795,7 +795,7 @@ def test_add_tag_not_authorized(self):
                         str(self.normal_user.apikey)},
                 status=403)
         assert response.json['success'] is False
-        assert response.json['error']['message'] == 'Access denied'
+        assert response.json['error']['__type'] == 'Authorization Error'
 
     def test_add_vocab_tag_to_dataset(self):
         '''Test that a tag belonging to a vocab can be added to a dataset,
@@ -1116,8 +1116,8 @@ def test_delete_tag_not_logged_in(self):
                 params=helpers.json.dumps(params),
                 status=403)
         assert response.json['success'] is False
-        msg = response.json['error']['message']
-        assert msg == u"Access denied", msg
+        error = response.json['error']['__type']
+        assert error == u"Authorization Error", error
 
     def test_delete_tag_not_authorized(self):
         vocab = self.genre_vocab
@@ -1131,5 +1131,5 @@ def test_delete_tag_not_authorized(self):
                     str(self.normal_user.apikey)},
                 status=403)
         assert response.json['success'] is False
-        msg = response.json['error']['message']
-        assert msg == u"Access denied"
+        msg = response.json['error']['__type']
+        assert msg == u"Authorization Error"

ckan/tests/functional/api/test_follow.py

@@ -424,36 +424,36 @@ def test_01_user_follow_user_bad_apikey(self):
             error = ckan.tests.call_action_api(self.app, 'follow_user',
                     id=self.russianfan['id'], apikey=apikey,
                     status=403)
-            assert error['message'] == 'Access denied'
+            assert error['__type'] == 'Authorization Error'
 
     def test_01_user_follow_dataset_bad_apikey(self):
         for apikey in ('bad api key', '', '     ', 'None', '3', '35.7', 'xxx'):
             error = ckan.tests.call_action_api(self.app, 'follow_dataset',
                     id=self.warandpeace['id'], apikey=apikey,
                     status=403)
-            assert error['message'] == 'Access denied'
+            assert error['__type'] == 'Authorization Error'
 
     def test_01_user_follow_group_bad_apikey(self):
         for apikey in ('bad api key', '', '     ', 'None', '3', '35.7', 'xxx'):
             error = ckan.tests.call_action_api(self.app, 'follow_group',
                     id=self.rogers_group['id'], apikey=apikey,
                     status=403)
-            assert error['message'] == 'Access denied'
+            assert error['__type'] == 'Authorization Error'
 
     def test_01_user_follow_user_missing_apikey(self):
         error = ckan.tests.call_action_api(self.app, 'follow_user',
                 id=self.russianfan['id'], status=403)
-        assert error['message'] == 'Access denied'
+        assert error['__type'] == 'Authorization Error'
 
     def test_01_user_follow_dataset_missing_apikey(self):
         error = ckan.tests.call_action_api(self.app, 'follow_dataset',
                 id=self.warandpeace['id'], status=403)
-        assert error['message'] == 'Access denied'
+        assert error['__type'] == 'Authorization Error'
 
     def test_01_user_follow_group_missing_apikey(self):
         error = ckan.tests.call_action_api(self.app, 'follow_group',
                 id=self.rogers_group['id'], status=403)
-        assert error['message'] == 'Access denied'
+        assert error['__type'] == 'Authorization Error'
 
     def test_01_follow_bad_object_id(self):
         for action in ('follow_user', 'follow_dataset', 'follow_group'):
@@ -878,14 +878,14 @@ def test_01_unfollow_bad_apikey(self):
                     'xxx'):
                 error = ckan.tests.call_action_api(self.app, action,
                         apikey=apikey, status=403, id=self.joeadmin['id'])
-                assert error['message'] == 'Access denied'
+                assert error['__type'] == 'Authorization Error'
 
     def test_01_unfollow_missing_apikey(self):
         '''Test error response when calling unfollow_* without api key.'''
         for action in ('unfollow_user', 'unfollow_dataset', 'unfollow_group'):
             error = ckan.tests.call_action_api(self.app, action, status=403,
                     id=self.joeadmin['id'])
-            assert error['message'] == 'Access denied'
+            assert error['__type'] == 'Authorization Error'
 
     def test_01_unfollow_bad_object_id(self):
         '''Test error response when calling unfollow_* with bad object id.'''

ckan/tests/functional/test_related.py

@@ -498,4 +498,4 @@ def test_api_delete_fail(self):
                             extra_environ=extra)
         r = json.loads(res.body)
         assert r['success'] == False, r
-        assert r[u'error'][u'message'] == u'Access denied' , r
+        assert r[u'error'][u'__type'] == "Authorization Error", r

ckan/tests/logic/test_action.py

@@ -528,10 +528,7 @@ def test_12_user_update(self):
 
         res_obj = json.loads(res.body)
         assert res_obj['help'].startswith("Update a user account.")
-        assert res_obj['error'] == {
-                '__type': 'Authorization Error',
-                'message': 'Access denied'
-            }
+        assert res_obj['error']['__type'] == 'Authorization Error'
         assert res_obj['success'] is False
 
     def test_12_user_update_errors(self):
@@ -893,7 +890,7 @@ def test_22_task_status_normal_user_not_authorized(self):
         res_obj = json.loads(res.body)
         assert res_obj['help'].startswith("Update a task status.")
         assert res_obj['success'] is False
-        assert res_obj['error'] == {'message': 'Access denied', '__type': 'Authorization Error'}
+        assert res_obj['error']['__type'] == 'Authorization Error'
 
     def test_23_task_status_validation(self):
         task_status = {}

ckan/tests/logic/test_tag.py

@@ -151,7 +151,7 @@ def test_08_user_create_not_authorized(self):
         res_obj = json.loads(res.body)
         assert res_obj['help'].startswith("Create a new user.")
         assert res_obj['success'] is False
-        assert res_obj['error'] == {'message': 'Access denied', '__type': 'Authorization Error'}
+        assert res_obj['error']['__type'] == 'Authorization Error'
 
     def test_09_user_create(self):
         user_dict = {'name':'test_create_from_action_api',

ckanext/example_iauthfunctions/tests/test_example_iauthfunctions.py

@@ -103,8 +103,7 @@ def test_group_create_with_visitor(self):
         result = tests.call_action_api(self.app, 'group_create',
                                        name='this_group_should_not_be_created',
                                        status=403)
-        assert result == {'__type': 'Authorization Error',
-                          'message': 'Access denied'}
+        assert result['__type'] == 'Authorization Error'
 
     def test_group_create_with_non_curator(self):
         '''A user who isn't a member of the curators group should not be able
@@ -116,8 +115,7 @@ def test_group_create_with_non_curator(self):
                                        name='this_group_should_not_be_created',
                                        apikey=noncurator['apikey'],
                                        status=403)
-        assert result == {'__type': 'Authorization Error',
-                          'message': 'Access denied'}
+        assert result['__type'] == 'Authorization Error'
 
     def test_group_create_with_curator(self):
         '''A member of the curators group should be able to create a group.
@@ -174,8 +172,7 @@ def test_group_create_with_visitor(self):
         response = tests.call_action_api(self.app, 'group_create',
                                          name='this_group_shouldnt_be_created',
                                          status=403)
-        assert response == {'__type': 'Authorization Error',
-                            'message': 'Access denied'}
+        assert response['__type'] == 'Authorization Error'
 
 
 class TestExampleIAuthFunctionsPluginV2(TestExampleIAuthFunctionsPlugin):
@@ -203,5 +200,4 @@ def test_group_create_with_curator(self):
                                        name='this_group_should_not_be_created',
                                        apikey=curator['apikey'],
                                        status=403)
-        assert result == {'__type': 'Authorization Error',
-                          'message': 'Access denied'}
+        assert result['__type'] == 'Authorization Error'