Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/OIDC - relaxed OIDC scope validation #3863

Merged
merged 12 commits into from
May 10, 2023
2 changes: 1 addition & 1 deletion docs/content/configuration/policy-resource.md
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ The OIDC policy defines a few internal locations that can't be customized: `/_jw
|``authExtraArgs`` | A list of extra URL arguments to pass to the authorization endpoint provided by your OpenID Connect provider. Arguments must be URL encoded, multiple arguments may be included in the list, for example ``[ arg1=value1, arg2=value2 ]`` | ``string[]`` | No |
|``tokenEndpoint`` | URL for the token endpoint provided by your OpenID Connect provider. | ``string`` | Yes |
|``jwksURI`` | URL for the JSON Web Key Set (JWK) document provided by your OpenID Connect provider. | ``string`` | Yes |
|``scope`` | List of OpenID Connect scopes. Possible values are ``openid``, ``profile``, ``email``, ``address`` and ``phone``. The scope ``openid`` always needs to be present and others can be added concatenating them with a ``+`` sign, for example ``openid+profile+email``. The default is ``openid``. | ``string`` | No |
|``scope`` | List of OpenID Connect scopes. The scope ``openid`` always needs to be present and others can be added concatenating them with a ``+`` sign, for example ``openid+profile+email``, ``openid+email+userDefinedScope``. The default is ``openid``. | ``string`` | No |
|``redirectURI`` | Allows overriding the default redirect URI. The default is ``/_codexch``. | ``string`` | No |
|``zoneSyncLeeway`` | Specifies the maximum timeout in milliseconds for synchronizing ID/access tokens and shared values between Ingress Controller pods. The default is ``200``. | ``int`` | No |
|``accessTokenEnable`` | Option of whether Bearer token is used to authorize NGINX to access protected backend. | ``boolean`` | No |
Expand Down
51 changes: 17 additions & 34 deletions pkg/apis/configuration/validation/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,11 @@ func ValidateEscapedString(body string, examples ...string) error {
}

func validateVariable(nVar string, validVars map[string]bool, fieldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

if !validVars[nVar] {
msg := fmt.Sprintf("'%v' contains an invalid NGINX variable. Accepted variables are: %v", nVar, mapToPrettyString(validVars))
allErrs = append(allErrs, field.Invalid(fieldPath, nVar, msg))
return field.ErrorList{field.Invalid(fieldPath, nVar, msg)}
}
return allErrs
return nil
}

// isValidSpecialHeaderLikeVariable validates special variables $http_, $jwt_header_, $jwt_claim_
Expand Down Expand Up @@ -103,12 +101,11 @@ func validateSpecialVariable(nVar string, fieldPath *field.Path, isPlus bool) fi
}

func validateStringWithVariables(str string, fieldPath *field.Path, specialVars []string, validVars map[string]bool, isPlus bool) field.ErrorList {
allErrs := field.ErrorList{}

if strings.HasSuffix(str, "$") {
return append(allErrs, field.Invalid(fieldPath, str, "must not end with $"))
return field.ErrorList{field.Invalid(fieldPath, str, "must not end with $")}
}

allErrs := field.ErrorList{}
for i, c := range str {
if c == '$' {
msg := "variables must be enclosed in curly braces, for example ${host}"
Expand Down Expand Up @@ -144,67 +141,55 @@ func validateStringWithVariables(str string, fieldPath *field.Path, specialVars
}

func validateTime(time string, fieldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

if time == "" {
return allErrs
return nil
}

if _, err := configs.ParseTime(time); err != nil {
return append(allErrs, field.Invalid(fieldPath, time, err.Error()))
return field.ErrorList{field.Invalid(fieldPath, time, err.Error())}
}

return allErrs
return nil
}

// http://nginx.org/en/docs/syntax.html
const offsetErrMsg = "must consist of numeric characters followed by a valid size suffix. 'k|K|m|M|g|G"

func validateOffset(offset string, fieldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

if offset == "" {
return allErrs
return nil
}

if _, err := configs.ParseOffset(offset); err != nil {
msg := validation.RegexError(offsetErrMsg, configs.OffsetFmt, "16", "32k", "64M", "2G")
return append(allErrs, field.Invalid(fieldPath, offset, msg))
return field.ErrorList{field.Invalid(fieldPath, offset, msg)}
}

return allErrs
return nil
}

// http://nginx.org/en/docs/syntax.html
const sizeErrMsg = "must consist of numeric characters followed by a valid size suffix. 'k|K|m|M"

func validateSize(size string, fieldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

if size == "" {
return allErrs
return nil
}

if _, err := configs.ParseSize(size); err != nil {
msg := validation.RegexError(sizeErrMsg, configs.SizeFmt, "16", "32k", "64M")
return append(allErrs, field.Invalid(fieldPath, size, msg))
return field.ErrorList{field.Invalid(fieldPath, size, msg)}
}
return allErrs
return nil
}

// validateSecretName checks if a secret name is valid.
// It performs the same validation as ValidateSecretName from k8s.io/kubernetes/pkg/apis/core/validation/validation.go.
func validateSecretName(name string, fieldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

if name == "" {
return allErrs
return nil
}

allErrs := field.ErrorList{}
for _, msg := range validation.IsDNS1123Subdomain(name) {
allErrs = append(allErrs, field.Invalid(fieldPath, name, msg))
}

return allErrs
}

Expand All @@ -220,11 +205,9 @@ func mapToPrettyString(m map[string]bool) string {

// ValidateParameter validates a parameter against a map of valid parameters for the directive
func ValidateParameter(nPar string, validParams map[string]bool, fieldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

if !validParams[nPar] {
msg := fmt.Sprintf("'%v' contains an invalid NGINX parameter. Accepted parameters are: %v", nPar, mapToPrettyString(validParams))
allErrs = append(allErrs, field.Invalid(fieldPath, nPar, msg))
return field.ErrorList{field.Invalid(fieldPath, nPar, msg)}
}
return allErrs
return nil
}
Loading