Skip to content

Commit e9f3e5f

Browse files
enjstlaz
authored andcommitted
oauth api: tighten client validation
Additional checks in client object: - no secret in client's additionalSecrets is empty - if accessTokenMaxAgeSeconds is set, check that it's a 0+ integer - ensure that grantMethod is not empty - restrict the possible values for grantMethod to "auto" and "prompt" (removes "deny")
1 parent 053ca43 commit e9f3e5f

1 file changed

Lines changed: 30 additions & 9 deletions

File tree

pkg/oauth/apis/oauth/validation/validation.go

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,7 @@ func ValidateAccessToken(accessToken *oauthapi.OAuthAccessToken) field.ErrorList
8282
}
8383
// negative values are not allowed
8484
if accessToken.InactivityTimeoutSeconds < 0 {
85-
allErrs = append(allErrs, field.Invalid(field.NewPath("inactivityTimeoutSeconds"),
86-
accessToken.InactivityTimeoutSeconds, "cannot be a negative value"))
85+
allErrs = append(allErrs, field.Invalid(field.NewPath("inactivityTimeoutSeconds"), accessToken.InactivityTimeoutSeconds, "cannot be a negative value"))
8786
}
8887
if ok, msg := ValidateRedirectURI(accessToken.RedirectURI); !ok {
8988
allErrs = append(allErrs, field.Invalid(field.NewPath("redirectURI"), accessToken.RedirectURI, msg))
@@ -179,22 +178,44 @@ func ValidateClient(client *oauthapi.OAuthClient) field.ErrorList {
179178
}
180179
}
181180

181+
for i, secret := range client.AdditionalSecrets {
182+
if len(secret) == 0 {
183+
allErrs = append(allErrs, field.Invalid(field.NewPath("additionalSecrets").Index(i), "", "may not be empty"))
184+
}
185+
}
186+
182187
for i, restriction := range client.ScopeRestrictions {
183188
allErrs = append(allErrs, ValidateScopeRestriction(restriction, field.NewPath("scopeRestrictions").Index(i))...)
184189
}
185190

191+
if accessTokenMaxAgeSeconds := client.AccessTokenMaxAgeSeconds; accessTokenMaxAgeSeconds != nil {
192+
if *accessTokenMaxAgeSeconds < 0 {
193+
allErrs = append(allErrs, field.Invalid(field.NewPath("accessTokenMaxAgeSeconds"), *accessTokenMaxAgeSeconds, "value cannot be negative"))
194+
}
195+
}
196+
186197
if client.AccessTokenInactivityTimeoutSeconds != nil {
187198
timeout := *client.AccessTokenInactivityTimeoutSeconds
188199
if timeout > 0 && timeout < MinimumInactivityTimeoutSeconds {
189-
allErrs = append(allErrs, field.Invalid(
190-
field.NewPath("accessTokenInactivityTimeoutSeconds"),
191-
client.AccessTokenInactivityTimeoutSeconds,
192-
fmt.Sprintf("The minimum valid timeout value is %d seconds", MinimumInactivityTimeoutSeconds)))
200+
msg := fmt.Sprintf("The minimum valid timeout value is %d seconds", MinimumInactivityTimeoutSeconds)
201+
allErrs = append(allErrs, field.Invalid(field.NewPath("accessTokenInactivityTimeoutSeconds"), timeout, msg))
193202
}
194203
if timeout < 0 {
195-
allErrs = append(allErrs, field.Invalid(
196-
field.NewPath("accessTokenInactivityTimeoutSeconds"),
197-
client.AccessTokenInactivityTimeoutSeconds, "value cannot be negative"))
204+
allErrs = append(allErrs, field.Invalid(field.NewPath("accessTokenInactivityTimeoutSeconds"), timeout, "value cannot be negative"))
205+
}
206+
}
207+
208+
if len(client.GrantMethod) == 0 {
209+
allErrs = append(allErrs, field.Required(field.NewPath("grantMethod"),
210+
fmt.Sprintf("must be %s or %s", oauthapi.GrantHandlerAuto, oauthapi.GrantHandlerPrompt)))
211+
} else {
212+
switch client.GrantMethod {
213+
case oauthapi.GrantHandlerAuto, oauthapi.GrantHandlerPrompt:
214+
// valid grant methods
215+
default:
216+
allErrs = append(allErrs, field.NotSupported(field.NewPath("grantMethod"), string(client.GrantMethod),
217+
[]string{string(oauthapi.GrantHandlerAuto), string(oauthapi.GrantHandlerPrompt)},
218+
))
198219
}
199220
}
200221

0 commit comments

Comments
 (0)