From 397d93b64e28a0bcebcc36996a39eca6a14c0958 Mon Sep 17 00:00:00 2001 From: Otavio Salvador Date: Fri, 11 Sep 2026 15:48:22 -0300 Subject: [PATCH 1/2] fix(server): stop documenting device status updates the API rejects The `unused` path value mapped to a status the request validator refuses, so it always answered 400. Nothing lets a client move a device to `unused`, so drop it from the documented enum of both the device and the container route (the latter is rewritten onto the former), and from the handler's map. A request for `unused` still gets 400, now from the `required` rule instead of `oneof`. PATCH /admin/api/devices/{uid}/{status} was documented in the combined, cloud and enterprise specs, but the admin router registers no such route. Remove it along with deviceStatusPath, its only consumer. The deviceStatus schema stays, as the device and container list filters use it. Fixes: shellhub-io/team#242 --- openapi/spec/cloud-openapi.yaml | 2 -- .../parameters/path/deviceStatusPath.yaml | 6 ----- openapi/spec/enterprise-openapi.yaml | 2 -- openapi/spec/openapi.yaml | 2 -- .../admin@api@devices@{uid}@{status}.yaml | 27 ------------------- .../paths/api@containers@{uid}@{status}.yaml | 1 - .../paths/api@devices@{uid}@{status}.yaml | 1 - server/api/routes/device.go | 1 - 8 files changed, 42 deletions(-) delete mode 100644 openapi/spec/components/parameters/path/deviceStatusPath.yaml delete mode 100644 openapi/spec/paths/admin@api@devices@{uid}@{status}.yaml diff --git a/openapi/spec/cloud-openapi.yaml b/openapi/spec/cloud-openapi.yaml index 8018326e4fc..7145c5d440f 100644 --- a/openapi/spec/cloud-openapi.yaml +++ b/openapi/spec/cloud-openapi.yaml @@ -162,8 +162,6 @@ paths: $ref: paths/admin@api@devices.yaml /admin/api/devices/{uid}: $ref: paths/admin@api@devices@{uid}.yaml - /admin/api/devices/{uid}/{status}: - $ref: paths/admin@api@devices@{uid}@{status}.yaml /admin/api/firewall/rules: $ref: paths/admin@api@firewall@rules.yaml /admin/api/firewall/rules/{id}: diff --git a/openapi/spec/components/parameters/path/deviceStatusPath.yaml b/openapi/spec/components/parameters/path/deviceStatusPath.yaml deleted file mode 100644 index a15f51f21b8..00000000000 --- a/openapi/spec/components/parameters/path/deviceStatusPath.yaml +++ /dev/null @@ -1,6 +0,0 @@ -name: status -description: Device's status -schema: - $ref: ../../schemas/deviceStatus.yaml -required: true -in: path diff --git a/openapi/spec/enterprise-openapi.yaml b/openapi/spec/enterprise-openapi.yaml index 553f69ba6e1..e5644305192 100644 --- a/openapi/spec/enterprise-openapi.yaml +++ b/openapi/spec/enterprise-openapi.yaml @@ -135,8 +135,6 @@ paths: $ref: paths/admin@api@devices.yaml /admin/api/devices/{uid}: $ref: paths/admin@api@devices@{uid}.yaml - /admin/api/devices/{uid}/{status}: - $ref: paths/admin@api@devices@{uid}@{status}.yaml /admin/api/firewall/rules: $ref: paths/admin@api@firewall@rules.yaml /admin/api/firewall/rules/{id}: diff --git a/openapi/spec/openapi.yaml b/openapi/spec/openapi.yaml index f59af58d6c8..14ddaab0097 100644 --- a/openapi/spec/openapi.yaml +++ b/openapi/spec/openapi.yaml @@ -351,8 +351,6 @@ paths: $ref: paths/admin@api@devices.yaml /admin/api/devices/{uid}: $ref: paths/admin@api@devices@{uid}.yaml - /admin/api/devices/{uid}/{status}: - $ref: paths/admin@api@devices@{uid}@{status}.yaml /admin/api/firewall/rules: $ref: paths/admin@api@firewall@rules.yaml /admin/api/firewall/rules/{id}: diff --git a/openapi/spec/paths/admin@api@devices@{uid}@{status}.yaml b/openapi/spec/paths/admin@api@devices@{uid}@{status}.yaml deleted file mode 100644 index 437971f834e..00000000000 --- a/openapi/spec/paths/admin@api@devices@{uid}@{status}.yaml +++ /dev/null @@ -1,27 +0,0 @@ -patch: - operationId: updateDeviceStatusAdmin - summary: Update status Admin - description: Update device's status. - tags: - - admin - - devices - - enterprise - security: - - jwt: [] - - instance-api-key: [] - parameters: - - $ref: ../components/parameters/path/deviceUIDPath.yaml - - $ref: ../components/parameters/path/deviceStatusPath.yaml - responses: - '200': - description: Success to update device status. - '400': - $ref: ../components/responses/400.yaml - '401': - $ref: ../components/responses/401.yaml - '402': - description: Payment required. - '403': - $ref: ../components/responses/403.yaml - '500': - $ref: ../components/responses/500.yaml diff --git a/openapi/spec/paths/api@containers@{uid}@{status}.yaml b/openapi/spec/paths/api@containers@{uid}@{status}.yaml index eba4b4cb496..86d46875e71 100644 --- a/openapi/spec/paths/api@containers@{uid}@{status}.yaml +++ b/openapi/spec/paths/api@containers@{uid}@{status}.yaml @@ -18,7 +18,6 @@ patch: - accept - reject - pending - - unused example: accept required: true in: path diff --git a/openapi/spec/paths/api@devices@{uid}@{status}.yaml b/openapi/spec/paths/api@devices@{uid}@{status}.yaml index efbeb431627..6bb889b2aee 100644 --- a/openapi/spec/paths/api@devices@{uid}@{status}.yaml +++ b/openapi/spec/paths/api@devices@{uid}@{status}.yaml @@ -18,7 +18,6 @@ patch: - accept - reject - pending - - unused example: accept required: true in: path diff --git a/server/api/routes/device.go b/server/api/routes/device.go index 684c4f56057..0b71b6da14f 100644 --- a/server/api/routes/device.go +++ b/server/api/routes/device.go @@ -219,7 +219,6 @@ func (h *Handler) UpdateDeviceStatus(c *gateway.Context) error { "accept": string(models.DeviceStatusAccepted), "reject": string(models.DeviceStatusRejected), "pending": string(models.DeviceStatusPending), - "unused": string(models.DeviceStatusUnused), } req.Status = status[req.Status] From 96464325f5f00bbb7b4c8735d8cf66a3f009493c Mon Sep 17 00:00:00 2001 From: Otavio Salvador Date: Fri, 11 Sep 2026 16:00:07 -0300 Subject: [PATCH 2/2] refactor(server): keep the settable device statuses in one list The statuses a client can set through PATCH /api/devices/{uid}/{status} were listed twice in Go, as the handler's path-value map and as the request's `oneof` rule, and twice in the spec, as inline enums on the device and container routes. shellhub-io/team#242 was those copies drifting apart: `unused` sat in the map and both enums but not in `oneof`. The handler's map is the only producer of the request's status, and it yields "" for any path value it does not know, so `required` alone rejects everything `oneof` did. Drop `oneof` and let the map be the list. Both spec routes now share the deviceStatusAction schema. With the rule no longer naming the values, a route test pins which path values are accepted and what each maps to, including the container route that is rewritten onto the device one. --- .../schemas/deviceStatusAction.yaml | 7 ++ .../paths/api@containers@{uid}@{status}.yaml | 7 +- .../paths/api@devices@{uid}@{status}.yaml | 7 +- pkg/api/requests/device.go | 2 +- server/api/routes/device_test.go | 66 +++++++++++++++++++ 5 files changed, 76 insertions(+), 13 deletions(-) create mode 100644 openapi/spec/components/schemas/deviceStatusAction.yaml diff --git a/openapi/spec/components/schemas/deviceStatusAction.yaml b/openapi/spec/components/schemas/deviceStatusAction.yaml new file mode 100644 index 00000000000..97b323f0a07 --- /dev/null +++ b/openapi/spec/components/schemas/deviceStatusAction.yaml @@ -0,0 +1,7 @@ +description: Status change to apply to a device +type: string +enum: + - accept + - reject + - pending +example: accept diff --git a/openapi/spec/paths/api@containers@{uid}@{status}.yaml b/openapi/spec/paths/api@containers@{uid}@{status}.yaml index 86d46875e71..464dabaad99 100644 --- a/openapi/spec/paths/api@containers@{uid}@{status}.yaml +++ b/openapi/spec/paths/api@containers@{uid}@{status}.yaml @@ -13,12 +13,7 @@ patch: - name: status description: Container's status schema: - type: string - enum: - - accept - - reject - - pending - example: accept + $ref: ../components/schemas/deviceStatusAction.yaml required: true in: path responses: diff --git a/openapi/spec/paths/api@devices@{uid}@{status}.yaml b/openapi/spec/paths/api@devices@{uid}@{status}.yaml index 6bb889b2aee..f9ec964e2d8 100644 --- a/openapi/spec/paths/api@devices@{uid}@{status}.yaml +++ b/openapi/spec/paths/api@devices@{uid}@{status}.yaml @@ -13,12 +13,7 @@ patch: - name: status description: Device's status schema: - type: string - enum: - - accept - - reject - - pending - example: accept + $ref: ../components/schemas/deviceStatusAction.yaml required: true in: path responses: diff --git a/pkg/api/requests/device.go b/pkg/api/requests/device.go index 382859c559d..d118b6f90e7 100644 --- a/pkg/api/requests/device.go +++ b/pkg/api/requests/device.go @@ -84,7 +84,7 @@ type DeviceLookup struct { type DeviceUpdateStatus struct { TenantID string `header:"X-Tenant-ID"` UID string `param:"uid" validate:"required"` - Status string `param:"status" validate:"required,oneof=accepted pending rejected"` + Status string `param:"status" validate:"required"` } // DeviceCreateTag is the structure to represent the request data for device create tag endpoint. diff --git a/server/api/routes/device_test.go b/server/api/routes/device_test.go index b0c6da0a2d1..4f27cfe4329 100644 --- a/server/api/routes/device_test.go +++ b/server/api/routes/device_test.go @@ -331,6 +331,72 @@ func TestRenameDevice(t *testing.T) { } } +func TestUpdateDeviceStatus(t *testing.T) { + cases := []struct { + title string + path string + expectedStatus models.DeviceStatus + expectedCode int + }{ + { + title: "accept sets the device accepted", + path: "/api/devices/uid/accept", + expectedStatus: models.DeviceStatusAccepted, + expectedCode: http.StatusOK, + }, + { + title: "reject sets the device rejected", + path: "/api/devices/uid/reject", + expectedStatus: models.DeviceStatusRejected, + expectedCode: http.StatusOK, + }, + { + title: "pending sets the device pending", + path: "/api/devices/uid/pending", + expectedStatus: models.DeviceStatusPending, + expectedCode: http.StatusOK, + }, + { + title: "the container route sets the device accepted", + path: "/api/containers/uid/accept", + expectedStatus: models.DeviceStatusAccepted, + expectedCode: http.StatusOK, + }, + { + title: "unused is rejected", + path: "/api/devices/uid/unused", + expectedCode: http.StatusBadRequest, + }, + { + title: "a model status is rejected", + path: "/api/devices/uid/accepted", + expectedCode: http.StatusBadRequest, + }, + } + + for _, tc := range cases { + t.Run(tc.title, func(t *testing.T) { + mock := mocks.NewMockService(t) + if tc.expectedStatus != "" { + mock.On("UpdateDeviceStatus", gomock.Anything, &requests.DeviceUpdateStatus{ + TenantID: "tenant-id", + UID: "uid", + Status: string(tc.expectedStatus), + }).Return(nil).Once() + } + + req := httptest.NewRequestWithContext(t.Context(), http.MethodPatch, tc.path, nil) + req.Header.Set("X-Role", authorizer.RoleOwner.String()) + req.Header.Set("X-Tenant-ID", "tenant-id") + rec := httptest.NewRecorder() + + NewRouter(mock).ServeHTTP(rec, req) + + assert.Equal(t, tc.expectedCode, rec.Result().StatusCode) + }) + } +} + func TestGetDeviceList(t *testing.T) { mock := mocks.NewMockService(t)