From 5279861e84c23f749eb4a1fac8ec79dfff7e9e64 Mon Sep 17 00:00:00 2001 From: Dave Page Date: Fri, 7 Aug 2026 15:08:30 +0100 Subject: [PATCH] Repoint handler tests at the WithSecurity constructors The follow-up to the dead-code removal. NewConnectionHandler and NewNotificationChannelHandler are unreachable from the server binary, because cmd/mcp-server wires both handlers through the WithSecurity variants instead. They survived the mechanical pass because 49 test call sites still used the shorter forms, and deleting them there would have taken out tests covering live handler code. The two constructor pairs are identical apart from one line: the short form sets hostValidator to DefaultHostValidator(), and the WithSecurity form sets it to NewHostValidator(allowInternal, allowedHosts, blockedHosts). Since DefaultHostValidator() is defined as exactly NewHostValidator(false, nil, nil), rewriting a three-argument call to pass an additional (false, nil, nil) is precisely equivalent, and no test changes behaviour as a result. With the short forms gone, DefaultHostValidator had no production caller either, so it goes too. Its two incidental uses in host_validation_test.go now call NewHostValidator(false, nil, nil) directly, and TestDefaultHostValidator is removed along with the function it existed to test. Server coverage is 55.5%, matching main exactly. The only failing tests are the two pre-existing vector(3) fixture failures from #337, which reproduce identically on unmodified main. This branch is cut from main rather than stacked on the dead-code branch, so CI runs against it. The two changesets touch entirely disjoint files and can merge in either order. --- docs/changelog.md | 12 ++++++++ .../src/internal/api/connection_handlers.go | 10 ------- .../internal/api/connection_handlers_test.go | 30 +++++++++---------- server/src/internal/api/host_validation.go | 7 ----- .../src/internal/api/host_validation_test.go | 20 ++----------- .../api/issue269_connection_name_test.go | 4 +-- .../api/notification_channel_handlers.go | 14 --------- .../api/notification_channel_handlers_test.go | 14 ++++----- .../src/internal/api/query_handlers_test.go | 2 +- .../src/internal/api/rbac_integration_test.go | 16 +++++----- .../api/rbac_issue233_connections_test.go | 8 ++--- server/src/internal/api/rbac_issue35_test.go | 24 +++++++-------- 12 files changed, 63 insertions(+), 98 deletions(-) diff --git a/docs/changelog.md b/docs/changelog.md index 1d847e9f..53802e48 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -253,6 +253,18 @@ project adheres to 512M; the collector connection-pool and timeout options are now documented in the sample configuration. (#308) +### Removed + +- Remove the superseded `NewConnectionHandler` and + `NewNotificationChannelHandler` constructors, along with the + `DefaultHostValidator` helper that only they used. The server has + wired both handlers through the `NewConnectionHandlerWithSecurity` and + `NewNotificationChannelHandlerWithSecurity` variants for some time, so + the shorter forms were unreachable in production whilst 49 test call + sites still used them. Those call sites now use the `WithSecurity` + constructors directly, passing the same host-validation settings the + removed helper supplied, so test behaviour is unchanged. + ### Security - Ignore a blank password when updating a database connection, so an diff --git a/server/src/internal/api/connection_handlers.go b/server/src/internal/api/connection_handlers.go index 7b969403..0a74753c 100644 --- a/server/src/internal/api/connection_handlers.go +++ b/server/src/internal/api/connection_handlers.go @@ -36,16 +36,6 @@ type ConnectionHandler struct { visibilityListerFn func() auth.ConnectionVisibilityLister } -// NewConnectionHandler creates a new connection handler -func NewConnectionHandler(datastore *database.Datastore, authStore *auth.AuthStore, rbacChecker *auth.RBACChecker) *ConnectionHandler { - return &ConnectionHandler{ - datastore: datastore, - authStore: authStore, - hostValidator: DefaultHostValidator(), - rbacChecker: rbacChecker, - } -} - // NewConnectionHandlerWithSecurity creates a new connection handler with custom security settings func NewConnectionHandlerWithSecurity(datastore *database.Datastore, authStore *auth.AuthStore, rbacChecker *auth.RBACChecker, allowInternal bool, allowedHosts, blockedHosts []string) *ConnectionHandler { diff --git a/server/src/internal/api/connection_handlers_test.go b/server/src/internal/api/connection_handlers_test.go index f89a4145..e22ca318 100644 --- a/server/src/internal/api/connection_handlers_test.go +++ b/server/src/internal/api/connection_handlers_test.go @@ -26,7 +26,7 @@ import ( ) func TestNewConnectionHandler(t *testing.T) { - handler := NewConnectionHandler(nil, nil, nil) + handler := NewConnectionHandlerWithSecurity(nil, nil, nil, false, nil, nil) if handler == nil { t.Fatal("NewConnectionHandler returned nil") } @@ -79,7 +79,7 @@ func TestConnectionHandler_HandleNotConfigured(t *testing.T) { } func TestConnectionHandler_HandleConnections_MethodNotAllowed(t *testing.T) { - handler := NewConnectionHandler(nil, nil, nil) + handler := NewConnectionHandlerWithSecurity(nil, nil, nil, false, nil, nil) tests := []struct { name string @@ -111,7 +111,7 @@ func TestConnectionHandler_HandleConnections_MethodNotAllowed(t *testing.T) { } func TestConnectionHandler_HandleConnectionSubpath_InvalidID(t *testing.T) { - handler := NewConnectionHandler(nil, nil, nil) + handler := NewConnectionHandlerWithSecurity(nil, nil, nil, false, nil, nil) req := httptest.NewRequest(http.MethodGet, "/api/v1/connections/abc", nil) rec := httptest.NewRecorder() @@ -133,7 +133,7 @@ func TestConnectionHandler_HandleConnectionSubpath_InvalidID(t *testing.T) { } func TestConnectionHandler_HandleConnectionSubpath_MethodNotAllowed(t *testing.T) { - handler := NewConnectionHandler(nil, nil, nil) + handler := NewConnectionHandlerWithSecurity(nil, nil, nil, false, nil, nil) req := httptest.NewRequest(http.MethodPatch, "/api/v1/connections/1", nil) rec := httptest.NewRecorder() @@ -151,7 +151,7 @@ func TestConnectionHandler_HandleConnectionSubpath_MethodNotAllowed(t *testing.T } func TestConnectionHandler_HandleConnectionSubpath_DatabasesMethodNotAllowed(t *testing.T) { - handler := NewConnectionHandler(nil, nil, nil) + handler := NewConnectionHandlerWithSecurity(nil, nil, nil, false, nil, nil) req := httptest.NewRequest(http.MethodPost, "/api/v1/connections/1/databases", nil) rec := httptest.NewRecorder() @@ -169,7 +169,7 @@ func TestConnectionHandler_HandleConnectionSubpath_DatabasesMethodNotAllowed(t * } func TestConnectionHandler_HandleCurrentConnection_MethodNotAllowed(t *testing.T) { - handler := NewConnectionHandler(nil, nil, nil) + handler := NewConnectionHandlerWithSecurity(nil, nil, nil, false, nil, nil) req := httptest.NewRequest(http.MethodPatch, "/api/v1/connections/current", nil) req.Header.Set("Authorization", "Bearer testtoken") @@ -188,7 +188,7 @@ func TestConnectionHandler_HandleCurrentConnection_MethodNotAllowed(t *testing.T } func TestConnectionHandler_HandleCurrentConnection_MissingAuth(t *testing.T) { - handler := NewConnectionHandler(nil, nil, nil) + handler := NewConnectionHandlerWithSecurity(nil, nil, nil, false, nil, nil) req := httptest.NewRequest(http.MethodGet, "/api/v1/connections/current", nil) rec := httptest.NewRecorder() @@ -210,7 +210,7 @@ func TestConnectionHandler_HandleCurrentConnection_MissingAuth(t *testing.T) { } func TestConnectionHandler_RegisterRoutes_NotConfigured(t *testing.T) { - handler := NewConnectionHandler(nil, nil, nil) + handler := NewConnectionHandlerWithSecurity(nil, nil, nil, false, nil, nil) mux := http.NewServeMux() noopWrapper := func(h http.HandlerFunc) http.HandlerFunc { return h } @@ -237,7 +237,7 @@ func TestConnectionHandler_RegisterRoutes_NotConfigured(t *testing.T) { func TestConnectionHandler_CreateConnection_NoAuth(t *testing.T) { // Test that createConnection requires authentication rbac := auth.NewRBACChecker(nil) - handler := NewConnectionHandler(nil, nil, rbac) + handler := NewConnectionHandlerWithSecurity(nil, nil, rbac, false, nil, nil) body, _ := json.Marshal(ConnectionCreateRequest{ Name: "test", @@ -273,7 +273,7 @@ func TestConnectionHandler_CreateConnection_NoAuth(t *testing.T) { func TestConnectionHandler_UpdateConnection_NoAuth(t *testing.T) { // Test that updateConnection requires authentication rbac := auth.NewRBACChecker(nil) - handler := NewConnectionHandler(nil, nil, rbac) + handler := NewConnectionHandlerWithSecurity(nil, nil, rbac, false, nil, nil) body, _ := json.Marshal(ConnectionFullUpdateRequest{}) req := httptest.NewRequest(http.MethodPut, "/api/v1/connections/1", @@ -299,7 +299,7 @@ func TestConnectionHandler_UpdateConnection_NoAuth(t *testing.T) { } func TestConnectionHandler_SetCurrentConnection_InvalidConnectionID(t *testing.T) { - handler := NewConnectionHandler(nil, nil, nil) + handler := NewConnectionHandlerWithSecurity(nil, nil, nil, false, nil, nil) body, _ := json.Marshal(CurrentConnectionRequest{ConnectionID: 0}) req := httptest.NewRequest(http.MethodPost, "/api/v1/connections/current", @@ -453,7 +453,7 @@ func TestCurrentConnectionResponse_JSON(t *testing.T) { } func TestConnectionHandler_HandleSubpath_NotFound(t *testing.T) { - handler := NewConnectionHandler(nil, nil, nil) + handler := NewConnectionHandlerWithSecurity(nil, nil, nil, false, nil, nil) // Test unknown subpath req := httptest.NewRequest(http.MethodGet, "/api/v1/connections/1/unknown", nil) @@ -467,7 +467,7 @@ func TestConnectionHandler_HandleSubpath_NotFound(t *testing.T) { } func TestConnectionHandler_HandleSubpath_EmptyPath(t *testing.T) { - handler := NewConnectionHandler(nil, nil, nil) + handler := NewConnectionHandlerWithSecurity(nil, nil, nil, false, nil, nil) req := httptest.NewRequest(http.MethodGet, "/api/v1/connections/", nil) rec := httptest.NewRecorder() @@ -649,7 +649,7 @@ func TestListConnectionsScopedTokenReturnsScopedConnection(t *testing.T) { return ds.GetConnectionSharingInfo(ctx, id) }, ) - handler := NewConnectionHandler(ds, store, checker) + handler := NewConnectionHandlerWithSecurity(ds, store, checker, false, nil, nil) req := httptest.NewRequest(http.MethodGet, "/api/v1/connections", nil) ctx := req.Context() @@ -761,7 +761,7 @@ func newListConnectionsIssue68Handler(ds *database.Datastore, store *auth.AuthSt return ds.GetConnectionSharingInfo(ctx, id) }, ) - return NewConnectionHandler(ds, store, checker) + return NewConnectionHandlerWithSecurity(ds, store, checker, false, nil, nil) } // TestListConnections_Issue68_Superuser_ReturnsAllConnections locks in diff --git a/server/src/internal/api/host_validation.go b/server/src/internal/api/host_validation.go index c732c22c..1333b3ef 100644 --- a/server/src/internal/api/host_validation.go +++ b/server/src/internal/api/host_validation.go @@ -196,10 +196,3 @@ func (v *HostValidator) ValidatePort(port int) error { return nil } - -// DefaultHostValidator returns a validator with secure defaults: -// - Blocks internal network connections -// - No allowed/blocked host lists -func DefaultHostValidator() *HostValidator { - return NewHostValidator(false, nil, nil) -} diff --git a/server/src/internal/api/host_validation_test.go b/server/src/internal/api/host_validation_test.go index 3bd1e035..228a6aeb 100644 --- a/server/src/internal/api/host_validation_test.go +++ b/server/src/internal/api/host_validation_test.go @@ -67,22 +67,6 @@ func TestNewHostValidator(t *testing.T) { } } -func TestDefaultHostValidator(t *testing.T) { - v := DefaultHostValidator() - if v == nil { - t.Fatal("DefaultHostValidator returned nil") - } - if v.AllowInternalNetworks { - t.Error("Default validator should not allow internal networks") - } - if len(v.AllowedHosts) != 0 { - t.Error("Default validator should have empty allowed hosts") - } - if len(v.BlockedHosts) != 0 { - t.Error("Default validator should have empty blocked hosts") - } -} - func TestHostValidator_ValidateHost(t *testing.T) { tests := []struct { name string @@ -239,7 +223,7 @@ func TestHostValidator_ValidateHost(t *testing.T) { } func TestHostValidator_ValidatePort(t *testing.T) { - v := DefaultHostValidator() + v := NewHostValidator(false, nil, nil) tests := []struct { name string @@ -403,7 +387,7 @@ func TestHostValidator_CIDRParsing(t *testing.T) { } func TestHostValidator_InternalNetworksList(t *testing.T) { - v := DefaultHostValidator() + v := NewHostValidator(false, nil, nil) // Test that common internal ranges are blocked internalIPs := []string{ diff --git a/server/src/internal/api/issue269_connection_name_test.go b/server/src/internal/api/issue269_connection_name_test.go index 5bb6d98c..bfdecd58 100644 --- a/server/src/internal/api/issue269_connection_name_test.go +++ b/server/src/internal/api/issue269_connection_name_test.go @@ -216,7 +216,7 @@ func TestConnectionHandler_UpdateConnection_Issue269_InvalidChars(t *testing.T) seedIssue269Connection(t, pool, connID, owner, "valid-name") checker := auth.NewRBACChecker(store) - handler := NewConnectionHandler(ds, store, checker) + handler := NewConnectionHandlerWithSecurity(ds, store, checker, false, nil, nil) invalid := issue269InvalidName body, _ := json.Marshal(ConnectionFullUpdateRequest{Name: &invalid}) @@ -254,7 +254,7 @@ func TestConnectionHandler_UpdateConnection_Issue269_ValidNameSucceeds(t *testin seedIssue269Connection(t, pool, connID, owner, "old-name") checker := auth.NewRBACChecker(store) - handler := NewConnectionHandler(ds, store, checker) + handler := NewConnectionHandlerWithSecurity(ds, store, checker, false, nil, nil) valid := "New Cluster (primary) - east_1.db" body, _ := json.Marshal(ConnectionFullUpdateRequest{Name: &valid}) diff --git a/server/src/internal/api/notification_channel_handlers.go b/server/src/internal/api/notification_channel_handlers.go index 9646f302..124f905b 100644 --- a/server/src/internal/api/notification_channel_handlers.go +++ b/server/src/internal/api/notification_channel_handlers.go @@ -32,20 +32,6 @@ type NotificationChannelHandler struct { checkPermission func(http.ResponseWriter, *http.Request) bool } -// NewNotificationChannelHandler creates a new notification channel handler -func NewNotificationChannelHandler(datastore *database.Datastore, authStore *auth.AuthStore, rbacChecker *auth.RBACChecker) *NotificationChannelHandler { - h := &NotificationChannelHandler{ - datastore: datastore, - authStore: authStore, - rbacChecker: rbacChecker, - hostValidator: DefaultHostValidator(), - } - if rbacChecker != nil { - h.checkPermission = RequireAdminPermission(rbacChecker, auth.PermManageNotificationChannels, "manage notification channels") - } - return h -} - // NewNotificationChannelHandlerWithSecurity creates a new notification channel handler with custom security settings func NewNotificationChannelHandlerWithSecurity(datastore *database.Datastore, authStore *auth.AuthStore, rbacChecker *auth.RBACChecker, allowInternal bool, allowedHosts, blockedHosts []string) *NotificationChannelHandler { diff --git a/server/src/internal/api/notification_channel_handlers_test.go b/server/src/internal/api/notification_channel_handlers_test.go index 91eab6cd..ebac87aa 100644 --- a/server/src/internal/api/notification_channel_handlers_test.go +++ b/server/src/internal/api/notification_channel_handlers_test.go @@ -39,7 +39,7 @@ import ( // the handler is constructed without a datastore, every route under // `/api/v1/notification-channels` returns 503. func TestNotificationChannelHandler_NotConfiguredRoutes(t *testing.T) { - handler := NewNotificationChannelHandler(nil, nil, nil) + handler := NewNotificationChannelHandlerWithSecurity(nil, nil, nil, false, nil, nil) mux := http.NewServeMux() noopWrapper := func(h http.HandlerFunc) http.HandlerFunc { return h } handler.RegisterRoutes(mux, noopWrapper) @@ -67,7 +67,7 @@ func TestNotificationChannelHandler_NotConfiguredRoutes(t *testing.T) { func TestNotificationChannelHandler_MethodNotAllowed(t *testing.T) { authStore, cleanup := newAuthStoreForChannelTests(t) defer cleanup() - handler := NewNotificationChannelHandler(nil, authStore, auth.NewRBACChecker(authStore)) + handler := NewNotificationChannelHandlerWithSecurity(nil, authStore, auth.NewRBACChecker(authStore), false, nil, nil) cases := []struct { path string @@ -115,7 +115,7 @@ func TestNotificationChannelHandler_MethodNotAllowed(t *testing.T) { func TestNotificationChannelHandler_InvalidIDs(t *testing.T) { authStore, cleanup := newAuthStoreForChannelTests(t) defer cleanup() - handler := NewNotificationChannelHandler(nil, authStore, auth.NewRBACChecker(authStore)) + handler := NewNotificationChannelHandlerWithSecurity(nil, authStore, auth.NewRBACChecker(authStore), false, nil, nil) cases := []struct { path string @@ -147,7 +147,7 @@ func TestNotificationChannelHandler_InvalidIDs(t *testing.T) { func TestNotificationChannelHandler_PermissionRequired(t *testing.T) { authStore, cleanup := newAuthStoreForChannelTests(t) defer cleanup() - handler := NewNotificationChannelHandler(nil, authStore, auth.NewRBACChecker(authStore)) + handler := NewNotificationChannelHandlerWithSecurity(nil, authStore, auth.NewRBACChecker(authStore), false, nil, nil) for _, method := range []string{http.MethodGet, http.MethodPost} { req := httptest.NewRequest(method, "/api/v1/notification-channels", nil) @@ -172,7 +172,7 @@ func TestNotificationChannelHandler_PermissionRequired(t *testing.T) { func TestNotificationChannelHandler_NotFoundPaths(t *testing.T) { authStore, cleanup := newAuthStoreForChannelTests(t) defer cleanup() - handler := NewNotificationChannelHandler(nil, authStore, auth.NewRBACChecker(authStore)) + handler := NewNotificationChannelHandlerWithSecurity(nil, authStore, auth.NewRBACChecker(authStore), false, nil, nil) paths := []string{ "/api/v1/notification-channels/", @@ -303,7 +303,7 @@ func setupChannelHandler(t *testing.T, ds *database.Datastore) (*NotificationCha userID := setupUserWithPermission(t, authStore, "channel_admin", auth.PermManageNotificationChannels) checker := auth.NewRBACChecker(authStore) - handler := NewNotificationChannelHandler(ds, authStore, checker) + handler := NewNotificationChannelHandlerWithSecurity(ds, authStore, checker, false, nil, nil) return handler, userID, cleanup } @@ -1178,7 +1178,7 @@ func TestCreateChannel_ValidationErrors(t *testing.T) { userID := setupUserWithPermission(t, authStore, "ch_validator", auth.PermManageNotificationChannels) checker := auth.NewRBACChecker(authStore) - handler := NewNotificationChannelHandler(nil, authStore, checker) + handler := NewNotificationChannelHandlerWithSecurity(nil, authStore, checker, false, nil, nil) cases := []struct { name string diff --git a/server/src/internal/api/query_handlers_test.go b/server/src/internal/api/query_handlers_test.go index caf3c0a8..94f24331 100644 --- a/server/src/internal/api/query_handlers_test.go +++ b/server/src/internal/api/query_handlers_test.go @@ -29,7 +29,7 @@ import ( // RBAC checks pass without requiring a database. func newTestConnectionHandlerWithRBAC() *ConnectionHandler { rbac := auth.NewRBACChecker(nil) - return NewConnectionHandler(nil, nil, rbac) + return NewConnectionHandlerWithSecurity(nil, nil, rbac, false, nil, nil) } func TestExecuteQuery_MethodNotAllowed(t *testing.T) { diff --git a/server/src/internal/api/rbac_integration_test.go b/server/src/internal/api/rbac_integration_test.go index 540a0de9..932ece24 100644 --- a/server/src/internal/api/rbac_integration_test.go +++ b/server/src/internal/api/rbac_integration_test.go @@ -231,7 +231,7 @@ func TestRBACEnforcement_AdminPermissions(t *testing.T) { url: "/api/v1/notification-channels", permission: auth.PermManageNotificationChannels, handler: func(store *auth.AuthStore, checker *auth.RBACChecker) http.HandlerFunc { - h := NewNotificationChannelHandler(nil, store, checker) + h := NewNotificationChannelHandlerWithSecurity(nil, store, checker, false, nil, nil) return h.handleChannels }, }, @@ -241,7 +241,7 @@ func TestRBACEnforcement_AdminPermissions(t *testing.T) { url: "/api/v1/notification-channels/1", permission: auth.PermManageNotificationChannels, handler: func(store *auth.AuthStore, checker *auth.RBACChecker) http.HandlerFunc { - h := NewNotificationChannelHandler(nil, store, checker) + h := NewNotificationChannelHandlerWithSecurity(nil, store, checker, false, nil, nil) return h.handleChannelSubpath }, }, @@ -251,7 +251,7 @@ func TestRBACEnforcement_AdminPermissions(t *testing.T) { url: "/api/v1/notification-channels/1", permission: auth.PermManageNotificationChannels, handler: func(store *auth.AuthStore, checker *auth.RBACChecker) http.HandlerFunc { - h := NewNotificationChannelHandler(nil, store, checker) + h := NewNotificationChannelHandlerWithSecurity(nil, store, checker, false, nil, nil) return h.handleChannelSubpath }, }, @@ -1070,7 +1070,7 @@ func TestConnectionHandler_GetConnection_NonOwnerUnshared_403(t *testing.T) { bobID, _ := store.GetUserID("bob") checker := mockSharingChecker(t, store, 42, "alice", false) - handler := NewConnectionHandler(nil, store, checker) + handler := NewConnectionHandlerWithSecurity(nil, store, checker, false, nil, nil) req := httptest.NewRequest(http.MethodGet, "/api/v1/connections/42", nil) req = withUser(req, bobID) @@ -1087,7 +1087,7 @@ func TestConnectionHandler_GetConnection_Superuser_NotDenied(t *testing.T) { defer cleanup() checker := mockSharingChecker(t, store, 42, "alice", false) - handler := NewConnectionHandler(nil, store, checker) + handler := NewConnectionHandlerWithSecurity(nil, store, checker, false, nil, nil) req := httptest.NewRequest(http.MethodGet, "/api/v1/connections/42", nil) req = withSuperuser(req) @@ -1115,7 +1115,7 @@ func TestConnectionHandler_GetConnection_Owner_NotDenied(t *testing.T) { aliceID, _ := store.GetUserID("alice") checker := mockSharingChecker(t, store, 42, "alice", false) - handler := NewConnectionHandler(nil, store, checker) + handler := NewConnectionHandlerWithSecurity(nil, store, checker, false, nil, nil) req := httptest.NewRequest(http.MethodGet, "/api/v1/connections/42", nil) req = withUser(req, aliceID) @@ -1143,7 +1143,7 @@ func TestConnectionHandler_GetConnection_SharedNonOwner_NotDenied(t *testing.T) // Shared resource: non-owner should have access. checker := mockSharingChecker(t, store, 42, "alice", true) - handler := NewConnectionHandler(nil, store, checker) + handler := NewConnectionHandlerWithSecurity(nil, store, checker, false, nil, nil) req := httptest.NewRequest(http.MethodGet, "/api/v1/connections/42", nil) req = withUser(req, bobID) @@ -1182,7 +1182,7 @@ func TestConnectionHandler_GetConnection_GroupGrantedUser_NotDenied(t *testing.T } checker := mockSharingChecker(t, store, 42, "alice", false) - handler := NewConnectionHandler(nil, store, checker) + handler := NewConnectionHandlerWithSecurity(nil, store, checker, false, nil, nil) req := httptest.NewRequest(http.MethodGet, "/api/v1/connections/42", nil) req = withUser(req, bobID) diff --git a/server/src/internal/api/rbac_issue233_connections_test.go b/server/src/internal/api/rbac_issue233_connections_test.go index 38d591c1..ee5a8226 100644 --- a/server/src/internal/api/rbac_issue233_connections_test.go +++ b/server/src/internal/api/rbac_issue233_connections_test.go @@ -88,7 +88,7 @@ func setupIssue233CreateConnection( } checker := auth.NewRBACChecker(store) - handler := NewConnectionHandler(nil, store, checker) + handler := NewConnectionHandlerWithSecurity(nil, store, checker, false, nil, nil) return handler, userID, token, cleanup } @@ -231,7 +231,7 @@ func TestConnectionHandler_CreateConnection_Issue233_SuperuserAllowed(t *testing } checker := auth.NewRBACChecker(store) - handler := NewConnectionHandler(nil, store, checker) + handler := NewConnectionHandlerWithSecurity(nil, store, checker, false, nil, nil) body, _ := json.Marshal(ConnectionCreateRequest{ Name: "super-conn", @@ -315,7 +315,7 @@ func setupIssue233UpdateConnectionCluster( } checker := auth.NewRBACChecker(store) - handler := NewConnectionHandler(nil, store, checker) + handler := NewConnectionHandlerWithSecurity(nil, store, checker, false, nil, nil) return handler, userID, token, cleanup } @@ -412,7 +412,7 @@ func TestConnectionHandler_UpdateConnectionCluster_Issue233(t *testing.T) { } checker := auth.NewRBACChecker(store) - handler := NewConnectionHandler(nil, store, checker) + handler := NewConnectionHandlerWithSecurity(nil, store, checker, false, nil, nil) clusterID := 1 body, _ := json.Marshal(ConnectionClusterUpdateRequest{ diff --git a/server/src/internal/api/rbac_issue35_test.go b/server/src/internal/api/rbac_issue35_test.go index c7e13d7a..77eb3377 100644 --- a/server/src/internal/api/rbac_issue35_test.go +++ b/server/src/internal/api/rbac_issue35_test.go @@ -202,7 +202,7 @@ func TestConnectionHandler_PerConnection_NonOwnerUnshared_403(t *testing.T) { bobID := newTestUser(t, store, "bob") checker := mockSharingChecker(t, store, rbacUnsharedConnID, "alice", false) - handler := NewConnectionHandler(nil, store, checker) + handler := NewConnectionHandlerWithSecurity(nil, store, checker, false, nil, nil) url := fmtURL(tc.urlTemplate, rbacUnsharedConnID) req := httptest.NewRequest(tc.method, url, bytes.NewReader(tc.body)) @@ -232,7 +232,7 @@ func TestConnectionHandler_PerConnection_Owner_NotDenied(t *testing.T) { aliceID := newTestUser(t, store, "alice") checker := mockSharingChecker(t, store, rbacUnsharedConnID, "alice", false) - handler := NewConnectionHandler(nil, store, checker) + handler := NewConnectionHandlerWithSecurity(nil, store, checker, false, nil, nil) url := fmtURL(tc.urlTemplate, rbacUnsharedConnID) req := httptest.NewRequest(tc.method, url, bytes.NewReader(tc.body)) @@ -262,7 +262,7 @@ func TestConnectionHandler_PerConnection_SharedNonOwner_NotDenied(t *testing.T) bobID := newTestUser(t, store, "bob") checker := mockSharingChecker(t, store, rbacUnsharedConnID, "alice", true) - handler := NewConnectionHandler(nil, store, checker) + handler := NewConnectionHandlerWithSecurity(nil, store, checker, false, nil, nil) url := fmtURL(tc.urlTemplate, rbacUnsharedConnID) req := httptest.NewRequest(tc.method, url, bytes.NewReader(tc.body)) @@ -294,7 +294,7 @@ func TestConnectionHandler_PerConnection_GroupGranted_NotDenied(t *testing.T) { rbacUnsharedConnID, auth.AccessLevelReadWrite) checker := mockSharingChecker(t, store, rbacUnsharedConnID, "alice", false) - handler := NewConnectionHandler(nil, store, checker) + handler := NewConnectionHandlerWithSecurity(nil, store, checker, false, nil, nil) url := fmtURL(tc.urlTemplate, rbacUnsharedConnID) req := httptest.NewRequest(tc.method, url, bytes.NewReader(tc.body)) @@ -322,7 +322,7 @@ func TestConnectionHandler_PerConnection_Superuser_NotDenied(t *testing.T) { defer cleanup() checker := mockSharingChecker(t, store, rbacUnsharedConnID, "alice", false) - handler := NewConnectionHandler(nil, store, checker) + handler := NewConnectionHandlerWithSecurity(nil, store, checker, false, nil, nil) url := fmtURL(tc.urlTemplate, rbacUnsharedConnID) req := httptest.NewRequest(tc.method, url, bytes.NewReader(tc.body)) @@ -352,7 +352,7 @@ func TestConnectionHandler_UpdateConnectionCluster_NoDatastoreSideEffect(t *test bobID := newTestUser(t, store, "bob") checker := mockSharingChecker(t, store, rbacUnsharedConnID, "alice", false) - handler := NewConnectionHandler(nil, store, checker) + handler := NewConnectionHandlerWithSecurity(nil, store, checker, false, nil, nil) body := []byte(`{"cluster_id": 7, "membership_source": "manual"}`) req := httptest.NewRequest(http.MethodPut, @@ -405,7 +405,7 @@ func TestConnectionHandler_SetCurrentConnection_NonOwnerUnshared_403(t *testing. tokenHash := auth.GetTokenHashByRawToken(rawToken) checker := mockSharingChecker(t, store, rbacUnsharedConnID, "alice", false) - handler := NewConnectionHandler(nil, store, checker) + handler := NewConnectionHandlerWithSecurity(nil, store, checker, false, nil, nil) body, _ := json.Marshal(CurrentConnectionRequest{ConnectionID: rbacUnsharedConnID}) req := httptest.NewRequest(http.MethodPost, "/api/v1/connections/current", @@ -443,7 +443,7 @@ func TestConnectionHandler_SetCurrentConnection_Owner_NotDenied(t *testing.T) { tokenHash := auth.GetTokenHashByRawToken(rawToken) checker := mockSharingChecker(t, store, rbacUnsharedConnID, "alice", false) - handler := NewConnectionHandler(nil, store, checker) + handler := NewConnectionHandlerWithSecurity(nil, store, checker, false, nil, nil) body, _ := json.Marshal(CurrentConnectionRequest{ConnectionID: rbacUnsharedConnID}) req := httptest.NewRequest(http.MethodPost, "/api/v1/connections/current", @@ -473,7 +473,7 @@ func TestConnectionHandler_SetCurrentConnection_SharedNonOwner_NotDenied(t *test tokenHash := auth.GetTokenHashByRawToken(rawToken) checker := mockSharingChecker(t, store, rbacUnsharedConnID, "alice", true) - handler := NewConnectionHandler(nil, store, checker) + handler := NewConnectionHandlerWithSecurity(nil, store, checker, false, nil, nil) body, _ := json.Marshal(CurrentConnectionRequest{ConnectionID: rbacUnsharedConnID}) req := httptest.NewRequest(http.MethodPost, "/api/v1/connections/current", @@ -511,7 +511,7 @@ func TestConnectionHandler_GetCurrentConnection_NonOwnerUnshared_403(t *testing. } checker := mockSharingChecker(t, store, rbacUnsharedConnID, "alice", false) - handler := NewConnectionHandler(nil, store, checker) + handler := NewConnectionHandlerWithSecurity(nil, store, checker, false, nil, nil) req := httptest.NewRequest(http.MethodGet, "/api/v1/connections/current", nil) req = withUser(req, bobID) @@ -540,7 +540,7 @@ func TestConnectionHandler_GetCurrentConnection_Owner_NotDenied(t *testing.T) { } checker := mockSharingChecker(t, store, rbacUnsharedConnID, "alice", false) - handler := NewConnectionHandler(nil, store, checker) + handler := NewConnectionHandlerWithSecurity(nil, store, checker, false, nil, nil) req := httptest.NewRequest(http.MethodGet, "/api/v1/connections/current", nil) req = withUser(req, aliceID) @@ -571,7 +571,7 @@ func TestConnectionHandler_GetCurrentConnection_SharedNonOwner_NotDenied(t *test } checker := mockSharingChecker(t, store, rbacUnsharedConnID, "alice", true) - handler := NewConnectionHandler(nil, store, checker) + handler := NewConnectionHandlerWithSecurity(nil, store, checker, false, nil, nil) req := httptest.NewRequest(http.MethodGet, "/api/v1/connections/current", nil) req = withUser(req, bobID)