kubermatic · kubermatic-bot · Apr 21, 2020 · Apr 21, 2020
diff --git a/api/pkg/handler/routes_v1.go b/api/pkg/handler/routes_v1.go
@@ -607,7 +607,7 @@ func (r Routing) listSSHKeys() http.Handler {
 		endpoint.Chain(
 			middleware.TokenVerifier(r.tokenVerifiers),
 			middleware.UserSaver(r.userProvider),
-		)(ssh.ListEndpoint(r.sshKeyProvider, r.projectProvider, r.userInfoGetter)),
+		)(ssh.ListEndpoint(r.sshKeyProvider, r.projectProvider, r.privilegedProjectProvider, r.userInfoGetter)),
 		ssh.DecodeListReq,
 		encodeJSON,
 		r.defaultServerOptions()...,
@@ -634,7 +634,7 @@ func (r Routing) createSSHKey() http.Handler {
 		endpoint.Chain(
 			middleware.TokenVerifier(r.tokenVerifiers),
 			middleware.UserSaver(r.userProvider),
-		)(ssh.CreateEndpoint(r.sshKeyProvider, r.projectProvider, r.userInfoGetter)),
+		)(ssh.CreateEndpoint(r.sshKeyProvider, r.privilegedSSHKeyProvider, r.projectProvider, r.privilegedProjectProvider, r.userInfoGetter)),
 		ssh.DecodeCreateReq,
 		setStatusCreatedHeader(encodeJSON),
 		r.defaultServerOptions()...,
@@ -658,7 +658,7 @@ func (r Routing) deleteSSHKey() http.Handler {
 		endpoint.Chain(
 			middleware.TokenVerifier(r.tokenVerifiers),
 			middleware.UserSaver(r.userProvider),
-		)(ssh.DeleteEndpoint(r.sshKeyProvider, r.projectProvider, r.userInfoGetter)),
+		)(ssh.DeleteEndpoint(r.sshKeyProvider, r.privilegedSSHKeyProvider, r.projectProvider, r.privilegedProjectProvider, r.userInfoGetter)),
 		ssh.DecodeDeleteReq,
 		encodeJSON,
 		r.defaultServerOptions()...,

diff --git a/api/pkg/handler/v1/ssh/ssh.go b/api/pkg/handler/v1/ssh/ssh.go
@@ -10,22 +10,20 @@ import (
 	"github.com/gorilla/mux"
 
 	apiv1 "github.com/kubermatic/kubermatic/api/pkg/api/v1"
+	kubermaticv1 "github.com/kubermatic/kubermatic/api/pkg/crd/kubermatic/v1"
 	"github.com/kubermatic/kubermatic/api/pkg/handler/v1/common"
 	"github.com/kubermatic/kubermatic/api/pkg/provider"
 	"github.com/kubermatic/kubermatic/api/pkg/util/errors"
 )
 
-func CreateEndpoint(keyProvider provider.SSHKeyProvider, projectProvider provider.ProjectProvider, userInfoGetter provider.UserInfoGetter) endpoint.Endpoint {
+func CreateEndpoint(keyProvider provider.SSHKeyProvider, privilegedSSHKeyProvider provider.PrivilegedSSHKeyProvider, projectProvider provider.ProjectProvider, privilegedProjectProvider provider.PrivilegedProjectProvider, userInfoGetter provider.UserInfoGetter) endpoint.Endpoint {
 	return func(ctx context.Context, request interface{}) (interface{}, error) {
 		req, ok := request.(CreateReq)
 		if !ok {
 			return nil, errors.NewBadRequest("invalid request")
 		}
-		userInfo, err := userInfoGetter(ctx, req.ProjectID)
-		if err != nil {
-			return nil, common.KubernetesErrorToHTTPError(err)
-		}
-		project, err := projectProvider.Get(userInfo, req.ProjectID, &provider.ProjectGetOptions{})
+
+		project, err := common.GetProject(ctx, userInfoGetter, projectProvider, privilegedProjectProvider, req.ProjectID)
 		if err != nil {
 			return nil, common.KubernetesErrorToHTTPError(err)
 		}
@@ -38,7 +36,7 @@ func CreateEndpoint(keyProvider provider.SSHKeyProvider, projectProvider provide
 			return nil, errors.NewAlreadyExists("ssh key", req.Key.Name)
 		}
 
-		key, err := keyProvider.Create(userInfo, project, req.Key.Name, req.Key.Spec.PublicKey)
+		key, err := createUserSSHKey(ctx, userInfoGetter, keyProvider, privilegedSSHKeyProvider, project, req.Key.Name, req.Key.Spec.PublicKey)
 		if err != nil {
 			return nil, common.KubernetesErrorToHTTPError(err)
 		}
@@ -58,30 +56,54 @@ func CreateEndpoint(keyProvider provider.SSHKeyProvider, projectProvider provide
 	}
 }
 
-func DeleteEndpoint(keyProvider provider.SSHKeyProvider, projectProvider provider.ProjectProvider, userInfoGetter provider.UserInfoGetter) endpoint.Endpoint {
+func createUserSSHKey(ctx context.Context, userInfoGetter provider.UserInfoGetter, keyProvider provider.SSHKeyProvider, privilegedSSHKeyProvider provider.PrivilegedSSHKeyProvider, project *kubermaticv1.Project, keyName, pubKey string) (*kubermaticv1.UserSSHKey, error) {
+	adminUserInfo, err := userInfoGetter(ctx, "")
+	if err != nil {
+		return nil, err
+	}
+	if adminUserInfo.IsAdmin {
+		return privilegedSSHKeyProvider.CreateUnsecured(project, keyName, pubKey)
+	}
+	userInfo, err := userInfoGetter(ctx, project.Name)
+	if err != nil {
+		return nil, err
+	}
+	return keyProvider.Create(userInfo, project, keyName, pubKey)
+}
+
+func DeleteEndpoint(keyProvider provider.SSHKeyProvider, privilegedSSHKeyProvider provider.PrivilegedSSHKeyProvider, projectProvider provider.ProjectProvider, privilegedProjectProvider provider.PrivilegedProjectProvider, userInfoGetter provider.UserInfoGetter) endpoint.Endpoint {
 	return func(ctx context.Context, request interface{}) (interface{}, error) {
 		req, ok := request.(DeleteReq)
 		if !ok {
 			return nil, errors.NewBadRequest("invalid request")
 		}
-		userInfo, err := userInfoGetter(ctx, req.ProjectID)
-		if err != nil {
-			return nil, common.KubernetesErrorToHTTPError(err)
-		}
-		_, err = projectProvider.Get(userInfo, req.ProjectID, &provider.ProjectGetOptions{})
+		project, err := common.GetProject(ctx, userInfoGetter, projectProvider, privilegedProjectProvider, req.ProjectID)
 		if err != nil {
 			return nil, common.KubernetesErrorToHTTPError(err)
 		}
-
-		err = keyProvider.Delete(userInfo, req.SSHKeyID)
-		if err != nil {
+		if err := deleteUserSSHKey(ctx, userInfoGetter, keyProvider, privilegedSSHKeyProvider, project, req.SSHKeyID); err != nil {
 			return nil, common.KubernetesErrorToHTTPError(err)
 		}
 		return nil, nil
 	}
 }
 
-func ListEndpoint(keyProvider provider.SSHKeyProvider, projectProvider provider.ProjectProvider, userInfoGetter provider.UserInfoGetter) endpoint.Endpoint {
+func deleteUserSSHKey(ctx context.Context, userInfoGetter provider.UserInfoGetter, keyProvider provider.SSHKeyProvider, privilegedSSHKeyProvider provider.PrivilegedSSHKeyProvider, project *kubermaticv1.Project, keyName string) error {
+	adminUserInfo, err := userInfoGetter(ctx, "")
+	if err != nil {
+		return err
+	}
+	if adminUserInfo.IsAdmin {
+		return privilegedSSHKeyProvider.DeleteUnsecured(keyName)
+	}
+	userInfo, err := userInfoGetter(ctx, project.Name)
+	if err != nil {
+		return err
+	}
+	return keyProvider.Delete(userInfo, keyName)
+}
+
+func ListEndpoint(keyProvider provider.SSHKeyProvider, projectProvider provider.ProjectProvider, privilegedProjectProvider provider.PrivilegedProjectProvider, userInfoGetter provider.UserInfoGetter) endpoint.Endpoint {
 	return func(ctx context.Context, request interface{}) (interface{}, error) {
 		req, ok := request.(ListReq)
 		if !ok {
@@ -92,11 +114,7 @@ func ListEndpoint(keyProvider provider.SSHKeyProvider, projectProvider provider.
 			return nil, errors.NewBadRequest("the name of the project to delete cannot be empty")
 		}
 
-		userInfo, err := userInfoGetter(ctx, req.ProjectID)
-		if err != nil {
-			return nil, common.KubernetesErrorToHTTPError(err)
-		}
-		project, err := projectProvider.Get(userInfo, req.ProjectID, &provider.ProjectGetOptions{})
+		project, err := common.GetProject(ctx, userInfoGetter, projectProvider, privilegedProjectProvider, req.ProjectID)
 		if err != nil {
 			return nil, common.KubernetesErrorToHTTPError(err)
 		}

diff --git a/api/pkg/handler/v1/ssh/ssh_test.go b/api/pkg/handler/v1/ssh/ssh_test.go
@@ -49,6 +49,48 @@ func TestDeleteSSHKey(t *testing.T) {
 			},
 			ExistingAPIUser: test.GenAPIUser("john", "john@acme.com"),
 		},
+		// scenario 2
+		{
+			Name:           "scenario 2: the admin user can delete SSH key from any project",
+			HTTPStatus:     http.StatusOK,
+			SSHKeyToDelete: "key-abc-second-key",
+			ExistingKubermaticObjs: []runtime.Object{
+				/*add projects*/
+				test.GenProject("my-first-project", kubermaticv1.ProjectActive, test.DefaultCreationTimestamp()),
+				/*add bindings*/
+				test.GenBinding("my-first-project-ID", "john@acme.com", "owners"),
+				/*add users*/
+				test.GenUser("", "john", "john@acme.com"),
+				genUser("admin", "admin@acme.com", true),
+				/*add cluster*/
+				test.GenDefaultCluster(),
+				/*add ssh keys*/
+				genSSHKey(test.DefaultCreationTimestamp(), "c08aa5c7abf34504f18552846485267d", "first-key", "my-first-project-ID", test.GenDefaultCluster().Name),
+				genSSHKey(test.DefaultCreationTimestamp(), "abc", "second-key", "my-first-project-ID", "abcd-ID"),
+			},
+			ExistingAPIUser: test.GenAPIUser("admin", "admin@acme.com"),
+		},
+		// scenario 3
+		{
+			Name:           "scenario 3: the user who doesn't belong to the project can not delete SSH key from the project",
+			HTTPStatus:     http.StatusForbidden,
+			SSHKeyToDelete: "key-abc-second-key",
+			ExistingKubermaticObjs: []runtime.Object{
+				/*add projects*/
+				test.GenProject("my-first-project", kubermaticv1.ProjectActive, test.DefaultCreationTimestamp()),
+				/*add bindings*/
+				test.GenBinding("my-first-project-ID", "john@acme.com", "owners"),
+				/*add users*/
+				test.GenUser("", "john", "john@acme.com"),
+				genUser("user", "user@acme.com", false),
+				/*add cluster*/
+				test.GenDefaultCluster(),
+				/*add ssh keys*/
+				genSSHKey(test.DefaultCreationTimestamp(), "c08aa5c7abf34504f18552846485267d", "first-key", "my-first-project-ID", test.GenDefaultCluster().Name),
+				genSSHKey(test.DefaultCreationTimestamp(), "abc", "second-key", "my-first-project-ID", "abcd-ID"),
+			},
+			ExistingAPIUser: test.GenAPIUser("user", "user@acme.com"),
+		},
 	}
 
 	for _, tc := range testcases {
@@ -148,6 +190,43 @@ func TestListSSHKeys(t *testing.T) {
 			},
 			ExistingAPIUser: test.GenAPIUser("john", "john@acme.com"),
 		},
+		// scenario 2
+		{
+			Name: "scenario 2: the admin can gets a list of ssh keys assigned to cluster for any project",
+			Body: ``,
+			ExpectedKeys: []apiv1.SSHKey{
+				{
+					ObjectMeta: apiv1.ObjectMeta{
+						ID:                "key-c08aa5c7abf34504f18552846485267d-first-key",
+						Name:              "first-key",
+						CreationTimestamp: apiv1.Date(2013, 02, 03, 19, 54, 0, 0, time.UTC),
+					},
+				},
+				{
+					ObjectMeta: apiv1.ObjectMeta{
+						ID:                "key-abc-second-key",
+						Name:              "second-key",
+						CreationTimestamp: apiv1.Date(2013, 02, 03, 19, 55, 0, 0, time.UTC),
+					},
+				},
+			},
+			HTTPStatus: http.StatusOK,
+			ExistingKubermaticObjs: []runtime.Object{
+				/*add projects*/
+				test.GenProject("my-first-project", kubermaticv1.ProjectActive, test.DefaultCreationTimestamp()),
+				/*add bindings*/
+				test.GenBinding("my-first-project-ID", "john@acme.com", "owners"),
+				/*add users*/
+				test.GenUser("", "john", "john@acme.com"),
+				/*add cluster*/
+				test.GenDefaultCluster(),
+				genUser("admin", "admin@acme.com", true),
+				/*add ssh keys*/
+				genSSHKey(creationTime, "c08aa5c7abf34504f18552846485267d", "first-key", "my-first-project-ID", test.GenDefaultCluster().Name),
+				genSSHKey(creationTime.Add(time.Minute), "abc", "second-key", "my-first-project-ID", "abcd-ID"),
+			},
+			ExistingAPIUser: test.GenAPIUser("admin", "admin@acme.com"),
+		},
 	}
 
 	for _, tc := range testcases {
@@ -231,6 +310,27 @@ func TestCreateSSHKeysEndpoint(t *testing.T) {
 			},
 			ExistingAPIUser: test.GenAPIUser("john", "john@acme.com"),
 		},
+		// scenario 3
+		{
+			Name:             "scenario 3: the admin can create ssh key that will be assigned to any project",
+			Body:             `{"name":"my-second-ssh-key","spec":{"publicKey":"ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAACAQC8LlXSRW4HUYAjzx1+r5JzpjXIDDyFkWZzBQ8aU14J8LdMyQsU6/ZKuO5IKoWWVoPi0e63qSjkXPTjnUAwpE62hDm6uLaPgIlc3ND+8d9xbItS+gyXk9TSkC3emrsCWpS76W3KjLwyz5euIfnMCQZSASM7F5CrNg6XSppOgRWlyY09VEKi9PmvEDKCy5JNt6afcUzB3rAOK3SYZ0BYDyrVjuqTcMZwRodryxKb/jxDS+qQNplBNuUBqUzqjuKyI5oAk+aVTYIfTwgBTQyZT7So/u70gSDbRp9uHI05PkH60IftAHdYu4TJTmCwJxLW/suOEx3PPvIsUP14XQUZgmDJEuIuWDlsvfOo9DXZNnl832SGvTyhclBpsauWJ1OwOllT+hlM7u8dwcb70GD/OzCG7RSEatVoiNtg4XdeUf4kiqqzKZEqpopHQqwVKMhlhPKKulY0vrtetJxaLokEwPOYyycxlXsNBK2ei/IbGan+uI39v0s30ySWKzr+M9z0QlLAG7rjgCSWFSmy+Ez2fxU5HQQTNCep8+VjNeI79uO9VDJ8qvV/y6fDtrwgl67hUgDcHyv80TzVROTGFBMCP7hyswArT0GxpL9q7PjPU92D43UEDY5YNOZN2A976O5jd4bPrWp0mKsye1BhLrct16Xdn9x68D8nS2T1uSSWovFhkQ== lukasz@loodse.com "}}`,
+			RewriteSSHKeyID:  true,
+			ExpectedResponse: `{"id":"%s","name":"my-second-ssh-key","creationTimestamp":"0001-01-01T00:00:00Z","spec":{"fingerprint":"c0:8a:a5:c7:ab:f3:45:04:f1:85:52:84:64:85:26:7d","publicKey":"ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAACAQC8LlXSRW4HUYAjzx1+r5JzpjXIDDyFkWZzBQ8aU14J8LdMyQsU6/ZKuO5IKoWWVoPi0e63qSjkXPTjnUAwpE62hDm6uLaPgIlc3ND+8d9xbItS+gyXk9TSkC3emrsCWpS76W3KjLwyz5euIfnMCQZSASM7F5CrNg6XSppOgRWlyY09VEKi9PmvEDKCy5JNt6afcUzB3rAOK3SYZ0BYDyrVjuqTcMZwRodryxKb/jxDS+qQNplBNuUBqUzqjuKyI5oAk+aVTYIfTwgBTQyZT7So/u70gSDbRp9uHI05PkH60IftAHdYu4TJTmCwJxLW/suOEx3PPvIsUP14XQUZgmDJEuIuWDlsvfOo9DXZNnl832SGvTyhclBpsauWJ1OwOllT+hlM7u8dwcb70GD/OzCG7RSEatVoiNtg4XdeUf4kiqqzKZEqpopHQqwVKMhlhPKKulY0vrtetJxaLokEwPOYyycxlXsNBK2ei/IbGan+uI39v0s30ySWKzr+M9z0QlLAG7rjgCSWFSmy+Ez2fxU5HQQTNCep8+VjNeI79uO9VDJ8qvV/y6fDtrwgl67hUgDcHyv80TzVROTGFBMCP7hyswArT0GxpL9q7PjPU92D43UEDY5YNOZN2A976O5jd4bPrWp0mKsye1BhLrct16Xdn9x68D8nS2T1uSSWovFhkQ== lukasz@loodse.com "}}`,
+			HTTPStatus:       http.StatusCreated,
+			ExistingProject:  test.GenProject("my-first-project", kubermaticv1.ProjectActive, test.DefaultCreationTimestamp()),
+			ExistingKubermaticObjs: []runtime.Object{
+				/*add projects*/
+				test.GenProject("my-first-project", kubermaticv1.ProjectActive, test.DefaultCreationTimestamp()),
+				/*add bindings*/
+				test.GenBinding("my-first-project-ID", "john@acme.com", "owners"),
+				/*add users*/
+				test.GenUser("", "john", "john@acme.com"),
+				genUser("admin", "admin@acme.com", true),
+				/*add cluster*/
+				test.GenDefaultCluster(),
+			},
+			ExistingAPIUser: test.GenAPIUser("admin", "admin@acme.com"),
+		},
 	}
 
 	for _, tc := range testcases {
@@ -286,3 +386,9 @@ func genSSHKey(creationTime time.Time, keyID string, keyName string, projectID s
 		},
 	}
 }
+
+func genUser(name, email string, isAdmin bool) *kubermaticv1.User {
+	user := test.GenUser("", name, email)
+	user.Spec.IsAdmin = isAdmin
+	return user
+}
diff --git a/api/pkg/provider/kubernetes/ssh.go b/api/pkg/provider/kubernetes/ssh.go
@@ -65,14 +65,45 @@ func (p *SSHKeyProvider) Create(userInfo *provider.UserInfo, project *kubermatic
 		return nil, errors.New("a userInfo is missing but required")
 	}
 
+	sshKey, err := genUserSSHKey(project, keyName, pubKey)
+	if err != nil {
+		return nil, err
+	}
+
+	masterImpersonatedClient, err := createKubermaticImpersonationClientWrapperFromUserInfo(userInfo, p.createMasterImpersonatedClient)
+	if err != nil {
+		return nil, err
+	}
+	return masterImpersonatedClient.UserSSHKeys().Create(sshKey)
+}
+
+// Create creates a ssh key that belongs to the given project
+// This function is unsafe in a sense that it uses privileged account to create the ssh key
+func (p *PrivilegedSSHKeyProvider) CreateUnsecured(project *kubermaticapiv1.Project, keyName, pubKey string) (*kubermaticapiv1.UserSSHKey, error) {
+	if keyName == "" {
+		return nil, fmt.Errorf("the ssh key name is missing but required")
+	}
+	if pubKey == "" {
+		return nil, fmt.Errorf("the ssh public part of the key is missing but required")
+	}
+
+	sshKey, err := genUserSSHKey(project, keyName, pubKey)
+	if err != nil {
+		return nil, err
+	}
+
+	return p.clientPrivileged.Create(sshKey)
+}
+
+func genUserSSHKey(project *kubermaticapiv1.Project, keyName, pubKey string) (*kubermaticapiv1.UserSSHKey, error) {
 	pubKeyParsed, _, _, _, err := ssh.ParseAuthorizedKey([]byte(pubKey))
 	if err != nil {
 		return nil, fmt.Errorf("the provided ssh key is invalid due to = %v", err)
 	}
 	sshKeyHash := ssh.FingerprintLegacyMD5(pubKeyParsed)
 
 	keyInternalName := fmt.Sprintf("key-%s-%s", strings.NewReplacer(":", "").Replace(sshKeyHash), uuid.ShortUID(4))
-	sshKey := &kubermaticapiv1.UserSSHKey{
+	return &kubermaticapiv1.UserSSHKey{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: keyInternalName,
 			OwnerReferences: []metav1.OwnerReference{
@@ -90,13 +121,7 @@ func (p *SSHKeyProvider) Create(userInfo *provider.UserInfo, project *kubermatic
 			Name:        keyName,
 			Clusters:    []string{},
 		},
-	}
-
-	masterImpersonatedClient, err := createKubermaticImpersonationClientWrapperFromUserInfo(userInfo, p.createMasterImpersonatedClient)
-	if err != nil {
-		return nil, err
-	}
-	return masterImpersonatedClient.UserSSHKeys().Create(sshKey)
+	}, nil
 }
 
 // List gets a list of ssh keys, by default it will get all the keys that belong to the given project.
@@ -173,6 +198,12 @@ func (p *SSHKeyProvider) Delete(userInfo *provider.UserInfo, keyName string) err
 	return masterImpersonatedClient.UserSSHKeys().Delete(keyName, &metav1.DeleteOptions{})
 }
 
+// Delete deletes the given ssh key
+// This function is unsafe in a sense that it uses privileged account to delete the ssh key
+func (p *PrivilegedSSHKeyProvider) DeleteUnsecured(keyName string) error {
+	return p.clientPrivileged.Delete(keyName, &metav1.DeleteOptions{})
+}
+
 // Update simply updates the given key
 func (p *SSHKeyProvider) Update(userInfo *provider.UserInfo, newKey *kubermaticapiv1.UserSSHKey) (*kubermaticapiv1.UserSSHKey, error) {
 	masterImpersonatedClient, err := createKubermaticImpersonationClientWrapperFromUserInfo(userInfo, p.createMasterImpersonatedClient)

diff --git a/api/pkg/provider/types.go b/api/pkg/provider/types.go
@@ -241,6 +241,14 @@ type PrivilegedSSHKeyProvider interface {
 	// UpdateUnsecured update a specific ssh key and returns the updated ssh key
 	// This function is unsafe in a sense that it uses privileged account to update the ssh key
 	UpdateUnsecured(sshKey *kubermaticv1.UserSSHKey) (*kubermaticv1.UserSSHKey, error)
+
+	// Create creates a ssh key that belongs to the given project
+	// This function is unsafe in a sense that it uses privileged account to create the ssh key
+	CreateUnsecured(project *kubermaticv1.Project, keyName, pubKey string) (*kubermaticv1.UserSSHKey, error)
+
+	// Delete deletes the given ssh key
+	// This function is unsafe in a sense that it uses privileged account to delete the ssh key
+	DeleteUnsecured(keyName string) error
 }
 
 // UserProvider declares the set of methods for interacting with kubermatic users

diff --git a/api/pkg/test/e2e/api/sa_test.go b/api/pkg/test/e2e/api/sa_test.go
@@ -123,7 +123,7 @@ func TestTokenAccessForProject(t *testing.T) {
 			if err != nil {
 				t.Fatalf("can not create project due error: %v", GetErrorResponse(err))
 			}
-			teardownNotOwnedProject := cleanUpProject(notOwnedProject.ID, 1)
+			teardownNotOwnedProject := cleanUpProject(notOwnedProject.ID, 3)
 			defer teardownNotOwnedProject(t)
 
 			_, err = apiRunnerWithSAToken.GetProject(notOwnedProject.ID, 1)