From 37121e7bacdfd7c7671aca53f6e51c896ea3b55b 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 16:50:32 +0200 Subject: [PATCH] Verify Crisp website ownership before connecting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Crisp is a managed (Model B) connector: Probo holds one plugin token server-side and each connection carries only a Website ID. Nothing stops one organization from entering another organization's Website ID, so prove control of the website before creating the connection. Probo derives a per-(organization, website) verification code as an HMAC over the token secret and exposes it through a new crispVerificationCode query. The customer pastes it into the Probo plugin's per-website settings; at connect time the resolver reads the setting back through the managed plugin token and requires a constant-time match before any row is written. The managed key and plugin ID come from bootstrap, so the connector stays hidden until the deployment configures them. The settings fetch is injected so the create-time gate's branch wiring is unit-tested (mismatch and not-subscribed reject, internal errors stay generic, a matching code passes), and the managed-versus-client key resolution is covered too. Signed-off-by: Aurélien Sibiril <81782+aureliensibiril@users.noreply.github.com> --- pkg/accessreview/drivers/crisp.go | 86 +++++ .../drivers/crisp_settings_test.go | 124 +++++++ pkg/accessreview/review_engine.go | 6 + pkg/accessreview/source_name_worker.go | 6 + pkg/accessreview/source_service.go | 7 + pkg/bootstrap/builder.go | 26 ++ pkg/bootstrap/builder_test.go | 55 +++ pkg/connector/provider/apply.go | 30 ++ pkg/connector/provider/crisp.go | 18 +- pkg/connector/provider/registry.go | 82 ++++- pkg/connector/provider/registry_test.go | 115 ++++++ pkg/connector/provider/types.go | 11 + pkg/probod/probod.go | 20 + pkg/probodconfig/connector_config.go | 30 ++ pkg/probodconfig/connector_config_test.go | 81 ++++ pkg/server/api/console/v1/base_resolvers.go | 30 +- .../api/console/v1/connector_resolvers.go | 23 +- .../api/console/v1/connector_settings.go | 138 ++++++- pkg/server/api/console/v1/crisp.go | 55 +++ pkg/server/api/console/v1/crisp_test.go | 347 ++++++++++++++++++ .../api/console/v1/graphql/base.graphql | 2 + .../api/console/v1/graphql/connector.graphql | 16 +- pkg/server/api/console/v1/graphql_handler.go | 2 + pkg/server/api/console/v1/resolver.go | 2 + 24 files changed, 1294 insertions(+), 18 deletions(-) create mode 100644 pkg/accessreview/drivers/crisp_settings_test.go create mode 100644 pkg/probodconfig/connector_config_test.go create mode 100644 pkg/server/api/console/v1/crisp.go create mode 100644 pkg/server/api/console/v1/crisp_test.go diff --git a/pkg/accessreview/drivers/crisp.go b/pkg/accessreview/drivers/crisp.go index ddd0851b2..567d500f0 100644 --- a/pkg/accessreview/drivers/crisp.go +++ b/pkg/accessreview/drivers/crisp.go @@ -17,6 +17,7 @@ package drivers import ( "context" "encoding/json" + "errors" "fmt" "net/http" "net/url" @@ -25,6 +26,32 @@ import ( "go.probo.inc/probo/pkg/coredata" ) +// ErrCrispPluginNotSubscribed is returned by GetCrispSubscriptionSettings when +// Crisp answers 404: the plugin is not subscribed to the given website, so no +// per-website settings exist yet. It is an expected verification state (the +// customer has not installed/configured the plugin on that website) rather than +// a failure, and callers distinguish it with errors.Is. +var ErrCrispPluginNotSubscribed = errors.New("crisp plugin not subscribed to website") + +// CrispSubscriptionSettings is the schema-defined, per-website configuration of +// the Probo Crisp plugin. Only the field Probo relies on for ownership +// verification is modeled; unknown schema properties are ignored on decode. +type CrispSubscriptionSettings struct { + ProboVerificationCode string `json:"probo_verification_code"` +} + +// crispSubscriptionSettingsResponse is the envelope of +// GET /v1/plugins/subscription/{website_id}/{plugin_id}/settings. The active +// per-website configuration lives at data.settings; data itself also carries +// subscription metadata (ids, secret token, JSONSchema, form/callback URLs) +// that verification does not need. +type crispSubscriptionSettingsResponse struct { + Error bool `json:"error"` + Data struct { + Settings CrispSubscriptionSettings `json:"settings"` + } `json:"data"` +} + const ( crispAPIBaseURL = "https://api.crisp.chat/v1" // crispTierHeader selects the token tier on every Crisp request. A Probo @@ -124,6 +151,65 @@ func (d *CrispDriver) ListAccounts(ctx context.Context) ([]AccountRecord, error) return records, nil } +// GetCrispSubscriptionSettings reads the Probo plugin's per-website +// subscription settings so the create-connector resolver can verify website +// ownership (matching probo_verification_code). The httpClient must already +// attach the plugin Basic credential (identifier:key); this helper only sets +// the Accept and X-Crisp-Tier headers, mirroring ListAccounts. A 404 (plugin +// not subscribed to the website) is reported as ErrCrispPluginNotSubscribed so +// callers can message it distinctly from a hard failure. +func GetCrispSubscriptionSettings( + ctx context.Context, + httpClient *http.Client, + websiteID string, + pluginID string, +) (*CrispSubscriptionSettings, error) { + endpoint, err := url.JoinPath( + crispAPIBaseURL, + "plugins", "subscription", + url.PathEscape(websiteID), + url.PathEscape(pluginID), + "settings", + ) + if err != nil { + return nil, fmt.Errorf("cannot build crisp subscription settings URL: %w", err) + } + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil) + if err != nil { + return nil, fmt.Errorf("cannot create crisp subscription settings request: %w", err) + } + + req.Header.Set("Accept", "application/json") + req.Header.Set(crispTierHeader, crispTierValue) + + httpResp, err := httpClient.Do(req) + if err != nil { + return nil, fmt.Errorf("cannot execute crisp subscription settings request: %w", err) + } + + defer func() { _ = httpResp.Body.Close() }() + + if httpResp.StatusCode == http.StatusNotFound { + return nil, ErrCrispPluginNotSubscribed + } + + if httpResp.StatusCode < 200 || httpResp.StatusCode >= 300 { + return nil, fmt.Errorf("cannot fetch crisp subscription settings: unexpected status %d", httpResp.StatusCode) + } + + var resp crispSubscriptionSettingsResponse + if err := json.NewDecoder(httpResp.Body).Decode(&resp); err != nil { + return nil, fmt.Errorf("cannot decode crisp subscription settings response: %w", err) + } + + if resp.Error { + return nil, fmt.Errorf("cannot fetch crisp subscription settings: crisp reported an error") + } + + return &resp.Data.Settings, nil +} + func crispFullName(details crispOperatorDetails, fallback string) string { if name := strings.TrimSpace(details.FirstName + " " + details.LastName); name != "" { return name diff --git a/pkg/accessreview/drivers/crisp_settings_test.go b/pkg/accessreview/drivers/crisp_settings_test.go new file mode 100644 index 000000000..231a378c4 --- /dev/null +++ b/pkg/accessreview/drivers/crisp_settings_test.go @@ -0,0 +1,124 @@ +// Copyright (c) 2026 Probo Inc . +// +// Permission to use, copy, modify, and/or distribute this software for any +// purpose with or without fee is hereby granted, provided that the above +// copyright notice and this permission notice appear in all copies. +// +// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH +// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY +// AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, +// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM +// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR +// OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR +// PERFORMANCE OF THIS SOFTWARE. + +package drivers + +import ( + "context" + "errors" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGetCrispSubscriptionSettings(t *testing.T) { + t.Parallel() + + const ( + websiteID = "e8592878-c0d0-4632-b2f7-7d882f288d43" + pluginID = "e979a1c3-2c41-4e93-a8ed-410ace27318e" + ) + + t.Run("200 returns the verification code from data.settings", func(t *testing.T) { + t.Parallel() + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // The plugin subscription-settings endpoint is plugins (plural) + // with both website_id and plugin_id in the path. + assert.Equal(t, http.MethodGet, r.Method) + assert.Equal(t, "/v1/plugins/subscription/"+websiteID+"/"+pluginID+"/settings", r.URL.Path) + assert.Equal(t, "plugin", r.Header.Get("X-Crisp-Tier")) + + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"error":false,"reason":"resolved","data":{"plugin_id":"` + pluginID + `","settings":{"probo_verification_code":"ABC234DEF567"}}}`)) + })) + defer srv.Close() + + client := &http.Client{Transport: &hostRewriter{target: srv.URL}} + + settings, err := GetCrispSubscriptionSettings(context.Background(), client, websiteID, pluginID) + require.NoError(t, err) + require.NotNil(t, settings) + assert.Equal(t, "ABC234DEF567", settings.ProboVerificationCode) + }) + + t.Run("200 without the code returns empty (mismatch handled by caller)", func(t *testing.T) { + t.Parallel() + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"error":false,"data":{"settings":{}}}`)) + })) + defer srv.Close() + + client := &http.Client{Transport: &hostRewriter{target: srv.URL}} + + settings, err := GetCrispSubscriptionSettings(context.Background(), client, websiteID, pluginID) + require.NoError(t, err) + require.NotNil(t, settings) + assert.Empty(t, settings.ProboVerificationCode) + }) + + t.Run("404 reports the plugin is not subscribed", func(t *testing.T) { + t.Parallel() + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"error":true,"reason":"subscription_not_found"}`)) + })) + defer srv.Close() + + client := &http.Client{Transport: &hostRewriter{target: srv.URL}} + + settings, err := GetCrispSubscriptionSettings(context.Background(), client, websiteID, pluginID) + require.ErrorIs(t, err, ErrCrispPluginNotSubscribed) + assert.Nil(t, settings) + }) + + t.Run("non-2xx returns an error", func(t *testing.T) { + t.Parallel() + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(`{"error":true,"reason":"server_error"}`)) + })) + defer srv.Close() + + client := &http.Client{Transport: &hostRewriter{target: srv.URL}} + + settings, err := GetCrispSubscriptionSettings(context.Background(), client, websiteID, pluginID) + require.Error(t, err) + assert.False(t, errors.Is(err, ErrCrispPluginNotSubscribed)) + assert.Nil(t, settings) + }) + + t.Run("2xx with error:true returns an error", func(t *testing.T) { + t.Parallel() + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"error":true,"reason":"route_forbidden","data":{}}`)) + })) + defer srv.Close() + + client := &http.Client{Transport: &hostRewriter{target: srv.URL}} + + settings, err := GetCrispSubscriptionSettings(context.Background(), client, websiteID, pluginID) + require.Error(t, err) + assert.Nil(t, settings) + }) +} diff --git a/pkg/accessreview/review_engine.go b/pkg/accessreview/review_engine.go index b213aaab1..b8c7c3320 100644 --- a/pkg/accessreview/review_engine.go +++ b/pkg/accessreview/review_engine.go @@ -236,6 +236,12 @@ func (s *Service) connectorHTTPClient( return s.oauthClient(ctx, oauth2Conn, dbConnector.Provider) } + // Inject the Probo-held key for ManagedAPIKey providers (no-op otherwise), + // resolving it fresh at use time rather than from the connection row. + if err := s.providerRegistry.ApplyManagedAPIKey(dbConnector); err != nil { + return nil, err + } + return dbConnector.Connection.Client(ctx) } diff --git a/pkg/accessreview/source_name_worker.go b/pkg/accessreview/source_name_worker.go index 4751fe2a0..22659a60c 100644 --- a/pkg/accessreview/source_name_worker.go +++ b/pkg/accessreview/source_name_worker.go @@ -236,6 +236,12 @@ func (h *sourceNameHandler) connectorHTTPClient( ) (*http.Client, error) { oauth2Conn, ok := dbConnector.Connection.(*connector.OAuth2Connection) if !ok { + // Inject the Probo-held key for ManagedAPIKey providers (no-op + // otherwise) before building the client. + if err := h.providerRegistry.ApplyManagedAPIKey(dbConnector); err != nil { + return nil, err + } + return dbConnector.Connection.Client(ctx) } diff --git a/pkg/accessreview/source_service.go b/pkg/accessreview/source_service.go index 58845cdb1..ba0cf6b4e 100644 --- a/pkg/accessreview/source_service.go +++ b/pkg/accessreview/source_service.go @@ -364,6 +364,13 @@ func (s *Service) ConnectorHTTPClient( } if httpClient == nil { + // Inject the Probo-held key for ManagedAPIKey providers (no-op + // otherwise), resolving it fresh at use time rather than from the + // connection row. + if err := s.providerRegistry.ApplyManagedAPIKey(&dbConnector); err != nil { + return nil, nil, err + } + var err error httpClient, err = dbConnector.Connection.Client(ctx) diff --git a/pkg/bootstrap/builder.go b/pkg/bootstrap/builder.go index 821835920..fd3fc89e3 100644 --- a/pkg/bootstrap/builder.go +++ b/pkg/bootstrap/builder.go @@ -513,6 +513,32 @@ func (b *Builder) Build() (*probodconfig.FullConfig, error) { ) } + // Crisp is a ManagedAPIKey (Model B) connector: Probo holds one + // Marketplace plugin token (the verbatim "identifier:key" pair) shared + // across all customer connections, and each connection carries only a + // Website ID. The plugin ID is a separate value (the token's Basic + // identifier is not the plugin ID) required by the per-website plugin API + // that verifies website ownership at connect time. Both must be set to + // activate the connector; until then it stays hidden from the driver + // catalog, so it ships deactivated and activates the moment Crisp + // validates the production plugin and both values are configured. + crispPluginToken := b.resolver.getEnv("PROBOD_CONNECTOR_CRISP_PLUGIN_TOKEN") + + crispPluginID := b.resolver.getEnv("PROBOD_CONNECTOR_CRISP_PLUGIN_ID") + if crispPluginToken != "" && crispPluginID != "" { + cfg.Probod.Connectors = append( + cfg.Probod.Connectors, + probodconfig.ConnectorConfig{ + Provider: "CRISP", + Protocol: "api_key", + RawConfig: probodconfig.ConnectorConfigAPIKey{ + APIKey: crispPluginToken, + ResourceID: crispPluginID, + }, + }, + ) + } + if b.resolver.Err() != nil { return nil, b.resolver.Err() } diff --git a/pkg/bootstrap/builder_test.go b/pkg/bootstrap/builder_test.go index 0c093caa3..1dd86df4c 100644 --- a/pkg/bootstrap/builder_test.go +++ b/pkg/bootstrap/builder_test.go @@ -687,6 +687,61 @@ func TestBuilder_Build_SlackConnector(t *testing.T) { assert.Equal(t, "slack-signing-secret", rawSettings["signing-secret"]) } +func TestBuilder_Build_CrispConnector(t *testing.T) { + env := requiredEnv() + env["PROBOD_CONNECTOR_CRISP_PLUGIN_TOKEN"] = "plugin-identifier:plugin-key" + env["PROBOD_CONNECTOR_CRISP_PLUGIN_ID"] = "e979a1c3-2c41-4e93-a8ed-410ace27318e" + + b := NewBuilder(NewResolver(mockEnv(env))) + b.samlCertificate = "test-cert" + b.samlPrivateKey = "test-key" + + cfg, err := b.Build() + require.NoError(t, err) + + require.Len(t, cfg.Probod.Connectors, 1) + connector := cfg.Probod.Connectors[0] + assert.Equal(t, "CRISP", connector.Provider) + assert.Equal(t, "api_key", string(connector.Protocol)) + rawConfig := connector.RawConfig.(probodconfig.ConnectorConfigAPIKey) + assert.Equal(t, "plugin-identifier:plugin-key", rawConfig.APIKey) + assert.Equal(t, "e979a1c3-2c41-4e93-a8ed-410ace27318e", rawConfig.ResourceID) +} + +func TestBuilder_Build_CrispConnectorAbsentWithoutToken(t *testing.T) { + // Without the plugin token the connector must not be configured, which + // is what keeps Crisp deactivated until Crisp validates the plugin. + b := NewBuilder(NewResolver(mockEnv(requiredEnv()))) + b.samlCertificate = "test-cert" + b.samlPrivateKey = "test-key" + + cfg, err := b.Build() + require.NoError(t, err) + + for _, c := range cfg.Probod.Connectors { + assert.NotEqual(t, "CRISP", c.Provider) + } +} + +func TestBuilder_Build_CrispConnectorAbsentWithoutPluginID(t *testing.T) { + // The plugin token alone is not enough: the per-website plugin API needs + // the plugin ID to verify website ownership, so a half-configured Crisp + // connector stays hidden rather than activating in a broken state. + env := requiredEnv() + env["PROBOD_CONNECTOR_CRISP_PLUGIN_TOKEN"] = "plugin-identifier:plugin-key" + + b := NewBuilder(NewResolver(mockEnv(env))) + b.samlCertificate = "test-cert" + b.samlPrivateKey = "test-key" + + cfg, err := b.Build() + require.NoError(t, err) + + for _, c := range cfg.Probod.Connectors { + assert.NotEqual(t, "CRISP", c.Provider) + } +} + func TestBuilder_Build_SAMLAutoGeneration(t *testing.T) { b := NewBuilder(NewResolver(mockEnv(requiredEnv()))) diff --git a/pkg/connector/provider/apply.go b/pkg/connector/provider/apply.go index 9b9c769ea..e25fc83d1 100644 --- a/pkg/connector/provider/apply.go +++ b/pkg/connector/provider/apply.go @@ -72,6 +72,36 @@ func (r *Registry) ApplyOAuth2Defaults(p string, redirectURI string, c *connecto return nil } +// ApplyManagedAPIKey injects the Probo-held API key into a freshly loaded +// ManagedAPIKey connector's connection, so the credential is resolved at +// use time — surviving key rotation and never persisted on the connection +// row (only the extra settings, e.g. a Crisp Website ID, are stored). It is +// a no-op for every non-managed provider, so callers may invoke it +// unconditionally before building a connection's HTTP client. It errors +// when a managed provider's key is unconfigured (the connector was +// deactivated after the connection was created) or when the connection is +// not an API-key connection. +func (r *Registry) ApplyManagedAPIKey(dbConnector *coredata.Connector) error { + reg, ok := r.Get(dbConnector.Provider) + if !ok || !reg.ManagedAPIKey { + return nil + } + + apiKeyConn, ok := dbConnector.Connection.(*connector.APIKeyConnection) + if !ok { + return fmt.Errorf("cannot apply managed api key for provider %q: connection is not an api-key connection", dbConnector.Provider) + } + + key, ok := r.ManagedAPIKey(dbConnector.Provider) + if !ok { + return fmt.Errorf("cannot apply managed api key for provider %q: not configured", dbConnector.Provider) + } + + apiKeyConn.APIKey = key + + return nil +} + // ProbeURL returns the registered probe URL for provider p, or the // empty string if no probe URL is configured. func (r *Registry) ProbeURL(p string) string { diff --git a/pkg/connector/provider/crisp.go b/pkg/connector/provider/crisp.go index 3b4d9574c..187bc8ff7 100644 --- a/pkg/connector/provider/crisp.go +++ b/pkg/connector/provider/crisp.go @@ -26,11 +26,19 @@ import ( func crispRegistration() *Registration { return &Registration{ - Provider: coredata.ConnectorProviderCrisp, - DisplayName: "Crisp", - SupportsAPIKey: true, - // Crisp authenticates with a plugin token presented as HTTP Basic, the - // credential being the verbatim "identifier:key" pair. + Provider: coredata.ConnectorProviderCrisp, + DisplayName: "Crisp", + // Model B: the plugin token is Probo's own Crisp Marketplace plugin + // credential, held server-side in bootstrap config, not pasted by + // the customer. ManagedAPIKey injects it at connect time; the + // customer supplies only the Website ID. SupportsAPIKey stays false + // so the provider is hidden from the driver catalog until the + // operator configures PROBOD_CONNECTOR_CRISP_PLUGIN_TOKEN — it ships + // deactivated until Crisp validates the production plugin and + // activates with no code change once the token is set. + ManagedAPIKey: true, + // Crisp authenticates with the plugin token presented as HTTP Basic, + // the credential being the verbatim "identifier:key" pair. // APIKeyBasicAuthUserPass base64-encodes it (the empty-password // APIKeyBasicAuth cannot carry the key). A plugin token can serve // several websites, so the reviewed website is captured via diff --git a/pkg/connector/provider/registry.go b/pkg/connector/provider/registry.go index 837131666..d938372f7 100644 --- a/pkg/connector/provider/registry.go +++ b/pkg/connector/provider/registry.go @@ -42,6 +42,18 @@ import ( type Registry struct { mu sync.RWMutex providers map[coredata.ConnectorProvider]*Registration + // managedAPIKeys holds the Probo-supplied API key for providers with + // ManagedAPIKey registrations (e.g. Crisp's marketplace plugin token). + // Populated by probod from bootstrap config via SetManagedAPIKey; empty + // until the operator configures the credential. + managedAPIKeys map[coredata.ConnectorProvider]string + // managedResourceIDs holds an optional Probo-supplied resource identifier + // for a ManagedAPIKey provider, distinct from the credential. Crisp needs + // it: the plugin token's Basic identifier is not the plugin ID, yet the + // per-website plugin API (used for ownership verification) requires the + // plugin ID in the path. Populated by probod via SetManagedResourceID; + // empty for providers that need no such identifier. + managedResourceIDs map[coredata.ConnectorProvider]string } // NewRegistry returns an empty *Registry. Production code uses @@ -49,7 +61,9 @@ type Registry struct { // empty Registry and register only the providers they need. func NewRegistry() *Registry { return &Registry{ - providers: make(map[coredata.ConnectorProvider]*Registration), + providers: make(map[coredata.ConnectorProvider]*Registration), + managedAPIKeys: make(map[coredata.ConnectorProvider]string), + managedResourceIDs: make(map[coredata.ConnectorProvider]string), } } @@ -98,6 +112,14 @@ func (r *Registry) Register(reg *Registration) error { return fmt.Errorf("cannot register connector provider %q: APIKeyBasicAuth, APIKeyBasicAuthUserPass, APIKeyHeader, and APIKeyAuthScheme are mutually exclusive", reg.Provider) } + // ManagedAPIKey injects a Probo-held key and ignores any customer + // credential, so pairing it with SupportsAPIKey/SupportsClientCredentials + // would advertise a credential field whose value is silently discarded — + // the same silent-winner class rejected above. Reject it at startup. + if reg.ManagedAPIKey && (reg.SupportsAPIKey || reg.SupportsClientCredentials) { + return fmt.Errorf("cannot register connector provider %q: ManagedAPIKey is mutually exclusive with SupportsAPIKey and SupportsClientCredentials", 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 @@ -224,6 +246,64 @@ func (r *Registry) APIKeyUsesBasicAuthUserPass(p coredata.ConnectorProvider) boo return false } +// SetManagedAPIKey records the Probo-supplied API key for a +// ManagedAPIKey provider (e.g. Crisp). probod calls this from bootstrap +// config so the create-connector resolver can inject the key and the +// driver catalog can surface the provider. An empty key is treated as +// "not configured": it is not stored, keeping the provider hidden. +func (r *Registry) SetManagedAPIKey(p coredata.ConnectorProvider, key string) { + if key == "" { + return + } + + r.mu.Lock() + defer r.mu.Unlock() + + r.managedAPIKeys[p] = key +} + +// ManagedAPIKey returns the Probo-supplied API key configured for a +// ManagedAPIKey provider and whether one is set. The boolean is false +// (and the string empty) until the operator configures the credential +// via bootstrap, which is what keeps such a provider deactivated. +func (r *Registry) ManagedAPIKey(p coredata.ConnectorProvider) (string, bool) { + r.mu.RLock() + defer r.mu.RUnlock() + + key, ok := r.managedAPIKeys[p] + + return key, ok +} + +// SetManagedResourceID records an optional Probo-supplied resource +// identifier for a ManagedAPIKey provider (e.g. the Crisp plugin ID used +// by the per-website plugin API). probod calls this from bootstrap config +// alongside SetManagedAPIKey. An empty id is treated as "not configured": +// it is not stored. +func (r *Registry) SetManagedResourceID(p coredata.ConnectorProvider, id string) { + if id == "" { + return + } + + r.mu.Lock() + defer r.mu.Unlock() + + r.managedResourceIDs[p] = id +} + +// ManagedResourceID returns the Probo-supplied resource identifier +// configured for a ManagedAPIKey provider and whether one is set. The +// boolean is false (and the string empty) until the operator configures it +// via bootstrap. +func (r *Registry) ManagedResourceID(p coredata.ConnectorProvider) (string, bool) { + r.mu.RLock() + defer r.mu.RUnlock() + + id, ok := r.managedResourceIDs[p] + + return id, ok +} + // ProviderOAuth2Scopes returns the OAuth2 scopes the access review // driver for the given provider needs to list user accounts. Returns // nil for providers that do not need any scopes (Notion, Intercom) diff --git a/pkg/connector/provider/registry_test.go b/pkg/connector/provider/registry_test.go index c55fd00f1..642a18216 100644 --- a/pkg/connector/provider/registry_test.go +++ b/pkg/connector/provider/registry_test.go @@ -19,6 +19,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.probo.inc/probo/pkg/connector" "go.probo.inc/probo/pkg/connector/provider" "go.probo.inc/probo/pkg/coredata" ) @@ -210,3 +211,117 @@ func TestRegistry_ProbeURL(t *testing.T) { assert.NotEmpty(t, r.ProbeURL("SLACK")) assert.Empty(t, r.ProbeURL("UNKNOWN")) } + +// TestRegistry_ManagedAPIKey covers the deactivated default (no key +// configured), a configured key, and that an empty key is a no-op so +// the provider stays deactivated. +func TestRegistry_ManagedAPIKey(t *testing.T) { + t.Parallel() + + r := provider.NewRegistry() + + key, ok := r.ManagedAPIKey(coredata.ConnectorProviderCrisp) + assert.False(t, ok) + assert.Empty(t, key) + + r.SetManagedAPIKey(coredata.ConnectorProviderCrisp, "") + _, ok = r.ManagedAPIKey(coredata.ConnectorProviderCrisp) + assert.False(t, ok, "empty key must not configure the provider") + + r.SetManagedAPIKey(coredata.ConnectorProviderCrisp, "identifier:secret") + key, ok = r.ManagedAPIKey(coredata.ConnectorProviderCrisp) + assert.True(t, ok) + assert.Equal(t, "identifier:secret", key) +} + +func TestRegistry_ManagedResourceID(t *testing.T) { + t.Parallel() + + r := provider.NewRegistry() + + id, ok := r.ManagedResourceID(coredata.ConnectorProviderCrisp) + assert.False(t, ok) + assert.Empty(t, id) + + r.SetManagedResourceID(coredata.ConnectorProviderCrisp, "") + _, ok = r.ManagedResourceID(coredata.ConnectorProviderCrisp) + assert.False(t, ok, "empty resource id must not configure the provider") + + r.SetManagedResourceID(coredata.ConnectorProviderCrisp, "plugin-id") + id, ok = r.ManagedResourceID(coredata.ConnectorProviderCrisp) + assert.True(t, ok) + assert.Equal(t, "plugin-id", id) +} + +// TestCrispIsManagedAPIKey pins Crisp's Model B shape: it is a managed +// API-key provider that does not accept a customer-pasted key, so the +// driver catalog hides it until the operator configures the plugin +// token. +func TestCrispIsManagedAPIKey(t *testing.T) { + t.Parallel() + + r := provider.NewBuiltinRegistry() + reg, ok := r.Get(coredata.ConnectorProviderCrisp) + require.True(t, ok) + assert.True(t, reg.ManagedAPIKey) + assert.False(t, reg.SupportsAPIKey) + assert.True(t, reg.APIKeyBasicAuthUserPass) +} + +// TestRegistry_RejectsManagedPlusCustomerCredential pins that a +// ManagedAPIKey registration cannot also advertise a customer-supplied +// credential path, whose value would be silently discarded. +func TestRegistry_RejectsManagedPlusCustomerCredential(t *testing.T) { + t.Parallel() + + r := provider.NewRegistry() + err := r.Register(&provider.Registration{ + Provider: coredata.ConnectorProviderCrisp, + DisplayName: "Crisp", + ManagedAPIKey: true, + SupportsAPIKey: true, + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "mutually exclusive") +} + +// TestRegistry_ApplyManagedAPIKey verifies the key is injected fresh into a +// managed provider's connection (so rotation propagates and the key is not +// persisted), while non-managed providers are left untouched. +func TestRegistry_ApplyManagedAPIKey(t *testing.T) { + t.Parallel() + + r := provider.NewBuiltinRegistry() + r.SetManagedAPIKey(coredata.ConnectorProviderCrisp, "identifier:secret") + + managed := &coredata.Connector{ + Provider: coredata.ConnectorProviderCrisp, + Connection: &connector.APIKeyConnection{BasicAuthUserPass: true}, + } + require.NoError(t, r.ApplyManagedAPIKey(managed)) + assert.Equal(t, "identifier:secret", managed.Connection.(*connector.APIKeyConnection).APIKey) + + // Non-managed provider: the connection is left untouched. + other := &coredata.Connector{ + Provider: coredata.ConnectorProviderSlack, + Connection: &connector.APIKeyConnection{APIKey: "customer-key"}, + } + require.NoError(t, r.ApplyManagedAPIKey(other)) + assert.Equal(t, "customer-key", other.Connection.(*connector.APIKeyConnection).APIKey) +} + +// TestRegistry_ApplyManagedAPIKey_Unconfigured verifies that a managed +// provider whose key was never configured (deactivated) errors rather than +// silently building a keyless client. +func TestRegistry_ApplyManagedAPIKey_Unconfigured(t *testing.T) { + t.Parallel() + + r := provider.NewBuiltinRegistry() + managed := &coredata.Connector{ + Provider: coredata.ConnectorProviderCrisp, + Connection: &connector.APIKeyConnection{BasicAuthUserPass: true}, + } + err := r.ApplyManagedAPIKey(managed) + require.Error(t, err) + assert.Contains(t, err.Error(), "not configured") +} diff --git a/pkg/connector/provider/types.go b/pkg/connector/provider/types.go index 1820bde6c..9a5b34af3 100644 --- a/pkg/connector/provider/types.go +++ b/pkg/connector/provider/types.go @@ -112,6 +112,17 @@ type Registration struct { // exclusive with the other API-key auth modes. Consumed when the // create-connector resolver builds the APIKeyConnection. APIKeyBasicAuthUserPass bool + // ManagedAPIKey marks a provider whose API key is supplied by Probo + // from bootstrap config (a single, Probo-held credential shared across + // all connections) rather than pasted per-connection by the customer. + // The connection carries only the ExtraSettings (e.g. a Crisp Website + // ID); the create-connector resolver injects the managed key registered + // via (*Registry).SetManagedAPIKey. Such a provider stays hidden from + // the driver catalog until the operator configures the key, so it ships + // deactivated and activates with no code change. Orthogonal to the + // APIKey*/SupportsAPIKey auth-mode flags, which still select how the + // injected key is presented on the wire. + ManagedAPIKey bool // BuildProbeURL derives a per-connector probe URL when the API host or // path depends on connector settings (e.g. a customer subdomain or diff --git a/pkg/probod/probod.go b/pkg/probod/probod.go index 240f231ea..231f60b53 100644 --- a/pkg/probod/probod.go +++ b/pkg/probod/probod.go @@ -318,6 +318,26 @@ func (impl *Implm) Run( defaultConnectorRegistry := connector.NewConnectorRegistry() for _, connectorCfg := range impl.cfg.Connectors { + // ManagedAPIKey (Model B) connectors carry a Probo-held API key + // instead of an OAuth2 client; register it on the provider registry + // and skip the OAuth-only connector registry. Fail loudly on a + // provider that is not a managed-api-key connector (e.g. a typo) + // rather than silently swallowing the key, mirroring how the OAuth2 + // path surfaces misconfiguration at startup. + if connectorCfg.Protocol == connector.ProtocolAPIKey { + p := coredata.ConnectorProvider(connectorCfg.Provider) + + reg, ok := providerRegistry.Get(p) + if !ok || !reg.ManagedAPIKey { + return fmt.Errorf("cannot configure api_key connector %q: not a managed-api-key provider", connectorCfg.Provider) + } + + providerRegistry.SetManagedAPIKey(p, connectorCfg.APIKey) + providerRegistry.SetManagedResourceID(p, connectorCfg.ResourceID) + + continue + } + if oauth2c, ok := connectorCfg.Config.(*connector.OAuth2Connector); ok { if err := providerRegistry.ApplyOAuth2Defaults(connectorCfg.Provider, redirectURI, oauth2c); err != nil { return fmt.Errorf("cannot apply oauth2 defaults: %w", err) diff --git a/pkg/probodconfig/connector_config.go b/pkg/probodconfig/connector_config.go index 19d9d8e66..4f41462a9 100644 --- a/pkg/probodconfig/connector_config.go +++ b/pkg/probodconfig/connector_config.go @@ -30,6 +30,17 @@ type ConnectorConfig struct { RawConfig any `json:"config,omitempty"` Settings any `json:"-"` RawSettings any `json:"settings,omitempty"` + // APIKey holds the Probo-supplied credential for an api_key-protocol + // connector (ManagedAPIKey providers such as Crisp). It is resolved + // from RawConfig by UnmarshalJSON and registered on the provider + // Registry by probod. Empty for OAuth2 connectors. + APIKey string `json:"-"` + // ResourceID holds an optional Probo-supplied resource identifier for an + // api_key-protocol connector, distinct from the credential (e.g. the + // Crisp plugin ID required by the per-website plugin API). Resolved from + // RawConfig by UnmarshalJSON and registered on the provider Registry by + // probod. Empty for connectors that need no such identifier. + ResourceID string `json:"-"` } type ConnectorConfigOAuth2 struct { @@ -42,6 +53,17 @@ type ConnectorConfigOAuth2 struct { IntegrationSlug string `json:"integration-slug,omitempty"` } +// ConnectorConfigAPIKey carries the Probo-held API key for a +// ManagedAPIKey connector (e.g. Crisp's marketplace plugin token). The +// operator supplies it via bootstrap env; probod registers it on the +// provider Registry so the create-connector resolver can inject it. +// ResourceID is an optional companion identifier (e.g. the Crisp plugin +// ID) some managed connectors need beyond the credential. +type ConnectorConfigAPIKey struct { + APIKey string `json:"api-key"` + ResourceID string `json:"resource-id,omitempty"` +} + func (c *Config) GetSlackSigningSecret() string { if c.Notifications.Slack.SigningSecret != "" { return c.Notifications.Slack.SigningSecret @@ -99,6 +121,14 @@ func (c *ConnectorConfig) UnmarshalJSON(data []byte) error { oauth2Connector.IntegrationSlug = config.IntegrationSlug c.Config = &oauth2Connector + case connector.ProtocolAPIKey: + var config ConnectorConfigAPIKey + if err := json.NewDecoder(bytes.NewReader(tmp.RawConfig)).Decode(&config); err != nil { + return fmt.Errorf("cannot unmarshal api key connector config: %w", err) + } + + c.APIKey = config.APIKey + c.ResourceID = config.ResourceID default: return fmt.Errorf("unknown connector protocol: %q", c.Protocol) } diff --git a/pkg/probodconfig/connector_config_test.go b/pkg/probodconfig/connector_config_test.go new file mode 100644 index 000000000..f09c9d4c6 --- /dev/null +++ b/pkg/probodconfig/connector_config_test.go @@ -0,0 +1,81 @@ +// Copyright (c) 2026 Probo Inc . +// +// Permission to use, copy, modify, and/or distribute this software for any +// purpose with or without fee is hereby granted, provided that the above +// copyright notice and this permission notice appear in all copies. +// +// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH +// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY +// AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, +// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM +// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR +// OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR +// PERFORMANCE OF THIS SOFTWARE. + +package probodconfig_test + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.probo.inc/probo/pkg/connector" + "go.probo.inc/probo/pkg/probodconfig" +) + +// TestConnectorConfig_APIKeyRoundTrip pins the bootstrap-to-probod path +// for a ManagedAPIKey (Model B) connector: bootstrap emits an api_key +// ConnectorConfig, it is marshalled to JSON, and probod's UnmarshalJSON +// must recover the key on ConnectorConfig.APIKey. This is what lets the +// Crisp plugin token reach the provider registry. +func TestConnectorConfig_APIKeyRoundTrip(t *testing.T) { + t.Parallel() + + original := probodconfig.ConnectorConfig{ + Provider: "CRISP", + Protocol: connector.ProtocolType("api_key"), + RawConfig: probodconfig.ConnectorConfigAPIKey{ + APIKey: "identifier:secret", + ResourceID: "plugin-id", + }, + } + + data, err := json.Marshal(original) + require.NoError(t, err) + + var got probodconfig.ConnectorConfig + require.NoError(t, json.Unmarshal(data, &got)) + + assert.Equal(t, "CRISP", got.Provider) + assert.Equal(t, connector.ProtocolAPIKey, got.Protocol) + assert.Equal(t, "identifier:secret", got.APIKey) + assert.Equal(t, "plugin-id", got.ResourceID) + assert.Nil(t, got.Config, "api_key connectors carry no OAuth2 Connector") +} + +// TestConnectorConfig_OAuth2RoundTrip guards the pre-existing OAuth2 +// path against regressions from the added api_key branch. +func TestConnectorConfig_OAuth2RoundTrip(t *testing.T) { + t.Parallel() + + original := probodconfig.ConnectorConfig{ + Provider: "SLACK", + Protocol: connector.ProtocolType("oauth2"), + RawConfig: probodconfig.ConnectorConfigOAuth2{ClientID: "cid", ClientSecret: "secret"}, + } + + data, err := json.Marshal(original) + require.NoError(t, err) + + var got probodconfig.ConnectorConfig + require.NoError(t, json.Unmarshal(data, &got)) + + assert.Equal(t, "SLACK", got.Provider) + assert.Equal(t, connector.ProtocolOAuth2, got.Protocol) + + oauth2c, ok := got.Config.(*connector.OAuth2Connector) + require.True(t, ok) + assert.Equal(t, "cid", oauth2c.ClientID) + assert.Equal(t, "secret", oauth2c.ClientSecret) +} diff --git a/pkg/server/api/console/v1/base_resolvers.go b/pkg/server/api/console/v1/base_resolvers.go index 480e82c15..a9235f2de 100644 --- a/pkg/server/api/console/v1/base_resolvers.go +++ b/pkg/server/api/console/v1/base_resolvers.go @@ -574,10 +574,16 @@ func (r *queryResolver) AccessReviewDrivers(ctx context.Context) ([]*types.Conne apiKeySupported := reg.SupportsAPIKey clientCredentialsSupported := reg.SupportsClientCredentials + // ManagedAPIKey (Model B, e.g. Crisp) providers are connectable only + // once the operator configures the Probo-held key; until then they + // stay hidden, so such a provider ships deactivated. + _, hasManaged := r.providerRegistry.ManagedAPIKey(provider) + apiKeyManaged := reg.ManagedAPIKey && hasManaged + // Skip providers that cannot be connected in this deployment: no // OAuth client credentials configured and no key-based fallback - // (API key or client credentials) supported. - if !oauthConfigured && !apiKeySupported && !clientCredentialsSupported { + // (API key, managed API key, or client credentials) supported. + if !oauthConfigured && !apiKeySupported && !clientCredentialsSupported && !apiKeyManaged { continue } @@ -603,6 +609,7 @@ func (r *queryResolver) AccessReviewDrivers(ctx context.Context) ([]*types.Conne DisplayName: reg.DisplayName, OauthConfigured: oauthConfigured, APIKeySupported: apiKeySupported, + APIKeyManaged: apiKeyManaged, ClientCredentialsSupported: clientCredentialsSupported, Oauth2Scopes: scopes, ExtraSettings: extraSettings, @@ -619,6 +626,25 @@ func (r *queryResolver) AccessReviewDrivers(ctx context.Context) ([]*types.Conne return infos, nil } +// CrispVerificationCode is the resolver for the crispVerificationCode field. It +// returns the deterministic ownership-verification code the customer must paste +// into the Probo plugin's per-website settings in their Crisp dashboard before +// connecting that website. Authorized against the organization with the same +// action as the create mutation because the code is organization-bound. This is +// a UI-only helper; MCP/CLI/n8n are intentionally not extended. +func (r *queryResolver) CrispVerificationCode(ctx context.Context, organizationID gid.GID, websiteID string) (string, error) { + if _, err := r.authorize(ctx, organizationID, probo.ActionConnectorCreate); err != nil { + return "", err + } + + websiteID = strings.TrimSpace(websiteID) + if websiteID == "" { + return "", gqlutils.Invalidf(ctx, "websiteId is required") + } + + return computeCrispVerificationCode(r.tokenSecret, organizationID.String(), websiteID), nil +} + // Mutation returns schema.MutationResolver implementation. func (r *Resolver) Mutation() schema.MutationResolver { return &mutationResolver{r} } diff --git a/pkg/server/api/console/v1/connector_resolvers.go b/pkg/server/api/console/v1/connector_resolvers.go index 5e94129d8..b934dfe9f 100644 --- a/pkg/server/api/console/v1/connector_resolvers.go +++ b/pkg/server/api/console/v1/connector_resolvers.go @@ -36,17 +36,16 @@ func (r *mutationResolver) CreateAPIKeyConnector(ctx context.Context, input type return nil, err } + apiKey, err := r.resolveAPIKeyConnectorCredential(input.Provider, input.APIKey) + if err != nil { + return nil, gqlutils.Invalid(ctx, err) + } + req := probo.CreateConnectorRequest{ OrganizationID: input.OrganizationID, Provider: input.Provider, Protocol: coredata.ConnectorProtocolAPIKey, - Connection: &connector.APIKeyConnection{ - APIKey: input.APIKey, - Header: r.providerRegistry.APIKeyHeader(input.Provider), - BasicAuth: r.providerRegistry.APIKeyUsesBasicAuth(input.Provider), - BasicAuthUserPass: r.providerRegistry.APIKeyUsesBasicAuthUserPass(input.Provider), - Scheme: r.providerRegistry.APIKeyAuthScheme(input.Provider), - }, + Connection: r.newAPIKeyConnection(input.Provider, apiKey), } raw, err := apiKeyConnectorSettings(input) @@ -56,6 +55,16 @@ func (r *mutationResolver) CreateAPIKeyConnector(ctx context.Context, input type req.RawSettings = raw + // Crisp (ManagedAPIKey) requires proof the organization controls the Crisp + // website before the connection is created; every other API-key provider is + // unaffected. Runs after settings validation and before any write, so a + // failed check leaves no row. + if input.Provider == coredata.ConnectorProviderCrisp { + if err := r.verifyCrispOwnership(ctx, input); err != nil { + return nil, err + } + } + cnnctr, err := r.probo.Connectors.Create(ctx, scope, req) if err != nil { if errors.Is(err, coredata.ErrResourceAlreadyExists) { diff --git a/pkg/server/api/console/v1/connector_settings.go b/pkg/server/api/console/v1/connector_settings.go index f952c5374..680f5b5fd 100644 --- a/pkg/server/api/console/v1/connector_settings.go +++ b/pkg/server/api/console/v1/connector_settings.go @@ -15,19 +15,144 @@ package console_v1 import ( + "context" "encoding/json" + "errors" "fmt" + "net/http" "net/url" + "strings" + "go.gearno.de/kit/log" "go.probo.inc/probo/pkg/accessreview/drivers" "go.probo.inc/probo/pkg/connector" "go.probo.inc/probo/pkg/coredata" "go.probo.inc/probo/pkg/server/api/console/v1/types" + "go.probo.inc/probo/pkg/server/gqlutils" ) // These helpers live outside connector_resolvers.go because that file is // regenerated by gqlgen, which does not preserve standalone functions. +// resolveAPIKeyConnectorCredential returns the API key to persist on a new +// API-key connection. For ManagedAPIKey providers (Model B, e.g. Crisp) it +// persists NOTHING (empty string): the Probo-held key is injected fresh at +// use time by (*provider.Registry).ApplyManagedAPIKey, so it survives key +// rotation and is not duplicated across tenant rows. It still requires the +// key to be configured, which is what keeps the provider deactivated, and +// ignores any client-supplied value. For all other providers it requires +// the client-supplied key. The returned error is surfaced verbatim to the +// client via gqlutils.Invalid, so it contains only provider/field names, +// never the key itself. +func (r *Resolver) resolveAPIKeyConnectorCredential(provider coredata.ConnectorProvider, clientKey *string) (string, error) { + if reg, ok := r.providerRegistry.Get(provider); ok && reg.ManagedAPIKey { + if _, ok := r.providerRegistry.ManagedAPIKey(provider); !ok { + return "", fmt.Errorf("connector is not configured for this deployment") + } + + return "", nil + } + + if clientKey == nil || *clientKey == "" { + return "", fmt.Errorf("apiKey is required") + } + + return *clientKey, nil +} + +// newAPIKeyConnection builds an API-key connection for provider, filling the auth +// presentation (header, basic-auth mode, scheme) from the provider registry and +// using key as the credential. Both CreateAPIKeyConnector (the persisted +// connection) and verifyCrispOwnership (the ownership-check client) construct +// their connection through it, so the verification client authenticates exactly +// as the persisted connector will: a new auth flag cannot be added to one path +// and silently missed on the other. +func (r *Resolver) newAPIKeyConnection(provider coredata.ConnectorProvider, key string) *connector.APIKeyConnection { + return &connector.APIKeyConnection{ + APIKey: key, + Header: r.providerRegistry.APIKeyHeader(provider), + BasicAuth: r.providerRegistry.APIKeyUsesBasicAuth(provider), + BasicAuthUserPass: r.providerRegistry.APIKeyUsesBasicAuthUserPass(provider), + Scheme: r.providerRegistry.APIKeyAuthScheme(provider), + } +} + +// crispSettingsFetcher reads a Crisp plugin's per-website subscription settings. +// It matches drivers.GetCrispSubscriptionSettings: verifyCrispOwnership injects +// the real fetch, and tests substitute a fake so the branch wiring (the security +// polarity and the Invalid-versus-Internal error mapping) is exercised without a +// live Crisp API. +type crispSettingsFetcher func(ctx context.Context, httpClient *http.Client, websiteID, pluginID string) (*drivers.CrispSubscriptionSettings, error) + +// verifyCrispOwnership proves the connecting organization controls the Crisp +// website before a connection is created (the #1b ownership check). It reads the +// Probo plugin's per-website settings through the managed plugin token and +// requires probo_verification_code to equal the code Probo showed for this exact +// (organization, website) pair. Because the code is bound to both, one +// organization cannot bind another organization's website, and only someone with +// dashboard access to the website could have written the setting. It runs only +// for Crisp/managed providers; nothing is persisted before it returns, so a +// failed check creates no row. Returned Invalid errors are surfaced to the +// client and contain only guidance, never the code or the token. +func (r *Resolver) verifyCrispOwnership(ctx context.Context, input types.CreateAPIKeyConnectorInput) error { + return r.verifyCrispOwnershipWith(ctx, input, drivers.GetCrispSubscriptionSettings) +} + +// verifyCrispOwnershipWith is verifyCrispOwnership with the settings fetch +// injected, so its branch wiring can be unit-tested without reaching the live +// Crisp API. verifyCrispOwnership passes the real +// drivers.GetCrispSubscriptionSettings. +func (r *Resolver) verifyCrispOwnershipWith(ctx context.Context, input types.CreateAPIKeyConnectorInput, fetch crispSettingsFetcher) error { + if input.CrispWebsiteID == nil || strings.TrimSpace(*input.CrispWebsiteID) == "" { + return gqlutils.Invalidf(ctx, "crispWebsiteId is required") + } + + websiteID := strings.TrimSpace(*input.CrispWebsiteID) + + // The managed plugin token gates the connector's visibility, so it is set + // here; treat its absence as an internal error rather than client input. + managedKey, ok := r.providerRegistry.ManagedAPIKey(input.Provider) + if !ok { + return gqlutils.Internal(ctx) + } + + // The plugin ID is a separate managed value the per-website plugin API + // needs; the bootstrap requires it alongside the token, so its absence is a + // deployment misconfiguration. + pluginID, ok := r.providerRegistry.ManagedResourceID(input.Provider) + if !ok { + r.logger.ErrorCtx(ctx, "crisp plugin id not configured") + + return gqlutils.Internal(ctx) + } + + conn := r.newAPIKeyConnection(input.Provider, managedKey) + + httpClient, err := conn.Client(ctx) + if err != nil { + r.logger.ErrorCtx(ctx, "cannot build crisp verification client", log.Error(err)) + + return gqlutils.Internal(ctx) + } + + settings, err := fetch(ctx, httpClient, websiteID, pluginID) + + switch { + case errors.Is(err, drivers.ErrCrispPluginNotSubscribed): + return gqlutils.Invalidf(ctx, "install and configure the Probo plugin on this Crisp website, then retry") + case err != nil: + r.logger.ErrorCtx(ctx, "cannot read crisp subscription settings", log.Error(err)) + + return gqlutils.Internal(ctx) + } + + if !verifyCrispVerificationCode(r.tokenSecret, input.OrganizationID.String(), websiteID, settings.ProboVerificationCode) { + return gqlutils.Invalidf(ctx, "verification code mismatch: paste the code shown in Probo into the plugin settings, then retry") + } + + return nil +} + // apiKeyConnectorSettings marshals the provider-specific extra settings // for an API-key connector from the typed gqlgen input into the JSON // blob persisted on coredata.Connector.RawSettings. It returns (nil, @@ -196,11 +321,20 @@ func apiKeyConnectorSettings(input types.CreateAPIKeyConnectorInput) (json.RawMe return json.Marshal(&coredata.ScalewayConnectorSettings{OrganizationID: *input.ScalewayOrganizationID}) case coredata.ConnectorProviderCrisp: - if input.CrispWebsiteID == nil || *input.CrispWebsiteID == "" { + websiteID := "" + if input.CrispWebsiteID != nil { + websiteID = strings.TrimSpace(*input.CrispWebsiteID) + } + + if websiteID == "" { return nil, fmt.Errorf("cannot create crisp connector: crispWebsiteId is required") } - return json.Marshal(&coredata.CrispConnectorSettings{WebsiteID: *input.CrispWebsiteID}) + // Persist the same trimmed value that verifyCrispOwnership proved and the + // crispVerificationCode query minted the code against, so the stored, + // verified, and displayed website are identical (a padded value would + // verify then break the driver's URL). + return json.Marshal(&coredata.CrispConnectorSettings{WebsiteID: websiteID}) } return nil, nil diff --git a/pkg/server/api/console/v1/crisp.go b/pkg/server/api/console/v1/crisp.go new file mode 100644 index 000000000..0aa2cf569 --- /dev/null +++ b/pkg/server/api/console/v1/crisp.go @@ -0,0 +1,55 @@ +// Copyright (c) 2026 Probo Inc . +// +// Permission to use, copy, modify, and/or distribute this software for any +// purpose with or without fee is hereby granted, provided that the above +// copyright notice and this permission notice appear in all copies. +// +// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH +// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY +// AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, +// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM +// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR +// OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR +// PERFORMANCE OF THIS SOFTWARE. + +package console_v1 + +import ( + "crypto/hmac" + "crypto/sha256" + "crypto/subtle" + "encoding/base32" + "strings" +) + +// crispVerificationCodeLength bounds the human-typeable verification code. 12 +// base32 characters carry ~60 bits, far beyond guessing given the code is only +// a proof-of-control challenge (not a secret) and is compared server side. +const crispVerificationCodeLength = 12 + +// computeCrispVerificationCode derives the deterministic ownership-verification +// code Probo shows for a (organization, Crisp website) pair. It is +// HMAC(tokenSecret, domain || orgID || websiteID) so only Probo can mint it and +// the value is bound to BOTH the organization and the website: a code minted for +// one organization cannot verify a website under another, and a code for one +// website cannot verify another. Nothing is stored: the same inputs always yield +// the same code, so the create-time check re-derives and compares it. +func computeCrispVerificationCode(secret, orgID, websiteID string) string { + mac := hmac.New(sha256.New, []byte(secret)) + mac.Write([]byte("probo/connector/crisp-verification:" + orgID + ":" + websiteID)) + + code := base32.StdEncoding.WithPadding(base32.NoPadding).EncodeToString(mac.Sum(nil)) + + return code[:crispVerificationCodeLength] +} + +// verifyCrispVerificationCode reports whether provided matches the code Probo +// would show for (orgID, websiteID). Operators paste the code back through the +// Crisp dashboard, so the comparison tolerates surrounding whitespace and a +// lowercased value; it is otherwise a constant-time compare. +func verifyCrispVerificationCode(secret, orgID, websiteID, provided string) bool { + expected := computeCrispVerificationCode(secret, orgID, websiteID) + got := strings.ToUpper(strings.TrimSpace(provided)) + + return subtle.ConstantTimeCompare([]byte(expected), []byte(got)) == 1 +} diff --git a/pkg/server/api/console/v1/crisp_test.go b/pkg/server/api/console/v1/crisp_test.go new file mode 100644 index 000000000..61537cf0b --- /dev/null +++ b/pkg/server/api/console/v1/crisp_test.go @@ -0,0 +1,347 @@ +// Copyright (c) 2026 Probo Inc . +// +// Permission to use, copy, modify, and/or distribute this software for any +// purpose with or without fee is hereby granted, provided that the above +// copyright notice and this permission notice appear in all copies. +// +// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH +// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY +// AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, +// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM +// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR +// OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR +// PERFORMANCE OF THIS SOFTWARE. + +package console_v1 + +import ( + "context" + "encoding/json" + "errors" + "io" + "net/http" + "regexp" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/vektah/gqlparser/v2/gqlerror" + "go.gearno.de/kit/log" + "go.probo.inc/probo/pkg/accessreview/drivers" + "go.probo.inc/probo/pkg/connector/provider" + "go.probo.inc/probo/pkg/coredata" + "go.probo.inc/probo/pkg/gid" + "go.probo.inc/probo/pkg/server/api/console/v1/types" +) + +func TestComputeCrispVerificationCode(t *testing.T) { + t.Parallel() + + const ( + secret = "test-token-secret" + org = "gid://organization/1" + website = "e8592878-c0d0-4632-b2f7-7d882f288d43" + ) + + t.Run("is deterministic for the same inputs", func(t *testing.T) { + t.Parallel() + + a := computeCrispVerificationCode(secret, org, website) + b := computeCrispVerificationCode(secret, org, website) + assert.Equal(t, a, b) + }) + + t.Run("is bound to the organization", func(t *testing.T) { + t.Parallel() + + a := computeCrispVerificationCode(secret, "gid://organization/1", website) + b := computeCrispVerificationCode(secret, "gid://organization/2", website) + assert.NotEqual(t, a, b) + }) + + t.Run("is bound to the website", func(t *testing.T) { + t.Parallel() + + a := computeCrispVerificationCode(secret, org, "website-a") + b := computeCrispVerificationCode(secret, org, "website-b") + assert.NotEqual(t, a, b) + }) + + t.Run("depends on the secret", func(t *testing.T) { + t.Parallel() + + a := computeCrispVerificationCode("secret-a", org, website) + b := computeCrispVerificationCode("secret-b", org, website) + assert.NotEqual(t, a, b) + }) + + t.Run("is 12 human-typeable base32 characters", func(t *testing.T) { + t.Parallel() + + code := computeCrispVerificationCode(secret, org, website) + assert.Len(t, code, crispVerificationCodeLength) + assert.Regexp(t, regexp.MustCompile(`^[A-Z2-7]{12}$`), code) + }) + + // Delimiting org and website prevents a boundary collision where a longer + // org and shorter website (or vice versa) concatenate to the same bytes. + t.Run("delimiter prevents org/website boundary collisions", func(t *testing.T) { + t.Parallel() + + a := computeCrispVerificationCode(secret, "ab", "c") + b := computeCrispVerificationCode(secret, "a", "bc") + assert.NotEqual(t, a, b) + }) +} + +func TestVerifyCrispVerificationCode(t *testing.T) { + t.Parallel() + + const ( + secret = "test-token-secret" + org = "gid://organization/1" + website = "e8592878-c0d0-4632-b2f7-7d882f288d43" + ) + + valid := computeCrispVerificationCode(secret, org, website) + require.NotEmpty(t, valid) + + t.Run("accepts the exact code", func(t *testing.T) { + t.Parallel() + assert.True(t, verifyCrispVerificationCode(secret, org, website, valid)) + }) + + t.Run("tolerates surrounding whitespace and lowercase", func(t *testing.T) { + t.Parallel() + assert.True(t, verifyCrispVerificationCode(secret, org, website, " "+valid+" ")) + assert.True(t, verifyCrispVerificationCode(secret, org, website, strings.ToLower(valid))) + }) + + t.Run("rejects a mismatched or empty code", func(t *testing.T) { + t.Parallel() + assert.False(t, verifyCrispVerificationCode(secret, org, website, "")) + assert.False(t, verifyCrispVerificationCode(secret, org, website, "AAAAAAAAAAAA")) + }) + + t.Run("rejects a code minted for another organization", func(t *testing.T) { + t.Parallel() + + other := computeCrispVerificationCode(secret, "gid://organization/2", website) + assert.False(t, verifyCrispVerificationCode(secret, org, website, other)) + }) + + t.Run("rejects a code minted for another website", func(t *testing.T) { + t.Parallel() + + other := computeCrispVerificationCode(secret, org, "another-website") + assert.False(t, verifyCrispVerificationCode(secret, org, website, other)) + }) +} + +// The stored Crisp Website ID must be the SAME trimmed value that +// verifyCrispOwnership and the CrispVerificationCode query derive the code +// against, or a padded ID would verify then break the driver's request URL. +func TestApiKeyConnectorSettings_CrispTrimsWebsiteID(t *testing.T) { + t.Parallel() + + website := " e8592878-c0d0-4632-b2f7-7d882f288d43 " + + raw, err := apiKeyConnectorSettings(types.CreateAPIKeyConnectorInput{ + Provider: coredata.ConnectorProviderCrisp, + CrispWebsiteID: &website, + }) + require.NoError(t, err) + + var settings coredata.CrispConnectorSettings + require.NoError(t, json.Unmarshal(raw, &settings)) + assert.Equal(t, "e8592878-c0d0-4632-b2f7-7d882f288d43", settings.WebsiteID) +} + +func TestApiKeyConnectorSettings_CrispRejectsWhitespaceOnly(t *testing.T) { + t.Parallel() + + website := " " + + _, err := apiKeyConnectorSettings(types.CreateAPIKeyConnectorInput{ + Provider: coredata.ConnectorProviderCrisp, + CrispWebsiteID: &website, + }) + require.Error(t, err) +} + +// resolveAPIKeyConnectorCredential is the sole gate keeping normal API-key +// providers requiring a customer key while ManagedAPIKey providers (Model B, +// e.g. Crisp) persist none. A regression either drops the required-key check for +// every provider or persists the managed key on the row. +func TestResolveAPIKeyConnectorCredential(t *testing.T) { + t.Parallel() + + // NewBuiltinRegistry registers Crisp as a ManagedAPIKey provider; the + // managed key must then be set for it to count as configured. + configuredReg := provider.NewBuiltinRegistry() + configuredReg.SetManagedAPIKey(coredata.ConnectorProviderCrisp, "identifier:secret") + configured := &Resolver{providerRegistry: configuredReg} + + unconfigured := &Resolver{providerRegistry: provider.NewBuiltinRegistry()} + + clientKey := "customer-key" + empty := "" + + t.Run("managed and configured persists no key", func(t *testing.T) { + t.Parallel() + + key, err := configured.resolveAPIKeyConnectorCredential(coredata.ConnectorProviderCrisp, nil) + require.NoError(t, err) + assert.Equal(t, "", key) + }) + + t.Run("managed and configured ignores a client-supplied key", func(t *testing.T) { + t.Parallel() + + key, err := configured.resolveAPIKeyConnectorCredential(coredata.ConnectorProviderCrisp, &clientKey) + require.NoError(t, err) + assert.Equal(t, "", key) + }) + + t.Run("managed but unconfigured is rejected", func(t *testing.T) { + t.Parallel() + + _, err := unconfigured.resolveAPIKeyConnectorCredential(coredata.ConnectorProviderCrisp, nil) + require.Error(t, err) + }) + + t.Run("non-managed requires a key", func(t *testing.T) { + t.Parallel() + + _, errNil := configured.resolveAPIKeyConnectorCredential(coredata.ConnectorProviderTally, nil) + require.EqualError(t, errNil, "apiKey is required") + + _, errEmpty := configured.resolveAPIKeyConnectorCredential(coredata.ConnectorProviderTally, &empty) + require.EqualError(t, errEmpty, "apiKey is required") + }) + + t.Run("non-managed returns the client key", func(t *testing.T) { + t.Parallel() + + key, err := configured.resolveAPIKeyConnectorCredential(coredata.ConnectorProviderTally, &clientKey) + require.NoError(t, err) + assert.Equal(t, "customer-key", key) + }) +} + +// verifyCrispOwnershipWith is the #1b create-time ownership gate. This pins its +// branch wiring: the security polarity (only a matching code passes) and the +// Invalid-versus-Internal error mapping. The settings fetch is faked so no live +// Crisp API is reached. +func TestVerifyCrispOwnershipWith(t *testing.T) { + t.Parallel() + + const ( + secret = "test-token-secret" + pluginID = "plugin-id" + ) + + orgID := gid.New(gid.NewTenantID(), coredata.OrganizationEntityType) + websiteID := "e8592878-c0d0-4632-b2f7-7d882f288d43" + validCode := computeCrispVerificationCode(secret, orgID.String(), websiteID) + + // newResolver builds a Resolver whose registry has Crisp configured as a + // managed provider; the plugin ID is set only when withPluginID is true. + newResolver := func(withPluginID bool) *Resolver { + reg := provider.NewBuiltinRegistry() + reg.SetManagedAPIKey(coredata.ConnectorProviderCrisp, "identifier:secret") + + if withPluginID { + reg.SetManagedResourceID(coredata.ConnectorProviderCrisp, pluginID) + } + + return &Resolver{ + providerRegistry: reg, + tokenSecret: secret, + logger: log.NewLogger(log.WithOutput(io.Discard)), + } + } + + newInput := func(website *string) types.CreateAPIKeyConnectorInput { + return types.CreateAPIKeyConnectorInput{ + OrganizationID: orgID, + Provider: coredata.ConnectorProviderCrisp, + CrispWebsiteID: website, + } + } + + fetchCode := func(code string) crispSettingsFetcher { + return func(context.Context, *http.Client, string, string) (*drivers.CrispSubscriptionSettings, error) { + return &drivers.CrispSubscriptionSettings{ProboVerificationCode: code}, nil + } + } + + assertCode := func(t *testing.T, err error, code string) { + t.Helper() + + require.Error(t, err) + + gqlErr, ok := err.(*gqlerror.Error) + require.True(t, ok, "expected *gqlerror.Error, got %T", err) + assert.Equal(t, code, gqlErr.Extensions["code"]) + } + + t.Run("missing website id is invalid", func(t *testing.T) { + t.Parallel() + + err := newResolver(true).verifyCrispOwnershipWith(context.Background(), newInput(nil), fetchCode(validCode)) + assertCode(t, err, "INVALID") + }) + + t.Run("whitespace website id is invalid", func(t *testing.T) { + t.Parallel() + + blank := " " + err := newResolver(true).verifyCrispOwnershipWith(context.Background(), newInput(&blank), fetchCode(validCode)) + assertCode(t, err, "INVALID") + }) + + t.Run("missing plugin id is internal", func(t *testing.T) { + t.Parallel() + + err := newResolver(false).verifyCrispOwnershipWith(context.Background(), newInput(&websiteID), fetchCode(validCode)) + assertCode(t, err, "INTERNAL") + }) + + t.Run("plugin not subscribed is invalid", func(t *testing.T) { + t.Parallel() + + fetch := func(context.Context, *http.Client, string, string) (*drivers.CrispSubscriptionSettings, error) { + return nil, drivers.ErrCrispPluginNotSubscribed + } + err := newResolver(true).verifyCrispOwnershipWith(context.Background(), newInput(&websiteID), fetch) + assertCode(t, err, "INVALID") + }) + + t.Run("other fetch error is internal and leaks nothing", func(t *testing.T) { + t.Parallel() + + fetch := func(context.Context, *http.Client, string, string) (*drivers.CrispSubscriptionSettings, error) { + return nil, errors.New("boom: plugin token 12345 rejected") + } + err := newResolver(true).verifyCrispOwnershipWith(context.Background(), newInput(&websiteID), fetch) + assertCode(t, err, "INTERNAL") + assert.NotContains(t, err.Error(), "boom") + assert.NotContains(t, err.Error(), "12345") + }) + + t.Run("mismatched code is invalid", func(t *testing.T) { + t.Parallel() + + err := newResolver(true).verifyCrispOwnershipWith(context.Background(), newInput(&websiteID), fetchCode("WRONGCODE000")) + assertCode(t, err, "INVALID") + }) + + t.Run("matching code passes", func(t *testing.T) { + t.Parallel() + + err := newResolver(true).verifyCrispOwnershipWith(context.Background(), newInput(&websiteID), fetchCode(validCode)) + require.NoError(t, err) + }) +} diff --git a/pkg/server/api/console/v1/graphql/base.graphql b/pkg/server/api/console/v1/graphql/base.graphql index 572136280..d561d6cd1 100644 --- a/pkg/server/api/console/v1/graphql/base.graphql +++ b/pkg/server/api/console/v1/graphql/base.graphql @@ -27,6 +27,8 @@ type Query { viewer: Viewer! commonThirdParties(name: String!): [CommonThirdParty!]! accessReviewDrivers: [ConnectorProviderInfo!]! @goField(forceResolver: true) + crispVerificationCode(organizationId: ID!, websiteId: String!): String! + @goField(forceResolver: true) } type Mutation diff --git a/pkg/server/api/console/v1/graphql/connector.graphql b/pkg/server/api/console/v1/graphql/connector.graphql index 7120bf1a7..cdf4c16b3 100644 --- a/pkg/server/api/console/v1/graphql/connector.graphql +++ b/pkg/server/api/console/v1/graphql/connector.graphql @@ -106,6 +106,14 @@ type ConnectorProviderInfo { displayName: String! oauthConfigured: Boolean! apiKeySupported: Boolean! + """ + apiKeyManaged is true when the provider connects with a Probo-supplied + API key (Model B, e.g. Crisp's marketplace plugin token) that the + operator has configured. The customer supplies only the extra settings + (e.g. a Website ID), not the key. False until the key is configured, + which keeps such a provider out of the catalog. + """ + apiKeyManaged: Boolean! clientCredentialsSupported: Boolean! oauth2Scopes: [String!]! extraSettings: [ConnectorProviderSettingInfo!]! @@ -164,7 +172,13 @@ extend type Mutation { input CreateAPIKeyConnectorInput { organizationId: ID! provider: ConnectorProvider! - apiKey: String! + """ + apiKey is the customer-supplied credential. It is null for + ManagedAPIKey (Model B) providers such as Crisp, where the server + injects Probo's own key and the customer provides only the extra + settings. + """ + apiKey: String tallyOrganizationId: String sentryOrganizationSlug: String supabaseOrganizationSlug: String diff --git a/pkg/server/api/console/v1/graphql_handler.go b/pkg/server/api/console/v1/graphql_handler.go index 2f12d5919..1f6c57fec 100644 --- a/pkg/server/api/console/v1/graphql_handler.go +++ b/pkg/server/api/console/v1/graphql_handler.go @@ -50,6 +50,7 @@ func NewGraphQLHandler( connectorRegistry *connector.ConnectorRegistry, providerRegistry *provider.Registry, customDomainCname string, + tokenSecret string, logger *log.Logger, thirdPartySvc *thirdparty.Service, riskManagementSvc *riskmanagement.Service, @@ -74,6 +75,7 @@ func NewGraphQLHandler( riskManagement: riskManagementSvc, thirdParty: thirdPartySvc, customDomainCname: customDomainCname, + tokenSecret: tokenSecret, fileManager: fileManagerSvc, baseURL: baseURL, logger: logger, diff --git a/pkg/server/api/console/v1/resolver.go b/pkg/server/api/console/v1/resolver.go index 0604cb1a9..67f177bce 100644 --- a/pkg/server/api/console/v1/resolver.go +++ b/pkg/server/api/console/v1/resolver.go @@ -71,6 +71,7 @@ type ( fileManager *filemanager.Service baseURL *baseurl.BaseURL customDomainCname string + tokenSecret string } ) @@ -111,6 +112,7 @@ func NewMux( connectorRegistry, providerRegistry, customDomainCname, + tokenSecret, logger, thirdPartySvc, riskManagementSvc,