diff --git a/api/routes/api-key_test.go b/api/routes/api-key_test.go index 435d718edce..752e2e74aac 100644 --- a/api/routes/api-key_test.go +++ b/api/routes/api-key_test.go @@ -288,6 +288,21 @@ func TestListAPIKey(t *testing.T) { requiredMocks func() expected Expected }{ + { + description: "fails with api key", + headers: map[string]string{ + "Content-Type": "application/json", + "X-API-KEY": "b2f7cc0e-d933-4aad-9ab2-b557f2f2554f", + "X-Tenant-ID": "00000000-0000-4000-0000-000000000000", + "X-Role": "owner", + }, + query: func() string { + return "" + }, + requiredMocks: func() { + }, + expected: Expected{body: nil, status: http.StatusForbidden}, + }, { description: "success", headers: map[string]string{ diff --git a/api/routes/nsadm.go b/api/routes/nsadm.go index 4b0c0945bd9..1c302177bc2 100644 --- a/api/routes/nsadm.go +++ b/api/routes/nsadm.go @@ -18,7 +18,6 @@ const ( AddNamespaceMemberURL = "/namespaces/:tenant/members" RemoveNamespaceMemberURL = "/namespaces/:tenant/members/:uid" EditNamespaceMemberURL = "/namespaces/:tenant/members/:uid" - GetSessionRecordURL = "/users/security" EditSessionRecordStatusURL = "/users/security/:tenant" EditDeviceAutoAcceptURL = "/namespaces/device-auto-accept/:tenant" ) @@ -249,17 +248,3 @@ func (h *Handler) EditSessionRecordStatus(c gateway.Context) error { return c.NoContent(http.StatusOK) } - -func (h *Handler) GetSessionRecord(c gateway.Context) error { - var tenant string - if v := c.Tenant(); v != nil { - tenant = v.ID - } - - status, err := h.service.GetSessionRecord(c.Ctx(), tenant) - if err != nil { - return err - } - - return c.JSON(http.StatusOK, status) -} diff --git a/api/routes/nsadm_test.go b/api/routes/nsadm_test.go index 962ad361270..1736bb6e0d0 100644 --- a/api/routes/nsadm_test.go +++ b/api/routes/nsadm_test.go @@ -244,53 +244,6 @@ func TestDeleteNamespace(t *testing.T) { mock.AssertExpectations(t) } -func TestGetSessionRecord(t *testing.T) { - mock := new(mocks.Service) - - cases := []struct { - name string - tenant string - requiredMocks func() - expectedStatus int - }{ - { - name: "fails when try to get a session record of a non-existing session", - tenant: "tenant", - requiredMocks: func() { - mock.On("GetSessionRecord", gomock.Anything, "tenant").Return(false, svc.ErrNotFound).Once() - }, - expectedStatus: http.StatusNotFound, - }, - { - name: "success when try to get a session record of a existing session", - tenant: "tenant", - requiredMocks: func() { - mock.On("GetSessionRecord", gomock.Anything, "tenant").Return(true, nil).Once() - }, - expectedStatus: http.StatusOK, - }, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - tc.requiredMocks() - - req := httptest.NewRequest(http.MethodGet, "/api/users/security", nil) - req.Header.Set("Content-Type", "application/json") - req.Header.Set("X-Role", authorizer.RoleOwner.String()) - req.Header.Set("X-Tenant-ID", tc.tenant) - rec := httptest.NewRecorder() - - e := NewRouter(mock) - e.ServeHTTP(rec, req) - - assert.Equal(t, tc.expectedStatus, rec.Result().StatusCode) - }) - } - - mock.AssertExpectations(t) -} - func TestEditNamespace(t *testing.T) { svcMock := new(mocks.Service) @@ -513,6 +466,16 @@ func TestHandler_LeaveNamespace(t *testing.T) { requiredMocks func() expected int }{ + { + description: "fails with api key", + tenantID: "00000000-0000-4000-0000-000000000000", + headers: map[string]string{ + "X-API-KEY": "b2f7cc0e-d933-4aad-9ab2-b557f2f2554f", + "X-Tenant-ID": "00000000-0000-4000-0000-000000000000", + }, + requiredMocks: func() {}, + expected: http.StatusForbidden, + }, { description: "fails to leave the namespace", tenantID: "00000000-0000-4000-0000-000000000000", @@ -673,29 +636,31 @@ func TestNamespaceCrossTenantAccess(t *testing.T) { } } -// TestListNamespacesAPIKeyScope ensures a caller without a user ID (API key -// path) cannot enumerate namespaces across tenants: the list is scoped to the -// caller's own tenant. Covers the regression described in GHSA-vwx9-7qcf-gg7f. -func TestListNamespacesAPIKeyScope(t *testing.T) { - const callerTenant = "00000000-0000-4000-0000-000000000000" - +func TestGetNamespaceListBlocksAPIKey(t *testing.T) { mock := new(mocks.Service) - scopedNS := &models.Namespace{TenantID: callerTenant, Name: "dev"} - mock. - On("ListNamespaces", gomock.Anything, gomock.MatchedBy(func(req *requests.NamespaceList) bool { - return req.UserID == "" && req.TenantID == callerTenant && !req.IsAdmin - })). - Return([]models.Namespace{*scopedNS}, 1, nil). - Once() req := httptest.NewRequest(http.MethodGet, "/api/namespaces", nil) - req.Header.Set("X-Role", authorizer.RoleOwner.String()) - req.Header.Set("X-Tenant-ID", callerTenant) - // No X-ID set: this simulates an API key caller. + req.Header.Set("X-API-KEY", "b2f7cc0e-d933-4aad-9ab2-b557f2f2554f") + req.Header.Set("X-Tenant-ID", "00000000-0000-4000-0000-000000000000") + + rec := httptest.NewRecorder() + NewRouter(mock).ServeHTTP(rec, req) + + assert.Equal(t, http.StatusForbidden, rec.Result().StatusCode) + mock.AssertExpectations(t) +} + +func TestCreateNamespaceBlocksAPIKey(t *testing.T) { + mock := new(mocks.Service) + + req := httptest.NewRequest(http.MethodPost, "/api/namespaces", strings.NewReader(`{}`)) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("X-API-KEY", "b2f7cc0e-d933-4aad-9ab2-b557f2f2554f") + req.Header.Set("X-Tenant-ID", "00000000-0000-4000-0000-000000000000") rec := httptest.NewRecorder() NewRouter(mock).ServeHTTP(rec, req) - assert.Equal(t, http.StatusOK, rec.Result().StatusCode) + assert.Equal(t, http.StatusForbidden, rec.Result().StatusCode) mock.AssertExpectations(t) } diff --git a/api/routes/routes.go b/api/routes/routes.go index c32fc734599..85155d6b279 100644 --- a/api/routes/routes.go +++ b/api/routes/routes.go @@ -133,7 +133,7 @@ func NewRouter(service services.Service, opts ...Option) *echo.Echo { publicAPI.POST(AuthPublicKeyURL, gateway.Handler(handler.AuthPublicKey)) publicAPI.POST(CreateAPIKeyURL, gateway.Handler(handler.CreateAPIKey), routesmiddleware.BlockAPIKey, routesmiddleware.RequiresPermission(authorizer.APIKeyCreate)) - publicAPI.GET(ListAPIKeysURL, gateway.Handler(handler.ListAPIKeys)) + publicAPI.GET(ListAPIKeysURL, gateway.Handler(handler.ListAPIKeys), routesmiddleware.BlockAPIKey) publicAPI.PATCH(UpdateAPIKeyURL, gateway.Handler(handler.UpdateAPIKey), routesmiddleware.BlockAPIKey, routesmiddleware.RequiresPermission(authorizer.APIKeyUpdate)) publicAPI.DELETE(DeleteAPIKeyURL, gateway.Handler(handler.DeleteAPIKey), routesmiddleware.BlockAPIKey, routesmiddleware.RequiresPermission(authorizer.APIKeyDelete)) @@ -178,18 +178,17 @@ func NewRouter(service services.Service, opts ...Option) *echo.Echo { publicAPI.PUT(UpdatePublicKeyURL, gateway.Handler(handler.UpdatePublicKey), routesmiddleware.DecodeParam(ParamPublicKeyFingerprint), routesmiddleware.RequiresPermission(authorizer.PublicKeyEdit)) publicAPI.DELETE(DeletePublicKeyURL, gateway.Handler(handler.DeletePublicKey), routesmiddleware.DecodeParam(ParamPublicKeyFingerprint), routesmiddleware.RequiresPermission(authorizer.PublicKeyRemove)) - publicAPI.POST(CreateNamespaceURL, gateway.Handler(handler.CreateNamespace)) + publicAPI.POST(CreateNamespaceURL, gateway.Handler(handler.CreateNamespace), routesmiddleware.BlockAPIKey) publicAPI.GET(GetNamespaceURL, gateway.Handler(handler.GetNamespace), routesmiddleware.RequiresTenant(ParamNamespaceTenant)) - publicAPI.GET(ListNamespaceURL, gateway.Handler(handler.GetNamespaceList)) + publicAPI.GET(ListNamespaceURL, gateway.Handler(handler.GetNamespaceList), routesmiddleware.BlockAPIKey) publicAPI.PUT(EditNamespaceURL, gateway.Handler(handler.EditNamespace), routesmiddleware.RequiresTenant(ParamNamespaceTenant), routesmiddleware.RequiresPermission(authorizer.NamespaceUpdate)) publicAPI.DELETE(DeleteNamespaceURL, gateway.Handler(handler.DeleteNamespace), routesmiddleware.RequiresTenant(ParamNamespaceTenant), routesmiddleware.RequiresPermission(authorizer.NamespaceDelete)) publicAPI.POST(AddNamespaceMemberURL, gateway.Handler(handler.AddNamespaceMember), routesmiddleware.RequiresPermission(authorizer.NamespaceAddMember)) publicAPI.PATCH(EditNamespaceMemberURL, gateway.Handler(handler.EditNamespaceMember), routesmiddleware.RequiresPermission(authorizer.NamespaceEditMember)) publicAPI.DELETE(RemoveNamespaceMemberURL, gateway.Handler(handler.RemoveNamespaceMember), routesmiddleware.RequiresPermission(authorizer.NamespaceRemoveMember)) - publicAPI.DELETE(LeaveNamespaceURL, gateway.Handler(handler.LeaveNamespace)) + publicAPI.DELETE(LeaveNamespaceURL, gateway.Handler(handler.LeaveNamespace), routesmiddleware.BlockAPIKey) - publicAPI.GET(GetSessionRecordURL, gateway.Handler(handler.GetSessionRecord)) publicAPI.PUT(EditSessionRecordStatusURL, gateway.Handler(handler.EditSessionRecordStatus), routesmiddleware.RequiresTenant(ParamNamespaceTenant), routesmiddleware.RequiresPermission(authorizer.NamespaceEnableSessionRecord)) publicAPI.PUT(EditDeviceAutoAcceptURL, gateway.Handler(handler.EditDeviceAutoAccept), routesmiddleware.RequiresTenant(ParamNamespaceTenant), routesmiddleware.RequiresPermission(authorizer.NamespaceDeviceAutoAccept)) diff --git a/api/services/mocks/services.go b/api/services/mocks/services.go index 33d33847011..0aa288167f9 100644 --- a/api/services/mocks/services.go +++ b/api/services/mocks/services.go @@ -855,34 +855,6 @@ func (_m *Service) GetSession(ctx context.Context, uid models.UID) (*models.Sess return r0, r1 } -// GetSessionRecord provides a mock function with given fields: ctx, tenantID -func (_m *Service) GetSessionRecord(ctx context.Context, tenantID string) (bool, error) { - ret := _m.Called(ctx, tenantID) - - if len(ret) == 0 { - panic("no return value specified for GetSessionRecord") - } - - var r0 bool - var r1 error - if rf, ok := ret.Get(0).(func(context.Context, string) (bool, error)); ok { - return rf(ctx, tenantID) - } - if rf, ok := ret.Get(0).(func(context.Context, string) bool); ok { - r0 = rf(ctx, tenantID) - } else { - r0 = ret.Get(0).(bool) - } - - if rf, ok := ret.Get(1).(func(context.Context, string) error); ok { - r1 = rf(ctx, tenantID) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - // GetStats provides a mock function with given fields: ctx, req func (_m *Service) GetStats(ctx context.Context, req *requests.GetStats) (*models.Stats, error) { ret := _m.Called(ctx, req) diff --git a/api/services/namespace.go b/api/services/namespace.go index ba52b9508fc..77ce8dfbdbb 100644 --- a/api/services/namespace.go +++ b/api/services/namespace.go @@ -20,7 +20,6 @@ type NamespaceService interface { GetNamespace(ctx context.Context, tenantID string) (*models.Namespace, error) DeleteNamespace(ctx context.Context, tenantID string) error EditSessionRecordStatus(ctx context.Context, sessionRecord bool, tenantID string) error - GetSessionRecord(ctx context.Context, tenantID string) (bool, error) EditDeviceAutoAccept(ctx context.Context, deviceAutoAccept bool, tenantID string) error } @@ -248,18 +247,3 @@ func (s *service) EditDeviceAutoAccept(ctx context.Context, deviceAutoAccept boo return nil } - -// GetSessionRecord gets the session record data. -// -// It receives a context, used to "control" the request flow, the tenant ID from models.Namespace. -// -// GetSessionRecord returns a boolean indicating the session record status and an error. When error is not nil, -// the boolean is false. -func (s *service) GetSessionRecord(ctx context.Context, tenantID string) (bool, error) { - n, err := s.store.NamespaceResolve(ctx, store.NamespaceTenantIDResolver, tenantID) - if err != nil { - return false, NewErrNamespaceNotFound(tenantID, err) - } - - return n.Settings.SessionRecord, nil -} diff --git a/api/services/namespace_test.go b/api/services/namespace_test.go index 12173bba867..6a13361ee02 100644 --- a/api/services/namespace_test.go +++ b/api/services/namespace_test.go @@ -1499,71 +1499,3 @@ func TestDeleteNamespace(t *testing.T) { storeMock.AssertExpectations(t) } - -func TestGetSessionRecord(t *testing.T) { - storeMock := new(storemock.Store) - - type Expected struct { - status bool - err error - } - - cases := []struct { - name string - tenantID string - mocks func(context.Context) - expected Expected - }{ - { - name: "fails when namespace not found", - tenantID: "a736a52b-5777-4f92-b0b8-e359bf484713", - mocks: func(ctx context.Context) { - storeMock.On("NamespaceResolve", ctx, store.NamespaceTenantIDResolver, "a736a52b-5777-4f92-b0b8-e359bf484713").Return(nil, store.ErrNoDocuments).Once() - }, - expected: Expected{false, NewErrNamespaceNotFound("a736a52b-5777-4f92-b0b8-e359bf484713", store.ErrNoDocuments)}, - }, - { - name: "fails when store namespace resolve fails", - tenantID: "a736a52b-5777-4f92-b0b8-e359bf484713", - mocks: func(ctx context.Context) { - storeMock. - On("NamespaceResolve", ctx, store.NamespaceTenantIDResolver, "a736a52b-5777-4f92-b0b8-e359bf484713"). - Return(nil, errors.New("error")). - Once() - }, - expected: Expected{false, NewErrNamespaceNotFound("a736a52b-5777-4f92-b0b8-e359bf484713", errors.New("error"))}, - }, - { - name: "succeeds", - tenantID: "a736a52b-5777-4f92-b0b8-e359bf484713", - mocks: func(ctx context.Context) { - storeMock. - On("NamespaceResolve", ctx, store.NamespaceTenantIDResolver, "a736a52b-5777-4f92-b0b8-e359bf484713"). - Return( - &models.Namespace{ - Name: "group1", - Owner: "hash1", - TenantID: "a736a52b-5777-4f92-b0b8-e359bf484713", - Settings: &models.NamespaceSettings{SessionRecord: false}, - }, - nil, - ). - Once() - }, - expected: Expected{false, nil}, - }, - } - - s := NewService(store.Store(storeMock), privateKey, publicKey, storecache.NewNullCache(), clientMock) - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - ctx := context.Background() - tc.mocks(ctx) - status, err := s.GetSessionRecord(ctx, tc.tenantID) - assert.Equal(t, tc.expected, Expected{status, err}) - }) - } - - storeMock.AssertExpectations(t) -} diff --git a/openapi/spec/paths/api@namespaces.yaml b/openapi/spec/paths/api@namespaces.yaml index 42718698fec..4a9e4bd09a6 100644 --- a/openapi/spec/paths/api@namespaces.yaml +++ b/openapi/spec/paths/api@namespaces.yaml @@ -1,6 +1,5 @@ get: operationId: getNamespaces - x-internal: true summary: Get namespaces list description: Returns a list of namespaces. tags: @@ -8,7 +7,6 @@ get: - namespaces security: - jwt: [] - - api-key: [] parameters: - name: filter description: | @@ -62,7 +60,6 @@ get: $ref: ../components/responses/500.yaml post: operationId: createNamespace - x-internal: true summary: Create namespace description: Create a namespace. tags: @@ -70,7 +67,6 @@ post: - namespaces security: - jwt: [] - - api-key: [] requestBody: content: application/json: diff --git a/openapi/spec/paths/api@namespaces@api-key.yaml b/openapi/spec/paths/api@namespaces@api-key.yaml index bb4ed5520ec..2a59ce1be26 100644 --- a/openapi/spec/paths/api@namespaces@api-key.yaml +++ b/openapi/spec/paths/api@namespaces@api-key.yaml @@ -34,7 +34,6 @@ post: $ref: ../components/responses/500.yaml get: operationId: apiKeyList - x-internal: true summary: List API Keys tags: - api-keys @@ -42,7 +41,6 @@ get: - namespaces security: - jwt: [] - - api-key: [] parameters: - $ref: ../components/parameters/query/pageQuery.yaml - $ref: ../components/parameters/query/perPageQuery.yaml diff --git a/openapi/spec/paths/api@namespaces@{tenant}@members.yaml b/openapi/spec/paths/api@namespaces@{tenant}@members.yaml index 2b4e8e88cce..845d2a62c24 100644 --- a/openapi/spec/paths/api@namespaces@{tenant}@members.yaml +++ b/openapi/spec/paths/api@namespaces@{tenant}@members.yaml @@ -1,6 +1,5 @@ delete: operationId: leaveNamespace - x-internal: true summary: Leave Namespace description: | Allows the authenticated user to leave the specified namespace. Owners @@ -13,7 +12,6 @@ delete: - namespaces security: - jwt: [] - - api-key: [] parameters: - $ref: ../components/parameters/path/namespaceTenantIDPath.yaml responses: