From 638d969ae971d16b239cc3d1f84a353682b10b8a Mon Sep 17 00:00:00 2001 From: Alexanderamiri Date: Sat, 24 Oct 2026 14:36:47 +0200 Subject: [PATCH 1/2] feat(team-provisioner): give team members access to their group mailbox Map registry maintainers to Google MANAGER (OWNER untouched) and enforce group settings so members can read the team group's mail and archive. Settings use a separate token for the apps.groups.settings scope and are skipped when the scope is not delegated, so member sync keeps working. Co-Authored-By: Claude Opus 5.5 (1M context) --- CLAUDE.md | 9 + docs/lambda-functions.md | 32 +++ .../lambda-src/team_provisioner/handler.py | 174 +++++++++++++- .../test_team_provisioner_google_group.py | 217 ++++++++++++++++++ 4 files changed, 420 insertions(+), 12 deletions(-) create mode 100644 terraform/lambda-src/tests/test_team_provisioner_google_group.py diff --git a/CLAUDE.md b/CLAUDE.md index 4bf4e23..27dd8f1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -272,6 +272,15 @@ in Google Workspace Admin. The team-provisioner Lambda uses it to manage Google The SA JSON key is at `/javabin/platform/google-admin-sa`, the impersonation target at `/javabin/platform/google-admin-email`. +Delegated scopes (Admin console > Security > API controls > Domain-wide delegation; the field replaces +the whole list, so always keep the existing ones): `admin.directory.group`, +`admin.directory.group.member`, `admin.directory.user`, `admin.directory.user.alias`, `gmail.send`, +`gmail.settings.sharing`, `apps.groups.settings` (prefix `https://www.googleapis.com/auth/`). +Google rejects a token if any requested scope is not delegated, so `apps.groups.settings` is +requested on a separate token (`_get_google_groups_settings_token`) and team group settings are +skipped, not failed, when it is missing. The Groups Settings API (`groupssettings.googleapis.com`) +must be enabled in `javabin-platform`. + ## Work Packages ### Task Status diff --git a/docs/lambda-functions.md b/docs/lambda-functions.md index 40c0a63..1c29902 100644 --- a/docs/lambda-functions.md +++ b/docs/lambda-functions.md @@ -83,6 +83,38 @@ Queries Security Hub for active findings at HIGH and CRITICAL severity, aggregat | `/javabin/platform/atlassian-org-id` | Atlassian organisation ID | | `/javabin/platform/atlassian-api-key` | Atlassian org admin API key (unscoped — full org admin) | +### Team Google group (`sync_google_group`) + +Each team gets the group `team-{name}@java.no`. Members are kept in step with the registry: a member +with `role: maintainer` is a Google **MANAGER**, everyone else a **MEMBER**. A differing role is +changed in place. Existing **OWNER**s are never changed. Members not in the registry are removed. + +After the member sync, `enforce_team_group_settings` makes the group's mail readable by its members. +It reads the Groups Settings API and writes only the keys that differ: + +| Setting | Value | +|---------|-------| +| `whoCanViewGroup` | `ALL_MEMBERS_CAN_VIEW` (members read the conversations at groups.google.com/a/java.no/g/team-{name}) | +| `isArchived` / `archiveOnly` | `true` / `false` (keep an archive, still deliver mail) | +| `whoCanViewMembership` | `ALL_IN_DOMAIN_CAN_VIEW` | +| `whoCanJoin` | `INVITED_CAN_JOIN` (the registry is the only way in) | +| `allowWebPosting` | `true` | +| `allowExternalMembers` | `false` | + +Not enforced, so outside senders reach the group as before: `whoCanPostMessage`, message and spam +moderation, collaborative inbox, and posting as the group. The current `whoCanPostMessage` and +`messageModerationLevel` are returned in the result for visibility. + +The settings use their own token with only the `apps.groups.settings` scope. If that scope is not +delegated, the settings step is skipped with a Slack `:warning:` and member sync still succeeds. If +the Groups Settings API is disabled in GCP project `javabin-platform`, the step reports that. +Org groups from the registry's `groups.yaml` are not touched by this. + +Delegated scopes the service account needs (client ID 103162295681564397615): +`admin.directory.group`, `admin.directory.group.member`, `admin.directory.user`, +`admin.directory.user.alias`, `gmail.send`, `gmail.settings.sharing`, `apps.groups.settings` +(all under `https://www.googleapis.com/auth/`). + ### Atlassian group sync (`sync_atlassian`) Invoked by the registry's `provision-groups.yml` via `scripts/provision-atlassian.py`, which resolves diff --git a/terraform/lambda-src/team_provisioner/handler.py b/terraform/lambda-src/team_provisioner/handler.py index 333c4aa..77a73d6 100644 --- a/terraform/lambda-src/team_provisioner/handler.py +++ b/terraform/lambda-src/team_provisioner/handler.py @@ -184,12 +184,21 @@ def _create_jwt(payload, private_key_pem): ) -def _get_google_access_token(): +GOOGLE_GROUPS_SETTINGS_SCOPE = "https://www.googleapis.com/auth/apps.groups.settings" + + +class GoogleScopeNotDelegated(Exception): + """Raised when the delegated service account lacks a requested scope.""" + + +def _get_delegated_token(scope, cache_key): """Exchange a domain-wide delegation JWT for a Google OAuth2 access token. - Token is cached for ~58 minutes (Google tokens last 1 hour). + Token is cached for ~58 minutes (Google tokens last 1 hour). Raises + GoogleScopeNotDelegated when Google answers `unauthorized_client`, which + happens when any requested scope is missing from the delegation grant. """ - cached = _credential_cache.get("_google_token") + cached = _credential_cache.get(cache_key) if cached and cached["exp"] > time.time() + 120: return cached["token"] @@ -201,7 +210,7 @@ def _get_google_access_token(): { "iss": sa_json["client_email"], "sub": admin_email, - "scope": GOOGLE_SCOPES, + "scope": scope, "aud": "https://oauth2.googleapis.com/token", "iat": now, "exp": now + 3600, @@ -227,15 +236,32 @@ def _get_google_access_token(): except urllib.error.HTTPError as e: body_text = e.read().decode("utf-8", errors="replace") logger.error("Google OAuth token exchange failed: %d %s", e.code, body_text) + if "unauthorized_client" in body_text: + raise GoogleScopeNotDelegated(scope) from e raise - _credential_cache["_google_token"] = { + _credential_cache[cache_key] = { "token": token, "exp": time.time() + 3500, } return token +def _get_google_access_token(): + """Access token for the Admin Directory + Gmail scopes (GOOGLE_SCOPES).""" + return _get_delegated_token(GOOGLE_SCOPES, "_google_token") + + +def _get_google_groups_settings_token(): + """Access token for the Groups Settings API only. + + Requested on its own token so a missing delegation for this scope does not + break the Directory sync that shares GOOGLE_SCOPES (Google rejects the + whole token if any one requested scope is not delegated). + """ + return _get_delegated_token(GOOGLE_GROUPS_SETTINGS_SCOPE, "_google_groups_settings_token") + + def _google_api(method, path, access_token, body=None): """Call the Google Admin Directory API. Returns parsed JSON or None.""" url = f"https://admin.googleapis.com/admin/directory/v1{path}" @@ -521,10 +547,107 @@ def _normalize_member(member): return {} +# Settings enforced on every team group so members can read the group's mail. +# Who-can-post and moderation are deliberately NOT enforced: outside senders +# keep reaching the group exactly as before. +TEAM_GROUP_SETTINGS = { + "whoCanViewGroup": "ALL_MEMBERS_CAN_VIEW", + "isArchived": "true", + "archiveOnly": "false", + "whoCanViewMembership": "ALL_IN_DOMAIN_CAN_VIEW", + "whoCanJoin": "INVITED_CAN_JOIN", + "allowWebPosting": "true", + "allowExternalMembers": "false", +} + +# Settings reported (never written) so the first run shows today's values. +TEAM_GROUP_REPORTED_SETTINGS = ("whoCanPostMessage", "messageModerationLevel") + +GOOGLE_MEMBER_ROLE_FOR = {"maintainer": "MANAGER", "member": "MEMBER"} + + +class GoogleGroupsSettingsApiDisabled(Exception): + """Raised when the Groups Settings API is not enabled in the GCP project.""" + + +def _groups_settings_api(method, email, access_token, body=None): + """Call the Google Groups Settings API. Returns parsed JSON, or None on 404.""" + url = ( + "https://www.googleapis.com/groups/v1/groups/" + f"{urllib.parse.quote(email, safe='')}?alt=json" + ) + data = json.dumps(body).encode("utf-8") if body is not None else None + req = urllib.request.Request(url, data=data, method=method) + req.add_header("Authorization", f"Bearer {access_token}") + req.add_header("Content-Type", "application/json") + try: + with urllib.request.urlopen(req) as resp: + raw = resp.read() + return json.loads(raw) if raw else {} + except urllib.error.HTTPError as e: + if e.code == 404: + return None + body_text = e.read().decode("utf-8", errors="replace") + logger.error("Groups Settings API %s %s -> %d: %s", method, email, e.code, body_text) + if e.code == 403 and ("accessNotConfigured" in body_text or "SERVICE_DISABLED" in body_text): + raise GoogleGroupsSettingsApiDisabled(body_text[:200]) from e + raise + + +def enforce_team_group_settings(group_email): + """Make sure team members can read the group's mail and archive. + + Reads the current settings and PATCHes only keys that differ, so a group + already in sync gets no write. Never raises: returns a dict describing the + outcome so member sync still succeeds when settings cannot be applied. + """ + try: + token = _get_google_groups_settings_token() + except GoogleScopeNotDelegated: + return {"skipped": True, "reason": "groups settings scope not delegated"} + + try: + current = None + # A freshly created group can 404 here for a few seconds. + for delay in (2, 4, 8, None): + current = _groups_settings_api("GET", group_email, token) + if current is not None or delay is None: + break + time.sleep(delay) + if current is None: + return {"error": f"groups settings for {group_email} not found"} + + reported = {k: current.get(k) for k in TEAM_GROUP_REPORTED_SETTINGS} + drift = { + k: v for k, v in TEAM_GROUP_SETTINGS.items() + if str(current.get(k, "")).lower() != v.lower() + } + if drift: + _groups_settings_api("PATCH", group_email, token, drift) + logger.info("Updated settings on %s: %s", group_email, sorted(drift)) + return {"changed": sorted(drift), "current": reported} + except GoogleGroupsSettingsApiDisabled: + return { + "error": "Groups Settings API (groupssettings.googleapis.com) is not " + "enabled in GCP project javabin-platform" + } + except Exception as e: # noqa: BLE001 — settings must not fail member sync + logger.exception("Failed to enforce settings on %s", group_email) + return {"error": str(e)[:200]} + + +def _google_role_for(member): + """Registry role -> Google group role. maintainer -> MANAGER, else MEMBER.""" + return GOOGLE_MEMBER_ROLE_FOR.get(str(member.get("role", "member")).lower(), "MEMBER") + + def sync_google_group(team): """Create or update a Google Workspace group and sync its membership. - Group email is derived as team-{name}@java.no. All members are MEMBER role. + Group email is derived as team-{name}@java.no. Registry `role: maintainer` + becomes Google MANAGER, everyone else MEMBER. Existing OWNERs are never + changed. Group settings are then enforced so members can read the mail + archive (see enforce_team_group_settings). """ team_name = team["name"] group_email = f"team-{team_name}@java.no" @@ -550,7 +673,7 @@ def sync_google_group(team): logger.info("Created Google group %s", group_email) # Current members (paginated — Google returns max 200 per page) - current_emails = set() + current_roles = {} # email -> role page_token = None while True: path = f"/groups/{group_key}/members?maxResults=200" @@ -559,7 +682,7 @@ def sync_google_group(team): resp = _google_api("GET", path, access_token) if resp and "members" in resp: for m in resp["members"]: - current_emails.add(m["email"].lower()) + current_roles[m["email"].lower()] = m.get("role", "MEMBER") if resp and resp.get("nextPageToken"): page_token = resp["nextPageToken"] else: @@ -567,24 +690,38 @@ def sync_google_group(team): # Desired members desired_emails = set() + added = 0 + role_changes = [] for member in team.get("members", []): m = _normalize_member(member) email = m.get("email", "").lower() if not email: continue desired_emails.add(email) + role = _google_role_for(m) - if email not in current_emails: + if email not in current_roles: _google_api( "POST", f"/groups/{group_key}/members", access_token, - {"email": email, "role": "MEMBER"}, + {"email": email, "role": role}, + ) + added += 1 + logger.info("Added %s to %s as %s", email, group_email, role) + elif current_roles[email] != role and current_roles[email] != "OWNER": + encoded_email = urllib.parse.quote(email, safe="") + _google_api( + "PATCH", + f"/groups/{group_key}/members/{encoded_email}", + access_token, + {"role": role}, ) - logger.info("Added %s to %s", email, group_email) + role_changes.append(f"{email} {current_roles[email]}->{role}") + logger.info("Changed %s in %s: %s -> %s", email, group_email, current_roles[email], role) # Remove members no longer in the team definition - for email in current_emails: + for email in current_roles: if email not in desired_emails: encoded_email = urllib.parse.quote(email, safe="") _google_api( @@ -598,6 +735,9 @@ def sync_google_group(team): "synced": True, "group": group_email, "member_count": len(desired_emails), + "added": added, + "role_changes": role_changes, + "settings": enforce_team_group_settings(group_email), } @@ -2134,8 +2274,18 @@ def _result_line(provider, outcome): details.append(f"{outcome['member_count']} total") if outcome.get("extra_preserved"): details.append(f"{outcome['extra_preserved']} manual") + if outcome.get("role_changes"): + details.append(f"{len(outcome['role_changes'])} role change(s)") + settings = outcome.get("settings") + if isinstance(settings, dict) and settings.get("changed"): + details.append(f"settings: {', '.join(settings['changed'])}") if details: parts.append(f"({', '.join(details)})") + if isinstance(settings, dict): + if settings.get("skipped"): + parts.append(f":warning: group settings skipped: {settings.get('reason')}") + elif settings.get("error"): + parts.append(f":warning: group settings: {settings['error'][:150]}") return " ".join(parts) diff --git a/terraform/lambda-src/tests/test_team_provisioner_google_group.py b/terraform/lambda-src/tests/test_team_provisioner_google_group.py new file mode 100644 index 0000000..4d0778e --- /dev/null +++ b/terraform/lambda-src/tests/test_team_provisioner_google_group.py @@ -0,0 +1,217 @@ +"""Tests for team_provisioner Google group role mapping and settings enforcement. + +Run: python3 -m unittest discover -s terraform/lambda-src/tests +""" + +import importlib.util +import io +import os +import sys +import types +import unittest +import urllib.error +from unittest import mock + +SRC = os.path.join(os.path.dirname(__file__), "..") + +IN_SYNC = { + "whoCanViewGroup": "ALL_MEMBERS_CAN_VIEW", + "isArchived": "true", + "archiveOnly": "false", + "whoCanViewMembership": "ALL_IN_DOMAIN_CAN_VIEW", + "whoCanJoin": "INVITED_CAN_JOIN", + "allowWebPosting": "true", + "allowExternalMembers": "false", + "whoCanPostMessage": "ANYONE_CAN_POST", + "messageModerationLevel": "MODERATE_NONE", +} + + +def load(): + os.environ.setdefault("MEMBERSHIPS_TABLE", "javabin-auth-memberships") + os.environ.setdefault("COGNITO_INTERNAL_POOL_ID", "pool") + boto3 = types.ModuleType("boto3") + boto3.client = lambda svc, **kw: mock.MagicMock() + sys.modules["boto3"] = boto3 + sys.path.insert(0, SRC) + sys.path.insert(0, os.path.join(SRC, "team_provisioner")) + spec = importlib.util.spec_from_file_location( + "team_provisioner_handler_gg", os.path.join(SRC, "team_provisioner", "handler.py")) + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + mod.time.sleep = lambda s: None + return mod + + +def http_error(code, body): + return urllib.error.HTTPError("https://x", code, "err", {}, io.BytesIO(body.encode())) + + +class FakeSettings: + """Stands in for _groups_settings_api; records writes.""" + + def __init__(self, state, not_found_first=0): + self.state = dict(state) if state is not None else None + self.not_found_first = not_found_first + self.writes = [] + + def __call__(self, method, email, token, body=None): + if method == "GET": + if self.not_found_first: + self.not_found_first -= 1 + return None + return dict(self.state) if self.state is not None else None + self.writes.append(body) + self.state.update(body) + return dict(self.state) + + +class FakeDirectory: + """Stands in for _google_api for one group; records calls.""" + + def __init__(self, members): + self.members = dict(members) # email -> role + self.calls = [] + + def __call__(self, method, path, token, body=None): + self.calls.append((method, path, body)) + if method == "GET" and "/members" in path: + return {"members": [{"email": e, "role": r} for e, r in self.members.items()]} + if method == "GET": + return {"email": "team-x@java.no"} + return {} + + def writes(self, method): + return [c for c in self.calls if c[0] == method and "/members" in c[1]] + + +class EnforceSettingsTest(unittest.TestCase): + def setUp(self): + self.h = load() + self.h._get_google_groups_settings_token = lambda: "tok" + + def test_new_group_gets_all_settings(self): + fake = FakeSettings({"whoCanPostMessage": "ANYONE_CAN_POST"}) + self.h._groups_settings_api = fake + result = self.h.enforce_team_group_settings("team-x@java.no") + self.assertEqual(fake.writes, [self.h.TEAM_GROUP_SETTINGS]) + self.assertEqual(result["changed"], sorted(self.h.TEAM_GROUP_SETTINGS)) + self.assertEqual(result["current"]["whoCanPostMessage"], "ANYONE_CAN_POST") + + def test_in_sync_group_gets_no_write(self): + fake = FakeSettings(IN_SYNC) + self.h._groups_settings_api = fake + result = self.h.enforce_team_group_settings("team-x@java.no") + self.assertEqual(fake.writes, []) + self.assertEqual(result["changed"], []) + + def test_partial_drift_writes_only_drifted_keys(self): + fake = FakeSettings({**IN_SYNC, "whoCanViewGroup": "ALL_MANAGERS_CAN_VIEW", "isArchived": "false"}) + self.h._groups_settings_api = fake + self.h.enforce_team_group_settings("team-x@java.no") + self.assertEqual(fake.writes, [{"whoCanViewGroup": "ALL_MEMBERS_CAN_VIEW", "isArchived": "true"}]) + + def test_post_and_moderation_settings_are_never_written(self): + fake = FakeSettings({"whoCanPostMessage": "ALL_MEMBERS_CAN_POST", "messageModerationLevel": "MODERATE_ALL_MESSAGES"}) + self.h._groups_settings_api = fake + self.h.enforce_team_group_settings("team-x@java.no") + for w in fake.writes: + self.assertNotIn("whoCanPostMessage", w) + self.assertNotIn("messageModerationLevel", w) + self.assertEqual(fake.state["whoCanPostMessage"], "ALL_MEMBERS_CAN_POST") + + def test_not_found_then_found_retries(self): + fake = FakeSettings(IN_SYNC, not_found_first=2) + self.h._groups_settings_api = fake + result = self.h.enforce_team_group_settings("team-x@java.no") + self.assertEqual(result["changed"], []) + + def test_missing_scope_skips(self): + def no_scope(): + raise self.h.GoogleScopeNotDelegated("scope") + self.h._get_google_groups_settings_token = no_scope + self.h._groups_settings_api = mock.Mock(side_effect=AssertionError("must not call")) + result = self.h.enforce_team_group_settings("team-x@java.no") + self.assertEqual(result, {"skipped": True, "reason": "groups settings scope not delegated"}) + + def test_api_disabled_gives_clear_error(self): + body = '{"error":{"code":403,"errors":[{"reason":"accessNotConfigured"}]}}' + with mock.patch.object(self.h.urllib.request, "urlopen", side_effect=http_error(403, body)): + result = self.h.enforce_team_group_settings("team-x@java.no") + self.assertIn("not enabled in GCP project javabin-platform", result["error"]) + + +class DelegatedTokenTest(unittest.TestCase): + def test_unauthorized_client_raises_scope_not_delegated(self): + h = load() + h._create_jwt = lambda payload, key: "jwt" + err = http_error(401, '{"error":"unauthorized_client","error_description":"Client is unauthorized"}') + with mock.patch.object(h, "_get_ssm_param", side_effect=lambda n: '{"client_email": "sa@x", "private_key": "k"}' if n == h.GOOGLE_SA_PARAM else "admin@java.no"), \ + mock.patch.object(h.urllib.request, "urlopen", side_effect=err): + with self.assertRaises(h.GoogleScopeNotDelegated): + h._get_google_groups_settings_token() + + +class SyncGoogleGroupRolesTest(unittest.TestCase): + def setUp(self): + self.h = load() + self.h._get_google_access_token = lambda: "tok" + self.h.enforce_team_group_settings = lambda email: {"changed": []} + + def sync(self, current, members): + fake = FakeDirectory(current) + self.h._google_api = fake + result = self.h.sync_google_group({"name": "x", "description": "d", "members": members}) + return fake, result + + def test_maintainer_added_as_manager(self): + fake, result = self.sync({}, [{"google": "ada", "github": "ada", "role": "maintainer"}, "bob"]) + posts = {c[2]["email"]: c[2]["role"] for c in fake.writes("POST")} + self.assertEqual(posts, {"ada@java.no": "MANAGER", "bob@java.no": "MEMBER"}) + self.assertEqual(result["added"], 2) + + def test_role_promotion_and_demotion(self): + fake, result = self.sync( + {"ada@java.no": "MEMBER", "bob@java.no": "MANAGER"}, + [{"google": "ada", "role": "maintainer"}, {"google": "bob", "role": "member"}], + ) + patches = {c[1].rsplit("/", 1)[1]: c[2]["role"] for c in fake.writes("PATCH")} + self.assertEqual(patches, {"ada%40java.no": "MANAGER", "bob%40java.no": "MEMBER"}) + self.assertEqual(len(result["role_changes"]), 2) + self.assertEqual(fake.writes("POST"), []) + + def test_matching_roles_cause_no_writes(self): + fake, _ = self.sync({"ada@java.no": "MANAGER"}, [{"google": "ada", "role": "maintainer"}]) + self.assertEqual(fake.writes("PATCH") + fake.writes("POST") + fake.writes("DELETE"), []) + + def test_owner_is_never_changed(self): + fake, _ = self.sync({"ada@java.no": "OWNER"}, [{"google": "ada", "role": "member"}]) + self.assertEqual(fake.writes("PATCH"), []) + + def test_members_not_in_registry_are_removed(self): + fake, _ = self.sync({"ada@java.no": "MEMBER", "gone@java.no": "MANAGER"}, ["ada"]) + self.assertEqual([c[1] for c in fake.writes("DELETE")], ["/groups/team-x%40java.no/members/gone%40java.no"]) + + def test_settings_result_is_included(self): + _, result = self.sync({}, ["ada"]) + self.assertEqual(result["settings"], {"changed": []}) + + +class ResultLineTest(unittest.TestCase): + def test_skipped_settings_shows_warning_but_still_synced(self): + h = load() + line = h._result_line("google", {"synced": True, "member_count": 2, "settings": { + "skipped": True, "reason": "groups settings scope not delegated"}}) + self.assertIn(":white_check_mark: google: synced", line) + self.assertIn(":warning: group settings skipped", line) + + def test_changes_are_listed(self): + h = load() + line = h._result_line("google", {"synced": True, "member_count": 2, "role_changes": ["a"], + "settings": {"changed": ["isArchived"]}}) + self.assertIn("1 role change(s)", line) + self.assertIn("settings: isArchived", line) + + +if __name__ == "__main__": + unittest.main() From f39e49ae53b069ca577013d9acd472ebe6f12142 Mon Sep 17 00:00:00 2001 From: Alexanderamiri Date: Sat, 24 Oct 2026 14:42:21 +0200 Subject: [PATCH 2/2] fix(team-provisioner): keep group owners, skip external members, quiet missing scope Address review on #153: never remove admin-set OWNERs, report non-java.no members instead of failing the team sync, cache a not-delegated scope for the invocation and log it as a warning, give the exception a readable message, assert the Groups Settings PATCH request shape, and document the Groups for Business prerequisite and the additive scope wording. Co-Authored-By: Claude Opus 5.5 (1M context) --- CLAUDE.md | 8 ++- docs/lambda-functions.md | 13 +++- .../lambda-src/team_provisioner/handler.py | 53 ++++++++++++--- .../test_team_provisioner_google_group.py | 66 +++++++++++++++++++ 4 files changed, 126 insertions(+), 14 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 27dd8f1..aee0e97 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -272,14 +272,16 @@ in Google Workspace Admin. The team-provisioner Lambda uses it to manage Google The SA JSON key is at `/javabin/platform/google-admin-sa`, the impersonation target at `/javabin/platform/google-admin-email`. -Delegated scopes (Admin console > Security > API controls > Domain-wide delegation; the field replaces -the whole list, so always keep the existing ones): `admin.directory.group`, +Delegated scopes the platform code requests (Admin console > Security > API controls > Domain-wide +delegation; the field replaces the whole list, so read what is there first and only add, since the +console may grant scopes not listed here): `admin.directory.group`, `admin.directory.group.member`, `admin.directory.user`, `admin.directory.user.alias`, `gmail.send`, `gmail.settings.sharing`, `apps.groups.settings` (prefix `https://www.googleapis.com/auth/`). Google rejects a token if any requested scope is not delegated, so `apps.groups.settings` is requested on a separate token (`_get_google_groups_settings_token`) and team group settings are skipped, not failed, when it is missing. The Groups Settings API (`groupssettings.googleapis.com`) -must be enabled in `javabin-platform`. +must be enabled in `javabin-platform`, and Groups for Business must be ON for members' org units. +Team group OWNERs are admin-managed and never removed by the sync; non-@java.no members are skipped. ## Work Packages diff --git a/docs/lambda-functions.md b/docs/lambda-functions.md index 1c29902..ad0304c 100644 --- a/docs/lambda-functions.md +++ b/docs/lambda-functions.md @@ -87,7 +87,11 @@ Queries Security Hub for active findings at HIGH and CRITICAL severity, aggregat Each team gets the group `team-{name}@java.no`. Members are kept in step with the registry: a member with `role: maintainer` is a Google **MANAGER**, everyone else a **MEMBER**. A differing role is -changed in place. Existing **OWNER**s are never changed. Members not in the registry are removed. +changed in place. Members not in the registry are removed, except **OWNER**s: an owner is only ever +set by a Workspace admin by hand (e.g. `platform@java.no`), so owners are never demoted or removed and +are reported as `kept_owners`. Team members must have a `@java.no` account: the group has +`allowExternalMembers=false`, so a non-java.no address (legacy `email:` field) is not added and is +reported as `skipped_external` with a Slack `:warning:`, while the rest of the sync continues. After the member sync, `enforce_team_group_settings` makes the group's mail readable by its members. It reads the Groups Settings API and writes only the keys that differ: @@ -110,7 +114,12 @@ delegated, the settings step is skipped with a Slack `:warning:` and member sync the Groups Settings API is disabled in GCP project `javabin-platform`, the step reports that. Org groups from the registry's `groups.yaml` are not touched by this. -Delegated scopes the service account needs (client ID 103162295681564397615): +Members read the archive at groups.google.com only if the **Groups for Business** service is ON for +their org unit (Admin console > Apps > Google Workspace > Groups for Business); without it the archive +settings have no visible effect. + +Delegated scopes the platform code requests (client ID 103162295681564397615). The console may grant +more; read the current list there first and add to it rather than pasting this one: `admin.directory.group`, `admin.directory.group.member`, `admin.directory.user`, `admin.directory.user.alias`, `gmail.send`, `gmail.settings.sharing`, `apps.groups.settings` (all under `https://www.googleapis.com/auth/`). diff --git a/terraform/lambda-src/team_provisioner/handler.py b/terraform/lambda-src/team_provisioner/handler.py index 77a73d6..b5a4e10 100644 --- a/terraform/lambda-src/team_provisioner/handler.py +++ b/terraform/lambda-src/team_provisioner/handler.py @@ -190,6 +190,15 @@ def _create_jwt(payload, private_key_pem): class GoogleScopeNotDelegated(Exception): """Raised when the delegated service account lacks a requested scope.""" + def __init__(self, scope): + super().__init__(f"scope not delegated to DWD client: {scope}") + self.scope = scope + + +# How long a "scope not delegated" answer is remembered, so one invocation does +# not retry (and log) the token exchange for every team. +SCOPE_NOT_DELEGATED_TTL = 300 + def _get_delegated_token(scope, cache_key): """Exchange a domain-wide delegation JWT for a Google OAuth2 access token. @@ -197,9 +206,13 @@ def _get_delegated_token(scope, cache_key): Token is cached for ~58 minutes (Google tokens last 1 hour). Raises GoogleScopeNotDelegated when Google answers `unauthorized_client`, which happens when any requested scope is missing from the delegation grant. + That outcome is cached for SCOPE_NOT_DELEGATED_TTL seconds. """ cached = _credential_cache.get(cache_key) - if cached and cached["exp"] > time.time() + 120: + if cached and cached.get("not_delegated"): + if cached["exp"] > time.time(): + raise GoogleScopeNotDelegated(scope) + elif cached and cached["exp"] > time.time() + 120: return cached["token"] sa_json = json.loads(_get_ssm_param(GOOGLE_SA_PARAM)) @@ -235,9 +248,14 @@ def _get_delegated_token(scope, cache_key): token = json.loads(resp.read())["access_token"] except urllib.error.HTTPError as e: body_text = e.read().decode("utf-8", errors="replace") - logger.error("Google OAuth token exchange failed: %d %s", e.code, body_text) if "unauthorized_client" in body_text: + logger.warning("Google scope not delegated (%s): %d %s", scope, e.code, body_text) + _credential_cache[cache_key] = { + "not_delegated": True, + "exp": time.time() + SCOPE_NOT_DELEGATED_TTL, + } raise GoogleScopeNotDelegated(scope) from e + logger.error("Google OAuth token exchange failed: %d %s", e.code, body_text) raise _credential_cache[cache_key] = { @@ -645,9 +663,12 @@ def sync_google_group(team): """Create or update a Google Workspace group and sync its membership. Group email is derived as team-{name}@java.no. Registry `role: maintainer` - becomes Google MANAGER, everyone else MEMBER. Existing OWNERs are never - changed. Group settings are then enforced so members can read the mail - archive (see enforce_team_group_settings). + becomes Google MANAGER, everyone else MEMBER. OWNERs (only ever added by + a Workspace admin by hand) are never demoted or removed, even when they are + not in the registry. Members outside java.no are not added (the group does + not allow external members); they are reported in `skipped_external`. + Group settings are then enforced so members can read the mail archive (see + enforce_team_group_settings). """ team_name = team["name"] group_email = f"team-{team_name}@java.no" @@ -692,6 +713,7 @@ def sync_google_group(team): desired_emails = set() added = 0 role_changes = [] + skipped_external = [] for member in team.get("members", []): m = _normalize_member(member) email = m.get("email", "").lower() @@ -700,7 +722,12 @@ def sync_google_group(team): desired_emails.add(email) role = _google_role_for(m) - if email not in current_roles: + if email not in current_roles and not email.endswith("@java.no"): + # allowExternalMembers=false: Google would reject the insert and + # abort the rest of this team's sync. Report instead. + skipped_external.append(email) + logger.warning("Not adding external address %s to %s", email, group_email) + elif email not in current_roles: _google_api( "POST", f"/groups/{group_key}/members", @@ -720,9 +747,13 @@ def sync_google_group(team): role_changes.append(f"{email} {current_roles[email]}->{role}") logger.info("Changed %s in %s: %s -> %s", email, group_email, current_roles[email], role) - # Remove members no longer in the team definition - for email in current_roles: - if email not in desired_emails: + # Remove members no longer in the team definition. OWNERs are kept: they + # are set by a Workspace admin, not by the registry. + kept_owners = [] + for email, current_role in current_roles.items(): + if email not in desired_emails and current_role == "OWNER": + kept_owners.append(email) + elif email not in desired_emails: encoded_email = urllib.parse.quote(email, safe="") _google_api( "DELETE", @@ -737,6 +768,8 @@ def sync_google_group(team): "member_count": len(desired_emails), "added": added, "role_changes": role_changes, + "skipped_external": skipped_external, + "kept_owners": kept_owners, "settings": enforce_team_group_settings(group_email), } @@ -2286,6 +2319,8 @@ def _result_line(provider, outcome): parts.append(f":warning: group settings skipped: {settings.get('reason')}") elif settings.get("error"): parts.append(f":warning: group settings: {settings['error'][:150]}") + if outcome.get("skipped_external"): + parts.append(f":warning: not added (not @java.no): {', '.join(outcome['skipped_external'])}") return " ".join(parts) diff --git a/terraform/lambda-src/tests/test_team_provisioner_google_group.py b/terraform/lambda-src/tests/test_team_provisioner_google_group.py index 4d0778e..e1588a4 100644 --- a/terraform/lambda-src/tests/test_team_provisioner_google_group.py +++ b/terraform/lambda-src/tests/test_team_provisioner_google_group.py @@ -5,6 +5,7 @@ import importlib.util import io +import json import os import sys import types @@ -152,6 +153,48 @@ def test_unauthorized_client_raises_scope_not_delegated(self): h._get_google_groups_settings_token() + def test_not_delegated_is_cached_for_the_invocation(self): + h = load() + h._create_jwt = lambda payload, key: "jwt" + calls = [] + + def fail(req): + calls.append(req) + raise http_error(401, '{"error":"unauthorized_client"}') + with mock.patch.object(h, "_get_ssm_param", side_effect=lambda n: '{"client_email": "sa@x", "private_key": "k"}' if n == h.GOOGLE_SA_PARAM else "admin@java.no"), \ + mock.patch.object(h.urllib.request, "urlopen", side_effect=fail): + for _ in range(3): + with self.assertRaises(h.GoogleScopeNotDelegated) as ctx: + h._get_google_groups_settings_token() + self.assertEqual(len(calls), 1) + self.assertIn("scope not delegated to DWD client: https://www.googleapis.com/auth/apps.groups.settings", + str(ctx.exception)) + + +class GroupsSettingsApiRequestTest(unittest.TestCase): + def test_patch_request_shape(self): + h = load() + captured = [] + + class Resp(io.BytesIO): + def __enter__(self): + return self + + def __exit__(self, *a): + return False + + def fake_urlopen(req): + captured.append(req) + return Resp(b'{"isArchived": "true"}') + with mock.patch.object(h.urllib.request, "urlopen", side_effect=fake_urlopen): + h._groups_settings_api("PATCH", "team-x@java.no", "tok", {"isArchived": "true"}) + req = captured[0] + self.assertEqual(req.get_method(), "PATCH") + self.assertEqual(req.full_url, "https://www.googleapis.com/groups/v1/groups/team-x%40java.no?alt=json") + self.assertEqual(json.loads(req.data), {"isArchived": "true"}) + self.assertEqual(req.get_header("Authorization"), "Bearer tok") + + class SyncGoogleGroupRolesTest(unittest.TestCase): def setUp(self): self.h = load() @@ -192,6 +235,23 @@ def test_members_not_in_registry_are_removed(self): fake, _ = self.sync({"ada@java.no": "MEMBER", "gone@java.no": "MANAGER"}, ["ada"]) self.assertEqual([c[1] for c in fake.writes("DELETE")], ["/groups/team-x%40java.no/members/gone%40java.no"]) + def test_owner_not_in_registry_is_kept(self): + fake, result = self.sync({"ada@java.no": "MEMBER", "platform@java.no": "OWNER"}, ["ada"]) + self.assertEqual(fake.writes("DELETE"), []) + self.assertEqual(result["kept_owners"], ["platform@java.no"]) + + def test_external_member_is_skipped_not_fatal(self): + fake, result = self.sync({}, [{"email": "anders@example.com"}, "ada"]) + posts = [c[2]["email"] for c in fake.writes("POST")] + self.assertEqual(posts, ["ada@java.no"]) + self.assertEqual(result["skipped_external"], ["anders@example.com"]) + self.assertEqual(result["settings"], {"changed": []}) + + def test_existing_external_member_is_not_removed(self): + fake, result = self.sync({"anders@example.com": "MEMBER"}, [{"email": "anders@example.com"}]) + self.assertEqual(fake.writes("DELETE") + fake.writes("POST"), []) + self.assertEqual(result["skipped_external"], []) + def test_settings_result_is_included(self): _, result = self.sync({}, ["ada"]) self.assertEqual(result["settings"], {"changed": []}) @@ -205,6 +265,12 @@ def test_skipped_settings_shows_warning_but_still_synced(self): self.assertIn(":white_check_mark: google: synced", line) self.assertIn(":warning: group settings skipped", line) + def test_skipped_external_is_flagged(self): + h = load() + line = h._result_line("google", {"synced": True, "member_count": 1, + "skipped_external": ["x@example.com"]}) + self.assertIn(":warning: not added (not @java.no): x@example.com", line) + def test_changes_are_listed(self): h = load() line = h._result_line("google", {"synced": True, "member_count": 2, "role_changes": ["a"],