From 91e3f2f0d8159c6f2b592268d1c6ffa0b5ea8fa7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Sibiril?= <81782+aureliensibiril@users.noreply.github.com> Date: Sat, 11 Jul 2026 19:47:56 +0200 Subject: [PATCH] Enforce ManagedAPIKey for resource ID flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RequiresManagedResourceID only has meaning for a ManagedAPIKey provider: ManagedConnectorReady consults it exclusively on that path. A non-managed provider that set it would advertise normally with the requirement silently doing nothing. Reject the combination at registration, matching the ManagedAPIKey/SupportsAPIKey mutual-exclusion guard already in Register, and cover it with a TestRegistry_Register subtest. Signed-off-by: Aurélien Sibiril <81782+aureliensibiril@users.noreply.github.com> --- pkg/connector/provider/registry.go | 8 ++++++++ pkg/connector/provider/registry_test.go | 13 +++++++++++++ 2 files changed, 21 insertions(+) diff --git a/pkg/connector/provider/registry.go b/pkg/connector/provider/registry.go index bd2b3ea09..925453e37 100644 --- a/pkg/connector/provider/registry.go +++ b/pkg/connector/provider/registry.go @@ -120,6 +120,14 @@ func (r *Registry) Register(reg *Registration) error { return fmt.Errorf("cannot register connector provider %q: ManagedAPIKey is mutually exclusive with SupportsAPIKey and SupportsClientCredentials", reg.Provider) } + // RequiresManagedResourceID only has meaning for a ManagedAPIKey provider: + // ManagedConnectorReady consults it exclusively on that path, so setting it + // on a non-managed provider is a silently ineffective flag. Reject it at + // startup rather than let the requirement quietly do nothing. + if reg.RequiresManagedResourceID && !reg.ManagedAPIKey { + return fmt.Errorf("cannot register connector provider %q: RequiresManagedResourceID requires ManagedAPIKey", reg.Provider) + } + // BuildTokenURLForDomain and BuildTokenURLForSite both build the token // endpoint host, but from different sources (a callback param vs. the // signed state). CompleteWithState checks them in order, so setting both diff --git a/pkg/connector/provider/registry_test.go b/pkg/connector/provider/registry_test.go index 2e30e184a..3725a182b 100644 --- a/pkg/connector/provider/registry_test.go +++ b/pkg/connector/provider/registry_test.go @@ -138,6 +138,19 @@ func TestRegistry_Register(t *testing.T) { assert.Contains(t, err.Error(), "mutually exclusive") }) + t.Run("RequiresManagedResourceID requires ManagedAPIKey", func(t *testing.T) { + t.Parallel() + + r := provider.NewRegistry() + err := r.Register(&provider.Registration{ + Provider: coredata.ConnectorProviderSlack, + DisplayName: "Slack", + RequiresManagedResourceID: true, + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "RequiresManagedResourceID requires ManagedAPIKey") + }) + t.Run("duplicate registration", func(t *testing.T) { t.Parallel()