From 32e9044f1f2bfdde485ac6c223260b876cdb06a2 Mon Sep 17 00:00:00 2001 From: Francesco Ilario Date: Mon, 18 Mar 2024 13:12:42 +0100 Subject: [PATCH] add public-viewer support This commit adds the notion of Public-Viewer to the Proxy as described in [JIRA ASC-532](https://issues.redhat.com/browse/ASC-532). Signed-off-by: Francesco Ilario --- go.mod | 4 +- go.sum | 8 +- pkg/application/service/services.go | 2 +- pkg/configuration/configuration.go | 4 + pkg/context/keys.go | 4 + pkg/context/public_viewer.go | 10 + pkg/log/log.go | 8 + pkg/proxy/handlers/spacelister_get.go | 77 +++++- pkg/proxy/handlers/spacelister_get_test.go | 267 ++++++++++++++++++- pkg/proxy/handlers/spacelister_list.go | 40 ++- pkg/proxy/handlers/spacelister_list_test.go | 81 +++++- pkg/proxy/handlers/spacelister_test.go | 26 +- pkg/proxy/proxy.go | 230 ++++++++++++++-- pkg/proxy/proxy_community_test.go | 278 ++++++++++++++++++++ pkg/proxy/proxy_test.go | 116 ++++---- pkg/proxy/service/cluster_service.go | 70 ++++- pkg/proxy/service/cluster_service_test.go | 46 ++-- pkg/server/in_cluster_application.go | 3 +- test/fake/proxy.go | 9 +- 19 files changed, 1142 insertions(+), 141 deletions(-) create mode 100644 pkg/context/public_viewer.go create mode 100644 pkg/proxy/proxy_community_test.go diff --git a/go.mod b/go.mod index c07e2a59..579d6483 100644 --- a/go.mod +++ b/go.mod @@ -4,8 +4,8 @@ go 1.20 require ( github.com/aws/aws-sdk-go v1.44.100 - github.com/codeready-toolchain/api v0.0.0-20240607180719-368c7afbaebe - github.com/codeready-toolchain/toolchain-common v0.0.0-20240613121043-7e6ef858cdff + github.com/codeready-toolchain/api v0.0.0-20240708122235-0af5a9a178bb + github.com/codeready-toolchain/toolchain-common v0.0.0-20240711082950-c7f9f4442ae0 github.com/go-logr/logr v1.4.1 github.com/gofrs/uuid v4.2.0+incompatible github.com/pkg/errors v0.9.1 diff --git a/go.sum b/go.sum index ae8b8e05..2b47df67 100644 --- a/go.sum +++ b/go.sum @@ -48,10 +48,10 @@ github.com/cloudflare/circl v1.1.0/go.mod h1:prBCrKB9DV4poKZY1l9zBXg2QJY7mvgRvtM github.com/cloudflare/circl v1.3.7 h1:qlCDlTPz2n9fu58M0Nh1J/JzcFpfgkFHHX3O35r5vcU= github.com/cloudflare/circl v1.3.7/go.mod h1:sRTcRWXGLrKw6yIGJ+l7amYJFfAXbZG0kBSc8r4zxgA= github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f/go.mod h1:M8M6+tZqaGXZJjfX53e64911xZQV5JYwmTeXPW+k8Sc= -github.com/codeready-toolchain/api v0.0.0-20240607180719-368c7afbaebe h1:l+KsEXkNe1mZ14Z/RaTgeUkEuX9r56mSZC6xlu5H6zY= -github.com/codeready-toolchain/api v0.0.0-20240607180719-368c7afbaebe/go.mod h1:ie9p4LenCCS0LsnbWp6/xwpFDdCWYE0KWzUO6Sk1g0E= -github.com/codeready-toolchain/toolchain-common v0.0.0-20240613121043-7e6ef858cdff h1:bVWL+2eayFKUnEzdEAwltPs+pzbGlGDSmrM3oOV2Ams= -github.com/codeready-toolchain/toolchain-common v0.0.0-20240613121043-7e6ef858cdff/go.mod h1:cyHrUfvBYEtsf+FbqQYmR9y0AQi9QAVtM3SUWLA5bd4= +github.com/codeready-toolchain/api v0.0.0-20240708122235-0af5a9a178bb h1:Wc9CMsv0ODZv9dM5qF3OI0mFDO95YNIXV/8oRvoz8aE= +github.com/codeready-toolchain/api v0.0.0-20240708122235-0af5a9a178bb/go.mod h1:ie9p4LenCCS0LsnbWp6/xwpFDdCWYE0KWzUO6Sk1g0E= +github.com/codeready-toolchain/toolchain-common v0.0.0-20240711082950-c7f9f4442ae0 h1:v7Z5i0JaF1H9SYxK/uEjWgH8Vpm4Eg3OJep/Pl/2iyM= +github.com/codeready-toolchain/toolchain-common v0.0.0-20240711082950-c7f9f4442ae0/go.mod h1:8M9k7w2VSyRKSK6P08Jo2ddW3uyGgxCcSitnYa3HK9o= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= diff --git a/pkg/application/service/services.go b/pkg/application/service/services.go index 4d15b6b0..2602dccc 100644 --- a/pkg/application/service/services.go +++ b/pkg/application/service/services.go @@ -38,7 +38,7 @@ type VerificationService interface { } type MemberClusterService interface { - GetClusterAccess(userID, username, workspace, proxyPluginName string) (*access.ClusterAccess, error) + GetClusterAccess(userID, username, workspace, proxyPluginName string, publicViewerEnabled bool) (*access.ClusterAccess, error) } type Services interface { diff --git a/pkg/configuration/configuration.go b/pkg/configuration/configuration.go index a6611d6f..3e49fa68 100644 --- a/pkg/configuration/configuration.go +++ b/pkg/configuration/configuration.go @@ -121,6 +121,10 @@ func (r RegistrationServiceConfig) Print() { logger.Info("Registration Service Configuration", "config", r.cfg.Host.RegistrationService) } +func (r RegistrationServiceConfig) PublicViewerEnabled() bool { + return r.cfg.Host.PublicViewerConfig != nil && r.cfg.Host.PublicViewerConfig.Enabled +} + func (r RegistrationServiceConfig) Environment() string { return commonconfig.GetString(r.cfg.Host.RegistrationService.Environment, prodEnvironment) } diff --git a/pkg/context/keys.go b/pkg/context/keys.go index 9477de21..4ae47251 100644 --- a/pkg/context/keys.go +++ b/pkg/context/keys.go @@ -25,4 +25,8 @@ const ( WorkspaceKey = "workspace" // RequestReceivedTime is the context key for the starting time of a request made RequestReceivedTime = "requestReceivedTime" + // PublicViewerEnabled is a boolean value indicating whether PublicViewer support is enabled + PublicViewerEnabled = "publicViewerEnabled" + // ImpersonateUser is the content key for the impersonated user in proxied call + ImpersonateUser = "impersonateUser" ) diff --git a/pkg/context/public_viewer.go b/pkg/context/public_viewer.go new file mode 100644 index 00000000..f0b14e03 --- /dev/null +++ b/pkg/context/public_viewer.go @@ -0,0 +1,10 @@ +package context + +import "github.com/labstack/echo/v4" + +// IsPublicViewerEnabled retrieves from the context the boolean value associated to the PublicViewerEnabled key. +// If the key is not set it returns false, otherwise it returns the boolean value stored in the context. +func IsPublicViewerEnabled(ctx echo.Context) bool { + publicViewerEnabled, _ := ctx.Get(PublicViewerEnabled).(bool) + return publicViewerEnabled +} diff --git a/pkg/log/log.go b/pkg/log/log.go index f2107f8e..7ab56dee 100644 --- a/pkg/log/log.go +++ b/pkg/log/log.go @@ -149,6 +149,14 @@ func (l *Logger) InfoEchof(ctx echo.Context, msg string, args ...string) { ctxFields = append(ctxFields, "url") ctxFields = append(ctxFields, ctx.Request().URL) + if impersonateUser, ok := ctx.Get(context.ImpersonateUser).(string); ok { + ctxFields = append(ctxFields, "impersonate-user", impersonateUser) + } + + if publicViewerEnabled, ok := ctx.Get(context.PublicViewerEnabled).(bool); ok { + ctxFields = append(ctxFields, "public-viewer-enabled", publicViewerEnabled) + } + l.infof(ctxFields, msg, args...) } diff --git a/pkg/proxy/handlers/spacelister_get.go b/pkg/proxy/handlers/spacelister_get.go index 53b98930..6925fce3 100644 --- a/pkg/proxy/handlers/spacelister_get.go +++ b/pkg/proxy/handlers/spacelister_get.go @@ -8,6 +8,7 @@ import ( "time" toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" + "github.com/codeready-toolchain/registration-service/pkg/context" regsercontext "github.com/codeready-toolchain/registration-service/pkg/context" "github.com/codeready-toolchain/registration-service/pkg/proxy/metrics" "github.com/codeready-toolchain/registration-service/pkg/signup" @@ -43,7 +44,7 @@ func HandleSpaceGetRequest(spaceLister *SpaceLister, GetMembersFunc cluster.GetM } } -// GetUserWorkspace returns a workspace object with the required fields used by the proxy +// GetUserWorkspace returns a workspace object with the required fields used by the proxy. func GetUserWorkspace(ctx echo.Context, spaceLister *SpaceLister, workspaceName string) (*toolchainv1alpha1.Workspace, error) { userSignup, space, err := getUserSignupAndSpace(ctx, spaceLister, workspaceName) if err != nil { @@ -54,6 +55,56 @@ func GetUserWorkspace(ctx echo.Context, spaceLister *SpaceLister, workspaceName return nil, nil } + // retrieve user space binding + userSpaceBinding, err := getUserOrPublicViewerSpaceBinding(ctx, spaceLister, space, userSignup, workspaceName) + if err != nil { + return nil, err + } + // consider this as not found + if userSpaceBinding == nil { + return nil, nil + } + + // create and return the result workspace object + return createWorkspaceObject(userSignup.Name, space, userSpaceBinding), nil +} + +// getUserOrPublicViewerSpaceBinding retrieves the user space binding for an user and a space. +// If the SpaceBinding is not found and the PublicViewer feature is enabled, it will retry +// with the PublicViewer credentials. +func getUserOrPublicViewerSpaceBinding(ctx echo.Context, spaceLister *SpaceLister, space *toolchainv1alpha1.Space, userSignup *signup.Signup, workspaceName string) (*toolchainv1alpha1.SpaceBinding, error) { + userSpaceBinding, err := getUserSpaceBinding(ctx, spaceLister, space, userSignup) + if err != nil { + return nil, err + } + + // if user space binding is not found and PublicViewer is enabled, + // retry with PublicViewer's signup + if userSpaceBinding == nil { + if context.IsPublicViewerEnabled(ctx) { + publicViewerUserSignup := signup.Signup{ + Name: toolchainv1alpha1.KubesawAuthenticatedUsername, + CompliantUsername: toolchainv1alpha1.KubesawAuthenticatedUsername, + } + pvSb, err := getUserSpaceBinding(ctx, spaceLister, space, &publicViewerUserSignup) + if err != nil { + ctx.Logger().Error(fmt.Sprintf("error checking if SpaceBinding is present for user %s and the workspace %s", publicViewerUserSignup.CompliantUsername, workspaceName)) + return nil, err + } + if pvSb == nil { + ctx.Logger().Error(fmt.Sprintf("unauthorized access - there is no SpaceBinding present for the user %s and the workspace %s", userSignup.CompliantUsername, workspaceName)) + return nil, nil + } + return pvSb, nil + } + ctx.Logger().Error(fmt.Sprintf("unauthorized access - there is no SpaceBinding present for the user %s and the workspace %s", userSignup.CompliantUsername, workspaceName)) + } + + return userSpaceBinding, nil +} + +// getUserSpaceBinding retrieves the user space binding for an user and a space. +func getUserSpaceBinding(ctx echo.Context, spaceLister *SpaceLister, space *toolchainv1alpha1.Space, userSignup *signup.Signup) (*toolchainv1alpha1.SpaceBinding, error) { // recursively get all the spacebindings for the current workspace listSpaceBindingsFunc := func(spaceName string) ([]toolchainv1alpha1.SpaceBinding, error) { spaceSelector, err := labels.NewRequirement(toolchainv1alpha1.SpaceBindingSpaceLabelKey, selection.Equals, []string{spaceName}) @@ -74,7 +125,6 @@ func GetUserWorkspace(ctx echo.Context, spaceLister *SpaceLister, workspaceName } if len(userSpaceBindings) == 0 { // let's only log the issue and consider this as not found - ctx.Logger().Error(fmt.Sprintf("unauthorized access - there is no SpaceBinding present for the user %s and the workspace %s", userSignup.CompliantUsername, workspaceName)) return nil, nil } @@ -84,10 +134,10 @@ func GetUserWorkspace(ctx echo.Context, spaceLister *SpaceLister, workspaceName return nil, userBindingsErr } - return createWorkspaceObject(userSignup.Name, space, &userSpaceBindings[0]), nil + return &userSpaceBindings[0], nil } -// GetUserWorkspaceWithBindings returns a workspace object with the required fields+bindings (the list with all the users access details) +// GetUserWorkspaceWithBindings returns a workspace object with the required fields+bindings (the list with all the users access details). func GetUserWorkspaceWithBindings(ctx echo.Context, spaceLister *SpaceLister, workspaceName string, GetMembersFunc cluster.GetMemberClustersFunc) (*toolchainv1alpha1.Workspace, error) { userSignup, space, err := getUserSignupAndSpace(ctx, spaceLister, workspaceName) if err != nil { @@ -116,9 +166,16 @@ func GetUserWorkspaceWithBindings(ctx echo.Context, spaceLister *SpaceLister, wo // check if user has access to this workspace userBinding := filterUserSpaceBinding(userSignup.CompliantUsername, allSpaceBindings) if userBinding == nil { - // let's only log the issue and consider this as not found - ctx.Logger().Error(fmt.Sprintf("unauthorized access - there is no SpaceBinding present for the user %s and the workspace %s", userSignup.CompliantUsername, workspaceName)) - return nil, nil + // if PublicViewer is enabled, check if the Space is visibile to PublicViewer + if context.IsPublicViewerEnabled(ctx) && userSignup.CompliantUsername != toolchainv1alpha1.KubesawAuthenticatedUsername { + userBinding = filterUserSpaceBinding(toolchainv1alpha1.KubesawAuthenticatedUsername, allSpaceBindings) + } + + if userBinding == nil { + // let's only log the issue and consider this as not found + ctx.Logger().Error(fmt.Sprintf("unauthorized access - there is no SpaceBinding present for the user %s and the workspace %s", userSignup.CompliantUsername, workspaceName)) + return nil, nil + } } // list all SpaceBindingRequests , just in case there might be some failing to create a SpaceBinding resource. @@ -155,6 +212,12 @@ func getUserSignupAndSpace(ctx echo.Context, spaceLister *SpaceLister, workspace if err != nil { return nil, nil, err } + if userSignup == nil && context.IsPublicViewerEnabled(ctx) { + userSignup = &signup.Signup{ + CompliantUsername: toolchainv1alpha1.KubesawAuthenticatedUsername, + Name: toolchainv1alpha1.KubesawAuthenticatedUsername, + } + } space, err := spaceLister.GetInformerServiceFunc().GetSpace(workspaceName) if err != nil { diff --git a/pkg/proxy/handlers/spacelister_get_test.go b/pkg/proxy/handlers/spacelister_get_test.go index 6195cf92..dfbb7981 100644 --- a/pkg/proxy/handlers/spacelister_get_test.go +++ b/pkg/proxy/handlers/spacelister_get_test.go @@ -31,8 +31,16 @@ import ( runtimeclient "sigs.k8s.io/controller-runtime/pkg/client" ) -func TestSpaceListerGet(t *testing.T) { - fakeSignupService, fakeClient := buildSpaceListerFakes(t) +func TestSpaceListerGetCommonCommunityEnabled(t *testing.T) { + testSpaceListerGet(t, true) +} + +func TestSpaceListerGetCommonCommunityDisabled(t *testing.T) { + testSpaceListerGet(t, false) +} + +func testSpaceListerGet(t *testing.T, publicViewerEnabled bool) { + fakeSignupService, fakeClient := buildSpaceListerFakes(t, publicViewerEnabled) memberFakeClient := fake.InitClient(t, // spacebinding requests @@ -564,6 +572,7 @@ func TestSpaceListerGet(t *testing.T) { if tc.overrideGetMembersFunc != nil { getMembersFunc = tc.overrideGetMembersFunc } + ctx.Set(rcontext.PublicViewerEnabled, publicViewerEnabled) err := handlers.HandleSpaceGetRequest(s, getMembersFunc)(ctx) // then @@ -726,3 +735,257 @@ func TestGetUserWorkspace(t *testing.T) { }) } } + +func TestSpaceListerGetCommunityEnabled(t *testing.T) { + + fakeSignupService := fake.NewSignupService( + newSignup("batman", "batman.space", true), + newSignup("robin", "robin.space", true), + ) + + fakeClient := fake.InitClient(t, + // space + fake.NewSpace("robin", "member-1", "robin"), + fake.NewSpace("batman", "member-1", "batman"), + + // spacebindings + fake.NewSpaceBinding("robin-1", "robin", "robin", "admin"), + fake.NewSpaceBinding("batman-1", "batman", "batman", "admin"), + + fake.NewSpaceBinding("community-robin", toolchainv1alpha1.KubesawAuthenticatedUsername, "robin", "viewer"), + ) + + robinWS := workspaceFor(t, fakeClient, "robin", "admin", true) + batmanWS := workspaceFor(t, fakeClient, "batman", "admin", true) + + tests := map[string]struct { + username string + expectedErr string + workspaceRequest string + expectedWorkspace *toolchainv1alpha1.Workspace + overrideInformerFunc func() service.InformerService + overrideSignupFunc func(ctx *gin.Context, userID, username string, checkUserSignupComplete bool) (*signup.Signup, error) + }{ + "robin can get robin workspace": { + username: "robin.space", + workspaceRequest: "robin", + expectedWorkspace: &robinWS, + }, + "batman can get batman workspace": { + username: "batman.space", + workspaceRequest: "batman", + expectedWorkspace: &batmanWS, + }, + "batman can get robin workspace": { + username: "batman.space", + workspaceRequest: "robin", + expectedWorkspace: func() *toolchainv1alpha1.Workspace { + batmansRobinWS := robinWS.DeepCopy() + batmansRobinWS.Status.Type = "" + batmansRobinWS.Status.Role = "viewer" + return batmansRobinWS + }(), + }, + "robin can not get batman workspace": { + username: "robin.space", + workspaceRequest: "batman", + expectedWorkspace: nil, + expectedErr: "", + }, + } + + for k, tc := range tests { + + t.Run(k, func(t *testing.T) { + // given + signupProvider := fakeSignupService.GetSignupFromInformer + if tc.overrideSignupFunc != nil { + signupProvider = tc.overrideSignupFunc + } + informerFunc := fake.GetInformerService(fakeClient) + if tc.overrideInformerFunc != nil { + informerFunc = tc.overrideInformerFunc + } + + proxyMetrics := metrics.NewProxyMetrics(prometheus.NewRegistry()) + s := &handlers.SpaceLister{ + GetSignupFunc: signupProvider, + GetInformerServiceFunc: informerFunc, + ProxyMetrics: proxyMetrics, + } + + e := echo.New() + req := httptest.NewRequest(http.MethodGet, "/", strings.NewReader("")) + rec := httptest.NewRecorder() + ctx := e.NewContext(req, rec) + ctx.Set(rcontext.UsernameKey, tc.username) + ctx.Set(rcontext.RequestReceivedTime, time.Now()) + ctx.SetParamNames("workspace") + ctx.SetParamValues(tc.workspaceRequest) + ctx.Set(rcontext.PublicViewerEnabled, true) + + // when + wrk, err := handlers.GetUserWorkspace(ctx, s, tc.workspaceRequest) + + // then + if tc.expectedErr != "" { + // error case + require.Error(t, err, tc.expectedErr) + } else { + require.NoError(t, err) + } + + if tc.expectedWorkspace != nil { + require.Equal(t, tc.expectedWorkspace, wrk) + } else { + require.Nil(t, wrk) // user is not authorized to get this workspace + } + }) + } +} + +func TestSpaceListerGetWithBindingsCommunityEnabled(t *testing.T) { + + fakeSignupService := fake.NewSignupService( + newSignup("batman", "batman.space", true), + newSignup("robin", "robin.space", true), + ) + + fakeClient := fake.InitClient(t, + // NSTemplateTiers + fake.NewBase1NSTemplateTier(), + + // space + fake.NewSpace("robin", "member-1", "robin"), + fake.NewSpace("batman", "member-1", "batman"), + + // spacebindings + fake.NewSpaceBinding("robin-1", "robin", "robin", "admin"), + fake.NewSpaceBinding("batman-1", "batman", "batman", "admin"), + + fake.NewSpaceBinding("community-robin", toolchainv1alpha1.KubesawAuthenticatedUsername, "robin", "viewer"), + ) + + robinWS := workspaceFor(t, fakeClient, "robin", "admin", true, + commonproxy.WithBindings([]toolchainv1alpha1.Binding{ + { + MasterUserRecord: toolchainv1alpha1.KubesawAuthenticatedUsername, + Role: "viewer", + AvailableActions: []string{}, + }, + { + MasterUserRecord: "robin", + Role: "admin", + AvailableActions: []string{}, + }, + }), + commonproxy.WithAvailableRoles([]string{"admin", "viewer"}), + ) + + batmanWS := workspaceFor(t, fakeClient, "batman", "admin", true, + commonproxy.WithBindings([]toolchainv1alpha1.Binding{ + { + MasterUserRecord: "batman", + Role: "admin", + AvailableActions: []string{}, + }, + }), + commonproxy.WithAvailableRoles([]string{"admin", "viewer"}), + ) + + tests := map[string]struct { + username string + expectedErr string + workspaceRequest string + expectedWorkspace *toolchainv1alpha1.Workspace + overrideInformerFunc func() service.InformerService + overrideSignupFunc func(ctx *gin.Context, userID, username string, checkUserSignupComplete bool) (*signup.Signup, error) + }{ + "robin can get robin workspace": { + username: "robin.space", + workspaceRequest: "robin", + expectedWorkspace: &robinWS, + }, + "batman can get batman workspace": { + username: "batman.space", + workspaceRequest: "batman", + expectedWorkspace: &batmanWS, + }, + "batman can get robin workspace": { + username: "batman.space", + workspaceRequest: "robin", + expectedWorkspace: func() *toolchainv1alpha1.Workspace { + batmansRobinWS := robinWS.DeepCopy() + batmansRobinWS.Status.Type = "" + batmansRobinWS.Status.Role = "viewer" + return batmansRobinWS + }(), + }, + "robin can not get batman workspace": { + username: "robin.space", + workspaceRequest: "batman", + expectedWorkspace: nil, + expectedErr: "", + }, + } + + for k, tc := range tests { + + t.Run(k, func(t *testing.T) { + // given + signupProvider := fakeSignupService.GetSignupFromInformer + if tc.overrideSignupFunc != nil { + signupProvider = tc.overrideSignupFunc + } + informerFunc := fake.GetInformerService(fakeClient) + if tc.overrideInformerFunc != nil { + informerFunc = tc.overrideInformerFunc + } + + proxyMetrics := metrics.NewProxyMetrics(prometheus.NewRegistry()) + s := &handlers.SpaceLister{ + GetSignupFunc: signupProvider, + GetInformerServiceFunc: informerFunc, + ProxyMetrics: proxyMetrics, + } + + e := echo.New() + req := httptest.NewRequest(http.MethodGet, "/", strings.NewReader("")) + rec := httptest.NewRecorder() + ctx := e.NewContext(req, rec) + ctx.Set(rcontext.UsernameKey, tc.username) + ctx.Set(rcontext.RequestReceivedTime, time.Now()) + ctx.SetParamNames("workspace") + ctx.SetParamValues(tc.workspaceRequest) + ctx.Set(rcontext.PublicViewerEnabled, true) + + getMembersFuncMock := func(_ ...commoncluster.Condition) []*commoncluster.CachedToolchainCluster { + return []*commoncluster.CachedToolchainCluster{ + { + Client: fake.InitClient(t), + Config: &commoncluster.Config{ + Name: "not-me", + }, + }, + } + } + + // when + wrk, err := handlers.GetUserWorkspaceWithBindings(ctx, s, tc.workspaceRequest, getMembersFuncMock) + + // then + if tc.expectedErr != "" { + // error case + require.Error(t, err, tc.expectedErr) + } else { + require.NoError(t, err) + } + + if tc.expectedWorkspace != nil { + require.Equal(t, tc.expectedWorkspace, wrk) + } else { + require.Nil(t, wrk) // user is not authorized to get this workspace + } + }) + } +} diff --git a/pkg/proxy/handlers/spacelister_list.go b/pkg/proxy/handlers/spacelister_list.go index bc2af8fe..3199c4d1 100644 --- a/pkg/proxy/handlers/spacelister_list.go +++ b/pkg/proxy/handlers/spacelister_list.go @@ -6,15 +6,17 @@ import ( "net/http" "time" - toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" - "github.com/codeready-toolchain/registration-service/pkg/context" - "github.com/codeready-toolchain/registration-service/pkg/proxy/metrics" "github.com/labstack/echo/v4" errs "github.com/pkg/errors" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/selection" + + toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" + "github.com/codeready-toolchain/registration-service/pkg/context" + "github.com/codeready-toolchain/registration-service/pkg/proxy/metrics" + "github.com/codeready-toolchain/registration-service/pkg/signup" ) func HandleSpaceListRequest(spaceLister *SpaceLister) echo.HandlerFunc { @@ -40,19 +42,38 @@ func ListUserWorkspaces(ctx echo.Context, spaceLister *SpaceLister) ([]toolchain } // signup is not ready if signup == nil { - return []toolchainv1alpha1.Workspace{}, nil + return nil, nil + } + + // get MUR Names + murNames := getMURNamesForList(ctx, signup) + if len(murNames) == 0 { + return nil, nil } - murName := signup.CompliantUsername // get all spacebindings with given mur since no workspace was provided - spaceBindings, err := listSpaceBindingsForUser(spaceLister, murName) + spaceBindings, err := listSpaceBindingsForUsers(spaceLister, murNames) if err != nil { ctx.Logger().Error(errs.Wrap(err, "error listing space bindings")) return nil, err } + return workspacesFromSpaceBindings(ctx, spaceLister, signup.Name, spaceBindings), nil } +// getMURNamesForList returns a list MasterUserRecord names to use in listing Workspaces. +// If PublicViewer is enabled, the list will contain at least the PublicViewer username. +func getMURNamesForList(ctx echo.Context, signup *signup.Signup) []string { + names := []string{} + if signup != nil && signup.CompliantUsername != "" { + names = append(names, signup.CompliantUsername) + } + if context.IsPublicViewerEnabled(ctx) { + names = append(names, toolchainv1alpha1.KubesawAuthenticatedUsername) + } + return names +} + func listWorkspaceResponse(ctx echo.Context, workspaces []toolchainv1alpha1.Workspace) error { workspaceList := &toolchainv1alpha1.WorkspaceList{ TypeMeta: metav1.TypeMeta{ @@ -67,13 +88,12 @@ func listWorkspaceResponse(ctx echo.Context, workspaces []toolchainv1alpha1.Work return json.NewEncoder(ctx.Response().Writer).Encode(workspaceList) } -func listSpaceBindingsForUser(spaceLister *SpaceLister, murName string) ([]toolchainv1alpha1.SpaceBinding, error) { - murSelector, err := labels.NewRequirement(toolchainv1alpha1.SpaceBindingMasterUserRecordLabelKey, selection.Equals, []string{murName}) +func listSpaceBindingsForUsers(spaceLister *SpaceLister, murNames []string) ([]toolchainv1alpha1.SpaceBinding, error) { + murSelector, err := labels.NewRequirement(toolchainv1alpha1.SpaceBindingMasterUserRecordLabelKey, selection.In, murNames) if err != nil { return nil, err } - requirements := []labels.Requirement{*murSelector} - return spaceLister.GetInformerServiceFunc().ListSpaceBindings(requirements...) + return spaceLister.GetInformerServiceFunc().ListSpaceBindings(*murSelector) } func workspacesFromSpaceBindings(ctx echo.Context, spaceLister *SpaceLister, signupName string, spaceBindings []toolchainv1alpha1.SpaceBinding) []toolchainv1alpha1.Workspace { diff --git a/pkg/proxy/handlers/spacelister_list_test.go b/pkg/proxy/handlers/spacelister_list_test.go index c07b4a09..0d1f3ce3 100644 --- a/pkg/proxy/handlers/spacelister_list_test.go +++ b/pkg/proxy/handlers/spacelister_list_test.go @@ -24,8 +24,86 @@ import ( "github.com/codeready-toolchain/registration-service/test/fake" ) +func TestSpaceListerListCommunity(t *testing.T) { + publicViewerEnabled := true + fakeSignupService, fakeClient := buildSpaceListerFakes(t, publicViewerEnabled) + tests := map[string]struct { + username string + expectedWs []toolchainv1alpha1.Workspace + expectedErr string + expectedErrCode int + expectedWorkspace string + overrideSignupFunc func(ctx *gin.Context, userID, username string, checkUserSignupComplete bool) (*signup.Signup, error) + overrideInformerFunc func() service.InformerService + }{ + "dancelover lists spaces": { + username: "dance.lover", + expectedWs: []toolchainv1alpha1.Workspace{ + workspaceFor(t, fakeClient, "communityspace", "viewer", false), + workspaceFor(t, fakeClient, "dancelover", "admin", true), + workspaceFor(t, fakeClient, "movielover", "other", false), + }, + expectedErr: "", + }, + } + + t.Run("HandleSpaceListRequest", func(t *testing.T) { + for k, tc := range tests { + t.Run(k, func(t *testing.T) { + // given + signupProvider := fakeSignupService.GetSignupFromInformer + if tc.overrideSignupFunc != nil { + signupProvider = tc.overrideSignupFunc + } + + informerFunc := fake.GetInformerService(fakeClient) + if tc.overrideInformerFunc != nil { + informerFunc = tc.overrideInformerFunc + } + + proxyMetrics := metrics.NewProxyMetrics(prometheus.NewRegistry()) + + s := &handlers.SpaceLister{ + GetSignupFunc: signupProvider, + GetInformerServiceFunc: informerFunc, + ProxyMetrics: proxyMetrics, + } + + e := echo.New() + req := httptest.NewRequest(http.MethodGet, "/", strings.NewReader("")) + rec := httptest.NewRecorder() + ctx := e.NewContext(req, rec) + ctx.Set(rcontext.UsernameKey, tc.username) + ctx.Set(rcontext.RequestReceivedTime, time.Now()) + + // when + ctx.Set(rcontext.PublicViewerEnabled, publicViewerEnabled) + err := handlers.HandleSpaceListRequest(s)(ctx) + + // then + if tc.expectedErr != "" { + // error case + require.Equal(t, tc.expectedErrCode, rec.Code) + require.Contains(t, rec.Body.String(), tc.expectedErr) + } else { + require.NoError(t, err) + // list workspace case + workspaceList, decodeErr := decodeResponseToWorkspaceList(rec.Body.Bytes()) + require.NoError(t, decodeErr) + require.Equal(t, len(tc.expectedWs), len(workspaceList.Items)) + for i := range tc.expectedWs { + assert.Equal(t, tc.expectedWs[i].Name, workspaceList.Items[i].Name) + assert.Equal(t, tc.expectedWs[i].Status, workspaceList.Items[i].Status) + } + } + }) + } + }) +} + func TestSpaceListerList(t *testing.T) { - fakeSignupService, fakeClient := buildSpaceListerFakes(t) + publicViewerEnabled := false + fakeSignupService, fakeClient := buildSpaceListerFakes(t, publicViewerEnabled) t.Run("HandleSpaceListRequest", func(t *testing.T) { // given @@ -124,6 +202,7 @@ func TestSpaceListerList(t *testing.T) { ctx.Set(rcontext.RequestReceivedTime, time.Now()) // when + ctx.Set(rcontext.PublicViewerEnabled, publicViewerEnabled) err := handlers.HandleSpaceListRequest(s)(ctx) // then diff --git a/pkg/proxy/handlers/spacelister_test.go b/pkg/proxy/handlers/spacelister_test.go index f9804e74..82f2854f 100644 --- a/pkg/proxy/handlers/spacelister_test.go +++ b/pkg/proxy/handlers/spacelister_test.go @@ -20,8 +20,8 @@ import ( spacetest "github.com/codeready-toolchain/toolchain-common/pkg/test/space" ) -func buildSpaceListerFakes(t *testing.T) (*fake.SignupService, *test.FakeClient) { - fakeSignupService := fake.NewSignupService( +func buildSpaceListerFakes(t *testing.T, publicViewerEnabled bool) (*fake.SignupService, *test.FakeClient) { + signups := []fake.SignupDef{ newSignup("dancelover", "dance.lover", true), newSignup("movielover", "movie.lover", true), newSignup("pandalover", "panda.lover", true), @@ -33,7 +33,14 @@ func buildSpaceListerFakes(t *testing.T) (*fake.SignupService, *test.FakeClient) newSignup("parentspace", "parent.space", true), newSignup("childspace", "child.space", true), newSignup("grandchildspace", "grandchild.space", true), - ) + } + if publicViewerEnabled { + signups = append(signups, + newSignup("communityspace", "community.space", true), + newSignup("communitylover", "community.lover", true), + ) + } + fakeSignupService := fake.NewSignupService(signups...) // space that is not provisioned yet spaceNotProvisionedYet := fake.NewSpace("pandalover", "member-2", "pandalover") @@ -60,7 +67,7 @@ func buildSpaceListerFakes(t *testing.T) (*fake.SignupService, *test.FakeClient) spaceBindingWithInvalidSBRNamespace.Labels[toolchainv1alpha1.SpaceBindingRequestLabelKey] = "anime-sbr" spaceBindingWithInvalidSBRNamespace.Labels[toolchainv1alpha1.SpaceBindingRequestNamespaceLabelKey] = "" // let's set the name to blank in order to trigger an error - fakeClient := fake.InitClient(t, + objs := []runtime.Object{ // spaces fake.NewSpace("dancelover", "member-1", "dancelover"), fake.NewSpace("movielover", "member-1", "movielover"), @@ -74,6 +81,8 @@ func buildSpaceListerFakes(t *testing.T) (*fake.SignupService, *test.FakeClient) fake.NewSpace("grandchildspace", "member-1", "grandchildspace", spacetest.WithSpecParentSpace("childspace")), // noise space, user will have a different role here , just to make sure this is not returned anywhere in the tests fake.NewSpace("otherspace", "member-1", "otherspace", spacetest.WithSpecParentSpace("otherspace")), + // space flagged as community + fake.NewSpace("communityspace", "member-2", "communityspace"), //spacebindings fake.NewSpaceBinding("dancer-sb1", "dancelover", "dancelover", "admin"), @@ -94,7 +103,14 @@ func buildSpaceListerFakes(t *testing.T) (*fake.SignupService, *test.FakeClient) //nstemplatetier fake.NewBase1NSTemplateTier(), - ) + } + if publicViewerEnabled { + objs = append(objs, + fake.NewSpaceBinding("communityspace-sb", "communityspace", "communityspace", "admin"), + fake.NewSpaceBinding("community-sb", toolchainv1alpha1.KubesawAuthenticatedUsername, "communityspace", "viewer"), + ) + } + fakeClient := fake.InitClient(t, objs...) return fakeSignupService, fakeClient } diff --git a/pkg/proxy/proxy.go b/pkg/proxy/proxy.go index d950c1bb..581ff967 100644 --- a/pkg/proxy/proxy.go +++ b/pkg/proxy/proxy.go @@ -26,6 +26,7 @@ import ( "github.com/codeready-toolchain/registration-service/pkg/proxy/access" "github.com/codeready-toolchain/registration-service/pkg/proxy/handlers" "github.com/codeready-toolchain/registration-service/pkg/proxy/metrics" + "github.com/codeready-toolchain/registration-service/pkg/signup" commoncluster "github.com/codeready-toolchain/toolchain-common/pkg/cluster" "github.com/labstack/echo/v4" "github.com/labstack/echo/v4/middleware" @@ -141,6 +142,7 @@ func (p *Proxy) StartProxy(port string) *http.Server { // Space lister routes wg.GET("/:workspace", handlers.HandleSpaceGetRequest(p.spaceLister, p.getMembersFunc)) wg.GET("", handlers.HandleSpaceListRequest(p.spaceLister)) + router.GET(proxyHealthEndpoint, p.health) // SSO routes. Used by web login (oc login -w). // Here is the expected flow for the "oc login -w" command: @@ -264,6 +266,7 @@ func (p *Proxy) health(ctx echo.Context) error { } func (p *Proxy) processRequest(ctx echo.Context) (string, *access.ClusterAccess, error) { + // retrieve required information from the HTTP request userID, _ := ctx.Get(context.SubKey).(string) username, _ := ctx.Get(context.UsernameKey).(string) proxyPluginName, workspaceName, err := getWorkspaceContext(ctx.Request()) @@ -271,44 +274,213 @@ func (p *Proxy) processRequest(ctx echo.Context) (string, *access.ClusterAccess, return "", nil, crterrors.NewBadRequest("unable to get workspace context", err.Error()) } - ctx.Set(context.WorkspaceKey, workspaceName) // set workspace context for logging - cluster, err := p.app.MemberClusterService().GetClusterAccess(userID, username, workspaceName, proxyPluginName) + // set workspace context for logging + ctx.Set(context.WorkspaceKey, workspaceName) + + // if the target workspace is NOT explicitly declared in the HTTP request, + // process the request against the user's home workspace + if workspaceName == "" { + cluster, err := p.processHomeWorkspaceRequest(ctx, userID, username, proxyPluginName) + if err != nil { + return "", nil, err + } + return proxyPluginName, cluster, nil + } + + // if the target workspace is explicitly declared in the HTTP request, + // process the request against the declared workspace + cluster, err := p.processWorkspaceRequest(ctx, userID, username, workspaceName, proxyPluginName) + if err != nil { + return "", nil, err + } + return proxyPluginName, cluster, nil +} + +// processHomeWorkspaceRequest process an HTTP Request targeting the user's home workspace. +func (p *Proxy) processHomeWorkspaceRequest(ctx echo.Context, userID, username, proxyPluginName string) (*access.ClusterAccess, error) { + // retrieves the ClusterAccess for the user and their home workspace + cluster, err := p.app.MemberClusterService().GetClusterAccess(userID, username, "", proxyPluginName, false) if err != nil { - return "", nil, crterrors.NewInternalError(errs.New("unable to get target cluster"), err.Error()) + return nil, crterrors.NewInternalError(errs.New("unable to get target cluster"), err.Error()) } - // before proxying the request, verify that the user has a spacebinding for the workspace and that the namespace (if any) belongs to the workspace - var workspaces []toolchainv1alpha1.Workspace - if workspaceName != "" { - // when a workspace name was provided - // validate that the user has access to the workspace by getting all spacebindings recursively, starting from this workspace and going up to the parent workspaces till the "root" of the workspace tree. - workspace, err := handlers.GetUserWorkspace(ctx, p.spaceLister, workspaceName) + // list all workspaces the user has access to + workspaces, err := handlers.ListUserWorkspaces(ctx, p.spaceLister) + if err != nil { + return nil, crterrors.NewInternalError(errs.New("unable to retrieve user workspaces"), err.Error()) + } + + // check whether the user's has access to the home workspace + // and whether the requestedNamespace -if any- exists in the workspace. + requestedNamespace := namespaceFromCtx(ctx) + if err := validateWorkspaceRequest("", requestedNamespace, workspaces); err != nil { + return nil, crterrors.NewForbiddenError("invalid workspace request", err.Error()) + } + + // return the cluster access + return cluster, nil +} + +// processWorkspaceRequest process an HTTP Request targeting a specific workspace. +func (p *Proxy) processWorkspaceRequest(ctx echo.Context, userID, username, workspaceName, proxyPluginName string) (*access.ClusterAccess, error) { + // check if the user has access to the target workspace + if err := p.validateSpaceAccess(ctx, userID, username, workspaceName); err != nil { + return nil, err + } + + // before proxying the request, verify that the user has a spacebinding for + // the workspace and that the namespace -if any- belongs to the workspace + workspaces, err := p.listUserWorkspaces(ctx, workspaceName) + if err != nil { + return nil, err + } + + // check whether the user's has access to the home workspace + // and whether the requestedNamespace -if any- exists in the workspace. + requestedNamespace := namespaceFromCtx(ctx) + if err := validateWorkspaceRequest(workspaceName, requestedNamespace, workspaces); err != nil { + return nil, crterrors.NewForbiddenError("invalid workspace request", err.Error()) + } + + // retrieve the ClusterAccess for the user and the target workspace + return p.getClusterAccess(ctx, userID, username, workspaceName, proxyPluginName, workspaces) +} + +// validateSpaceAccess checks whether the user has access to the target workspace and that the Space exists. +func (p *Proxy) validateSpaceAccess(ctx echo.Context, userID, username, workspaceName string) error { + if err := p.validateUserAccess(ctx, userID, username); err != nil { + return crterrors.NewInternalError(errs.New("unable to get target cluster"), err.Error()) + } + if err := p.validateSpace(workspaceName); err != nil { + return crterrors.NewInternalError(errs.New("unable to get target cluster"), err.Error()) + } + + return nil +} + +// validateSpace checks whether the Space exists. +func (p *Proxy) validateSpace(workspaceName string) error { + if _, err := p.app.InformerService().GetSpace(workspaceName); err != nil { + // log the actual error but do not return it so that it doesn't reveal information about a space that may not belong to the requestor + log.Error(nil, err, "unable to get target cluster for workspace "+workspaceName) + return fmt.Errorf("the requested space is not available") + } + return nil +} + +// validateUserAccess checks whether the user is Approved, if they are not an error is returned. +// If public-viewer is enabled, user validation is skipped. +func (p *Proxy) validateUserAccess(ctx echo.Context, userID, username string) error { + // skip if public-viewer is enabled: read-only operations on community workspaces are always permitted. + if context.IsPublicViewerEnabled(ctx) { + return nil + } + + // retrieve the UserSignup for the requesting user. + // + // UserSignup complete status is not checked, since it might cause the proxy blocking the request + // and returning an error when quick transitions from ready to provisioning are happening. + userSignup, err := p.app.SignupService().GetSignupFromInformer(nil, userID, username, false) + if err != nil { + return err + } + + // if the UserSignup is nil or has NOT the CompliantUsername set, + // it means that MUR was NOT created and useraccount is NOT provisioned yet + // TODO(@filariow): should we also check if the user is Banned? + if userSignup == nil || userSignup.CompliantUsername == "" { + cause := errs.New("user is not provisioned (yet)") + log.Error(nil, cause, fmt.Sprintf("signup object: %+v", userSignup)) + return cause + } + return nil +} + +// getClusterAccess retrieves the access to the cluster hosting the requested workspace, +// if the user has access to it. +// Access can be either direct (an SpaceBinding linking the user to the workspace exists) +// or community (a SpaceBinding linking the PublicViewer user to the workspace exists). +func (p *Proxy) getClusterAccess(ctx echo.Context, userID, username, workspaceName, proxyPluginName string, workspaces []toolchainv1alpha1.Workspace) (*access.ClusterAccess, error) { + // retrieve by name the workspace from the list of workspaces the user has access to. + w := findWorkspaceByName(workspaceName, workspaces) + if w == nil { + return nil, crterrors.NewInternalError(errs.New("unable to get target cluster"), "workspace not found") + } + + // retrieve the requesting user's UserSignup + signup, err := p.spaceLister.GetSignupFunc(nil, userID, username, false) + if err != nil { + return nil, crterrors.NewInternalError(errs.New("unable to get target cluster"), "user not found") + } + + // if PublicViewer is enabled and user has no direct access to the workspace, proceed as PublicViewer + publicViewerEnabled := context.IsPublicViewerEnabled(ctx) + if publicViewerEnabled && !hasDirectAccess(signup, w) { + cluster, err := p.app.MemberClusterService().GetClusterAccess( + toolchainv1alpha1.KubesawAuthenticatedUsername, + toolchainv1alpha1.KubesawAuthenticatedUsername, + workspaceName, + proxyPluginName, + publicViewerEnabled) if err != nil { - return "", nil, crterrors.NewInternalError(errs.New("unable to retrieve user workspaces"), err.Error()) + return nil, crterrors.NewInternalError(errs.New("unable to get target cluster"), err.Error()) } - if workspace == nil { - // not found - return "", nil, crterrors.NewForbiddenError("invalid workspace request", fmt.Sprintf("access to workspace '%s' is forbidden", workspaceName)) - } - // workspace was found means we can forward the request - workspaces = []toolchainv1alpha1.Workspace{*workspace} - } else { - // list all workspaces - workspaces, err = handlers.ListUserWorkspaces(ctx, p.spaceLister) - if err != nil { - return "", nil, crterrors.NewInternalError(errs.New("unable to retrieve user workspaces"), err.Error()) + return cluster, nil + } + + // retrieve the ClusterAccess for the cluster hosting the workspace and the given user. + cluster, err := p.app.MemberClusterService().GetClusterAccess(userID, username, workspaceName, proxyPluginName, publicViewerEnabled) + if err != nil { + return nil, crterrors.NewInternalError(errs.New("unable to get target cluster"), err.Error()) + } + return cluster, nil +} + +// hasDirectAccess checks if an UserSignup has access to a workspace. +// Workspace's bindings are obtained from its `status.bindings` property. +func hasDirectAccess(signup *signup.Signup, workspace *toolchainv1alpha1.Workspace) bool { + if signup == nil || workspace == nil { + return false + } + + for _, b := range workspace.Status.Bindings { + if b.MasterUserRecord == signup.CompliantUsername { + return true } } - requestedNamespace := namespaceFromCtx(ctx) - if err := validateWorkspaceRequest(workspaceName, requestedNamespace, workspaces); err != nil { - return "", nil, crterrors.NewForbiddenError("invalid workspace request", err.Error()) + return false +} + +// findWorkspaceByName finds a workspace by name in a list of Workspaces. +func findWorkspaceByName(workspaceName string, workspaces []toolchainv1alpha1.Workspace) *toolchainv1alpha1.Workspace { + for _, w := range workspaces { + if w.Name == workspaceName { + w := w // TODO We won't need it after upgrading to go 1.22: https://go.dev/blog/loopvar-preview + return &w + } } + return nil +} - return proxyPluginName, cluster, nil +func (p *Proxy) listUserWorkspaces(ctx echo.Context, workspaceName string) ([]toolchainv1alpha1.Workspace, error) { + // when a workspace name was provided + // validate that the user has access to the workspace by getting all spacebindings recursively, starting from this workspace and going up to the parent workspaces till the "root" of the workspace tree. + workspace, err := handlers.GetUserWorkspaceWithBindings(ctx, p.spaceLister, workspaceName, p.getMembersFunc) + if err != nil { + return nil, crterrors.NewInternalError(errs.New("unable to retrieve user workspaces"), err.Error()) + } + if workspace == nil { + // not found + return nil, crterrors.NewForbiddenError("invalid workspace request", fmt.Sprintf("access to workspace '%s' is forbidden", workspaceName)) + } + // workspace was found means we can forward the request + return []toolchainv1alpha1.Workspace{*workspace}, nil } func (p *Proxy) handleRequestAndRedirect(ctx echo.Context) error { requestReceivedTime := ctx.Get(context.RequestReceivedTime).(time.Time) + publicViewerEnabled := configuration.GetRegistrationServiceConfig().PublicViewerEnabled() + ctx.Set(context.PublicViewerEnabled, publicViewerEnabled) proxyPluginName, cluster, err := p.processRequest(ctx) if err != nil { p.metrics.RegServProxyAPIHistogramVec.WithLabelValues(fmt.Sprintf("%d", http.StatusNotAcceptable), metrics.MetricLabelRejected).Observe(time.Since(requestReceivedTime).Seconds()) @@ -464,11 +636,16 @@ func extractUserToken(req *http.Request) (string, error) { func (p *Proxy) newReverseProxy(ctx echo.Context, target *access.ClusterAccess, isPlugin bool) *httputil.ReverseProxy { req := ctx.Request() targetQuery := target.APIURL().RawQuery + username, _ := ctx.Get(context.UsernameKey).(string) + ctx.Set(context.ImpersonateUser, target.Username()) + director := func(req *http.Request) { origin := req.URL.String() req.URL.Scheme = target.APIURL().Scheme req.URL.Host = target.APIURL().Host req.URL.Path = singleJoiningSlash(target.APIURL().Path, req.URL.Path) + req.Header.Set("X-SSO-User", username) + if isPlugin { // for non k8s clients testing, like vanilla http clients accessing plugin proxy flows, testing has proven that the request // host needs to be updated in addition to the URL in order to have the reverse proxy contact the openshift @@ -632,6 +809,9 @@ func replaceTokenInWebsocketRequest(req *http.Request, newToken string) { req.Header.Set(ph, strings.Join(protocols, ",")) } +// validateWorkspaceRequest checks whether the requested workspace is in the list of workspaces the user has visibility on (retrieved via the spaceLister). +// If `requestedWorkspace` is zero, this function looks for the home workspace (the one with `status.Type` set to `home`). +// If `requestedNamespace` is NOT zero, this function checks if the namespace exists in the workspace. func validateWorkspaceRequest(requestedWorkspace, requestedNamespace string, workspaces []toolchainv1alpha1.Workspace) error { // check workspace access isHomeWSRequested := requestedWorkspace == "" diff --git a/pkg/proxy/proxy_community_test.go b/pkg/proxy/proxy_community_test.go new file mode 100644 index 00000000..8e2ed5e6 --- /dev/null +++ b/pkg/proxy/proxy_community_test.go @@ -0,0 +1,278 @@ +package proxy + +import ( + "context" + "fmt" + "log" + "net/http" + "net/http/httptest" + "time" + + appservice "github.com/codeready-toolchain/registration-service/pkg/application/service" + "github.com/codeready-toolchain/registration-service/pkg/auth" + "github.com/codeready-toolchain/registration-service/pkg/proxy/handlers" + "github.com/codeready-toolchain/registration-service/pkg/signup" + "github.com/codeready-toolchain/registration-service/test/fake" + "github.com/google/uuid" + + toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" + testconfig "github.com/codeready-toolchain/toolchain-common/pkg/test/config" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/labels" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +func (s *TestProxySuite) TestProxyCommunityEnabled() { + // given + + port := "30456" + + env := s.DefaultConfig().Environment() + defer s.SetConfig(testconfig.RegistrationService(). + Environment(env)) + s.SetConfig(testconfig.RegistrationService(). + Environment(string(testconfig.E2E))) // We use e2e-test environment just to be able to re-use token generation + _, err := auth.InitializeDefaultTokenParser() + require.NoError(s.T(), err) + + for _, environment := range []testconfig.EnvName{testconfig.E2E, testconfig.Dev, testconfig.Prod} { + s.Run("for environment "+string(environment), func() { + // spin up proxy + s.SetConfig( + testconfig.RegistrationService().Environment(string(environment)), + testconfig.PublicViewerConfig(true), + ) + fakeApp := &fake.ProxyFakeApp{} + p, server := s.spinUpProxy(fakeApp, port) + defer func() { + _ = server.Close() + }() + + // wait for proxy to be alive + s.Run("is alive", func() { + s.waitForProxyToBeAlive(port) + }) + s.Run("health check ok", func() { + s.checkProxyIsHealthy(port) + }) + + // run community tests + s.checkProxyCommunityOK(fakeApp, p, port) + }) + } +} + +func (s *TestProxySuite) checkProxyCommunityOK(fakeApp *fake.ProxyFakeApp, p *Proxy, port string) { + s.Run("successfully proxy", func() { + owner := uuid.New() + communityUser := uuid.New() + httpTestServerResponse := "my response" + + // Start the member-2 API Server + testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + // Set the Access-Control-Allow-Origin header to make sure it's overridden by the proxy response modifier + w.Header().Set("Access-Control-Allow-Origin", "dummy") + w.WriteHeader(http.StatusOK) + _, err := w.Write([]byte(httpTestServerResponse)) + require.NoError(s.T(), err) + })) + defer testServer.Close() + + type testCase struct { + ProxyRequestMethod string + ProxyRequestHeaders http.Header + ExpectedAPIServerRequestHeaders http.Header + ExpectedProxyResponseStatus int + RequestPath string + ExpectedResponse string + } + + tests := map[string]testCase{ + // Given smith2 owns a workspace named communityspace + // And communityspace is publicly visible (shared with PublicViewer) + // When smith2 requests the list of pods in workspace communityspace + // Then the request is forwarded from the proxy + // And the request impersonates smith2 + // And the request's X-SSO-User Header is set to smith2's ID + // And the request is successful + "plain http actual request as owner": { + ProxyRequestMethod: "GET", + ProxyRequestHeaders: map[string][]string{"Authorization": {"Bearer " + s.token(owner)}}, + ExpectedAPIServerRequestHeaders: map[string][]string{ + "Authorization": {"Bearer clusterSAToken"}, + "Impersonate-User": {"smith2"}, + "X-SSO-User": {"username-" + owner.String()}, + }, + ExpectedProxyResponseStatus: http.StatusOK, + RequestPath: fmt.Sprintf("http://localhost:%s/workspaces/communityspace/api/communityspace/pods", port), + ExpectedResponse: httpTestServerResponse, + }, + // Given smith2 owns a workspace named communityspace + // And communityspace is publicly visible (shared with PublicViewer) + // And a user named communityuser exists + // And smith2's communityspace is not directly shared with communityuser + // When communityuser requests the list of pods in workspace communityspace + // Then the request is forwarded from the proxy + // And the request impersonates the PublicViewer + // And the request's X-SSO-User Header is set to communityuser's ID + // And the request is successful + "plain http actual request as community user": { + ProxyRequestMethod: "GET", + ProxyRequestHeaders: map[string][]string{"Authorization": {"Bearer " + s.token(communityUser)}}, + ExpectedAPIServerRequestHeaders: map[string][]string{ + "Authorization": {"Bearer clusterSAToken"}, + "Impersonate-User": {toolchainv1alpha1.KubesawAuthenticatedUsername}, + "X-SSO-User": {"username-" + communityUser.String()}, + }, + ExpectedProxyResponseStatus: http.StatusOK, + RequestPath: fmt.Sprintf("http://localhost:%s/workspaces/communityspace/api/communityspace/pods", port), + ExpectedResponse: httpTestServerResponse, + }, + // Given user alice exists + // And alice owns a private workspace + // When smith2 requests the list of pods in alice's workspace + // Then the request is forwarded from the proxy + // And the request impersonates smith2 + // And the request's X-SSO-User Header is set to smith2's ID + // And the request is NOT successful + "plain http actual request as not owner to private workspace": { + ProxyRequestMethod: "GET", + ProxyRequestHeaders: map[string][]string{"Authorization": {"Bearer " + s.token(owner)}}, + ExpectedAPIServerRequestHeaders: map[string][]string{ + "Authorization": {"Bearer clusterSAToken"}, + "Impersonate-User": {"smith2"}, + "X-SSO-User": {"username-" + owner.String()}, + }, + ExpectedProxyResponseStatus: http.StatusForbidden, + RequestPath: fmt.Sprintf("http://localhost:%s/workspaces/alice-private/api/alice-private/pods", port), + ExpectedResponse: "invalid workspace request: access to workspace 'alice-private' is forbidden", + }, + } + + for k, tc := range tests { + s.Run(k, func() { + + // given + fakeApp.Err = nil + + testServer.Config.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + // Set the Access-Control-Allow-Origin header to make sure it's overridden by the proxy response modifier + w.Header().Set("Access-Control-Allow-Origin", "dummy") + w.WriteHeader(http.StatusOK) + _, err := w.Write([]byte("my response")) + require.NoError(s.T(), err) + for hk, hv := range tc.ExpectedAPIServerRequestHeaders { + require.Len(s.T(), r.Header.Values(hk), len(hv)) + for i := range hv { + assert.Equal(s.T(), hv[i], r.Header.Values(hk)[i], "header %s", hk) + } + } + }) + fakeApp.SignupServiceMock = fake.NewSignupService( + fake.Signup(owner.String(), &signup.Signup{ + Name: "smith2", + APIEndpoint: testServer.URL, + ClusterName: "member-2", + CompliantUsername: "smith2", + Username: "smith2@", + Status: signup.Status{ + Ready: true, + }, + }), + fake.Signup(communityUser.String(), &signup.Signup{ + Name: "communityUser", + APIEndpoint: testServer.URL, + ClusterName: "member-2", + CompliantUsername: "communityuser", + Username: "communityUser@", + Status: signup.Status{ + Ready: true, + }, + }), + fake.Signup(communityUser.String(), &signup.Signup{ + Name: "alice", + APIEndpoint: testServer.URL, + ClusterName: "member-2", + CompliantUsername: "alice", + Username: "alice@", + Status: signup.Status{ + Ready: true, + }, + }), + ) + s.Application.MockSignupService(fakeApp.SignupServiceMock) + inf := fake.NewFakeInformer() + inf.GetSpaceFunc = func(name string) (*toolchainv1alpha1.Space, error) { + switch name { + case "communityspace": + return fake.NewSpace("communityspace", "member-2", "smith2"), nil + case "alice-private": + return fake.NewSpace("alice-private", "member-2", "alice"), nil + } + return nil, fmt.Errorf("space not found error") + } + + sbmycoolSmith2 := fake.NewSpaceBinding("communityspace-smith2", "smith2", "communityspace", "admin") + commSpacePublicViewer := fake.NewSpaceBinding("communityspace-publicviewer", toolchainv1alpha1.KubesawAuthenticatedUsername, "communityspace", "viewer") + alicePrivate := fake.NewSpaceBinding("alice-default", "alice", "alice-default", "admin") + + cli := fake.InitClient(s.T(), sbmycoolSmith2, commSpacePublicViewer, alicePrivate) + inf.ListSpaceBindingFunc = func(reqs ...labels.Requirement) ([]toolchainv1alpha1.SpaceBinding, error) { + sbs := toolchainv1alpha1.SpaceBindingList{} + opts := &client.ListOptions{ + LabelSelector: labels.NewSelector().Add(reqs...), + } + log.Printf("received reqs: %v", reqs) + if err := cli.Client.List(context.TODO(), &sbs, opts); err != nil { + return nil, err + } + log.Printf("returning sbs: %v", sbs.Items) + return sbs.Items, nil + } + inf.GetProxyPluginConfigFunc = func(_ string) (*toolchainv1alpha1.ProxyPlugin, error) { + return nil, fmt.Errorf("proxy plugin not found") + } + inf.GetNSTemplateTierFunc = func(_ string) (*toolchainv1alpha1.NSTemplateTier, error) { + return fake.NewBase1NSTemplateTier(), nil + } + s.Application.MockInformerService(inf) + fakeApp.MemberClusterServiceMock = s.newMemberClusterServiceWithMembers(testServer.URL) + fakeApp.InformerServiceMock = inf + + p.spaceLister = &handlers.SpaceLister{ + GetSignupFunc: fakeApp.SignupServiceMock.GetSignupFromInformer, + GetInformerServiceFunc: func() appservice.InformerService { + return inf + }, + ProxyMetrics: p.metrics, + } + + // prepare request + req, err := http.NewRequest(tc.ProxyRequestMethod, tc.RequestPath, nil) + require.NoError(s.T(), err) + require.NotNil(s.T(), req) + + for hk, hv := range tc.ProxyRequestHeaders { + for _, v := range hv { + req.Header.Add(hk, v) + } + } + + // when + client := http.Client{Timeout: 3 * time.Second} + resp, err := client.Do(req) + + // then + require.NoError(s.T(), err) + require.NotNil(s.T(), resp) + defer resp.Body.Close() + assert.Equal(s.T(), tc.ExpectedProxyResponseStatus, resp.StatusCode) + s.assertResponseBody(resp, tc.ExpectedResponse) + }) + } + }) +} diff --git a/pkg/proxy/proxy_test.go b/pkg/proxy/proxy_test.go index d465cfda..b530b710 100644 --- a/pkg/proxy/proxy_test.go +++ b/pkg/proxy/proxy_test.go @@ -27,6 +27,7 @@ import ( "github.com/codeready-toolchain/registration-service/pkg/signup" "github.com/codeready-toolchain/registration-service/test" "github.com/codeready-toolchain/registration-service/test/fake" + "github.com/google/uuid" "github.com/prometheus/client_golang/prometheus" toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" @@ -35,7 +36,6 @@ import ( authsupport "github.com/codeready-toolchain/toolchain-common/pkg/test/auth" testconfig "github.com/codeready-toolchain/toolchain-common/pkg/test/config" - "github.com/google/uuid" routev1 "github.com/openshift/api/route/v1" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -72,66 +72,82 @@ func (s *TestProxySuite) TestProxy() { s.SetConfig(testconfig.RegistrationService(). Environment(string(environment))) fakeApp := &fake.ProxyFakeApp{} - proxyMetrics := metrics.NewProxyMetrics(prometheus.NewRegistry()) - p, err := newProxyWithClusterClient(fakeApp, nil, proxyMetrics, proxytest.NewGetMembersFunc(fake.InitClient(s.T()))) - require.NoError(s.T(), err) - - server := p.StartProxy(DefaultPort) - require.NotNil(s.T(), server) + p, server := s.spinUpProxy(fakeApp, DefaultPort) defer func() { _ = server.Close() }() - // Wait up to N seconds for the Proxy server to start - ready := false - sec := 10 - for i := 0; i < sec; i++ { - log.Println("Checking if Proxy is started...") - req, err := http.NewRequest("GET", "http://localhost:8081/api/mycoolworkspace/pods", nil) - require.NoError(s.T(), err) - require.NotNil(s.T(), req) - resp, err := http.DefaultClient.Do(req) - if err != nil { - time.Sleep(time.Second) - continue - } - _, _ = io.Copy(io.Discard, resp.Body) - _ = resp.Body.Close() - if resp.StatusCode != http.StatusUnauthorized { - // The server may be running but still not fully ready to accept requests - time.Sleep(time.Second) - continue - } - // Server is up and running! - ready = true - break - } - require.True(s.T(), ready, "Proxy is not ready after %d seconds", sec) - + s.Run("is alive", func() { + s.waitForProxyToBeAlive(DefaultPort) + }) s.Run("health check ok", func() { - req, err := http.NewRequest("GET", "http://localhost:8081/proxyhealth", nil) - require.NoError(s.T(), err) - require.NotNil(s.T(), req) - - // when - resp, err := http.DefaultClient.Do(req) - - // then - require.NoError(s.T(), err) - require.NotNil(s.T(), resp) - defer resp.Body.Close() - assert.Equal(s.T(), http.StatusOK, resp.StatusCode) - s.assertResponseBody(resp, `{"alive": true}`) + s.checkProxyIsHealthy(DefaultPort) }) s.checkPlainHTTPErrors(fakeApp) s.checkWebsocketsError() s.checkWebLogin() - s.checkProxyOK(fakeApp, p) + s.checkProxyOK(fakeApp, p, false) }) } } +func (s *TestProxySuite) spinUpProxy(fakeApp *fake.ProxyFakeApp, port string) (*Proxy, *http.Server) { + proxyMetrics := metrics.NewProxyMetrics(prometheus.NewRegistry()) + p, err := newProxyWithClusterClient( + fakeApp, nil, proxyMetrics, proxytest.NewGetMembersFunc(fake.InitClient(s.T()))) + require.NoError(s.T(), err) + + server := p.StartProxy(port) + require.NotNil(s.T(), server) + + return p, server +} + +func (s *TestProxySuite) waitForProxyToBeAlive(port string) { + // Wait up to N seconds for the Proxy server to start + ready := false + sec := 10 + for i := 0; i < sec; i++ { + log.Println("Checking if Proxy is started...") + req, err := http.NewRequest("GET", fmt.Sprintf("http://localhost:%s/api/mycoolworkspace/pods", port), nil) + require.NoError(s.T(), err) + require.NotNil(s.T(), req) + resp, err := http.DefaultClient.Do(req) + if err != nil { + time.Sleep(time.Second) + continue + } + _, _ = io.Copy(io.Discard, resp.Body) + _ = resp.Body.Close() + if resp.StatusCode != http.StatusUnauthorized { + // The server may be running but still not fully ready to accept requests + time.Sleep(time.Second) + continue + } + // Server is up and running! + ready = true + break + } + require.True(s.T(), ready, "Proxy is not ready after %d seconds", sec) +} + +func (s *TestProxySuite) checkProxyIsHealthy(port string) { + req, err := http.NewRequest("GET", fmt.Sprintf("http://localhost:%s/proxyhealth", port), nil) + require.NoError(s.T(), err) + require.NotNil(s.T(), req) + + // when + resp, err := http.DefaultClient.Do(req) + + // then + require.NoError(s.T(), err) + require.NotNil(s.T(), resp) + defer resp.Body.Close() + assert.Equal(s.T(), http.StatusOK, resp.StatusCode) + s.assertResponseBody(resp, `{"alive": true}`) +} + func (s *TestProxySuite) checkPlainHTTPErrors(fakeApp *fake.ProxyFakeApp) { s.Run("plain http error", func() { s.Run("unauthorized if no token present", func() { @@ -382,7 +398,7 @@ func (s *TestProxySuite) checkWebLogin() { }) } -func (s *TestProxySuite) checkProxyOK(fakeApp *fake.ProxyFakeApp, p *Proxy) { +func (s *TestProxySuite) checkProxyOK(fakeApp *fake.ProxyFakeApp, p *Proxy, publicViewerEnabled bool) { s.Run("successfully proxy", func() { userID := uuid.New() @@ -665,6 +681,9 @@ func (s *TestProxySuite) checkProxyOK(fakeApp *fake.ProxyFakeApp, p *Proxy) { if req.Values().List()[0] == "smith2" || req.Values().List()[0] == "mycoolworkspace" { spaceBindings = []toolchainv1alpha1.SpaceBinding{*fake.NewSpaceBinding("mycoolworkspace-smith2", "smith2", "mycoolworkspace", "admin")} } + if publicViewerEnabled && req.Values().List()[0] == toolchainv1alpha1.KubesawAuthenticatedUsername { + spaceBindings = []toolchainv1alpha1.SpaceBinding{*fake.NewSpaceBinding("communityspace-publicviewer", "publicviewer", "communityspace", "viewer")} + } } return spaceBindings, nil } @@ -692,6 +711,7 @@ func (s *TestProxySuite) checkProxyOK(fakeApp *fake.ProxyFakeApp, p *Proxy) { } s.Application.MockInformerService(inf) fakeApp.MemberClusterServiceMock = s.newMemberClusterServiceWithMembers(testServer.URL) + fakeApp.InformerServiceMock = inf p.spaceLister = &handlers.SpaceLister{ GetSignupFunc: fakeApp.SignupServiceMock.GetSignupFromInformer, diff --git a/pkg/proxy/service/cluster_service.go b/pkg/proxy/service/cluster_service.go index 95551105..ca7ca26f 100644 --- a/pkg/proxy/service/cluster_service.go +++ b/pkg/proxy/service/cluster_service.go @@ -40,21 +40,21 @@ func NewMemberClusterService(context servicecontext.ServiceContext, options ...O return si } -func (s *ServiceImpl) GetClusterAccess(userID, username, workspace, proxyPluginName string) (*access.ClusterAccess, error) { - signup, err := s.Services().SignupService().GetSignupFromInformer(nil, userID, username, false) // don't check for usersignup complete status, since it might cause the proxy blocking the request and returning an error when quick transitions from ready to provisioning are happening. - if err != nil { - return nil, err - } - // if signup has the CompliantUsername set it means that MUR was created and useraccount is provisioned - if signup == nil || signup.CompliantUsername == "" { - cause := errs.New("user is not provisioned (yet)") - log.Error(nil, cause, fmt.Sprintf("signup object: %+v", signup)) - return nil, cause - } - +func (s *ServiceImpl) GetClusterAccess(userID, username, workspace, proxyPluginName string, publicViewerEnabled bool) (*access.ClusterAccess, error) { // if workspace is not provided then return the default space access if workspace == "" { - return s.accessForCluster(signup.APIEndpoint, signup.ClusterName, signup.CompliantUsername, proxyPluginName) + return s.getClusterAccessForDefaultWorkspace(userID, username, proxyPluginName) + } + + return s.getSpaceAccess(userID, username, workspace, proxyPluginName, publicViewerEnabled) +} + +// getSpaceAccess retrieves space access for an user +func (s *ServiceImpl) getSpaceAccess(userID, username, workspace, proxyPluginName string, publicViewerEnabled bool) (*access.ClusterAccess, error) { + // retrieve the user's complaint name + userName, err := s.getUserSignupComplaintName(userID, username, publicViewerEnabled) + if err != nil { + return nil, err } // look up space @@ -65,7 +65,49 @@ func (s *ServiceImpl) GetClusterAccess(userID, username, workspace, proxyPluginN return nil, fmt.Errorf("the requested space is not available") } - return s.accessForSpace(space, signup.CompliantUsername, proxyPluginName) + return s.accessForSpace(space, userName, proxyPluginName) +} + +func (s *ServiceImpl) getUserSignupComplaintName(userID, username string, publicViewerEnabled bool) (string, error) { + // if PublicViewer is enabled and the requested user is the PublicViewer, than no lookup is required + if publicViewerEnabled && username == userID && userID == toolchainv1alpha1.KubesawAuthenticatedUsername { + return username, nil + } + + // retrieve the UserSignup from cache + // + // don't check for usersignup complete status, since it might cause the proxy blocking the request + // and returning an error when quick transitions from ready to provisioning are happening. + userSignup, err := s.Services().SignupService().GetSignupFromInformer(nil, userID, username, false) + if err != nil { + return "", err + } + // if signup has the CompliantUsername set it means that MUR was created and useraccount is provisioned + if userSignup == nil || userSignup.CompliantUsername == "" { + cause := errs.New("user is not provisioned (yet)") + log.Error(nil, cause, fmt.Sprintf("signup object: %+v", userSignup)) + return "", cause + } + + return userSignup.CompliantUsername, nil +} + +// getClusterAccessForDefaultWorkspace retrieves the cluster for the user's default workspace +func (s *ServiceImpl) getClusterAccessForDefaultWorkspace(userID, username, proxyPluginName string) (*access.ClusterAccess, error) { + // don't check for usersignup complete status, since it might cause the proxy blocking the request + // and returning an error when quick transitions from ready to provisioning are happening. + signup, err := s.Services().SignupService().GetSignupFromInformer(nil, userID, username, false) + if err != nil { + return nil, err + } + // if signup has the CompliantUsername set it means that MUR was created and useraccount is provisioned + if signup == nil || signup.CompliantUsername == "" { + cause := errs.New("user is not provisioned (yet)") + log.Error(nil, cause, fmt.Sprintf("signup object: %+v", signup)) + return nil, cause + } + + return s.accessForCluster(signup.APIEndpoint, signup.ClusterName, signup.CompliantUsername, proxyPluginName) } func (s *ServiceImpl) accessForSpace(space *toolchainv1alpha1.Space, username, proxyPluginName string) (*access.ClusterAccess, error) { diff --git a/pkg/proxy/service/cluster_service_test.go b/pkg/proxy/service/cluster_service_test.go index 6ec55e78..a32dc217 100644 --- a/pkg/proxy/service/cluster_service_test.go +++ b/pkg/proxy/service/cluster_service_test.go @@ -36,7 +36,15 @@ func TestRunClusterServiceSuite(t *testing.T) { suite.Run(t, &TestClusterServiceSuite{test.UnitTestSuite{}}) } -func (s *TestClusterServiceSuite) TestGetClusterAccess() { +func (s *TestClusterServiceSuite) TestGetClusterAccessCommunityDisabled() { + s.testGetClusterAccessCommon(false) +} + +func (s *TestClusterServiceSuite) TestGetClusterAccessCommunityEnabled() { + s.testGetClusterAccessCommon(true) +} + +func (s *TestClusterServiceSuite) testGetClusterAccessCommon(publicViewerEnabled bool) { // given sc := fake.NewSignupService(fake.Signup("123-noise", &signup.Signup{ CompliantUsername: "noise1", @@ -114,7 +122,7 @@ func (s *TestClusterServiceSuite) TestGetClusterAccess() { } // when - _, err := svc.GetClusterAccess("789-ready", "", "", "") + _, err := svc.GetClusterAccess("789-ready", "", "", "", publicViewerEnabled) // then require.EqualError(s.T(), err, "oopsi woopsi") @@ -124,7 +132,7 @@ func (s *TestClusterServiceSuite) TestGetClusterAccess() { s.Run("userid is not found", func() { // when - _, err := svc.GetClusterAccess("unknown_id", "", "", "") + _, err := svc.GetClusterAccess("unknown_id", "", "", "", publicViewerEnabled) // then require.EqualError(s.T(), err, "user is not provisioned (yet)") @@ -132,7 +140,7 @@ func (s *TestClusterServiceSuite) TestGetClusterAccess() { s.Run("username is not found", func() { // when - _, err := svc.GetClusterAccess("", "unknown_username", "", "") + _, err := svc.GetClusterAccess("", "unknown_username", "", "", publicViewerEnabled) // then require.EqualError(s.T(), err, "user is not provisioned (yet)") @@ -140,7 +148,7 @@ func (s *TestClusterServiceSuite) TestGetClusterAccess() { s.Run("user is not provisioned yet", func() { // when - _, err := svc.GetClusterAccess("456-not-ready", "", "", "") + _, err := svc.GetClusterAccess("456-not-ready", "", "", "", publicViewerEnabled) // then require.EqualError(s.T(), err, "user is not provisioned (yet)") @@ -160,7 +168,7 @@ func (s *TestClusterServiceSuite) TestGetClusterAccess() { s.Application.MockInformerService(inf) // when - _, err := svc.GetClusterAccess("789-ready", "", "smith2", "") + _, err := svc.GetClusterAccess("789-ready", "", "smith2", "", publicViewerEnabled) // then // original error is only logged so that it doesn't reveal information about a space that may not belong to the requestor @@ -169,7 +177,7 @@ func (s *TestClusterServiceSuite) TestGetClusterAccess() { s.Run("space not found", func() { // when - _, err := svc.GetClusterAccess("789-ready", "", "unknown", "") // unknown workspace requested + _, err := svc.GetClusterAccess("789-ready", "", "unknown", "", publicViewerEnabled) // unknown workspace requested // then require.EqualError(s.T(), err, "the requested space is not available") @@ -191,7 +199,7 @@ func (s *TestClusterServiceSuite) TestGetClusterAccess() { ) s.Run("default workspace case", func() { // when - _, err := svc.GetClusterAccess("789-ready", "", "", "") + _, err := svc.GetClusterAccess("789-ready", "", "", "", publicViewerEnabled) // then require.EqualError(s.T(), err, "no member clusters found") @@ -199,7 +207,7 @@ func (s *TestClusterServiceSuite) TestGetClusterAccess() { s.Run("workspace context case", func() { // when - _, err := svc.GetClusterAccess("789-ready", "", "smith2", "") + _, err := svc.GetClusterAccess("789-ready", "", "smith2", "", publicViewerEnabled) // then require.EqualError(s.T(), err, "no member clusters found") @@ -221,7 +229,7 @@ func (s *TestClusterServiceSuite) TestGetClusterAccess() { s.Run("default workspace case", func() { // when - _, err := svc.GetClusterAccess("012-ready-unknown-cluster", "", "", "") + _, err := svc.GetClusterAccess("012-ready-unknown-cluster", "", "", "", publicViewerEnabled) // then require.EqualError(s.T(), err, "no member cluster found for the user") @@ -229,7 +237,7 @@ func (s *TestClusterServiceSuite) TestGetClusterAccess() { s.Run("workspace context case", func() { // when - _, err := svc.GetClusterAccess("012-ready-unknown-cluster", "", "unknown-cluster", "") + _, err := svc.GetClusterAccess("012-ready-unknown-cluster", "", "unknown-cluster", "", publicViewerEnabled) // then require.EqualError(s.T(), err, "no member cluster found for space 'unknown-cluster'") @@ -299,7 +307,7 @@ func (s *TestClusterServiceSuite) TestGetClusterAccess() { expectedToken := "abc123" // should match member 2 bearer token // when - ca, err := svc.GetClusterAccess("789-ready", "", "", "tekton-results") + ca, err := svc.GetClusterAccess("789-ready", "", "", "tekton-results", publicViewerEnabled) // then require.NoError(s.T(), err) @@ -312,7 +320,7 @@ func (s *TestClusterServiceSuite) TestGetClusterAccess() { s.Run("cluster access correct when username provided", func() { // when - ca, err := svc.GetClusterAccess("", "smith@", "", "tekton-results") + ca, err := svc.GetClusterAccess("", "smith@", "", "tekton-results", publicViewerEnabled) // then require.NoError(s.T(), err) @@ -325,7 +333,7 @@ func (s *TestClusterServiceSuite) TestGetClusterAccess() { s.Run("cluster access correct when using workspace context", func() { // when - ca, err := svc.GetClusterAccess("789-ready", "", "smith2", "tekton-results") // workspace-context specified + ca, err := svc.GetClusterAccess("789-ready", "", "smith2", "tekton-results", publicViewerEnabled) // workspace-context specified // then require.NoError(s.T(), err) @@ -353,7 +361,7 @@ func (s *TestClusterServiceSuite) TestGetClusterAccess() { return memberClient.Client.Get(ctx, key, obj, opts...) } memberArray[0].Client = mC - ca, err := svc.GetClusterAccess("789-ready", "", "teamspace", "tekton-results") // workspace-context specified + ca, err := svc.GetClusterAccess("789-ready", "", "teamspace", "tekton-results", publicViewerEnabled) // workspace-context specified // then require.NoError(s.T(), err) @@ -371,7 +379,7 @@ func (s *TestClusterServiceSuite) TestGetClusterAccess() { expectedToken := "abc123" // should match member 2 bearer token // when - ca, err := svc.GetClusterAccess("789-ready", "", "", "") + ca, err := svc.GetClusterAccess("789-ready", "", "", "", publicViewerEnabled) // then require.NoError(s.T(), err) @@ -384,7 +392,7 @@ func (s *TestClusterServiceSuite) TestGetClusterAccess() { s.Run("cluster access correct when username provided", func() { // when - ca, err := svc.GetClusterAccess("", "smith@", "", "") + ca, err := svc.GetClusterAccess("", "smith@", "", "", publicViewerEnabled) // then require.NoError(s.T(), err) @@ -397,7 +405,7 @@ func (s *TestClusterServiceSuite) TestGetClusterAccess() { s.Run("cluster access correct when using workspace context", func() { // when - ca, err := svc.GetClusterAccess("789-ready", "", "smith2", "") // workspace-context specified + ca, err := svc.GetClusterAccess("789-ready", "", "smith2", "", publicViewerEnabled) // workspace-context specified // then require.NoError(s.T(), err) @@ -409,7 +417,7 @@ func (s *TestClusterServiceSuite) TestGetClusterAccess() { s.Run("another workspace on another cluster", func() { // when - ca, err := svc.GetClusterAccess("789-ready", "", "teamspace", "") // workspace-context specified + ca, err := svc.GetClusterAccess("789-ready", "", "teamspace", "", publicViewerEnabled) // workspace-context specified // then require.NoError(s.T(), err) diff --git a/pkg/server/in_cluster_application.go b/pkg/server/in_cluster_application.go index 979cba2d..ad11922b 100644 --- a/pkg/server/in_cluster_application.go +++ b/pkg/server/in_cluster_application.go @@ -29,7 +29,8 @@ func NewInClusterApplication(informer informers.Informer) (application.Applicati serviceFactory: factory.NewServiceFactory( factory.WithServiceContextOptions(factory.CRTClientOption(kubeClient), factory.InformerOption(informer), - )), + ), + ), }, nil } diff --git a/test/fake/proxy.go b/test/fake/proxy.go index bad15e29..22d350cd 100644 --- a/test/fake/proxy.go +++ b/test/fake/proxy.go @@ -17,10 +17,15 @@ type ProxyFakeApp struct { Err error SignupServiceMock service.SignupService MemberClusterServiceMock service.MemberClusterService + InformerServiceMock service.InformerService } func (a *ProxyFakeApp) InformerService() service.InformerService { - panic("InformerService shouldn't be called") + if a.InformerServiceMock == nil { + panic("InformerService shouldn't be called") + } + + return a.InformerServiceMock } func (a *ProxyFakeApp) SignupService() service.SignupService { @@ -45,7 +50,7 @@ type fakeClusterService struct { fakeApp *ProxyFakeApp } -func (f *fakeClusterService) GetClusterAccess(userID, _, _, _ string) (*access.ClusterAccess, error) { +func (f *fakeClusterService) GetClusterAccess(userID, _, _, _ string, _ bool) (*access.ClusterAccess, error) { return f.fakeApp.Accesses[userID], f.fakeApp.Err }