Security changes included, please review!!!!
Author: mramm
Date: Sat Mar 14 18:29:34 2009
New Revision: 6503
URL: http://trac.turbogears.org/changeset/6503
Log:
Fixed controller auth to always use _check_security, and to flash proper
messages, and properly return 403's when the user is authenticated, but not
authorized.
Fixed some issues with the auth texts (extra_environ not being set).
Fixed one test which was checking for the wrong auth denial flash message.
Made controller dispatch only look for notfound handlers on 404's not 403 (
or any other http error for that matter).
Modified:
trunk/tg/controllers.py
trunk/tg/test_stack/test_authz.py
Modified: trunk/tg/controllers.py
==============================================================================
--- trunk/tg/controllers.py (original)
+++ trunk/tg/controllers.py Sat Mar 14 18:29:34 2009
@@ -18,6 +18,8 @@
import pylons
from pylons import url as pylons_url, config
from pylons.controllers import WSGIController
+from pylons.controllers.util import abort
+
from repoze.what.predicates import NotAuthorizedError
import tw
@@ -26,6 +28,7 @@
from tg.render import render as tg_render
from tg.decorators import expose, allow_only
from tg.i18n import setup_i18n
+from tg.flash import flash
from webob import Request
from webob.exc import HTTPUnauthorized
@@ -457,7 +460,7 @@
"""
pass
-def _check_security(obj):
+def _check_controller_auth(obj):
"""this function checks if a controller has a 'alow_only' attribute and
if
it is the case, test that this require predicate can be evaled to True.
It will raise a Forbidden exception if the predicate is not valid.
@@ -468,10 +471,7 @@
klass_instance = obj
if hasattr(klass_instance, "_check_security"):
- if not klass_instance._check_security():
- if hasattr(klass_instance, "_failed_authorization"):
- return klass_instance._failed_authorization()
- raise HTTPUnauthorized().exception
+ klass_instance._check_security()
def _object_dispatch(obj, url_path):
remainder = url_path
@@ -503,9 +503,12 @@
log.debug("a 401 error occured for obj: %s" % obj)
raise
- except HTTPException:
+ except HTTPException, e:
+ if e.status_int != 404:
+ raise e.exception
+
if not notfound_handlers:
- raise HTTPNotFound().exception
+ raise e.exception
name, obj, parent, remainder = notfound_handlers.pop()
if name == 'default':
@@ -517,7 +520,7 @@
continue
def _find_restful_dispatch(obj, parent, remainder):
- _check_security(obj)
+ _check_controller_auth(obj)
if not inspect.isclass(obj) and not isinstance(obj, RestController):
return obj, remainder
if inspect.isclass(obj) and not issubclass(obj, RestController):
@@ -611,7 +614,7 @@
if obj is None:
raise HTTPNotFound().exception
- _check_security(obj)
+ _check_controller_auth(obj)
if _iscontroller(obj):
return obj, parent, remainder
@@ -781,21 +784,32 @@
return result
def _check_security(self):
- if not hasattr(self, "allow_only") or self.allow_only is None:
+ if not hasattr(self, "allow_only") or self.allow_only is None:
log.debug('No controller-wide authorization at %s',
pylons.request.path)
return True
-
- environ = pylons.request.environ
try:
- self.allow_only.check_authorization(environ)
- log.debug('Succeeded controller-wide authorization at %s',
- pylons.request.path)
- return True
- except NotAuthorizedError, error:
- log.debug('Failed controller authorization at %s',
- pylons.request.path)
- return False
+ predicate = self.allow_only
+ predicate.check_authorization(pylons.request.environ)
+ except NotAuthorizedError, e:
+ reason = unicode(e)
+ if hasattr(self, '_failed_authorization'):
+ # Should shortcircut the rest, but if not we will still
+ # deny authorization
+ self._failed_authorization(reason)
+ if pylons.request.environ.get('REMOTE_USER'):
+ # The user is authenticated but not allowed.
+ code = 403
+ status = 'error'
+ else:
+ # The user has not been not authenticated.
+ code = 401
+ status = 'warning'
+ reason = "The current user must have been authenticated"
+ pylons.response.status = code
+ flash(reason, status=status)
+ abort(code, comment=reason)
+
class WSGIAppController(TGController):
"""
Modified: trunk/tg/test_stack/test_authz.py
==============================================================================
--- trunk/tg/test_stack/test_authz.py (original)
+++ trunk/tg/test_stack/test_authz.py Sat Mar 14 18:29:34 2009
@@ -155,7 +155,7 @@
class ControllerWithAllowOnlyDecoratorAndAuthzDenialHandler(TGController):
"""
Mock TG2 protected controller using the @allow_only decorator, but also
- using .failed_authorization()
+ using ._failed_authorization()
"""
@@ -311,7 +311,7 @@
self._check_flash(resp, 'The current user must have been
authenticated')
# As an authenticated user:
environ = {'REMOTE_USER': 'someone'}
- resp = self.app.get('/hr/', status=403)
+ resp = self.app.get('/hr/', extra_environ = environ, status=403)
assert "you can manage Human Resources" not in resp.body
self._check_flash(resp, r'The current user must be
\"hiring-manager\"')
@@ -328,7 +328,7 @@
self._check_flash(resp, 'The current user must have been
authenticated')
# As an authenticated user:
environ = {'REMOTE_USER': 'someone'}
- resp = self.app.get('/hr/hire/gustavo', status=403)
+ resp = self.app.get('/hr/hire/gustavo', extra_environ = environ,
status=403)
assert "was just hired" not in resp.body
self._check_flash(resp, r'The current user must be
\"hiring-manager\"')
@@ -387,6 +387,7 @@
def test_authz_granted_without_require(self):
environ = {'REMOTE_USER': 'hiring-manager'}
resp = self.app.get('/', extra_environ=environ, status=200)
+ print resp
self.assertEqual("you can manage Human Resources", resp.body)
def test_authz_denied_without_require(self):
@@ -396,7 +397,7 @@
self._check_flash(resp, 'The current user must have been
authenticated')
# As an authenticated user:
environ = {'REMOTE_USER': 'someone'}
- resp = self.app.get('/', status=403)
+ resp = self.app.get('/', extra_environ = environ, status=403)
assert "you can manage Human Resources" not in resp.body
self._check_flash(resp, r'The current user must be
\"hiring-manager\"')
@@ -410,10 +411,11 @@
# As an anonymous user:
resp = self.app.get('/hire/gustavo', status=401)
assert "was just hired" not in resp.body
+ print resp.body
self._check_flash(resp, 'The current user must have been
authenticated')
# As an authenticated user:
environ = {'REMOTE_USER': 'someone'}
- resp = self.app.get('/hire/gustavo', status=403)
+ resp = self.app.get('/hire/gustavo', extra_environ = environ,
status=403)
assert "was just hired" not in resp.body
self._check_flash(resp, r'The current user must be
\"hiring-manager\"')
@@ -452,7 +454,7 @@
# As an anonymous user:
resp = self.app.get('/mounted_app/da-path', status=401)
assert "Hello from /mounted_app/" not in resp.body
- self._check_flash(resp, r'The current user must be \"gustavo\"')
+ self._check_flash(resp, r'The current user must have been
authenticated')
# As an authenticated user:
environ = {'REMOTE_USER': 'non-gustavo'}
resp = self.app.get('/mounted_app/da-path', extra_environ=environ,
--
Mark Ramm-Christensen
email: mark at compoundthinking dot com
blog: www.compoundthinking.com/blog
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"TurboGears Trunk" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to
[email protected]
For more options, visit this group at
http://groups.google.com/group/turbogears-trunk?hl=en
-~----------~----~----~----~------~----~------~--~---
