Skip to content

Commit

Permalink
updates based on review comments
Browse files Browse the repository at this point in the history
Signed-off-by: everettraven <everettraven@gmail.com>
  • Loading branch information
everettraven committed Dec 12, 2024
1 parent c473b6b commit f5c20de
Showing 1 changed file with 43 additions and 12 deletions.
55 changes: 43 additions & 12 deletions enhancements/authentication/direct-external-oidc-provider.md
Original file line number Diff line number Diff line change
Expand Up @@ -220,15 +220,29 @@ subjects do not violate any restrictions in the namespace they are going to be c
These restrictions are specified by `RoleBindingRestriction` resources that are served
by the openshift-apiserver.

Removing the OAuth stack entirely during external OIDC setup may result in the `authorization.openshift.io/RestrictSubjectBindings` admission plugin, and thus the KAS appearing degraded.
Removing the OAuth stack entirely during external OIDC setup may result in the `authorization.openshift.io/RestrictSubjectBindings` admission plugin not functioning as expected.

Anticipated impacts are:

- Logs in the KAS reflecting that informers for OAuth APIs could not be started
- Logs in the KAS reflecting that the oauth-apiserver is unreachable
- Errors, that are not actionable from the perspective of users, occurring during evaluation of `RoleBinding` resources when a `RoleBindingRestriction` resource in the same namespace specifies user/group restrictions

These impacts may cause concern for cluster administrators that their cluster is in an unhealthy state and may not be easily correlated to the enablement of the OIDC functionality.

To ensure a holistic change in cluster behavior when enabling the OIDC functionality, and reduce potentially concerning behavior in the control plane components, it is proposed that changes are made to:

- The `authorization.openshift.io/RestrictSubjectBindings` admission plugin to account for necessary behavior changes
- The openshift-apiserver to reject `RoleBindingRestriction` resources that specify user/group restrictions when OIDC functionality is enabled
- The `Authentication` api to communicate when OIDC can't be enabled due to existing `RoleBindingRestriction` resources that specify user/group restrictions

##### Changes to the kube-apiserver

To account for the impacts to the `authorization.openshift.io/RestrictSubjectBindings` admission plugin, the OpenShift-specific patch to the kube-apiserver that adds this admission plugin will be updated such that:

- Informers for the `Group` API are not started if the `Authentication` resource `.Spec.Type` is set to `OIDC`
- The post-start hook that checks for oauth-apiserver connectivity will be skipped if the `Authentication` resource `.Spec.Type` is set to `OIDC`
- `RoleBinding`s will be rejected if there exists a `RoleBindingRestriction` that specifies user and/or group restrictions
- `RoleBinding`s will be rejected if there exists a `RoleBindingRestriction` that specifies user and/or group restrictions in the namespace the `RoleBinding` is being created
- It is considered a failure if we are unable to determine the authentication type for the cluster, leading to rejection of the `RoleBinding`

##### Changes to openshift-apiserver
Expand All @@ -239,30 +253,47 @@ To help keep users informed of the expected behavior of the `authorization.opens

- Do not reject admission, but issue a warning of the impacts creating a `RoleBindingRestriction` may have when using OIDC as the cluster authentication method.
- Use a `ValidatingAdmissionPolicy` + `ValidatingAdmissionPolicyBinding` instead of an admission plugin.
- Don't do anything special, let changes in admission plugin and documentation be the source of this information.
- Don't do anything special, let changes in the `authorization.openshift.io/RestrictSubjectBindings` admission plugin and documentation be the source of this information.

##### Changes to the `Authentication` API

Continuing with the notion of keeping users informed of impacts, the `Authentication` resource that users must update to enable the OIDC authentication mode on the cluster will be extended with a new status field to inform users of any potential impacts. In the event there are existing `RoleBindingRestriction` resources on the cluster that specify user/group restrictions, this new status field will be populated with a message stating the potential impact.
The OIDC authentication mode on the cluster will not be enabled if there exists `RoleBindingRestriction` resources that specify user/group restrictions.

To communicate the reason for the enablement of the OIDC functionality being blocked, the `Authentication` API will be extended with a new status field to communicate the condition of the OIDC feature.

```go
type AuthenticationStatus struct {
...

// OIDCWarnings are warnings related to potential cluster impacts
// when enabling the OIDC authentication type. These warnings
// are only evaluated when the OIDC authentication type has
// been enabled.
// OIDCConditions are conditions related to the
// enablement of the OIDC authentication feature.
//
// The Enabled condition represents whether or not the OIDC
// feature has been successfully enabled.
//
// When Enabled is False and the reason is RoleBindingRestrictionViolations,
// the rollout of the OIDC feature could not progress due to the existence
// of RoleBindingRestrictions that specify user and/or group subject restrictions.
// Enabling the OIDC functionality removes the User and Group APIs from the cluster
// and as such, RoleBindingRestrictions should not specify user or group restrictions
// when using an OIDC provider for authentication.
//
// +listType=atomic
// +openshif:enable:FeatureGate=ExternalOIDC
OIDCWarnings []string `json:"oidcWarnings"`
// When Enabled is True and the reason is Succeeded, the rollout of the OIDC feature
// has been successful and your cluster is officially using the OIDC provider
// for authentication.
//
// +patchMergeKey=type
// +patchStrategy=merge
// +listType=map
// +listMapKey=type
// +openshift:enable:FeatureGate=ExternalOIDC
OIDCConditions []metav1.Condition `json:"oidcConditions"`
}
```

**Alternatives**

- Don't allow enablement of the OIDC authentication type if there are `RoleBindingRestriction` resources on the cluster that specify user/group restrictions. Would likely still need a new field to communicate problems to users (`OIDCWarnings` might not make sense in this case).
- Allow enablement of the OIDC functionality when there exists `RoleBindingRestriction` resources that specify user/group restrictions. Ideally, warn about the violating `RoleBindingRestriction`s upon enablement of the OIDC functionality.
- Don't do anything special, let the changes in other components + documentation be the source of information.

#### Authentication disruptions
Expand Down

0 comments on commit f5c20de

Please sign in to comment.