Skip to content

Commit 7a3a6aa

Browse files
everettraveneverettraven
committed
WIP: poc: OIDC patch for restrictusers
Signed-off-by: everettraven <[email protected]>
1 parent 276310a commit 7a3a6aa

File tree

6 files changed

+341
-63
lines changed

6 files changed

+341
-63
lines changed

‎openshift-kube-apiserver/admission/authorization/restrictusers/authncache/authentication.go

+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
package authncache
2+
3+
import (
4+
configv1 "github.com/openshift/api/config/v1"
5+
configv1informer "github.com/openshift/client-go/config/informers/externalversions/config/v1"
6+
)
7+
8+
type AuthnCache struct {
9+
authnInformer configv1informer.AuthenticationInformer
10+
}
11+
12+
func NewAuthnCache(authInformer configv1informer.AuthenticationInformer) *AuthnCache {
13+
return &AuthnCache{
14+
authnInformer: authInformer,
15+
}
16+
}
17+
18+
func (ac *AuthnCache) Authn() (*configv1.Authentication, error) {
19+
return ac.authnInformer.Lister().Get("cluster")
20+
}
21+
22+
func (ac *AuthnCache) HasSynced() bool {
23+
return ac.authnInformer.Informer().HasSynced()
24+
}

‎openshift-kube-apiserver/admission/authorization/restrictusers/authncache_test.go

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
package restrictusers
2+
3+
import configv1 "github.com/openshift/api/config/v1"
4+
5+
type fakeAuthnCache struct {
6+
authn *configv1.Authentication
7+
err error
8+
hasSynced bool
9+
}
10+
11+
func (f *fakeAuthnCache) Authn() (*configv1.Authentication, error) {
12+
return f.authn, f.err
13+
}
14+
15+
func (f *fakeAuthnCache) HasSynced() bool {
16+
return f.hasSynced
17+
}

‎openshift-kube-apiserver/admission/authorization/restrictusers/intializers.go

+12-2
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,25 @@ import (
44
"k8s.io/apiserver/pkg/admission"
55

66
userinformer "github.com/openshift/client-go/user/informers/externalversions"
7+
configv1informer "github.com/openshift/client-go/config/informers/externalversions"
78
)
89

9-
func NewInitializer(userInformer userinformer.SharedInformerFactory) admission.PluginInitializer {
10-
return &localInitializer{userInformer: userInformer}
10+
func NewInitializer(userInformer userinformer.SharedInformerFactory, configInformer configv1informer.SharedInformerFactory) admission.PluginInitializer {
11+
return &localInitializer{userInformer: userInformer, configInformer: configInformer}
1112
}
1213

1314
type WantsUserInformer interface {
1415
SetUserInformer(userinformer.SharedInformerFactory)
1516
admission.InitializationValidator
1617
}
1718

19+
type WantsConfigInformer interface {
20+
SetConfigInformer(configv1informer.SharedInformerFactory)
21+
}
22+
1823
type localInitializer struct {
1924
userInformer userinformer.SharedInformerFactory
25+
configInformer configv1informer.SharedInformerFactory
2026
}
2127

2228
// Initialize will check the initialization interfaces implemented by each plugin
@@ -25,4 +31,8 @@ func (i *localInitializer) Initialize(plugin admission.Interface) {
2531
if wants, ok := plugin.(WantsUserInformer); ok {
2632
wants.SetUserInformer(i.userInformer)
2733
}
34+
35+
if wants, ok := plugin.(WantsConfigInformer); ok {
36+
wants.SetConfigInformer(i.configInformer)
37+
}
2838
}

‎openshift-kube-apiserver/admission/authorization/restrictusers/restrictusers.go

+69-37
Original file line numberDiff line numberDiff line change
@@ -5,23 +5,28 @@ import (
55
"errors"
66
"fmt"
77
"io"
8+
"time"
89

910
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1011
kerrors "k8s.io/apimachinery/pkg/util/errors"
1112
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
13+
"k8s.io/apimachinery/pkg/util/wait"
1214
"k8s.io/apiserver/pkg/admission"
1315
"k8s.io/apiserver/pkg/admission/initializer"
1416
"k8s.io/client-go/kubernetes"
1517
"k8s.io/client-go/rest"
18+
"k8s.io/client-go/tools/cache"
1619
"k8s.io/klog/v2"
1720
"k8s.io/kubernetes/pkg/apis/rbac"
1821

22+
configv1 "github.com/openshift/api/config/v1"
1923
userv1 "github.com/openshift/api/user/v1"
2024
authorizationtypedclient "github.com/openshift/client-go/authorization/clientset/versioned/typed/authorization/v1"
21-
configclient "github.com/openshift/client-go/config/clientset/versioned"
25+
configv1informer "github.com/openshift/client-go/config/informers/externalversions"
2226
userclient "github.com/openshift/client-go/user/clientset/versioned"
2327
userinformer "github.com/openshift/client-go/user/informers/externalversions"
2428
"github.com/openshift/library-go/pkg/apiserver/admission/admissionrestconfig"
29+
"k8s.io/kubernetes/openshift-kube-apiserver/admission/authorization/restrictusers/authncache"
2530
"k8s.io/kubernetes/openshift-kube-apiserver/admission/authorization/restrictusers/usercache"
2631
)
2732

@@ -37,6 +42,11 @@ type GroupCache interface {
3742
HasSynced() bool
3843
}
3944

45+
type AuthnCache interface {
46+
Authn() (*configv1.Authentication, error)
47+
HasSynced() bool
48+
}
49+
4050
// restrictUsersAdmission implements admission.ValidateInterface and enforces
4151
// restrictions on adding rolebindings in a project to permit only designated
4252
// subjects.
@@ -46,8 +56,9 @@ type restrictUsersAdmission struct {
4656
roleBindingRestrictionsGetter authorizationtypedclient.RoleBindingRestrictionsGetter
4757
userClient userclient.Interface
4858
kubeClient kubernetes.Interface
49-
configClient configclient.Interface
59+
groupCacheFunc func() (GroupCache, error)
5060
groupCache GroupCache
61+
authnCache AuthnCache
5162
}
5263

5364
var (
@@ -88,32 +99,39 @@ func (q *restrictUsersAdmission) SetRESTClientConfig(restClientConfig rest.Confi
8899
utilruntime.HandleError(err)
89100
return
90101
}
102+
}
91103

92-
// OpenQuestion: Does this get configured before SetUserInformer is called?
93-
q.configClient, err = configclient.NewForConfig(&restClientConfig)
94-
if err != nil {
95-
utilruntime.HandleError(err)
96-
return
97-
}
104+
func (q *restrictUsersAdmission) isAuthTypeOIDC() (bool, error) {
105+
err := wait.PollImmediate(1*time.Second, 10*time.Second, func() (bool, error) {
106+
return q.authnCache.HasSynced(), nil
107+
})
108+
if err != nil {
109+
return false, errors.New("authentications.config.openshift.io cache is not synchronized")
110+
}
111+
112+
auth, err := q.authnCache.Authn()
113+
if err == nil && auth != nil {
114+
return auth.Spec.Type == configv1.AuthenticationTypeOIDC, nil
115+
}
116+
return false, err
98117
}
99118

100-
// TODO: a better implementation where we don't make a live request every time
101-
func (q *restrictUsersAdmission) isAuthTypeOIDC() bool {
102-
auth, err := q.configClient.ConfigV1().Authentications().Get(context.TODO(), "cluster",metav1.GetOptions{})
103-
if err == nil {
104-
return auth.Spec.Type == "OIDC"
105-
}
106-
return false
119+
func (q *restrictUsersAdmission) SetConfigInformer(configInformer configv1informer.SharedInformerFactory) {
120+
q.authnCache = authncache.NewAuthnCache(configInformer.Config().V1().Authentications())
107121
}
108122

109123
func (q *restrictUsersAdmission) SetUserInformer(userInformers userinformer.SharedInformerFactory) {
110-
// if the authentication type is OIDC, meaning external OIDC provider(s)
111-
// have been configured, the oauth-apiserver is not running and thus
112-
// the Group API is not served.
113-
if q.isAuthTypeOIDC() {
114-
return
115-
}
116-
q.groupCache = usercache.NewGroupCache(userInformers.User().V1().Groups())
124+
// defer the allocation of the group cache until later in the process so we can
125+
// ensure we aren't creating informers for the Group resources if the authentication
126+
// type is OIDC.
127+
q.groupCacheFunc = func() (GroupCache, error) {
128+
if err := userInformers.User().V1().Groups().Informer().AddIndexers(cache.Indexers{
129+
usercache.ByUserIndexName: usercache.ByUserIndexKeys,
130+
}); err != nil {
131+
return nil, err
132+
}
133+
return usercache.NewGroupCache(userInformers.User().V1().Groups()), nil
134+
}
117135
}
118136

119137
// subjectsDelta returns the relative complement of elementsToIgnore in
@@ -203,29 +221,41 @@ func (q *restrictUsersAdmission) Validate(ctx context.Context, a admission.Attri
203221
return nil
204222
}
205223

206-
isAuthOIDC := q.isAuthTypeOIDC()
224+
isAuthOIDC, err := q.isAuthTypeOIDC()
225+
if err != nil {
226+
return admission.NewForbidden(a, fmt.Errorf("could not determine if authentication type is OIDC: %v", err))
227+
}
207228

208229
checkers := []SubjectChecker{}
209230
for _, rbr := range roleBindingRestrictionList.Items {
210-
// if the auth type is OIDC, the oauth-apiserver is down and as such
211-
// we cannot properly evaluate the user and/or group subjects. Fail fast
212-
// if the RBR has user and/or group restrictions applied if auth type is OIDC
213-
if isAuthOIDC {
214-
if rbr.Spec.UserRestriction != nil {
215-
return admission.NewForbidden(a,errors.New("auth type is OIDC and rolebinding restriction specifies user restrictions. Unable to get user information due to OIDC configuration, rejecting"))
216-
}
217-
218-
if rbr.Spec.GroupRestriction != nil {
219-
return admission.NewForbidden(a, errors.New("auth type is OIDC and rolebinding restriction specifies group restrictions. Unable to get group information due to OIDC configuration, rejecting"))
220-
}
221-
}
231+
// if the auth type is OIDC, the oauth-apiserver is down and as such
232+
// we cannot properly evaluate the user and/or group subjects. Fail fast
233+
// if the RBR has user and/or group restrictions applied if auth type is OIDC
234+
if isAuthOIDC {
235+
if rbr.Spec.UserRestriction != nil {
236+
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"))
237+
}
238+
239+
if rbr.Spec.GroupRestriction != nil {
240+
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"))
241+
}
242+
}
222243
checker, err := NewSubjectChecker(&rbr.Spec)
223244
if err != nil {
224245
return admission.NewForbidden(a, fmt.Errorf("could not create rolebinding restriction subject checker: %v", err))
225246
}
226247
checkers = append(checkers, checker)
227248
}
228249

250+
// If auth type is OIDC, we should never create checkers for the user/group restrictions
251+
// so it should be ok to provide a nil group cache
252+
if !isAuthOIDC && q.groupCache == nil && q.groupCacheFunc != nil {
253+
q.groupCache, err = q.groupCacheFunc()
254+
if err != nil {
255+
return admission.NewForbidden(a, fmt.Errorf("could not create group cache: %v", err))
256+
}
257+
}
258+
229259
roleBindingRestrictionContext, err := newRoleBindingRestrictionContext(ns,
230260
q.kubeClient, q.userClient.UserV1(), q.groupCache)
231261
if err != nil {
@@ -265,8 +295,10 @@ func (q *restrictUsersAdmission) ValidateInitialization() error {
265295
if q.userClient == nil {
266296
return errors.New("RestrictUsersAdmission plugin requires an OpenShift user client")
267297
}
268-
// Only worry about the group cache if authentication type is not OIDC
269-
if q.groupCache == nil && !q.isAuthTypeOIDC() {
298+
if q.authnCache == nil {
299+
return errors.New("RestrictUsersAdmission plugin requires an authentication cache")
300+
}
301+
if q.groupCache == nil && q.groupCacheFunc == nil {
270302
return errors.New("RestrictUsersAdmission plugin requires a group cache")
271303
}
272304

0 commit comments

Comments
 (0)