diff --git a/apps/cloud/drizzle/0008_slow_mephisto.sql b/apps/cloud/drizzle/0008_slow_mephisto.sql new file mode 100644 index 000000000..b1ccf708c --- /dev/null +++ b/apps/cloud/drizzle/0008_slow_mephisto.sql @@ -0,0 +1 @@ +ALTER TABLE "oauth_client" ADD COLUMN "origin_issuer" text; \ No newline at end of file diff --git a/apps/cloud/drizzle/meta/0008_snapshot.json b/apps/cloud/drizzle/meta/0008_snapshot.json new file mode 100644 index 000000000..4f506eac3 --- /dev/null +++ b/apps/cloud/drizzle/meta/0008_snapshot.json @@ -0,0 +1,1278 @@ +{ + "id": "a0208f9c-622b-4c94-bd09-33bcf37c2462", + "prevId": "2f557ed1-5afc-431b-8623-2a2162793f43", + "version": "7", + "dialect": "postgresql", + "tables": { + "public.accounts": { + "name": "accounts", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": {}, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.memberships": { + "name": "memberships", + "schema": "", + "columns": { + "account_id": { + "name": "account_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "organization_id": { + "name": "organization_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": {}, + "foreignKeys": { + "memberships_account_id_accounts_id_fk": { + "name": "memberships_account_id_accounts_id_fk", + "tableFrom": "memberships", + "tableTo": "accounts", + "columnsFrom": ["account_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "memberships_organization_id_organizations_id_fk": { + "name": "memberships_organization_id_organizations_id_fk", + "tableFrom": "memberships", + "tableTo": "organizations", + "columnsFrom": ["organization_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": { + "memberships_account_id_organization_id_pk": { + "name": "memberships_account_id_organization_id_pk", + "columns": ["account_id", "organization_id"] + } + }, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.organizations": { + "name": "organizations", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "slug": { + "name": "slug", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": { + "organizations_slug_unique": { + "name": "organizations_slug_unique", + "columns": [ + { + "expression": "slug", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": true, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.blob": { + "name": "blob", + "schema": "", + "columns": { + "namespace": { + "name": "namespace", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true + }, + "key": { + "name": "key", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true + }, + "value": { + "name": "value", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "row_id": { + "name": "row_id", + "type": "varchar(255)", + "primaryKey": true, + "notNull": true + }, + "id": { + "name": "id", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true + } + }, + "indexes": { + "blob_id_uidx": { + "name": "blob_id_uidx", + "columns": [ + { + "expression": "id", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": true, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.connection": { + "name": "connection", + "schema": "", + "columns": { + "integration": { + "name": "integration", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true + }, + "name": { + "name": "name", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true + }, + "template": { + "name": "template", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "provider": { + "name": "provider", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "item_ids": { + "name": "item_ids", + "type": "json", + "primaryKey": false, + "notNull": true + }, + "identity_label": { + "name": "identity_label", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "description": { + "name": "description", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "tools_synced_at": { + "name": "tools_synced_at", + "type": "bigint", + "primaryKey": false, + "notNull": false + }, + "oauth_client": { + "name": "oauth_client", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "oauth_client_owner": { + "name": "oauth_client_owner", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "refresh_item_id": { + "name": "refresh_item_id", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "expires_at": { + "name": "expires_at", + "type": "bigint", + "primaryKey": false, + "notNull": false + }, + "oauth_scope": { + "name": "oauth_scope", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "oauth_token_url": { + "name": "oauth_token_url", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "provider_state": { + "name": "provider_state", + "type": "json", + "primaryKey": false, + "notNull": false + }, + "created_at": { + "name": "created_at", + "type": "timestamp", + "primaryKey": false, + "notNull": true + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp", + "primaryKey": false, + "notNull": true + }, + "row_id": { + "name": "row_id", + "type": "varchar(255)", + "primaryKey": true, + "notNull": true + }, + "tenant": { + "name": "tenant", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true + }, + "owner": { + "name": "owner", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true + }, + "subject": { + "name": "subject", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true + } + }, + "indexes": { + "connection_uidx": { + "name": "connection_uidx", + "columns": [ + { + "expression": "tenant", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "owner", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "subject", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "integration", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "name", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": true, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.definition": { + "name": "definition", + "schema": "", + "columns": { + "integration": { + "name": "integration", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true + }, + "connection": { + "name": "connection", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true + }, + "plugin_id": { + "name": "plugin_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "schema": { + "name": "schema", + "type": "json", + "primaryKey": false, + "notNull": true + }, + "created_at": { + "name": "created_at", + "type": "timestamp", + "primaryKey": false, + "notNull": true + }, + "row_id": { + "name": "row_id", + "type": "varchar(255)", + "primaryKey": true, + "notNull": true + }, + "tenant": { + "name": "tenant", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true + }, + "owner": { + "name": "owner", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true + }, + "subject": { + "name": "subject", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true + } + }, + "indexes": { + "definition_uidx": { + "name": "definition_uidx", + "columns": [ + { + "expression": "tenant", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "owner", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "subject", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "integration", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "connection", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "name", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": true, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.integration": { + "name": "integration", + "schema": "", + "columns": { + "slug": { + "name": "slug", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true + }, + "plugin_id": { + "name": "plugin_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "description": { + "name": "description", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "config_revised_at": { + "name": "config_revised_at", + "type": "bigint", + "primaryKey": false, + "notNull": false + }, + "config": { + "name": "config", + "type": "json", + "primaryKey": false, + "notNull": false + }, + "can_remove": { + "name": "can_remove", + "type": "boolean", + "primaryKey": false, + "notNull": true, + "default": true + }, + "can_refresh": { + "name": "can_refresh", + "type": "boolean", + "primaryKey": false, + "notNull": true, + "default": false + }, + "created_at": { + "name": "created_at", + "type": "timestamp", + "primaryKey": false, + "notNull": true + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp", + "primaryKey": false, + "notNull": true + }, + "row_id": { + "name": "row_id", + "type": "varchar(255)", + "primaryKey": true, + "notNull": true + }, + "tenant": { + "name": "tenant", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true + } + }, + "indexes": { + "integration_uidx": { + "name": "integration_uidx", + "columns": [ + { + "expression": "tenant", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "slug", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": true, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.oauth_client": { + "name": "oauth_client", + "schema": "", + "columns": { + "slug": { + "name": "slug", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true + }, + "authorization_url": { + "name": "authorization_url", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "token_url": { + "name": "token_url", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "grant": { + "name": "grant", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "client_id": { + "name": "client_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "client_secret_item_id": { + "name": "client_secret_item_id", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "resource": { + "name": "resource", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "origin_kind": { + "name": "origin_kind", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "origin_integration": { + "name": "origin_integration", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "origin_issuer": { + "name": "origin_issuer", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "created_at": { + "name": "created_at", + "type": "timestamp", + "primaryKey": false, + "notNull": true + }, + "row_id": { + "name": "row_id", + "type": "varchar(255)", + "primaryKey": true, + "notNull": true + }, + "tenant": { + "name": "tenant", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true + }, + "owner": { + "name": "owner", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true + }, + "subject": { + "name": "subject", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true + } + }, + "indexes": { + "oauth_client_uidx": { + "name": "oauth_client_uidx", + "columns": [ + { + "expression": "tenant", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "owner", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "subject", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "slug", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": true, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.oauth_session": { + "name": "oauth_session", + "schema": "", + "columns": { + "state": { + "name": "state", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true + }, + "client_slug": { + "name": "client_slug", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "integration": { + "name": "integration", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "template": { + "name": "template", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "redirect_url": { + "name": "redirect_url", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "pkce_verifier": { + "name": "pkce_verifier", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "identity_label": { + "name": "identity_label", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "payload": { + "name": "payload", + "type": "json", + "primaryKey": false, + "notNull": true + }, + "expires_at": { + "name": "expires_at", + "type": "bigint", + "primaryKey": false, + "notNull": true + }, + "created_at": { + "name": "created_at", + "type": "timestamp", + "primaryKey": false, + "notNull": true + }, + "row_id": { + "name": "row_id", + "type": "varchar(255)", + "primaryKey": true, + "notNull": true + }, + "tenant": { + "name": "tenant", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true + }, + "owner": { + "name": "owner", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true + }, + "subject": { + "name": "subject", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true + } + }, + "indexes": { + "oauth_session_uidx": { + "name": "oauth_session_uidx", + "columns": [ + { + "expression": "tenant", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "state", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": true, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.plugin_storage": { + "name": "plugin_storage", + "schema": "", + "columns": { + "plugin_id": { + "name": "plugin_id", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true + }, + "collection": { + "name": "collection", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true + }, + "key": { + "name": "key", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true + }, + "data": { + "name": "data", + "type": "json", + "primaryKey": false, + "notNull": true + }, + "created_at": { + "name": "created_at", + "type": "timestamp", + "primaryKey": false, + "notNull": true + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp", + "primaryKey": false, + "notNull": true + }, + "row_id": { + "name": "row_id", + "type": "varchar(255)", + "primaryKey": true, + "notNull": true + }, + "tenant": { + "name": "tenant", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true + }, + "owner": { + "name": "owner", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true + }, + "subject": { + "name": "subject", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true + } + }, + "indexes": { + "plugin_storage_uidx": { + "name": "plugin_storage_uidx", + "columns": [ + { + "expression": "tenant", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "owner", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "subject", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "plugin_id", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "collection", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "key", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": true, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.private_executor_cloud_settings": { + "name": "private_executor_cloud_settings", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "varchar(255)", + "primaryKey": true, + "notNull": true + }, + "version": { + "name": "version", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true, + "default": "'1.0.0'" + } + }, + "indexes": {}, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.tool": { + "name": "tool", + "schema": "", + "columns": { + "integration": { + "name": "integration", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true + }, + "connection": { + "name": "connection", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true + }, + "plugin_id": { + "name": "plugin_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "name": { + "name": "name", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true + }, + "description": { + "name": "description", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "input_schema": { + "name": "input_schema", + "type": "json", + "primaryKey": false, + "notNull": false + }, + "output_schema": { + "name": "output_schema", + "type": "json", + "primaryKey": false, + "notNull": false + }, + "annotations": { + "name": "annotations", + "type": "json", + "primaryKey": false, + "notNull": false + }, + "created_at": { + "name": "created_at", + "type": "timestamp", + "primaryKey": false, + "notNull": true + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp", + "primaryKey": false, + "notNull": true + }, + "row_id": { + "name": "row_id", + "type": "varchar(255)", + "primaryKey": true, + "notNull": true + }, + "tenant": { + "name": "tenant", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true + }, + "owner": { + "name": "owner", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true + }, + "subject": { + "name": "subject", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true + } + }, + "indexes": { + "tool_uidx": { + "name": "tool_uidx", + "columns": [ + { + "expression": "tenant", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "owner", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "subject", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "integration", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "connection", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "name", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": true, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.tool_policy": { + "name": "tool_policy", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true + }, + "pattern": { + "name": "pattern", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "action": { + "name": "action", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "position": { + "name": "position", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "created_at": { + "name": "created_at", + "type": "timestamp", + "primaryKey": false, + "notNull": true + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp", + "primaryKey": false, + "notNull": true + }, + "row_id": { + "name": "row_id", + "type": "varchar(255)", + "primaryKey": true, + "notNull": true + }, + "tenant": { + "name": "tenant", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true + }, + "owner": { + "name": "owner", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true + }, + "subject": { + "name": "subject", + "type": "varchar(255)", + "primaryKey": false, + "notNull": true + } + }, + "indexes": { + "tool_policy_uidx": { + "name": "tool_policy_uidx", + "columns": [ + { + "expression": "tenant", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "owner", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "subject", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "id", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": true, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + } + }, + "enums": {}, + "schemas": {}, + "sequences": {}, + "roles": {}, + "policies": {}, + "views": {}, + "_meta": { + "columns": {}, + "schemas": {}, + "tables": {} + } +} diff --git a/apps/cloud/drizzle/meta/_journal.json b/apps/cloud/drizzle/meta/_journal.json index 903e5515a..ac4c2c168 100644 --- a/apps/cloud/drizzle/meta/_journal.json +++ b/apps/cloud/drizzle/meta/_journal.json @@ -57,6 +57,13 @@ "when": 1782149425764, "tag": "0007_red_dragon_lord", "breakpoints": true + }, + { + "idx": 8, + "version": "7", + "when": 1783025157232, + "tag": "0008_slow_mephisto", + "breakpoints": true } ] } diff --git a/apps/cloud/scripts/code-migrations/gc-dead-dcr-oauth-clients.ts b/apps/cloud/scripts/code-migrations/gc-dead-dcr-oauth-clients.ts new file mode 100644 index 000000000..fcc05a003 --- /dev/null +++ b/apps/cloud/scripts/code-migrations/gc-dead-dcr-oauth-clients.ts @@ -0,0 +1,142 @@ +// --------------------------------------------------------------------------- +// Cloud code migration: GC dead DCR `oauth_client` rows + backfill the +// surviving ones' `origin_issuer` (issue #1120, Part C). +// +// The cloud counterpart to the local libSQL boot migration +// (`sqlite-oauth-client-gc-migration` in @executor-js/sdk). It runs through the +// stamped `cloud_code_migration` ledger (see runner.ts), out-of-band via +// `scripts/migrate.ts`, with `--dry-run` support and an advisory lock. +// +// It is a CODE migration, not a numbered `.sql` one, deliberately: the DCR +// classification and the registrable-origin backfill are the SAME TypeScript +// predicates the local path and the runtime reuse lookup use +// (`classifyOAuthClientGc`, `registrableOriginOfUrl`). Re-encoding the eTLD+1 +// registrable-origin logic and the DCR heuristic as Postgres regex/SQL would be +// a second, drift-prone source of truth for a migration that DELETES user data. +// Reading the (small) oauth_client table and deciding in-process keeps exactly +// one source of truth across both hosts. +// --------------------------------------------------------------------------- + +import { + classifyOAuthClientGc, + isDcrClassifiedRow, + registrableOriginOfUrl, +} from "@executor-js/sdk"; + +import type { CodeMigration, CodeMigrationContext } from "./runner"; + +interface OAuthClientRow { + readonly tenant: string; + readonly owner: string; + readonly slug: string; + readonly grant: string | null; + readonly resource: string | null; + readonly origin_kind: string | null; + readonly origin_issuer: string | null; + readonly token_url: string | null; +} + +const readOAuthClients = (context: CodeMigrationContext): Promise => + // `grant` is a reserved word in Postgres, so it must be quoted. Deterministic + // row order keeps the dry-run report + logs stable. + context.sql.unsafe(` + SELECT + tenant, + owner, + slug, + "grant", + resource, + origin_kind, + origin_issuer, + token_url + FROM oauth_client + ORDER BY tenant, owner, slug + `); + +interface ConnectionCountRow { + readonly tenant: string; + readonly oauth_client_owner: string; + readonly oauth_client: string; + readonly count: string | number; +} + +const referenceKey = (tenant: string, owner: string, slug: string): string => + `${tenant}\u0000${owner}\u0000${slug}`; + +/** One grouped pass over `connection` instead of a COUNT per oauth_client row + * (N+1). Returns a Map from (tenant, oauth_client_owner, oauth_client) to the + * number of referencing connections. */ +const readReferencingConnectionCounts = async ( + context: CodeMigrationContext, +): Promise> => { + const rows = await context.sql.unsafe(` + SELECT tenant, oauth_client_owner, oauth_client, COUNT(*)::int AS count + FROM connection + WHERE oauth_client IS NOT NULL + GROUP BY tenant, oauth_client_owner, oauth_client + `); + const counts = new Map(); + for (const row of rows) { + counts.set( + referenceKey(row.tenant, row.oauth_client_owner, row.oauth_client), + Number(row.count ?? 0), + ); + } + return counts; +}; + +export const gcDeadDcrOAuthClientsMigration: CodeMigration = { + name: "2026-07-02-gc-dead-dcr-oauth-clients", + run: async (context) => { + const rows = await readOAuthClients(context); + const referenceCounts = await readReferencingConnectionCounts(context); + + let deleted = 0; + let backfilled = 0; + let referencedDcr = 0; + + for (const row of rows) { + if (!isDcrClassifiedRow(row)) continue; // manual apps: never touched. + + const count = referenceCounts.get(referenceKey(row.tenant, row.owner, row.slug)) ?? 0; + const decision = classifyOAuthClientGc(row, count); + + if (decision.action === "delete") { + if (!context.dryRun) { + await context.sql.unsafe( + `DELETE FROM oauth_client WHERE tenant = $1 AND owner = $2 AND slug = $3`, + [row.tenant, row.owner, row.slug], + ); + } + deleted += 1; + continue; + } + + referencedDcr += 1; + // Surviving (referenced) DCR row with no stored issuer: backfill it from + // the registrable origin of token_url so the per-AS reuse lookup keys on + // it and mints no new duplicate. + if (row.origin_issuer == null) { + const issuer = row.token_url == null ? null : registrableOriginOfUrl(row.token_url); + if (issuer !== null) { + if (!context.dryRun) { + await context.sql.unsafe( + `UPDATE oauth_client SET origin_issuer = $1 + WHERE tenant = $2 AND owner = $3 AND slug = $4`, + [issuer, row.tenant, row.owner, row.slug], + ); + } + backfilled += 1; + } + } + } + + const verb = context.dryRun ? "would delete" : "deleted"; + const backfillVerb = context.dryRun ? "would backfill" : "backfilled"; + return { + summary: + `${rows.length} oauth_client row(s): ${verb} ${deleted} orphaned DCR client(s), ` + + `${backfillVerb} ${backfilled} of ${referencedDcr} referenced DCR client(s)`, + }; + }, +}; diff --git a/apps/cloud/scripts/code-migrations/index.ts b/apps/cloud/scripts/code-migrations/index.ts index 9783d2cf9..16da55f3a 100644 --- a/apps/cloud/scripts/code-migrations/index.ts +++ b/apps/cloud/scripts/code-migrations/index.ts @@ -1,5 +1,6 @@ import type { CodeMigration } from "./runner"; import { googleOpenApiR2BlobMigration } from "./google-openapi-r2-blobs"; +import { gcDeadDcrOAuthClientsMigration } from "./gc-dead-dcr-oauth-clients"; export interface CloudCodeMigrationRegistryOptions { readonly r2Bucket?: string; @@ -11,6 +12,9 @@ export const cloudCodeMigrations = ({ limit, }: CloudCodeMigrationRegistryOptions): readonly CodeMigration[] => [ ...(r2Bucket ? [googleOpenApiR2BlobMigration({ bucket: r2Bucket, limit })] : []), + // GC dead DCR oauth_client rows + backfill surviving rows' origin_issuer + // (issue #1120, Part C). No R2/bucket dependency, so always registered. + gcDeadDcrOAuthClientsMigration, ]; export { runCodeMigrations } from "./runner"; diff --git a/apps/cloud/src/db/executor-schema.ts b/apps/cloud/src/db/executor-schema.ts index 89e9688cb..897e4a06f 100644 --- a/apps/cloud/src/db/executor-schema.ts +++ b/apps/cloud/src/db/executor-schema.ts @@ -83,6 +83,7 @@ export const oauth_client = pgTable( resource: text("resource"), origin_kind: text("origin_kind"), origin_integration: text("origin_integration"), + origin_issuer: text("origin_issuer"), created_at: timestamp("created_at").notNull(), row_id: varchar("row_id", { length: 255 }) .primaryKey() diff --git a/apps/cloud/src/db/gc-dead-dcr-oauth-clients-code-migration.test.ts b/apps/cloud/src/db/gc-dead-dcr-oauth-clients-code-migration.test.ts new file mode 100644 index 000000000..cfb563c1d --- /dev/null +++ b/apps/cloud/src/db/gc-dead-dcr-oauth-clients-code-migration.test.ts @@ -0,0 +1,160 @@ +/* oxlint-disable executor/no-error-constructor, executor/no-try-catch-or-throw -- test fake throws on an unexpected query to catch wiring drift */ + +import { describe, expect, it } from "@effect/vitest"; +import { Effect } from "effect"; + +import { gcDeadDcrOAuthClientsMigration } from "../../scripts/code-migrations/gc-dead-dcr-oauth-clients"; +import type { CodeMigrationContext, CodeMigrationSql } from "../../scripts/code-migrations/runner"; + +// Cloud counterpart of the local libSQL GC migration test. A small in-memory +// fake models the SQL shapes the migration issues (the oauth_client SELECT, the +// single GROUPED connection-count query, and the DELETE/UPDATE mutations) so the +// same shared decision matrix is proven end-to-end over the Postgres wiring. + +interface OAuthClientRow { + readonly tenant: string; + readonly owner: string; + readonly slug: string; + readonly grant: string | null; + readonly resource: string | null; + readonly origin_kind: string | null; + readonly origin_issuer: string | null; + readonly token_url: string | null; +} + +const makeFake = (input: { + rows: OAuthClientRow[]; + /** connection count keyed by `${tenant}|${owner}|${slug}`. */ + readonly references: Record; +}) => { + const deletes: string[] = []; + const updates: { slug: string; issuer: string }[] = []; + + const sql: CodeMigrationSql = { + unsafe: (query: string, params?: readonly unknown[]) => { + const q = query.trim(); + if (q.startsWith("SELECT") && q.includes("FROM oauth_client")) { + return Promise.resolve(input.rows as never); + } + if (q.startsWith("SELECT") && q.includes("FROM connection")) { + // One grouped pass: emit a row per referenced (tenant, owner, slug). + const grouped = Object.entries(input.references).map(([composite, count]) => { + const [tenant, owner, slug] = composite.split("|"); + return { tenant, oauth_client_owner: owner, oauth_client: slug, count }; + }); + return Promise.resolve(grouped as never); + } + if (q.startsWith("DELETE FROM oauth_client")) { + const [tenant, owner, slug] = params as [string, string, string]; + input.rows = input.rows.filter( + (r) => !(r.tenant === tenant && r.owner === owner && r.slug === slug), + ); + deletes.push(slug); + return Promise.resolve([] as never); + } + if (q.startsWith("UPDATE oauth_client")) { + const [issuer, , , slug] = params as [string, string, string, string]; + updates.push({ slug, issuer }); + return Promise.resolve([] as never); + } + throw new Error(`unexpected query: ${q}`); + }, + }; + + return { sql, deletes, updates, rowsRef: () => input.rows }; +}; + +const row = (over: Partial & { readonly slug: string }): OAuthClientRow => ({ + tenant: "t1", + owner: "org", + slug: over.slug, + grant: over.grant ?? "authorization_code", + resource: over.resource ?? null, + origin_kind: over.origin_kind ?? null, + origin_issuer: over.origin_issuer ?? null, + token_url: over.token_url ?? "https://oauth.cloudflare.com/token", +}); + +const runMigration = ( + fake: ReturnType, + dryRun: boolean, +): Promise<{ readonly summary: string }> => { + const context: CodeMigrationContext = { sql: fake.sql, dryRun, log: () => {} }; + return gcDeadDcrOAuthClientsMigration.run(context); +}; + +describe("gcDeadDcrOAuthClientsMigration", () => { + it.effect("deletes orphaned DCR, keeps manual + referenced, backfills survivors", () => + Effect.promise(async () => { + const fake = makeFake({ + rows: [ + row({ + slug: "dcr-cloudflare-com", + resource: "https://cf.example/mcp", + origin_kind: "dynamic_client_registration", + origin_issuer: "https://cloudflare.com", + }), + row({ slug: "cloudflare-mcp-2", resource: "https://cf.example/mcp" }), + row({ slug: "cloudflare-mcp", resource: "https://cf.example/mcp" }), + row({ + slug: "my-github-app", + token_url: "https://github.com/login/oauth/access_token", + origin_kind: "manual", + }), + ], + references: { "t1|org|cloudflare-mcp": 1 }, + }); + + const result = await runMigration(fake, false); + + expect(fake.deletes.sort()).toEqual(["cloudflare-mcp-2", "dcr-cloudflare-com"]); + expect(fake.updates).toEqual([{ slug: "cloudflare-mcp", issuer: "https://cloudflare.com" }]); + expect( + fake + .rowsRef() + .map((r) => r.slug) + .sort(), + ).toEqual(["cloudflare-mcp", "my-github-app"]); + expect(result.summary).toContain("deleted 2 orphaned DCR client(s)"); + expect(result.summary).toContain("backfilled 1 of 1 referenced DCR client(s)"); + }), + ); + + it.effect("dry run mutates nothing but reports the plan", () => + Effect.promise(async () => { + const fake = makeFake({ + rows: [ + row({ slug: "cloudflare-mcp-2", resource: "https://cf.example/mcp" }), + row({ slug: "cloudflare-mcp", resource: "https://cf.example/mcp" }), + ], + references: { "t1|org|cloudflare-mcp": 1 }, + }); + + const result = await runMigration(fake, true); + + expect(fake.deletes).toEqual([]); + expect(fake.updates).toEqual([]); + expect(fake.rowsRef()).toHaveLength(2); + expect(result.summary).toContain("would delete 1 orphaned DCR client(s)"); + expect(result.summary).toContain("would backfill 1 of 1 referenced DCR client(s)"); + }), + ); + + it.effect("never deletes a manual app even when orphaned and MCP-shaped", () => + Effect.promise(async () => { + const fake = makeFake({ + rows: [ + row({ + slug: "cloudflare-mcp", + resource: "https://cf.example/mcp", + origin_kind: "manual", + }), + ], + references: {}, + }); + await runMigration(fake, false); + expect(fake.deletes).toEqual([]); + expect(fake.rowsRef().map((r) => r.slug)).toEqual(["cloudflare-mcp"]); + }), + ); +}); diff --git a/apps/local/drizzle/0003_graceful_the_fallen.sql b/apps/local/drizzle/0003_graceful_the_fallen.sql new file mode 100644 index 000000000..fdb9dd8f0 --- /dev/null +++ b/apps/local/drizzle/0003_graceful_the_fallen.sql @@ -0,0 +1 @@ +ALTER TABLE `oauth_client` ADD `origin_issuer` text; \ No newline at end of file diff --git a/apps/local/drizzle/meta/0003_snapshot.json b/apps/local/drizzle/meta/0003_snapshot.json new file mode 100644 index 000000000..b6224618f --- /dev/null +++ b/apps/local/drizzle/meta/0003_snapshot.json @@ -0,0 +1,920 @@ +{ + "version": "6", + "dialect": "sqlite", + "id": "80ff1a5d-ab69-416c-868e-2bf6609fa5b4", + "prevId": "03321915-aef6-4791-9558-7fc82273af05", + "tables": { + "blob": { + "name": "blob", + "columns": { + "namespace": { + "name": "namespace", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "key": { + "name": "key", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "value": { + "name": "value", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "row_id": { + "name": "row_id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "id": { + "name": "id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "blob_id_uidx": { + "name": "blob_id_uidx", + "columns": ["id"], + "isUnique": true + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "connection": { + "name": "connection", + "columns": { + "integration": { + "name": "integration", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "template": { + "name": "template", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "provider": { + "name": "provider", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "item_ids": { + "name": "item_ids", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "identity_label": { + "name": "identity_label", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "oauth_client": { + "name": "oauth_client", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "oauth_client_owner": { + "name": "oauth_client_owner", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "refresh_item_id": { + "name": "refresh_item_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "expires_at": { + "name": "expires_at", + "type": "blob", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "oauth_scope": { + "name": "oauth_scope", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "oauth_token_url": { + "name": "oauth_token_url", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "provider_state": { + "name": "provider_state", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updated_at": { + "name": "updated_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "row_id": { + "name": "row_id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "tenant": { + "name": "tenant", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "owner": { + "name": "owner", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "subject": { + "name": "subject", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "connection_uidx": { + "name": "connection_uidx", + "columns": ["tenant", "owner", "subject", "integration", "name"], + "isUnique": true + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "definition": { + "name": "definition", + "columns": { + "integration": { + "name": "integration", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "connection": { + "name": "connection", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "plugin_id": { + "name": "plugin_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "schema": { + "name": "schema", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "row_id": { + "name": "row_id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "tenant": { + "name": "tenant", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "owner": { + "name": "owner", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "subject": { + "name": "subject", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "definition_uidx": { + "name": "definition_uidx", + "columns": ["tenant", "owner", "subject", "integration", "connection", "name"], + "isUnique": true + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "integration": { + "name": "integration", + "columns": { + "slug": { + "name": "slug", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "plugin_id": { + "name": "plugin_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "description": { + "name": "description", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "config": { + "name": "config", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "can_remove": { + "name": "can_remove", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 1 + }, + "can_refresh": { + "name": "can_refresh", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 0 + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updated_at": { + "name": "updated_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "row_id": { + "name": "row_id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "tenant": { + "name": "tenant", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "integration_uidx": { + "name": "integration_uidx", + "columns": ["tenant", "slug"], + "isUnique": true + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "oauth_client": { + "name": "oauth_client", + "columns": { + "slug": { + "name": "slug", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "authorization_url": { + "name": "authorization_url", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "token_url": { + "name": "token_url", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "grant": { + "name": "grant", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "client_id": { + "name": "client_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "client_secret_item_id": { + "name": "client_secret_item_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "resource": { + "name": "resource", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "origin_kind": { + "name": "origin_kind", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "origin_integration": { + "name": "origin_integration", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "origin_issuer": { + "name": "origin_issuer", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "row_id": { + "name": "row_id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "tenant": { + "name": "tenant", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "owner": { + "name": "owner", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "subject": { + "name": "subject", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "oauth_client_uidx": { + "name": "oauth_client_uidx", + "columns": ["tenant", "owner", "subject", "slug"], + "isUnique": true + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "oauth_session": { + "name": "oauth_session", + "columns": { + "state": { + "name": "state", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "client_slug": { + "name": "client_slug", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "integration": { + "name": "integration", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "template": { + "name": "template", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "redirect_url": { + "name": "redirect_url", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "pkce_verifier": { + "name": "pkce_verifier", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "identity_label": { + "name": "identity_label", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "payload": { + "name": "payload", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "expires_at": { + "name": "expires_at", + "type": "blob", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "row_id": { + "name": "row_id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "tenant": { + "name": "tenant", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "owner": { + "name": "owner", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "subject": { + "name": "subject", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "oauth_session_uidx": { + "name": "oauth_session_uidx", + "columns": ["tenant", "state"], + "isUnique": true + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "plugin_storage": { + "name": "plugin_storage", + "columns": { + "plugin_id": { + "name": "plugin_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "collection": { + "name": "collection", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "key": { + "name": "key", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "data": { + "name": "data", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updated_at": { + "name": "updated_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "row_id": { + "name": "row_id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "tenant": { + "name": "tenant", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "owner": { + "name": "owner", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "subject": { + "name": "subject", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "plugin_storage_uidx": { + "name": "plugin_storage_uidx", + "columns": ["tenant", "owner", "subject", "plugin_id", "collection", "key"], + "isUnique": true + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "tool": { + "name": "tool", + "columns": { + "integration": { + "name": "integration", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "connection": { + "name": "connection", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "plugin_id": { + "name": "plugin_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "description": { + "name": "description", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "input_schema": { + "name": "input_schema", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "output_schema": { + "name": "output_schema", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "annotations": { + "name": "annotations", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updated_at": { + "name": "updated_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "row_id": { + "name": "row_id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "tenant": { + "name": "tenant", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "owner": { + "name": "owner", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "subject": { + "name": "subject", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "tool_uidx": { + "name": "tool_uidx", + "columns": ["tenant", "owner", "subject", "integration", "connection", "name"], + "isUnique": true + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "tool_policy": { + "name": "tool_policy", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "pattern": { + "name": "pattern", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "action": { + "name": "action", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "position": { + "name": "position", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updated_at": { + "name": "updated_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "row_id": { + "name": "row_id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "tenant": { + "name": "tenant", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "owner": { + "name": "owner", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "subject": { + "name": "subject", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "tool_policy_uidx": { + "name": "tool_policy_uidx", + "columns": ["tenant", "owner", "subject", "id"], + "isUnique": true + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + } + }, + "views": {}, + "enums": {}, + "_meta": { + "schemas": {}, + "tables": {}, + "columns": {} + }, + "internal": { + "indexes": {} + } +} diff --git a/apps/local/drizzle/meta/_journal.json b/apps/local/drizzle/meta/_journal.json index 2f022e787..05148dbb7 100644 --- a/apps/local/drizzle/meta/_journal.json +++ b/apps/local/drizzle/meta/_journal.json @@ -22,6 +22,13 @@ "when": 1782150869757, "tag": "0002_empty_rictor", "breakpoints": true + }, + { + "idx": 3, + "version": "6", + "when": 1783025153772, + "tag": "0003_graceful_the_fallen", + "breakpoints": true } ] } diff --git a/apps/local/src/db/data-migrations.ts b/apps/local/src/db/data-migrations.ts index 21d685b9b..8b7a0871e 100644 --- a/apps/local/src/db/data-migrations.ts +++ b/apps/local/src/db/data-migrations.ts @@ -5,7 +5,12 @@ // renamed. // --------------------------------------------------------------------------- -import { Effect, sqliteDataMigration, type SqliteDataMigration } from "@executor-js/sdk"; +import { + Effect, + oauthClientGcSqliteMigration, + sqliteDataMigration, + type SqliteDataMigration, +} from "@executor-js/sdk"; import { runSqliteAuthConfigMigration } from "@executor-js/sdk/http-auth"; import { openApiOutputSchemaDataMigration, @@ -37,4 +42,7 @@ export const localDataMigrations: readonly SqliteDataMigration[] = [ openApiSpecBlobDataMigration, graphqlIntrospectionBlobDataMigration, googleOpenApiOwnershipDataMigration, + // GC dead DCR oauth_client rows (old always-register duplicates) and backfill + // the surviving DCR rows' origin_issuer from token_url (issue #1120, Part C). + oauthClientGcSqliteMigration, ]; diff --git a/apps/local/src/db/executor-schema.ts b/apps/local/src/db/executor-schema.ts index 6d005e4c5..83cfa9d5f 100644 --- a/apps/local/src/db/executor-schema.ts +++ b/apps/local/src/db/executor-schema.ts @@ -75,6 +75,7 @@ export const oauth_client = sqliteTable( resource: text("resource"), origin_kind: text("origin_kind"), origin_integration: text("origin_integration"), + origin_issuer: text("origin_issuer"), created_at: integer("created_at").notNull(), row_id: text("row_id").primaryKey().notNull(), tenant: text("tenant").notNull(), diff --git a/apps/local/src/db/sqlite-fumadb.ts b/apps/local/src/db/sqlite-fumadb.ts index 5bee04a1c..0f9390a73 100644 --- a/apps/local/src/db/sqlite-fumadb.ts +++ b/apps/local/src/db/sqlite-fumadb.ts @@ -93,6 +93,12 @@ export const createSqliteFumaDb = async ( ) { await client.execute("ALTER TABLE oauth_client ADD COLUMN origin_integration TEXT"); } + if ( + oauthClientColumns.rows.length > 0 && + !oauthClientColumns.rows.some((column) => column["name"] === "origin_issuer") + ) { + await client.execute("ALTER TABLE oauth_client ADD COLUMN origin_issuer TEXT"); + } const { db, fuma } = createExecutorFumaDb(drizzleDb, { tables: options.tables, diff --git a/bun.lock b/bun.lock index 93f1ed1be..b123d5d25 100644 --- a/bun.lock +++ b/bun.lock @@ -582,6 +582,7 @@ "@standard-schema/spec": "^1.1.0", "fractional-indexing": "^3.2.0", "oauth4webapi": "^3.8.5", + "tldts": "^7.0.28", }, "devDependencies": { "@effect/atom-react": "catalog:", diff --git a/e2e/cloud/dcr-root-domain-isolation.test.ts b/e2e/cloud/dcr-root-domain-isolation.test.ts new file mode 100644 index 000000000..e9eb9e015 --- /dev/null +++ b/e2e/cloud/dcr-root-domain-isolation.test.ts @@ -0,0 +1,176 @@ +// Cloud: the DCR root-domain collision, end to end over the product API. +// +// The bug this guards (issue #1120): two DIFFERENT integrations can share one +// registrable root domain but differ by host, e.g. a "Cloudflare" MCP +// integration whose OAuth lives on mcp.cloudflare.com vs the plain "Cloudflare" +// REST integration on api.cloudflare.com. When the MCP integration is connected +// it Dynamic-Client-Registers an oauth_client automatically. That auto-minted +// client must NEVER leak into the REST integration's Add-connection app picker +// (it is plumbing, not an app the user picked), and connecting the MCP +// integration a second time must REUSE the same client rather than mint a +// "name 2" duplicate. +// +// Everything here is over the wire: the typed product client drives the real +// cloud API, and a real authorization server (with an RFC 7591 registration +// endpoint) runs inside the scenario on 127.0.0.1. To reproduce the collision +// the scenario stores mcp.cloudflare.com / api.cloudflare.com as the clients' +// endpoint URLs (the classifier keys off those strings, never the network) +// while pointing the actual registration round-trip at the local server. +// +// This e2e proves the SERVER-SIDE half over the wire: (b) DCR mints a client +// against the MCP host, (d) reconnecting reuses it with no duplicate row, and +// the REAL `listClients` projection marks that client as +// `dynamic_client_registration` while preserving its mcp.cloudflare.com +// endpoints. Those two facts (origin kind + persisted host) are the entire +// input the picker classifier keys off. The picker's own verdict for the REST +// integration (that the DCR client is offered in no tier) is asserted against +// the exact production classifier in the React package's integration test +// (packages/react/src/plugins/dcr-root-domain-isolation.test.ts); it lives +// there because that pure function ships in @executor-js/react, and the e2e +// package deliberately does not depend on the React layer. See the report. +import { randomBytes } from "node:crypto"; + +import { expect } from "@effect/vitest"; +import { Effect } from "effect"; +import { composePluginApi } from "@executor-js/api/server"; +import { openApiHttpPlugin } from "@executor-js/plugin-openapi/api"; +import { IntegrationSlug, OAuthClientSlug } from "@executor-js/sdk/shared"; +import { serveOAuthTestServer } from "@executor-js/sdk/testing"; + +import { scenario } from "../src/scenario"; +import { Api, Target } from "../src/services"; + +const api = composePluginApi([openApiHttpPlugin()] as const); + +const unique = (prefix: string) => `${prefix}_${randomBytes(4).toString("hex")}`; + +// A same-provider, different-host pair: same registrable root (cloudflare.com), +// distinct hosts. These are the endpoint URLs the clients are PERSISTED with, +// which is all the classifier looks at. +const MCP_ISSUER = "https://mcp.cloudflare.com"; +const MCP_AUTHORIZE = "https://mcp.cloudflare.com/authorize"; +const MCP_TOKEN = "https://mcp.cloudflare.com/token"; +const REST_AUTHORIZE = "https://api.cloudflare.com/client/v4/oauth/authorize"; +const REST_TOKEN = "https://api.cloudflare.com/client/v4/oauth/token"; + +scenario( + "OAuth DCR · an MCP integration's auto-registered client never leaks into a same-root REST integration's app picker, and reconnecting reuses it", + {}, + Effect.scoped( + Effect.gen(function* () { + const target = yield* Target; + const { client: makeClient } = yield* Api; + // A real authorization server with an RFC 7591 registration endpoint. Its + // /register mints a client_id; the mcp.cloudflare.com URLs above are what + // we persist so the classifier sees the real-world collision shape. + const oauth = yield* serveOAuthTestServer(); + const identity = yield* target.newIdentity(); + const client = yield* makeClient(api, identity); + + // Two integrations on the SAME provider root, different hosts: an MCP-style + // one (mcp.cloudflare.com) and a plain REST one (api.cloudflare.com). + const mcpIntegration = IntegrationSlug.make(unique("cloudflare_mcp")); + const restIntegration = IntegrationSlug.make(unique("cloudflare_api")); + + // The REST integration declares its OAuth endpoints up front (api.cloudflare.com). + yield* client.openapi.addSpec({ + payload: { + spec: { + kind: "blob", + value: JSON.stringify({ + openapi: "3.0.3", + info: { title: "Cloudflare REST API", version: "1.0.0" }, + paths: { + "/zones": { + get: { + operationId: "listZones", + tags: ["default"], + responses: { "200": { description: "zones" } }, + }, + }, + }, + }), + }, + slug: restIntegration, + baseUrl: "https://api.cloudflare.com/client/v4", + authenticationTemplate: [ + { + slug: "oauth", + kind: "oauth2", + authorizationUrl: REST_AUTHORIZE, + tokenUrl: REST_TOKEN, + scopes: ["read"], + }, + ], + }, + }); + + // (b) Connect the MCP-style integration via Dynamic Client Registration. + // The registration round-trip hits the real local server; the persisted + // client carries the mcp.cloudflare.com endpoints and its issuer. + const registerMcp = () => + client.oauth.registerDynamic({ + payload: { + owner: "org", + // Passed but overridden by the server's deterministic dcr- slug + // when an issuer is present; kept as a fallback for the no-issuer path. + slug: OAuthClientSlug.make(unique("cloudflare-mcp")), + issuer: MCP_ISSUER, + registrationEndpoint: oauth.registrationEndpoint, + authorizationUrl: MCP_AUTHORIZE, + tokenUrl: MCP_TOKEN, + scopes: ["read"], + clientName: "Cloudflare MCP", + originIntegration: mcpIntegration, + }, + }); + + const first = yield* registerMcp(); + expect( + String(first.client), + "the auto-registered client gets a deterministic issuer-host slug", + ).toBe("dcr-mcp-cloudflare-com"); + + // (d) Reconnect the MCP integration: the SAME authorization server (issuer) + // must reuse the existing client, not mint a duplicate "…-2". + const second = yield* registerMcp(); + expect( + String(second.client), + "reconnecting the MCP integration reuses the same DCR client (no duplicate row)", + ).toBe(String(first.client)); + + // The REAL API projection of every client the owner can see. This is the + // exact shape the picker classifier consumes. + const clients = yield* client.oauth.listClients(); + const dcrRows = clients.filter( + (entry) => entry.origin.kind === "dynamic_client_registration", + ); + expect( + dcrRows.map((entry) => String(entry.slug)), + "exactly one DCR client exists for the shared authorization server (no duplicate)", + ).toEqual(["dcr-mcp-cloudflare-com"]); + + const dcrRow = dcrRows[0]!; + // The picker excludes a client iff its origin is DCR — assert the API + // projects that origin so the classifier will hide it. + expect( + dcrRow.origin.kind, + "the auto-registered client is projected as a DCR client (the flag the picker hides on)", + ).toBe("dynamic_client_registration"); + // The persisted endpoints are the MCP host, NOT the local registration + // server: this is the same-root/different-host collision the classifier + // must not silently promote into the REST integration's picker. + expect( + dcrRow.tokenUrl, + "the DCR client keeps the MCP host it was registered for, not the registration endpoint", + ).toBe(MCP_TOKEN); + // Sanity: the REST integration itself has NO manual app, so its picker + // would fall back to near/other tiers if the DCR client were ever + // classified as a real app — the React test proves it never is. + expect( + clients.some((entry) => entry.origin.kind === "manual"), + "no manual OAuth app was registered in this scenario", + ).toBe(false); + }), + ), +); diff --git a/packages/core/api/src/handlers/oauth.ts b/packages/core/api/src/handlers/oauth.ts index 924aff682..cea145f2e 100644 --- a/packages/core/api/src/handlers/oauth.ts +++ b/packages/core/api/src/handlers/oauth.ts @@ -103,6 +103,7 @@ export const OAuthHandlers = HttpApiBuilder.group(ExecutorApi, "oauth", (handler clientId: payload.clientId, clientSecret: payload.clientSecret, resource: payload.resource ?? null, + origin: { kind: "manual", integration: payload.originIntegration ?? null }, }); return { client }; }), @@ -115,6 +116,7 @@ export const OAuthHandlers = HttpApiBuilder.group(ExecutorApi, "oauth", (handler const client = yield* executor.oauth.registerDynamicClient({ owner: payload.owner, slug: payload.slug, + issuer: payload.issuer ?? null, registrationEndpoint: payload.registrationEndpoint, authorizationUrl: payload.authorizationUrl, tokenUrl: payload.tokenUrl, diff --git a/packages/core/api/src/oauth/api.ts b/packages/core/api/src/oauth/api.ts index 8e3e27455..66404af5e 100644 --- a/packages/core/api/src/oauth/api.ts +++ b/packages/core/api/src/oauth/api.ts @@ -65,6 +65,9 @@ const CreateClientPayload = Schema.Struct({ clientId: Schema.String, clientSecret: Schema.String, resource: Schema.optional(Schema.NullOr(Schema.String)), + /** Integration whose connect dialog registered this manual app. Recorded so + * the picker matches it to this integration by intent, not root domain. */ + originIntegration: Schema.optional(Schema.NullOr(IntegrationSlug)), }); const CreateClientResponse = Schema.Struct({ @@ -81,6 +84,7 @@ const CreateClientResponse = Schema.Struct({ const RegisterDynamicPayload = Schema.Struct({ owner: Owner, slug: OAuthClientSlug, + issuer: Schema.optional(Schema.NullOr(Schema.String)), registrationEndpoint: Schema.String, authorizationUrl: Schema.String, tokenUrl: Schema.String, @@ -203,6 +207,7 @@ const ProbePayload = Schema.Struct({ }); const ProbeResponse = Schema.Struct({ + issuer: Schema.optional(Schema.NullOr(Schema.String)), authorizationUrl: Schema.String, tokenUrl: Schema.String, resource: Schema.optional(Schema.NullOr(Schema.String)), diff --git a/packages/core/sdk/package.json b/packages/core/sdk/package.json index e3f2cd331..7c9720a97 100644 --- a/packages/core/sdk/package.json +++ b/packages/core/sdk/package.json @@ -96,7 +96,8 @@ "@executor-js/fumadb": "workspace:*", "@standard-schema/spec": "^1.1.0", "fractional-indexing": "^3.2.0", - "oauth4webapi": "^3.8.5" + "oauth4webapi": "^3.8.5", + "tldts": "^7.0.28" }, "devDependencies": { "@effect/atom-react": "catalog:", diff --git a/packages/core/sdk/src/core-schema.ts b/packages/core/sdk/src/core-schema.ts index ef300973f..ab8331ee7 100644 --- a/packages/core/sdk/src/core-schema.ts +++ b/packages/core/sdk/src/core-schema.ts @@ -212,6 +212,14 @@ export const coreTables = defineTables({ // "manual" by the service layer. origin_kind: nullableTextColumn("origin_kind"), origin_integration: nullableTextColumn("origin_integration"), + // Authorization-server issuer that owns a DCR client, keying per-AS reuse. + // For a NEW DCR registration this is the DISCOVERED OIDC issuer (real + // information from the AS metadata, which can legitimately differ from what + // token_url would suggest). For a BACKFILLED legacy row it is instead a + // derived registrable-origin of token_url (a cache of the pure + // `registrableOriginOfUrl`, since no discovered issuer was ever recorded). + // Null for manual clients and legacy rows not yet backfilled. + origin_issuer: nullableTextColumn("origin_issuer"), created_at: dateColumn("created_at"), }, ["tenant", "owner", "subject", "slug"], diff --git a/packages/core/sdk/src/core-tools.ts b/packages/core/sdk/src/core-tools.ts index be9a62b40..a61aa85a9 100644 --- a/packages/core/sdk/src/core-tools.ts +++ b/packages/core/sdk/src/core-tools.ts @@ -243,6 +243,9 @@ const OAuthCreateClientInput = Schema.Struct({ grant: OAuthGrantSchema, clientId: Schema.String, resource: Schema.optional(Schema.NullOr(Schema.String)), + /** Integration whose connect dialog registered this manual app. Recorded so + * the picker can match it by intent (exact) instead of by root domain. */ + originIntegration: Schema.optional(Schema.NullOr(Schema.String)), }); // Browser-handoff for a CONFIDENTIAL OAuth app: carries only the NON-secret // fields the form pre-fills. The client secret is typed by the human in the web @@ -269,6 +272,7 @@ const OAuthClientOutputRef = Schema.Struct({ const OAuthRegisterDynamicInput = Schema.Struct({ owner: OwnerSchema, slug: Schema.String, + issuer: Schema.optional(Schema.NullOr(Schema.String)), registrationEndpoint: Schema.String, authorizationUrl: Schema.String, tokenUrl: Schema.String, @@ -287,6 +291,7 @@ const OAuthProbeInput = Schema.Struct({ url: Schema.String, }); const OAuthProbeOutput = Schema.Struct({ + issuer: Schema.optional(Schema.NullOr(Schema.String)), authorizationUrl: Schema.String, tokenUrl: Schema.String, resource: Schema.optional(Schema.NullOr(Schema.String)), @@ -711,6 +716,13 @@ export const coreToolsPlugin = definePlugin((options: CoreToolsPluginOptions = { // `oauth.clients.createHandoff`. clientSecret: "", resource: input.resource ?? null, + origin: { + kind: "manual", + integration: + input.originIntegration == null + ? null + : IntegrationSlug.make(input.originIntegration), + }, }), (client) => ({ client: String(client) }), ), @@ -748,6 +760,7 @@ export const coreToolsPlugin = definePlugin((options: CoreToolsPluginOptions = { ctx.oauth.registerDynamicClient({ owner: input.owner as Owner, slug: OAuthClientSlug.make(input.slug), + issuer: input.issuer ?? null, registrationEndpoint: input.registrationEndpoint, authorizationUrl: input.authorizationUrl, tokenUrl: input.tokenUrl, @@ -789,6 +802,7 @@ export const coreToolsPlugin = definePlugin((options: CoreToolsPluginOptions = { outputSchema: OAuthProbeOutputStd, execute: (input: typeof OAuthProbeInput.Type, { ctx }) => Effect.map(ctx.oauth.probe({ url: input.url }), (result) => ({ + issuer: result.issuer ?? null, authorizationUrl: result.authorizationUrl, tokenUrl: result.tokenUrl, resource: result.resource ?? null, diff --git a/packages/core/sdk/src/index.ts b/packages/core/sdk/src/index.ts index 69b2e83bd..7be768201 100644 --- a/packages/core/sdk/src/index.ts +++ b/packages/core/sdk/src/index.ts @@ -363,6 +363,19 @@ export { runSqliteConfigBlobMigration, type SqliteConfigBlobMigrationOptions, } from "./sqlite-config-blob-migration"; +// DCR oauth_client GC + issuer backfill: shared classification predicates and +// the libSQL boot-migration body (issue #1120, Part C). +export { + classifyOAuthClientGc, + isDcrClassifiedRow, + registrableOriginOfUrl, + type OAuthClientGcDecision, + type OAuthClientGcRow, +} from "./oauth-gc"; +export { + oauthClientGcSqliteMigration, + runSqliteOAuthClientGcMigration, +} from "./sqlite-oauth-client-gc-migration"; export { authToolFailure, type AuthToolFailureCode, diff --git a/packages/core/sdk/src/oauth-client.ts b/packages/core/sdk/src/oauth-client.ts index b37894987..b9ff81a23 100644 --- a/packages/core/sdk/src/oauth-client.ts +++ b/packages/core/sdk/src/oauth-client.ts @@ -64,7 +64,15 @@ export interface OAuthClient { } export type OAuthClientOrigin = - | { readonly kind: "manual" } + | { + readonly kind: "manual"; + /** Integration whose connect dialog registered this manual app, when the + * registration happened from within one. Lets the picker match a BYO app + * to its integration by recorded intent (exact) instead of guessing by + * root domain. Null for apps registered outside any integration context + * (and for legacy rows predating the stamp). */ + readonly integration?: IntegrationSlug | null; + } | { readonly kind: "dynamic_client_registration"; readonly integration?: IntegrationSlug | null; @@ -72,6 +80,7 @@ export type OAuthClientOrigin = export type CreateOAuthClientInput = OAuthClient & { readonly origin?: OAuthClientOrigin; + readonly originIssuer?: string | null; }; /** Metadata-only projection of a registered client for listing in the UI. @@ -134,6 +143,8 @@ export interface OAuthProbeInput { } export interface OAuthProbeResult { + /** RFC 8414 authorization-server issuer. Used to key DCR clients per AS. */ + readonly issuer?: string | null; readonly authorizationUrl: string; readonly tokenUrl: string; /** RFC 8707 resource indicator discovered from protected-resource metadata. @@ -157,6 +168,8 @@ export interface OAuthProbeResult { export interface RegisterDynamicClientInput { readonly owner: Owner; readonly slug: OAuthClientSlug; + /** RFC 8414 authorization-server issuer, when discovered before DCR. */ + readonly issuer?: string | null; /** RFC 7591 registration endpoint advertised by the authorization server. */ readonly registrationEndpoint: string; readonly authorizationUrl: string; diff --git a/packages/core/sdk/src/oauth-gc.test.ts b/packages/core/sdk/src/oauth-gc.test.ts new file mode 100644 index 000000000..0d7a0b19f --- /dev/null +++ b/packages/core/sdk/src/oauth-gc.test.ts @@ -0,0 +1,212 @@ +import { describe, expect, test } from "@effect/vitest"; + +import { + classifyOAuthClientGc, + isDcrClassifiedRow, + registrableOriginOfUrl, + type OAuthClientGcRow, +} from "./oauth-gc"; + +// The GC decision matrix (issue #1120, Part C). These lock the conjunctive, +// fail-safe deletion predicate the local libSQL migration and the cloud code +// migration both encode: delete ⇔ classified-DCR AND zero referencing +// connections. Everything else is kept. + +const explicitDcr: OAuthClientGcRow = { + slug: "dcr-cloudflare-com", + grant: "authorization_code", + resource: "https://cloudflare.example/mcp", + origin_kind: "dynamic_client_registration", +}; + +const legacyDcr: OAuthClientGcRow = { + // Pre-Part-A row: null origin_kind, MCP-shaped slug + resource. + slug: "cloudflare-mcp-2", + grant: "authorization_code", + resource: "https://cloudflare.example/mcp", + origin_kind: null, +}; + +const manual: OAuthClientGcRow = { + slug: "my-github-app", + grant: "authorization_code", + resource: null, + origin_kind: "manual", +}; + +describe("isDcrClassifiedRow", () => { + test("classifies an explicit-origin DCR row as DCR", () => { + expect(isDcrClassifiedRow(explicitDcr)).toBe(true); + }); + + test("classifies a legacy null-origin MCP-shaped row as DCR via the heuristic", () => { + expect(isDcrClassifiedRow(legacyDcr)).toBe(true); + expect(isDcrClassifiedRow({ ...legacyDcr, slug: "cloudflare-mcp" })).toBe(true); + expect( + isDcrClassifiedRow({ + slug: "mcp", + grant: "authorization_code", + resource: "https://x.example/mcp?foo=1", + origin_kind: null, + }), + ).toBe(true); + }); + + test("an explicit manual row is never DCR, even if MCP-shaped", () => { + expect(isDcrClassifiedRow(manual)).toBe(false); + expect( + isDcrClassifiedRow({ + slug: "cloudflare-mcp", + grant: "authorization_code", + resource: "https://cloudflare.example/mcp", + origin_kind: "manual", + }), + ).toBe(false); + }); + + test("a null-origin row that is not MCP-shaped is NOT DCR (ambiguous stays manual)", () => { + // Non-MCP slug. + expect( + isDcrClassifiedRow({ + slug: "notion", + grant: "authorization_code", + resource: "https://notion.example/mcp", + origin_kind: null, + }), + ).toBe(false); + // MCP slug but non-MCP resource. + expect( + isDcrClassifiedRow({ + slug: "cloudflare-mcp", + grant: "authorization_code", + resource: "https://cloudflare.example/oauth", + origin_kind: null, + }), + ).toBe(false); + // MCP-shaped but wrong grant. + expect( + isDcrClassifiedRow({ + slug: "cloudflare-mcp", + grant: "client_credentials", + resource: "https://cloudflare.example/mcp", + origin_kind: null, + }), + ).toBe(false); + // Null resource can never match the resource arm of the heuristic. + expect( + isDcrClassifiedRow({ + slug: "cloudflare-mcp", + grant: "authorization_code", + resource: null, + origin_kind: null, + }), + ).toBe(false); + // A word merely containing "mcp" (not a bounded segment) does not match. + expect( + isDcrClassifiedRow({ + slug: "mcphaven", + grant: "authorization_code", + resource: "https://x.example/mcphaven", + origin_kind: null, + }), + ).toBe(false); + }); +}); + +describe("classifyOAuthClientGc decision matrix", () => { + test("DCR + orphaned (zero connections) → delete", () => { + expect(classifyOAuthClientGc(explicitDcr, 0)).toEqual({ + action: "delete", + reason: "dcr-orphaned", + }); + expect(classifyOAuthClientGc(legacyDcr, 0)).toEqual({ + action: "delete", + reason: "dcr-orphaned", + }); + }); + + test("DCR + referenced (>=1 connection) → keep", () => { + expect(classifyOAuthClientGc(explicitDcr, 1)).toEqual({ + action: "keep", + reason: "referenced", + }); + expect(classifyOAuthClientGc(legacyDcr, 3)).toEqual({ + action: "keep", + reason: "referenced", + }); + }); + + test("manual + orphaned → keep (a hand-registered app is never GC'd)", () => { + expect(classifyOAuthClientGc(manual, 0)).toEqual({ action: "keep", reason: "not-dcr" }); + }); + + test("manual + referenced → keep", () => { + expect(classifyOAuthClientGc(manual, 5)).toEqual({ action: "keep", reason: "not-dcr" }); + }); + + test("legacy-heuristic-classified DCR is treated exactly like an explicit DCR row", () => { + // Same (row-shape, count) inputs produce the same decision whether the DCR + // classification came from origin_kind or the heuristic. + expect(classifyOAuthClientGc(legacyDcr, 0).action).toBe( + classifyOAuthClientGc(explicitDcr, 0).action, + ); + expect(classifyOAuthClientGc(legacyDcr, 2).action).toBe( + classifyOAuthClientGc(explicitDcr, 2).action, + ); + }); + + test("idempotency: once a DCR row is referenced (kept), a re-run keeps it (never deletes)", () => { + // A second GC pass sees the same referenced DCR rows and the same manual + // rows; none flip to delete. (Orphaned DCR rows are gone after pass 1, so + // the only rows a second pass sees are keeps.) + for (const count of [1, 2, 10]) { + expect(classifyOAuthClientGc(explicitDcr, count).action).toBe("keep"); + expect(classifyOAuthClientGc(legacyDcr, count).action).toBe("keep"); + } + expect(classifyOAuthClientGc(manual, 0).action).toBe("keep"); + }); +}); + +describe("registrableOriginOfUrl (issuer backfill value)", () => { + test("collapses a subdomain token host to its registrable origin", () => { + expect(registrableOriginOfUrl("https://oauth.cloudflare.com/token")).toBe( + "https://cloudflare.com", + ); + expect(registrableOriginOfUrl("https://login.microsoftonline.com/common/oauth2/token")).toBe( + "https://microsoftonline.com", + ); + }); + + test("keeps an already-registrable host as-is", () => { + expect(registrableOriginOfUrl("https://github.com/login/oauth/access_token")).toBe( + "https://github.com", + ); + }); + + test("respects multi-label public suffixes via the full PSL", () => { + expect(registrableOriginOfUrl("https://api.foo.co.uk/token")).toBe("https://foo.co.uk"); + // Suffixes the old hardcoded 12-entry list missed: .co.in, .com.cn, .co.za. + // `accounts.example.co.in` must collapse to `example.co.in`, not `co.in`. + expect(registrableOriginOfUrl("https://accounts.example.co.in/token")).toBe( + "https://example.co.in", + ); + expect(registrableOriginOfUrl("https://oauth.example.com.cn/token")).toBe( + "https://example.com.cn", + ); + expect(registrableOriginOfUrl("https://login.example.co.za/token")).toBe( + "https://example.co.za", + ); + }); + + test("preserves scheme and port for loopback / dev token hosts", () => { + expect(registrableOriginOfUrl("http://127.0.0.1:8787/token")).toBe("http://127.0.0.1:8787"); + expect(registrableOriginOfUrl("http://localhost:4788/oauth/token")).toBe( + "http://localhost:4788", + ); + }); + + test("returns null for a non-URL token value", () => { + expect(registrableOriginOfUrl("not a url")).toBeNull(); + expect(registrableOriginOfUrl("")).toBeNull(); + }); +}); diff --git a/packages/core/sdk/src/oauth-gc.ts b/packages/core/sdk/src/oauth-gc.ts new file mode 100644 index 000000000..626a48772 --- /dev/null +++ b/packages/core/sdk/src/oauth-gc.ts @@ -0,0 +1,156 @@ +// --------------------------------------------------------------------------- +// Garbage collection + backfill for legacy Dynamic Client Registration (DCR) +// `oauth_client` rows (issue #1120, Part C). +// +// Old always-register behavior minted a fresh `oauth_client` per connect +// attempt ("cloudflare-mcp", "cloudflare-mcp-2", …). Parts A+B stopped the +// multiplication (per-AS reuse) and hid the rows from pickers, but the dead +// rows remain and legacy rows have `origin_issuer = NULL`, so the reuse lookup +// can't key on them. This module supplies the shared, testable predicates that +// both host paths (local libSQL boot migration, cloud Drizzle SQL migration) +// encode: +// +// 1. GC: delete a row that is classified DCR AND has ZERO referencing +// connections. The predicate is conjunctive and fail-safe — an ambiguous +// row (manual, or DCR but still referenced) is always KEPT. +// 2. Backfill: for a surviving DCR row with `origin_issuer IS NULL`, set it +// to the registrable origin of `token_url` so the per-AS reuse lookup can +// find it and mint no new duplicate. +// +// The predicates here are the SINGLE source of truth, imported (never +// re-encoded) by every call site: `oauth-service`'s `parseOAuthClientOrigin` +// calls `isDcrClassifiedRow` for the runtime reuse lookup, and BOTH GC +// migrations (the local libSQL boot migration and the cloud code migration) +// import `classifyOAuthClientGc` / `registrableOriginOfUrl` and decide in +// process rather than rewriting the logic as SQL. There is no parallel SQL copy +// to drift: changing a predicate here changes all three call sites at once. +// --------------------------------------------------------------------------- + +import { getDomain } from "tldts"; + +/** Parse a string into a URL, or null when it is not a valid absolute URL. */ +export const parseUrl = (value: string): URL | null => { + if (!URL.canParse(value)) return null; + return new URL(value); +}; + +/** Canonicalize a discovered issuer to `origin` + path with trailing slashes + * stripped (RFC 8414 issuers are compared without a trailing slash). Null for + * a blank or unparseable value. Part A's DCR-identity canonicalization. */ +export const canonicalIssuerUrl = (value: string | null | undefined): string | null => { + if (value == null) return null; + const trimmed = value.trim(); + if (trimmed.length === 0) return null; + const url = parseUrl(trimmed); + if (url === null) return null; + const path = url.pathname.replace(/\/+$/g, ""); + return path.length > 0 ? `${url.origin}${path}` : url.origin; +}; + +export const hostOfUrl = (value: string): string | null => + parseUrl(value)?.host.toLowerCase() ?? null; + +/** The registrable domain of a hostname (eTLD+1), backed by the full Public + * Suffix List via `tldts` (so `api.foo.co.uk` → `foo.co.uk`, `x.co.in` → + * `x.co.in`, etc., not a hardcoded suffix subset). `tldts.getDomain` returns + * null for hosts with no registrable domain (`localhost`, bare IPv4/IPv6, + * single-label hosts); those fall back to the raw lowercase hostname so + * local-dev token hosts still key on their exact host. This mirrors how + * `packages/react/src/plugins/use-effective-oauth-client.tsx` uses tldts. */ +export const registrableHostname = (hostname: string): string => { + const host = hostname.toLowerCase(); + return getDomain(host) ?? host; +}; + +/** The registrable host (incl. port when the hostname is registered verbatim) + * of a URL, or null when the value is not a URL. */ +export const registrableHostOfUrl = (value: string): string | null => { + const url = parseUrl(value); + if (url === null) return null; + const hostname = registrableHostname(url.hostname); + return hostname === url.hostname.toLowerCase() ? url.host.toLowerCase() : hostname; +}; + +/** The registrable ORIGIN of a URL: its scheme + the registrable host (dropping + * any subdomain). Used to backfill a legacy DCR row's `origin_issuer` from its + * `token_url`, so the per-AS reuse lookup can find it. Null when not a URL. + * + * Preserves scheme and port (a loopback `http://127.0.0.1:8787` token host + * keeps its port). Reuses `registrableHostname` — no duplicated eTLD logic. */ +export const registrableOriginOfUrl = (value: string): string | null => { + const url = parseUrl(value); + if (url === null) return null; + const registrable = registrableHostname(url.hostname); + // When the hostname collapses to a registrable domain, rebuild the origin + // from scheme + registrable host + original port; otherwise keep url.origin + // verbatim (covers localhost / IPv4 / already-registrable hosts, incl. port). + if (registrable === url.hostname.toLowerCase()) return url.origin; + const port = url.port.length > 0 ? `:${url.port}` : ""; + return `${url.protocol}//${registrable}${port}`; +}; + +/** The minimal shape the DCR classifier reads off an `oauth_client` row. Every + * field is optional/unknown so it applies equally to a fuma row, a raw SQL + * row, or a hand-built test fixture. */ +export interface OAuthClientGcRow { + readonly slug?: unknown; + readonly grant?: unknown; + readonly resource?: unknown; + readonly origin_kind?: unknown; +} + +const legacyMcpSlug = (slug: string): boolean => /(^|[-_])mcp($|[-_])/.test(slug); +const legacyMcpResource = (resource: string): boolean => /(^|\/)mcp($|[/?#])/.test(resource); + +/** + * Is this `oauth_client` row a Dynamic Client Registration client? + * + * True when EITHER: + * - it carries the explicit `origin_kind = 'dynamic_client_registration'` + * stamp (rows minted after Part A), OR + * - it is a legacy null-origin row that matches the MCP heuristic: an + * `authorization_code` grant whose slug and resource both look MCP-shaped + * (`…mcp…`). This is the same heuristic `parseOAuthClientOrigin` applies to + * classify pre-Part-A rows, kept in ONE place so the runtime and the GC + * migrations agree exactly. + * + * A row that does not match either arm is treated as manual and is NEVER a GC + * candidate — the conservative default that protects hand-registered apps. + */ +export const isDcrClassifiedRow = (row: OAuthClientGcRow): boolean => { + if (row.origin_kind === "dynamic_client_registration") return true; + if (row.origin_kind != null) return false; + if (row.grant !== "authorization_code") return false; + const slug = row.slug == null ? "" : String(row.slug); + const resource = row.resource == null ? "" : String(row.resource); + return legacyMcpSlug(slug) && legacyMcpResource(resource); +}; + +/** The GC decision for one `oauth_client` row. */ +export type OAuthClientGcDecision = + /** Classified DCR AND unreferenced — safe to delete. */ + | { readonly action: "delete"; readonly reason: "dcr-orphaned" } + /** Kept — either not DCR, or DCR but still referenced by a connection. */ + | { readonly action: "keep"; readonly reason: "not-dcr" | "referenced" }; + +/** + * Decide whether a single `oauth_client` row should be garbage-collected. + * + * The deletion predicate is strictly conjunctive and fail-safe: + * delete ⇔ isDcrClassifiedRow(row) AND referencingConnectionCount === 0 + * + * Anything else is kept. A manual app is kept even when orphaned (users delete + * their own apps deliberately); a DCR app is kept while any connection still + * references it (deleting it would break refresh/reconnect for that + * connection). `referencingConnectionCount` is the number of `connection` rows + * whose stored `(oauth_client_owner, oauth_client)` reference resolves to this + * row's `(owner, slug)`. + */ +export const classifyOAuthClientGc = ( + row: OAuthClientGcRow, + referencingConnectionCount: number, +): OAuthClientGcDecision => { + if (!isDcrClassifiedRow(row)) return { action: "keep", reason: "not-dcr" }; + if (referencingConnectionCount > 0) return { action: "keep", reason: "referenced" }; + return { action: "delete", reason: "dcr-orphaned" }; +}; diff --git a/packages/core/sdk/src/oauth-list-clients.test.ts b/packages/core/sdk/src/oauth-list-clients.test.ts index 4a04688c4..086513ce3 100644 --- a/packages/core/sdk/src/oauth-list-clients.test.ts +++ b/packages/core/sdk/src/oauth-list-clients.test.ts @@ -91,7 +91,9 @@ describe("oauth.listClients", () => { tokenUrl: "https://acme.test/token", resource: null, clientId: "org-client-id", - origin: { kind: "manual" }, + // Manual apps carry a nullable recorded-intent integration; a client + // created outside any integration dialog stamps null. + origin: { kind: "manual", integration: null }, }); expect(user!.owner).toBe("user"); expect(user!.grant).toBe("client_credentials"); diff --git a/packages/core/sdk/src/oauth-register-dynamic.test.ts b/packages/core/sdk/src/oauth-register-dynamic.test.ts index 6a83986cb..8b593b6e6 100644 --- a/packages/core/sdk/src/oauth-register-dynamic.test.ts +++ b/packages/core/sdk/src/oauth-register-dynamic.test.ts @@ -25,6 +25,18 @@ const TEMPLATE = AuthTemplateSlug.make("oauth"); const CLIENT = OAuthClientSlug.make("acme-dcr"); const FLOW_REDIRECT_URI = "https://localhost:5394/api/oauth/callback"; +const dcrSlugFor = (issuerUrl: string): OAuthClientSlug => + OAuthClientSlug.make( + `dcr-${new URL(issuerUrl).host + .toLowerCase() + .replace(/[^a-z0-9]+/g, "-") + .replace(/^-+|-+$/g, "")}`, + ); + +const registerRequestCount = ( + requests: readonly { readonly path: string; readonly method: string }[], +): number => requests.filter((r) => r.path === "/register" && r.method === "POST").length; + const oauthPlugin = definePlugin(() => ({ id: "acme" as const, storage: () => ({}), @@ -56,6 +68,7 @@ describe("oauth.registerDynamicClient", () => { // Probe surfaces the registration endpoint + advertised auth methods so // the caller knows a public client is allowed. const probe = yield* executor.oauth.probe({ url: server.mcpResourceUrl }); + expect(probe.issuer).toBe(server.issuerUrl); expect(probe.registrationEndpoint).toBe(server.registrationEndpoint); expect(probe.tokenEndpointAuthMethodsSupported).toContain("none"); expect(probe.resource).toBe(server.mcpResourceUrl); @@ -64,6 +77,7 @@ describe("oauth.registerDynamicClient", () => { const slug = yield* executor.oauth.registerDynamicClient({ owner: "org", slug: CLIENT, + issuer: probe.issuer, registrationEndpoint: probe.registrationEndpoint!, authorizationUrl: probe.authorizationUrl, tokenUrl: probe.tokenUrl, @@ -74,12 +88,13 @@ describe("oauth.registerDynamicClient", () => { redirectUri: FLOW_REDIRECT_URI, originIntegration: INTEG, }); - expect(String(slug)).toBe(String(CLIENT)); + const expectedSlug = dcrSlugFor(server.issuerUrl); + expect(String(slug)).toBe(String(expectedSlug)); // The minted client appears in listClients with a server-issued // client_id and NO secret ever projected. const clients = yield* executor.oauth.listClients(); - const minted = clients.find((c) => String(c.slug) === String(CLIENT)); + const minted = clients.find((c) => String(c.slug) === String(expectedSlug)); expect(minted).toBeDefined(); expect(minted!.owner).toBe("org"); expect(minted!.grant).toBe("authorization_code"); @@ -98,7 +113,7 @@ describe("oauth.registerDynamicClient", () => { // PKCE flow with NO client_secret on the token exchange. const started = yield* executor.oauth.start({ owner: "org", - client: CLIENT, + client: expectedSlug, clientOwner: "org", name: ConnectionName.make("main"), integration: INTEG, @@ -145,6 +160,310 @@ describe("oauth.registerDynamicClient", () => { ), ); + it.effect("reuses an existing DCR client for the same owner and authorization server", () => + Effect.scoped( + Effect.gen(function* () { + const server = yield* serveOAuthTestServer({ scopes: ["read"] }); + const { executor } = yield* makeTestWorkspaceHarness({ plugins }); + yield* executor.acme.seed(); + const probe = yield* executor.oauth.probe({ url: server.mcpResourceUrl }); + + const first = yield* executor.oauth.registerDynamicClient({ + owner: "org", + slug: OAuthClientSlug.make("first-attempt"), + issuer: probe.issuer, + registrationEndpoint: probe.registrationEndpoint!, + authorizationUrl: probe.authorizationUrl, + tokenUrl: probe.tokenUrl, + resource: probe.resource, + scopes: ["read"], + tokenEndpointAuthMethodsSupported: probe.tokenEndpointAuthMethodsSupported, + clientName: "Acme DCR", + redirectUri: FLOW_REDIRECT_URI, + originIntegration: INTEG, + }); + yield* server.clearRequests; + + const second = yield* executor.oauth.registerDynamicClient({ + owner: "org", + slug: OAuthClientSlug.make("second-attempt"), + issuer: probe.issuer, + registrationEndpoint: probe.registrationEndpoint!, + authorizationUrl: probe.authorizationUrl, + tokenUrl: probe.tokenUrl, + resource: probe.resource, + scopes: ["read"], + tokenEndpointAuthMethodsSupported: probe.tokenEndpointAuthMethodsSupported, + clientName: "Other integration DCR", + redirectUri: FLOW_REDIRECT_URI, + originIntegration: IntegrationSlug.make("other"), + }); + + expect(second).toBe(first); + const requests = yield* server.requests; + expect(registerRequestCount(requests)).toBe(0); + }), + ), + ); + + it.effect("does not reuse a DCR client across owners", () => + Effect.scoped( + Effect.gen(function* () { + const server = yield* serveOAuthTestServer({ scopes: ["read"] }); + const { executor } = yield* makeTestWorkspaceHarness({ plugins }); + yield* executor.acme.seed(); + const probe = yield* executor.oauth.probe({ url: server.mcpResourceUrl }); + + const orgSlug = yield* executor.oauth.registerDynamicClient({ + owner: "org", + slug: OAuthClientSlug.make("org-attempt"), + issuer: probe.issuer, + registrationEndpoint: probe.registrationEndpoint!, + authorizationUrl: probe.authorizationUrl, + tokenUrl: probe.tokenUrl, + resource: probe.resource, + scopes: ["read"], + tokenEndpointAuthMethodsSupported: probe.tokenEndpointAuthMethodsSupported, + clientName: "Org DCR", + redirectUri: FLOW_REDIRECT_URI, + originIntegration: INTEG, + }); + yield* server.clearRequests; + + const userSlug = yield* executor.oauth.registerDynamicClient({ + owner: "user", + slug: OAuthClientSlug.make("user-attempt"), + issuer: probe.issuer, + registrationEndpoint: probe.registrationEndpoint!, + authorizationUrl: probe.authorizationUrl, + tokenUrl: probe.tokenUrl, + resource: probe.resource, + scopes: ["read"], + tokenEndpointAuthMethodsSupported: probe.tokenEndpointAuthMethodsSupported, + clientName: "User DCR", + redirectUri: FLOW_REDIRECT_URI, + originIntegration: INTEG, + }); + + expect(String(userSlug)).toBe(String(orgSlug)); + const requests = yield* server.requests; + expect(registerRequestCount(requests)).toBe(1); + }), + ), + ); + + it.effect("reuses legacy DCR-looking rows with no stored issuer by token host", () => + Effect.scoped( + Effect.gen(function* () { + const server = yield* serveOAuthTestServer({ scopes: ["read"] }); + const { config, executor } = yield* makeTestWorkspaceHarness({ plugins }); + yield* executor.acme.seed(); + const probe = yield* executor.oauth.probe({ url: server.mcpResourceUrl }); + const legacySlug = OAuthClientSlug.make("cloudflare-mcp"); + + yield* executor.oauth.createClient({ + owner: "org", + slug: legacySlug, + authorizationUrl: probe.authorizationUrl, + tokenUrl: probe.tokenUrl, + resource: server.mcpResourceUrl, + grant: "authorization_code", + clientId: "legacy-dcr-client", + clientSecret: "", + }); + yield* Effect.promise(() => + config.db.updateMany("oauth_client", { + where: (b) => b("slug", "=", String(legacySlug)), + set: { origin_kind: null, origin_integration: null, origin_issuer: null }, + }), + ); + yield* server.clearRequests; + + const reused = yield* executor.oauth.registerDynamicClient({ + owner: "org", + slug: OAuthClientSlug.make("new-attempt"), + issuer: probe.issuer, + registrationEndpoint: probe.registrationEndpoint!, + authorizationUrl: probe.authorizationUrl, + tokenUrl: probe.tokenUrl, + resource: server.mcpResourceUrl, + scopes: ["read"], + tokenEndpointAuthMethodsSupported: probe.tokenEndpointAuthMethodsSupported, + clientName: "Acme DCR", + redirectUri: FLOW_REDIRECT_URI, + originIntegration: INTEG, + }); + + expect(reused).toBe(legacySlug); + const requests = yield* server.requests; + expect(registerRequestCount(requests)).toBe(0); + }), + ), + ); + + it.effect("uses resource to distinguish DCR clients only after an issuer already differs", () => + Effect.scoped( + Effect.gen(function* () { + const server = yield* serveOAuthTestServer({ scopes: ["read"] }); + const { executor } = yield* makeTestWorkspaceHarness({ plugins }); + yield* executor.acme.seed(); + const probe = yield* executor.oauth.probe({ url: server.mcpResourceUrl }); + const resourceA = `${server.issuerUrl}/mcp/a`; + const resourceB = `${server.issuerUrl}/mcp/b`; + + const first = yield* executor.oauth.registerDynamicClient({ + owner: "org", + slug: OAuthClientSlug.make("first-resource"), + issuer: probe.issuer, + registrationEndpoint: probe.registrationEndpoint!, + authorizationUrl: probe.authorizationUrl, + tokenUrl: probe.tokenUrl, + resource: resourceA, + scopes: ["read"], + tokenEndpointAuthMethodsSupported: probe.tokenEndpointAuthMethodsSupported, + clientName: "Resource A", + redirectUri: FLOW_REDIRECT_URI, + originIntegration: INTEG, + }); + expect(String(first)).toBe(String(dcrSlugFor(server.issuerUrl))); + + const second = yield* executor.oauth.registerDynamicClient({ + owner: "org", + slug: OAuthClientSlug.make("second-resource"), + issuer: probe.issuer, + registrationEndpoint: probe.registrationEndpoint!, + authorizationUrl: probe.authorizationUrl, + tokenUrl: probe.tokenUrl, + resource: resourceB, + scopes: ["read"], + tokenEndpointAuthMethodsSupported: probe.tokenEndpointAuthMethodsSupported, + clientName: "Resource B", + redirectUri: FLOW_REDIRECT_URI, + originIntegration: INTEG, + }); + expect(String(second)).not.toBe(String(first)); + expect(String(second)).toContain(String(first)); + yield* server.clearRequests; + + const third = yield* executor.oauth.registerDynamicClient({ + owner: "org", + slug: OAuthClientSlug.make("third-resource"), + issuer: probe.issuer, + registrationEndpoint: probe.registrationEndpoint!, + authorizationUrl: probe.authorizationUrl, + tokenUrl: probe.tokenUrl, + resource: resourceB, + scopes: ["read"], + tokenEndpointAuthMethodsSupported: probe.tokenEndpointAuthMethodsSupported, + clientName: "Resource B again", + redirectUri: FLOW_REDIRECT_URI, + originIntegration: IntegrationSlug.make("other"), + }); + + expect(third).toBe(second); + const requests = yield* server.requests; + expect(registerRequestCount(requests)).toBe(0); + }), + ), + ); + + it.effect("does not reuse a resource-scoped DCR client for a resource-less request", () => + Effect.scoped( + Effect.gen(function* () { + const server = yield* serveOAuthTestServer({ scopes: ["read"] }); + const { executor } = yield* makeTestWorkspaceHarness({ plugins }); + yield* executor.acme.seed(); + const probe = yield* executor.oauth.probe({ url: server.mcpResourceUrl }); + const resourceA = `${server.issuerUrl}/mcp/a`; + const resourceB = `${server.issuerUrl}/mcp/b`; + + // Two resource-scoped clients for the same authorization server. The first + // takes the base `dcr-` slug; the second gets a resource-suffixed one. + const scopedA = yield* executor.oauth.registerDynamicClient({ + owner: "org", + slug: OAuthClientSlug.make("scoped-a"), + issuer: probe.issuer, + registrationEndpoint: probe.registrationEndpoint!, + authorizationUrl: probe.authorizationUrl, + tokenUrl: probe.tokenUrl, + resource: resourceA, + scopes: ["read"], + tokenEndpointAuthMethodsSupported: probe.tokenEndpointAuthMethodsSupported, + clientName: "Resource A", + redirectUri: FLOW_REDIRECT_URI, + originIntegration: INTEG, + }); + const scopedB = yield* executor.oauth.registerDynamicClient({ + owner: "org", + slug: OAuthClientSlug.make("scoped-b"), + issuer: probe.issuer, + registrationEndpoint: probe.registrationEndpoint!, + authorizationUrl: probe.authorizationUrl, + tokenUrl: probe.tokenUrl, + resource: resourceB, + scopes: ["read"], + tokenEndpointAuthMethodsSupported: probe.tokenEndpointAuthMethodsSupported, + clientName: "Resource B", + redirectUri: FLOW_REDIRECT_URI, + originIntegration: INTEG, + }); + expect(String(scopedB)).not.toBe(String(scopedA)); + yield* server.clearRequests; + + // A resource-LESS request for the same owner + issuer must NOT borrow + // either resource-scoped client (their tokens are bound to a resource): + // it registers a fresh resource-less client. + const resourceLess = yield* executor.oauth.registerDynamicClient({ + owner: "org", + slug: OAuthClientSlug.make("resource-less"), + issuer: probe.issuer, + registrationEndpoint: probe.registrationEndpoint!, + authorizationUrl: probe.authorizationUrl, + tokenUrl: probe.tokenUrl, + resource: null, + scopes: ["read"], + tokenEndpointAuthMethodsSupported: probe.tokenEndpointAuthMethodsSupported, + clientName: "Resource-less", + redirectUri: FLOW_REDIRECT_URI, + originIntegration: INTEG, + }); + expect(String(resourceLess)).not.toBe(String(scopedA)); + expect(String(resourceLess)).not.toBe(String(scopedB)); + const requests = yield* server.requests; + expect(registerRequestCount(requests)).toBe(1); + + // The minted resource-less client is stored with a null resource, and the + // two resource-scoped clients still exist (nothing was clobbered). + const clients = yield* executor.oauth.listClients(); + const minted = clients.find((c) => String(c.slug) === String(resourceLess)); + expect(minted).toBeDefined(); + expect(minted!.resource ?? null).toBeNull(); + const slugs = clients.map((c) => String(c.slug)); + expect(slugs).toContain(String(scopedA)); + expect(slugs).toContain(String(scopedB)); + + // A SECOND resource-less request now reuses the resource-less client. + yield* server.clearRequests; + const reused = yield* executor.oauth.registerDynamicClient({ + owner: "org", + slug: OAuthClientSlug.make("resource-less-again"), + issuer: probe.issuer, + registrationEndpoint: probe.registrationEndpoint!, + authorizationUrl: probe.authorizationUrl, + tokenUrl: probe.tokenUrl, + resource: null, + scopes: ["read"], + tokenEndpointAuthMethodsSupported: probe.tokenEndpointAuthMethodsSupported, + clientName: "Resource-less again", + redirectUri: FLOW_REDIRECT_URI, + originIntegration: IntegrationSlug.make("other"), + }); + expect(String(reused)).toBe(String(resourceLess)); + expect(registerRequestCount(yield* server.requests)).toBe(0); + }), + ), + ); + // Regression: issue #770. Vercel (and other RFC 8252-strict servers) only // approve loopback redirect URIs for anonymous DCR, so a hosted/tailnet/LAN // origin trips `invalid_redirect_uri`. The failure must explain the loopback diff --git a/packages/core/sdk/src/oauth-service.ts b/packages/core/sdk/src/oauth-service.ts index 8f49ae945..86e216e71 100644 --- a/packages/core/sdk/src/oauth-service.ts +++ b/packages/core/sdk/src/oauth-service.ts @@ -70,6 +70,13 @@ import { type OAuthEndpointUrlPolicy, } from "./oauth-helpers"; import { OAUTH2_SESSION_TTL_MS, encodeOAuthCallbackState } from "./oauth"; +import { + canonicalIssuerUrl, + hostOfUrl, + isDcrClassifiedRow, + parseUrl, + registrableHostOfUrl, +} from "./oauth-gc"; /** Connection-minting input for the OAuth flow — extends a connection create * with the OAuth lifecycle fields (client slug, refresh material, expiry, @@ -239,6 +246,68 @@ const clientOwnerFromPayload = (payload: unknown): Owner | null => { const parseGrant = (grant: unknown): OAuthGrant | null => grant === "client_credentials" || grant === "authorization_code" ? grant : null; +const canonicalDcrIssuer = ( + issuer: string | null | undefined, + registrationEndpoint: string, +): string | null => { + const discovered = canonicalIssuerUrl(issuer); + if (discovered !== null) return discovered; + const endpoint = parseUrl(registrationEndpoint); + return endpoint === null ? null : endpoint.origin; +}; + +const issuerOrigin = (issuer: string): string | null => parseUrl(issuer)?.origin ?? null; + +const issuerIsOriginOnly = (issuer: string): boolean => issuerOrigin(issuer) === issuer; + +const dcrIssuerMatches = (rowIssuer: string, inputIssuer: string | null): boolean => + inputIssuer !== null && + (rowIssuer === inputIssuer || + (issuerIsOriginOnly(inputIssuer) && issuerOrigin(rowIssuer) === inputIssuer)); + +const slugifyOAuthKey = (value: string): string => + value + .toLowerCase() + .replace(/[^a-z0-9]+/g, "-") + .replace(/^-+|-+$/g, ""); + +const shortStableHash = (value: string): string => { + let hash = 0x811c9dc5; + for (let i = 0; i < value.length; i += 1) { + hash ^= value.charCodeAt(i); + hash = Math.imul(hash, 0x01000193); + } + return (hash >>> 0).toString(36); +}; + +const dcrClientSlug = ( + issuer: string | null, + resource: string | null, + fallback: OAuthClientSlug, +): OAuthClientSlug => { + if (issuer === null) return fallback; + const issuerHost = hostOfUrl(issuer); + if (issuerHost === null) return fallback; + const base = `dcr-${slugifyOAuthKey(issuerHost) || "authorization-server"}`; + if (resource === null) return OAuthClientSlug.make(base); + const resourceUrl = parseUrl(resource); + const resourceSource = + resourceUrl === null ? resource : `${resourceUrl.host}${resourceUrl.pathname}`; + const resourcePart = slugifyOAuthKey(resourceSource).slice(0, 60) || "resource"; + return OAuthClientSlug.make(`${base}-${resourcePart}-${shortStableHash(resource)}`.slice(0, 240)); +}; + +/** Dedupe a freshly-minted DCR slug against slugs already held by an owner's + * DCR candidates, appending `-2`, `-3`, … so a new client never collides with + * (and clobbers, via createClient's delete-then-create) an existing one. */ +const uniqueDcrSlug = (slug: OAuthClientSlug, taken: ReadonlySet): OAuthClientSlug => { + const base = String(slug); + if (!taken.has(base)) return slug; + let suffix = 2; + while (taken.has(`${base}-${suffix}`)) suffix += 1; + return OAuthClientSlug.make(`${base}-${suffix}`); +}; + const parseOAuthClientOrigin = (row: { readonly slug?: unknown; readonly grant?: unknown; @@ -246,26 +315,27 @@ const parseOAuthClientOrigin = (row: { readonly origin_kind?: unknown; readonly origin_integration?: unknown; }): OAuthClientOrigin => { - if (row.origin_kind === "dynamic_client_registration") { + // Shared DCR classification (explicit origin_kind OR the legacy MCP-shaped + // heuristic) lives in `oauth-gc` so the runtime and the GC/backfill + // migrations agree exactly on what counts as a DCR row. + if (!isDcrClassifiedRow(row)) { return { - kind: "dynamic_client_registration", + kind: "manual", integration: row.origin_integration == null ? null : IntegrationSlug.make(String(row.origin_integration)), }; } - const slug = row.slug == null ? "" : String(row.slug); - const resource = row.resource == null ? "" : String(row.resource); - if ( - row.origin_kind == null && - row.grant === "authorization_code" && - /(^|[-_])mcp($|[-_])/.test(slug) && - /(^|\/)mcp($|[/?#])/.test(resource) - ) { - return { kind: "dynamic_client_registration", integration: null }; - } - return { kind: "manual" }; + // An explicit-origin DCR row carries its requesting integration; a + // heuristic-classified legacy row (null origin_kind) has none. + return { + kind: "dynamic_client_registration", + integration: + row.origin_kind === "dynamic_client_registration" && row.origin_integration != null + ? IntegrationSlug.make(String(row.origin_integration)) + : null, + }; }; interface LoadedOAuthClient { @@ -512,11 +582,15 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { client_secret_item_id: clientSecretItemIdValue, resource: input.resource ?? null, origin_kind: input.origin?.kind ?? "manual", + // Recorded intent, kept for BOTH origins: a manual app registered from + // an integration's dialog stamps its integration so the picker can + // match it exactly, the same way a DCR client records the integration + // that requested it. origin_integration: + input.origin?.integration == null ? null : String(input.origin.integration), + origin_issuer: input.origin?.kind === "dynamic_client_registration" - ? input.origin.integration == null - ? null - : String(input.origin.integration) + ? (canonicalIssuerUrl(input.originIssuer) ?? null) : null, created_at: now, }), @@ -578,10 +652,128 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { ? "none" : "client_secret_post"; + type DcrReuseCandidate = { + readonly slug: OAuthClientSlug; + readonly resource: string | null; + }; + + // `oauth_client.created_at` is a date column that surfaces as a Date, an ISO + // string, or an epoch number depending on the storage backend. Normalize to + // epoch ms for deterministic oldest-first ordering; an unparseable/missing + // value sorts as 0 (oldest), so slug then breaks the tie stably. + const candidateCreatedAt = (value: unknown): number => { + if (value instanceof Date) { + const ms = value.getTime(); + return Number.isFinite(ms) ? ms : 0; + } + if (typeof value === "number") return Number.isFinite(value) ? value : 0; + if (typeof value === "string") { + const ms = Date.parse(value); + return Number.isFinite(ms) ? ms : 0; + } + return 0; + }; + + const dcrCandidatesForIssuer = ( + owner: Owner, + issuer: string | null, + tokenUrl: string, + ): Effect.Effect => + deps.fuma + .use("oauth_client.findMany", (db) => + looseDb(db).findMany("oauth_client", { + where: (b: any) => b("owner", "=", owner), + }), + ) + .pipe( + Effect.map((rows) => { + const inputTokenHost = registrableHostOfUrl(tokenUrl); + const matches = rows.flatMap( + (row): readonly (DcrReuseCandidate & { readonly createdAt: number })[] => { + if (parseOAuthClientOrigin(row).kind !== "dynamic_client_registration") return []; + const rowIssuer = + row.origin_issuer == null ? null : canonicalIssuerUrl(String(row.origin_issuer)); + const issuerMatches = + rowIssuer !== null + ? dcrIssuerMatches(rowIssuer, issuer) + : inputTokenHost !== null && + registrableHostOfUrl(String(row.token_url)) === inputTokenHost; + if (!issuerMatches) return []; + return [ + { + slug: OAuthClientSlug.make(String(row.slug)), + resource: row.resource == null ? null : String(row.resource), + createdAt: candidateCreatedAt(row.created_at), + }, + ]; + }, + ); + // Deterministic reuse order: oldest first, slug as a stable tiebreak + // when timestamps collide or are missing. Without this, which of + // several live duplicates sharing an (owner, issuer) gets reused is + // whatever order the storage backend returned rows in — the reuse + // pick must be stable across boots and backends. + return [...matches] + .sort( + (a, b) => + a.createdAt - b.createdAt || (a.slug < b.slug ? -1 : a.slug > b.slug ? 1 : 0), + ) + .map(({ slug, resource }): DcrReuseCandidate => ({ slug, resource })); + }), + ); + + const decideDcrClientReuse = ( + input: RegisterDynamicClientInput, + issuer: string | null, + ): Effect.Effect< + { + readonly existingSlug: OAuthClientSlug | null; + readonly registrationSlug: OAuthClientSlug; + }, + StorageFailure + > => + Effect.gen(function* () { + const candidates = yield* dcrCandidatesForIssuer(input.owner, issuer, input.tokenUrl); + const resource = input.resource ?? null; + if (resource !== null) { + const matchingResource = candidates.find((client) => client.resource === resource); + if (matchingResource) { + return { existingSlug: matchingResource.slug, registrationSlug: matchingResource.slug }; + } + const slug = dcrClientSlug(issuer, candidates.length > 0 ? resource : null, input.slug); + return { + existingSlug: null, + registrationSlug: slug, + }; + } + + // Resource-less request: only reuse a resource-LESS candidate. A client + // minted for a specific RFC 8707 resource must NOT be reused for a + // resource-less flow (its tokens are bound to that resource), so when only + // resource-scoped candidates exist we register a fresh resource-less client + // rather than silently borrowing one (the old `?? candidates[0]` bug). + const reusable = candidates.find((client) => client.resource === null); + if (reusable) return { existingSlug: reusable.slug, registrationSlug: reusable.slug }; + // Fresh resource-less client. Its slug is the bare `dcr-` base, but + // the FIRST resource-scoped registration for an issuer also takes that base + // (dcrClientSlug only suffixes once candidates exist). `createClient` + // deletes any row with a colliding (owner, slug) first, so reusing the base + // here would silently clobber that resource-scoped client. Dedupe against + // the existing candidate slugs so the resource-less client keeps its own row. + const takenSlugs = new Set(candidates.map((client) => String(client.slug))); + const slug = uniqueDcrSlug(dcrClientSlug(issuer, null, input.slug), takenSlugs); + return { existingSlug: null, registrationSlug: slug }; + }); + const registerDynamicClient = ( input: RegisterDynamicClientInput, ): Effect.Effect => Effect.gen(function* () { + const issuer = canonicalDcrIssuer(input.issuer, input.registrationEndpoint); + const reuse = yield* decideDcrClientReuse(input, issuer); + if (reuse.existingSlug !== null) return reuse.existingSlug; + + const slug = reuse.registrationSlug; const flowRedirectUri = input.redirectUri ?? redirectUri; // DCR registers our callback as the client's redirect_uri — fail loudly // if the executor has none rather than registering a localhost URL. @@ -633,7 +825,7 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { // stored client carries no scope set (the integration drives requests). yield* createClient({ owner: input.owner, - slug: input.slug, + slug, authorizationUrl: input.authorizationUrl, tokenUrl: input.tokenUrl, resource: input.resource ?? null, @@ -644,8 +836,9 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { kind: "dynamic_client_registration", integration: input.originIntegration ?? null, }, + originIssuer: issuer, }); - return input.slug; + return slug; }); // ----------------------------------------------------------------------- @@ -1158,6 +1351,7 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { }); } return { + issuer: as.metadata.issuer, authorizationUrl: as.metadata.authorization_endpoint, tokenUrl: as.metadata.token_endpoint, resource: resource?.metadata.resource ?? null, diff --git a/packages/core/sdk/src/shared.ts b/packages/core/sdk/src/shared.ts index b4a543b1e..82f262973 100644 --- a/packages/core/sdk/src/shared.ts +++ b/packages/core/sdk/src/shared.ts @@ -110,6 +110,7 @@ export { type OAuthGrant, type OAuthAuthentication, type OAuthClient, + type OAuthClientOrigin, type OAuthClientSummary, type CreateOAuthClientInput, type RegisterDynamicClientInput, diff --git a/packages/core/sdk/src/sqlite-oauth-client-gc-migration.test.ts b/packages/core/sdk/src/sqlite-oauth-client-gc-migration.test.ts new file mode 100644 index 000000000..3d54acbff --- /dev/null +++ b/packages/core/sdk/src/sqlite-oauth-client-gc-migration.test.ts @@ -0,0 +1,218 @@ +import { describe, expect, it } from "@effect/vitest"; +import { Effect } from "effect"; + +import { collectTables } from "./executor"; +import { createSqliteTestFumaDb, type SqliteTestFumaDb } from "./sqlite-test-db"; +import { runSqliteOAuthClientGcMigration } from "./sqlite-oauth-client-gc-migration"; + +// Integration coverage for the local libSQL GC + backfill migration against a +// real SQLite schema (issue #1120, Part C). Complements the pure decision-matrix +// tests in oauth-gc.test.ts by exercising the actual SQL: seeded rows in, the +// migration runs, surviving rows asserted. + +const TENANT = "t1"; +const OWNER = "org"; +const SUBJECT = "s1"; + +const withDb = (body: (db: SqliteTestFumaDb) => Promise): Promise => + Effect.runPromise( + Effect.acquireUseRelease( + Effect.promise(() => createSqliteTestFumaDb({ tables: collectTables() })), + (db) => Effect.promise(() => body(db)), + (db) => Effect.promise(() => db.close()), + ), + ); + +const insertOAuthClient = ( + db: SqliteTestFumaDb, + row: { + readonly slug: string; + readonly tokenUrl: string; + readonly grant?: string; + readonly resource?: string | null; + readonly originKind?: string | null; + readonly originIssuer?: string | null; + }, +): Promise => + db.client.execute({ + sql: `INSERT INTO oauth_client + (row_id, tenant, owner, subject, slug, authorization_url, token_url, "grant", + client_id, resource, origin_kind, origin_issuer, created_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, + args: [ + `oc_${row.slug}`, + TENANT, + OWNER, + SUBJECT, + row.slug, + "https://as.example/authorize", + row.tokenUrl, + row.grant ?? "authorization_code", + `client-${row.slug}`, + row.resource ?? null, + row.originKind ?? null, + row.originIssuer ?? null, + Date.now(), + ], + }); + +const insertConnection = ( + db: SqliteTestFumaDb, + connectionName: string, + clientSlug: string, +): Promise => + db.client.execute({ + sql: `INSERT INTO connection + (row_id, tenant, owner, subject, integration, name, template, provider, item_ids, + oauth_client, oauth_client_owner, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, + args: [ + `cn_${connectionName}`, + TENANT, + OWNER, + SUBJECT, + "acme", + connectionName, + "oauth", + "memory", + "{}", + clientSlug, + OWNER, + Date.now(), + Date.now(), + ], + }); + +const slugs = async (db: SqliteTestFumaDb): Promise => { + const result = await db.client.execute("SELECT slug FROM oauth_client ORDER BY slug"); + return result.rows.map((r) => String(r.slug)); +}; + +const issuerOf = async (db: SqliteTestFumaDb, slug: string): Promise => { + const result = await db.client.execute({ + sql: "SELECT origin_issuer FROM oauth_client WHERE slug = ?", + args: [slug], + }); + const value = result.rows[0]?.origin_issuer; + return value == null ? null : String(value); +}; + +describe("runSqliteOAuthClientGcMigration", () => { + it.effect("deletes orphaned DCR rows, keeps manual + referenced rows, backfills survivors", () => + Effect.gen(function* () { + const outcome = yield* Effect.promise(() => + withDb(async (db) => { + // Orphaned explicit DCR → delete. + await insertOAuthClient(db, { + slug: "dcr-cloudflare-com", + tokenUrl: "https://oauth.cloudflare.com/token", + resource: "https://cloudflare.example/mcp", + originKind: "dynamic_client_registration", + originIssuer: "https://cloudflare.com", + }); + // Orphaned legacy heuristic DCR (null origin) → delete. + await insertOAuthClient(db, { + slug: "cloudflare-mcp-2", + tokenUrl: "https://oauth.cloudflare.com/token", + resource: "https://cloudflare.example/mcp", + originKind: null, + }); + // Referenced legacy DCR (has a connection) → keep + backfill issuer. + await insertOAuthClient(db, { + slug: "cloudflare-mcp", + tokenUrl: "https://oauth.cloudflare.com/token", + resource: "https://cloudflare.example/mcp", + originKind: null, + originIssuer: null, + }); + await insertConnection(db, "cf-main", "cloudflare-mcp"); + // Orphaned manual app → keep (never GC a hand-registered app). + await insertOAuthClient(db, { + slug: "my-github-app", + tokenUrl: "https://github.com/login/oauth/access_token", + resource: null, + originKind: "manual", + }); + + const applied = await Effect.runPromise(runSqliteOAuthClientGcMigration(db.client)); + const surviving = await slugs(db); + const backfilled = await issuerOf(db, "cloudflare-mcp"); + return { applied, surviving, backfilled }; + }), + ); + + expect(outcome.applied).toEqual({ deleted: 2, backfilled: 1 }); + // Both orphaned DCR rows gone; the referenced DCR row and the manual app + // survive. + expect(outcome.surviving).toEqual(["cloudflare-mcp", "my-github-app"]); + // The surviving legacy DCR row got its issuer backfilled from token_url's + // registrable origin, so the per-AS reuse lookup can now key on it. + expect(outcome.backfilled).toBe("https://cloudflare.com"); + }), + ); + + it.effect("is idempotent: a second run deletes nothing more and re-backfills nothing", () => + Effect.gen(function* () { + const outcome = yield* Effect.promise(() => + withDb(async (db) => { + await insertOAuthClient(db, { + slug: "cloudflare-mcp-2", + tokenUrl: "https://oauth.cloudflare.com/token", + resource: "https://cloudflare.example/mcp", + originKind: null, + }); + await insertOAuthClient(db, { + slug: "cloudflare-mcp", + tokenUrl: "https://oauth.cloudflare.com/token", + resource: "https://cloudflare.example/mcp", + originKind: null, + originIssuer: null, + }); + await insertConnection(db, "cf-main", "cloudflare-mcp"); + + const first = await Effect.runPromise(runSqliteOAuthClientGcMigration(db.client)); + const afterFirst = await slugs(db); + const second = await Effect.runPromise(runSqliteOAuthClientGcMigration(db.client)); + const afterSecond = await slugs(db); + return { first, afterFirst, second, afterSecond }; + }), + ); + + expect(outcome.first).toEqual({ deleted: 1, backfilled: 1 }); + // Second pass is a no-op: the orphan is already gone and the survivor's + // issuer is already set (so it is no longer a null-issuer candidate). + expect(outcome.second).toEqual({ deleted: 0, backfilled: 0 }); + expect(outcome.afterSecond).toEqual(outcome.afterFirst); + expect(outcome.afterSecond).toEqual(["cloudflare-mcp"]); + }), + ); + + it.effect("never deletes a manual app even when it is orphaned and MCP-shaped", () => + Effect.gen(function* () { + const surviving = yield* Effect.promise(() => + withDb(async (db) => { + // Explicit manual origin_kind must shield even an MCP-shaped slug from + // the heuristic. + await insertOAuthClient(db, { + slug: "cloudflare-mcp", + tokenUrl: "https://oauth.cloudflare.com/token", + resource: "https://cloudflare.example/mcp", + originKind: "manual", + }); + await Effect.runPromise(runSqliteOAuthClientGcMigration(db.client)); + return slugs(db); + }), + ); + expect(surviving).toEqual(["cloudflare-mcp"]); + }), + ); + + it.effect("no-op on an empty oauth_client table", () => + Effect.gen(function* () { + const applied = yield* Effect.promise(() => + withDb((db) => Effect.runPromise(runSqliteOAuthClientGcMigration(db.client))), + ); + expect(applied).toEqual({ deleted: 0, backfilled: 0 }); + }), + ); +}); diff --git a/packages/core/sdk/src/sqlite-oauth-client-gc-migration.ts b/packages/core/sdk/src/sqlite-oauth-client-gc-migration.ts new file mode 100644 index 000000000..9f53a5991 --- /dev/null +++ b/packages/core/sdk/src/sqlite-oauth-client-gc-migration.ts @@ -0,0 +1,142 @@ +// --------------------------------------------------------------------------- +// libSQL boot migration: GC dead DCR `oauth_client` rows + backfill the +// surviving ones' `origin_issuer` (issue #1120, Part C). +// +// This is the local/self-host arm of the cleanup (the cloud arm is a numbered +// Drizzle SQL migration). It runs once through the stamped `data_migration` +// ledger (see sqlite-data-migrations.ts). The decision matrix is NOT re-encoded +// as SQL — it reads the rows and applies the shared `classifyOAuthClientGc` / +// `isDcrClassifiedRow` predicates in-process, then issues targeted DELETE/UPDATE +// by primary key. Rewriting the DCR heuristic as SQLite LIKE/GLOB would be a +// second, drift-prone source of truth for a query that DELETES user data; the +// oauth_client table is tiny (a handful of rows per install), so reading it in +// full at boot is cheap. Idempotent: a second run finds no orphaned DCR rows +// and no null-issuer DCR survivors, so it is a no-op (and the ledger skips it +// anyway once stamped). +// --------------------------------------------------------------------------- + +import { Effect } from "effect"; + +import { + DataMigrationError, + type SqliteDataMigration, + type SqliteDataMigrationClient, +} from "./sqlite-data-migrations"; +import { classifyOAuthClientGc, isDcrClassifiedRow, registrableOriginOfUrl } from "./oauth-gc"; + +const MIGRATION_NAME = "2026-07-02-gc-dead-dcr-oauth-clients"; + +const execute = ( + client: SqliteDataMigrationClient, + stmt: string | { readonly sql: string; readonly args: readonly unknown[] }, +) => + Effect.tryPromise({ + try: () => client.execute(stmt), + catch: (cause) => new DataMigrationError({ migration: MIGRATION_NAME, cause }), + }); + +const tableExists = ( + client: SqliteDataMigrationClient, + table: string, +): Effect.Effect => + execute(client, { + sql: "SELECT name FROM sqlite_master WHERE type = 'table' AND name = ?", + args: [table], + }).pipe(Effect.map((result) => result.rows.length > 0)); + +/** + * GC + backfill the `oauth_client` table on a libSQL/SQLite database. + * + * Returns the number of rows deleted and backfilled. Transactionally wrapped + * (BEGIN … COMMIT, ROLLBACK on error) so a mid-run failure leaves the table + * untouched and the (unstamped) migration re-runs cleanly next boot. + * + * Deletion predicate (conjunctive, fail-safe): a row is deleted iff it is + * classified DCR AND zero `connection` rows reference its (owner, slug). The + * connection reference is `(oauth_client_owner, oauth_client)` within the same + * tenant. Manual apps and still-referenced DCR apps are always kept. + */ +export const runSqliteOAuthClientGcMigration = ( + client: SqliteDataMigrationClient, +): Effect.Effect<{ readonly deleted: number; readonly backfilled: number }, DataMigrationError> => + Effect.gen(function* () { + // A pre-oauth database (or a partial baseline) simply has nothing to do. + if (!(yield* tableExists(client, "oauth_client"))) return { deleted: 0, backfilled: 0 }; + const hasConnections = yield* tableExists(client, "connection"); + + const rowsResult = yield* execute( + client, + "SELECT tenant, owner, slug, grant, resource, origin_kind, origin_issuer, token_url FROM oauth_client", + ); + const rows = rowsResult.rows; + + // Count connections that reference each (tenant, owner, slug) in ONE grouped + // pass (not a COUNT per oauth_client row), so the GC decision is conjunctive + // without an N+1. When the connection table is absent (impossible in + // practice, but keeps this total) every DCR row counts as orphaned. + const referenceKey = (tenant: unknown, owner: unknown, slug: unknown): string => + `${String(tenant ?? "")} ${String(owner ?? "")} ${String(slug ?? "")}`; + const referenceCounts = new Map(); + if (hasConnections) { + const countsResult = yield* execute( + client, + "SELECT tenant, oauth_client_owner, oauth_client, COUNT(*) AS count FROM connection WHERE oauth_client IS NOT NULL GROUP BY tenant, oauth_client_owner, oauth_client", + ); + for (const countRow of countsResult.rows) { + referenceCounts.set( + referenceKey(countRow.tenant, countRow.oauth_client_owner, countRow.oauth_client), + Number(countRow.count ?? 0), + ); + } + } + + const applyAll = Effect.gen(function* () { + let deleted = 0; + let backfilled = 0; + + for (const row of rows) { + if (!isDcrClassifiedRow(row)) continue; // manual apps: never touched. + + const count = referenceCounts.get(referenceKey(row.tenant, row.owner, row.slug)) ?? 0; + const decision = classifyOAuthClientGc(row, count); + + if (decision.action === "delete") { + yield* execute(client, { + sql: "DELETE FROM oauth_client WHERE tenant = ? AND owner = ? AND slug = ?", + args: [row.tenant ?? null, row.owner ?? null, row.slug ?? null], + }); + deleted += 1; + continue; + } + + // Surviving (referenced) DCR row with no stored issuer: backfill it from + // the registrable origin of token_url so the per-AS reuse lookup keys on + // it and mints no new duplicate. + if (row.origin_issuer == null) { + const issuer = + row.token_url == null ? null : registrableOriginOfUrl(String(row.token_url)); + if (issuer !== null) { + yield* execute(client, { + sql: "UPDATE oauth_client SET origin_issuer = ? WHERE tenant = ? AND owner = ? AND slug = ?", + args: [issuer, row.tenant ?? null, row.owner ?? null, row.slug ?? null], + }); + backfilled += 1; + } + } + } + + yield* execute(client, "COMMIT"); + return { deleted, backfilled }; + }); + + yield* execute(client, "BEGIN"); + return yield* applyAll.pipe( + Effect.tapError(() => execute(client, "ROLLBACK").pipe(Effect.ignore)), + ); + }); + +/** Ledger entry for the local/self-host boot data-migration registry. */ +export const oauthClientGcSqliteMigration: SqliteDataMigration = { + name: MIGRATION_NAME, + run: (client) => runSqliteOAuthClientGcMigration(client).pipe(Effect.asVoid), +}; diff --git a/packages/react/src/api/atoms.tsx b/packages/react/src/api/atoms.tsx index 9cc364fda..99e57153a 100644 --- a/packages/react/src/api/atoms.tsx +++ b/packages/react/src/api/atoms.tsx @@ -460,6 +460,7 @@ export const createOAuthClientOptimistic = oauthClientsOptimisticAtom.pipe( readonly grant: OAuthGrant; readonly clientId: string; readonly resource?: string | null; + readonly originIntegration?: IntegrationSlug | null; }; }, ) => @@ -472,7 +473,9 @@ export const createOAuthClientOptimistic = oauthClientsOptimisticAtom.pipe( tokenUrl: arg.payload.tokenUrl, resource: arg.payload.resource ?? null, clientId: arg.payload.clientId, - origin: { kind: "manual" }, + // Mirror the server's stamp so the just-registered app matches its + // integration in the picker immediately (before the refetch lands). + origin: { kind: "manual", integration: arg.payload.originIntegration ?? null }, }; const index = rows.findIndex( (client) => client.owner === summary.owner && client.slug === summary.slug, diff --git a/packages/react/src/components/add-account-modal.test.ts b/packages/react/src/components/add-account-modal.test.ts index 2e9c1c8b6..93b63c3c7 100644 --- a/packages/react/src/components/add-account-modal.test.ts +++ b/packages/react/src/components/add-account-modal.test.ts @@ -31,6 +31,7 @@ const apiKeyMethod = (id: string, source: "spec" | "custom", template = id): Aut }); type ProbeResult = { + readonly issuer?: string | null; readonly authorizationUrl: string; readonly tokenUrl: string; readonly resource?: string | null; @@ -42,6 +43,7 @@ type ProbeResult = { type RegisterArgs = { readonly owner: Owner; readonly slug: OAuthClientSlug; + readonly issuer?: string | null; readonly registrationEndpoint: string; readonly authorizationUrl: string; readonly tokenUrl: string; @@ -306,6 +308,7 @@ describe("runDcrConnect", () => { const probe = (_url: string): Promise => { calls.push("probe"); return Promise.resolve({ + issuer: "https://auth.example.com", authorizationUrl: "https://auth.example.com/authorize", tokenUrl: "https://auth.example.com/token", resource: "https://mcp.example.com/mcp", @@ -330,7 +333,6 @@ describe("runDcrConnect", () => { discoveryUrl: "https://mcp.example.com/mcp", owner: "user", integrationName: "Linear MCP", - existingSlugs: [], redirectUri: "https://localhost:5394/api/oauth/callback", integration: TEST_INTEGRATION, }, @@ -342,6 +344,8 @@ describe("runDcrConnect", () => { // DCR always mints an authorization-code/PKCE client; callers do not choose // a grant here. expect(registerArgs).not.toBeNull(); + expect(String(registerArgs!.slug)).toBe("dcr-auth-example-com"); + expect(registerArgs!.issuer).toBe("https://auth.example.com"); expect(registerArgs!.registrationEndpoint).toBe("https://auth.example.com/register"); expect(registerArgs!.authorizationUrl).toBe("https://auth.example.com/authorize"); expect(registerArgs!.tokenUrl).toBe("https://auth.example.com/token"); @@ -379,7 +383,6 @@ describe("runDcrConnect", () => { resourceFallback: "https://mcp.example.com/mcp", owner: "user", integrationName: "App", - existingSlugs: [], declaredScopes: ["declared.scope"], integration: TEST_INTEGRATION, }, @@ -416,7 +419,6 @@ describe("runDcrConnect", () => { discoveryUrl: "https://auth.example.com/token", owner: "user", integrationName: "App", - existingSlugs: [], integration: TEST_INTEGRATION, }, ); @@ -448,7 +450,6 @@ describe("runDcrConnect", () => { discoveryUrl: "https://mcp.example.com/mcp", owner: "user", integrationName: "App", - existingSlugs: [], integration: TEST_INTEGRATION, }, ); @@ -483,7 +484,6 @@ describe("runDcrConnect", () => { discoveryUrl: "https://mcp.example.com/mcp", owner: "user", integrationName: "App", - existingSlugs: [], integration: TEST_INTEGRATION, }, ); @@ -513,7 +513,6 @@ describe("runDcrConnect", () => { discoveryUrl: "https://mcp.example.com/mcp", owner: "user", integrationName: "App", - existingSlugs: [], integration: TEST_INTEGRATION, }, ); @@ -552,7 +551,6 @@ describe("runDcrConnect", () => { discoveryUrl: "https://mcp.example.com/mcp", owner: "user", integrationName: "App", - existingSlugs: [], integration: TEST_INTEGRATION, }, ); @@ -570,4 +568,37 @@ describe("runDcrConnect", () => { }); expect(started).toBe(false); }); + + it("mints a deterministic server-keyed slug WITHOUT the picker's slug list", async () => { + // Part B removed the DCR connect path's dependency on the picker's app list: + // the slug is derived from the authorization server (Part A), so callers no + // longer thread `existingSlugs`. This omits it entirely and still succeeds. + let registerArgs: RegisterArgs | null = null; + const outcome = await runDcrConnect( + { + probe: (): Promise => + Promise.resolve({ + issuer: "https://auth.example.com", + authorizationUrl: "https://auth.example.com/authorize", + tokenUrl: "https://auth.example.com/token", + registrationEndpoint: "https://auth.example.com/register", + }), + register: (args: RegisterArgs): Promise => { + registerArgs = args; + return Promise.resolve(OAuthClientSlug.make("app")); + }, + start: (): void => {}, + }, + { + discoveryUrl: "https://mcp.example.com/mcp", + owner: "user", + integrationName: "App", + integration: TEST_INTEGRATION, + }, + ); + expect(outcome.kind).toBe("started"); + expect(registerArgs).not.toBeNull(); + // Slug comes from the issuer host, independent of any picker state. + expect(String(registerArgs!.slug)).toBe("dcr-auth-example-com"); + }); }); diff --git a/packages/react/src/components/add-account-modal.tsx b/packages/react/src/components/add-account-modal.tsx index 08d065e8b..03ad8d103 100644 --- a/packages/react/src/components/add-account-modal.tsx +++ b/packages/react/src/components/add-account-modal.tsx @@ -47,6 +47,8 @@ import { import { clientDisplayName, clientHost, + optimisticDcrClientSlug, + selectDcrClientsForIntegration, uniqueClientSlug, useOAuthClientsForIntegration, type OAuthClientOption, @@ -64,13 +66,14 @@ import { PlacementLine, type AuthMethod } from "../lib/auth-placements"; import { connectionIdentifier } from "../lib/connection-name"; import { Badge } from "./badge"; import { Button } from "./button"; +import { Collapsible, CollapsibleContent, CollapsibleTrigger } from "./collapsible"; import { DropdownMenu, DropdownMenuContent, DropdownMenuItem, DropdownMenuTrigger, } from "./dropdown-menu"; -import { PlusIcon, XIcon } from "lucide-react"; +import { ChevronDown, PlusIcon, XIcon } from "lucide-react"; import { Dialog, DialogContent, @@ -613,6 +616,7 @@ export async function runCimdConnect( /** Discovery result from the probe step (subset of the `probeOAuth` response). */ type DcrProbeResult = { + readonly issuer?: string | null; readonly authorizationUrl: string; readonly tokenUrl: string; readonly resource?: string | null; @@ -624,6 +628,7 @@ type DcrProbeResult = { type DcrRegisterArgs = { readonly owner: Owner; readonly slug: OAuthClientSlug; + readonly issuer?: string | null; readonly registrationEndpoint: string; readonly authorizationUrl: string; readonly tokenUrl: string; @@ -684,8 +689,6 @@ type RunDcrConnectInput = { readonly resourceFallback?: string; readonly owner: Owner; readonly integrationName: string; - /** The owner's existing client slugs, so the minted slug stays unique. */ - readonly existingSlugs: readonly string[]; /** Scopes declared by the integration's method (override the probed ones). */ readonly declaredScopes?: readonly string[]; /** Browser-facing callback URL registered with DCR when available. */ @@ -718,11 +721,12 @@ export async function runDcrConnect( const registrationEndpoint = probe.registrationEndpoint; if (!registrationEndpoint) return { kind: "fallback", reason: "no-registration-endpoint", probe }; - const slug = uniqueClientSlug(input.integrationName, input.existingSlugs); + const slug = optimisticDcrClientSlug(probe.issuer ?? registrationEndpoint); const scopes = registrationScopes(input.declaredScopes ?? [], probe.scopesSupported ?? []); const minted = await deps.register({ owner: input.owner, slug, + issuer: probe.issuer ?? null, registrationEndpoint, authorizationUrl: probe.authorizationUrl, tokenUrl: probe.tokenUrl, @@ -898,6 +902,10 @@ function AddAccountModalView(props: AddAccountModalProps) { // FIX 3 escape hatch: when no registered app matched the integration's // endpoints, the unmatched apps are collapsed behind an opt-in expander. const [showOtherApps, setShowOtherApps] = useState(false); + // Auto-registered (DCR) clients are plumbing, not pickable apps: they live in a + // collapsed management section at the bottom of the OAuth tab, revealed on + // demand so they can be inspected and deleted without cluttering the picker. + const [showAutoRegistered, setShowAutoRegistered] = useState(false); // Inline OAuth app management — edit re-opens the registration form for an // existing app (upsert by owner+slug); remove confirms before deleting. Both // hold the FULL app summary (with endpoints + resource) so the edit prefill @@ -1076,6 +1084,7 @@ function AddAccountModalView(props: AddAccountModalProps) { // unconditionally; in DCR mode the result is ignored until/unless we fall back. const { clients: oauthApps, + nearMatches: oauthNearApps, otherClients: oauthOtherApps, loading: oauthLoading, endpointMatched: oauthEndpointMatched, @@ -1088,8 +1097,29 @@ function AddAccountModalView(props: AddAccountModalProps) { // unrelated provider's app. tokenUrl: method?.oauth?.tokenUrl ?? oauthFallbackProbe?.tokenUrl, authorizationUrl: method?.oauth?.authorizationUrl ?? oauthFallbackProbe?.authorizationUrl, + // Recorded intent: a manual app registered from THIS integration's dialog is + // a tier-1 match regardless of host. + integration, requireEndpointMatch: isDcr, }); + // Auto-registered (DCR) clients for THIS integration. Excluded from the picker + // above, but surfaced in a collapsed management section so they stay + // deletable. Reads the same optimistic list the picker does. + const dcrClients = useMemo( + () => + selectDcrClientsForIntegration(clientSummaries as readonly OAuthClientOption[], { + integration, + tokenUrl: method?.oauth?.tokenUrl ?? oauthFallbackProbe?.tokenUrl, + authorizationUrl: method?.oauth?.authorizationUrl ?? oauthFallbackProbe?.authorizationUrl, + }), + [ + clientSummaries, + integration, + method?.oauth?.tokenUrl, + method?.oauth?.authorizationUrl, + oauthFallbackProbe, + ], + ); const oauthPopup = useOAuthPopupFlow({ popupName: "add-account-oauth", detectPopupClosed: false, @@ -1106,8 +1136,15 @@ function AddAccountModalView(props: AddAccountModalProps) { // Editing reuses the registration form (createClient upserts by owner+slug), // so it occupies the same full-bleed sub-view as registering. const oauthEditing = isOAuth && editingClient !== null; + // Resolve the picked slug across every PICKABLE tier: exact matches, the + // subdued near-match tier, and the unrelated escape-hatch apps. All three are + // selectable (only the default auto-selection is restricted to tier 1), so the + // connect button must resolve a client from any of them. DCR clients are not + // in these lists, so they can never be chosen as the connect app. const chosenClient: OAuthClientOption | null = - oauthApps.find((c: OAuthClientOption) => String(c.slug) === selectedApp) ?? null; + [...oauthApps, ...oauthNearApps, ...oauthOtherApps].find( + (c: OAuthClientOption) => String(c.slug) === selectedApp, + ) ?? null; const oauthBusy = ccBusy || oauthPopup.busy; const cimdConnecting = cimdBusy || oauthPopup.busy; const dcrConnecting = dcrBusy || oauthPopup.busy; @@ -1444,6 +1481,7 @@ function AddAccountModalView(props: AddAccountModalProps) { payload: { owner: args.owner, slug: args.slug, + issuer: args.issuer ?? null, registrationEndpoint: args.registrationEndpoint, authorizationUrl: args.authorizationUrl, tokenUrl: args.tokenUrl, @@ -1491,9 +1529,8 @@ function AddAccountModalView(props: AddAccountModalProps) { resourceFallback: method.oauth?.discoveryUrl, owner: dcrOwner, integrationName, - existingSlugs: [...oauthApps, ...oauthOtherApps].map((app: OAuthClientOption) => - String(app.slug), - ), + // DCR slugs are server-keyed (Part A): the connect path no longer depends + // on the picker's app list, so it need not be threaded here. declaredScopes: method.oauth?.scopes, redirectUri: oauthCallbackUrl(), integration, @@ -1553,9 +1590,7 @@ function AddAccountModalView(props: AddAccountModalProps) {
- String(app.slug), - )} + existingSlugs={clientSummaries.map((c: OAuthClientSummary) => String(c.slug))} fixedSlug={editingClient.slug} fixedOwner={editingClient.owner} prefill={{ @@ -1582,9 +1617,8 @@ function AddAccountModalView(props: AddAccountModalProps) {
- String(app.slug), - )} + integrationSlug={integration} + existingSlugs={clientSummaries.map((c: OAuthClientSummary) => String(c.slug))} autoRegisterRejectedReason={dcrFallbackMessage} prefill={{ authorizationUrl: @@ -1602,6 +1636,7 @@ function AddAccountModalView(props: AddAccountModalProps) { null, scopes: method.oauth?.scopes, discoveredScopes: oauthFallbackProbe?.scopesSupported, + issuer: oauthFallbackProbe?.issuer ?? null, registrationEndpoint: method.oauth?.registrationEndpoint ?? oauthFallbackProbe?.registrationEndpoint ?? @@ -1875,6 +1910,107 @@ function AddAccountModalView(props: AddAccountModalProps) { )} + + {/* Tier 2 — apps that only match by root domain (a + near-miss, e.g. the same provider's MCP app for a REST + integration). Visually separated and subdued so they are + never mistaken for the integration's own app, but still + selectable as an escape hatch. Shown alongside tier 1, not + mixed into it. */} + {oauthNearApps.length > 0 && ( +
+

+ Other apps on this provider +

+

+ These share {integrationName}'s domain but weren't + registered for it. Pick one only if you know it applies. +

+ + {oauthNearApps.map((app: OAuthClientOption) => ( + + ))} + +
+ )} + + {/* Auto-registered (DCR) clients: plumbing, never + pickable. Kept here, collapsed, so they can be reviewed + and deleted without cluttering the picker. */} + {dcrClients.length > 0 && ( +
+ + + Auto-registered clients ({dcrClients.length}) + + +
+

+ Created automatically when connecting. Reused across + connections, not selectable as your app. +

+ {dcrClients.map((app: OAuthClientOption) => ( +
+ + + {clientDisplayName(String(app.slug))} + + + {clientHost(app.tokenUrl)} + + + {ownerDisplay.showOwnerLabels ? ( + + {ownerLabel(app.owner)} + + ) : null} + +
+ ))} +
+ + +
+ )}
) ) : ( diff --git a/packages/react/src/components/oauth-client-form.tsx b/packages/react/src/components/oauth-client-form.tsx index c09dd3756..87f274122 100644 --- a/packages/react/src/components/oauth-client-form.tsx +++ b/packages/react/src/components/oauth-client-form.tsx @@ -1,7 +1,12 @@ import { useMemo, useState } from "react"; import { useAtomSet } from "@effect/atom-react"; import * as Exit from "effect/Exit"; -import { OAuthClientSlug, type OAuthGrant, type Owner } from "@executor-js/sdk/shared"; +import { + OAuthClientSlug, + type IntegrationSlug, + type OAuthGrant, + type Owner, +} from "@executor-js/sdk/shared"; import { toast } from "sonner"; import { createOAuthClientOptimistic, probeOAuth, registerDynamicOAuthClient } from "../api/atoms"; @@ -9,7 +14,7 @@ import { ownerLabelForHost } from "../api/owner-display"; import { trackEvent } from "../api/analytics"; import { useOrganizationId } from "../api/organization-context"; import { oauthClientWriteKeys } from "../api/reactivity-keys"; -import { uniqueClientSlug } from "../plugins/use-effective-oauth-client"; +import { optimisticDcrClientSlug, uniqueClientSlug } from "../plugins/use-effective-oauth-client"; import { oauthCallbackUrl } from "../plugins/oauth-sign-in"; import { ConnectionOwnerDropdown, @@ -36,6 +41,7 @@ import { RadioGroup, RadioGroupItem } from "./radio-group"; // --------------------------------------------------------------------------- export interface OAuthClientFormPrefill { + readonly issuer?: string | null; readonly authorizationUrl?: string; readonly tokenUrl?: string; readonly resource?: string | null; @@ -89,6 +95,12 @@ export const canSubmitOAuthClientForm = (input: { export function OAuthClientForm(props: { /** Human label for the integration this app backs (used in toasts + default name). */ readonly integrationName: string; + /** Slug of the integration whose dialog this form is registering from, when + * known. Stamped onto the created MANUAL app as recorded intent so the picker + * matches it to this integration exactly (not by root-domain guess). Omitted + * when editing (an existing app's origin is fixed) or when there is no single + * integration context. */ + readonly integrationSlug?: IntegrationSlug; /** Existing client slugs, so the generated slug stays unique across apps. */ readonly existingSlugs: readonly string[]; /** Endpoints/scopes declared by the integration's OAuth method. */ @@ -112,6 +124,7 @@ export function OAuthClientForm(props: { }) { const { integrationName, + integrationSlug, existingSlugs, prefill, fixedSlug, @@ -147,6 +160,7 @@ export function OAuthClientForm(props: { const [clientId, setClientId] = useState(prefill?.clientId ?? ""); const [clientSecret, setClientSecret] = useState(""); const [issuerUrl, setIssuerUrl] = useState(""); + const [discoveredIssuer, setDiscoveredIssuer] = useState(prefill?.issuer ?? null); const [authorizationUrl, setAuthorizationUrl] = useState(prefill?.authorizationUrl ?? ""); const [tokenUrl, setTokenUrl] = useState(prefill?.tokenUrl ?? ""); const [resource, setResource] = useState(prefill?.resource ?? null); @@ -226,6 +240,7 @@ export function OAuthClientForm(props: { return; } const result = exit.value; + setDiscoveredIssuer(result.issuer ?? null); setAuthorizationUrl(result.authorizationUrl); setTokenUrl(result.tokenUrl); setResource(result.resource ?? null); @@ -247,11 +262,13 @@ export function OAuthClientForm(props: { const handleRegisterDynamic = async () => { if (!canRegisterDynamic || registering) return; setRegistering(true); - const slug = fixedSlug ?? uniqueClientSlug(name, existingSlugs); + const slug = + fixedSlug ?? optimisticDcrClientSlug(discoveredIssuer ?? registrationEndpoint.trim()); const exit = await doRegisterDynamic({ payload: { owner, slug, + issuer: discoveredIssuer, registrationEndpoint: registrationEndpoint.trim(), authorizationUrl: authorizationUrl.trim(), tokenUrl: tokenUrl.trim(), @@ -273,7 +290,7 @@ export function OAuthClientForm(props: { } trackEvent("oauth_client_registered", { owner, grant, via_dcr: true, success: true }); toast.success(`Registered ${integrationName} OAuth app`); - onCreated({ owner, slug }); + onCreated({ owner, slug: exit.value.client }); }; const handleSubmit = async () => { @@ -290,6 +307,9 @@ export function OAuthClientForm(props: { clientId: clientId.trim(), clientSecret: clientSecret.trim(), resource, + // Editing keeps the app's original origin; only a fresh registration + // from an integration's dialog stamps recorded intent. + originIntegration: fixedSlug ? null : (integrationSlug ?? null), }, reactivityKeys: oauthClientWriteKeys, }); diff --git a/packages/react/src/plugins/dcr-root-domain-isolation.test.ts b/packages/react/src/plugins/dcr-root-domain-isolation.test.ts new file mode 100644 index 000000000..30e24c53a --- /dev/null +++ b/packages/react/src/plugins/dcr-root-domain-isolation.test.ts @@ -0,0 +1,125 @@ +// The DCR root-domain collision at the picker-classification level (issue #1120). +// +// Two DIFFERENT integrations share one registrable root domain (cloudflare.com) +// but differ by host: a "Cloudflare" MCP integration whose OAuth is on +// mcp.cloudflare.com, and the plain "Cloudflare" REST integration on +// api.cloudflare.com. Connecting the MCP integration Dynamic-Client-Registers an +// oauth_client (origin: dynamic_client_registration) against mcp.cloudflare.com. +// +// The regression PR #1139 missed: that auto-registered client leaking into the +// REST integration's Add-connection app picker. The existing unit tests in +// use-effective-oauth-client.test.ts check DCR exclusion and root-domain tiering +// one picker at a time; this file asserts the CROSS-INTEGRATION property those +// don't: the MCP integration's DCR client is offered in NONE of the REST +// integration's picker tiers, yet stays visible in the MCP integration's own +// auto-registered management list. It runs the exact pure classifier the app +// (add-account-modal.tsx via useOAuthClientsForIntegration) uses to build the +// picker, so it guards the shipped behaviour, not a reimplementation. +// +// The server-side half (that DCR actually mints this row over the wire and +// reuses it on reconnect) is covered by e2e/cloud/dcr-root-domain-isolation.test.ts. +import { describe, expect, it } from "@effect/vitest"; +import { IntegrationSlug, OAuthClientSlug, type Owner } from "@executor-js/sdk/shared"; +import type { OAuthClientOrigin } from "@executor-js/sdk/shared"; + +import { + selectClientsForEndpoints, + selectDcrClientsForIntegration, + type OAuthClientOption, +} from "./use-effective-oauth-client"; + +const client = ( + slug: string, + opts: { + readonly owner?: Owner; + readonly authorizationUrl: string; + readonly tokenUrl: string; + readonly origin: OAuthClientOrigin; + }, +): OAuthClientOption => ({ + owner: opts.owner ?? "org", + slug: OAuthClientSlug.make(slug), + grant: "authorization_code", + authorizationUrl: opts.authorizationUrl, + tokenUrl: opts.tokenUrl, + clientId: "client-id", + origin: opts.origin, +}); + +const MCP_INTEGRATION = IntegrationSlug.make("cloudflare_mcp"); +const REST_INTEGRATION = IntegrationSlug.make("cloudflare_api"); + +// The auto-registered client the MCP connect flow mints, exactly as the API +// projects it (origin.kind = dynamic_client_registration, endpoints on the MCP +// host, stamped with the MCP integration as its origin). +const mcpDcrClient = client("dcr-mcp-cloudflare-com", { + authorizationUrl: "https://mcp.cloudflare.com/authorize", + tokenUrl: "https://mcp.cloudflare.com/token", + origin: { kind: "dynamic_client_registration", integration: MCP_INTEGRATION }, +}); + +// What the REST integration (api.cloudflare.com) declares to its picker. +const restEndpoints = { + tokenUrl: "https://api.cloudflare.com/client/v4/oauth/token", + authorizationUrl: "https://api.cloudflare.com/client/v4/oauth/authorize", + integration: REST_INTEGRATION, + requireEndpointMatch: true, +} as const; + +describe("DCR root-domain isolation across two same-provider integrations", () => { + it("the MCP integration's DCR client is offered in NO tier of the REST integration's picker", () => { + const rest = selectClientsForEndpoints([mcpDcrClient], restEndpoints); + + const offeredInAnyTier = [...rest.matched, ...rest.nearMatches, ...rest.unmatched].map((c) => + String(c.slug), + ); + expect( + offeredInAnyTier, + "the auto-registered client never surfaces as a selectable app for a same-root integration", + ).not.toContain("dcr-mcp-cloudflare-com"); + // The three tiers ARE the whole picker; nothing selectable remains. + expect(offeredInAnyTier, "the picker offers nothing to select").toEqual([]); + expect( + rest.matched.length, + "no exact/intent match, so the REST picker shows the register-an-app CTA", + ).toBe(0); + expect(rest.endpointMatched, "an endpoint was declared but nothing matched exactly").toBe( + false, + ); + }); + + it("a same-root MANUAL app (not DCR) still lands in the near-matches tier, proving the exclusion is DCR-specific", () => { + // Same mcp.cloudflare.com endpoints, but a manual app: the classifier must + // still surface it (as a subdued near-match), so the empty picker above is + // due to the DCR origin, not the host mismatch. + const manualNearMiss = client("cloudflare-mcp-manual", { + authorizationUrl: "https://mcp.cloudflare.com/authorize", + tokenUrl: "https://mcp.cloudflare.com/token", + origin: { kind: "manual", integration: null }, + }); + const rest = selectClientsForEndpoints([manualNearMiss, mcpDcrClient], restEndpoints); + + expect( + rest.nearMatches.map((c) => String(c.slug)), + "the manual same-root app is a tier-2 near-match", + ).toEqual(["cloudflare-mcp-manual"]); + expect( + [...rest.matched, ...rest.nearMatches, ...rest.unmatched].map((c) => String(c.slug)), + "the DCR client is still excluded even alongside a pickable manual app", + ).not.toContain("dcr-mcp-cloudflare-com"); + }); + + it("the DCR client stays visible in the MCP integration's own auto-registered management list", () => { + // Hidden from the picker is not hidden from management: the MCP integration + // that minted it must still be able to review and remove it. + const managed = selectDcrClientsForIntegration([mcpDcrClient], { + integration: MCP_INTEGRATION, + tokenUrl: "https://mcp.cloudflare.com/token", + authorizationUrl: "https://mcp.cloudflare.com/authorize", + }); + expect( + managed.map((c) => String(c.slug)), + "the auto-registered client is manageable from the integration that created it", + ).toEqual(["dcr-mcp-cloudflare-com"]); + }); +}); diff --git a/packages/react/src/plugins/use-effective-oauth-client.test.ts b/packages/react/src/plugins/use-effective-oauth-client.test.ts index 92642e077..6bdbf612f 100644 --- a/packages/react/src/plugins/use-effective-oauth-client.test.ts +++ b/packages/react/src/plugins/use-effective-oauth-client.test.ts @@ -1,8 +1,11 @@ import { describe, expect, it } from "@effect/vitest"; -import { OAuthClientSlug, type Owner } from "@executor-js/sdk/shared"; +import { IntegrationSlug, OAuthClientSlug, type Owner } from "@executor-js/sdk/shared"; +import type { OAuthClientOrigin } from "@executor-js/sdk/shared"; import { + optimisticDcrClientSlug, selectClientsForEndpoints, + selectDcrClientsForIntegration, uniqueClientSlug, type OAuthClientOption, } from "./use-effective-oauth-client"; @@ -13,6 +16,7 @@ const app = ( readonly owner?: Owner; readonly authorizationUrl: string; readonly tokenUrl: string; + readonly origin?: OAuthClientOrigin; }, ): OAuthClientOption => ({ owner: opts.owner ?? "user", @@ -21,6 +25,7 @@ const app = ( authorizationUrl: opts.authorizationUrl, tokenUrl: opts.tokenUrl, clientId: "client-id", + origin: opts.origin ?? { kind: "manual" }, }); const google = app("google-app", { @@ -89,6 +94,27 @@ describe("selectClientsForEndpoints", () => { ]); }); + it("does not tier-1 match an unparseable app URL against a scheme-less declared endpoint", () => { + // Regression: `hostOf` returns undefined for URLs `new URL()` can't parse. + // An integration whose manifest declares a scheme-less authorization URL + // ("oauth.cloudflare.com", no validation on manifest Schema.String) and an + // unrelated app whose stored URL is also unparseable would both yield + // `undefined` hosts, so a naive `appAuthorizationHost === wantedAuthorizationHost` + // is `undefined === undefined` → a false tier-1 (default-picked) match. The + // guard must require BOTH hosts defined, so this app lands in `unmatched`. + const unparseable = app("mystery-app", { + authorizationUrl: "not a url", + tokenUrl: "not a url", + }); + const result = selectClientsForEndpoints([unparseable], { + authorizationUrl: "oauth.cloudflare.com", + }); + expect(result.matched).toEqual([]); + expect(result.nearMatches).toEqual([]); + expect(result.endpointMatched).toBe(false); + expect(result.unmatched.map((a: OAuthClientOption) => String(a.slug))).toEqual(["mystery-app"]); + }); + it("treats every app as usable when no endpoint is declared", () => { const result = selectClientsForEndpoints([google, spotify], {}); expect(result.endpointMatched).toBe(true); @@ -133,6 +159,161 @@ describe("selectClientsForEndpoints", () => { }); expect(result.matched.map((a: OAuthClientOption) => a.owner)).toEqual(["user", "org"]); }); + + it("excludes DCR clients from the picker entirely (they are plumbing, not apps)", () => { + // A DCR client minted against the integration's exact endpoint STILL must not + // appear in the picker — it is reused automatically, never user-picked. + const dcr = app("dcr-oauth2-googleapis-com", { + authorizationUrl: "https://accounts.google.com/o/oauth2/v2/auth", + tokenUrl: "https://oauth2.googleapis.com/token", + origin: { kind: "dynamic_client_registration", integration: null }, + }); + const result = selectClientsForEndpoints([dcr, google], { + tokenUrl: "https://oauth2.googleapis.com/token", + }); + // Only the manual google-app survives into any tier; the DCR client is gone. + expect(result.matched.map((a: OAuthClientOption) => String(a.slug))).toEqual(["google-app"]); + expect(result.nearMatches).toEqual([]); + expect(result.unmatched).toEqual([]); + }); + + it("origin-integration (recorded intent) is a tier-1 match, beating a root-domain near-miss", () => { + const integration = IntegrationSlug.make("cloudflare_api"); + // Registered from the Cloudflare API dialog: exact intent, even though its + // OAuth host (dash.cloudflare.com) is not the declared api.cloudflare.com. + const stamped = app("my-cloudflare-app", { + authorizationUrl: "https://dash.cloudflare.com/oauth2/auth", + tokenUrl: "https://dash.cloudflare.com/oauth2/token", + origin: { kind: "manual", integration }, + }); + // Same root domain but different host and no intent stamp: a near-miss. + const nearMiss = app("cloudflare-mcp-app", { + authorizationUrl: "https://mcp.cloudflare.com/authorize", + tokenUrl: "https://mcp.cloudflare.com/token", + }); + const result = selectClientsForEndpoints([nearMiss, stamped], { + tokenUrl: "https://api.cloudflare.com/client/v4/token", + authorizationUrl: "https://api.cloudflare.com/client/v4/authorize", + integration, + }); + expect(result.endpointMatched).toBe(true); + // Intent wins tier 1; the root-domain near-miss lands in tier 2, never mixed in. + expect(result.matched.map((a: OAuthClientOption) => String(a.slug))).toEqual([ + "my-cloudflare-app", + ]); + expect(result.nearMatches.map((a: OAuthClientOption) => String(a.slug))).toEqual([ + "cloudflare-mcp-app", + ]); + expect(result.unmatched).toEqual([]); + }); + + it("a root-domain-only match lands in tier 2 (nearMatches), not tier 1", () => { + // The declared endpoint is api.cloudflare.com; the app is on mcp.cloudflare.com. + // Same registrable root (cloudflare.com), different host, no intent stamp: this + // is exactly the bug case — it must NOT be presented as a real match. + const nearMiss = app("cloudflare-mcp-app", { + authorizationUrl: "https://mcp.cloudflare.com/authorize", + tokenUrl: "https://mcp.cloudflare.com/token", + }); + const result = selectClientsForEndpoints([nearMiss], { + tokenUrl: "https://api.cloudflare.com/client/v4/token", + authorizationUrl: "https://api.cloudflare.com/client/v4/authorize", + integration: IntegrationSlug.make("cloudflare_api"), + }); + expect(result.endpointMatched).toBe(false); + expect(result.matched).toEqual([]); + expect(result.nearMatches.map((a: OAuthClientOption) => String(a.slug))).toEqual([ + "cloudflare-mcp-app", + ]); + expect(result.unmatched).toEqual([]); + }); + + it("exact endpoint host is a tier-1 match even without an intent stamp", () => { + const exact = app("cloudflare-api-app", { + authorizationUrl: "https://api.cloudflare.com/client/v4/authorize", + tokenUrl: "https://api.cloudflare.com/client/v4/token", + }); + const result = selectClientsForEndpoints([exact], { + tokenUrl: "https://api.cloudflare.com/client/v4/token", + integration: IntegrationSlug.make("cloudflare_api"), + }); + expect(result.endpointMatched).toBe(true); + expect(result.matched.map((a: OAuthClientOption) => String(a.slug))).toEqual([ + "cloudflare-api-app", + ]); + expect(result.nearMatches).toEqual([]); + }); + + it("recorded intent matches even when the integration declares no endpoints (requireEndpointMatch)", () => { + const integration = IntegrationSlug.make("linear_mcp"); + const stamped = app("linear-mcp-app", { + authorizationUrl: "https://linear.app/oauth/authorize", + tokenUrl: "https://api.linear.app/oauth/token", + origin: { kind: "manual", integration }, + }); + const unrelated = app("spotify-app-2", { + authorizationUrl: "https://accounts.spotify.com/authorize", + tokenUrl: "https://accounts.spotify.com/api/token", + }); + const result = selectClientsForEndpoints([unrelated, stamped], { + requireEndpointMatch: true, + integration, + }); + expect(result.endpointMatched).toBe(true); + expect(result.matched.map((a: OAuthClientOption) => String(a.slug))).toEqual([ + "linear-mcp-app", + ]); + expect(result.unmatched.map((a: OAuthClientOption) => String(a.slug))).toEqual([ + "spotify-app-2", + ]); + }); +}); + +describe("selectDcrClientsForIntegration", () => { + const integration = IntegrationSlug.make("linear_mcp"); + const dcrStamped = app("dcr-auth-linear-app", { + authorizationUrl: "https://linear.app/oauth/authorize", + tokenUrl: "https://api.linear.app/oauth/token", + origin: { kind: "dynamic_client_registration", integration }, + }); + const dcrOtherProvider = app("dcr-mcp-cloudflare-com", { + authorizationUrl: "https://mcp.cloudflare.com/authorize", + tokenUrl: "https://mcp.cloudflare.com/token", + origin: { kind: "dynamic_client_registration", integration: null }, + }); + const manualLinear = app("linear-manual", { + authorizationUrl: "https://linear.app/oauth/authorize", + tokenUrl: "https://api.linear.app/oauth/token", + origin: { kind: "manual", integration }, + }); + + it("returns only the DCR clients stamped with this integration", () => { + const result = selectDcrClientsForIntegration([dcrStamped, dcrOtherProvider, manualLinear], { + integration, + }); + expect(result.map((a: OAuthClientOption) => String(a.slug))).toEqual(["dcr-auth-linear-app"]); + }); + + it("surfaces legacy unstamped DCR clients by root domain of the declared endpoint", () => { + const legacyUnstamped = app("dcr-api-linear-app", { + authorizationUrl: "https://linear.app/oauth/authorize", + tokenUrl: "https://api.linear.app/oauth/token", + origin: { kind: "dynamic_client_registration", integration: null }, + }); + const result = selectDcrClientsForIntegration([legacyUnstamped, dcrOtherProvider], { + integration, + tokenUrl: "https://api.linear.app/oauth/token", + }); + expect(result.map((a: OAuthClientOption) => String(a.slug))).toEqual(["dcr-api-linear-app"]); + }); + + it("never returns manual apps", () => { + const result = selectDcrClientsForIntegration([manualLinear], { + integration, + tokenUrl: "https://api.linear.app/oauth/token", + }); + expect(result).toEqual([]); + }); }); describe("uniqueClientSlug", () => { @@ -144,3 +325,14 @@ describe("uniqueClientSlug", () => { ); }); }); + +describe("optimisticDcrClientSlug", () => { + it("derives a deterministic optimistic DCR slug from the authorization server host", () => { + expect(String(optimisticDcrClientSlug("https://mcp.cloudflare.com/register"))).toBe( + "dcr-mcp-cloudflare-com", + ); + expect(String(optimisticDcrClientSlug("http://127.0.0.1:8787/register"))).toBe( + "dcr-127-0-0-1-8787", + ); + }); +}); diff --git a/packages/react/src/plugins/use-effective-oauth-client.tsx b/packages/react/src/plugins/use-effective-oauth-client.tsx index eb904961f..0c0467254 100644 --- a/packages/react/src/plugins/use-effective-oauth-client.tsx +++ b/packages/react/src/plugins/use-effective-oauth-client.tsx @@ -1,6 +1,12 @@ +import { useMemo } from "react"; import { useAtomValue } from "@effect/atom-react"; import * as AsyncResult from "effect/unstable/reactivity/AsyncResult"; -import { OAuthClientSlug, type Owner } from "@executor-js/sdk/shared"; +import { + OAuthClientSlug, + type IntegrationSlug, + type OAuthClientOrigin, + type Owner, +} from "@executor-js/sdk/shared"; import { getDomain } from "tldts"; import { oauthClientsOptimisticAtom } from "../api/atoms"; @@ -9,12 +15,17 @@ import { oauthClientsOptimisticAtom } from "../api/atoms"; // OAuth client (registered app) selection for an integration's connect flow. // // An owner can register MANY apps; each is a distinct owner-scoped `oauth_client` -// row with its own slug. The connect flow lists the apps usable for an -// integration (owner-visible clients whose OAuth endpoints share a registrable -// root domain with the integration's declared endpoints when known) and lets the -// user pick one or register a new one. User-owned apps are listed before -// workspace ones. When NOTHING matches a declared endpoint, the picker shows an -// empty state + a "register an app" CTA rather than unrelated providers' apps. +// row with its own slug. The connect flow lists the MANUAL apps usable for an +// integration and lets the user pick one or register a new one. Matching is by +// recorded intent first (an app registered from this integration's dialog, or +// whose OAuth endpoint HOST exactly matches the integration's declared host), +// then by registrable root domain as a subdued near-miss tier. User-owned apps +// are listed before workspace ones. +// +// DCR (dynamic client registration) clients are plumbing, not apps: they are +// minted automatically and reused per authorization server, so they NEVER appear +// in this picker. They stay visible for management elsewhere (the modal's +// "Auto-registered clients" surface), keyed off `origin.kind`. // --------------------------------------------------------------------------- export interface OAuthClientOption { @@ -24,8 +35,18 @@ export interface OAuthClientOption { readonly authorizationUrl: string; readonly tokenUrl: string; readonly clientId: string; + /** How the app came to exist. `manual` apps are pickable (with `integration` + * recording which dialog registered them); `dynamic_client_registration` + * apps are excluded from the picker entirely. */ + readonly origin: OAuthClientOrigin; } +/** True for the auto-minted DCR clients that must never surface in the app + * picker. Legacy null-origin rows are already classified by the backend's + * `parseOAuthClientOrigin` heuristic, so this reads the parsed kind directly. */ +export const isDcrClient = (app: OAuthClientOption): boolean => + app.origin.kind === "dynamic_client_registration"; + const hostOf = (url: string): string | undefined => { // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: URL() throws on invalid input; treat as "no host" try { @@ -46,35 +67,54 @@ const getRootDomain = (url: string): string | undefined => { }; export interface UseOAuthClientsResult { - /** Apps usable for this integration, user-owned first. When an endpoint was - * declared but nothing matched, this is EMPTY (the unmatched apps move to - * `otherClients`). */ + /** Tier 1 — apps that match this integration by RECORDED INTENT (registered + * from its dialog) or EXACT endpoint host, user-owned first. These are the + * apps that belong to this integration; the picker defaults to the first one. + * When an endpoint was declared but nothing matched exactly, this is EMPTY. */ readonly clients: readonly OAuthClientOption[]; - /** Unmatched owner-visible apps — surfaced only as an opt-in escape hatch - * ("use a different registered app") when no app matched the declared - * endpoint. Empty when `endpointMatched` is true. */ + /** Tier 2 — apps that only match by registrable root domain (a near-miss: same + * provider family, different exact host / not stamped to this integration). + * Surfaced in a visually separate, subdued "Other apps on this provider" + * section — NEVER silently mixed into `clients`. Present whenever there are + * such near-misses, even when tier 1 is non-empty. */ + readonly nearMatches: readonly OAuthClientOption[]; + /** Unrelated apps (different root domain), kept only as the opt-in escape hatch + * ("use a different registered app") when NOTHING matched the declared + * endpoint at all. Empty once anything in tier 1 or tier 2 matched. */ readonly otherClients: readonly OAuthClientOption[]; /** True until the clients list has loaded at least once. */ readonly loading: boolean; /** - * Whether the returned `clients` are matched to the integration's declared - * OAuth endpoints (by registrable root domain across authorize + token). + * Whether an app is EXACT-matched (tier 1) to the integration. * * - `true` — either no endpoint filter was requested (the integration - * declares no endpoints), or at least one registered app's authorize/token - * root domain matched. `clients` are the matched subset. - * - `false` — the integration declared endpoint(s) but NO registered app - * matched. `clients` is then EMPTY (so the UI shows an empty state + a - * register CTA rather than unrelated providers' apps), and the unmatched - * apps are surfaced separately in `otherClients` for the opt-in escape hatch. + * declares no endpoints), or at least one app matched by recorded intent or + * exact endpoint host. `clients` is that tier-1 subset. + * - `false` — the integration declared endpoint(s) but no app matched exactly. + * `clients` is EMPTY (the UI shows an empty state + a register CTA). Any + * root-domain near-misses are still offered via `nearMatches`; truly + * unrelated apps via `otherClients`. */ readonly endpointMatched: boolean; /** Convenience flag for the UI: a register-an-app CTA should be shown because - * an endpoint was declared and nothing matched. Equals `!endpointMatched` + * an endpoint was declared and nothing exact-matched. Equals `!endpointMatched` * once loaded. */ readonly displayRegisterCTA: boolean; } +/** Stable empty-list reference so the picker memo doesn't re-run while the + * optimistic clients atom is still loading (a fresh `[]` each render would + * invalidate the memo key). */ +const EMPTY_CLIENTS: readonly OAuthClientOption[] = []; + +/** Host/root equality that matches ONLY when both sides parsed to a real value. + * `hostOf`/`getRootDomain` return undefined for URLs `new URL()` can't parse, + * so a bare `a === b` would treat two unparseable endpoints as equal + * (`undefined === undefined`). Every host/root comparison in this module must + * go through this so an unparseable value never counts as a match. */ +const hostEq = (a: string | undefined, b: string | undefined): boolean => + a !== undefined && b !== undefined && a === b; + /** Sort apps user-owned first (so the user's own apps surface before shared * workspace apps). */ const sortUserFirst = (apps: readonly OAuthClientOption[]): readonly OAuthClientOption[] => @@ -83,23 +123,37 @@ const sortUserFirst = (apps: readonly OAuthClientOption[]): readonly OAuthClient ); /** - * Pure matcher (no React/atoms) — split owner-visible apps into the ones that - * match the integration's declared OAuth endpoints and the ones that don't. + * Pure matcher (no React/atoms) — split owner-visible apps into three honest + * tiers for an integration's connect picker. + * + * FIRST, DCR clients are dropped entirely: they are auto-minted plumbing reused + * per authorization server, not apps a user picks. They stay manageable through + * a separate surface, never the picker. * - * Matching is by REGISTRABLE ROOT DOMAIN ("tld+1"). When the integration - * declares a token endpoint, an app must match by token endpoint root; the token - * endpoint is what the SDK will call during code exchange/refresh and avoids - * authorize-root coincidences. When only an authorization endpoint is declared, - * the authorize root is used as the fallback compatibility signal. Unrelated - * providers (different root) never match. + * Then the remaining MANUAL apps are graded: + * - `matched` (tier 1): the app was registered from THIS integration's dialog + * (`origin.integration === integration`, recorded intent), OR its OAuth + * endpoint HOST exactly matches the integration's declared host (token host + * when a token endpoint is declared, else the authorize host). This is the + * subset that truly belongs to the integration. + * - `nearMatches` (tier 2): the app shares the integration's REGISTRABLE ROOT + * DOMAIN ("tld+1") but is not a tier-1 match — a same-provider near-miss (e.g. + * an app on `mcp.cloudflare.com` for a REST integration on `api.cloudflare.com`). + * Silently promoting these into tier 1 is the bug this change fixes. + * - `unmatched`: unrelated provider (different root), offered only as the escape + * hatch when nothing matched at all. * - * When the integration declares no endpoints, every app is "matched" (no filter). + * When the integration declares no endpoints (and no match is required), every + * manual app is tier 1 (no filter to grade against). */ export function selectClientsForEndpoints( all: readonly OAuthClientOption[], endpoints: { readonly tokenUrl?: string; readonly authorizationUrl?: string; + /** The integration whose picker this is. A manual app stamped with this + * integration (recorded intent) is a tier-1 match regardless of host. */ + readonly integration?: IntegrationSlug; /** When set, an integration that targets a SPECIFIC server (MCP, whose * endpoints are discovered at connect) must match by endpoint — absent * endpoints mean NO match (show the register CTA), never "every app @@ -108,36 +162,81 @@ export function selectClientsForEndpoints( }, ): { readonly matched: readonly OAuthClientOption[]; + readonly nearMatches: readonly OAuthClientOption[]; readonly unmatched: readonly OAuthClientOption[]; readonly endpointMatched: boolean; } { + // DCR clients are plumbing, never picker options. + const manual = all.filter((app) => !isDcrClient(app)); + + const intent = endpoints.integration; + const matchesIntent = (app: OAuthClientOption): boolean => + intent != null && + app.origin.kind === "manual" && + app.origin.integration != null && + app.origin.integration === intent; + + const wantedTokenHost = endpoints.tokenUrl ? hostOf(endpoints.tokenUrl) : undefined; + const wantedAuthorizationHost = endpoints.authorizationUrl + ? hostOf(endpoints.authorizationUrl) + : undefined; const wantedTokenRoot = endpoints.tokenUrl ? getRootDomain(endpoints.tokenUrl) : undefined; const wantedAuthorizationRoot = endpoints.authorizationUrl ? getRootDomain(endpoints.authorizationUrl) : undefined; + // No declared endpoints. A server-targeting integration must NOT match every // app (it would auto-select an unrelated provider); surface the register CTA - // instead. Otherwise (no endpoint filter at all) every app is usable. + // instead. Otherwise (no endpoint filter at all) every manual app is usable. if (!wantedTokenRoot && !wantedAuthorizationRoot) { if (endpoints.requireEndpointMatch) { - return { matched: [], unmatched: sortUserFirst(all), endpointMatched: false }; + // Even with no declared endpoints, an app registered from THIS dialog is a + // real tier-1 match (the user built it here on purpose). + const matched = manual.filter(matchesIntent); + const rest = manual.filter((app) => !matchesIntent(app)); + return { + matched: sortUserFirst(matched), + nearMatches: [], + unmatched: sortUserFirst(rest), + endpointMatched: matched.length > 0, + }; } - return { matched: sortUserFirst(all), unmatched: [], endpointMatched: true }; + return { + matched: sortUserFirst(manual), + nearMatches: [], + unmatched: [], + endpointMatched: true, + }; } + const matched: OAuthClientOption[] = []; + const nearMatches: OAuthClientOption[] = []; const unmatched: OAuthClientOption[] = []; - for (const app of all) { + for (const app of manual) { + const appTokenHost = hostOf(app.tokenUrl); + const appAuthorizationHost = hostOf(app.authorizationUrl); const appTokenRoot = getRootDomain(app.tokenUrl); const appAuthorizationRoot = getRootDomain(app.authorizationUrl); - const fits = wantedTokenRoot - ? appTokenRoot === wantedTokenRoot - : appAuthorizationRoot === wantedAuthorizationRoot || - appTokenRoot === wantedAuthorizationRoot; - if (fits) matched.push(app); + // Exact HOST match against the declared endpoint (token host first, since the + // token endpoint is what the SDK actually calls; authorize host as fallback + // when no token endpoint was declared). + const exactHostMatch = wantedTokenHost + ? hostEq(appTokenHost, wantedTokenHost) + : hostEq(appAuthorizationHost, wantedAuthorizationHost) || + hostEq(appTokenHost, wantedAuthorizationHost); + // Same registrable root domain (the old, looser heuristic) — now only a + // tier-2 signal. + const rootMatch = wantedTokenRoot + ? hostEq(appTokenRoot, wantedTokenRoot) + : hostEq(appAuthorizationRoot, wantedAuthorizationRoot) || + hostEq(appTokenRoot, wantedAuthorizationRoot); + if (matchesIntent(app) || exactHostMatch) matched.push(app); + else if (rootMatch) nearMatches.push(app); else unmatched.push(app); } return { matched: sortUserFirst(matched), + nearMatches: sortUserFirst(nearMatches), unmatched: sortUserFirst(unmatched), endpointMatched: matched.length > 0, }; @@ -146,6 +245,7 @@ export function selectClientsForEndpoints( export function useOAuthClientsForIntegration(opts: { readonly tokenUrl?: string; readonly authorizationUrl?: string; + readonly integration?: IntegrationSlug; readonly requireEndpointMatch?: boolean; }): UseOAuthClientsResult { // Read the optimistic list so a just-registered/edited/removed app paints @@ -153,9 +253,29 @@ export function useOAuthClientsForIntegration(opts: { // lands. The modal's management menu reads the same optimistic atom, so the // picker rows and their actions stay consistent. const clientsResult = useAtomValue(oauthClientsOptimisticAtom); - if (!AsyncResult.isSuccess(clientsResult)) { + const loaded = AsyncResult.isSuccess(clientsResult); + const all = loaded ? (clientsResult.value as readonly OAuthClientOption[]) : EMPTY_CLIENTS; + + // Memoize the grade: `selectClientsForEndpoints` parses every client's URLs + // with tldts, and the modal passes a FRESH inline options object each render, + // so without this every keystroke would re-grade all clients. Key on the + // primitive inputs plus the `all` array reference (a new optimistic list is a + // new array), NOT on `opts` identity. + const selection = useMemo( + () => + selectClientsForEndpoints(all, { + tokenUrl: opts.tokenUrl, + authorizationUrl: opts.authorizationUrl, + integration: opts.integration, + requireEndpointMatch: opts.requireEndpointMatch, + }), + [all, opts.tokenUrl, opts.authorizationUrl, opts.integration, opts.requireEndpointMatch], + ); + + if (!loaded) { return { clients: [], + nearMatches: [], otherClients: [], loading: true, endpointMatched: true, @@ -163,19 +283,16 @@ export function useOAuthClientsForIntegration(opts: { }; } - const all = clientsResult.value as readonly OAuthClientOption[]; - const { matched, unmatched, endpointMatched } = selectClientsForEndpoints(all, { - tokenUrl: opts.tokenUrl, - authorizationUrl: opts.authorizationUrl, - requireEndpointMatch: opts.requireEndpointMatch, - }); - // EXPLICIT outcome: when at least one app matched (or no endpoint was - // declared) we present the matched subset. When an endpoint was declared but - // nothing matched, `clients` is EMPTY — the unmatched apps move to - // `otherClients` for an opt-in escape hatch — and we flag a register CTA so - // the UI offers "register an app" instead of surfacing unrelated providers. + const { matched, nearMatches, unmatched, endpointMatched } = selection; + // EXPLICIT outcome: `clients` is the tier-1 (exact/intent) subset the picker + // defaults into. `nearMatches` (root-domain-only) is always surfaced, but in a + // separate subdued section so it is never mistaken for a real match. When an + // endpoint was declared but nothing exact-matched, `clients` is EMPTY and a + // register CTA is flagged; the unrelated `unmatched` apps stay in `otherClients` + // as the last-resort escape hatch. return { clients: endpointMatched ? matched : [], + nearMatches, otherClients: endpointMatched ? [] : unmatched, loading: false, endpointMatched, @@ -183,6 +300,49 @@ export function useOAuthClientsForIntegration(opts: { }; } +/** + * The auto-registered (DCR) clients relevant to an integration, for the + * management surface (list + delete). These are hidden from the app picker but + * must stay visible SOMEWHERE. A DCR client is relevant when it records this + * integration as its origin (Part A stamps `origin.integration`), or — for + * legacy clients minted before the stamp — when its endpoints share the + * integration's declared/discovered root domain. + */ +export function selectDcrClientsForIntegration( + all: readonly OAuthClientOption[], + opts: { + readonly integration?: IntegrationSlug; + readonly tokenUrl?: string; + readonly authorizationUrl?: string; + }, +): readonly OAuthClientOption[] { + const wantedTokenRoot = opts.tokenUrl ? getRootDomain(opts.tokenUrl) : undefined; + const wantedAuthorizationRoot = opts.authorizationUrl + ? getRootDomain(opts.authorizationUrl) + : undefined; + const relevant = all.filter((app) => { + if (!isDcrClient(app)) return false; + if ( + opts.integration != null && + app.origin.kind === "dynamic_client_registration" && + app.origin.integration != null && + app.origin.integration === opts.integration + ) { + return true; + } + if (!wantedTokenRoot && !wantedAuthorizationRoot) return false; + const appTokenRoot = getRootDomain(app.tokenUrl); + const appAuthorizationRoot = getRootDomain(app.authorizationUrl); + return ( + hostEq(appTokenRoot, wantedTokenRoot) || + hostEq(appAuthorizationRoot, wantedTokenRoot) || + hostEq(appAuthorizationRoot, wantedAuthorizationRoot) || + hostEq(appTokenRoot, wantedAuthorizationRoot) + ); + }); + return sortUserFirst(relevant); +} + const slugifyName = (name: string): string => name .toLowerCase() @@ -199,6 +359,24 @@ export function uniqueClientSlug(name: string, existing: readonly string[]): OAu return OAuthClientSlug.make(`${base}-${suffix}`); } +/** + * Optimistic (client-side) DCR client slug derived from the authorization + * server host, for immediate/placeholder display only. + * + * The AUTHORITATIVE slug is computed server-side by `dcrClientSlug` in + * `packages/core/sdk/src/oauth-service.ts`, which is resource-aware (an AS + * serving multiple RFC 8707 resources gets distinct, hash-suffixed slugs). This + * host-only form deliberately ignores resource: it is a best-effort placeholder + * the UI shows before the server responds, and the server recomputes and + * persists the real slug on registration. Do NOT rely on it matching the stored + * slug for a multi-resource server. + */ +export function optimisticDcrClientSlug(issuerOrEndpoint: string): OAuthClientSlug { + const host = hostOf(issuerOrEndpoint); + const base = host === undefined ? "" : slugifyName(host); + return OAuthClientSlug.make(`dcr-${base || "authorization-server"}`); +} + /** Humanize a client slug for display ("spotify-prod" → "Spotify prod"). */ export function clientDisplayName(slug: string): string { const text = slug.replace(/[-_]/g, " ").trim();