Skip to content

Commit f911cda

Browse files
sudhir-intcsudhir-intc
committed
fix(api): return bad request for malformed payloads
Classify malformed JSON, timestamp, and provisioning certificate input as client errors.
1 parent df8cdd6 commit f911cda

9 files changed

Lines changed: 286 additions & 8 deletions

File tree

‎integration-test/collections/console_mps_apis.postman_collection.json‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -508,6 +508,103 @@
508508
},
509509
"response": []
510510
},
511+
{
512+
"name": "Reject Power Action with Invalid JSON Type",
513+
"event": [
514+
{
515+
"listen": "test",
516+
"script": {
517+
"exec": [
518+
"pm.test(\"Status code is 400\", function () {\r",
519+
" pm.response.to.have.status(400);\r",
520+
"});\r",
521+
"\r",
522+
"pm.test(\"Response contains an error\", function () {\r",
523+
" pm.expect(pm.response.json().error).to.be.a(\"string\").and.not.empty;\r",
524+
"});"
525+
],
526+
"type": "text/javascript"
527+
}
528+
}
529+
],
530+
"request": {
531+
"method": "POST",
532+
"header": [],
533+
"body": {
534+
"mode": "raw",
535+
"raw": "{\r\n \"action\": false\r\n}",
536+
"options": {
537+
"raw": {
538+
"language": "json"
539+
}
540+
}
541+
},
542+
"url": {
543+
"raw": "{{protocol}}://{{host}}/api/v1/amt/power/action/{{deviceId}}",
544+
"protocol": "{{protocol}}",
545+
"host": [
546+
"{{host}}"
547+
],
548+
"path": [
549+
"api",
550+
"v1",
551+
"amt",
552+
"power",
553+
"action",
554+
"{{deviceId}}"
555+
]
556+
}
557+
},
558+
"response": []
559+
},
560+
{
561+
"name": "Reject Alarm with Invalid Timestamp",
562+
"event": [
563+
{
564+
"listen": "test",
565+
"script": {
566+
"exec": [
567+
"pm.test(\"Status code is 400\", function () {\r",
568+
" pm.response.to.have.status(400);\r",
569+
"});\r",
570+
"\r",
571+
"pm.test(\"Response contains an error\", function () {\r",
572+
" pm.expect(pm.response.json().error).to.be.a(\"string\").and.not.empty;\r",
573+
"});"
574+
],
575+
"type": "text/javascript"
576+
}
577+
}
578+
],
579+
"request": {
580+
"method": "POST",
581+
"header": [],
582+
"body": {
583+
"mode": "raw",
584+
"raw": "{\r\n \"StartTime\": \"fuzzstring\"\r\n}",
585+
"options": {
586+
"raw": {
587+
"language": "json"
588+
}
589+
}
590+
},
591+
"url": {
592+
"raw": "{{protocol}}://{{host}}/api/v1/amt/alarmOccurrences/{{deviceId}}",
593+
"protocol": "{{protocol}}",
594+
"host": [
595+
"{{host}}"
596+
],
597+
"path": [
598+
"api",
599+
"v1",
600+
"amt",
601+
"alarmOccurrences",
602+
"{{deviceId}}"
603+
]
604+
}
605+
},
606+
"response": []
607+
},
511608
{
512609
"name": "Send Advanced Power Action",
513610
"event": [

‎integration-test/collections/console_rps_apis.postman_collection.json‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,58 @@
174174
},
175175
"response": []
176176
},
177+
{
178+
"name": "Create Domain with Invalid Provisioning Certificate",
179+
"event": [
180+
{
181+
"listen": "test",
182+
"script": {
183+
"exec": [
184+
"pm.test(\"Status code is 400\", function () {\r",
185+
" pm.response.to.have.status(400);\r",
186+
"});\r",
187+
"\r",
188+
"pm.test(\"Response reports invalid certificate\", function () {\r",
189+
" pm.expect(pm.response.json().error).to.eql(\"invalid provisioning certificate\");\r",
190+
"});"
191+
],
192+
"type": "text/javascript"
193+
}
194+
}
195+
],
196+
"request": {
197+
"method": "POST",
198+
"header": [
199+
{
200+
"key": "Content-Type",
201+
"value": "application/json"
202+
}
203+
],
204+
"body": {
205+
"mode": "raw",
206+
"raw": "{\r\n \"profileName\": \"InvalidCertificateDomain\",\r\n \"domainSuffix\": \"example.com\",\r\n \"provisioningCert\": \"fuzzstring\",\r\n \"provisioningCertStorageFormat\": \"string\",\r\n \"provisioningCertPassword\": \"P@ssw0rd123\"\r\n}",
207+
"options": {
208+
"raw": {
209+
"language": "json"
210+
}
211+
}
212+
},
213+
"url": {
214+
"raw": "{{protocol}}://{{host}}/api/v1/admin/domains",
215+
"protocol": "{{protocol}}",
216+
"host": [
217+
"{{host}}"
218+
],
219+
"path": [
220+
"api",
221+
"v1",
222+
"admin",
223+
"domains"
224+
]
225+
}
226+
},
227+
"response": []
228+
},
177229
{
178230
"name": "Create Domain without suffix, password, cert, or format",
179231
"event": [

‎internal/controller/httpapi/v1/boot_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ func TestSetRemoteEraseOptions(t *testing.T) {
112112
requestBody: "invalid-json",
113113
mock: func(_ *mocks.MockDeviceManagementFeature) {
114114
},
115-
expectedCode: http.StatusInternalServerError,
115+
expectedCode: http.StatusBadRequest,
116116
},
117117
{
118118
name: "setRemoteEraseOptions - service failure",

‎internal/controller/httpapi/v1/devicemanagement_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ func TestDeviceManagement(t *testing.T) {
167167
requestBody: "invalid-json",
168168
mock: func(_ *mocks.MockDeviceManagementFeature) {
169169
},
170-
expectedCode: http.StatusInternalServerError,
170+
expectedCode: http.StatusBadRequest,
171171
response: nil,
172172
},
173173
{
@@ -364,7 +364,7 @@ func TestDeviceManagement(t *testing.T) {
364364
method: http.MethodPatch,
365365
requestBody: map[string]interface{}{"dhcpEnabled": "not-a-bool"},
366366
mock: func(_ *mocks.MockDeviceManagementFeature) {},
367-
expectedCode: http.StatusInternalServerError,
367+
expectedCode: http.StatusBadRequest,
368368
response: nil,
369369
},
370370
{

‎internal/controller/httpapi/v1/error.go‎

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
package v1
22

33
import (
4+
"encoding/json"
45
"errors"
6+
"io"
57
"net"
68
"net/http"
79
"strings"
10+
"time"
811

912
"github.com/gin-gonic/gin"
1013
"github.com/go-playground/validator/v10"
@@ -36,6 +39,9 @@ type response struct {
3639
func handleValidationErrors(c *gin.Context, err error) bool {
3740
var (
3841
odataValidationErr *ValidationError
42+
jsonSyntaxErr *json.SyntaxError
43+
jsonTypeErr *json.UnmarshalTypeError
44+
timeParseErr *time.ParseError
3945
validatorErr validator.ValidationErrors
4046
notValidErr dto.NotValidError
4147
validationErr devices.ValidationError
@@ -47,14 +53,29 @@ func handleValidationErrors(c *gin.Context, err error) bool {
4753
msg := err.Error()
4854
c.AbortWithStatusJSON(http.StatusBadRequest, response{Error: msg, Message: msg})
4955

50-
return true
51-
case errors.As(err, &validatorErr):
52-
validatorErrorHandle(c, validatorErr)
53-
5456
return true
5557
case errors.As(err, &notValidErr):
5658
notValidErrorHandle(c, notValidErr)
5759

60+
return true
61+
case errors.As(err, &jsonSyntaxErr):
62+
msg := err.Error()
63+
c.AbortWithStatusJSON(http.StatusBadRequest, response{Error: msg, Message: msg})
64+
65+
return true
66+
case errors.As(err, &jsonTypeErr):
67+
msg := err.Error()
68+
c.AbortWithStatusJSON(http.StatusBadRequest, response{Error: msg, Message: msg})
69+
70+
return true
71+
case errors.As(err, &timeParseErr) || errors.Is(err, io.EOF) || errors.Is(err, io.ErrUnexpectedEOF):
72+
msg := err.Error()
73+
c.AbortWithStatusJSON(http.StatusBadRequest, response{Error: msg, Message: msg})
74+
75+
return true
76+
case errors.As(err, &validatorErr):
77+
validatorErrorHandle(c, validatorErr)
78+
5879
return true
5980
case errors.As(err, &validationErr):
6081
msg := validationErr.Console.FriendlyMessage()
@@ -70,6 +91,7 @@ func handleValidationErrors(c *gin.Context, err error) bool {
7091
func handleDomainErrors(c *gin.Context, err error) bool {
7192
var (
7293
certExpErr domains.CertExpirationError
94+
certFormatErr domains.CertFormatError
7395
certPasswordErr domains.CertPasswordError
7496
notSupportedErr devices.NotSupportedError
7597
)
@@ -79,6 +101,11 @@ func handleDomainErrors(c *gin.Context, err error) bool {
79101
msg := certExpErr.Console.FriendlyMessage()
80102
c.AbortWithStatusJSON(http.StatusBadRequest, response{Error: msg, Message: msg})
81103

104+
return true
105+
case errors.As(err, &certFormatErr):
106+
msg := certFormatErr.Console.FriendlyMessage()
107+
c.AbortWithStatusJSON(http.StatusBadRequest, response{Error: msg, Message: msg})
108+
82109
return true
83110
case errors.As(err, &certPasswordErr):
84111
msg := certPasswordErr.Console.FriendlyMessage()

‎internal/controller/httpapi/v1/error_test.go‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,22 @@
11
package v1
22

33
import (
4+
"encoding/json"
45
"errors"
6+
"io"
57
"net/http"
68
"net/http/httptest"
79
"testing"
10+
"time"
811

912
"github.com/gin-gonic/gin"
1013
"github.com/stretchr/testify/assert"
1114

15+
"github.com/device-management-toolkit/console/internal/entity/dto/v1"
1216
wsmanAPI "github.com/device-management-toolkit/console/internal/usecase/devices/wsman"
17+
"github.com/device-management-toolkit/console/internal/usecase/domains"
1318
"github.com/device-management-toolkit/console/internal/usecase/profiles"
19+
"github.com/device-management-toolkit/console/pkg/consoleerrors"
1420
)
1521

1622
func TestMain(m *testing.M) {
@@ -44,6 +50,70 @@ func TestErrorResponse_CIRADeviceNotConnected(t *testing.T) {
4450
assert.Equal(t, http.StatusServiceUnavailable, w.Code)
4551
}
4652

53+
func TestErrorResponse_InvalidProvisioningCertificate(t *testing.T) {
54+
t.Parallel()
55+
56+
err := domains.ErrCertFormat.Wrap("test", "base64.StdEncoding.DecodeString", errors.New("illegal base64 data"))
57+
w := runErrorResponse(t, err)
58+
59+
assert.Equal(t, http.StatusBadRequest, w.Code)
60+
assert.JSONEq(t, `{"error":"invalid provisioning certificate","message":"invalid provisioning certificate"}`, w.Body.String())
61+
}
62+
63+
func TestErrorResponse_JSONBindingError(t *testing.T) {
64+
t.Parallel()
65+
66+
for _, requestBody := range []string{
67+
`"fuzzstring"`,
68+
`{"action":false}`,
69+
`{"action":"fuzzstring"}`,
70+
`{"bootPath":"\OemPba.efi"}`,
71+
} {
72+
var powerAction dto.PowerAction
73+
74+
err := json.Unmarshal([]byte(requestBody), &powerAction)
75+
w := runErrorResponse(t, err)
76+
77+
assert.Equal(t, http.StatusBadRequest, w.Code)
78+
}
79+
}
80+
81+
func TestErrorResponse_WrappedJSONBindingError(t *testing.T) {
82+
t.Parallel()
83+
84+
var device dto.Device
85+
86+
err := json.Unmarshal([]byte(`{"tags":"test"}`), &device)
87+
err = dto.NotValidError{Console: consoleerrors.CreateConsoleError("ProfileAPI")}.Wrap("insert", "json.Unmarshal", err)
88+
w := runErrorResponse(t, err)
89+
90+
assert.Equal(t, http.StatusBadRequest, w.Code)
91+
assert.JSONEq(t, `{"error":"Invalid input: json: cannot unmarshal string into Go struct field Device.tags of type []string","message":"Invalid input: json: cannot unmarshal string into Go struct field Device.tags of type []string"}`, w.Body.String())
92+
}
93+
94+
func TestErrorResponse_InvalidTimestamp(t *testing.T) {
95+
t.Parallel()
96+
97+
var alarm dto.AlarmClockOccurrenceInput
98+
99+
err := json.Unmarshal([]byte(`{"StartTime":"fuzzstring"}`), &alarm)
100+
w := runErrorResponse(t, err)
101+
102+
var parseErr *time.ParseError
103+
assert.ErrorAs(t, err, &parseErr)
104+
assert.Equal(t, http.StatusBadRequest, w.Code)
105+
}
106+
107+
func TestErrorResponse_EmptyRequestBody(t *testing.T) {
108+
t.Parallel()
109+
110+
for _, err := range []error{io.EOF, io.ErrUnexpectedEOF} {
111+
w := runErrorResponse(t, err)
112+
113+
assert.Equal(t, http.StatusBadRequest, w.Code)
114+
}
115+
}
116+
47117
func TestHandleSentinelErrors_CIRADeviceNotConnected(t *testing.T) {
48118
t.Parallel()
49119

‎internal/usecase/domains/certpassword.go‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,23 @@ package domains
22

33
import "github.com/device-management-toolkit/console/pkg/consoleerrors"
44

5+
const invalidCertificate = "invalid provisioning certificate"
6+
7+
type CertFormatError struct {
8+
Console consoleerrors.InternalError
9+
}
10+
11+
func (e CertFormatError) Error() string {
12+
return invalidCertificate
13+
}
14+
15+
func (e CertFormatError) Wrap(call, function string, err error) error {
16+
_ = e.Console.Wrap(call, function, err)
17+
e.Console.Message = invalidCertificate
18+
19+
return e
20+
}
21+
522
type CertPasswordError struct {
623
Console consoleerrors.InternalError
724
}

0 commit comments

Comments
 (0)