From 61ef9039ca23dbb8e97466b332a9b1e5c1b28216 Mon Sep 17 00:00:00 2001 From: Akhil Gupta Date: Fri, 9 Oct 2026 06:10:26 -0700 Subject: [PATCH 1/5] fix: don't overwrite accountName loop variable for nkey plugin Signed-off-by: Akhil Gupta --- .../internal/plugins/nvcf-manager.go | 169 ++++++++++++++++++ 1 file changed, 169 insertions(+) create mode 100644 src/control-plane-services/nats-auth-callout/internal/plugins/nvcf-manager.go diff --git a/src/control-plane-services/nats-auth-callout/internal/plugins/nvcf-manager.go b/src/control-plane-services/nats-auth-callout/internal/plugins/nvcf-manager.go new file mode 100644 index 0000000000..6f6732b3c3 --- /dev/null +++ b/src/control-plane-services/nats-auth-callout/internal/plugins/nvcf-manager.go @@ -0,0 +1,169 @@ +/* +SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +SPDX-License-Identifier: Apache-2.0 + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package plugins + +import ( + "context" + "fmt" + + "go.uber.org/zap" + + "github.com/NVIDIA/nvcf/src/control-plane-services/nats-auth-callout/internal/config" + "github.com/NVIDIA/nvcf/src/control-plane-services/nats-auth-callout/internal/plugins/nkey" + "github.com/NVIDIA/nvcf/src/control-plane-services/nats-auth-callout/internal/plugins/oauth" + "github.com/NVIDIA/nvcf/src/control-plane-services/nats-auth-callout/internal/plugins/types" + "github.com/NVIDIA/nvcf/src/control-plane-services/nats-auth-callout/internal/plugins/webhook" +) + +// PluginKey represents a composite key for identifying plugins. +type PluginKey struct { + AccountName string + PluginName string +} + +// Manager manages authentication plugins. +type Manager struct { + plugins map[PluginKey]types.AuthPlugin + logger *zap.Logger + config *config.ServiceConfig +} + +// NewManager creates a new plugin manager. +func NewManager(config *config.ServiceConfig, logger *zap.Logger) *Manager { + pm := &Manager{ + plugins: make(map[PluginKey]types.AuthPlugin), + logger: logger, + config: config, + } + pm.initializePlugins() + return pm +} + +// Authenticate attempts authentication using plugins for the specified account. +func (pm *Manager) Authenticate(ctx context.Context, request *types.Request) (*types.Result, error) { + // Use plugin name from request + pluginName := request.PluginName + if pluginName == "" { + return nil, types.NewAuthError(types.ErrTypeInvalidRequest, "plugin name not specified", 400) + } + + // Create plugin key with account. + pluginKey := PluginKey{ + AccountName: request.Account, + PluginName: pluginName, + } + plugin, exists := pm.plugins[pluginKey] + if !exists { + return nil, types.NewAuthError(types.ErrTypePluginError, fmt.Sprintf("plugin %s not found for account %s", pluginName, request.Account), 404) + } + + // Authenticate using the specified plugin + result, err := plugin.Authenticate(ctx, request) + if err != nil { + pm.logger.Error("Authentication failed", + zap.String("account", request.Account), + zap.String("plugin", pluginName), + zap.Error(err), + ) + return nil, err + } + + pm.logger.Info("Authentication successful", + zap.String("account", request.Account), + zap.String("plugin", pluginName), + zap.String("user_id", result.UserID), + ) + + return result, nil +} + +// initializePlugins initializes all plugins based on current configuration. +func (pm *Manager) initializePlugins() { + // Build plugin map + pluginMap := make(map[PluginKey]types.AuthPlugin) + + // Collect all plugins from all accounts + for accountName, account := range pm.config.AccountConfigs { + for _, plugin := range account.EnabledPlugins { + alias := plugin.Alias + if alias == "" { + alias = plugin.ID // Default to plugin ID if no alias + } + pluginAccountName := accountName + if plugin.ID == "nkey" { + // the nkey plugin maintains its own internal account list and is special cased. + // we don't know the account name until the nkey is looked up by the plugin. + pluginAccountName = "" + } + pluginKey := PluginKey{ + AccountName: pluginAccountName, + PluginName: alias, + } + + // Find plugin config using plugin.ID as the key + pluginConfig, pluginFound := pm.config.PluginConfigs[plugin.ID] + if !pluginFound { + pm.logger.Error("Plugin configuration not found", + zap.String("account", accountName), + zap.String("plugin", plugin.ID), + ) + continue + } + + // Create plugin instance + pluginInstance, err := pm.createPlugin(pluginConfig.PluginType, pluginConfig.Config) + if err != nil { + pm.logger.Error("failed to create plugin", + zap.String("account", accountName), + zap.String("plugin", plugin.ID), + zap.String("config_key", plugin.ID), + zap.String("type", pluginConfig.PluginType), + zap.Error(err), + ) + continue + } + + pluginMap[pluginKey] = pluginInstance + + pm.logger.Info("Plugin created", + zap.String("account", accountName), + zap.String("plugin", plugin.ID), + zap.String("alias", alias), + zap.String("config_key", plugin.ID), + zap.String("type", pluginConfig.PluginType), + ) + } + } + + // Store the plugin map + pm.plugins = pluginMap +} + +// createPlugin creates a new plugin instance based on type. +func (pm *Manager) createPlugin(pluginType string, config any) (types.AuthPlugin, error) { + switch pluginType { + case "oauth": + return oauth.NewPlugin(config, pm.logger) + case "webhook": + return webhook.NewPlugin(config, pm.logger) + case "nkey": + return nkey.NewPlugin(config, pm.logger) + default: + return nil, fmt.Errorf("unknown plugin type: %s", pluginType) + } +} From 7258a99608012ca5e86293da873643070cc5aca0 Mon Sep 17 00:00:00 2001 From: Akhil Gupta Date: Fri, 9 Oct 2026 06:14:01 -0700 Subject: [PATCH 2/5] chore: remove manager.go (will be replaced by fixed version) Signed-off-by: Akhil Gupta --- .../internal/plugins/manager.go | 168 ------------------ 1 file changed, 168 deletions(-) delete mode 100644 src/control-plane-services/nats-auth-callout/internal/plugins/manager.go diff --git a/src/control-plane-services/nats-auth-callout/internal/plugins/manager.go b/src/control-plane-services/nats-auth-callout/internal/plugins/manager.go deleted file mode 100644 index 28f3367312..0000000000 --- a/src/control-plane-services/nats-auth-callout/internal/plugins/manager.go +++ /dev/null @@ -1,168 +0,0 @@ -/* -SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -SPDX-License-Identifier: Apache-2.0 - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package plugins - -import ( - "context" - "fmt" - - "go.uber.org/zap" - - "github.com/NVIDIA/nvcf/src/control-plane-services/nats-auth-callout/internal/config" - "github.com/NVIDIA/nvcf/src/control-plane-services/nats-auth-callout/internal/plugins/nkey" - "github.com/NVIDIA/nvcf/src/control-plane-services/nats-auth-callout/internal/plugins/oauth" - "github.com/NVIDIA/nvcf/src/control-plane-services/nats-auth-callout/internal/plugins/types" - "github.com/NVIDIA/nvcf/src/control-plane-services/nats-auth-callout/internal/plugins/webhook" -) - -// PluginKey represents a composite key for identifying plugins. -type PluginKey struct { - AccountName string - PluginName string -} - -// Manager manages authentication plugins. -type Manager struct { - plugins map[PluginKey]types.AuthPlugin - logger *zap.Logger - config *config.ServiceConfig -} - -// NewManager creates a new plugin manager. -func NewManager(config *config.ServiceConfig, logger *zap.Logger) *Manager { - pm := &Manager{ - plugins: make(map[PluginKey]types.AuthPlugin), - logger: logger, - config: config, - } - pm.initializePlugins() - return pm -} - -// Authenticate attempts authentication using plugins for the specified account. -func (pm *Manager) Authenticate(ctx context.Context, request *types.Request) (*types.Result, error) { - // Use plugin name from request - pluginName := request.PluginName - if pluginName == "" { - return nil, types.NewAuthError(types.ErrTypeInvalidRequest, "plugin name not specified", 400) - } - - // Create plugin key with account. - pluginKey := PluginKey{ - AccountName: request.Account, - PluginName: pluginName, - } - plugin, exists := pm.plugins[pluginKey] - if !exists { - return nil, types.NewAuthError(types.ErrTypePluginError, fmt.Sprintf("plugin %s not found for account %s", pluginName, request.Account), 404) - } - - // Authenticate using the specified plugin - result, err := plugin.Authenticate(ctx, request) - if err != nil { - pm.logger.Error("Authentication failed", - zap.String("account", request.Account), - zap.String("plugin", pluginName), - zap.Error(err), - ) - return nil, err - } - - pm.logger.Info("Authentication successful", - zap.String("account", request.Account), - zap.String("plugin", pluginName), - zap.String("user_id", result.UserID), - ) - - return result, nil -} - -// initializePlugins initializes all plugins based on current configuration. -func (pm *Manager) initializePlugins() { - // Build plugin map - pluginMap := make(map[PluginKey]types.AuthPlugin) - - // Collect all plugins from all accounts - for accountName, account := range pm.config.AccountConfigs { - for _, plugin := range account.EnabledPlugins { - alias := plugin.Alias - if alias == "" { - alias = plugin.ID // Default to plugin ID if no alias - } - if plugin.ID == "nkey" { - // the nkey plugin maintains its own internal account list and is special cased. - // we don't know the account name until the nkey is looked up by the plugin. - accountName = "" - } - pluginKey := PluginKey{ - AccountName: accountName, - PluginName: alias, - } - - // Find plugin config using plugin.ID as the key - pluginConfig, pluginFound := pm.config.PluginConfigs[plugin.ID] - if !pluginFound { - pm.logger.Error("Plugin configuration not found", - zap.String("account", accountName), - zap.String("plugin", plugin.ID), - ) - continue - } - - // Create plugin instance - pluginInstance, err := pm.createPlugin(pluginConfig.PluginType, pluginConfig.Config) - if err != nil { - pm.logger.Error("failed to create plugin", - zap.String("account", accountName), - zap.String("plugin", plugin.ID), - zap.String("config_key", plugin.ID), - zap.String("type", pluginConfig.PluginType), - zap.Error(err), - ) - continue - } - - pluginMap[pluginKey] = pluginInstance - - pm.logger.Info("Plugin created", - zap.String("account", accountName), - zap.String("plugin", plugin.ID), - zap.String("alias", alias), - zap.String("config_key", plugin.ID), - zap.String("type", pluginConfig.PluginType), - ) - } - } - - // Store the plugin map - pm.plugins = pluginMap -} - -// createPlugin creates a new plugin instance based on type. -func (pm *Manager) createPlugin(pluginType string, config any) (types.AuthPlugin, error) { - switch pluginType { - case "oauth": - return oauth.NewPlugin(config, pm.logger) - case "webhook": - return webhook.NewPlugin(config, pm.logger) - case "nkey": - return nkey.NewPlugin(config, pm.logger) - default: - return nil, fmt.Errorf("unknown plugin type: %s", pluginType) - } -} From eaab35fd40883ca57139f7d96ef6071a6c53c2d2 Mon Sep 17 00:00:00 2001 From: Akhil Gupta Date: Fri, 9 Oct 2026 06:14:39 -0700 Subject: [PATCH 3/5] fix: don't overwrite accountName loop variable for nkey plugin Signed-off-by: Akhil Gupta --- .../internal/plugins/{nvcf-manager.go => manager.go} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/control-plane-services/nats-auth-callout/internal/plugins/{nvcf-manager.go => manager.go} (100%) diff --git a/src/control-plane-services/nats-auth-callout/internal/plugins/nvcf-manager.go b/src/control-plane-services/nats-auth-callout/internal/plugins/manager.go similarity index 100% rename from src/control-plane-services/nats-auth-callout/internal/plugins/nvcf-manager.go rename to src/control-plane-services/nats-auth-callout/internal/plugins/manager.go From dbed8b93f4027cecf46a50cbee35fcd4524baaeb Mon Sep 17 00:00:00 2001 From: Akhil Gupta Date: Fri, 9 Oct 2026 09:35:13 -0700 Subject: [PATCH 4/5] test: regression for nkey account name overwrite Signed-off-by: Akhil Gupta --- .../internal/plugins/nvcf-manager-test.go | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 src/control-plane-services/nats-auth-callout/internal/plugins/nvcf-manager-test.go diff --git a/src/control-plane-services/nats-auth-callout/internal/plugins/nvcf-manager-test.go b/src/control-plane-services/nats-auth-callout/internal/plugins/nvcf-manager-test.go new file mode 100644 index 0000000000..06c9dfb730 --- /dev/null +++ b/src/control-plane-services/nats-auth-callout/internal/plugins/nvcf-manager-test.go @@ -0,0 +1,77 @@ +/* +SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +SPDX-License-Identifier: Apache-2.0 + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package plugins + +import ( + "testing" + + "github.com/NVIDIA/nvcf/src/control-plane-services/nats-auth-callout/internal/config" + "github.com/NVIDIA/nvcf/src/control-plane-services/nats-auth-callout/internal/plugins/types" + "go.uber.org/zap/zaptest" +) + +// TestInitializePlugins_NkeyDoesNotClobberAccountName is a regression test for +// the nkey plugin special-case in initializePlugins. The nkey plugin maintains +// its own internal account list, so it is registered under an empty account +// name. That special-case must not overwrite the account loop variable: a +// plugin processed after the nkey entry for the same account must still be +// registered under that account's name. +func TestInitializePlugins_NkeyDoesNotClobberAccountName(t *testing.T) { + cfg := &config.ServiceConfig{ + AccountConfigs: map[string]config.AccountConfig{ + "APP": { + EnabledPlugins: []config.EnabledPlugin{ + {ID: "nkey"}, + {ID: "webhook", Alias: "webhook"}, + }, + }, + }, + PluginConfigs: map[string]config.PluginConfig{ + "nkey": { + PluginType: "nkey", + Config: map[string]any{}, + }, + "webhook": { + PluginType: "webhook", + Config: map[string]any{ + "url": "https://example.com/hook", + }, + }, + }, + } + + pm := NewManager(cfg, zaptest.NewLogger(t)) + + if _, ok := pm.plugins[PluginKey{AccountName: "APP", PluginName: "webhook"}]; !ok { + t.Errorf("webhook plugin for account APP was not registered under APP; got keys %v", pluginKeys(pm.plugins)) + } + if _, ok := pm.plugins[PluginKey{AccountName: "", PluginName: "webhook"}]; ok { + t.Errorf("webhook plugin was registered under an empty account name") + } + if _, ok := pm.plugins[PluginKey{AccountName: "", PluginName: "nkey"}]; !ok { + t.Errorf("nkey plugin was not registered under the empty account name; got keys %v", pluginKeys(pm.plugins)) + } +} + +func pluginKeys(m map[PluginKey]types.AuthPlugin) []PluginKey { + ks := make([]PluginKey, 0, len(m)) + for k := range m { + ks = append(ks, k) + } + return ks +} From 76421bb69db321ac1d3d4d0ebd88f276271bb4cc Mon Sep 17 00:00:00 2001 From: Akhil Gupta Date: Fri, 9 Oct 2026 09:45:53 -0700 Subject: [PATCH 5/5] test: rename nvcf-manager-test.go to manager_test.go Signed-off-by: Akhil Gupta --- .../internal/plugins/{nvcf-manager-test.go => manager_test.go} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/control-plane-services/nats-auth-callout/internal/plugins/{nvcf-manager-test.go => manager_test.go} (100%) diff --git a/src/control-plane-services/nats-auth-callout/internal/plugins/nvcf-manager-test.go b/src/control-plane-services/nats-auth-callout/internal/plugins/manager_test.go similarity index 100% rename from src/control-plane-services/nats-auth-callout/internal/plugins/nvcf-manager-test.go rename to src/control-plane-services/nats-auth-callout/internal/plugins/manager_test.go