Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions internal/api/handler_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ func (h *Handler) updateConfig(ctx context.Context, req *events.LambdaFunctionUR
}
_, gracePresent := present["grace_period_days"]
modeSent := riExchangeModeSent(present)
providersSent := bodyHasKey(present, "enabled_providers")
if err := h.requireGlobalConfigScope(ctx, session); err != nil {
return nil, err
}
Expand Down Expand Up @@ -114,7 +115,7 @@ func (h *Handler) updateConfig(ctx context.Context, req *events.LambdaFunctionUR
if uErr := json.Unmarshal([]byte(req.Body), existing); uErr != nil {
return NewClientError(400, "invalid request body")
}
if vErr := validateGlobalConfigWrite(existing, modeSent || existing.RIExchangeMode != before.mode); vErr != nil {
if vErr := validateGlobalConfigWrite(existing, modeSent || existing.RIExchangeMode != before.mode, providersSent); vErr != nil {
return vErr
}
if gErr := requireRIExchangeAutoModeGrant(before, riExchangeArmStateOf(existing), grants.riExchange); gErr != nil {
Expand Down Expand Up @@ -145,6 +146,17 @@ func globalConfigWriteFields(body string) (map[string]json.RawMessage, error) {
return present, nil
}

// bodyHasKey reports whether the body carries the key under any casing, as
// json.Unmarshal matches keys case-insensitively. An explicit null counts.
func bodyHasKey(present map[string]json.RawMessage, name string) bool {
for key := range present {
if strings.EqualFold(key, name) {
return true
}
}
return false
}

// riExchangeModeSent reports whether the body writes ri_exchange_mode. A JSON
// null does not: json.Unmarshal leaves the stored value in place for it.
func riExchangeModeSent(present map[string]json.RawMessage) bool {
Expand All @@ -166,7 +178,12 @@ func riExchangeModeSent(present map[string]json.RawMessage) bool {
// such as "Auto". Such a row keeps its value until a caller sends a valid one,
// and armed() keeps treating it as auto. Runs before the arming gates so a
// malformed write never reveals grant state.
func validateGlobalConfigWrite(cfg *config.GlobalConfig, modeSent bool) error {
func validateGlobalConfigWrite(cfg *config.GlobalConfig, modeSent, providersSent bool) error {
// Presence-based, not in Validate: a fresh install stores [] and partial
// saves that omit the key must keep working.
if providersSent && len(cfg.EnabledProviders) == 0 {
return NewClientError(400, "enabled_providers must list at least one provider")
}
if modeSent && !config.IsValidRIExchangeMode(cfg.RIExchangeMode) {
return NewClientError(400, "ri_exchange_mode must be 'manual' or 'auto'")
}
Expand Down
79 changes: 79 additions & 0 deletions internal/api/handler_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1613,3 +1613,82 @@ func TestHandler_updateConfig_RequiresUpdateConfigPermission(t *testing.T) {
mockStore.AssertNotCalled(t, "GetGlobalConfig", mock.Anything)
mockStore.AssertNotCalled(t, "SaveGlobalConfig", mock.Anything, mock.Anything)
}

func TestHandler_updateConfig_EnabledProvidersWrite(t *testing.T) {
cases := []struct {
name string
stored []string
body string
wantErr bool
wantSave bool
}{
{"empty list refused", []string{"aws"}, `{"enabled_providers": []}`, true, false},
{"null refused", []string{"aws"}, `{"enabled_providers": null}`, true, false},
{"duplicate key differing in case refused", []string{"aws"}, `{"enabled_providers":["aws"],"ENABLED_PROVIDERS":[]}`, true, false},
{"mixed-case key refused", []string{"aws"}, `{"Enabled_Providers": []}`, true, false},
{"omitted key on stored empty row accepted", []string{}, `{"laddering_enabled": false}`, false, true},
{"one provider accepted", []string{}, `{"enabled_providers": ["aws"]}`, false, true},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
ctx := context.Background()
mockStore := new(MockConfigStore)
mockAuth := new(MockAuthService)
t.Cleanup(func() { mockStore.AssertExpectations(t); mockAuth.AssertExpectations(t) })
mockAuth.On("ValidateSession", ctx, "admin-token").
Return(&Session{UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", Email: "admin@example.com"}, nil)
mockAuth.grantAdmin()
mockStore.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{
EnabledProviders: tc.stored, DefaultTerm: 3, DefaultPayment: "all-upfront",
DefaultCoverage: 80, DefaultRampSchedule: "immediate", RIExchangeMode: "manual",
RIExchangeLookbackDays: 30, RecommendationsCacheStaleHours: 24, RecommendationsLookbackDays: 7,
}, nil)
if tc.wantSave {
mockStore.On("SaveGlobalConfig", ctx, mock.AnythingOfType("*config.GlobalConfig")).Return(nil)
}
handler := &Handler{config: mockStore, auth: mockAuth}
req := &events.LambdaFunctionURLRequest{
Headers: map[string]string{"Authorization": "Bearer admin-token"},
Body: tc.body,
}
result, err := handler.updateConfig(ctx, req)
if tc.wantErr {
require.Error(t, err)
assert.Nil(t, result)
ce, ok := IsClientError(err)
require.True(t, ok, "expected ClientError, got %T: %v", err, err)
assert.Equal(t, 400, ce.code)
assert.Contains(t, err.Error(), "enabled_providers must list at least one provider")
mockStore.AssertNotCalled(t, "SaveGlobalConfig", mock.Anything, mock.Anything)
return
}
require.NoError(t, err)
assert.Equal(t, "updated", result.Status)
})
}
}

func TestHandler_updateConfig_EmptyEnabledProvidersWithoutPermissionIs403(t *testing.T) {
ctx := context.Background()
mockStore := new(MockConfigStore)
mockAuth := new(MockAuthService)
t.Cleanup(func() { mockStore.AssertExpectations(t); mockAuth.AssertExpectations(t) })

userSession := &Session{UserID: "dddddddd-dddd-dddd-dddd-dddddddddddd", Email: "standard@example.com"}
mockAuth.On("ValidateSession", ctx, "user-token").Return(userSession, nil)
mockAuth.On("HasPermissionAPI", ctx, userSession.UserID, mock.AnythingOfType("string"), mock.AnythingOfType("string")).
Return(false, nil).Maybe()

handler := &Handler{config: mockStore, auth: mockAuth}
req := &events.LambdaFunctionURLRequest{
Headers: map[string]string{"Authorization": "Bearer user-token"},
Body: `{"enabled_providers": []}`,
}
result, err := handler.updateConfig(ctx, req)
require.Error(t, err)
assert.Nil(t, result)
ce, ok := IsClientError(err)
require.True(t, ok, "expected ClientError, got %T: %v", err, err)
assert.Equal(t, 403, ce.code)
mockStore.AssertNotCalled(t, "SaveGlobalConfig", mock.Anything, mock.Anything)
}
1 change: 1 addition & 0 deletions internal/api/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6631,6 +6631,7 @@ components:
properties:
enabled_providers:
type: array
description: A write that carries this key must list at least one provider; an empty list or null returns 400. A GET on a fresh install can return an empty list.
items:
type: string
enum: [aws, azure, gcp]
Expand Down
Loading