From 6c85e39820cf13937aeb4d136fbbecb2dbaaef6b Mon Sep 17 00:00:00 2001 From: Derrick Hammer Date: Wed, 2 Sep 2026 21:26:08 +0000 Subject: [PATCH] fix(admin): render social provider results, accept keys - add *admin.SocialProvider and SocialProvidersDeleteResult cases to renderAdminResult so create/get/update/enable/disable no longer fail with an unroutable result type - resolve a non-numeric positional to a provider key (e.g. google) via the configured provider list, mirroring platform-domain name lookup - add render and key-resolution tests --- internal/catalogops/admin_social_providers.go | 23 +++- ...alog_admin_social_providers_render_test.go | 108 ++++++++++++++++++ internal/cli/catalog_admin_wiring.go | 76 ++++++++++++ 3 files changed, 201 insertions(+), 6 deletions(-) create mode 100644 internal/cli/catalog_admin_social_providers_render_test.go diff --git a/internal/catalogops/admin_social_providers.go b/internal/catalogops/admin_social_providers.go index b4cb5263..47fc5c35 100644 --- a/internal/catalogops/admin_social_providers.go +++ b/internal/catalogops/admin_social_providers.go @@ -8,6 +8,17 @@ import ( "go.lumeweb.com/portal-sdk/admin" ) +// Operation names (constants kept alongside the platform-domains ops for the +// wiring layer, which resolves provider keys to numeric IDs by op name). +const ( + OpAdminSocialProvidersCreate = "admin_social_providers_create" + OpAdminSocialProvidersGet = "admin_social_providers_get" + OpAdminSocialProvidersUpdate = "admin_social_providers_update" + OpAdminSocialProvidersDelete = "admin_social_providers_delete" + OpAdminSocialProvidersEnable = "admin_social_providers_enable" + OpAdminSocialProvidersDisable = "admin_social_providers_disable" +) + // socialProvidersListResult builds the shared ListResult view for the social // providers list operation. func socialProvidersListResult(providers []*admin.SocialProvider) ListResult { @@ -93,7 +104,7 @@ func adminSocialProvidersList(d AdminDeps) catalog.Operation { // adminSocialProvidersGet is the `admin social-providers get` operation. func adminSocialProvidersGet(d AdminDeps) catalog.Operation { return catalog.NewOperation(catalog.OperationSpec{ - Name: "admin_social_providers_get", + Name: OpAdminSocialProvidersGet, Title: "Get a social provider", Summary: "Get a social login provider by ID", Description: "Get a single social login provider by numeric ID. Client secrets are never returned. Requires admin privileges.", @@ -125,7 +136,7 @@ func adminSocialProvidersGet(d AdminDeps) catalog.Operation { // adminSocialProvidersCreate is the `admin social-providers create` operation. func adminSocialProvidersCreate(d AdminDeps) catalog.Operation { return catalog.NewOperation(catalog.OperationSpec{ - Name: "admin_social_providers_create", + Name: OpAdminSocialProvidersCreate, Title: "Create a social provider", Summary: "Create a social login provider configuration", Description: "Create a new social login provider configuration (OAuth2 endpoints, client credentials, attribute keys and display metadata). Requires admin privileges.", @@ -177,7 +188,7 @@ func adminSocialProvidersCreate(d AdminDeps) catalog.Operation { // adminSocialProvidersUpdate is the `admin social-providers update` operation. func adminSocialProvidersUpdate(d AdminDeps) catalog.Operation { return catalog.NewOperation(catalog.OperationSpec{ - Name: "admin_social_providers_update", + Name: OpAdminSocialProvidersUpdate, Title: "Update a social provider", Summary: "Update a social login provider configuration", // Nullable arg types matter for updates: omitted must be distinguishable @@ -270,7 +281,7 @@ func adminSocialProvidersUpdate(d AdminDeps) catalog.Operation { // DESTRUCTIVE: requires confirm=true. func adminSocialProvidersDelete(d AdminDeps) catalog.Operation { return catalog.NewOperation(catalog.OperationSpec{ - Name: "admin_social_providers_delete", + Name: OpAdminSocialProvidersDelete, Title: "Delete a social provider", Summary: "Delete a social login provider by ID", Description: "Delete a social login provider configuration by ID. DESTRUCTIVE: users will no longer be able to sign in with this provider. Requires confirm=true. Requires admin privileges.", @@ -309,7 +320,7 @@ func adminSocialProvidersDelete(d AdminDeps) catalog.Operation { // adminSocialProvidersEnable is the `admin social-providers enable` operation. func adminSocialProvidersEnable(d AdminDeps) catalog.Operation { return catalog.NewOperation(catalog.OperationSpec{ - Name: "admin_social_providers_enable", + Name: OpAdminSocialProvidersEnable, Title: "Enable a social provider", Summary: "Enable a social login provider", Description: "Enable a previously disabled social login provider so users can sign in with it. Requires admin privileges.", @@ -342,7 +353,7 @@ func adminSocialProvidersEnable(d AdminDeps) catalog.Operation { // operation. func adminSocialProvidersDisable(d AdminDeps) catalog.Operation { return catalog.NewOperation(catalog.OperationSpec{ - Name: "admin_social_providers_disable", + Name: OpAdminSocialProvidersDisable, Title: "Disable a social provider", Summary: "Disable a social login provider", Description: "Disable a social login provider so it can no longer be used to authenticate. Requires admin privileges.", diff --git a/internal/cli/catalog_admin_social_providers_render_test.go b/internal/cli/catalog_admin_social_providers_render_test.go new file mode 100644 index 00000000..19d40133 --- /dev/null +++ b/internal/cli/catalog_admin_social_providers_render_test.go @@ -0,0 +1,108 @@ +package cli + +import ( + "bytes" + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/urfave/cli/v3" + "go.lumeweb.com/portal-sdk/admin" + + "go.lumeweb.com/pinner-cli/internal/catalog" + "go.lumeweb.com/pinner-cli/internal/catalogops" +) + +// TestRenderAdminSocialProviderResult verifies renderAdminResult handles the +// *admin.SocialProvider type returned by the admin_social_providers create, +// get, update, enable and disable operations. Without a case it fell through +// to the default "unroutable result type" error, breaking every human-readable +// invocation of those commands. +func TestRenderAdminSocialProviderResult(t *testing.T) { + provider := &admin.SocialProvider{} + provider.Id = 3 + provider.ProviderId = "google" + provider.DisplayName = "Google" + provider.Enabled = true + provider.OrderIndex = 2 + provider.ClientId = "client-abc" + provider.Scopes = []string{"openid", "email", "profile"} + + op := catalog.NewOperation(catalog.OperationSpec{Name: catalogops.OpAdminSocialProvidersCreate}) + + t.Run("renders provider as a field group", func(t *testing.T) { + var buf bytes.Buffer + cmd := &cli.Command{ + Name: "create", + Writer: &buf, + Action: func(ctx context.Context, c *cli.Command) error { + return renderAdminResult(ctx, c, op, provider) + }, + } + + require.NoError(t, cmd.Run(t.Context(), []string{"create"})) + got := buf.String() + for _, want := range []string{ + "Social provider", + "google", + "client-abc", + "openid", + } { + assert.Contains(t, got, want) + } + }) + + t.Run("renders JSON when --json is set", func(t *testing.T) { + var buf bytes.Buffer + cmd := &cli.Command{ + Name: "create", + Writer: &buf, + Flags: []cli.Flag{&cli.BoolFlag{Name: FlagJSON}}, + Action: func(ctx context.Context, c *cli.Command) error { + return renderAdminResult(ctx, c, op, provider) + }, + } + + require.NoError(t, cmd.Run(t.Context(), []string{"create", "--json"})) + got := buf.String() + assert.Contains(t, got, `"provider_id": "google"`) + assert.Contains(t, got, `"client_id": "client-abc"`) + assert.Contains(t, got, `"display_name": "Google"`) + }) +} + +// TestRenderAdminSocialProvidersDeleteResult verifies renderAdminResult handles +// the *catalogops.SocialProvidersDeleteResult type returned by +// admin_social_providers_delete. +func TestRenderAdminSocialProvidersDeleteResult(t *testing.T) { + var buf bytes.Buffer + op := catalog.NewOperation(catalog.OperationSpec{Name: catalogops.OpAdminSocialProvidersDelete}) + + cmd := &cli.Command{ + Name: "delete", + Writer: &buf, + Action: func(ctx context.Context, c *cli.Command) error { + return renderAdminResult(ctx, c, op, &catalogops.SocialProvidersDeleteResult{Deleted: true, ID: "7"}) + }, + } + + require.NoError(t, cmd.Run(t.Context(), []string{"delete"})) + assert.Contains(t, buf.String(), "Social provider 7 deleted") +} + +// TestResolveSocialProviderID verifies numeric IDs pass through and provider +// keys resolve against the configured provider list. +func TestResolveSocialProviderID(t *testing.T) { + t.Run("numeric id passes through", func(t *testing.T) { + got, err := resolveSocialProviderID(t.Context(), catalogops.AdminDeps{}, "12") + require.NoError(t, err) + assert.Equal(t, "12", got) + }) + + t.Run("unwired deps error clearly on key", func(t *testing.T) { + _, err := resolveSocialProviderID(t.Context(), catalogops.AdminDeps{}, "google") + require.Error(t, err) + assert.Contains(t, err.Error(), "social provider service unavailable") + }) +} diff --git a/internal/cli/catalog_admin_wiring.go b/internal/cli/catalog_admin_wiring.go index 80e0f8f7..49e5477c 100644 --- a/internal/cli/catalog_admin_wiring.go +++ b/internal/cli/catalog_admin_wiring.go @@ -282,6 +282,21 @@ func adminActionAdapter(op catalog.Operation) cli.ActionFunc { } input["id"] = resolved } + + // The social-provider ops are likewise keyed by numeric ID, but the + // provider key (e.g. google) is the natural handle an operator has. + case catalogops.OpAdminSocialProvidersGet, + catalogops.OpAdminSocialProvidersUpdate, + catalogops.OpAdminSocialProvidersDelete, + catalogops.OpAdminSocialProvidersEnable, + catalogops.OpAdminSocialProvidersDisable: + if id := catalog.StrArg(input, "id", ""); id != "" { + resolved, err := resolveSocialProviderID(ctx, adminCatalogDepsVar, id) + if err != nil { + return err + } + input["id"] = resolved + } } // Destructive gate: destructive admin ops require confirm=true. Other @@ -377,6 +392,37 @@ func resolvePlatformDomainID(ctx context.Context, deps catalogops.AdminDeps, idO return "", fmt.Errorf("platform domain not found for %q", idOrDomain) } +// resolveSocialProviderID resolves a social-provider identifier an operator may +// supply either as the numeric record ID or as the provider key (e.g. google). +// Numeric identifiers pass through unchanged; a key is resolved by listing the +// configured providers and matching on ProviderId. Mirrors resolvePlatformDomainID. +func resolveSocialProviderID(ctx context.Context, deps catalogops.AdminDeps, idOrKey string) (string, error) { + if _, err := strconv.Atoi(idOrKey); err == nil { + return idOrKey, nil + } + if deps.CfgMgr == nil || deps.SocialProviderAdminService == nil { + return "", fmt.Errorf("social provider service unavailable (not wired)") + } + cfgMgr := deps.CfgMgr() + svc, err := deps.SocialProviderAdminService(cfgMgr) + if err != nil { + return "", fmt.Errorf("failed to resolve social provider service: %w", err) + } + if err := svc.RequireAuthenticated(); err != nil { + return "", err + } + providers, _, err := svc.ListSocialProviders(ctx) + if err != nil { + return "", fmt.Errorf("failed to look up social provider by key: %w", err) + } + for _, p := range providers { + if p.ProviderId == idOrKey { + return fmt.Sprintf("%d", p.Id), nil + } + } + return "", fmt.Errorf("social provider %q not found; run 'pinner admin social-providers list' to see configured providers", idOrKey) +} + // renderAdminResult renders an admin handler's typed result through the CLI // Output formatter. func renderAdminResult(_ context.Context, c *cli.Command, op catalog.Operation, result any) error { @@ -423,6 +469,36 @@ func renderAdminResult(_ context.Context, c *cli.Command, op catalog.Operation, output.Printfln("Platform domain %s deleted", r.ID) return nil + case *catalogops.SocialProvidersDeleteResult: + if output.IsJSON() { + return output.PrintJSON(map[string]any{"deleted": r.Deleted, "id": r.ID}) + } + output.Printfln("Social provider %s deleted", r.ID) + return nil + + case *admin.SocialProvider: + // create/get/update/enable/disable all return the provider object; + // client secrets are never present on this response. + if output.IsJSON() { + return output.PrintJSON(r) + } + output.PrintFields(FieldGroup{Title: "Social provider", Fields: []Field{ + {"ID", fmt.Sprintf("%d", r.Id)}, + {"Provider", r.ProviderId}, + {"Display name", r.DisplayName}, + {"Enabled", yesNo(r.Enabled)}, + {"Order", fmt.Sprintf("%d", r.OrderIndex)}, + {"Client ID", r.ClientId}, + {"Auth URL", r.AuthUrl}, + {"Token URL", r.TokenUrl}, + {"User URL", r.UserUrl}, + {"Scopes", strings.Join(r.Scopes, ", ")}, + {"User ID key", r.UserIdKey}, + {"User email key", r.UserEmailKey}, + {"User name key", r.UserNameKey}, + }}) + return nil + case *admin.QuotaPlan: if output.IsJSON() { return output.PrintJSON(r)