Add Google Group Management app-group-lifecycle plugin#498
Conversation
a3ed064 to
8c35ed5
Compare
825499e to
e9e3d9e
Compare
e9e3d9e to
fbe4662
Compare
|
bugbot run |
|
Skipping Bugbot: Bugbot is disabled for this repository. Visit the Bugbot dashboard to update your settings. |
Add create/delete/list helpers for Okta group push mappings on OktaService, the link primitive the app group lifecycle plugins need to bind an Access-managed Okta group to an external group. delete PATCHes the mapping INACTIVE before DELETE (Okta rejects deleting active mappings) and defaults deleteTargetGroup to False so unlinking never removes the external group. Timeouts propagate like the other data-returning OktaService methods. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…elpers The Google plugin needs an app's configuration when describing and validating a group's config (e.g. to surface an app-level email pattern), and an operation needs to tell whether a config edit actually changed. So: - thread the owning app's plugin config into the group-config-properties and group-config validation hooks (and their wrappers). The hookspec args are additive and pluggy passes only the args an impl declares, so existing implementations keep working unchanged. - add is_plugin_config_changed(old, new, plugin_id), comparing only a plugin's configuration (not status, which the plugin writes itself). - add set_config_value alongside set_status_value so plugins backfill inferred config without touching the raw "configuration" key directly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The hook reconciles all of an app's groups, not just memberships, so the name was misleading. Rename it to sync_all_groups and rename the CLI command to `sync-app-groups`. Both renames are backward compatible: the old sync_all_group_membership hookspec stays as a deprecated alias (so plugins on the old name still register and run), the old `sync-app-group-memberships` command remains as a hidden alias, and a new invoke_sync_all_groups wrapper dispatches to both hook names exactly once. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…gh it Group plugin_data edits previously merged the patch inline in put_group, and nothing fired the app group lifecycle group_updated hook when a group's plugin configuration changed outside of a rename/description edit. Introduce ModifyGroupPluginData to own that business logic: it merges the partial patch (preserving other plugins' entries and plugin-managed status the patch omits) and fires group_updated only when the configured plugin's configuration actually changes -- never on status-only writes, which the plugin makes itself and which would otherwise re-trigger reconciliation in a loop. put_group now delegates to it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
post_group accepted any plugin_data without checking it against the configured plugin's schema, so an invalid group config was only caught later during reconciliation. Validate it up front and return 400, mirroring put_group. Extract the shared check into _validate_group_plugin_config so create and update validate identically. The helper takes the resolved plugin id and the owning app's plugin_data from the caller, because a freshly-built group can't lazy-load its app (lazy="raise_on_sql"); post_group loads the App directly to get them. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Some plugin group config can't change after creation -- e.g. a Google group's email, which the external API treats as an immutable key. Add an `immutable` flag to the group config property schema (and its wire mirror plus generated client type) and enforce it host-side: on update, validation rejects any change to a field the plugin declares immutable, while create still sets it freely. put_group passes the existing group's plugin_data so the validator can compare. Enforcement is generic, so a plugin only declares immutable=True. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The group config schema can depend on the owning app's plugin config (for example, an app-level pattern surfaced as a client-side validation rule), but the group-config-props endpoint had no app context. Accept an optional app_id query param, load that app's plugin_data, and thread it into the schema lookup; regenerate the client so the param is available to the UI. Without app_id the behavior is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Surface plugin group config validation in the configuration form so the UI catches bad input before submitting (the backend remains authoritative): each text field applies the schema's validation.patterns as react-hook-form rules. Immutable fields stay editable at create time and become read-only (not disabled, so their value is still submitted) when editing an existing app or group, with a helper-text note explaining why. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Logging was hard-coded to INFO, so diagnosing issues in a deployed environment (e.g. tracing app group lifecycle plugin reconciliation) meant editing code. Read the level from a LOG_LEVEL environment variable, defaulting to INFO. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Running the app group lifecycle sync locally needs the plugin credentials (Google/Okta) from .env, but `access` has no --env-file flag and the existing `sync`/`notify` targets don't load it. Add a sync-app-groups target that sources .env (set -a auto-exports it) while still forcing DATABASE_URI to the local instance, like the other targets, so it hits the same migrated database the dev server uses. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add the package skeleton for an example app group lifecycle plugin that manages Google groups for Access groups: __init__, a setup.py declaring the access_app_group_lifecycle entry point and Google client dependencies, and a README documenting configuration, the Cloud Identity Groups API auth model (a Group Administrator service account, no domain-wide delegation), the GOOGLE_WORKSPACE_CUSTOMER_ID group parent, and that the group email is immutable after creation. Also broaden tox's pytest invocation to collect the whole repo so this plugin's standalone suite runs in CI alongside core tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Introduce the GoogleGroupManagerPlugin class with its app/group config and status schemas, config validation, and the email helpers that compose and parse the group's address. Group config validation enforces the Google-safe email charset and, when the app sets one, its email_pattern; both are also surfaced as client-side validation patterns. The email prefix is declared immutable, since the Cloud Identity groupKey can't change after creation. Google API and Okta wiring follow in later changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add the create/get/patch/delete and lookup-by-email helpers over the Cloud Identity Groups API (cloudidentity.googleapis.com). Groups are addressed by resource name; create sets the customer parent and the discussion-forum label; patch only ever touches the mutable displayName/description (groupKey is immutable); lookup resolves an email to the bare group id. These back the reconcile logic added later. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add the Okta side of linking: create a group push mapping between an Access-managed Okta group and the Google group, find the Okta-imported target group by its googleGroupEmail profile attribute, and discover an existing link (mapping id + Google email) for a group so out-of-band links can be adopted during reconciliation. Cloud Identity group ids differ from the Directory ids Okta stores, so the join is by email, which the caller resolves to an id. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reconcile resolves a group's Cloud Identity id (cached id, re-looked-up by email when missing or 404, or adopted from an existing Okta link), creates the Google group when none exists and config is present, and otherwise adopts or enforces its properties: missing Access config is backfilled from the live group, present config is pushed to Google. The email/groupKey is immutable so it's never patched, which also grandfathers groups whose prefix predates a later email_pattern. Blocking Google/Okta errors are recorded as an error sync status (committed inside the hook so it survives the host's post-error rollback), and the push mapping is (re)created, deferring to pending until Okta has imported the group. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Implement the app group lifecycle hooks on top of reconcile: group_created and group_updated reconcile the group; group_deleted unlinks the Okta push mapping (without deleting the target) and deletes the Google group, resolving the id by email when no status id is stored; sync_all_groups reconciles every group for an app, isolating per-group failures so one bad group doesn't abort the sweep. Register the plugin singleton for entry-point discovery. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Run ruff format over the new google_group_manager plugin (which was hand-wrapped and never formatted) and the few app_group_lifecycle framework/test lines these changes rewrapped, so `tox -e ruff` (ruff format --check) passes. Formatting only; no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Annotate the fixtures, the _group helper, and every test function in the google_group_manager plugin's test module so `mypy` passes. The helper's return annotation also clears the cascading no-untyped-call errors at its ~40 call sites. Annotations only; no behavior change (58 tests still pass). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copy examples/plugins/app_group_lifecycle_google into the image at /app/plugins without installing it here, so downstream images (e.g. Discord's overlay) can opt in by pip installing it. This keeps the plugin source single-homed in this repo while leaving activation to the consumer. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
fbe4662 to
ea6855b
Compare
| for name, prop in properties.items(): | ||
| if not getattr(prop, "immutable", False): | ||
| continue | ||
| if old_configuration.get(name) != configuration.get(name): |
There was a problem hiding this comment.
This compares the raw incoming patch against the old config, but the patch can be partial (that's the point of ModifyGroupPluginData's merge). A client that PUTs {configuration: {display_name: ...}} and omits the immutable email trips this: email goes from set to None, reads as a change, and 400s even though the user never touched it. The UI hides it by resubmitting the read-only field, but partial API updates break.
Fix: only treat a field as edited when it's actually present in the patch. An omission isn't a change; an explicit value (even null) still is. The grandfathering else branch is unaffected, and a present-and-different value is still rejected.
| if old_configuration.get(name) != configuration.get(name): | |
| if name in configuration and old_configuration.get(name) != configuration.get(name): |
| # Description sync (both directions): clobber if Access has one, else backfill. | ||
| access_desc = group.description or "" | ||
| google_desc = live.get("description", "") or "" | ||
| if not access_desc and google_desc: |
There was a problem hiding this comment.
I think this makes it impossible to clear a description. If a user empties the Access description and Google still has one, access_desc is "" so the enforce branch won't push the clear, and then this backfills Google's value right back (and pushes it to Okta)
| if type(group) is AppGroup: | ||
| merge_app_lifecycle_plugin_data(group, old_plugin_data) | ||
| db.commit() | ||
| ModifyGroupPluginData(group=group, plugin_data=new_plugin_data).execute() |
There was a problem hiding this comment.
ModifyGroupDetails fires group_updated on a name/description change, and this fires it again on a config change. A PUT that changes both name and plugin config in one request runs a full reconcile twice (Google + Okta round-trips)
Proposed fix: fire group_updated exactly once in put_group after both operations apply, and suppress the per-operation fires.
ModifyGroupDetails— add an opt-out (default preserves behavior for its other callers):
def __init__(self, *, group, name=None, description=None, current_user_id=None,
validate_app_group_prefix=True, fire_lifecycle_hook=True):
...
self.fire_lifecycle_hook = fire_lifecycle_hook
# in execute():
if self.fire_lifecycle_hook and (old_name != self.group.name or old_description != self.group.description):
... # existing group_updated fire-
ModifyGroupPluginData— samefire_lifecycle_hook=Trueopt-out, soput_groupcan suppress its internal fire (the merge still runs; only the hook call is gated). -
put_group— capture old values beforeModifyGroupDetails, suppress both, fire once at the end:
old_name = group.name
old_description = group.description or ""
plugin_id = get_app_group_lifecycle_plugin_to_invoke(group)
...
ModifyGroupDetails(..., fire_lifecycle_hook=False).execute()
...
old_plugin_data_for_audit = copy.deepcopy(group.plugin_data) if group.plugin_data else {}
if new_plugin_data is not None:
ModifyGroupPluginData(group=group, plugin_data=new_plugin_data, fire_lifecycle_hook=False).execute()
else:
db.commit()
name_or_desc_changed = group.name != old_name or (group.description or "") != old_description
config_changed = (
new_plugin_data is not None
and plugin_id is not None
and is_plugin_config_changed(old_plugin_data_for_audit, group.plugin_data or {}, plugin_id)
)
if plugin_id is not None and (name_or_desc_changed or config_changed):
try:
get_app_group_lifecycle_hook().group_updated(
session=db, group=group, old_name=old_name,
old_description=old_description, plugin_id=plugin_id,
)
db.commit()
except Exception:
logging.getLogger("api").exception(
f"Failed to invoke group_updated hook for group {group.id} with plugin '{plugin_id}'"
)
db.rollback()| hook = get_app_group_lifecycle_hook() | ||
| hook.sync_all_groups(session=session, app=app, plugin_id=plugin_id) | ||
| hook.sync_all_group_membership(session=session, app=app, plugin_id=plugin_id) |
There was a problem hiding this comment.
This dispatches both the new and deprecated hook names. The docstring says implement only one, but nothing enforces it, so a plugin implementing both (e.g. mid-migration) gets every group reconciled twice. Suggestion below guards against it by raising if any plugin implements both:
| hook = get_app_group_lifecycle_hook() | |
| hook.sync_all_groups(session=session, app=app, plugin_id=plugin_id) | |
| hook.sync_all_group_membership(session=session, app=app, plugin_id=plugin_id) | |
| hook = get_app_group_lifecycle_hook() | |
| current_impls = {impl.plugin for impl in hook.sync_all_groups.get_hookimpls()} | |
| deprecated_impls = {impl.plugin for impl in hook.sync_all_group_membership.get_hookimpls()} | |
| both = current_impls & deprecated_impls | |
| if both: | |
| names = ", ".join(getattr(p, "__name__", repr(p)) for p in both) | |
| raise ValueError( | |
| f"Plugin(s) {names} implement both sync_all_groups and the deprecated " | |
| "sync_all_group_membership; implement exactly one." | |
| ) | |
| hook.sync_all_groups(session=session, app=app, plugin_id=plugin_id) | |
| hook.sync_all_group_membership(session=session, app=app, plugin_id=plugin_id) |
Summary
Adds a
google_group_managerapp-group-lifecycle plugin that automatically creates, updates, and deletes Google groups for Access groups and links them to the Access-managed Okta group via Okta group push, plus the core-Access enablers it needs. Group CRUD uses the Cloud Identity Groups API.Urgency: 1 week?
Expected review effort: HIGH (~1h), since there's quite a lot of code but it's well-organized and -tested, if I do say so myself :)
Motivation
Access apps need Google groups that mirror their Access groups: when an Access group is created, reconfigured, or deleted, the matching Google group should be created/updated/deleted and linked via Okta group push, which then keeps membership in sync. This plugin delivers that as an opt-in example plugin which can replace manual toil to keep the two group management systems in sync.
Plugin Capabilities
_reconciledrives create/update and a periodicsync_all_groupssweep; it is safe to re-run and converges to the desired state.^gcp-) is enforced on create/update and surfaced as both client-side and server-side validation.sync_status(synced / pending / error),sync_error,last_synced_at,google_group_id, andpush_mapping_id.Limitations & behavior notes
groupKey) cannot change after creation, so theemailconfig field is enforced immutable on update (rendered read-only in the UI when editing). Display name and description stay mutable.pendingand linked on a later reconcile/sweep.googleGroupEmailprofile attribute on the imported target app group (Cloud Identity IDs differ from the Directory IDs Okta stores--two different Google APIs for group management).sync_status=error(committed so they survive the host's post-error rollback) rather than aborting the operation; a legacy email that predates a later-added pattern is grandfathered (never re-enforced, since the email can't change), and editing a group's other properties is not blocked by an unchanged, non-conforming legacy email./app/plugins) but does not install it; a downstream/overlay image opts in by pip-installing it. The README documents this, the Cloud Identity auth model, and the requiredGOOGLE_WORKSPACE_*env vars.Description of Changes
Core Access enablers (
api/,src/)OktaServicegainscreate/delete/list_group_push_mappings(Okta Group Push Mapping API).set_config_value/is_plugin_config_changedframework helpers. The bulk-sync hooksync_all_group_membershipis renamed tosync_all_groups(with a backward-compatible deprecated alias, and renamed CLI command / make target).ModifyGroupPluginDataoperation: merges a partialplugin_datapatch (preserving other plugins' entries and plugin-managed status) and firesgroup_updatedonly when the plugin'sconfigurationchanges — never on status writes, which would otherwise re-trigger reconciliation in a loop.put_grouproutes through it;post_groupvalidates group config at creation.group-config-propsendpoint accepts?app_id=so the schema can reflect app-level constraints; the config form validates patterns client-side./app/plugins) without installing it, so downstream images can opt in while the source stays single-homed in this repo.LOG_LEVEL; async-app-groupsmake target that loads.env.Plugin (
examples/plugins/app_group_lifecycle_google/)group_created/group_updated/group_deleted) and thesync_all_groupssweep.Web UI Screenshots
App Config


Group Config & Status






Creating a group (with validation):
Pending Okta re-pulling groups from Google:
Success:
With sync errors:
Notice grandfathering of bad-pattern email in backfill:
Failure to takeover an existing group:
Validation of Changes
ruff checkandruff format --checkare clean; the frontendtsctypecheck and the plugin config form's vitest pass.ModifyGroupPluginData(config-change vs status-only, and viaput_group); create/update validation including immutable-field rejection; and full plugin behavior (schema/validation, Cloud Identity wrappers, push-mapping/discovery, reconcile create/adopt/enforce/grandfather/pending/error branches, and the lifecycle hooks including delete ordering and sweep isolation).Guidance for Reviewers
History is clean and deliberately logically grouped — review commit-by-commit. It is two halves: the core-Access enablers (
api/,src/) precede the plugin (examples/); the tail is two formatting-only commits and a final Dockerfile commit that ships (but doesn't install) the plugin source.I've reviewed and exercised the code both manually and with the Claude code-review skill. Consider overall functionality--does the behavior match what you'd expect/want this plugin to do?
🤖 Generated with Claude Code