diff --git a/api/openapi/markdown/web-openapi.md b/api/openapi/markdown/web-openapi.md index 8ecec4c5db6..0d2efc3d6bd 100644 --- a/api/openapi/markdown/web-openapi.md +++ b/api/openapi/markdown/web-openapi.md @@ -1751,7 +1751,7 @@ Too many emails have been sent from this IP address recently. Please try again l | ---- | ---- | ----------- | -------- | | connection_revision | string (uuid) | Opaque revision issued for the current Web App. Use App parameters for tool-node operations or the owned Connection revision endpoint for Provider management. | Yes | | credentials | object | | Yes | -| name | string | | Yes | +| name | string | Omit or leave empty to generate an incremental API KEY name. | No | | node_id | string | | Yes | #### EndUserConnectionListQuery @@ -1775,7 +1775,7 @@ Too many emails have been sent from this IP address recently. Please try again l | Name | Type | Description | Required | | ---- | ---- | ----------- | -------- | | connection_revision | string (uuid) | Opaque revision issued for the current Web App. Use App parameters for tool-node operations or the owned Connection revision endpoint for Provider management. | Yes | -| name | string | | Yes | +| name | string | Omit or leave empty to generate an incremental AUTH name. | No | | node_id | string | | Yes | #### EndUserConnectionOAuthResponse diff --git a/api/repositories/end_user_connection_repository.py b/api/repositories/end_user_connection_repository.py index 4160f14b831..eaacb6edfad 100644 --- a/api/repositories/end_user_connection_repository.py +++ b/api/repositories/end_user_connection_repository.py @@ -17,6 +17,8 @@ from constants.end_user_connections import ( ) from context.connection_execution import ConnectionExecutionContext from core.app.public_runtime import published_app_filter +from core.helper.name_generator import generate_incremental_name +from core.plugin.entities.plugin_daemon import CredentialType from libs.datetime_utils import naive_utc_now from models.account import Account, AccountStatus from models.end_user_connection import ( @@ -225,6 +227,25 @@ class EndUserConnectionRepository: else EndUserConnection.end_user_id.is_(None), ) + @classmethod + def _generate_connection_name( + cls, + session: Session, + context: WebConnectionContext, + provider: str, + credential_type: ConnectionCredentialType, + ) -> str: + """Generate under the caller's owner lock so concurrent creations see prior names.""" + names = session.scalars( + sa.select(EndUserConnection.name).where( + EndUserConnection.tenant_id == context.tenant_id, + *cls._owner(context.owner), + EndUserConnection.provider == provider, + EndUserConnection.credential_type == credential_type, + ) + ).all() + return generate_incremental_name(names, CredentialType(credential_type.value).get_name()) + @staticmethod def _require_context(session: Session, context: WebConnectionContext, *, lock: bool) -> App: stmt = ( @@ -690,7 +711,7 @@ class EndUserConnectionRepository: *, workflow_id: str, provider: str, - name: str, + name: str | None, encrypted_credentials: str, revision_acceptance: ConnectionRevisionAcceptance, ) -> ConnectionSummary: @@ -698,6 +719,8 @@ class EndUserConnectionRepository: app = self._require_context(session, context, lock=True) self._require_workflow_scope(session, context, app, workflow_id, None) owner = context.owner + if not name: + name = self._generate_connection_name(session, context, provider, ConnectionCredentialType.API_KEY) row = EndUserConnection( tenant_id=context.tenant_id, account_id=owner.account_id, @@ -921,7 +944,7 @@ class EndUserConnectionRepository: ): raise ConnectionReauthorizationRequiredError() if attempt.guard is None: - if attempt.target is None or attempt.name is None: + if attempt.target is None: raise ConnectionConfigurationError() if attempt.target_workflow_id is None: raise ConnectionConfigurationError() @@ -932,7 +955,10 @@ class EndUserConnectionRepository: end_user_id=context.owner.end_user_id, provider=attempt.provider, credential_type=ConnectionCredentialType.OAUTH2, - name=attempt.name, + name=attempt.name + or self._generate_connection_name( + session, context, attempt.provider, ConnectionCredentialType.OAUTH2 + ), encrypted_credentials=encrypted_credentials, status=ConnectionCredentialStatus.CONNECTED, oauth_client_source=client.source, diff --git a/api/services/end_user_connections/contracts.py b/api/services/end_user_connections/contracts.py index 995e395e044..bb74853f3a3 100644 --- a/api/services/end_user_connections/contracts.py +++ b/api/services/end_user_connections/contracts.py @@ -44,7 +44,9 @@ class ConnectionTargetInput(ConnectionContractInput): class CreateApiKeyConnectionInput(ConnectionTargetInput): - name: str = Field(min_length=1, max_length=100) + name: str | None = Field( + default=None, max_length=100, description="Omit or leave empty to generate an incremental API KEY name." + ) # Provider-schema validation and remote checks precede persistence. credentials: dict[str, JsonValue] = Field(min_length=1, repr=False) @@ -87,7 +89,9 @@ class ConnectionRevisionView(BaseModel): class StartConnectionOAuthInput(ConnectionTargetInput): - name: str = Field(min_length=1, max_length=100) + name: str | None = Field( + default=None, max_length=100, description="Omit or leave empty to generate an incremental AUTH name." + ) class ListConnectionsInput(ConnectionInput): diff --git a/api/services/end_user_connections/management.py b/api/services/end_user_connections/management.py index 1d2a993f722..6dd7c8783f2 100644 --- a/api/services/end_user_connections/management.py +++ b/api/services/end_user_connections/management.py @@ -66,10 +66,12 @@ class ConnectionStore(Protocol): *, workflow_id: str, provider: str, - name: str, + name: str | None, encrypted_credentials: str, revision_acceptance: ConnectionRevisionAcceptance, - ) -> ConnectionSummary: ... + ) -> ConnectionSummary: + """Generate an omitted or empty name within the locked owner/Provider scope.""" + ... def update( self, context: WebConnectionContext, guard: ConnectionMutationGuard, changes: ConnectionChanges diff --git a/api/services/end_user_connections/oauth.py b/api/services/end_user_connections/oauth.py index 5b95ff53b42..6007616dd03 100644 --- a/api/services/end_user_connections/oauth.py +++ b/api/services/end_user_connections/oauth.py @@ -66,9 +66,10 @@ class ConnectionOAuthAttempt: provider: str client: ConnectionOAuthClient state_digest: str = field(repr=False) - # Creation captures a published target/name; reauthorization captures a CAS guard. + # Creation captures a published target; reauthorization captures a CAS guard. target: ConnectionTargetInput | None target_workflow_id: str | None + # None generates a name on creation and preserves the name on reauthorization. name: str | None guard: ConnectionMutationGuard | None connection_revision: str @@ -266,7 +267,7 @@ class OAuthConnectionService: if connection_mutation_guard(context, record.access) != attempt.guard: raise ConnectionConflictError() else: - if attempt.target is None or attempt.name is None: + if attempt.target is None: raise ConnectionOAuthError() target = self._targets.resolve(context, attempt.target) if target.provider != attempt.provider or target.oauth_client_source != attempt.client.source: diff --git a/api/tests/unit_tests/controllers/web/test_connection_oauth.py b/api/tests/unit_tests/controllers/web/test_connection_oauth.py index a0647e337a0..c6982902b62 100644 --- a/api/tests/unit_tests/controllers/web/test_connection_oauth.py +++ b/api/tests/unit_tests/controllers/web/test_connection_oauth.py @@ -85,8 +85,19 @@ def callback_query(response: TestResponse) -> str: return f"state={state}&code=provider-code" -def test_cross_host_oauth_callback_returns_to_cookie_owner(client: FlaskClient, harness: OAuthHarness) -> None: - response = start(client, harness) +@pytest.mark.parametrize("name", [None, "", "My calendar"]) +def test_cross_host_oauth_callback_returns_to_cookie_owner( + client: FlaskClient, harness: OAuthHarness, name: str | None +) -> None: + payload = harness.command().model_dump(mode="json", exclude={"name"}) + if name is not None: + payload["name"] = name + response = client.post( + "/api/connections/oauth/authorization-url", + base_url="https://web-api.example", + headers={"X-App-Code": harness.app_code}, + json=payload, + ) assert response.status_code == 200 assert set(response.get_json()) == {"authorization_url"} cookie = response.headers["Set-Cookie"] @@ -112,6 +123,9 @@ def test_cross_host_oauth_callback_returns_to_cookie_owner(client: FlaskClient, assert "X-App-Passport" not in harness.handler.callback_headers with harness.base.sessions() as session: assert session.scalar(sa.select(sa.func.count()).select_from(EndUserConnection)) == 1 + connection = session.scalar(sa.select(EndUserConnection)) + assert connection is not None + assert connection.name == (name or "AUTH 1") replay = client.get(routed.location) assert replay.status_code == 400 assert replay.get_json()["code"] == "connection_oauth_failed" @@ -127,6 +141,12 @@ def test_callback_requires_initiating_browser_cookie(client: FlaskClient, harnes assert harness.handler.exchanges == 0 +def test_oauth_schema_documents_optional_name() -> None: + schema = connection_oauth.EndUserConnectionOAuthPayload.model_json_schema() + assert "name" not in schema["required"] + assert schema["properties"]["name"]["anyOf"] == [{"maxLength": 100, "type": "string"}, {"type": "null"}] + + def test_invalid_oauth_payload_does_not_echo_input(client: FlaskClient, harness: OAuthHarness) -> None: response = client.post( "/api/connections/oauth/authorization-url", diff --git a/api/tests/unit_tests/controllers/web/test_connections.py b/api/tests/unit_tests/controllers/web/test_connections.py index 7619c39ddd9..4b15e6763ac 100644 --- a/api/tests/unit_tests/controllers/web/test_connections.py +++ b/api/tests/unit_tests/controllers/web/test_connections.py @@ -45,6 +45,7 @@ def client(harness: ConnectionHarness, monkeypatch: pytest.MonkeyPatch) -> Itera @pytest.mark.parametrize("anonymous", [False, True]) +@pytest.mark.parametrize("name", [None, "", "Custom name"]) @pytest.mark.parametrize("allow_anonymous", [False, True]) def test_http_api_key_management_round_trip( client: FlaskClient, @@ -52,13 +53,17 @@ def test_http_api_key_management_round_trip( config_overrides: Callable[..., None], anonymous: bool, allow_anonymous: bool, + name: str | None, ) -> None: config_overrides(END_USER_AUTH_ALLOW_ANONYMOUS=allow_anonymous) if anonymous: with harness.sessions.begin() as session: session.execute(sa.update(EndUser).where(EndUser.id == harness.context.end_user_id).values(account_id=None)) harness.context = replace(harness.context, account_id=None) - response = client.post("/api/connections", json=harness.command().model_dump(mode="json")) + payload = harness.command().model_dump(mode="json", exclude={"name"}) + if name is not None: + payload["name"] = name + response = client.post("/api/connections", json=payload) if anonymous and not allow_anonymous: assert response.status_code == 401 assert response.get_json()["code"] == "end_user_login_required" @@ -66,6 +71,7 @@ def test_http_api_key_management_round_trip( return assert response.status_code == 201 data = response.get_json() + assert data["name"] == (name or "API KEY 1") assert data["credential_status"] == "connected" assert isinstance(data["created_at"], int) assert "original-secret" not in response.get_data(as_text=True) @@ -206,6 +212,9 @@ def test_patch_rejects_empty_or_null_changes( def test_public_schemas_match_patch_and_timestamp_semantics() -> None: + create = connections.EndUserConnectionCreatePayload.model_json_schema() + assert "name" not in create["required"] + assert create["properties"]["name"]["anyOf"] == [{"maxLength": 100, "type": "string"}, {"type": "null"}] payload = connections.EndUserConnectionUpdatePayload.model_json_schema() assert payload["minProperties"] == 1 assert payload["properties"]["name"]["type"] == "string" diff --git a/api/tests/unit_tests/repositories/test_end_user_connection_repository.py b/api/tests/unit_tests/repositories/test_end_user_connection_repository.py index 421c4d849e0..6deb83ec320 100644 --- a/api/tests/unit_tests/repositories/test_end_user_connection_repository.py +++ b/api/tests/unit_tests/repositories/test_end_user_connection_repository.py @@ -1,16 +1,109 @@ -"""Persisted authorization revisions are immutable, deduplicated, and Web App scoped.""" +"""Connection persistence preserves owner scope and immutable authorization revisions.""" from concurrent.futures import ThreadPoolExecutor +from dataclasses import replace from uuid import UUID, uuid4 import pytest import sqlalchemy as sa from sqlalchemy.orm import Session, sessionmaker +from constants.end_user_connections import ConnectionCredentialType, OAuthClientSource from models.end_user_connection import EndUserConnectionRevision +from models.model import EndUser from repositories.end_user_connection_repository import EndUserConnectionRepository from services.end_user_connections.errors import ConnectionRevisionChangedError -from services.end_user_connections.revision import ConnectionContractSnapshot, ProviderContractSnapshot +from services.end_user_connections.revision import ( + ConnectionContractSnapshot, + ConnectionRevisionAcceptance, + ProviderContractSnapshot, +) +from tests.helpers.end_user_api_key import make_harness +from tests.helpers.end_user_connection_contract import new_connection +from tests.helpers.end_user_oauth import make_oauth_harness + + +@pytest.mark.parametrize("anonymous", [False, True]) +@pytest.mark.parametrize("credential_type", [ConnectionCredentialType.API_KEY, ConnectionCredentialType.OAUTH2]) +def test_connection_names_increment_within_tenant_owner_provider_and_type( + sqlite_session_factory: sessionmaker[Session], + monkeypatch: pytest.MonkeyPatch, + anonymous: bool, + credential_type: ConnectionCredentialType, +) -> None: + oauth = ( + make_oauth_harness(sqlite_session_factory, monkeypatch) + if credential_type == ConnectionCredentialType.OAUTH2 + else None + ) + harness = oauth.base if oauth is not None else make_harness(sqlite_session_factory) + attempt = ( + oauth.service.consume_attempt(oauth.start().context_id, provider="vendor/calendar/calendar") + if oauth is not None + else None + ) + prefix = "AUTH" if oauth is not None else "API KEY" + context = harness.context + if anonymous: + with sqlite_session_factory.begin() as session: + user = session.get(EndUser, context.end_user_id) + assert user is not None + user.account_id = None + context = replace(context, account_id=None) + owner = context.owner + seed = replace( + new_connection(), + tenant_id=context.tenant_id, + account_id=owner.account_id, + end_user_id=owner.end_user_id, + credential_type=credential_type, + oauth_client_source=OAuthClientSource.WORKSPACE if oauth is not None else None, + ) + with sqlite_session_factory.begin() as session: + session.add_all( + [ + replace(seed, name=f"{prefix} 1"), + replace(seed, name=f"{prefix} 3"), + replace(seed, name="Custom name"), + replace(seed, name=f"{prefix} x"), + replace(seed, name=f"{prefix} 90", tenant_id=str(uuid4())), + replace( + seed, + name=f"{prefix} 91", + account_id=None if anonymous else str(uuid4()), + end_user_id=str(uuid4()) if anonymous else None, + ), + replace(seed, name=f"{prefix} 92", provider="vendor/mail/mail"), + replace( + seed, + name=f"{prefix} 93", + credential_type=ConnectionCredentialType.API_KEY + if oauth is not None + else ConnectionCredentialType.OAUTH2, + oauth_client_source=None if oauth is not None else OAuthClientSource.WORKSPACE, + ), + ] + ) + acceptance = ConnectionRevisionAcceptance(harness.connection_revision, None) + for name, expected in [(None, f"{prefix} 4"), ("", f"{prefix} 5"), ("Personal key", "Personal key")]: + if attempt is None: + created = harness.repository.create_api_key( + context, + workflow_id=harness.workflow_id, + provider=seed.provider, + name=name, + encrypted_credentials="encrypted-secret", + revision_acceptance=acceptance, + ) + else: + created = harness.repository.save_oauth( + replace(attempt, context=context, name=name), + encrypted_credentials="encrypted-secret", + expires_at=None, + account_label=None, + revision_acceptance=acceptance, + ) + assert created.name == expected @pytest.fixture diff --git a/api/tests/unit_tests/services/end_user_connections/test_contracts.py b/api/tests/unit_tests/services/end_user_connections/test_contracts.py index 9fe0de598ce..0d7aedc9e8b 100644 --- a/api/tests/unit_tests/services/end_user_connections/test_contracts.py +++ b/api/tests/unit_tests/services/end_user_connections/test_contracts.py @@ -54,12 +54,50 @@ def test_create_input_keeps_credentials_out_of_repr_and_validation_messages() -> assert "secret-sentinel" not in str(exc.value) -@pytest.mark.parametrize("name", ["", "n" * 101]) -def test_create_name_respects_storage_limits(name: str) -> None: +def test_create_name_can_be_omitted() -> None: + payload = create_payload() + del payload["name"] + assert CreateApiKeyConnectionInput.model_validate(payload).name is None + + +@pytest.mark.parametrize("name", [None, ""]) +def test_create_name_can_be_empty(name: str | None) -> None: + assert CreateApiKeyConnectionInput.model_validate({**create_payload(), "name": name}).name == name + + +@pytest.mark.parametrize("name", ["n" * 101, 42]) +def test_create_name_respects_storage_limits(name: object) -> None: with pytest.raises(ValidationError): CreateApiKeyConnectionInput.model_validate({**create_payload(), "name": name}) +def test_oauth_name_can_be_omitted() -> None: + assert ( + StartConnectionOAuthInput.model_validate( + {"connection_revision": create_payload()["connection_revision"], "node_id": "tool-1"} + ).name + is None + ) + + +@pytest.mark.parametrize("name", [None, "", "Personal account"]) +def test_oauth_name_can_be_empty_or_explicit(name: str | None) -> None: + assert ( + StartConnectionOAuthInput.model_validate( + {"connection_revision": create_payload()["connection_revision"], "node_id": "tool-1", "name": name} + ).name + == name + ) + + +@pytest.mark.parametrize("name", ["n" * 101, 42]) +def test_oauth_name_respects_storage_limits(name: object) -> None: + with pytest.raises(ValidationError): + StartConnectionOAuthInput.model_validate( + {"connection_revision": create_payload()["connection_revision"], "node_id": "tool-1", "name": name} + ) + + @pytest.mark.parametrize("node_id", ["", "n" * 256]) def test_create_node_reference_respects_storage_limits(node_id: str) -> None: with pytest.raises(ValidationError): diff --git a/api/tests/unit_tests/services/end_user_connections/test_oauth.py b/api/tests/unit_tests/services/end_user_connections/test_oauth.py index 3c60ace46e9..0c8c84eb748 100644 --- a/api/tests/unit_tests/services/end_user_connections/test_oauth.py +++ b/api/tests/unit_tests/services/end_user_connections/test_oauth.py @@ -21,6 +21,7 @@ from models.model import App, EndUser, Site from models.tools import ToolOAuthSystemClient, ToolOAuthTenantClient from models.workflow import Workflow from services.end_user_connection_oauth_adapters import PluginService +from services.end_user_connections.contracts import StartConnectionOAuthInput from services.end_user_connections.errors import ( ConnectionConfigurationError, ConnectionConflictError, @@ -72,6 +73,24 @@ def test_authorization_persists_encrypted_grant_and_masked_identity_only(harness assert harness.handler.exchanges == 1 +def test_unnamed_authorizations_generate_names_when_callbacks_complete(harness: OAuthHarness) -> None: + command = StartConnectionOAuthInput.model_validate(harness.command().model_dump(exclude={"name"})) + pending = [ + harness.service.start_oauth( + harness.base.context, command, callback_url="http://web-api.example/api/connections/oauth" + ) + for _ in range(2) + ] + named = harness.service.start_oauth( + harness.base.context, + command.model_copy(update={"name": "AUTH 7"}), + callback_url="http://web-api.example/api/connections/oauth", + ) + assert harness.finish(named).name == "AUTH 7" + assert harness.finish(pending[1]).name == "AUTH 8" + assert harness.finish(pending[0]).name == "AUTH 9" + + @pytest.mark.parametrize( "query", [b"code=secret", b"state=wrong&code=secret", b"state=a&state=b", b"error=denied", b"\xff"] ) @@ -184,8 +203,14 @@ def test_oauth_does_not_depend_on_site_after_authenticated_start( assert stored.access.revision == 2 -def test_reauthorization_preserves_id_and_name_and_replaces_grant(harness: OAuthHarness) -> None: - connection = harness.finish(harness.start()) +@pytest.mark.parametrize("name", [None, "My calendar"]) +def test_reauthorization_preserves_id_and_name_and_replaces_grant(harness: OAuthHarness, name: str | None) -> None: + initial = harness.service.start_oauth( + harness.base.context, + harness.command().model_copy(update={"name": name}), + callback_url="http://web-api.example/api/connections/oauth", + ) + connection = harness.finish(initial) with harness.base.sessions.begin() as session: session.execute( sa.update(EndUserConnection) diff --git a/packages/contracts/generated/api/web/types.gen.ts b/packages/contracts/generated/api/web/types.gen.ts index 55691f42a7b..b0d0302fedb 100644 --- a/packages/contracts/generated/api/web/types.gen.ts +++ b/packages/contracts/generated/api/web/types.gen.ts @@ -362,7 +362,7 @@ export type EndUserConnectionCreatePayload = { credentials: { [key: string]: JsonValue } - name: string + name?: string | null node_id: string } @@ -380,7 +380,7 @@ export type EndUserConnectionListResponse = { export type EndUserConnectionOAuthPayload = { connection_revision: string - name: string + name?: string | null node_id: string } diff --git a/packages/contracts/generated/api/web/zod.gen.ts b/packages/contracts/generated/api/web/zod.gen.ts index 99c7d9eafe6..466e3ad8504 100644 --- a/packages/contracts/generated/api/web/zod.gen.ts +++ b/packages/contracts/generated/api/web/zod.gen.ts @@ -334,7 +334,7 @@ export const zEndUserConnectionListQuery = z.object({ */ export const zEndUserConnectionOAuthPayload = z.object({ connection_revision: z.uuid(), - name: z.string().min(1).max(100), + name: z.string().max(100).nullish(), node_id: z.string().min(1).max(255), }) @@ -973,7 +973,7 @@ export const zJsonValue = z.unknown() export const zEndUserConnectionCreatePayload = z.object({ connection_revision: z.uuid(), credentials: z.record(z.string(), zJsonValue), - name: z.string().min(1).max(100), + name: z.string().max(100).nullish(), node_id: z.string().min(1).max(255), })