diff --git a/pkg/auth/authenticator/interfaces.go b/pkg/auth/authenticator/interfaces.go index e63c804f7198..26bef8499ee5 100644 --- a/pkg/auth/authenticator/interfaces.go +++ b/pkg/auth/authenticator/interfaces.go @@ -4,6 +4,8 @@ import ( "k8s.io/apiserver/pkg/authentication/user" "github.com/openshift/origin/pkg/auth/api" + "github.com/openshift/origin/pkg/oauth/apis/oauth" + userapi "github.com/openshift/origin/pkg/user/apis/user" ) type Assertion interface { @@ -13,3 +15,28 @@ type Assertion interface { type Client interface { AuthenticateClient(client api.Client) (user.Info, bool, error) } + +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 +} diff --git a/pkg/auth/oauth/registry/expirationvalidator.go b/pkg/auth/oauth/registry/expirationvalidator.go new file mode 100644 index 000000000000..876a7b9c7013 --- /dev/null +++ b/pkg/auth/oauth/registry/expirationvalidator.go @@ -0,0 +1,32 @@ +package registry + +import ( + "errors" + "time" + + "github.com/openshift/origin/pkg/auth/authenticator" + "github.com/openshift/origin/pkg/oauth/apis/oauth" + "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 { + if token.ExpiresIn > 0 { + if expire(token).Before(time.Now()) { + return errExpired + } + } + if token.DeletionTimestamp != nil { + return errExpired + } + return nil + }, + ) +} + +func expire(token *oauth.OAuthAccessToken) time.Time { + return token.CreationTimestamp.Add(time.Duration(token.ExpiresIn) * time.Second) +} diff --git a/pkg/auth/oauth/registry/registry_test.go b/pkg/auth/oauth/registry/registry_test.go index 0f5ed8976bfc..dfe2481386df 100644 --- a/pkg/auth/oauth/registry/registry_test.go +++ b/pkg/auth/oauth/registry/registry_test.go @@ -318,6 +318,7 @@ func TestAuthenticateTokenNotFound(t *testing.T) { t.Errorf("Unexpected user: %v", userInfo) } } + func TestAuthenticateTokenOtherGetError(t *testing.T) { fakeOAuthClient := oauthfake.NewSimpleClientset() fakeOAuthClient.PrependReactor("get", "oauthaccesstokens", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { @@ -340,27 +341,67 @@ func TestAuthenticateTokenOtherGetError(t *testing.T) { t.Errorf("Unexpected user: %v", userInfo) } } + 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", + }, + ) + userRegistry := usertest.NewUserRegistry() + userRegistry.GetUsers["foo"] = &userapi.User{ObjectMeta: metav1.ObjectMeta{UID: "bar"}} + + tokenAuthenticator := NewTokenAuthenticator(fakeOAuthClient.Oauth().OAuthAccessTokens(), userRegistry, identitymapper.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 TestAuthenticateTokenInvalidUID(t *testing.T) { fakeOAuthClient := oauthfake.NewSimpleClientset( &oapi.OAuthAccessToken{ - ObjectMeta: metav1.ObjectMeta{Name: "token", CreationTimestamp: metav1.Time{Time: time.Now().Add(-1 * time.Hour)}}, + ObjectMeta: metav1.ObjectMeta{Name: "token", CreationTimestamp: metav1.Time{Time: time.Now()}}, ExpiresIn: 600, // 10 minutes + UserName: "foo", + UserUID: string("bar1"), }, ) userRegistry := usertest.NewUserRegistry() - tokenAuthenticator := NewTokenAuthenticator(fakeOAuthClient.Oauth().OAuthAccessTokens(), userRegistry, identitymapper.NoopGroupMapper{}) + userRegistry.GetUsers["foo"] = &userapi.User{ObjectMeta: metav1.ObjectMeta{UID: "bar2"}} + + tokenAuthenticator := NewTokenAuthenticator(fakeOAuthClient.Oauth().OAuthAccessTokens(), userRegistry, identitymapper.NoopGroupMapper{}, NewUIDValidator()) userInfo, found, err := tokenAuthenticator.AuthenticateToken("token") if found { t.Error("Found token, but it should be missing!") } - if err != ErrExpired { + if err.Error() != "user.UID (bar2) does not match token.userUID (bar1)" { 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{ @@ -373,7 +414,7 @@ func TestAuthenticateTokenValidated(t *testing.T) { userRegistry := usertest.NewUserRegistry() userRegistry.GetUsers["foo"] = &userapi.User{ObjectMeta: metav1.ObjectMeta{UID: "bar"}} - tokenAuthenticator := NewTokenAuthenticator(fakeOAuthClient.Oauth().OAuthAccessTokens(), userRegistry, identitymapper.NoopGroupMapper{}) + tokenAuthenticator := NewTokenAuthenticator(fakeOAuthClient.Oauth().OAuthAccessTokens(), userRegistry, identitymapper.NoopGroupMapper{}, NewExpirationValidator(), NewUIDValidator()) userInfo, found, err := tokenAuthenticator.AuthenticateToken("token") if !found { diff --git a/pkg/auth/oauth/registry/tokenauthenticator.go b/pkg/auth/oauth/registry/tokenauthenticator.go index 6f85171d7a70..34dd64eeb2dc 100644 --- a/pkg/auth/oauth/registry/tokenauthenticator.go +++ b/pkg/auth/oauth/registry/tokenauthenticator.go @@ -1,70 +1,61 @@ package registry import ( - "errors" - "fmt" - "time" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + kauthenticator "k8s.io/apiserver/pkg/authentication/authenticator" kuser "k8s.io/apiserver/pkg/authentication/user" + "github.com/openshift/origin/pkg/auth/authenticator" "github.com/openshift/origin/pkg/auth/userregistry/identitymapper" authorizationapi "github.com/openshift/origin/pkg/authorization/apis/authorization" oauthclient "github.com/openshift/origin/pkg/oauth/generated/internalclientset/typed/oauth/internalversion" userclient "github.com/openshift/origin/pkg/user/generated/internalclientset/typed/user/internalversion" ) -type TokenAuthenticator struct { +type tokenAuthenticator struct { tokens oauthclient.OAuthAccessTokenInterface users userclient.UserResourceInterface groupMapper identitymapper.UserToGroupMapper + validators authenticator.OAuthTokenValidator } -var ErrExpired = errors.New("Token is expired") - -func NewTokenAuthenticator(tokens oauthclient.OAuthAccessTokenInterface, users userclient.UserResourceInterface, groupMapper identitymapper.UserToGroupMapper) *TokenAuthenticator { - return &TokenAuthenticator{ +func NewTokenAuthenticator(tokens oauthclient.OAuthAccessTokenInterface, users userclient.UserResourceInterface, groupMapper identitymapper.UserToGroupMapper, validators ...authenticator.OAuthTokenValidator) kauthenticator.Token { + return &tokenAuthenticator{ tokens: tokens, users: users, groupMapper: groupMapper, + validators: authenticator.OAuthTokenValidators(validators), } } -func (a *TokenAuthenticator) AuthenticateToken(value string) (kuser.Info, bool, error) { - token, err := a.tokens.Get(value, metav1.GetOptions{}) +func (a *tokenAuthenticator) AuthenticateToken(name string) (kuser.Info, bool, error) { + token, err := a.tokens.Get(name, metav1.GetOptions{}) if err != nil { return nil, false, err } - if token.ExpiresIn > 0 { - if token.CreationTimestamp.Time.Add(time.Duration(token.ExpiresIn) * time.Second).Before(time.Now()) { - return nil, false, ErrExpired - } - } - if token.DeletionTimestamp != nil { - return nil, false, ErrExpired - } - u, err := a.users.Get(token.UserName, metav1.GetOptions{}) + user, err := a.users.Get(token.UserName, metav1.GetOptions{}) if err != nil { return nil, false, err } - if string(u.UID) != token.UserUID { - return nil, false, fmt.Errorf("user.UID (%s) does not match token.userUID (%s)", u.UID, token.UserUID) + + if err := a.validators.Validate(token, user); err != nil { + return nil, false, err } - groups, err := a.groupMapper.GroupsFor(u.Name) + groups, err := a.groupMapper.GroupsFor(user.Name) if err != nil { return nil, false, err } - groupNames := []string{} + groupNames := make([]string, 0, len(groups)+len(user.Groups)) for _, group := range groups { groupNames = append(groupNames, group.Name) } - groupNames = append(groupNames, u.Groups...) + groupNames = append(groupNames, user.Groups...) return &kuser.DefaultInfo{ - Name: u.Name, - UID: string(u.UID), + Name: user.Name, + UID: string(user.UID), Groups: groupNames, Extra: map[string][]string{ authorizationapi.ScopesKey: token.Scopes, diff --git a/pkg/auth/oauth/registry/uidvalidator.go b/pkg/auth/oauth/registry/uidvalidator.go new file mode 100644 index 000000000000..8b0fd2dc5b85 --- /dev/null +++ b/pkg/auth/oauth/registry/uidvalidator.go @@ -0,0 +1,22 @@ +package registry + +import ( + "fmt" + + "github.com/openshift/origin/pkg/auth/authenticator" + "github.com/openshift/origin/pkg/oauth/apis/oauth" + userapi "github.com/openshift/origin/pkg/user/apis/user" +) + +const errInvalidUIDStr = "user.UID (%s) does not match token.userUID (%s)" + +func NewUIDValidator() authenticator.OAuthTokenValidator { + return authenticator.OAuthTokenValidatorFunc( + func(token *oauth.OAuthAccessToken, user *userapi.User) error { + if string(user.UID) != token.UserUID { + return fmt.Errorf(errInvalidUIDStr, user.UID, token.UserUID) + } + return nil + }, + ) +} diff --git a/pkg/cmd/server/origin/authenticator.go b/pkg/cmd/server/origin/authenticator.go index 1ff582d94e78..de48ef7841e6 100644 --- a/pkg/cmd/server/origin/authenticator.go +++ b/pkg/cmd/server/origin/authenticator.go @@ -20,6 +20,7 @@ import ( sacontroller "k8s.io/kubernetes/pkg/controller/serviceaccount" "k8s.io/kubernetes/pkg/serviceaccount" + openshiftauthenticator "github.com/openshift/origin/pkg/auth/authenticator" "github.com/openshift/origin/pkg/auth/authenticator/request/paramtoken" authnregistry "github.com/openshift/origin/pkg/auth/oauth/registry" "github.com/openshift/origin/pkg/auth/userregistry/identitymapper" @@ -88,7 +89,8 @@ func newAuthenticator(config configapi.MasterConfig, accessTokenGetter oauthclie // OAuth token if config.OAuthConfig != nil { - oauthTokenAuthenticator := authnregistry.NewTokenAuthenticator(accessTokenGetter, userGetter, groupMapper) + validators := []openshiftauthenticator.OAuthTokenValidator{authnregistry.NewExpirationValidator(), authnregistry.NewUIDValidator()} + oauthTokenAuthenticator := authnregistry.NewTokenAuthenticator(accessTokenGetter, userGetter, groupMapper, validators...) tokenAuthenticators = append(tokenAuthenticators, // if you have a bearer token, you're a human (usually) // if you change this, have a look at the impersonationFilter where we attach groups to the impersonated user