mirror of
https://github.com/netbirdio/netbird.git
synced 2026-08-26 09:31:30 +02:00
[management] Let a discovery request name a record and a new upstream
Editing a provider's upstream URL meant retyping its API key. The key is the one field the API never returns, so there was nothing to retype from, and the dashboard had to demand it because a provider_id request resolved the upstream from the stored row — listing the old endpoint while the form showed the new one. The request now reads as a partial edit of the record it names, the same way PUT on a provider already does: an upstream in the body overrides the stored one, an omitted upstream keeps it. The credential and the catalog entry still come from the record only, so a caller cannot aim a stored key at a vendor of their choosing.
This commit is contained in:
@@ -456,3 +456,50 @@ func TestDiscoverProviderModels_SurfacesTheVendorRefusal(t *testing.T) {
|
||||
|
||||
require.EqualError(t, err, "the provider rejected the credential")
|
||||
}
|
||||
|
||||
// TestDiscoverProviderModels_ListsAgainstTheUrlOnTheForm covers the edit the
|
||||
// operator cannot otherwise make: the upstream has been retyped and the
|
||||
// credential has not, because the API never returned it to be retyped. Naming
|
||||
// the record supplies the key; the request supplies the URL under test.
|
||||
func TestDiscoverProviderModels_ListsAgainstTheUrlOnTheForm(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
f := newBootstrapFixture(t)
|
||||
// Twice: the create, and the listing, which is gated on Create too.
|
||||
f.expectPermission("account1", "user1", modules.AgentNetworkProviders, operations.Create, true)
|
||||
f.expectPermission("account1", "user1", modules.AgentNetworkProviders, operations.Create, true)
|
||||
|
||||
created, err := f.manager.CreateProvider(ctx, "user1", newCheckedProvider("account1"))
|
||||
require.NoError(t, err)
|
||||
f.vendor.requests = nil
|
||||
|
||||
_, err = f.manager.DiscoverProviderModels(ctx, "account1", "user1", modeldiscovery.Request{
|
||||
CatalogID: "openai_api",
|
||||
UpstreamURL: "https://gateway.example.com",
|
||||
}, created.ID)
|
||||
require.NoError(t, err)
|
||||
|
||||
asked := f.vendor.only(t)
|
||||
require.Equal(t, "https://gateway.example.com", asked.UpstreamURL, "the typed url must be the one listed against")
|
||||
require.Equal(t, "sk-good", asked.APIKey, "and the stored key must be what lists it")
|
||||
}
|
||||
|
||||
// TestDiscoverProviderModels_FallsBackToTheStoredUrl keeps the plain refresh
|
||||
// working: a request naming only the record still reaches the saved upstream.
|
||||
func TestDiscoverProviderModels_FallsBackToTheStoredUrl(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
f := newBootstrapFixture(t)
|
||||
f.expectPermission("account1", "user1", modules.AgentNetworkProviders, operations.Create, true)
|
||||
f.expectPermission("account1", "user1", modules.AgentNetworkProviders, operations.Create, true)
|
||||
|
||||
created, err := f.manager.CreateProvider(ctx, "user1", newCheckedProvider("account1"))
|
||||
require.NoError(t, err)
|
||||
stored := f.vendor.only(t).UpstreamURL
|
||||
f.vendor.requests = nil
|
||||
|
||||
_, err = f.manager.DiscoverProviderModels(ctx, "account1", "user1", modeldiscovery.Request{
|
||||
CatalogID: "openai_api",
|
||||
}, created.ID)
|
||||
require.NoError(t, err)
|
||||
|
||||
require.Equal(t, stored, f.vendor.only(t).UpstreamURL)
|
||||
}
|
||||
|
||||
@@ -204,9 +204,11 @@ func (m *managerImpl) GetProvider(ctx context.Context, accountID, userID, provid
|
||||
|
||||
// DiscoverProviderModels asks the vendor which models a credential can reach.
|
||||
//
|
||||
// recordID, when set, names an existing provider whose stored credential and
|
||||
// upstream are used instead of the ones in req — so the dashboard can refresh
|
||||
// the list without ever holding the key.
|
||||
// recordID, when set, names an existing provider whose stored credential is
|
||||
// used instead of the one in req — so the dashboard can refresh the list
|
||||
// without ever holding the key. An upstream in req overrides the stored one,
|
||||
// which is what lets a form list against a URL the operator has typed but not
|
||||
// saved yet, using the credential they cannot retype.
|
||||
//
|
||||
// Gated on Create rather than Read: this spends the operator's credential
|
||||
// against a third party, which is not something a read-only role should be
|
||||
@@ -227,8 +229,15 @@ func (m *managerImpl) DiscoverProviderModels(ctx context.Context, accountID, use
|
||||
// name a different one would run a provider's credential against
|
||||
// whichever vendor endpoint they picked.
|
||||
req.CatalogID = record.ProviderID
|
||||
req.UpstreamURL = record.UpstreamURL
|
||||
req.APIKey = record.APIKey
|
||||
// The upstream is the one field the caller may override, and only for
|
||||
// entries that serve their listing from it. Those are the operator's
|
||||
// own endpoints, reached with their own credential, so an unsaved URL
|
||||
// is no more reachable here than a saved one — and the SSRF guard
|
||||
// treats both alike.
|
||||
if strings.TrimSpace(req.UpstreamURL) == "" {
|
||||
req.UpstreamURL = record.UpstreamURL
|
||||
}
|
||||
}
|
||||
|
||||
models, err := m.modelDiscovery.Fetch(ctx, req)
|
||||
|
||||
@@ -5345,7 +5345,7 @@ components:
|
||||
upstream_url:
|
||||
type: string
|
||||
description: |
|
||||
The upstream being configured. Used to reach vendors that serve their listing from the same host as inference, and to read back the region for those whose host embeds one. Ignored when provider_id is supplied.
|
||||
The upstream being configured. Used to reach vendors that serve their listing from the same host as inference, and to read back the region for those whose host embeds one. Sent alongside provider_id, it overrides the stored upstream, so an edit can be listed against the URL on the form before it is saved.
|
||||
example: "https://bedrock-runtime.eu-central-1.amazonaws.com"
|
||||
api_key:
|
||||
type: string
|
||||
@@ -5353,7 +5353,7 @@ components:
|
||||
example: "sk-..."
|
||||
provider_id:
|
||||
type: string
|
||||
description: Existing Agent Network provider record whose stored credential and upstream should be used. Lets the form refresh the list without the client holding the key.
|
||||
description: Existing Agent Network provider record to query with. Its stored credential is used, and its upstream unless upstream_url overrides it, so the form can refresh the list without the client holding the key.
|
||||
example: "ch8i4ug6lnn4g9hqv7m0"
|
||||
required:
|
||||
- catalog_provider_id
|
||||
|
||||
Reference in New Issue
Block a user