From 58e90cd541516f88c0104efd2aa5270eaccccf56 Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Tue, 17 May 2022 11:15:03 +0000 Subject: [PATCH 1/6] Add GetOwnerToken method to APIoverJSONRPC --- .../gitpod-protocol/go/gitpod-service.go | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/components/gitpod-protocol/go/gitpod-service.go b/components/gitpod-protocol/go/gitpod-service.go index e544dbbef79347..97cce8540461f9 100644 --- a/components/gitpod-protocol/go/gitpod-service.go +++ b/components/gitpod-protocol/go/gitpod-service.go @@ -23,6 +23,7 @@ import ( // APIInterface wraps the type APIInterface interface { + GetOwnerToken(ctx context.Context, workspaceID string) (res string, err error) AdminBlockUser(ctx context.Context, req *AdminBlockUserRequest) (err error) GetLoggedInUser(ctx context.Context) (res *User, err error) UpdateLoggedInUser(ctx context.Context, user *User) (res *User, err error) @@ -85,6 +86,8 @@ type APIInterface interface { type FunctionName string const ( + // FunctionGetOwnerToken is the name of the getOwnerToken function + FunctionGetOwnerToken FunctionName = "getOwnerToken" // FunctionAdminBlockUser is the name of the adminBlockUser function FunctionAdminBlockUser FunctionName = "adminBlockUser" // FunctionGetLoggedInUser is the name of the getLoggedInUser function @@ -337,6 +340,23 @@ func (gp *APIoverJSONRPC) handler(ctx context.Context, conn *jsonrpc2.Conn, req return } +func (gp *APIoverJSONRPC) GetOwnerToken(ctx context.Context, workspaceID string) (res string, err error) { + if gp == nil { + err = errNotConnected + return + } + var _params []interface{} + _params = append(_params, workspaceID) + + var _result string + err = gp.C.Call(ctx, "getOwnerToken", _params, &_result) + if err != nil { + return "", err + } + res = _result + return +} + // AdminBlockUser calls adminBlockUser on the server func (gp *APIoverJSONRPC) AdminBlockUser(ctx context.Context, message *AdminBlockUserRequest) (err error) { if gp == nil { From c35e6e917471532954f60f2d43821af39237d24c Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Tue, 17 May 2022 11:24:40 +0000 Subject: [PATCH 2/6] Implement GetOwnerToken rpc Replace the stub implementation with a real one that invokes the server JSON rpc API. --- .../public-api-server/pkg/apiv1/workspace.go | 29 ++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/components/public-api-server/pkg/apiv1/workspace.go b/components/public-api-server/pkg/apiv1/workspace.go index 09899559273b3f..6b94a364d5300c 100644 --- a/components/public-api-server/pkg/apiv1/workspace.go +++ b/components/public-api-server/pkg/apiv1/workspace.go @@ -72,7 +72,34 @@ func (w *WorkspaceService) GetWorkspace(ctx context.Context, r *v1.GetWorkspaceR } func (w *WorkspaceService) GetOwnerToken(ctx context.Context, r *v1.GetOwnerTokenRequest) (*v1.GetOwnerTokenResponse, error) { - return &v1.GetOwnerTokenResponse{Token: "some-owner-token"}, nil + logger := ctxlogrus.Extract(ctx) + token, err := bearerTokenFromContext(ctx) + if err != nil { + return nil, err + } + + server, err := w.connectionPool.Get(ctx, token) + if err != nil { + logger.WithError(err).Error("Failed to get connection to server.") + return nil, status.Error(codes.Internal, "failed to establish connection to downstream services") + } + + ownerToken, err := server.GetOwnerToken(ctx, r.GetWorkspaceId()) + + if err != nil { + logger.WithError(err).Error("Failed to get owner token.") + converted := proxy.ConvertError(err) + switch status.Code(converted) { + case codes.PermissionDenied: + return nil, status.Error(codes.PermissionDenied, "insufficient permission to retrieve ownertoken") + case codes.NotFound: + return nil, status.Error(codes.NotFound, "workspace does not exist") + default: + return nil, status.Error(codes.Internal, "unable to retrieve owner token") + } + } + + return &v1.GetOwnerTokenResponse{Token: ownerToken}, nil } func bearerTokenFromContext(ctx context.Context) (string, error) { From a058b3fde0f83c1ec439fa1afcfdc8ebff788f91 Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Tue, 17 May 2022 11:34:29 +0000 Subject: [PATCH 3/6] Generate mocks Via `go generate components/gitpod-protocol/go/gitpod-service.go` --- components/gitpod-protocol/go/mock.go | 87 ++++++++++++++++----------- 1 file changed, 51 insertions(+), 36 deletions(-) diff --git a/components/gitpod-protocol/go/mock.go b/components/gitpod-protocol/go/mock.go index 25ab5d9a14655b..6f016632438878 100644 --- a/components/gitpod-protocol/go/mock.go +++ b/components/gitpod-protocol/go/mock.go @@ -1,4 +1,4 @@ -// Copyright (c) 2021 Gitpod GmbH. All rights reserved. +// Copyright (c) 2022 Gitpod GmbH. All rights reserved. // Licensed under the GNU Affero General Public License (AGPL). // See License-AGPL.txt in the project root for license information. @@ -285,6 +285,21 @@ func (mr *MockAPIInterfaceMockRecorder) GetFeaturedRepositories(ctx interface{}) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetFeaturedRepositories", reflect.TypeOf((*MockAPIInterface)(nil).GetFeaturedRepositories), ctx) } +// GetGitpodTokenScopes mocks base method. +func (m *MockAPIInterface) GetGitpodTokenScopes(ctx context.Context, tokenHash string) ([]string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetGitpodTokenScopes", ctx, tokenHash) + ret0, _ := ret[0].([]string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetGitpodTokenScopes indicates an expected call of GetGitpodTokenScopes. +func (mr *MockAPIInterfaceMockRecorder) GetGitpodTokenScopes(ctx, tokenHash interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetGitpodTokenScopes", reflect.TypeOf((*MockAPIInterface)(nil).GetGitpodTokenScopes), ctx, tokenHash) +} + // GetGitpodTokens mocks base method. func (m *MockAPIInterface) GetGitpodTokens(ctx context.Context) ([]*APIToken, error) { m.ctrl.T.Helper() @@ -360,6 +375,21 @@ func (mr *MockAPIInterfaceMockRecorder) GetOwnAuthProviders(ctx interface{}) *go return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetOwnAuthProviders", reflect.TypeOf((*MockAPIInterface)(nil).GetOwnAuthProviders), ctx) } +// GetOwnerToken mocks base method. +func (m *MockAPIInterface) GetOwnerToken(ctx context.Context, workspaceID string) (string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetOwnerToken", ctx, workspaceID) + ret0, _ := ret[0].(string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetOwnerToken indicates an expected call of GetOwnerToken. +func (mr *MockAPIInterfaceMockRecorder) GetOwnerToken(ctx, workspaceID interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetOwnerToken", reflect.TypeOf((*MockAPIInterface)(nil).GetOwnerToken), ctx, workspaceID) +} + // GetPortAuthenticationToken mocks base method. func (m *MockAPIInterface) GetPortAuthenticationToken(ctx context.Context, workspaceID string) (*Token, error) { m.ctrl.T.Helper() @@ -390,21 +420,6 @@ func (mr *MockAPIInterfaceMockRecorder) GetSnapshots(ctx, workspaceID interface{ return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSnapshots", reflect.TypeOf((*MockAPIInterface)(nil).GetSnapshots), ctx, workspaceID) } -// GetGitpodTokenScopes indicates an expected call of GetGitpodTokenScopes. -func (mr *MockAPIInterfaceMockRecorder) GetGitpodTokenScopes(ctx, tokenHash interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetGitpodTokenScopes", reflect.TypeOf((*MockAPIInterface)(nil).GetGitpodTokenScopes), ctx, tokenHash) -} - -// GetGitpodTokenScopes mocks base method. -func (m *MockAPIInterface) GetGitpodTokenScopes(ctx context.Context, tokenHash string) ([]string, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetGitpodTokenScopes", ctx, tokenHash) - ret0, _ := ret[0].([]string) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - // GetToken mocks base method. func (m *MockAPIInterface) GetToken(ctx context.Context, query *GetTokenSearchOptions) (*Token, error) { m.ctrl.T.Helper() @@ -525,20 +540,6 @@ func (mr *MockAPIInterfaceMockRecorder) GuessGitTokenScopes(ctx, params interfac return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GuessGitTokenScopes", reflect.TypeOf((*MockAPIInterface)(nil).GuessGitTokenScopes), ctx, params) } -// TrackEvent indicates an expected call of TrackEvent. -func (m *MockAPIInterface) TrackEvent(ctx context.Context, params *RemoteTrackMessage) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "TrackEvent", ctx, params) - ret0, _ := ret[0].(error) - return ret0 -} - -// TrackEvent indicates an expected call of TrackEvent. -func (mr *MockAPIInterfaceMockRecorder) TrackEvent(ctx, params interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GuessGitTokenScopes", reflect.TypeOf((*MockAPIInterface)(nil).TrackEvent), ctx, params) -} - // HasPermission mocks base method. func (m *MockAPIInterface) HasPermission(ctx context.Context, permission *PermissionName) (bool, error) { m.ctrl.T.Helper() @@ -758,18 +759,18 @@ func (mr *MockAPIInterfaceMockRecorder) TakeSnapshot(ctx, options interface{}) * return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "TakeSnapshot", reflect.TypeOf((*MockAPIInterface)(nil).TakeSnapshot), ctx, options) } -// WaitForSnapshot mocks base method. -func (m *MockAPIInterface) WaitForSnapshot(ctx context.Context, snapshotId string) error { +// TrackEvent mocks base method. +func (m *MockAPIInterface) TrackEvent(ctx context.Context, event *RemoteTrackMessage) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "WaitForSnapshot", ctx, snapshotId) + ret := m.ctrl.Call(m, "TrackEvent", ctx, event) ret0, _ := ret[0].(error) return ret0 } -// WaitForSnapshot indicates an expected call of WaitForSnapshot. -func (mr *MockAPIInterfaceMockRecorder) WaitForSnapshot(ctx, options interface{}) *gomock.Call { +// TrackEvent indicates an expected call of TrackEvent. +func (mr *MockAPIInterfaceMockRecorder) TrackEvent(ctx, event interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "WaitForSnapshot", reflect.TypeOf((*MockAPIInterface)(nil).WaitForSnapshot), ctx, options) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "TrackEvent", reflect.TypeOf((*MockAPIInterface)(nil).TrackEvent), ctx, event) } // UpdateLoggedInUser mocks base method. @@ -829,6 +830,20 @@ func (mr *MockAPIInterfaceMockRecorder) UpdateWorkspaceUserPin(ctx, id, action i return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateWorkspaceUserPin", reflect.TypeOf((*MockAPIInterface)(nil).UpdateWorkspaceUserPin), ctx, id, action) } +// WaitForSnapshot mocks base method. +func (m *MockAPIInterface) WaitForSnapshot(ctx context.Context, snapshotId string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "WaitForSnapshot", ctx, snapshotId) + ret0, _ := ret[0].(error) + return ret0 +} + +// WaitForSnapshot indicates an expected call of WaitForSnapshot. +func (mr *MockAPIInterfaceMockRecorder) WaitForSnapshot(ctx, snapshotId interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "WaitForSnapshot", reflect.TypeOf((*MockAPIInterface)(nil).WaitForSnapshot), ctx, snapshotId) +} + // WatchWorkspaceImageBuildLogs mocks base method. func (m *MockAPIInterface) WatchWorkspaceImageBuildLogs(ctx context.Context, workspaceID string) error { m.ctrl.T.Helper() From 093392982567571323d564fccaed12da5041ea2c Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Tue, 17 May 2022 11:42:44 +0000 Subject: [PATCH 4/6] Add method to FakeGitpodAPI --- components/public-api-server/pkg/apiv1/workspace_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/components/public-api-server/pkg/apiv1/workspace_test.go b/components/public-api-server/pkg/apiv1/workspace_test.go index 03a5ba04b6afc4..47ef6b7e8e2905 100644 --- a/components/public-api-server/pkg/apiv1/workspace_test.go +++ b/components/public-api-server/pkg/apiv1/workspace_test.go @@ -157,6 +157,10 @@ func (f *FakeGitpodAPI) GetWorkspace(ctx context.Context, id string) (res *gitpo return w, nil } +func (f *FakeGitpodAPI) GetOwnerToken(ctx context.Context, workspaceID string) (res string, err error) { + panic("implement me") +} + func (f *FakeGitpodAPI) AdminBlockUser(ctx context.Context, req *gitpod.AdminBlockUserRequest) (err error) { panic("implement me") } From 5ba8499b82bb9d8cc76d941593990b62c2d6570d Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Tue, 17 May 2022 14:38:54 +0000 Subject: [PATCH 5/6] Implement test for GetOwnerToken --- .../pkg/apiv1/workspace_test.go | 67 +++++++++++++++++-- 1 file changed, 60 insertions(+), 7 deletions(-) diff --git a/components/public-api-server/pkg/apiv1/workspace_test.go b/components/public-api-server/pkg/apiv1/workspace_test.go index 47ef6b7e8e2905..e7b3ba7369ebef 100644 --- a/components/public-api-server/pkg/apiv1/workspace_test.go +++ b/components/public-api-server/pkg/apiv1/workspace_test.go @@ -116,11 +116,21 @@ func TestWorkspaceService_GetWorkspace(t *testing.T) { } func TestWorkspaceService_GetOwnerToken(t *testing.T) { + const ( + bearerToken = "bearer-token-for-tests" + foundWorkspaceID = "easycz-seer-xl8o1zacpyw" + ownerToken = "some-owner-token" + ) + srv := baseserver.NewForTests(t, baseserver.WithGRPC(baseserver.MustUseRandomLocalAddress(t)), ) - var connPool *FakeServerConnPool + connPool := &FakeServerConnPool{ + api: &FakeGitpodAPI{ + ownertokens: map[string]string{foundWorkspaceID: ownerToken}, + }, + } v1.RegisterWorkspacesServiceServer(srv.GRPC(), NewWorkspaceService(connPool)) baseserver.StartServerForTests(t, srv) @@ -128,12 +138,50 @@ func TestWorkspaceService_GetOwnerToken(t *testing.T) { require.NoError(t, err) client := v1.NewWorkspacesServiceClient(conn) - ctx := context.Background() + ctx := metadata.AppendToOutgoingContext(context.Background(), "authorization", bearerToken) - actualOwnerId, err := client.GetOwnerToken(ctx, &v1.GetOwnerTokenRequest{WorkspaceId: "some-workspace-id"}) - require.NoError(t, err) + type Expectation struct { + Code codes.Code + Response *v1.GetOwnerTokenResponse + } + + scenarios := []struct { + name string + WorkspaceID string + Expect Expectation + }{ + { + name: "returns an owner token when workspace is found by ID", + WorkspaceID: foundWorkspaceID, + Expect: Expectation{ + Code: codes.OK, + Response: &v1.GetOwnerTokenResponse{ + Token: ownerToken, + }, + }, + }, + { + name: "not found when workspace is not found by ID", + WorkspaceID: "some-not-found-workspace-id", + Expect: Expectation{ + Code: codes.NotFound, + }, + }, + } - require.Equal(t, "some-owner-token", actualOwnerId.Token) + for _, scenario := range scenarios { + t.Run(scenario.name, func(t *testing.T) { + resp, err := client.GetOwnerToken(ctx, &v1.GetOwnerTokenRequest{ + WorkspaceId: scenario.WorkspaceID, + }) + if diff := cmp.Diff(scenario.Expect, Expectation{ + Code: status.Code(err), + Response: resp, + }, protocmp.Transform()); diff != "" { + t.Errorf("unexpected difference:\n%v", diff) + } + }) + } } type FakeServerConnPool struct { @@ -145,7 +193,8 @@ func (f *FakeServerConnPool) Get(ctx context.Context, token string) (gitpod.APII } type FakeGitpodAPI struct { - workspaces map[string]*gitpod.WorkspaceInfo + workspaces map[string]*gitpod.WorkspaceInfo + ownertokens map[string]string } func (f *FakeGitpodAPI) GetWorkspace(ctx context.Context, id string) (res *gitpod.WorkspaceInfo, err error) { @@ -158,7 +207,11 @@ func (f *FakeGitpodAPI) GetWorkspace(ctx context.Context, id string) (res *gitpo } func (f *FakeGitpodAPI) GetOwnerToken(ctx context.Context, workspaceID string) (res string, err error) { - panic("implement me") + w, ok := f.ownertokens[workspaceID] + if !ok { + return "", errors.New("code 404") + } + return w, nil } func (f *FakeGitpodAPI) AdminBlockUser(ctx context.Context, req *gitpod.AdminBlockUserRequest) (err error) { From 0d747eeecb54898da0231fac3f73fdcf87cf5610 Mon Sep 17 00:00:00 2001 From: Andrew Farries Date: Tue, 17 May 2022 15:07:15 +0000 Subject: [PATCH 6/6] Make proxy treat 403 error as rpc PermissionDenied --- components/public-api-server/pkg/proxy/errors.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/components/public-api-server/pkg/proxy/errors.go b/components/public-api-server/pkg/proxy/errors.go index 9a3f2b556b138a..fa00dd1b8fa296 100644 --- a/components/public-api-server/pkg/proxy/errors.go +++ b/components/public-api-server/pkg/proxy/errors.go @@ -21,6 +21,11 @@ func ConvertError(err error) error { return status.Error(codes.PermissionDenied, s) } + // components/gitpod-protocol/src/messaging/error.ts + if strings.Contains(s, "code 403") { + return status.Error(codes.PermissionDenied, s) + } + // components/gitpod-protocol/src/messaging/error.ts if strings.Contains(s, "code 404") { return status.Error(codes.NotFound, s)