Skip to content

Commit 26b5a8c

Browse files
openshift-merge-robotopenshift-merge-robot
authored
Merge pull request #15968 from deads2k/oauth-03-use-client
Automatic merge from submit-queue use oauth REST endpoints instead of raw etcd @mfojtik this is needed to remove direct etcd access which bypasses any admission checks and auditing and prevents the future separation of processes.
2 parents 1f2388e + df6eccb commit 26b5a8c

28 files changed

+363
-820
lines changed

‎pkg/auth/oauth/registry/grantchecker.go

+7-7
Original file line numberDiff line numberDiff line change
@@ -5,25 +5,25 @@ import (
55

66
"k8s.io/apimachinery/pkg/api/errors"
77
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
8-
apirequest "k8s.io/apiserver/pkg/endpoints/request"
8+
"k8s.io/apiserver/pkg/authentication/user"
99

1010
"github.com/openshift/origin/pkg/auth/api"
11+
oauthclient "github.com/openshift/origin/pkg/oauth/generated/internalclientset/typed/oauth/internalversion"
1112
"github.com/openshift/origin/pkg/oauth/registry/oauthclientauthorization"
1213
"github.com/openshift/origin/pkg/oauth/scope"
13-
"k8s.io/apiserver/pkg/authentication/user"
1414
)
1515

1616
type ClientAuthorizationGrantChecker struct {
17-
registry oauthclientauthorization.Registry
17+
client oauthclient.OAuthClientAuthorizationInterface
1818
}
1919

20-
func NewClientAuthorizationGrantChecker(registry oauthclientauthorization.Registry) *ClientAuthorizationGrantChecker {
21-
return &ClientAuthorizationGrantChecker{registry}
20+
func NewClientAuthorizationGrantChecker(client oauthclient.OAuthClientAuthorizationInterface) *ClientAuthorizationGrantChecker {
21+
return &ClientAuthorizationGrantChecker{client}
2222
}
2323

2424
func (c *ClientAuthorizationGrantChecker) HasAuthorizedClient(user user.Info, grant *api.Grant) (approved bool, err error) {
25-
id := c.registry.ClientAuthorizationName(user.GetName(), grant.Client.GetId())
26-
authorization, err := c.registry.GetClientAuthorization(apirequest.NewContext(), id, &metav1.GetOptions{})
25+
id := oauthclientauthorization.ClientAuthorizationName(user.GetName(), grant.Client.GetId())
26+
authorization, err := c.client.Get(id, metav1.GetOptions{})
2727
if errors.IsNotFound(err) {
2828
return false, nil
2929
}

‎pkg/auth/oauth/registry/registry_test.go

+35-33
Original file line numberDiff line numberDiff line change
@@ -9,19 +9,22 @@ import (
99

1010
"github.com/RangelReale/osin"
1111
"github.com/RangelReale/osincli"
12+
1213
apierrs "k8s.io/apimachinery/pkg/api/errors"
1314
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
15+
"k8s.io/apimachinery/pkg/runtime"
16+
"k8s.io/apiserver/pkg/authentication/user"
17+
clienttesting "k8s.io/client-go/testing"
1418

1519
"github.com/openshift/origin/pkg/auth/api"
1620
"github.com/openshift/origin/pkg/auth/oauth/handlers"
1721
"github.com/openshift/origin/pkg/auth/userregistry/identitymapper"
1822
oapi "github.com/openshift/origin/pkg/oauth/apis/oauth"
19-
"github.com/openshift/origin/pkg/oauth/registry/test"
23+
oauthfake "github.com/openshift/origin/pkg/oauth/generated/internalclientset/fake"
2024
"github.com/openshift/origin/pkg/oauth/server/osinserver"
2125
"github.com/openshift/origin/pkg/oauth/server/osinserver/registrystorage"
2226
userapi "github.com/openshift/origin/pkg/user/apis/user"
2327
usertest "github.com/openshift/origin/pkg/user/registry/test"
24-
"k8s.io/apiserver/pkg/authentication/user"
2528
)
2629

2730
type testHandlers struct {
@@ -167,6 +170,7 @@ func TestRegistryAndServer(t *testing.T) {
167170
Name: "user",
168171
},
169172
ClientAuth: &oapi.OAuthClientAuthorization{
173+
ObjectMeta: metav1.ObjectMeta{Name: "user:test"},
170174
UserName: "user",
171175
ClientName: "test",
172176
Scopes: []string{"user:info"},
@@ -185,6 +189,7 @@ func TestRegistryAndServer(t *testing.T) {
185189
Name: "user",
186190
},
187191
ClientAuth: &oapi.OAuthClientAuthorization{
192+
ObjectMeta: metav1.ObjectMeta{Name: "user:test"},
188193
UserName: "user",
189194
ClientName: "test",
190195
Scopes: []string{"user:info", "user:check-access"},
@@ -203,6 +208,7 @@ func TestRegistryAndServer(t *testing.T) {
203208
Name: "user",
204209
},
205210
ClientAuth: &oapi.OAuthClientAuthorization{
211+
ObjectMeta: metav1.ObjectMeta{Name: "user:test"},
206212
UserName: "user",
207213
ClientName: "test",
208214
Scopes: []string{"user:full"},
@@ -227,20 +233,15 @@ func TestRegistryAndServer(t *testing.T) {
227233
h := &testHandlers{}
228234
h.Authenticate = testCase.AuthSuccess
229235
h.User = testCase.AuthUser
230-
access, authorize := &test.AccessTokenRegistry{}, &test.AuthorizeTokenRegistry{}
231-
client := &test.ClientRegistry{
232-
Client: testCase.Client,
233-
}
234-
if testCase.Client == nil {
235-
client.Err = apierrs.NewNotFound(oapi.Resource("OAuthClient"), "unknown")
236+
objs := []runtime.Object{}
237+
if testCase.Client != nil {
238+
objs = append(objs, testCase.Client)
236239
}
237-
grant := &test.ClientAuthorizationRegistry{
238-
ClientAuthorization: testCase.ClientAuth,
240+
if testCase.ClientAuth != nil {
241+
objs = append(objs, testCase.ClientAuth)
239242
}
240-
if testCase.ClientAuth == nil {
241-
grant.GetErr = apierrs.NewNotFound(oapi.Resource("OAuthClientAuthorization"), "test:test")
242-
}
243-
storage := registrystorage.New(access, authorize, client, NewUserConversion())
243+
fakeOAuthClient := oauthfake.NewSimpleClientset(objs...)
244+
storage := registrystorage.New(fakeOAuthClient.Oauth().OAuthAccessTokens(), fakeOAuthClient.Oauth().OAuthAuthorizeTokens(), fakeOAuthClient.Oauth().OAuthClients(), NewUserConversion())
244245
config := osinserver.NewDefaultServerConfig()
245246

246247
h.AuthorizeHandler = osinserver.AuthorizeHandlers{
@@ -250,7 +251,7 @@ func TestRegistryAndServer(t *testing.T) {
250251
h,
251252
),
252253
handlers.NewGrantCheck(
253-
NewClientAuthorizationGrantChecker(grant),
254+
NewClientAuthorizationGrantChecker(fakeOAuthClient.Oauth().OAuthClientAuthorizations()),
254255
h,
255256
h,
256257
),
@@ -299,9 +300,9 @@ func TestRegistryAndServer(t *testing.T) {
299300
}
300301

301302
func TestAuthenticateTokenNotFound(t *testing.T) {
302-
tokenRegistry := &test.AccessTokenRegistry{Err: apierrs.NewNotFound(oapi.Resource("OAuthAccessToken"), "token")}
303+
fakeOAuthClient := oauthfake.NewSimpleClientset()
303304
userRegistry := usertest.NewUserRegistry()
304-
tokenAuthenticator := NewTokenAuthenticator(tokenRegistry, userRegistry, identitymapper.NoopGroupMapper{})
305+
tokenAuthenticator := NewTokenAuthenticator(fakeOAuthClient.Oauth().OAuthAccessTokens(), userRegistry, identitymapper.NoopGroupMapper{})
305306

306307
userInfo, found, err := tokenAuthenticator.AuthenticateToken("token")
307308
if found {
@@ -318,9 +319,12 @@ func TestAuthenticateTokenNotFound(t *testing.T) {
318319
}
319320
}
320321
func TestAuthenticateTokenOtherGetError(t *testing.T) {
321-
tokenRegistry := &test.AccessTokenRegistry{Err: errors.New("get error")}
322+
fakeOAuthClient := oauthfake.NewSimpleClientset()
323+
fakeOAuthClient.PrependReactor("get", "oauthaccesstokens", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) {
324+
return true, nil, errors.New("get error")
325+
})
322326
userRegistry := usertest.NewUserRegistry()
323-
tokenAuthenticator := NewTokenAuthenticator(tokenRegistry, userRegistry, identitymapper.NoopGroupMapper{})
327+
tokenAuthenticator := NewTokenAuthenticator(fakeOAuthClient.Oauth().OAuthAccessTokens(), userRegistry, identitymapper.NoopGroupMapper{})
324328

325329
userInfo, found, err := tokenAuthenticator.AuthenticateToken("token")
326330
if found {
@@ -329,23 +333,22 @@ func TestAuthenticateTokenOtherGetError(t *testing.T) {
329333
if err == nil {
330334
t.Error("Expected error is missing!")
331335
}
332-
if err.Error() != tokenRegistry.Err.Error() {
333-
t.Errorf("Expected error %v, but got error %v", tokenRegistry.Err, err)
336+
if err.Error() != "get error" {
337+
t.Errorf("Expected error %v, but got error %v", "get error", err)
334338
}
335339
if userInfo != nil {
336340
t.Errorf("Unexpected user: %v", userInfo)
337341
}
338342
}
339343
func TestAuthenticateTokenExpired(t *testing.T) {
340-
tokenRegistry := &test.AccessTokenRegistry{
341-
Err: nil,
342-
AccessToken: &oapi.OAuthAccessToken{
343-
ObjectMeta: metav1.ObjectMeta{CreationTimestamp: metav1.Time{Time: time.Now().Add(-1 * time.Hour)}},
344+
fakeOAuthClient := oauthfake.NewSimpleClientset(
345+
&oapi.OAuthAccessToken{
346+
ObjectMeta: metav1.ObjectMeta{Name: "token", CreationTimestamp: metav1.Time{Time: time.Now().Add(-1 * time.Hour)}},
344347
ExpiresIn: 600, // 10 minutes
345348
},
346-
}
349+
)
347350
userRegistry := usertest.NewUserRegistry()
348-
tokenAuthenticator := NewTokenAuthenticator(tokenRegistry, userRegistry, identitymapper.NoopGroupMapper{})
351+
tokenAuthenticator := NewTokenAuthenticator(fakeOAuthClient.Oauth().OAuthAccessTokens(), userRegistry, identitymapper.NoopGroupMapper{})
349352

350353
userInfo, found, err := tokenAuthenticator.AuthenticateToken("token")
351354
if found {
@@ -359,19 +362,18 @@ func TestAuthenticateTokenExpired(t *testing.T) {
359362
}
360363
}
361364
func TestAuthenticateTokenValidated(t *testing.T) {
362-
tokenRegistry := &test.AccessTokenRegistry{
363-
Err: nil,
364-
AccessToken: &oapi.OAuthAccessToken{
365-
ObjectMeta: metav1.ObjectMeta{CreationTimestamp: metav1.Time{Time: time.Now()}},
365+
fakeOAuthClient := oauthfake.NewSimpleClientset(
366+
&oapi.OAuthAccessToken{
367+
ObjectMeta: metav1.ObjectMeta{Name: "token", CreationTimestamp: metav1.Time{Time: time.Now()}},
366368
ExpiresIn: 600, // 10 minutes
367369
UserName: "foo",
368370
UserUID: string("bar"),
369371
},
370-
}
372+
)
371373
userRegistry := usertest.NewUserRegistry()
372374
userRegistry.GetUsers["foo"] = &userapi.User{ObjectMeta: metav1.ObjectMeta{UID: "bar"}}
373375

374-
tokenAuthenticator := NewTokenAuthenticator(tokenRegistry, userRegistry, identitymapper.NoopGroupMapper{})
376+
tokenAuthenticator := NewTokenAuthenticator(fakeOAuthClient.Oauth().OAuthAccessTokens(), userRegistry, identitymapper.NoopGroupMapper{})
375377

376378
userInfo, found, err := tokenAuthenticator.AuthenticateToken("token")
377379
if !found {

‎pkg/auth/oauth/registry/tokenauthenticator.go

+7-9
Original file line numberDiff line numberDiff line change
@@ -5,24 +5,24 @@ import (
55
"fmt"
66
"time"
77

8+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
9+
kuser "k8s.io/apiserver/pkg/authentication/user"
10+
811
"github.com/openshift/origin/pkg/auth/userregistry/identitymapper"
912
authorizationapi "github.com/openshift/origin/pkg/authorization/apis/authorization"
10-
"github.com/openshift/origin/pkg/oauth/registry/oauthaccesstoken"
13+
oauthclient "github.com/openshift/origin/pkg/oauth/generated/internalclientset/typed/oauth/internalversion"
1114
userclient "github.com/openshift/origin/pkg/user/generated/internalclientset/typed/user/internalversion"
12-
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
13-
kuser "k8s.io/apiserver/pkg/authentication/user"
14-
kapirequest "k8s.io/apiserver/pkg/endpoints/request"
1515
)
1616

1717
type TokenAuthenticator struct {
18-
tokens oauthaccesstoken.Registry
18+
tokens oauthclient.OAuthAccessTokenInterface
1919
users userclient.UserResourceInterface
2020
groupMapper identitymapper.UserToGroupMapper
2121
}
2222

2323
var ErrExpired = errors.New("Token is expired")
2424

25-
func NewTokenAuthenticator(tokens oauthaccesstoken.Registry, users userclient.UserResourceInterface, groupMapper identitymapper.UserToGroupMapper) *TokenAuthenticator {
25+
func NewTokenAuthenticator(tokens oauthclient.OAuthAccessTokenInterface, users userclient.UserResourceInterface, groupMapper identitymapper.UserToGroupMapper) *TokenAuthenticator {
2626
return &TokenAuthenticator{
2727
tokens: tokens,
2828
users: users,
@@ -31,9 +31,7 @@ func NewTokenAuthenticator(tokens oauthaccesstoken.Registry, users userclient.Us
3131
}
3232

3333
func (a *TokenAuthenticator) AuthenticateToken(value string) (kuser.Info, bool, error) {
34-
ctx := kapirequest.NewContext()
35-
36-
token, err := a.tokens.GetAccessToken(ctx, value, &metav1.GetOptions{})
34+
token, err := a.tokens.Get(value, metav1.GetOptions{})
3735
if err != nil {
3836
return nil, false, err
3937
}

‎pkg/auth/server/grant/grant.go

+13-14
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,14 @@ import (
1111
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
1212
"k8s.io/apiserver/pkg/authentication/serviceaccount"
1313
"k8s.io/apiserver/pkg/authentication/user"
14-
apirequest "k8s.io/apiserver/pkg/endpoints/request"
1514

1615
"github.com/golang/glog"
1716
"github.com/openshift/origin/pkg/auth/authenticator"
1817
"github.com/openshift/origin/pkg/auth/server/csrf"
1918
scopeauthorizer "github.com/openshift/origin/pkg/authorization/authorizer/scope"
2019
oapi "github.com/openshift/origin/pkg/oauth/apis/oauth"
21-
"github.com/openshift/origin/pkg/oauth/registry/oauthclient"
20+
oauthclient "github.com/openshift/origin/pkg/oauth/generated/internalclientset/typed/oauth/internalversion"
21+
oauthclientregistry "github.com/openshift/origin/pkg/oauth/registry/oauthclient"
2222
"github.com/openshift/origin/pkg/oauth/registry/oauthclientauthorization"
2323
"github.com/openshift/origin/pkg/oauth/scope"
2424
)
@@ -82,11 +82,11 @@ type Grant struct {
8282
auth authenticator.Request
8383
csrf csrf.CSRF
8484
render FormRenderer
85-
clientregistry oauthclient.Getter
86-
authregistry oauthclientauthorization.Registry
85+
clientregistry oauthclientregistry.Getter
86+
authregistry oauthclient.OAuthClientAuthorizationInterface
8787
}
8888

89-
func NewGrant(csrf csrf.CSRF, auth authenticator.Request, render FormRenderer, clientregistry oauthclient.Getter, authregistry oauthclientauthorization.Registry) *Grant {
89+
func NewGrant(csrf csrf.CSRF, auth authenticator.Request, render FormRenderer, clientregistry oauthclientregistry.Getter, authregistry oauthclient.OAuthClientAuthorizationInterface) *Grant {
9090
return &Grant{
9191
auth: auth,
9292
csrf: csrf,
@@ -129,7 +129,7 @@ func (l *Grant) handleForm(user user.Info, w http.ResponseWriter, req *http.Requ
129129
scopes := scope.Split(q.Get(scopeParam))
130130
redirectURI := q.Get(redirectURIParam)
131131

132-
client, err := l.clientregistry.GetClient(apirequest.NewContext(), clientID, &metav1.GetOptions{})
132+
client, err := l.clientregistry.Get(clientID, metav1.GetOptions{})
133133
if err != nil || client == nil {
134134
l.failed("Could not find client for client_id", w, req)
135135
return
@@ -152,8 +152,8 @@ func (l *Grant) handleForm(user user.Info, w http.ResponseWriter, req *http.Requ
152152
grantedScopes := []Scope{}
153153
requestedScopes := []Scope{}
154154

155-
clientAuthID := l.authregistry.ClientAuthorizationName(user.GetName(), client.Name)
156-
if clientAuth, err := l.authregistry.GetClientAuthorization(apirequest.NewContext(), clientAuthID, &metav1.GetOptions{}); err == nil {
155+
clientAuthID := oauthclientauthorization.ClientAuthorizationName(user.GetName(), client.Name)
156+
if clientAuth, err := l.authregistry.Get(clientAuthID, metav1.GetOptions{}); err == nil {
157157
grantedScopeNames = clientAuth.Scopes
158158
}
159159

@@ -233,7 +233,7 @@ func (l *Grant) handleGrant(user user.Info, w http.ResponseWriter, req *http.Req
233233
}
234234

235235
clientID := req.PostFormValue(clientIDParam)
236-
client, err := l.clientregistry.GetClient(apirequest.NewContext(), clientID, &metav1.GetOptions{})
236+
client, err := l.clientregistry.Get(clientID, metav1.GetOptions{})
237237
if err != nil || client == nil {
238238
l.failed("Could not find client for client_id", w, req)
239239
return
@@ -244,14 +244,13 @@ func (l *Grant) handleGrant(user user.Info, w http.ResponseWriter, req *http.Req
244244
return
245245
}
246246

247-
clientAuthID := l.authregistry.ClientAuthorizationName(user.GetName(), client.Name)
247+
clientAuthID := oauthclientauthorization.ClientAuthorizationName(user.GetName(), client.Name)
248248

249-
ctx := apirequest.NewContext()
250-
clientAuth, err := l.authregistry.GetClientAuthorization(ctx, clientAuthID, &metav1.GetOptions{})
249+
clientAuth, err := l.authregistry.Get(clientAuthID, metav1.GetOptions{})
251250
if err == nil && clientAuth != nil {
252251
// Add new scopes and update
253252
clientAuth.Scopes = scope.Add(clientAuth.Scopes, scope.Split(scopes))
254-
if _, err = l.authregistry.UpdateClientAuthorization(ctx, clientAuth); err != nil {
253+
if _, err = l.authregistry.Update(clientAuth); err != nil {
255254
glog.Errorf("Unable to update authorization: %v", err)
256255
l.failed("Could not update client authorization", w, req)
257256
return
@@ -266,7 +265,7 @@ func (l *Grant) handleGrant(user user.Info, w http.ResponseWriter, req *http.Req
266265
}
267266
clientAuth.Name = clientAuthID
268267

269-
if _, err = l.authregistry.CreateClientAuthorization(ctx, clientAuth); err != nil {
268+
if _, err = l.authregistry.Create(clientAuth); err != nil {
270269
glog.Errorf("Unable to create authorization: %v", err)
271270
l.failed("Could not create client authorization", w, req)
272271
return

0 commit comments

Comments
 (0)