Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Factor common IAM policy bits into 'google.cloud.iam'. #3188

Merged
merged 10 commits into from
Apr 17, 2017
Prev Previous commit
Next Next commit
Accomodate (future) user-defined roles.
- google.cloud.iam.Policy holds a 'bindings' mapping, which doesn't
  enforce using known roles.

- Its 'owners', 'editors', and 'viewers' are now properties which
  indirect over that 'bindings' attribute.  Note that this is a
  breaking change, as users who relied on mutating one of those sets
  (rather than re-assigning it) will need to update.
  • Loading branch information
tseaver committed Apr 5, 2017
commit ea63f8cf595dcdb41b049f77a0e68c341ce462b8
111 changes: 49 additions & 62 deletions core/google/cloud/iam.py
Original file line number Diff line number Diff line change
@@ -53,9 +53,49 @@ class Policy(object):
def __init__(self, etag=None, version=None):
self.etag = etag
self.version = version
self.owners = set()
self.editors = set()
self.viewers = set()
self.bindings = {}

@property
def owners(self):
"""Legacy access to owner role."""
result = set()
for role in self._OWNER_ROLES:
for member in self.bindings.get(role, ()):
result.add(member)
return result

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.


@owners.setter
def owners(self, value):
"""Update owners."""
self.bindings[OWNER_ROLE] = list(value)

@property
def editors(self):
"""Legacy access to editor role."""
result = set()
for role in self._EDITOR_ROLES:
for member in self.bindings.get(role, ()):
result.add(member)
return result

@editors.setter
def editors(self, value):
"""Update editors."""
self.bindings[EDITOR_ROLE] = list(value)

@property
def viewers(self):
"""Legacy access to viewer role."""
result = set()
for role in self._VIEWER_ROLES:
for member in self.bindings.get(role, ()):
result.add(member)
return result

@viewers.setter
def viewers(self, value):
"""Update viewers."""
self.bindings[VIEWER_ROLE] = list(value)

@staticmethod
def user(email):

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@@ -123,22 +163,6 @@ def authenticated_users():
"""
return 'allAuthenticatedUsers'

def _bind_custom_role(self, role, members): # pylint: disable=no-self-use
"""Bind an API-specific role to members.

Helper for :meth:`from_api_repr`.

:type role: str
:param role: role to bind.

:type members: set of str
:param members: member IDs to be bound to the role.

Subclasses may override.
"""
raise ValueError(
'Unknown binding: role=%s, members=%s' % (role, members))

@classmethod
def from_api_repr(cls, resource):
"""Create a policy from the resource returned from the API.
@@ -154,47 +178,10 @@ def from_api_repr(cls, resource):
policy = cls(etag, version)
for binding in resource.get('bindings', ()):
role = binding['role']
members = set(binding['members'])
if role in cls._OWNER_ROLES:
policy.owners |= members
elif role in cls._EDITOR_ROLES:
policy.editors |= members
elif role in cls._VIEWER_ROLES:
policy.viewers |= members
else:
policy._bind_custom_role(role, members)
members = sorted(binding['members'])

This comment was marked as spam.

policy.bindings[role] = members
return policy

def _role_bindings(self):
"""Enumerate members bound to roles for the policy.

Helper for :meth:`to_api_repr`.

:rtype: list of mapping
:returns: zero or more mappings describing roles / members bound by
the policy.

Subclasses may override.
"""
bindings = []

if self.owners:
bindings.append(
{'role': OWNER_ROLE,
'members': sorted(self.owners)})

if self.editors:
bindings.append(
{'role': EDITOR_ROLE,
'members': sorted(self.editors)})

if self.viewers:
bindings.append(
{'role': VIEWER_ROLE,
'members': sorted(self.viewers)})

return bindings

def to_api_repr(self):
"""Construct a Policy resource.

@@ -209,9 +196,9 @@ def to_api_repr(self):
if self.version is not None:
resource['version'] = self.version

bindings = self._role_bindings()

if bindings:
resource['bindings'] = bindings
if len(self.bindings) > 0:
resource['bindings'] = [
{'role': role, 'members': members}

This comment was marked as spam.

for role, members in sorted(self.bindings.items())]

return resource
62 changes: 36 additions & 26 deletions core/unit_tests/test_iam.py
Original file line number Diff line number Diff line change
@@ -33,6 +33,7 @@ def test_ctor_defaults(self):
self.assertEqual(list(policy.owners), [])
self.assertEqual(list(policy.editors), [])
self.assertEqual(list(policy.viewers), [])
self.assertEqual(dict(policy.bindings), {})

def test_ctor_explicit(self):
VERSION = 17
@@ -43,6 +44,7 @@ def test_ctor_explicit(self):
self.assertEqual(list(policy.owners), [])
self.assertEqual(list(policy.editors), [])
self.assertEqual(list(policy.viewers), [])
self.assertEqual(dict(policy.bindings), {})

def test_user(self):
EMAIL = 'phred@example.com'
@@ -87,6 +89,7 @@ def test_from_api_repr_only_etag(self):
self.assertEqual(list(policy.owners), [])
self.assertEqual(list(policy.editors), [])
self.assertEqual(list(policy.viewers), [])
self.assertEqual(dict(policy.bindings), {})

def test_from_api_repr_complete(self):
from google.cloud.iam import (
@@ -95,8 +98,8 @@ def test_from_api_repr_complete(self):
VIEWER_ROLE,
)

OWNER1 = 'user:phred@example.com'
OWNER2 = 'group:cloud-logs@google.com'
OWNER1 = 'group:cloud-logs@google.com'
OWNER2 = 'user:phred@example.com'
EDITOR1 = 'domain:google.com'
EDITOR2 = 'user:phred@example.com'
VIEWER1 = 'serviceAccount:1234-abcdef@service.example.com'
@@ -114,23 +117,31 @@ def test_from_api_repr_complete(self):
policy = klass.from_api_repr(RESOURCE)
self.assertEqual(policy.etag, 'DEADBEEF')
self.assertEqual(policy.version, 17)
self.assertEqual(sorted(policy.owners), [OWNER2, OWNER1])
self.assertEqual(sorted(policy.owners), [OWNER1, OWNER2])
self.assertEqual(sorted(policy.editors), [EDITOR1, EDITOR2])
self.assertEqual(sorted(policy.viewers), [VIEWER1, VIEWER2])

def test_from_api_repr_bad_role(self):
BOGUS1 = 'user:phred@example.com'
BOGUS2 = 'group:cloud-logs@google.com'
self.assertEqual(
dict(policy.bindings), {
OWNER_ROLE: [OWNER1, OWNER2],
EDITOR_ROLE: [EDITOR1, EDITOR2],
VIEWER_ROLE: [VIEWER1, VIEWER2],
})

def test_from_api_repr_unknown_role(self):
USER = 'user:phred@example.com'
GROUP = 'group:cloud-logs@google.com'
RESOURCE = {
'etag': 'DEADBEEF',
'version': 17,
'bindings': [
{'role': 'nonesuch', 'members': [BOGUS1, BOGUS2]},
{'role': 'unknown', 'members': [USER, GROUP]},
],
}
klass = self._get_target_class()
with self.assertRaises(ValueError):
klass.from_api_repr(RESOURCE)
policy = klass.from_api_repr(RESOURCE)
self.assertEqual(policy.etag, 'DEADBEEF')
self.assertEqual(policy.version, 17)
self.assertEqual(policy.bindings, {'unknown': [GROUP, USER]})

def test_to_api_repr_defaults(self):
policy = self._make_one()
@@ -141,6 +152,7 @@ def test_to_api_repr_only_etag(self):
self.assertEqual(policy.to_api_repr(), {'etag': 'DEADBEEF'})

def test_to_api_repr_full(self):
import operator
from google.cloud.iam import (
OWNER_ROLE,
EDITOR_ROLE,
@@ -153,20 +165,18 @@ def test_to_api_repr_full(self):
EDITOR2 = 'user:phred@example.com'
VIEWER1 = 'serviceAccount:1234-abcdef@service.example.com'
VIEWER2 = 'user:phred@example.com'
EXPECTED = {
'etag': 'DEADBEEF',
'version': 17,
'bindings': [
{'role': OWNER_ROLE, 'members': [OWNER1, OWNER2]},
{'role': EDITOR_ROLE, 'members': [EDITOR1, EDITOR2]},
{'role': VIEWER_ROLE, 'members': [VIEWER1, VIEWER2]},
],
}
BINDINGS = [
{'role': OWNER_ROLE, 'members': [OWNER1, OWNER2]},
{'role': EDITOR_ROLE, 'members': [EDITOR1, EDITOR2]},
{'role': VIEWER_ROLE, 'members': [VIEWER1, VIEWER2]},
]
policy = self._make_one('DEADBEEF', 17)
policy.owners.add(OWNER1)
policy.owners.add(OWNER2)
policy.editors.add(EDITOR1)
policy.editors.add(EDITOR2)
policy.viewers.add(VIEWER1)
policy.viewers.add(VIEWER2)
self.assertEqual(policy.to_api_repr(), EXPECTED)
policy.owners = [OWNER1, OWNER2]
policy.editors = [EDITOR1, EDITOR2]
policy.viewers = [VIEWER1, VIEWER2]
resource = policy.to_api_repr()
self.assertEqual(resource['etag'], 'DEADBEEF')
self.assertEqual(resource['version'], 17)
key = operator.itemgetter('role')
self.assertEqual(
sorted(resource['bindings'], key=key), sorted(BINDINGS, key=key))
102 changes: 30 additions & 72 deletions pubsub/google/cloud/pubsub/iam.py
Original file line number Diff line number Diff line change
@@ -17,9 +17,11 @@
https://cloud.google.com/pubsub/access_control#permissions
"""

from google.cloud.iam import OWNER_ROLE
from google.cloud.iam import EDITOR_ROLE
from google.cloud.iam import VIEWER_ROLE
# pylint: disable=unused-import
from google.cloud.iam import OWNER_ROLE # noqa - backward compat
from google.cloud.iam import EDITOR_ROLE # noqa - backward compat
from google.cloud.iam import VIEWER_ROLE # noqa - backward compat
# pylint: enable=unused-import
from google.cloud.iam import Policy as _BasePolicy

# Pubsub-specific IAM roles
@@ -94,76 +96,32 @@ class Policy(_BasePolicy):
See:
https://cloud.google.com/pubsub/docs/reference/rest/Shared.Types/Policy
https://cloud.google.com/pubsub/docs/reference/rest/Shared.Types/Binding

:type etag: str
:param etag: ETag used to identify a unique of the policy

:type version: int
:param version: unique version of the policy
"""
_OWNER_ROLES = (OWNER_ROLE, PUBSUB_ADMIN_ROLE)
_EDITOR_ROLES = (EDITOR_ROLE, PUBSUB_EDITOR_ROLE)
_VIEWER_ROLES = (VIEWER_ROLE, PUBSUB_VIEWER_ROLE)

def __init__(self, etag=None, version=None):
super(Policy, self).__init__(etag, version)
self.publishers = set()
self.subscribers = set()

def _bind_custom_role(self, role, members):
"""Bind an API-specific role to members.

Helper for :meth:`from_api_repr`.

:type role: str
:param role: role to bind.

:type members: set of str
:param members: member IDs to be bound to the role.

Subclasses may override.
"""
if role == PUBSUB_PUBLISHER_ROLE:
self.publishers |= members
elif role == PUBSUB_SUBSCRIBER_ROLE:
self.subscribers |= members
else:
super(Policy, self)._bind_custom_role(role, members)
"""Roles mapped onto our ``owners`` attribute."""

def _role_bindings(self):
"""Enumerate members bound to roles for the policy.

Helper for :meth:`to_api_repr`.

:rtype: list of mapping
:returns: zero or more mappings describing roles / members bound by
the policy.
"""
bindings = []

if self.owners:
bindings.append(
{'role': PUBSUB_ADMIN_ROLE,
'members': sorted(self.owners)})

if self.editors:
bindings.append(
{'role': PUBSUB_EDITOR_ROLE,
'members': sorted(self.editors)})

if self.viewers:
bindings.append(
{'role': PUBSUB_VIEWER_ROLE,
'members': sorted(self.viewers)})

if self.publishers:
bindings.append(
{'role': PUBSUB_PUBLISHER_ROLE,
'members': sorted(self.publishers)})

if self.subscribers:
bindings.append(
{'role': PUBSUB_SUBSCRIBER_ROLE,
'members': sorted(self.subscribers)})
_EDITOR_ROLES = (EDITOR_ROLE, PUBSUB_EDITOR_ROLE)
"""Roles mapped onto our ``editors`` attribute."""

return bindings
_VIEWER_ROLES = (VIEWER_ROLE, PUBSUB_VIEWER_ROLE)
"""Roles mapped onto our ``viewers`` attribute."""

@property
def publishers(self):
"""Legacy access to owner role."""
return self.bindings.get(PUBSUB_PUBLISHER_ROLE, ())

@publishers.setter
def publishers(self, value):
"""Update publishers."""
self.bindings[PUBSUB_PUBLISHER_ROLE] = list(value)

@property
def subscribers(self):
"""Legacy access to owner role."""
return self.bindings.get(PUBSUB_SUBSCRIBER_ROLE, ())

@subscribers.setter
def subscribers(self, value):
"""Update subscribers."""
self.bindings[PUBSUB_SUBSCRIBER_ROLE] = list(value)
8 changes: 6 additions & 2 deletions pubsub/tests/system.py
Original file line number Diff line number Diff line change
@@ -252,7 +252,9 @@ def test_topic_iam_policy(self):

if topic.check_iam_permissions([PUBSUB_TOPICS_GET_IAM_POLICY]):
policy = topic.get_iam_policy()
policy.viewers.add(policy.user('jjg@google.com'))
viewers = set(policy.viewers)
viewers.add(policy.user('jjg@google.com'))
policy.viewers = viewers
new_policy = topic.set_iam_policy(policy)
self.assertEqual(new_policy.viewers, policy.viewers)

@@ -280,6 +282,8 @@ def test_subscription_iam_policy(self):
if subscription.check_iam_permissions(
[PUBSUB_SUBSCRIPTIONS_GET_IAM_POLICY]):
policy = subscription.get_iam_policy()
policy.viewers.add(policy.user('jjg@google.com'))
viewers = set(policy.viewers)
viewers.add(policy.user('jjg@google.com'))
policy.viewers = viewers
new_policy = subscription.set_iam_policy(policy)
self.assertEqual(new_policy.viewers, policy.viewers)
Loading