Merge pull request #18370 from deads2k/oauth-02-apis

Automatic merge from submit-queue (batch tested with PRs 17942, 18370).

switch oauthserver to external user client

This switches the vast majority of calls to external user calls. There's a bad unit test that needs to move, but that is involved.

I also found some packages that are not used inside of the oauthserver and used instead in the apiserver.

/assign enj
/assign mfojtik

Author: openshift-merge-robot

hack/import-restrictions.json

@@ -343,6 +343,7 @@
     ],
     "allowedImportPackageRoots": [
       "vendor/k8s.io/apimachinery",
+      "vendor/k8s.io/api",
       "vendor/k8s.io/apiserver",
       "vendor/k8s.io/client-go",
       "vendor/golang.org",
@@ -353,28 +354,26 @@
       "vendor/github.com/gorilla/sessions",
       "vendor/github.com/gorilla/context",
       "vendor/github.com/rackspace/gophercloud",
-      "vendor/github.com/openshift/api",
       "vendor/gopkg.in/ldap.v2",
       "vendor/k8s.io/kubernetes",
       "vendor/github.com/prometheus/client_golang/prometheus",
+      "vendor/github.com/openshift/api",
+      "vendor/github.com/openshift/client-go",
+
       "github.com/openshift/origin/pkg/oauthserver",
-      "github.com/openshift/origin/pkg/user/generated",
       "github.com/openshift/origin/pkg/route/generated",
       "github.com/openshift/origin/pkg/oauth/generated"
     ],
     "allowedImportPackages": [
       "vendor/github.com/golang/glog",
-      "vendor/k8s.io/kubernetes/pkg/apis/core",
       "vendor/k8s.io/kubernetes/pkg/api/legacyscheme",
       "github.com/openshift/origin/pkg/authorization/apis/authorization",
-      "github.com/openshift/origin/pkg/user/apis/user",
       "github.com/openshift/origin/pkg/cmd/server/api",
       "github.com/openshift/origin/pkg/util/http/links",
       "github.com/openshift/origin/pkg/authorization/authorizer/scope",
       "github.com/openshift/origin/pkg/oauth/apis/oauth/validation",
       "github.com/openshift/origin/pkg/oauth/scope",
       "github.com/openshift/origin/pkg/oauth/apis/oauth",
-      "github.com/openshift/origin/pkg/util/rankedset",
       "github.com/openshift/origin/pkg/oauth/registry/oauthclientauthorization",
       "github.com/openshift/origin/pkg/cmd/server/api",
       "github.com/openshift/origin/pkg/cmd/server/api/latest",
@@ -385,7 +384,8 @@
       "github.com/openshift/origin/pkg/user/registry/test",
       "github.com/openshift/origin/pkg/oauth/util",
       "github.com/openshift/origin/pkg/api/install",
-      "github.com/openshift/origin/pkg/user",
+      "github.com/openshift/origin/pkg/user/apis/user/v1",
+      "github.com/openshift/origin/pkg/user/apis/user",
       "github.com/openshift/origin/pkg/user/registry/test",
       "github.com/openshift/origin/pkg/user/registry/useridentitymapping",
       "github.com/openshift/origin/pkg/util/httprequest"

pkg/oauthserver/oauth/registry/expirationvalidator.go pkg/apiserver/authentication/internaloauth/expirationvalidator.go

@@ -1,19 +1,18 @@
-package registry
+package internaloauth
 
 import (
 	"errors"
 	"time"
 
+	userv1 "github.com/openshift/api/user/v1"
 	"github.com/openshift/origin/pkg/oauth/apis/oauth"
-	"github.com/openshift/origin/pkg/oauthserver/authenticator"
-	"github.com/openshift/origin/pkg/user/apis/user"
 )
 
 var errExpired = errors.New("token is expired")
 
-func NewExpirationValidator() authenticator.OAuthTokenValidator {
-	return authenticator.OAuthTokenValidatorFunc(
-		func(token *oauth.OAuthAccessToken, _ *user.User) error {
+func NewExpirationValidator() OAuthTokenValidator {
+	return OAuthTokenValidatorFunc(
+		func(token *oauth.OAuthAccessToken, _ *userv1.User) error {
 			if token.ExpiresIn > 0 {
 				if expire(token).Before(time.Now()) {
 					return errExpired

pkg/apiserver/authentication/internaloauth/expirationvalidator_test.go

@@ -0,0 +1,70 @@
+package internaloauth
+
+import (
+	"testing"
+	"time"
+
+	userapi "github.com/openshift/api/user/v1"
+	userfake "github.com/openshift/client-go/user/clientset/versioned/fake"
+	oapi "github.com/openshift/origin/pkg/oauth/apis/oauth"
+	oauthfake "github.com/openshift/origin/pkg/oauth/generated/internalclientset/fake"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func TestAuthenticateTokenExpired(t *testing.T) {
+	fakeOAuthClient := oauthfake.NewSimpleClientset(
+		// expired token that had a lifetime of 10 minutes
+		&oapi.OAuthAccessToken{
+			ObjectMeta: metav1.ObjectMeta{Name: "token1", CreationTimestamp: metav1.Time{Time: time.Now().Add(-1 * time.Hour)}},
+			ExpiresIn:  600,
+			UserName:   "foo",
+		},
+		// non-expired token that has a lifetime of 10 minutes, but has a non-nil deletion timestamp
+		&oapi.OAuthAccessToken{
+			ObjectMeta: metav1.ObjectMeta{Name: "token2", CreationTimestamp: metav1.Time{Time: time.Now()}, DeletionTimestamp: &metav1.Time{}},
+			ExpiresIn:  600,
+			UserName:   "foo",
+		},
+	)
+	fakeUserClient := userfake.NewSimpleClientset(&userapi.User{ObjectMeta: metav1.ObjectMeta{Name: "foo", UID: "bar"}})
+
+	tokenAuthenticator := NewTokenAuthenticator(fakeOAuthClient.Oauth().OAuthAccessTokens(), fakeUserClient.UserV1().Users(), NoopGroupMapper{}, NewExpirationValidator())
+
+	for _, tokenName := range []string{"token1", "token2"} {
+		userInfo, found, err := tokenAuthenticator.AuthenticateToken(tokenName)
+		if found {
+			t.Error("Found token, but it should be missing!")
+		}
+		if err != errExpired {
+			t.Errorf("Unexpected error: %v", err)
+		}
+		if userInfo != nil {
+			t.Errorf("Unexpected user: %v", userInfo)
+		}
+	}
+}
+
+func TestAuthenticateTokenValidated(t *testing.T) {
+	fakeOAuthClient := oauthfake.NewSimpleClientset(
+		&oapi.OAuthAccessToken{
+			ObjectMeta: metav1.ObjectMeta{Name: "token", CreationTimestamp: metav1.Time{Time: time.Now()}},
+			ExpiresIn:  600, // 10 minutes
+			UserName:   "foo",
+			UserUID:    string("bar"),
+		},
+	)
+	fakeUserClient := userfake.NewSimpleClientset(&userapi.User{ObjectMeta: metav1.ObjectMeta{Name: "foo", UID: "bar"}})
+
+	tokenAuthenticator := NewTokenAuthenticator(fakeOAuthClient.Oauth().OAuthAccessTokens(), fakeUserClient.UserV1().Users(), NoopGroupMapper{}, NewExpirationValidator(), NewUIDValidator())
+
+	userInfo, found, err := tokenAuthenticator.AuthenticateToken("token")
+	if !found {
+		t.Error("Did not find a token!")
+	}
+	if err != nil {
+		t.Errorf("Unexpected error: %v", err)
+	}
+	if userInfo == nil {
+		t.Error("Did not get a user!")
+	}
+}

pkg/apiserver/authentication/internaloauth/interfaces.go

@@ -0,0 +1,41 @@
+package internaloauth
+
+import (
+	userapi "github.com/openshift/api/user/v1"
+	"github.com/openshift/origin/pkg/oauth/apis/oauth"
+)
+
+type OAuthTokenValidator interface {
+	Validate(token *oauth.OAuthAccessToken, user *userapi.User) error
+}
+
+var _ OAuthTokenValidator = OAuthTokenValidatorFunc(nil)
+
+type OAuthTokenValidatorFunc func(token *oauth.OAuthAccessToken, user *userapi.User) error
+
+func (f OAuthTokenValidatorFunc) Validate(token *oauth.OAuthAccessToken, user *userapi.User) error {
+	return f(token, user)
+}
+
+var _ OAuthTokenValidator = OAuthTokenValidators(nil)
+
+type OAuthTokenValidators []OAuthTokenValidator
+
+func (v OAuthTokenValidators) Validate(token *oauth.OAuthAccessToken, user *userapi.User) error {
+	for _, validator := range v {
+		if err := validator.Validate(token, user); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+type UserToGroupMapper interface {
+	GroupsFor(username string) ([]*userapi.Group, error)
+}
+
+type NoopGroupMapper struct{}
+
+func (n NoopGroupMapper) GroupsFor(username string) ([]*userapi.Group, error) {
+	return []*userapi.Group{}, nil
+}

pkg/oauthserver/oauth/registry/timeoutvalidator.go pkg/apiserver/authentication/internaloauth/timeoutvalidator.go

@@ -1,4 +1,4 @@
-package registry
+package internaloauth
 
 import (
 	"errors"
@@ -10,10 +10,10 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/runtime"
 
+	userv1 "github.com/openshift/api/user/v1"
 	"github.com/openshift/origin/pkg/oauth/apis/oauth"
 	oauthclient "github.com/openshift/origin/pkg/oauth/generated/internalclientset/typed/oauth/internalversion"
 	oauthclientlister "github.com/openshift/origin/pkg/oauth/generated/listers/oauth/internalversion"
-	"github.com/openshift/origin/pkg/user/apis/user"
 	"github.com/openshift/origin/pkg/util/rankedset"
 )
 
@@ -105,7 +105,7 @@ func NewTimeoutValidator(tokens oauthclient.OAuthAccessTokenInterface, oauthClie
 
 // Validate is called with a token when it is seen by an authenticator
 // it touches only the tokenChannel so it is safe to call from other threads
-func (a *TimeoutValidator) Validate(token *oauth.OAuthAccessToken, _ *user.User) error {
+func (a *TimeoutValidator) Validate(token *oauth.OAuthAccessToken, _ *userv1.User) error {
 	if token.InactivityTimeoutSeconds == 0 {
 		// We care only if the token was created with a timeout to start with
 		return nil

pkg/oauthserver/oauth/registry/tokenauthenticator.go pkg/apiserver/authentication/internaloauth/tokenauthenticator.go

@@ -1,30 +1,28 @@
-package registry
+package internaloauth
 
 import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	kauthenticator "k8s.io/apiserver/pkg/authentication/authenticator"
 	kuser "k8s.io/apiserver/pkg/authentication/user"
 
+	userclient "github.com/openshift/client-go/user/clientset/versioned/typed/user/v1"
 	authorizationapi "github.com/openshift/origin/pkg/authorization/apis/authorization"
 	oauthclient "github.com/openshift/origin/pkg/oauth/generated/internalclientset/typed/oauth/internalversion"
-	"github.com/openshift/origin/pkg/oauthserver/authenticator"
-	"github.com/openshift/origin/pkg/oauthserver/userregistry/identitymapper"
-	userclient "github.com/openshift/origin/pkg/user/generated/internalclientset/typed/user/internalversion"
 )
 
 type tokenAuthenticator struct {
 	tokens      oauthclient.OAuthAccessTokenInterface
-	users       userclient.UserResourceInterface
-	groupMapper identitymapper.UserToGroupMapper
-	validators  authenticator.OAuthTokenValidator
+	users       userclient.UserInterface
+	groupMapper UserToGroupMapper
+	validators  OAuthTokenValidator
 }
 
-func NewTokenAuthenticator(tokens oauthclient.OAuthAccessTokenInterface, users userclient.UserResourceInterface, groupMapper identitymapper.UserToGroupMapper, validators ...authenticator.OAuthTokenValidator) kauthenticator.Token {
+func NewTokenAuthenticator(tokens oauthclient.OAuthAccessTokenInterface, users userclient.UserInterface, groupMapper UserToGroupMapper, validators ...OAuthTokenValidator) kauthenticator.Token {
 	return &tokenAuthenticator{
 		tokens:      tokens,
 		users:       users,
 		groupMapper: groupMapper,
-		validators:  authenticator.OAuthTokenValidators(validators),
+		validators:  OAuthTokenValidators(validators),
 	}
 }