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.

Sign up for GitHub

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

Jump to bottom

THREESCALE-10523 - oidc in OpenAPI CR ignores some attributes for Product CR #909

Merged
merged 3 commits into from
Jan 17, 2024
Merged

THREESCALE-10523 - oidc in OpenAPI CR ignores some attributes for Product CR #909

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
78e1417
THREESCALE-10518- Operator crash - OpenAPI CR doesn't create correct …
valerymo Dec 20, 2023
aec599b
THREESCALE-10523 oidc in OpenAPI CR ignores some attributes for Produ…
valerymo Jan 3, 2024
0e27914
THREESCALE-10523 - address review comments
valerymo Jan 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 12 additions & 3 deletions controllers/capabilities/openapi_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -398,11 +398,14 @@ func (r *OpenAPIReconciler) validateOIDCSettingsInCR(openapiCR *capabilitiesv1be
logger := r.Logger().WithValues("openapi", openapiCR.Name)
fieldErrors := field.ErrorList{}
specFldPath := field.NewPath("spec")
openapiRefFldPath := specFldPath.Child("openapiRef")
openapiOidcFldPath := specFldPath.Child("oidc")
openapiIssuerEndpointFldPath := openapiOidcFldPath.Child("IssuerEndpoint")
openapiIssuerEndpointRefFldPath := openapiOidcFldPath.Child("IssuerEndpointRef")

if openapiCR.Spec.OIDC != nil &&
(openapiCR.Spec.OIDC.IssuerEndpoint == "" && openapiCR.Spec.OIDC.IssuerEndpointRef == nil) {
fieldErrors = append(fieldErrors, field.Invalid(openapiRefFldPath, openapiCR.Spec.OpenAPIRef, "OIDC issuer endpoint definition is missing in CR - "+
fieldErrors = append(fieldErrors, field.Invalid(openapiIssuerEndpointFldPath, openapiCR.Spec.OIDC.IssuerEndpoint, "OIDC IssuerEndpoint definition is missing in CR."))
fieldErrors = append(fieldErrors, field.Invalid(openapiIssuerEndpointRefFldPath, openapiCR.Spec.OIDC.IssuerEndpointRef, "OIDC IssuerEndpointRef definition is missing in CR. "+
"No IssuerEndpoint nor IssuerEndpointRef found in OIDC spec in CR, one of them must be set."))
return &helper.SpecFieldError{
ErrorType: helper.InvalidError,
Expand All @@ -420,14 +423,20 @@ func (r *OpenAPIReconciler) validateOIDCSettingsInCR(openapiCR *capabilitiesv1be
// when the referenced OpenAPI spec's sec scheme is openIdConnect or oauth2, the spec.oidc must not be nil or empty
if globalSecRequirements[0].Value.Type == "openIdConnect" || globalSecRequirements[0].Value.Type == "oauth2" {
if openapiCR.Spec.OIDC == nil {
fieldErrors = append(fieldErrors, field.Invalid(openapiRefFldPath, openapiCR.Spec.OpenAPIRef, "Missing "+
fieldErrors = append(fieldErrors, field.Invalid(openapiOidcFldPath, openapiCR.Spec.OIDC, "Missing "+
"OIDC definitions in CR. The referenced OpenAPI spec's sec scheme is openIdConnect or oauth2, the spec.oidc must not be nil or empty"))
return &helper.SpecFieldError{
ErrorType: helper.InvalidError,
FieldErrorList: fieldErrors,
}
}
}
// when OAS securitySchemes type is oauth2, and openapiCR spec is OIDC, then CR OIDC Authentication Flows parameters will be ignored,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line 423 has ref to openapiref which causes constant updates to status of the CR which in turn re-reconciles on invalid error. Please double check the refs in invalidErrors.
Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated. Thanks very much for this comment and discussion. Hope it's ok now. Images for 2 cases below:

  • No IssuerEndpoint nor IssuerEndpointRef found in OIDC spec in CR
    Screenshot from 2024-01-17 12-17-25

  • Missing OIDC definitions in CR
    Screenshot from 2024-01-17 12-22-23

// and Product authentication flows will be set to match oauth2 flows in OAS
if openapiCR.Spec.OIDC != nil && globalSecRequirements[0].Value.Type == "oauth2" {
logger.Info("OIDC authentication flows in CR will be ignored and Product OIDC authentication flows will be set to match oauth2 flows in OAS since the SecuritySchemes type in OAS is \"oauth2\" (for OIDC it should be \"openIdConnect\")")
r.EventRecorder().Eventf(openapiCR, corev1.EventTypeWarning, "OIDC authentication flows in CR will be ignored and Product OIDC authentication flows will be set to match oauth2 flows in OAS since the SecuritySchemes type in OAS is \"oauth2\" (for OIDC it should be \"openIdConnect\")", "%v", "Product OIDC authentication flows parameters will be set to match oauth2 flows as following (OIDC ~ OAuth2): StandardFlowEnabled ~ AuthorizationCode, ImplicitFlowEnabled ~ Implicit, DirectAccessGrantsEnabled ~ Password, ServiceAccountsEnabled ~ ClientCredentials")
}
}

return nil
Expand Down
10 changes: 9 additions & 1 deletion controllers/capabilities/proxy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package controllers

import (
"fmt"
"k8s.io/apimachinery/pkg/util/validation/field"
"strconv"

"github.com/3scale/3scale-operator/pkg/helper"
Expand Down Expand Up @@ -196,13 +197,20 @@ func (t *ProductThreescaleReconciler) syncProxyOIDC(params threescaleapi.Params,
if oidcSpec == nil {
return nil
}
fieldErrors := field.ErrorList{}
specFldPath := field.NewPath("spec")
oidcRefFldPath := specFldPath.Child("oidc")

// If plain value is not nil - use plain value as precedence over secret
issuerEndpoint := oidcSpec.IssuerEndpoint
if issuerEndpoint == "" {
if oidcSpec.IssuerEndpointRef == nil {
// If missing both IssuerEndpoint and IssuerEndpointRef in OpenApi CR - Product will fail SyncProxy
return fmt.Errorf("missing IssuerEndpoint definition in OIDC spec in openapi CR. Product OpenID Connect Issuer will not be set.")
fieldErrors = append(fieldErrors, field.Invalid(oidcRefFldPath.Child("IssuerEndpoint"), t.resource.Spec.OIDCSpec().IssuerEndpoint, "no IssuerEndpoint nor IssuerEndpointRef found in OIDC spec in CR. Product OpenID Connect Issuer will not be set"))
return &helper.SpecFieldError{
ErrorType: helper.InvalidError,
FieldErrorList: fieldErrors,
}
}
secretSource := helper.NewSecretSource(t.Client(), t.resource.Namespace)
val, err := secretSource.RequiredFieldValueFromRequiredSecret(oidcSpec.IssuerEndpointRef.Name, "issuerEndpoint")
Expand Down
38 changes: 36 additions & 2 deletions doc/openapi-user-guide.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,6 @@ spec:
- The format of issuerEndpoint is determined on your OpenID Provider setup;
see in 3scale portal - `Product/Integration/Settings/AUTHENTICATION SETTINGS/OpenID Connect Issuer`.



OpenAPI CR example where issuerEndpoint defined both as plain value and in secret (plain value will be used):
```yaml
apiVersion: capabilities.3scale.net/v1beta1
Expand All @@ -190,6 +188,42 @@ spec:
directAccessGrantsEnabled: true
```

- **If OpenAPI CR spec is OIDC but securitySchemes type in OAS is oauth2** then CR OIDC Authentication Flows parameters will be ignored,
and Product OIDC Authentication Flows will be set to match oauth2 flows that defined in OAS, as following
- StandardFlowEnabled = true if oauth2 AuthorizationCode is defined
- ImplicitFlowEnabled = true if oauth2 Implicit is defined
- DirectAccessGrantsEnabled = true if oauth2 Password is defined
- ServiceAccountsEnabled = true if oauth2 ClientCredentials is defined

An example of **OAS securitySchemes** definition that allows selection of all Product OIDC Authentication Flows (OIDC should be defined in OpenAPI CR)
```yaml
securitySchemes:
myOauth:
description: This API uses OAuth 2 with the implicit grant flow. [More info](https://api.example.com/docs/auth)
flows:
password:
scopes:
read_pets: read your pets
write_pets: modify pets in your account
tokenUrl: https://api.example.com/oauth2/token
implicit:
authorizationUrl: https://example.com/api/oauth/dialog
scopes:
write_pets: modify pets in your account
read_pets: read your pets
authorizationCode:
authorizationUrl: https://example.com/api/oauth/dialog
tokenUrl: https://example.com/api/oauth/token
scopes:
write_pets: modify pets in your account
read_pets: read your pets
clientCredentials:
tokenUrl: https://example.com/api/oauth/token
scopes:
write_pets: modify pets in your account
read_pets: read your pets
type: oauth2
```

## Supported OpenAPI spec version and limitations

Expand Down