Skip to content

Commit

Permalink
Merge 4aa4c27 into c34ea3a
Browse files Browse the repository at this point in the history
  • Loading branch information
Avantol13 committed Jul 14, 2020
2 parents c34ea3a + 4aa4c27 commit 768aec5
Show file tree
Hide file tree
Showing 4 changed files with 158 additions and 30 deletions.
1 change: 1 addition & 0 deletions fence/config-default.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,7 @@ DBGAP_ACCESSION_WITH_CONSENT_REGEX: '(?P<phsid>phs[0-9]+)(.(?P<version>v[0-9]+))
# Also used during User Syncing process to automate managing Storage
# access for users.
# //////////////////////////////////////////////////////////////////////////////////////
GOOGLE_BULK_UPDATES: false
# Configuration for various storage systems for the backend
# NOTE: Remove the {} and supply backends if needed. Example in comments below
STORAGE_CREDENTIALS: {}
Expand Down
34 changes: 34 additions & 0 deletions fence/resources/google/access_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,40 @@
logger = get_logger(__name__)


def bulk_update_google_groups(google_bulk_mapping):
google_project_id = (
config["STORAGE_CREDENTIALS"].get("google", {}).get("google_project_id")
)
with GoogleCloudManager(google_project_id) as gcm:
for group, expected_members in google_bulk_mapping.items():
expected_members = set(expected_members)
logger.debug(f"Starting diff for group {group}...")

# get members list from google
google_members = set(
member.get("email") for member in gcm.get_group_members(group)
)
logger.debug(f"Google membership for {group}: {google_members}")
logger.debug(f"Expected membership for {group}: {expected_members}")

# diff between expected group membership and actual membership
to_delete = set.difference(google_members, expected_members)
to_add = set.difference(expected_members, google_members)
to_update = set.intersection(google_members, expected_members)

# do add
for member_email in to_add:
logger.info(f"Adding to group {group}: {member_email}")
gcm.add_member_to_group(member_email, group)

# do remove
for member_email in to_delete:
logger.info(f"Removing from group {group}: {member_email}")
gcm.remove_member_from_group(member_email, group)

logger.info(f"All already in group {group}: {to_update}")


def get_google_project_number(google_project_id, google_cloud_manager):
"""
Return a project's "projectNumber" which uniquely identifies it.
Expand Down
115 changes: 90 additions & 25 deletions fence/resources/storage/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,9 @@ def create_bucket(self, provider, session, bucketname, project):
c.get_or_create_bucket(bucketname)

@check_exist
def grant_access(self, provider, username, project, access, session):
def grant_access(
self, provider, username, project, access, session, google_bulk_mapping=None
):
"""
this should be exposed via admin endpoint
grant user access to a project in storage backend
Expand All @@ -167,11 +169,19 @@ def grant_access(self, provider, username, project, access, session):
if storage_username:
for b in project.buckets:
self._update_access_to_bucket(
b, provider, storage_user, storage_username, access, session
b,
provider,
storage_user,
storage_username,
access,
session,
google_bulk_mapping=google_bulk_mapping,
)

@check_exist
def revoke_access(self, provider, username, project, session):
def revoke_access(
self, provider, username, project, session, google_bulk_mapping=None
):
"""
this should be exposed via admin endpoint
revoke user access to a project in storage backend
Expand All @@ -188,7 +198,12 @@ def revoke_access(self, provider, username, project, session):
if storage_username:
for b in project.buckets:
self._revoke_access_to_bucket(
b, provider, storage_user, storage_username, session
b,
provider,
storage_user,
storage_username,
session,
google_bulk_mapping=google_bulk_mapping,
)

@check_exist
Expand Down Expand Up @@ -346,7 +361,14 @@ def _get_or_create_storage_user(self, username, provider, session):
return self.clients[provider].get_or_create_user(username)

def _update_access_to_bucket(
self, bucket, provider, storage_user, storage_username, access, session
self,
bucket,
provider,
storage_user,
storage_username,
access,
session,
google_bulk_mapping=None,
):
# Need different logic for google (since buckets can have multiple
# access groups)
Expand All @@ -368,16 +390,26 @@ def _update_access_to_bucket(
if set(bucket_privileges).issubset(access):
bucket_name = bucket_access_group.email

# NOTE: bucket_name for Google is the Google Access Group's
# email address.
# TODO Update storageclient API for more clarity
self.clients[provider].add_bucket_acl(bucket_name, storage_username)

self.logger.info(
"User {}'s Google proxy group ({}) added to Google Bucket Access Group {}.".format(
storage_user.email, storage_username, bucket_name
if google_bulk_mapping is not None:
google_bulk_mapping.setdefault(bucket_name, []).append(
storage_username
)
self.logger.info(
"User {}'s Google proxy group ({}) added to bulk mapping for Google Bucket Access Group {}.".format(
storage_user.email, storage_username, bucket_name
)
)
else:
# NOTE: bucket_name for Google is the Google Access Group's
# email address.
# TODO Update storageclient API for more clarity
self.clients[provider].add_bucket_acl(bucket_name, storage_username)

self.logger.info(
"User {}'s Google proxy group ({}) added to Google Bucket Access Group {}.".format(
storage_user.email, storage_username, bucket_name
)
)
)

StorageManager._add_google_db_entry_for_bucket_access(
storage_user, bucket_access_group, session
Expand All @@ -393,16 +425,36 @@ def _update_access_to_bucket(
)

bucket_name = bucket_access_group.email
self.clients[provider].delete_bucket_acl(bucket_name, storage_username)

self.logger.info(
"User {}'s Google proxy group ({}) removed or never existed in Google Bucket Access Group {}.".format(
storage_user.email, storage_username, bucket_name
if google_bulk_mapping is not None:
google_bulk_mapping.setdefault(bucket_name, [])
while storage_username in google_bulk_mapping[bucket_name]:
google_bulk_mapping[bucket_name].remove(storage_username)
self.logger.debug(
"User {}'s Google proxy group ({}) removed from bulk mapping in Google Bucket Access Group {}.".format(
storage_user.email, storage_username, bucket_name
)
)

else:
self.clients[provider].delete_bucket_acl(
bucket_name, storage_username
)

self.logger.info(
"User {}'s Google proxy group ({}) removed or never existed in Google Bucket Access Group {}.".format(
storage_user.email, storage_username, bucket_name
)
)
)

def _revoke_access_to_bucket(
self, bucket, provider, storage_user, storage_username, session
self,
bucket,
provider,
storage_user,
storage_username,
session,
google_bulk_mapping=None,
):
# Need different logic for google (since buckets can have multiple
# access groups)
Expand All @@ -412,13 +464,26 @@ def _revoke_access_to_bucket(
storage_user, bucket_access_group, session
)
bucket_name = bucket_access_group.email
self.clients[provider].delete_bucket_acl(bucket_name, storage_username)

self.logger.info(
"User {}'s Google proxy group ({}) removed or never existed in from Google Bucket Access Group {}.".format(
storage_user.email, storage_username, bucket_name
if google_bulk_mapping is not None:
google_bulk_mapping.setdefault(bucket_name, [])
while storage_username in google_bulk_mapping[bucket_name]:
google_bulk_mapping[bucket_name].remove(storage_username)
self.logger.debug(
"User {}'s Google proxy group ({}) removed from bulk mapping in Google Bucket Access Group {}.".format(
storage_user.email, storage_username, bucket_name
)
)
else:
self.clients[provider].delete_bucket_acl(
bucket_name, storage_username
)

self.logger.info(
"User {}'s Google proxy group ({}) removed or never existed in from Google Bucket Access Group {}.".format(
storage_user.email, storage_username, bucket_name
)
)
)
else:
self.clients[provider].delete_bucket_acl(bucket.name, storage_username)

Expand Down
38 changes: 33 additions & 5 deletions fence/sync/sync_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
Client,
)
from fence.resources.storage import StorageManager
from fence.resources.google.access_utils import bulk_update_google_groups
from fence.sync import utils


Expand Down Expand Up @@ -684,6 +685,10 @@ def sync_to_db_and_storage_backend(self, user_project, user_info, sess):
Return:
None
"""
google_bulk_mapping = None
if config["GOOGLE_BULK_UPDATES"]:
google_bulk_mapping = {}

self._init_projects(user_project, sess)

auth_provider_list = [
Expand Down Expand Up @@ -718,9 +723,20 @@ def sync_to_db_and_storage_backend(self, user_project, user_info, sess):
# when updating users we want to maintain case sesitivity in the username so
# pass the original, non-lowered user_info dict
self._upsert_userinfo(sess, user_info)
self._revoke_from_storage(to_delete, sess)

self._revoke_from_storage(
to_delete, sess, google_bulk_mapping=google_bulk_mapping
)

self._revoke_from_db(sess, to_delete)
self._grant_from_storage(to_add, user_project_lowercase, sess)

self._grant_from_storage(
to_add,
user_project_lowercase,
sess,
google_bulk_mapping=google_bulk_mapping,
)

self._grant_from_db(
sess,
to_add,
Expand All @@ -730,11 +746,21 @@ def sync_to_db_and_storage_backend(self, user_project, user_info, sess):
)

# re-grant
self._grant_from_storage(to_update, user_project_lowercase, sess)
self._grant_from_storage(
to_update,
user_project_lowercase,
sess,
google_bulk_mapping=google_bulk_mapping,
)
self._update_from_db(sess, to_update, user_project_lowercase)

self._validate_and_update_user_admin(sess, user_info_lowercase)

if config["GOOGLE_BULK_UPDATES"]:
self.logger.info("Doing bulk Google update...")
bulk_update_google_groups(google_bulk_mapping)
self.logger.info("Bulk Google update done!")

sess.commit()

def _revoke_from_db(self, sess, to_delete):
Expand Down Expand Up @@ -900,7 +926,7 @@ def _upsert_userinfo(self, sess, user_info):
tag = Tag(key=k, value=v)
u.tags.append(tag)

def _revoke_from_storage(self, to_delete, sess):
def _revoke_from_storage(self, to_delete, sess, google_bulk_mapping=None):
"""
If a project have storage backend, revoke user's access to buckets in
the storage backend.
Expand Down Expand Up @@ -936,9 +962,10 @@ def _revoke_from_storage(self, to_delete, sess):
username=username,
project=project,
session=sess,
google_bulk_mapping=google_bulk_mapping,
)

def _grant_from_storage(self, to_add, user_project, sess):
def _grant_from_storage(self, to_add, user_project, sess, google_bulk_mapping=None):
"""
If a project have storage backend, grant user's access to buckets in
the storage backend.
Expand Down Expand Up @@ -977,6 +1004,7 @@ def _grant_from_storage(self, to_add, user_project, sess):
project=project,
access=access,
session=sess,
google_bulk_mapping=google_bulk_mapping,
)

def _init_projects(self, user_project, sess):
Expand Down

0 comments on commit 768aec5

Please sign in to comment.