Skip to content

Commit 2bd3262

Browse files
JGAntunesJGAntunes
authored
feat(api): don't show hidden app config items to the client (#2593)
1 parent 3fac959 commit 2bd3262

File tree

6 files changed

+637
-24
lines changed

6 files changed

+637
-24
lines changed

‎api/controllers/app/install/config.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ func (c *InstallController) PatchAppConfigValues(ctx context.Context, values typ
5757
}
5858

5959
func (c *InstallController) TemplateAppConfig(ctx context.Context, values types.AppConfigValues, maskPasswords bool) (types.AppConfig, error) {
60-
return c.appConfigManager.TemplateConfig(values, maskPasswords)
60+
return c.appConfigManager.TemplateConfig(values, maskPasswords, true)
6161
}
6262

6363
func (c *InstallController) GetAppConfigValues(ctx context.Context) (types.AppConfigValues, error) {

‎api/integration/app/install/config_test.go‎

Lines changed: 271 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,151 @@ func (s *AppInstallTestSuite) TestPatchAppConfigValues() {
350350
assert.Equal(t, apiError.Errors[1].Field, "required-password")
351351
assert.Equal(t, apiError.Errors[1].Message, "Required Password is required")
352352
})
353+
354+
// Test required hidden item should produce error
355+
s.T().Run("Required hidden item should produce error", func(t *testing.T) {
356+
// Create app config with required hidden item
357+
configWithHiddenRequired := kotsv1beta1.Config{
358+
Spec: kotsv1beta1.ConfigSpec{
359+
Groups: []kotsv1beta1.ConfigGroup{
360+
{
361+
Name: "security-group",
362+
Title: "Security Settings",
363+
Items: []kotsv1beta1.ConfigItem{
364+
{
365+
Name: "api-key",
366+
Type: "password",
367+
Title: "API Key",
368+
Required: true,
369+
Hidden: true, // This item is hidden but required
370+
},
371+
{
372+
Name: "visible-setting",
373+
Type: "text",
374+
Title: "Visible Setting",
375+
Required: false,
376+
Hidden: false,
377+
},
378+
},
379+
},
380+
},
381+
},
382+
}
383+
384+
// Create an install controller with the app config
385+
apiInstance := s.createAPI(t, states.StateNew, &release.ReleaseData{
386+
AppConfig: &configWithHiddenRequired,
387+
}, nil)
388+
389+
// Create a router and register the API routes
390+
router := mux.NewRouter()
391+
apiInstance.RegisterRoutes(router)
392+
393+
// Create a request to patch config values without the required hidden item
394+
patchRequest := types.PatchAppConfigValuesRequest{
395+
Values: types.AppConfigValues{
396+
"visible-setting": types.AppConfigValue{Value: "some-value"},
397+
},
398+
}
399+
400+
reqBodyBytes, err := json.Marshal(patchRequest)
401+
require.NoError(t, err)
402+
403+
// Create a request
404+
req := httptest.NewRequest(http.MethodPatch, s.baseURL+"/app/config/values", bytes.NewReader(reqBodyBytes))
405+
req.Header.Set("Content-Type", "application/json")
406+
req.Header.Set("Authorization", "Bearer "+"TOKEN")
407+
rec := httptest.NewRecorder()
408+
409+
// Serve the request
410+
router.ServeHTTP(rec, req)
411+
412+
// Check the response - should return 400 Bad Request due to missing required hidden item
413+
assert.Equal(t, http.StatusBadRequest, rec.Code)
414+
415+
// Parse the response body
416+
var apiError types.APIError
417+
err = json.NewDecoder(rec.Body).Decode(&apiError)
418+
require.NoError(t, err)
419+
assert.Equal(t, http.StatusBadRequest, apiError.StatusCode)
420+
421+
// Verify the error mentions the required hidden field
422+
require.Len(t, apiError.Errors, 1)
423+
assert.Equal(t, apiError.Errors[0].Field, "api-key")
424+
assert.Equal(t, apiError.Errors[0].Message, "API Key is required")
425+
})
426+
427+
// Test required hidden item with predefined value should not produce error
428+
s.T().Run("Required hidden item with value should not produce error", func(t *testing.T) {
429+
// Create app config with required hidden item wtih a value
430+
configWithHiddenRequired := kotsv1beta1.Config{
431+
Spec: kotsv1beta1.ConfigSpec{
432+
Groups: []kotsv1beta1.ConfigGroup{
433+
{
434+
Name: "security-group",
435+
Title: "Security Settings",
436+
Items: []kotsv1beta1.ConfigItem{
437+
{
438+
Name: "api-key",
439+
Type: "password",
440+
Title: "API Key",
441+
Value: multitype.FromString("hidden-key-value"),
442+
Required: true,
443+
Hidden: true, // This item is hidden but required
444+
},
445+
{
446+
Name: "visible-setting",
447+
Type: "text",
448+
Title: "Visible Setting",
449+
Required: false,
450+
Hidden: false,
451+
},
452+
},
453+
},
454+
},
455+
},
456+
}
457+
458+
// Create an install controller with the app config
459+
apiInstance := s.createAPI(t, states.StateNew, &release.ReleaseData{
460+
AppConfig: &configWithHiddenRequired,
461+
}, nil)
462+
463+
// Create a router and register the API routes
464+
router := mux.NewRouter()
465+
apiInstance.RegisterRoutes(router)
466+
467+
// Create a request to patch config values without the required hidden item
468+
patchRequest := types.PatchAppConfigValuesRequest{
469+
Values: types.AppConfigValues{
470+
"visible-setting": types.AppConfigValue{Value: "some-value"},
471+
},
472+
}
473+
474+
reqBodyBytes, err := json.Marshal(patchRequest)
475+
require.NoError(t, err)
476+
477+
// Create a request
478+
req := httptest.NewRequest(http.MethodPatch, s.baseURL+"/app/config/values", bytes.NewReader(reqBodyBytes))
479+
req.Header.Set("Content-Type", "application/json")
480+
req.Header.Set("Authorization", "Bearer "+"TOKEN")
481+
rec := httptest.NewRecorder()
482+
483+
// Serve the request
484+
router.ServeHTTP(rec, req)
485+
486+
// Check the response
487+
assert.Equal(t, http.StatusOK, rec.Code)
488+
489+
// Parse the response body
490+
var response types.AppConfigValuesResponse
491+
err = json.NewDecoder(rec.Body).Decode(&response)
492+
require.NoError(t, err)
493+
require.NotNil(t, response.Values, "response values should not be nil")
494+
495+
// Verify the app config values are returned from the store
496+
assert.Equal(t, "some-value", response.Values["visible-setting"].Value, "visible-setting should be updated")
497+
})
353498
}
354499

355500
func (s *AppInstallTestSuite) TestTemplateAppConfig() {
@@ -678,6 +823,132 @@ func (s *AppInstallTestSuite) TestTemplateAppConfig() {
678823
require.NoError(t, err)
679824
assert.Equal(t, http.StatusBadRequest, apiError.StatusCode)
680825
})
826+
827+
// Test hidden items should not be returned
828+
s.T().Run("Hidden items should not be returned", func(t *testing.T) {
829+
// Create app config with hidden items
830+
configWithHidden := kotsv1beta1.Config{
831+
TypeMeta: metav1.TypeMeta{
832+
APIVersion: "kots.io/v1beta1",
833+
Kind: "Config",
834+
},
835+
ObjectMeta: metav1.ObjectMeta{
836+
Name: "config-with-hidden",
837+
},
838+
Spec: kotsv1beta1.ConfigSpec{
839+
Groups: []kotsv1beta1.ConfigGroup{
840+
{
841+
Name: "mixed-visibility",
842+
Title: "Mixed Visibility Group",
843+
Items: []kotsv1beta1.ConfigItem{
844+
{
845+
Name: "visible-item",
846+
Type: "text",
847+
Title: "Visible Item",
848+
Default: multitype.FromString("visible-default"),
849+
Value: multitype.FromString("visible-value"),
850+
Hidden: false,
851+
},
852+
{
853+
Name: "hidden-item",
854+
Type: "text",
855+
Title: "Hidden Item",
856+
Default: multitype.FromString("hidden-default"),
857+
Value: multitype.FromString("hidden-value"),
858+
Hidden: true, // This item should not appear in the response
859+
},
860+
{
861+
Name: "hidden-password",
862+
Type: "password",
863+
Title: "Hidden Password",
864+
Default: multitype.FromString("hidden-password-default"),
865+
Value: multitype.FromString("hidden-password-value"),
866+
Hidden: true, // This item should not appear in the response
867+
},
868+
{
869+
Name: "another-visible-item",
870+
Type: "text",
871+
Title: "Another Visible Item",
872+
Default: multitype.FromString("another-visible-default"),
873+
Value: multitype.FromString("another-visible-value"),
874+
Hidden: false,
875+
},
876+
},
877+
},
878+
},
879+
},
880+
}
881+
882+
// Create an install controller with the app config
883+
apiInstance := s.createAPI(t, states.StateNew, &release.ReleaseData{
884+
AppConfig: &configWithHidden,
885+
}, nil)
886+
887+
// Create a router and register the API routes
888+
router := mux.NewRouter()
889+
apiInstance.RegisterRoutes(router)
890+
891+
// Create template request
892+
templateRequest := types.TemplateAppConfigRequest{
893+
Values: types.AppConfigValues{
894+
"visible-item": {Value: "user-visible-value"},
895+
"hidden-item": {Value: "user-hidden-value"}, // Even if user provides this, it shouldn't appear
896+
"another-visible-item": {Value: "user-another-visible-value"},
897+
},
898+
}
899+
900+
reqBodyBytes, err := json.Marshal(templateRequest)
901+
require.NoError(t, err)
902+
903+
// Create a request
904+
req := httptest.NewRequest(http.MethodPost, s.baseURL+"/app/config/template", bytes.NewReader(reqBodyBytes))
905+
req.Header.Set("Content-Type", "application/json")
906+
req.Header.Set("Authorization", "Bearer TOKEN")
907+
rec := httptest.NewRecorder()
908+
909+
// Serve the request
910+
router.ServeHTTP(rec, req)
911+
912+
// Check the response
913+
require.Equal(t, http.StatusOK, rec.Code)
914+
assert.Equal(t, "application/json", rec.Header().Get("Content-Type"))
915+
916+
// Parse the response body
917+
var response types.AppConfig
918+
err = json.NewDecoder(rec.Body).Decode(&response)
919+
require.NoError(t, err)
920+
921+
// Verify hidden items are filtered out
922+
require.Len(t, response.Groups, 1, "group should be present")
923+
group := response.Groups[0]
924+
assert.Equal(t, "Mixed Visibility Group", group.Title)
925+
926+
// Should only have the visible items (hidden items should be filtered out)
927+
require.Len(t, group.Items, 2, "only visible items should be present")
928+
929+
// Check that only visible items are present
930+
visibleItemNames := make([]string, len(group.Items))
931+
for i, item := range group.Items {
932+
visibleItemNames[i] = item.Name
933+
}
934+
935+
assert.Contains(t, visibleItemNames, "visible-item", "visible-item should be present")
936+
assert.Contains(t, visibleItemNames, "another-visible-item", "another-visible-item should be present")
937+
assert.NotContains(t, visibleItemNames, "hidden-item", "hidden-item should not be present")
938+
assert.NotContains(t, visibleItemNames, "hidden-password", "hidden-password should not be present")
939+
940+
// Verify the visible items have correct values
941+
for _, item := range group.Items {
942+
switch item.Name {
943+
case "visible-item":
944+
assert.Equal(t, "user-visible-value", item.Value.String(), "visible-item should have user-provided value")
945+
assert.False(t, item.Hidden, "visible-item should not be hidden")
946+
case "another-visible-item":
947+
assert.Equal(t, "user-another-visible-value", item.Value.String(), "another-visible-item should have user-provided value")
948+
assert.False(t, item.Hidden, "another-visible-item should not be hidden")
949+
}
950+
}
951+
})
681952
}
682953

683954
// Runner function that executes the suite for both install types

‎api/internal/managers/app/config/config.go‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ var (
2525
)
2626

2727
// TemplateConfig templates the config with provided values and returns the templated config
28-
func (m *appConfigManager) TemplateConfig(newValues types.AppConfigValues, maskPasswords bool) (types.AppConfig, error) {
28+
func (m *appConfigManager) TemplateConfig(newValues types.AppConfigValues, maskPasswords bool, filterHiddenItems bool) (types.AppConfig, error) {
2929
// Get current config values from the store
3030
storedValues, err := m.appConfigStore.GetConfigValues()
3131
if err != nil {
@@ -51,7 +51,7 @@ func (m *appConfigManager) TemplateConfig(newValues types.AppConfigValues, maskP
5151
}
5252

5353
// Filter out disabled groups and items
54-
filteredConfig, err := filterAppConfig(processedConfig)
54+
filteredConfig, err := filterAppConfig(processedConfig, filterHiddenItems)
5555
if err != nil {
5656
return types.AppConfig{}, fmt.Errorf("filter app config: %w", err)
5757
}
@@ -81,7 +81,8 @@ func (m *appConfigManager) TemplateConfig(newValues types.AppConfigValues, maskP
8181
func (m *appConfigManager) ValidateConfigValues(configValues types.AppConfigValues) error {
8282
var ve *types.APIError
8383

84-
processedConfig, err := m.TemplateConfig(configValues, false)
84+
// Don't filter hidden items here since we want to validate hidden required items
85+
processedConfig, err := m.TemplateConfig(configValues, false, false)
8586
if err != nil {
8687
return fmt.Errorf("template config: %w", err)
8788
}
@@ -113,7 +114,7 @@ func (m *appConfigManager) PatchConfigValues(newValues types.AppConfigValues) er
113114
}
114115

115116
// Get processed config to determine enabled groups and items
116-
processedConfig, err := m.TemplateConfig(newValues, false)
117+
processedConfig, err := m.TemplateConfig(newValues, false, true)
117118
if err != nil {
118119
return fmt.Errorf("template config: %w", err)
119120
}
@@ -156,7 +157,7 @@ func (m *appConfigManager) GetConfigValues() (types.AppConfigValues, error) {
156157
}
157158

158159
func (m *appConfigManager) GetKotsadmConfigValues() (kotsv1beta1.ConfigValues, error) {
159-
processedConfig, err := m.TemplateConfig(nil, false)
160+
processedConfig, err := m.TemplateConfig(nil, false, true)
160161
if err != nil {
161162
return kotsv1beta1.ConfigValues{}, fmt.Errorf("template config: %w", err)
162163
}
@@ -245,8 +246,8 @@ func getConfigValueFromChildItem(item kotsv1beta1.ConfigItem, childItem kotsv1be
245246
return configValue
246247
}
247248

248-
// filterAppConfig filters out disabled groups and items based on their 'when' condition
249-
func filterAppConfig(config kotsv1beta1.Config) (kotsv1beta1.Config, error) {
249+
// filterAppConfig filters out disabled groups, items based on their 'when' condition and hidden items
250+
func filterAppConfig(config kotsv1beta1.Config, filterHiddenItems bool) (kotsv1beta1.Config, error) {
250251
// deepcopy the config to avoid mutating the original config
251252
var updatedConfig kotsv1beta1.Config
252253
if err := deepcopy.Copy(&updatedConfig, &config); err != nil {
@@ -264,6 +265,9 @@ func filterAppConfig(config kotsv1beta1.Config) (kotsv1beta1.Config, error) {
264265
if !isItemEnabled(item.When) {
265266
continue
266267
}
268+
if filterHiddenItems && item.Hidden {
269+
continue
270+
}
267271
filteredItems = append(filteredItems, item)
268272
}
269273
if len(filteredItems) > 0 {

0 commit comments

Comments
 (0)