From 9a2827538424b8db1eecdd844c643ecf06a74418 Mon Sep 17 00:00:00 2001 From: Buhleva Date: Tue, 6 Apr 2021 18:27:59 +0300 Subject: [PATCH 1/9] Close connection opened on cron sync Connection pushed from the app context are never poped, which causes connections leaks. --- app.py | 43 +++++++++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/app.py b/app.py index b5d4541..e0a7374 100644 --- a/app.py +++ b/app.py @@ -236,9 +236,12 @@ def get_app_installations(): :return: """ with app.app_context() as ctx: - c = ctx.push() - gh = GitHubApp(c) - installations = gh.app_client.app_installations + try: + c = ctx.push() + gh = GitHubApp(c) + installations = gh.app_client.app_installations + finally: + ctx.pop() return installations @@ -256,21 +259,25 @@ def sync_all_teams(): print("========================================================") print(f"## Processing Organization: {i.account['login']}") print("========================================================") - try: - gh = GitHubApp(app.app_context().push()) - client = gh.app_installation(installation_id=i.id) - org = client.organization(i.account["login"]) - for team in org.teams(): - try: - sync_team( - client=client, owner=org.login, team_id=team.id, slug=team.slug, - ) - except Exception as e: - print(f"Organization: {org.login}") - print(f"Unable to sync team: {team.slug}") - print(f"DEBUG: {e}") - except Exception as e: - print(f"DEBUG: {e}") + with app.app_context() as ctx: + try: + gh = GitHubApp(ctx.push()) + client = gh.app_installation(installation_id=i.id) + org = client.organization(i.account["login"]) + for team in org.teams(): + try: + sync_team( + client=client, owner=org.login, team_id=team.id, slug=team.slug, + ) + except Exception as e: + print(f"Organization: {org.login}") + print(f"Unable to sync team: {team.slug}") + print(f"DEBUG: {e}") + clean_up_orgs(org) + except Exception as e: + print(f"DEBUG: {e}") + finally: + ctx.pop() sync_all_teams() From f6fbc5c0aca18de1585fe9af7af3df283fabb74c Mon Sep 17 00:00:00 2001 From: Buhleva Date: Tue, 6 Apr 2021 18:35:50 +0300 Subject: [PATCH 2/9] Add user to organization if found add_or_update_membership - can add the user to the organization if the user has logged in --- app.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/app.py b/app.py index b5d4541..f7534c7 100644 --- a/app.py +++ b/app.py @@ -2,6 +2,7 @@ import os import time import json +import github3 from distutils.util import strtobool from apscheduler.schedulers.background import BackgroundScheduler @@ -13,6 +14,7 @@ app = Flask(__name__) github_app = GitHubApp(app) directory = DirectoryClient() +addUserAsMember = os.environ.get("ADD_MEMBER", False) # Schedule a full sync scheduler = BackgroundScheduler(daemon=True) @@ -180,9 +182,13 @@ def execute_sync(org, team, slug, state): else: for user in state["action"]["add"]: # Validate that user is in org - if org.is_member(user): - print(f"Adding {user} to {slug}") - team.add_or_update_membership(user) + if org.is_member(user) or addUserAsMember: + try: + print(f"Adding {user} to {slug}") + team.add_or_update_membership(user) + except github3.exceptions.NotFoundError: + print(f"User: {user} not found") + pass else: print(f"Skipping {user} as they are not part of the org") From 9f06c6ed18ac7eecff688f6776a0922f972434a7 Mon Sep 17 00:00:00 2001 From: Buhleva Date: Tue, 6 Apr 2021 18:40:25 +0300 Subject: [PATCH 3/9] Encode team name for AAD request Teams with special symbols in the name are receiving 'Invalid filter clause' from the AAD. --- githubapp/azuread.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/githubapp/azuread.py b/githubapp/azuread.py index 3326bc1..2e9f278 100644 --- a/githubapp/azuread.py +++ b/githubapp/azuread.py @@ -70,6 +70,8 @@ def get_group_members(self, token=None, group_name=None): token = self.get_access_token() if not token else token member_list = [] # Calling graph using the access token + # url encode the group name + group_name=requests.utils.quote(group_name) graph_data = requests.get( # Use token to call downstream service f"{self.AZURE_API_ENDPOINT}/groups?$filter=startswith(displayName,'{group_name}')", headers={"Authorization": f"Bearer {token}"}, From d8b13c844f2e19f290842192afacebfa8729c7ad Mon Sep 17 00:00:00 2001 From: Buhleva Date: Fri, 9 Apr 2021 16:10:08 +0300 Subject: [PATCH 4/9] Read all members of Azure AD Group on nextPageLink attribute - collect the users --- githubapp/azuread.py | 52 +++++++++++++++++++++++++++++++------------- 1 file changed, 37 insertions(+), 15 deletions(-) diff --git a/githubapp/azuread.py b/githubapp/azuread.py index 2e9f278..ba44d44 100644 --- a/githubapp/azuread.py +++ b/githubapp/azuread.py @@ -79,26 +79,48 @@ def get_group_members(self, token=None, group_name=None): # print("Graph API call result: %s" % json.dumps(graph_data, indent=2)) try: group_info = json.loads(json.dumps(graph_data, indent=2))["value"][0] - members = requests.get( - f'{self.AZURE_API_ENDPOINT}/groups/{group_info["id"]}/members', - headers={"Authorization": f"Bearer {token}"}, - ).json()["value"] + members = self.get_group_members_pages(token, f'{self.AZURE_API_ENDPOINT}/groups/{group_info["id"]}/members') except IndexError as e: members = [] for member in members: - user_info = self.get_user_info(token=token, user=member["id"]) - if self.AZURE_USER_IS_UPN: - user = { - "username": user_info[self.USERNAME_ATTRIBUTE].split("@")[0], - "email": user_info["mail"], - } + if member['@odata.type'] == '#microsoft.graph.group': + print("Nested group: ", member["displayName"]) else: - user = { - "username": user_info[self.USERNAME_ATTRIBUTE], - "email": user_info["mail"], - } - member_list.append(user) + user_info = self.get_user_info(token=token, user=member["id"]) + if self.AZURE_USER_IS_UPN: + user = { + "username": user_info[self.USERNAME_ATTRIBUTE].split("@")[0], + "email": user_info["mail"], + } + else: + user = { + "username": user_info[self.USERNAME_ATTRIBUTE], + "email": user_info["mail"], + } + member_list.append(user) return member_list + + def get_group_members_pages(self, token=None, url=None): + """ + Get group members + :param token: + :param url: + :return members: + :rtype members: dict + """ + members_data = requests.get( + url, + headers={"Authorization": f"Bearer {token}"} + ) + if members_data.ok != True: + print(f"[GetMembers]: Error getting members data error code {members_data.status_code}") + return [] + + members_data_content = members_data.json() + members = members_data_content["value"] + if "@odata.nextLink" in members_data_content: + members.extend(self.get_group_members_pages(token, members_data_content["@odata.nextLink"])) + return members def get_user_info(self, token=None, user=None): """ From b75b01e9bafac5b65799c7b30e7f0ac121534c3e Mon Sep 17 00:00:00 2001 From: Buhleva Date: Fri, 9 Apr 2021 16:20:42 +0300 Subject: [PATCH 5/9] Reformat --- githubapp/azuread.py | 59 ++++++++++++++++++++++++-------------------- 1 file changed, 32 insertions(+), 27 deletions(-) diff --git a/githubapp/azuread.py b/githubapp/azuread.py index 313c172..4f00b70 100644 --- a/githubapp/azuread.py +++ b/githubapp/azuread.py @@ -79,27 +79,29 @@ def get_group_members(self, token=None, group_name=None): # print("Graph API call result: %s" % json.dumps(graph_data, indent=2)) try: group_info = json.loads(json.dumps(graph_data, indent=2))["value"][0] - members = self.get_group_members_pages(token, f'{self.AZURE_API_ENDPOINT}/groups/{group_info["id"]}/members') + members = self.get_group_members_pages( + token, f'{self.AZURE_API_ENDPOINT}/groups/{group_info["id"]}/members' + ) except IndexError as e: members = [] for member in members: - if member['@odata.type'] == '#microsoft.graph.group': - print("Nested group: ", member["displayName"]) + if member["@odata.type"] == "#microsoft.graph.group": + print("Nested group: ", member["displayName"]) else: - user_info = self.get_user_info(token=token, user=member["id"]) - if self.AZURE_USER_IS_UPN: - user = { - "username": user_info[self.USERNAME_ATTRIBUTE].split("@")[0], - "email": user_info["mail"], - } - else: - user = { - "username": user_info[self.USERNAME_ATTRIBUTE], - "email": user_info["mail"], - } - member_list.append(user) + user_info = self.get_user_info(token=token, user=member["id"]) + if self.AZURE_USER_IS_UPN: + user = { + "username": user_info[self.USERNAME_ATTRIBUTE].split("@")[0], + "email": user_info["mail"], + } + else: + user = { + "username": user_info[self.USERNAME_ATTRIBUTE], + "email": user_info["mail"], + } + member_list.append(user) return member_list - + def get_group_members_pages(self, token=None, url=None): """ Get group members @@ -107,20 +109,23 @@ def get_group_members_pages(self, token=None, url=None): :param url: :return members: :rtype members: dict - """ - members_data = requests.get( - url, - headers={"Authorization": f"Bearer {token}"} - ) + """ + members_data = requests.get(url, headers={"Authorization": f"Bearer {token}"}) if members_data.ok != True: - print(f"[GetMembers]: Error getting members data error code {members_data.status_code}") - return [] - + print( + f"[GetMembers]: Error getting members data error code {members_data.status_code}" + ) + return [] + members_data_content = members_data.json() members = members_data_content["value"] - if "@odata.nextLink" in members_data_content: - members.extend(self.get_group_members_pages(token, members_data_content["@odata.nextLink"])) - return members + if "@odata.nextLink" in members_data_content: + members.extend( + self.get_group_members_pages( + token, members_data_content["@odata.nextLink"] + ) + ) + return members def get_user_info(self, token=None, user=None): """ From ee370b811517d467a7df5b2dd372f3f95e746ff6 Mon Sep 17 00:00:00 2001 From: Kalina Buhleva Date: Mon, 12 Apr 2021 10:42:33 +0300 Subject: [PATCH 6/9] Update app.py --- app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app.py b/app.py index c17ceae..7bc0105 100644 --- a/app.py +++ b/app.py @@ -14,7 +14,7 @@ app = Flask(__name__) github_app = GitHubApp(app) directory = DirectoryClient() -addUserAsMember = os.environ.get("ADD_MEMBER", False) +addUserAsMember = os.getenv('ADD_MEMBER', 'False') == 'True' # Schedule a full sync scheduler = BackgroundScheduler(daemon=True) From ad941d2eb82fe6ae90dd620dd6d78521bd21fe91 Mon Sep 17 00:00:00 2001 From: Buhleva Date: Mon, 12 Apr 2021 15:33:41 +0300 Subject: [PATCH 7/9] Init new env variable in __init__ --- app.py | 5 ++--- githubapp/__init__.py | 2 ++ 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/app.py b/app.py index 7bc0105..205fd0c 100644 --- a/app.py +++ b/app.py @@ -9,12 +9,11 @@ from apscheduler.triggers.cron import CronTrigger from flask import Flask -from githubapp import GitHubApp, DirectoryClient, CRON_INTERVAL, TEST_MODE +from githubapp import GitHubApp, DirectoryClient, CRON_INTERVAL, TEST_MODE, ADD_MEMBER app = Flask(__name__) github_app = GitHubApp(app) directory = DirectoryClient() -addUserAsMember = os.getenv('ADD_MEMBER', 'False') == 'True' # Schedule a full sync scheduler = BackgroundScheduler(daemon=True) @@ -182,7 +181,7 @@ def execute_sync(org, team, slug, state): else: for user in state["action"]["add"]: # Validate that user is in org - if org.is_member(user) or addUserAsMember: + if org.is_member(user) or ADD_MEMBER: try: print(f"Adding {user} to {slug}") team.add_or_update_membership(user) diff --git a/githubapp/__init__.py b/githubapp/__init__.py index affc84b..ff32f01 100644 --- a/githubapp/__init__.py +++ b/githubapp/__init__.py @@ -37,3 +37,5 @@ rootlogger.warn('TEST_MODE should be set to "true" or "false"') rootlogger.warn(e) TEST_MODE = False +# Check if should add member to organization +ADD_MEMBER = os.getenv("ADD_MEMBER", "False") == "True" From fc9129b2d6fce686c5ef7d73b8fa1ea6bf9e74af Mon Sep 17 00:00:00 2001 From: Kalina Buhleva Date: Tue, 27 Apr 2021 08:45:42 +0300 Subject: [PATCH 8/9] Update githubapp/__init__.py Co-authored-by: Jared Murrell --- githubapp/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/githubapp/__init__.py b/githubapp/__init__.py index ff32f01..f5b0e1c 100644 --- a/githubapp/__init__.py +++ b/githubapp/__init__.py @@ -38,4 +38,4 @@ rootlogger.warn(e) TEST_MODE = False # Check if should add member to organization -ADD_MEMBER = os.getenv("ADD_MEMBER", "False") == "True" +TEST_MODE = strtobool(os.environ.get("ADD_MEMBER", "False")) From 0c4dad2de5bb01aacb8308020613bde2a1cb349a Mon Sep 17 00:00:00 2001 From: Kalina Buhleva Date: Tue, 27 Apr 2021 08:48:01 +0300 Subject: [PATCH 9/9] Fix vatiable name --- githubapp/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/githubapp/__init__.py b/githubapp/__init__.py index f5b0e1c..eef1eca 100644 --- a/githubapp/__init__.py +++ b/githubapp/__init__.py @@ -38,4 +38,4 @@ rootlogger.warn(e) TEST_MODE = False # Check if should add member to organization -TEST_MODE = strtobool(os.environ.get("ADD_MEMBER", "False")) +ADD_MEMBER = strtobool(os.environ.get("ADD_MEMBER", "False"))