Skip to content

Commit

Permalink
remove OIDC checks from restrictusers admission plugin since it will …
Browse files Browse the repository at this point in the history
…be disabled

Signed-off-by: Bryce Palmer <[email protected]>
  • Loading branch information
everettraven committed Jan 7, 2025
1 parent 61014ac commit 705dcdb
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 282 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,20 @@ package restrictusers
import (
"k8s.io/apiserver/pkg/admission"

configv1informer "github.com/openshift/client-go/config/informers/externalversions"
userinformer "github.com/openshift/client-go/user/informers/externalversions"
)

func NewInitializer(userInformer userinformer.SharedInformerFactory, configInformer configv1informer.SharedInformerFactory) admission.PluginInitializer {
return &localInitializer{userInformer: userInformer, configInformer: configInformer}
func NewInitializer(userInformer userinformer.SharedInformerFactory) admission.PluginInitializer {
return &localInitializer{userInformer: userInformer}
}

type WantsUserInformer interface {
SetUserInformer(userinformer.SharedInformerFactory)
admission.InitializationValidator
}

type WantsConfigInformer interface {
SetConfigInformer(configv1informer.SharedInformerFactory)
}

type localInitializer struct {
userInformer userinformer.SharedInformerFactory
configInformer configv1informer.SharedInformerFactory
}

// Initialize will check the initialization interfaces implemented by each plugin
Expand All @@ -31,8 +25,4 @@ func (i *localInitializer) Initialize(plugin admission.Interface) {
if wants, ok := plugin.(WantsUserInformer); ok {
wants.SetUserInformer(i.userInformer)
}

if wants, ok := plugin.(WantsConfigInformer); ok {
wants.SetConfigInformer(i.configInformer)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,10 @@ import (
"errors"
"fmt"
"io"
"time"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
kerrors "k8s.io/apimachinery/pkg/util/errors"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/apiserver/pkg/admission"
"k8s.io/apiserver/pkg/admission/initializer"
"k8s.io/client-go/kubernetes"
Expand All @@ -19,14 +17,11 @@ import (
"k8s.io/klog/v2"
"k8s.io/kubernetes/pkg/apis/rbac"

configv1 "github.com/openshift/api/config/v1"
userv1 "github.com/openshift/api/user/v1"
authorizationtypedclient "github.com/openshift/client-go/authorization/clientset/versioned/typed/authorization/v1"
configv1informer "github.com/openshift/client-go/config/informers/externalversions"
userclient "github.com/openshift/client-go/user/clientset/versioned"
userinformer "github.com/openshift/client-go/user/informers/externalversions"
"github.com/openshift/library-go/pkg/apiserver/admission/admissionrestconfig"
"k8s.io/kubernetes/openshift-kube-apiserver/admission/authorization/restrictusers/authncache"
"k8s.io/kubernetes/openshift-kube-apiserver/admission/authorization/restrictusers/usercache"
)

Expand All @@ -42,11 +37,6 @@ type GroupCache interface {
HasSynced() bool
}

type AuthnCache interface {
Authn() (*configv1.Authentication, error)
HasSynced() bool
}

// restrictUsersAdmission implements admission.ValidateInterface and enforces
// restrictions on adding rolebindings in a project to permit only designated
// subjects.
Expand All @@ -58,7 +48,6 @@ type restrictUsersAdmission struct {
kubeClient kubernetes.Interface
groupCacheFunc func() (GroupCache, error)
groupCache GroupCache
authnCache AuthnCache
}

var (
Expand Down Expand Up @@ -101,29 +90,11 @@ func (q *restrictUsersAdmission) SetRESTClientConfig(restClientConfig rest.Confi
}
}

func (q *restrictUsersAdmission) isAuthTypeOIDC() (bool, error) {
err := wait.PollImmediate(1*time.Second, 10*time.Second, func() (bool, error) {
return q.authnCache.HasSynced(), nil
})
if err != nil {
return false, errors.New("authentications.config.openshift.io cache is not synchronized")
}

auth, err := q.authnCache.Authn()
if err == nil && auth != nil {
return auth.Spec.Type == configv1.AuthenticationTypeOIDC, nil
}
return false, err
}

func (q *restrictUsersAdmission) SetConfigInformer(configInformer configv1informer.SharedInformerFactory) {
q.authnCache = authncache.NewAuthnCache(configInformer.Config().V1().Authentications())
}

func (q *restrictUsersAdmission) SetUserInformer(userInformers userinformer.SharedInformerFactory) {
// defer the allocation of the group cache until later in the process so we can
// ensure we aren't creating informers for the Group resources if the authentication
// type is OIDC.
// ensure we aren't creating informers for the Group resources until this admission
// plugin actually runs. If authentication type is OIDC, this plugin should be disabled
// resulting in the Group informer never being configured and started.
q.groupCacheFunc = func() (GroupCache, error) {
if err := userInformers.User().V1().Groups().Informer().AddIndexers(cache.Indexers{
usercache.ByUserIndexName: usercache.ByUserIndexKeys,
Expand Down Expand Up @@ -221,35 +192,16 @@ func (q *restrictUsersAdmission) Validate(ctx context.Context, a admission.Attri
return nil
}

isAuthOIDC, err := q.isAuthTypeOIDC()
if err != nil {
return admission.NewForbidden(a, fmt.Errorf("could not determine if authentication type is OIDC: %v", err))
}

checkers := []SubjectChecker{}
for _, rbr := range roleBindingRestrictionList.Items {
// if the auth type is OIDC, the oauth-apiserver is down and as such
// we cannot properly evaluate the user and/or group subjects. Fail fast
// if the RBR has user and/or group restrictions applied if auth type is OIDC
if isAuthOIDC {
if rbr.Spec.UserRestriction != nil {
return admission.NewForbidden(a, errors.New("authentication type is OIDC and rolebinding restriction specifies user restrictions. Unable to get user information due to OIDC configuration, rejecting"))
}

if rbr.Spec.GroupRestriction != nil {
return admission.NewForbidden(a, errors.New("authentication type is OIDC and rolebinding restriction specifies group restrictions. Unable to get group information due to OIDC configuration, rejecting"))
}
}
checker, err := NewSubjectChecker(&rbr.Spec)
if err != nil {
return admission.NewForbidden(a, fmt.Errorf("could not create rolebinding restriction subject checker: %v", err))
}
checkers = append(checkers, checker)
}

// If auth type is OIDC, we should never create checkers for the user/group restrictions
// so it should be ok to provide a nil group cache
if !isAuthOIDC && q.groupCache == nil && q.groupCacheFunc != nil {
if q.groupCache == nil && q.groupCacheFunc != nil {
q.groupCache, err = q.groupCacheFunc()
if err != nil {
return admission.NewForbidden(a, fmt.Errorf("could not create group cache: %v", err))
Expand Down Expand Up @@ -295,9 +247,6 @@ func (q *restrictUsersAdmission) ValidateInitialization() error {
if q.userClient == nil {
return errors.New("RestrictUsersAdmission plugin requires an OpenShift user client")
}
if q.authnCache == nil {
return errors.New("RestrictUsersAdmission plugin requires an authentication cache")
}
if q.groupCache == nil && q.groupCacheFunc == nil {
return errors.New("RestrictUsersAdmission plugin requires a group cache")
}
Expand Down
Loading

0 comments on commit 705dcdb

Please sign in to comment.