Skip to content

Commit

Permalink
Implement security checking
Browse files Browse the repository at this point in the history
  • Loading branch information
jamadden committed Jul 6, 2020
1 parent 4bbad69 commit d2e7284
Show file tree
Hide file tree
Showing 3 changed files with 176 additions and 21 deletions.
110 changes: 97 additions & 13 deletions docs/security.rst
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,11 @@ Example
Lets put these pieces together and check how security applies.

We'll begin by defining the same subscription we used :doc:`previously
<static>`, but we'll add a permission and an owner. We'll use
<static>`, but we'll establish a security policy, and require a
permission and owner for the subscription. We'll use
:mod:`zope.principalregistry` to provide a global ``IAuthentication``
utility and a defined principal:
utility; this also provides a global ``IUnauthenticatedPrincipal``
that is used if the ``owner`` cannot be found at runtime:

.. doctest::

Expand All @@ -81,22 +83,15 @@ utility and a defined principal:
... <include package="zope.component" />
... <include package="zope.container" />
... <include package="zope.principalregistry" />
... <include package="zope.principalregistry" file="meta.zcml" />
... <include package="zope.securitypolicy" />
... <include package="zope.securitypolicy" file="securitypolicy.zcml" />
... <include package="nti.webhooks" />
... <principal
... id="zope.manager"
... title="Manager"
... description="System Manager"
... login="admin"
... password_manager="SHA1"
... password="40bd001563085fc35165329ea1ff5c5ecbdbbeef"
... />
... <webhooks:staticSubscription
... to="https://this_domain_does_not_exist"
... for="zope.container.interfaces.IContentContainer"
... when="zope.lifecycleevent.interfaces.IObjectCreatedEvent"
... permission="zope.View"
... owner="zope.manager" />
... owner="some.one" />
... </configure>
... """)

Expand All @@ -112,11 +107,100 @@ Next, we can find the :term:`active` subscription, just as before:
[<...Subscription ... to='https://this_domain_does_not_exist' for=IContentContainer when=IObjectCreatedEvent>]


Subscription Is Not Applicable By Default
-----------------------------------------

Next, we need to know if the subscription is :term:`applicable` to the
data. Unlike before, since we have security constraints in place, the subscription is *not* applicable:
data. Unlike before, since we have security constraints in place, the
subscription is *not* applicable:

.. doctest::

>>> subscriptions = find_active_subscriptions_for(event.object, event)
>>> [subscription.isApplicable(event.object) for subscription in subscriptions]
[True]

Wait, wait...what happened there? It turns out that since we don't
have any defined principal identified by ``some.one``, we use the
global ``IUnauthenticatedPrincipal``, an anonymous user. In turn, the
directives executed by loading ``securitypolicy.zcml`` from
``zope.securitypolicy`` give anonymous users the ``zope.View``
permission by default. Let's reverse that and check again.

.. doctest::

>>> from zope.securitypolicy.rolepermission import rolePermissionManager
>>> rolePermissionManager.denyPermissionToRole('zope.View', 'zope.Anonymous')
>>> [subscription.isApplicable(event.object) for subscription in subscriptions]
[False]

Ahh, that's better.

Subscription Applicable Once Principals are Defined
---------------------------------------------------

To grant access in an expected way, we'll use
``zope.principalregistry`` to globally define the prinicpal we're
looking for, as well as globally grant that principal the permissions
necessary:

>>> conf_context = xmlconfig.string("""
... <configure
... xmlns="http://namespaces.zope.org/zope"
... xmlns:webhooks="http://nextthought.com/ntp/webhooks"
... >
... <include package="zope.securitypolicy" file="meta.zcml" />
... <include package="zope.principalregistry" file="meta.zcml" />
... <principal
... id="some.one"
... title="Some One"
... login="some.one"
... password_manager="SHA1"
... password="40bd001563085fc35165329ea1ff5c5ecbdbbeef"
... />
... <grant principal="some.one" permission="zope.View" />
... </configure>
... """)

Now our webhook is :term:`applicable`:

.. doctest::

>>> [subscription.isApplicable(event.object) for subscription in subscriptions]
[True]

Existing Interactions
---------------------

If there was already an interaction going on (e.g., for the logged in
user that created the object), the owner of the subscription is added
to that interaction for purposes of checking permissions. Security
policies generally only grant access if all participations in the
interaction have access.

We'll demonstrate this by creating and acting as a new principal and
then checking access. Because our new user has no permissions on the
object being created (which of course is highly unusual), the
permission check will fail.

.. doctest::

>>> conf_context = xmlconfig.string("""
... <configure
... xmlns="http://namespaces.zope.org/zope"
... xmlns:webhooks="http://nextthought.com/ntp/webhooks"
... >
... <include package="zope.principalregistry" file="meta.zcml" />
... <principal
... id="some.one.else"
... title="Some One Else"
... login="some.one.else"
... password_manager="SHA1"
... password="40bd001563085fc35165329ea1ff5c5ecbdbbeef"
... />
... </configure>
... """)
>>> from zope.security.testing import interaction
>>> with interaction('some.one.else'):
... [subscription.isApplicable(event.object) for subscription in subscriptions]
[False]
1 change: 1 addition & 0 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
'nti.testing',
'zope.testrunner',
'zope.lifecycleevent',
'zope.securitypolicy', # ZCML directives for granting/denying
# Easy mocking of ``requests``.
'responses',
]
Expand Down
86 changes: 78 additions & 8 deletions src/nti/webhooks/subscriptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,25 @@
from zope.interface import implementer
from zope.component.globalregistry import BaseGlobalComponents

from zope.authentication.interfaces import IAuthentication
from zope.authentication.interfaces import IUnauthenticatedPrincipal
from zope.authentication.interfaces import PrincipalLookupError

from zope.security.interfaces import IPermission
from zope.security.management import newInteraction
from zope.security.management import queryInteraction
from zope.security.management import getInteraction
from zope.security.management import endInteraction
from zope.security.management import checkPermission
from zope.security.testing import Participation


from zope.container.interfaces import INameChooser
from zope.container.btree import BTreeContainer
from zope.container.constraints import checkObject
from zope.cachedescriptors.property import CachedProperty


from nti.schema.fieldproperty import createDirectFieldProperties
from nti.schema.schema import SchemaConfigured

Expand Down Expand Up @@ -53,6 +67,38 @@ def __init__(self, **kwargs):
SchemaConfigured.__init__(self, **kwargs)
_CheckObjectOnSetBTreeContainer.__init__(self)

def _find_principal(self, data):
principal = None
for context in (data, None):
auth = component.queryUtility(IAuthentication, context=context)
if auth is None:
continue

try:
principal = auth.getPrincipal(self.owner_id)
except PrincipalLookupError:
# If no principal by that name exists, use the unauthenticatedPrincipal.
# This could return None. It will still be replaced by the
# named principal in the other IAuthentication, if need be.
principal = auth.unauthenticatedPrincipal()
else:
assert principal is not None
break
if principal is None:
# Hmm. Either no IAuthentication found, or none of them found a
# principal while also not having an unauthenticated principal.
# In that case, we will fall back to the global IUnauthenticatedPrincipal as
# defined by zope.principalregistry. This should typically not happen.
principal = component.getUtility(IUnauthenticatedPrincipal)
return principal

def _find_permission(self, data):
for context in (data, None):
perm = component.queryUtility(IPermission, self.permission_id, context=context)
if perm is not None:
break
return perm

def isApplicable(self, data):
if hasattr(self.for_, 'providedBy'):
if not self.for_.providedBy(data):
Expand All @@ -61,15 +107,39 @@ def isApplicable(self, data):
if not isinstance(data, self.for_): # pylint:disable=isinstance-second-argument-not-valid-type
return False

if not self.permission_id or not self.owner_id:
if not self.permission_id and not self.owner_id:
# If no security is requested, we're good.
return True

# TODO: Check the principal and the permission. Find the permission
# by name in the registry, find the principal_id by name in the
# registry, use the security policy to check access.
# TODO: We probably need to do the principal lookup in the context of the data, just in case
# there are local principal registries.
return False
# OK, now we need to find the permission and the principal.
# Both should be found in the context of the data; if not
# there, then check the currently installed site.
principal = self._find_principal(data)
permission = self._find_permission(data)

if principal is None or permission is None:
# A missing permission causes zope.security to grant full access.
# It's treated the same as zope.Public. So don't let that happen.
return False

# Now, we need to set up the interaction and do the security check.
participation = Participation(principal)
current_interaction = queryInteraction()
if current_interaction is not None:
# Cool, we can add our participation to the interaction.
current_interaction.add(participation)
else:
newInteraction(participation)

try:
# Yes, this needs the ID of the permission, not the permission object.
return checkPermission(self.permission_id, data)
finally:
if current_interaction is not None:
current_interaction.remove(participation)
else:
endInteraction()


@CachedProperty('dialect_id')
def dialect(self):
Expand All @@ -91,7 +161,7 @@ def createDeliveryAttempt(self, payload_data):
validator = component.getUtility(IWebhookDestinationValidator, u'', self)
try:
validator.validateTarget(self.to)
except Exception as ex: # pylint:disable=broad-except
except Exception: # pylint:disable=broad-except
attempt.status = 'failed'
# The exception value can vary; it's not intended to be presented to end
# users as-is
Expand Down

0 comments on commit d2e7284

Please sign in to comment.