Skip to content

Commit 9e0190b

Browse files
ghaskinsghaskins
committed
[#3109] Implement authorization abstraction
This patch implements an 'authorizer' abstraction under pkg/authz, and then moves the Cedar implementation as the first/canonical form under pkg/authz/authorizers/cedar. The configuration schema remains untouched, though the mechanism for loading configuration has been reworked to avoid violating the authorizer abstraction with Cedar-isms. This fixes #3109 Signed-off-by: Greg Haskins <greg@manetu.com>
1 parent 5866023 commit 9e0190b

33 files changed

+3541
-882
lines changed

‎Taskfile.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ tasks:
8686
platforms: [linux, darwin]
8787
# we have to use ldflags to avoid the LC_DYSYMTAB linker error.
8888
# https://github.com/stacklok/toolhive/issues/1687
89-
- go test -ldflags=-extldflags=-Wl,-w -json -race -coverprofile=coverage/coverage.out $(go list ./... | grep -v '/test/e2e' | grep -v '/cmd/thv-operator/test-integration') | gotestfmt -hide "all"
89+
- go test -ldflags=-extldflags=-Wl,-w -json -race -coverpkg=./... -coverprofile=coverage/coverage.out $(go list ./... | grep -v '/test/e2e' | grep -v '/cmd/thv-operator/test-integration') | gotestfmt -hide "all"
9090
- go tool cover -func=coverage/coverage.out
9191
- echo "Generating HTML coverage report in coverage/coverage.html"
9292
- go tool cover -html=coverage/coverage.out -o coverage/coverage.html
@@ -101,7 +101,7 @@ tasks:
101101
cmds:
102102
- cmd: cmd.exe /c mkdir coverage
103103
ignore_error: true # Windows has no mkdir -p, so just ignore error if it exists
104-
- go test -race -coverprofile=coverage/coverage.out {{.DIR_LIST | catLines}}
104+
- go test -race -coverpkg=./... -coverprofile=coverage/coverage.out {{.DIR_LIST | catLines}}
105105
- go tool cover -func=coverage/coverage.out
106106
- echo "Generating HTML coverage report in coverage/coverage.html"
107107
- go tool cover -html=coverage/coverage.out -o coverage/coverage.html

‎cmd/thv-operator/controllers/mcpremoteproxy_runconfig_test.go‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030

3131
mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1"
3232
"github.com/stacklok/toolhive/pkg/authz"
33+
"github.com/stacklok/toolhive/pkg/authz/authorizers/cedar"
3334
"github.com/stacklok/toolhive/pkg/runner"
3435
transporttypes "github.com/stacklok/toolhive/pkg/transport/types"
3536
)
@@ -158,10 +159,12 @@ func TestCreateRunConfigFromMCPRemoteProxy(t *testing.T) {
158159
t.Helper()
159160
assert.Equal(t, "authz-proxy", config.Name)
160161
assert.NotNil(t, config.AuthzConfig)
161-
assert.Equal(t, authz.ConfigTypeCedarV1, config.AuthzConfig.Type)
162-
assert.NotNil(t, config.AuthzConfig.Cedar)
163-
assert.Len(t, config.AuthzConfig.Cedar.Policies, 2)
164-
assert.Contains(t, config.AuthzConfig.Cedar.Policies[0], "tools/list")
162+
assert.Equal(t, authz.ConfigType(cedar.ConfigType), config.AuthzConfig.Type)
163+
164+
cedarCfg, err := cedar.ExtractConfig(config.AuthzConfig)
165+
require.NoError(t, err)
166+
assert.Len(t, cedarCfg.Options.Policies, 2)
167+
assert.Contains(t, cedarCfg.Options.Policies[0], "tools/list")
165168
},
166169
},
167170
{

‎cmd/thv-operator/controllers/mcpserver_runconfig_test.go‎

Lines changed: 30 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
ctrlutil "github.com/stacklok/toolhive/cmd/thv-operator/pkg/controllerutil"
2020
"github.com/stacklok/toolhive/cmd/thv-operator/pkg/runconfig/configmap/checksum"
2121
"github.com/stacklok/toolhive/pkg/authz"
22+
"github.com/stacklok/toolhive/pkg/authz/authorizers/cedar"
2223
"github.com/stacklok/toolhive/pkg/container/kubernetes"
2324
"github.com/stacklok/toolhive/pkg/runner"
2425
transporttypes "github.com/stacklok/toolhive/pkg/transport/types"
@@ -322,14 +323,15 @@ func TestCreateRunConfigFromMCPServer(t *testing.T) {
322323
// Verify authorization config is set
323324
assert.NotNil(t, config.AuthzConfig)
324325
assert.Equal(t, "v1", config.AuthzConfig.Version)
325-
assert.Equal(t, authz.ConfigTypeCedarV1, config.AuthzConfig.Type)
326-
assert.NotNil(t, config.AuthzConfig.Cedar)
326+
assert.Equal(t, authz.ConfigType(cedar.ConfigType), config.AuthzConfig.Type)
327327

328328
// Check Cedar-specific configuration
329-
assert.Len(t, config.AuthzConfig.Cedar.Policies, 2)
330-
assert.Contains(t, config.AuthzConfig.Cedar.Policies, `permit(principal, action == Action::"call_tool", resource == Tool::"weather");`)
331-
assert.Contains(t, config.AuthzConfig.Cedar.Policies, `permit(principal, action == Action::"get_prompt", resource == Prompt::"greeting");`)
332-
assert.Equal(t, `[{"uid": {"type": "User", "id": "user1"}, "attrs": {}}]`, config.AuthzConfig.Cedar.EntitiesJSON)
329+
cedarCfg, err := cedar.ExtractConfig(config.AuthzConfig)
330+
require.NoError(t, err)
331+
assert.Len(t, cedarCfg.Options.Policies, 2)
332+
assert.Contains(t, cedarCfg.Options.Policies, `permit(principal, action == Action::"call_tool", resource == Tool::"weather");`)
333+
assert.Contains(t, cedarCfg.Options.Policies, `permit(principal, action == Action::"get_prompt", resource == Prompt::"greeting");`)
334+
assert.Equal(t, `[{"uid": {"type": "User", "id": "user1"}, "attrs": {}}]`, cedarCfg.Options.EntitiesJSON)
333335
},
334336
},
335337
{
@@ -359,11 +361,13 @@ func TestCreateRunConfigFromMCPServer(t *testing.T) {
359361
// For ConfigMap type, with new feature, authorization config is embedded in RunConfig
360362
require.NotNil(t, config.AuthzConfig)
361363
assert.Equal(t, "v1", config.AuthzConfig.Version)
362-
assert.Equal(t, authz.ConfigTypeCedarV1, config.AuthzConfig.Type)
363-
require.NotNil(t, config.AuthzConfig.Cedar)
364-
assert.Len(t, config.AuthzConfig.Cedar.Policies, 1)
365-
assert.Contains(t, config.AuthzConfig.Cedar.Policies[0], "call_tool")
366-
assert.Equal(t, "[]", config.AuthzConfig.Cedar.EntitiesJSON)
364+
assert.Equal(t, authz.ConfigType(cedar.ConfigType), config.AuthzConfig.Type)
365+
366+
cedarCfg, err := cedar.ExtractConfig(config.AuthzConfig)
367+
require.NoError(t, err)
368+
assert.Len(t, cedarCfg.Options.Policies, 1)
369+
assert.Contains(t, cedarCfg.Options.Policies[0], "call_tool")
370+
assert.Equal(t, "[]", cedarCfg.Options.EntitiesJSON)
367371
},
368372
},
369373
{
@@ -748,14 +752,15 @@ func TestEnsureRunConfigConfigMap(t *testing.T) {
748752
// Verify authorization configuration is properly serialized
749753
assert.NotNil(t, runConfig.AuthzConfig, "AuthzConfig should be present in runconfig.json")
750754
assert.Equal(t, "v1", runConfig.AuthzConfig.Version)
751-
assert.Equal(t, authz.ConfigTypeCedarV1, runConfig.AuthzConfig.Type)
752-
assert.NotNil(t, runConfig.AuthzConfig.Cedar)
755+
assert.Equal(t, authz.ConfigType(cedar.ConfigType), runConfig.AuthzConfig.Type)
753756

754757
// Check Cedar-specific configuration
755-
assert.Len(t, runConfig.AuthzConfig.Cedar.Policies, 2)
756-
assert.Contains(t, runConfig.AuthzConfig.Cedar.Policies, `permit(principal, action == Action::"call_tool", resource == Tool::"weather");`)
757-
assert.Contains(t, runConfig.AuthzConfig.Cedar.Policies, `permit(principal, action == Action::"get_prompt", resource == Prompt::"greeting");`)
758-
assert.Equal(t, `[{"uid": {"type": "User", "id": "user1"}, "attrs": {}}]`, runConfig.AuthzConfig.Cedar.EntitiesJSON)
758+
cedarCfg, err := cedar.ExtractConfig(runConfig.AuthzConfig)
759+
require.NoError(t, err)
760+
assert.Len(t, cedarCfg.Options.Policies, 2)
761+
assert.Contains(t, cedarCfg.Options.Policies, `permit(principal, action == Action::"call_tool", resource == Tool::"weather");`)
762+
assert.Contains(t, cedarCfg.Options.Policies, `permit(principal, action == Action::"get_prompt", resource == Prompt::"greeting");`)
763+
assert.Equal(t, `[{"uid": {"type": "User", "id": "user1"}, "attrs": {}}]`, cedarCfg.Options.EntitiesJSON)
759764
},
760765
},
761766
{
@@ -968,12 +973,14 @@ func TestEnsureRunConfigConfigMap(t *testing.T) {
968973

969974
require.NotNil(t, runConfig.AuthzConfig)
970975
assert.Equal(t, "v1", runConfig.AuthzConfig.Version)
971-
assert.Equal(t, authz.ConfigTypeCedarV1, runConfig.AuthzConfig.Type)
972-
require.NotNil(t, runConfig.AuthzConfig.Cedar)
973-
assert.Len(t, runConfig.AuthzConfig.Cedar.Policies, 2)
974-
assert.Contains(t, runConfig.AuthzConfig.Cedar.Policies, `permit(principal, action == Action::"call_tool", resource == Tool::"weather");`)
975-
assert.Contains(t, runConfig.AuthzConfig.Cedar.Policies, `permit(principal, action == Action::"get_prompt", resource == Prompt::"greeting");`)
976-
assert.Equal(t, `[{"uid": {"type": "User", "id": "user1"}, "attrs": {}}]`, runConfig.AuthzConfig.Cedar.EntitiesJSON)
976+
assert.Equal(t, authz.ConfigType(cedar.ConfigType), runConfig.AuthzConfig.Type)
977+
978+
cedarCfg, err := cedar.ExtractConfig(runConfig.AuthzConfig)
979+
require.NoError(t, err)
980+
assert.Len(t, cedarCfg.Options.Policies, 2)
981+
assert.Contains(t, cedarCfg.Options.Policies, `permit(principal, action == Action::"call_tool", resource == Tool::"weather");`)
982+
assert.Contains(t, cedarCfg.Options.Policies, `permit(principal, action == Action::"get_prompt", resource == Prompt::"greeting");`)
983+
assert.Equal(t, `[{"uid": {"type": "User", "id": "user1"}, "attrs": {}}]`, cedarCfg.Options.EntitiesJSON)
977984
})
978985
}
979986

‎cmd/thv-operator/pkg/controllerutil/authz.go‎

Lines changed: 32 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1"
1818
"github.com/stacklok/toolhive/cmd/thv-operator/pkg/kubernetes/configmaps"
1919
"github.com/stacklok/toolhive/pkg/authz"
20+
"github.com/stacklok/toolhive/pkg/authz/authorizers/cedar"
2021
"github.com/stacklok/toolhive/pkg/runner"
2122
)
2223

@@ -162,6 +163,36 @@ func EnsureAuthzConfigMap(
162163
return nil
163164
}
164165

166+
func addAuthzInlineConfigOptions(
167+
authzRef *mcpv1alpha1.AuthzConfigRef,
168+
options *[]runner.RunConfigBuilderOption,
169+
) error {
170+
if authzRef.Inline == nil {
171+
return fmt.Errorf("inline authz config type specified but inline config is nil")
172+
}
173+
174+
policies := authzRef.Inline.Policies
175+
entitiesJSON := authzRef.Inline.EntitiesJSON
176+
177+
// Create authorization config using the full config structure
178+
// This maintains backwards compatibility with the v1.0 schema
179+
authzCfg, err := authz.NewConfig(cedar.Config{
180+
Version: "v1",
181+
Type: cedar.ConfigType,
182+
Options: &cedar.ConfigOptions{
183+
Policies: policies,
184+
EntitiesJSON: entitiesJSON,
185+
},
186+
})
187+
if err != nil {
188+
return fmt.Errorf("failed to create authz config: %w", err)
189+
}
190+
191+
// Add authorization config to options
192+
*options = append(*options, runner.WithAuthzConfig(authzCfg))
193+
return nil
194+
}
195+
165196
// AddAuthzConfigOptions adds authorization configuration options to builder options
166197
func AddAuthzConfigOptions(
167198
ctx context.Context,
@@ -176,26 +207,7 @@ func AddAuthzConfigOptions(
176207

177208
switch authzRef.Type {
178209
case mcpv1alpha1.AuthzConfigTypeInline:
179-
if authzRef.Inline == nil {
180-
return fmt.Errorf("inline authz config type specified but inline config is nil")
181-
}
182-
183-
policies := authzRef.Inline.Policies
184-
entitiesJSON := authzRef.Inline.EntitiesJSON
185-
186-
// Create authorization config
187-
authzCfg := &authz.Config{
188-
Version: "v1",
189-
Type: authz.ConfigTypeCedarV1,
190-
Cedar: &authz.CedarConfig{
191-
Policies: policies,
192-
EntitiesJSON: entitiesJSON,
193-
},
194-
}
195-
196-
// Add authorization config to options
197-
*options = append(*options, runner.WithAuthzConfig(authzCfg))
198-
return nil
210+
return addAuthzInlineConfigOptions(authzRef, options)
199211

200212
case mcpv1alpha1.AuthzConfigTypeConfigMap:
201213
// Validate reference

0 commit comments

Comments
 (0)