Chris/joe/GitHub team sync

README.md

@@ -29,7 +29,6 @@ These environment variables are required to work on the relevant cog.
 | KING_ARTHUR_CLOUDFLARE_TOKEN          | Zones                 | A token for the Cloudflare API used for the Cloudflare commands in King Arthur \* | Required                  |
 | KING_ARTHUR_GITHUB_ORG                | GitHubManagement      | The github organisation to fetch teams from                                       | python-discord            |
 | KING_ARTHUR_GITHUB_TOKEN              | GitHubManagement      | The github token used to manage the GitHub organisation                           | Required                  |
-| KING_ARTHUR_GITHUB_TEAM               | GitHubManagement      | The slug of the GitHub team to add new members to                                 | staff                     |
 | KING_ARTHUR_GRAFANA_URL               | GrafanaLDAPTeamSync   | The URL to the grafana instance to manage teams                                   | https://grafana.pydis.wtf |
 | KING_ARTHUR_GRAFANA_TOKEN             | GrafanaLDAPTeamSync   | The grafana token used to sync teams with LDAP                                    | Required                  |
 | KING_ARTHUR_YOUTUBE_API_KEY           | Motivation            | The YouTube API key to fetch missions with                                        | Required                  |

arthur/apis/directory/keycloak.py

@@ -30,3 +30,20 @@ def force_password_reset(username: str, password: str) -> None:
         raise ValueError(msg)
 
     client.set_user_password(user_id, password, temporary=True)
+
+
+def get_user_github_id(username: str) -> str | None:
+    """Fetch a users GitHub ID from Keycloak."""
+    client = create_client()
+
+    user = client.get_user(username)
+    github_id = None
+    for ident in user["federatedIdentities"]:
+        if ident["identityProvider"] == "github":
+            github_id = ident["userId"]
+            break
+
+    if not github_id:
+        return None
+
+    return github_id

arthur/apis/github/__init__.py

@@ -1,5 +1,5 @@
 from .common import GitHubError
 from .orgs import remove_org_member
-from .teams import add_staff_member
+from .teams import add_member_to_team
 
-__all__ = ("GitHubError", "add_staff_member", "remove_org_member")
+__all__ = ("GitHubError", "add_member_to_team", "remove_org_member")

arthur/apis/github/teams.py

@@ -4,10 +4,10 @@
 from arthur.config import CONFIG
 
 
-async def add_staff_member(username: str) -> None:
-    """Add a user to the default GitHub team."""
+async def add_member_to_team(username: str, github_team_slug: str) -> None:
+    """Add a user to a GitHub team."""
     async with aiohttp.ClientSession() as session:
-        endpoint = f"https://api.github.com/orgs/{CONFIG.github_org}/teams/{CONFIG.github_team}/memberships/{username}"
+        endpoint = f"https://api.github.com/orgs/{CONFIG.github_org}/teams/{github_team_slug}/memberships/{username}"
         async with session.put(endpoint, headers=HEADERS) as response:
             try:
                 response.raise_for_status()
@@ -25,3 +25,22 @@ async def add_staff_member(username: str) -> None:
 
                 msg = f"Unexpected error: {e.message}"
                 raise GitHubError(msg)
+
+
+async def remove_member_from_team(username: str, github_team_slug: str) -> None:
+    """Remove a user from a GitHub team."""
+    async with aiohttp.ClientSession() as session:
+        endpoint = f"https://api.github.com/orgs/{CONFIG.github_org}/teams/{github_team_slug}/memberships/{username}"
+        async with session.delete(endpoint, headers=HEADERS) as response:
+            try:
+                response.raise_for_status()
+            except aiohttp.ClientResponseError as e:
+                if e.status == HTTP_404:
+                    msg = f"Team or user not found: {e.message}"
+                    raise GitHubError(msg)
+                if e.status == HTTP_403:
+                    msg = f"Forbidden: {e.message}"
+                    raise GitHubError(msg)
+
+                msg = f"Unexpected error: {e.message}"
+                raise GitHubError(msg)

arthur/config.py

@@ -23,7 +23,6 @@ class Config(
     grafana_token: pydantic.SecretStr | None = None
     github_token: pydantic.SecretStr | None = None
     github_org: str = "python-discord"
-    github_team: str = "staff"
 
     devops_role: int = 409416496733880320
     helpers_role: int = 267630620367257601

arthur/constants.py

@@ -1,16 +1,27 @@
 """Constants, primarily used for LDAP enrollment preferences."""
 
-# Users are only checked for enrollment if they have this role. This doesn't grant them any
-# permissions, it is for performance to avoid iterating roles for every other user in the guild.
-LDAP_BASE_STAFF_ROLE = 267630620367257601
+from typing import TypedDict
+
+
+class LDAPGroupMapping(TypedDict):
+    """Mapping of an LDAP group to its Discord role ID and GitHub team ID."""
+
+    discord_role_id: int
+    github_team_slug: str
 
-# This is a mapping of LDAP groups to Discord roles. It is used to determine which users should be
-# eligible for LDAP enrollment.
-LDAP_ROLE_MAPPING = {
-    "devops": 409416496733880320,
-    "administrators": 267628507062992896,
-    "moderators": 267629731250176001,
-    "coredevs": 587606783669829632,
-    "events": 787816728474288181,
-    "directors": 267627879762755584,
+
+# This is a mapping of LDAP groups to Discord role IDs and GitHub team IDs. It is used to determine
+# which users should be eligible for LDAP enrollment.
+LDAP_ROLE_MAPPING: dict[str, LDAPGroupMapping] = {
+    # "helpers": {"discord_role_id": 267630620367257601, "github_team_slug": "helpers"},  # noqa: ERA001
+    "devops": {"discord_role_id": 409416496733880320, "github_team_slug": "devops"},
+    "administrators": {"discord_role_id": 267628507062992896, "github_team_slug": "admins"},
+    "moderators": {"discord_role_id": 267629731250176001, "github_team_slug": "moderators"},
+    "coredevs": {"discord_role_id": 587606783669829632, "github_team_slug": "core-developers"},
+    "events": {"discord_role_id": 787816728474288181, "github_team_slug": "events"},
+    "directors": {"discord_role_id": 267627879762755584, "github_team_slug": "directors"},
 }
+
+# Users are only checked for enrollment if they have this role. This doesn't grant them any
+# permissions, it is for performance to avoid iterating roles for every other user in the guild.
+HELPER_ROLE_ID = 267630620367257601  # LDAP_ROLE_MAPPING["helpers"]["discord_role_id"]

arthur/exts/directory/ldap.py

@@ -12,7 +12,7 @@
 
 from arthur.apis.directory import freeipa, keycloak, ldap
 from arthur.config import CONFIG
-from arthur.constants import LDAP_BASE_STAFF_ROLE, LDAP_ROLE_MAPPING
+from arthur.constants import HELPER_ROLE_ID, LDAP_ROLE_MAPPING
 from arthur.log import logger
 
 if TYPE_CHECKING:
@@ -126,7 +126,7 @@ async def generate_creds(self, interaction: discord.Interaction, _button: ui.But
         """Generate credentials for the user."""
         user = interaction.user
 
-        if LDAP_BASE_STAFF_ROLE not in [role.id for role in user.roles]:
+        if HELPER_ROLE_ID not in [role.id for role in user.roles]:
             await interaction.response.send_message(
                 "You are not eligible for LDAP enrollment.", ephemeral=True
             )
@@ -265,7 +265,7 @@ async def on_member_update(self, before: discord.Member, after: discord.Member)
         before_roles = {role.id for role in before.roles}
         after_roles = {role.id for role in after.roles}
 
-        if LDAP_BASE_STAFF_ROLE in before_roles or LDAP_BASE_STAFF_ROLE in after_roles:
+        if HELPER_ROLE_ID in before_roles or HELPER_ROLE_ID in after_roles:
             await self.sync_users()
 
     async def bootstrap(self, user: discord.Member) -> tuple[BootstrapType, str, str | None]:
@@ -330,8 +330,8 @@ async def cog_load(self) -> None:  # noqa: C901, PLR0912
             await channel.send(bootstrap_message, view=BootstrapView(self))
 
         # Validate all enrolled roles can see the channel
-        for role_id in LDAP_ROLE_MAPPING.values():
-            role = channel.guild.get_role(role_id)
+        for mapping in LDAP_ROLE_MAPPING.values():
+            role = channel.guild.get_role(mapping["discord_role_id"])
 
             if not role:
                 continue
@@ -361,8 +361,8 @@ def _user_groups(user: discord.Member) -> list[str]:
         """Return the groups a user is enrolled in."""
         return [
             role
-            for role, discord_role_id in LDAP_ROLE_MAPPING.items()
-            if discord_role_id in [r.id for r in user.roles]
+            for role, mapping in LDAP_ROLE_MAPPING.items()
+            if mapping["discord_role_id"] in [r.id for r in user.roles]
         ]
 
     async def get_user_diff(
@@ -373,9 +373,9 @@ async def get_user_diff(
         users = await ldap.find_users()
         ldap_discord_id_map = {user.employee_number: user for user in users}
 
-        enrolled_roles = set(LDAP_ROLE_MAPPING.values())
+        enrolled_roles = {mapping["discord_role_id"] for mapping in LDAP_ROLE_MAPPING.values()}
 
-        base_role = guild.get_role(LDAP_BASE_STAFF_ROLE)
+        base_role = guild.get_role(HELPER_ROLE_ID)
 
         diff = []
         missing_emp = [user for user in users if user.employee_number is None]

arthur/exts/github/management.py

@@ -4,7 +4,7 @@
 
 from discord.ext.commands import Cog, Context, group
 
-from arthur.apis.github import GitHubError, add_staff_member, remove_org_member
+from arthur.apis.github import GitHubError, add_member_to_team, remove_org_member
 from arthur.config import CONFIG
 
 if TYPE_CHECKING:
@@ -35,7 +35,7 @@ async def github(self, ctx: Context) -> None:
     async def add_team_member(self, ctx: Context, username: str) -> None:
         """Add a user to the default GitHub team."""
         try:
-            await add_staff_member(username)
+            await add_member_to_team(username, "helpers")
             await ctx.send(f":white_check_mark: Successfully invited {username} to the staff team.")
         except GitHubError as e:
             await ctx.send(f":x: Failed to add {username} to the staff team: {e}")