Skip to content

Commit ec76591

Browse files
author
Enda Phelan
committed
fix(serviceaccount): add input validation
1 parent 6f2f44c commit ec76591

File tree

7 files changed

+262
-13
lines changed

7 files changed

+262
-13
lines changed

‎cmd/rhoas/pkged.go‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ require (
2222
github.com/nicksnyder/go-i18n/v2 v2.1.2
2323
github.com/openconfig/goyang v0.2.4
2424
github.com/phayes/freeport v0.0.0-20180830031419-95f893ade6f2
25-
github.com/pkg/errors v0.9.1 // indirect
25+
github.com/pkg/errors v0.9.1
2626
github.com/pquerna/cachecontrol v0.0.0-20200921180117-858c6e7e6b7e // indirect
2727
github.com/spf13/cobra v1.1.1
2828
github.com/spf13/pflag v1.0.5

‎locales/cmd/kafka/topic/common/active.en.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ one = 'topic name is required'
4141
one = 'topic name cannot exceed {{.MaxNameLen}} characters'
4242

4343
[kafka.topic.common.validation.name.error.invalidChars]
44-
one = 'invalid topic name "{{.Name}}", only letters (Aa-Zz), numbers, "_" and "-" are accepted'
44+
one = 'invalid topic name "{{.Name}}"; only letters (Aa-Zz), numbers, "_" and "-" are accepted'
4545

4646
[kafka.topic.common.validation.partitions.error.invalid]
4747
one = 'invalid partition count {{.Partitions}}, minimum partition count is {{.MinPartitions}}'

‎locales/cmd/serviceaccount/active.en.toml‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,3 +70,18 @@ one = 'Sets a custom file location to save the credentials.'
7070
[serviceAccount.common.log.debug.interactive.fileFormatNotSet]
7171
description = 'debug message'
7272
one = '--file-format flag is not set, prompting user to enter a value'
73+
74+
[serviceAccount.common.validation.name.error.required]
75+
one = 'service account name is required'
76+
77+
[serviceAccount.common.validation.name.error.lengthError]
78+
one = 'service account name cannot exceed {{.MaxNameLen}} characters'
79+
80+
[serviceAccount.common.validation.name.error.invalidChars]
81+
one = 'invalid service account name "{{.Name}}"; only lowercase letters (a-z), numbers, and "-" are accepted'
82+
83+
[serviceAccount.common.validation.description.error.invalidChars]
84+
one = 'invalid service account description "{{.Description}}"; only lowercase letters (a-z) and numbers are accepted'
85+
86+
[serviceAccount.common.validation.description.error.lengthError]
87+
one = 'service account description cannot exceed {{.MaxNameLen}} characters'

‎pkg/cmd/serviceaccount/create/create.go‎

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import (
55
"fmt"
66
"os"
77

8+
"github.com/bf2fc6cc711aee1a0c2a/cli/pkg/serviceaccount/validation"
9+
810
kasclient "github.com/bf2fc6cc711aee1a0c2a/cli/pkg/api/kas/client"
911
"github.com/bf2fc6cc711aee1a0c2a/cli/pkg/connection"
1012

@@ -50,7 +52,8 @@ func NewCreateCommand(f *factory.Factory) *cobra.Command {
5052
Short: localizer.MustLocalizeFromID("serviceAccount.create.cmd.shortDescription"),
5153
Long: localizer.MustLocalizeFromID("serviceAccount.create.cmd.longDescription"),
5254
Example: localizer.MustLocalizeFromID("serviceAccount.create.cmd.example"),
53-
RunE: func(cmd *cobra.Command, _ []string) error {
55+
Args: cobra.NoArgs,
56+
RunE: func(cmd *cobra.Command, _ []string) (err error) {
5457
if !opts.IO.CanPrompt() && opts.name == "" {
5558
return fmt.Errorf(localizer.MustLocalize(&localizer.Config{
5659
MessageID: "flag.error.requiredWhenNonInteractive",
@@ -62,13 +65,22 @@ func NewCreateCommand(f *factory.Factory) *cobra.Command {
6265
opts.interactive = true
6366
}
6467

65-
if !opts.interactive && opts.fileFormat == "" {
66-
return fmt.Errorf(localizer.MustLocalize(&localizer.Config{
67-
MessageID: "flag.error.requiredFlag",
68-
TemplateData: map[string]interface{}{
69-
"Flag": "file-format",
70-
},
71-
}))
68+
if !opts.interactive {
69+
if opts.fileFormat == "" {
70+
return fmt.Errorf(localizer.MustLocalize(&localizer.Config{
71+
MessageID: "flag.error.requiredFlag",
72+
TemplateData: map[string]interface{}{
73+
"Flag": "file-format",
74+
},
75+
}))
76+
}
77+
78+
if err = validation.ValidateName(opts.name); err != nil {
79+
return err
80+
}
81+
if err = validation.ValidateDescription(opts.description); err != nil {
82+
return err
83+
}
7284
}
7385

7486
// check that a valid --file-format flag value is used
@@ -199,7 +211,7 @@ func runInteractivePrompt(opts *Options) (err error) {
199211
Help: localizer.MustLocalizeFromID("serviceAccount.create.input.name.help"),
200212
}
201213

202-
err = survey.AskOne(promptName, &opts.name, survey.WithValidator(survey.Required))
214+
err = survey.AskOne(promptName, &opts.name, survey.WithValidator(survey.Required), survey.WithValidator(validation.ValidateName))
203215
if err != nil {
204216
return err
205217
}
@@ -228,7 +240,7 @@ func runInteractivePrompt(opts *Options) (err error) {
228240

229241
promptDescription := &survey.Multiline{Message: localizer.MustLocalizeFromID("serviceAccount.create.input.description.message")}
230242

231-
err = survey.AskOne(promptDescription, &opts.description)
243+
err = survey.AskOne(promptDescription, &opts.description, survey.WithValidator(validation.ValidateDescription))
232244
if err != nil {
233245
return err
234246
}

‎pkg/serviceaccount/validation/validation.go‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
package validation
2+
3+
import (
4+
"errors"
5+
"regexp"
6+
7+
"github.com/bf2fc6cc711aee1a0c2a/cli/internal/localizer"
8+
)
9+
10+
const (
11+
// name validation rules
12+
legalNameChars = "^[a-z]([-a-z0-9]*[a-z0-9])?$"
13+
maxNameLength = 50
14+
minNameLength = 1
15+
// description validation rules
16+
legalDescriptionChars = "^[a-z0-9\\s]*$"
17+
maxDescriptionLength = 255
18+
)
19+
20+
// ValidateName validates the name of the service account
21+
func ValidateName(val interface{}) error {
22+
name, ok := val.(string)
23+
if !ok {
24+
return errors.New(localizer.MustLocalize(&localizer.Config{
25+
MessageID: "common.error.castError",
26+
TemplateData: map[string]interface{}{
27+
"Value": val,
28+
"Type": "string",
29+
},
30+
}))
31+
}
32+
33+
if len(name) < minNameLength {
34+
return errors.New(localizer.MustLocalizeFromID("serviceAccount.common.validation.name.error.required"))
35+
} else if len(name) > maxNameLength {
36+
return errors.New(localizer.MustLocalize(&localizer.Config{
37+
MessageID: "serviceAccount.common.validation.name.error.lengthError",
38+
TemplateData: map[string]interface{}{
39+
"MaxNameLen": maxNameLength,
40+
},
41+
}))
42+
}
43+
44+
matched, _ := regexp.Match(legalNameChars, []byte(name))
45+
46+
if matched {
47+
return nil
48+
}
49+
50+
return errors.New(localizer.MustLocalize(&localizer.Config{
51+
MessageID: "serviceAccount.common.validation.name.error.invalidChars",
52+
TemplateData: map[string]interface{}{
53+
"Name": name,
54+
},
55+
}))
56+
}
57+
58+
// ValidateDescription validates the service account description text
59+
func ValidateDescription(val interface{}) error {
60+
description, ok := val.(string)
61+
if !ok {
62+
return errors.New(localizer.MustLocalize(&localizer.Config{
63+
MessageID: "common.error.castError",
64+
TemplateData: map[string]interface{}{
65+
"Value": val,
66+
"Type": "string",
67+
},
68+
}))
69+
}
70+
71+
if description == "" {
72+
return nil
73+
}
74+
75+
if len(description) > maxDescriptionLength {
76+
return errors.New(localizer.MustLocalize(&localizer.Config{
77+
MessageID: "serviceAccount.common.validation.description.error.lengthError",
78+
TemplateData: map[string]interface{}{
79+
"MaxNameLen": maxDescriptionLength,
80+
},
81+
}))
82+
}
83+
84+
matched, _ := regexp.Match(legalDescriptionChars, []byte(description))
85+
86+
if matched {
87+
return nil
88+
}
89+
90+
return errors.New(localizer.MustLocalize(&localizer.Config{
91+
MessageID: "serviceAccount.common.validation.description.error.invalidChars",
92+
TemplateData: map[string]interface{}{
93+
"Description": description,
94+
},
95+
}))
96+
}

‎pkg/serviceaccount/validation/validation_test.go‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
package validation
2+
3+
import (
4+
"testing"
5+
6+
"github.com/bf2fc6cc711aee1a0c2a/cli/internal/localizer"
7+
)
8+
9+
func TestValidateName(t *testing.T) {
10+
_ = localizer.IncludeAssetsAndLoadMessageFiles()
11+
12+
type args struct {
13+
val interface{}
14+
}
15+
tests := []struct {
16+
name string
17+
args args
18+
wantErr bool
19+
}{
20+
{
21+
name: "fails when length is 0",
22+
args: args{""},
23+
wantErr: true,
24+
},
25+
{
26+
name: "passes when length is 1",
27+
args: args{"s"},
28+
wantErr: false,
29+
},
30+
{
31+
name: "passes when length is 50 (max length)",
32+
args: args{"ssssssssssssssssssssssssssssssssssssssssssssssssss"},
33+
wantErr: false,
34+
},
35+
{
36+
name: "fails when length exceeds max length",
37+
args: args{"sssssssssssssssssssssssssssssssssssssssssssssssssss"},
38+
wantErr: true,
39+
},
40+
{
41+
name: "passes on valid name",
42+
args: args{"svcacctone"},
43+
wantErr: false,
44+
},
45+
{
46+
name: "passes on valid name with hyphens",
47+
args: args{"svc-acct-one"},
48+
wantErr: false,
49+
},
50+
{
51+
name: "passes on valid name with digits",
52+
args: args{"svc-acct-1s"},
53+
wantErr: false,
54+
},
55+
{
56+
name: "fails with capital letters",
57+
args: args{"Svc-acct-one"},
58+
wantErr: true,
59+
},
60+
{
61+
name: "fails number in first section",
62+
args: args{"1svc-acct-one"},
63+
wantErr: true,
64+
},
65+
}
66+
// nolint:scopelint
67+
for _, tt := range tests {
68+
t.Run(tt.name, func(t *testing.T) {
69+
if err := ValidateName(tt.args.val); (err != nil) != tt.wantErr {
70+
t.Errorf("ValidateName() error = %v, wantErr %v", err, tt.wantErr)
71+
}
72+
})
73+
}
74+
}
75+
76+
func TestValidateDescription(t *testing.T) {
77+
_ = localizer.IncludeAssetsAndLoadMessageFiles()
78+
79+
type args struct {
80+
val interface{}
81+
}
82+
tests := []struct {
83+
name string
84+
args args
85+
wantErr bool
86+
}{
87+
{
88+
name: "passes when empty",
89+
args: args{""},
90+
wantErr: false,
91+
},
92+
{
93+
name: "passes on max length (255)",
94+
args: args{"trl1rmcyl6dp4xxqy0rwudhodbpjc4crja8ibf2yco6obalko6qor9n2a1wsqruolg0ewrndumw2xkezzuwg8pjo6ntsmi1cjw99hjcko4t2kjkxmaswzgk8ko75pcs4js0pzypuyjxxnld4dijxadzs8peioi6d5jjxxtfl9vicufmxuacvu7m8ycbwhsbiu9ipw5fxplf0ojs8bxd7hwt4rn4phbcdgivxdzprhyfjamkgjzytjz25cmqagtw"},
95+
wantErr: false,
96+
},
97+
{
98+
name: "fails when exceeds max length",
99+
args: args{"trl1rmcyl6dp4xxqy0rwudhodbpjc4crja8ibf2yco6obalko6qor9n2a1wsqruolg0ewrndumw2xkezzuwg8pjo6ntsmi1cjw99hjcko4t2kjkxmaswzgk8ko75pcs4js0pzypuyjxxnld4dijxadzs8peioi6d5jjxxtfl9vicufmxuacvu7m8ycbwhsbiu9ipw5fxplf0ojs8bxd7hwt4rn4phbcdgivxdzprhyfjamkgjzytjz25cmqagtwa"},
100+
wantErr: true,
101+
},
102+
{
103+
name: "passes with spaces",
104+
args: args{"here is a description"},
105+
wantErr: false,
106+
},
107+
{
108+
name: "fails with special character",
109+
args: args{"here is a description!"},
110+
wantErr: true,
111+
},
112+
{
113+
name: "fails with capital letters",
114+
args: args{"Hello"},
115+
wantErr: true,
116+
},
117+
}
118+
for _, tt := range tests {
119+
// nolint:scopelint
120+
t.Run(tt.name, func(t *testing.T) {
121+
if err := ValidateDescription(tt.args.val); (err != nil) != tt.wantErr {
122+
t.Errorf("ValidateDescription() error = %v, wantErr %v", err, tt.wantErr)
123+
}
124+
})
125+
}
126+
}

0 commit comments

Comments
 (0)