fix(api): 契约检查对请求体按字段严格比对且不看 Content-Type,补上 approve 的 expected_digest 请求体
This commit is contained in:
4 files changed
+48
-14
No files matched your search
+17
-1
@@ -6571,6 +6571,20 @@ paths:
|
|||||||
security: [{ sessionCookie: [] }]
|
security: [{ sessionCookie: [] }]
|
||||||
parameters:
|
parameters:
|
||||||
- { name: id, in: path, required: true, schema: { type: string } }
|
- { name: id, in: path, required: true, schema: { type: string } }
|
||||||
|
requestBody:
|
||||||
|
required: true
|
||||||
|
content:
|
||||||
|
application/json:
|
||||||
|
schema:
|
||||||
|
type: object
|
||||||
|
required: [expected_digest]
|
||||||
|
properties:
|
||||||
|
expected_digest:
|
||||||
|
type: string
|
||||||
|
description: >-
|
||||||
|
The context_sha256 the admin reviewed: the X-Felis-Context-Sha256 header of
|
||||||
|
the context they downloaded, or the listed one. A context uploaded again since
|
||||||
|
answers 409 context_changed.
|
||||||
responses:
|
responses:
|
||||||
'200':
|
'200':
|
||||||
description: The approved submission, with the linked build id.
|
description: The approved submission, with the linked build id.
|
||||||
@@ -6586,7 +6600,9 @@ paths:
|
|||||||
'404':
|
'404':
|
||||||
$ref: '#/components/responses/NotFound'
|
$ref: '#/components/responses/NotFound'
|
||||||
'409':
|
'409':
|
||||||
description: Submission has already been reviewed.
|
description: >-
|
||||||
|
The submission has already been reviewed (already_reviewed), or its context was
|
||||||
|
uploaded again after the reviewed digest (context_changed).
|
||||||
content:
|
content:
|
||||||
application/json:
|
application/json:
|
||||||
schema: { $ref: '#/components/schemas/Error' }
|
schema: { $ref: '#/components/schemas/Error' }
|
||||||
|
|||||||
@@ -58,7 +58,7 @@ func TestPasskeyRegisterVertical(t *testing.T) {
|
|||||||
|
|
||||||
// 1) begin returns the creation options FLAT (envelope stripped for the panel) and
|
// 1) begin returns the creation options FLAT (envelope stripped for the panel) and
|
||||||
// stashes exactly one challenge bound to the caller.
|
// stashes exactly one challenge bound to the caller.
|
||||||
w := do(eh, "POST", "/api/v1/account/passkey/register/begin", `{}`, nil)
|
w := do(eh, "POST", "/api/v1/account/passkey/register/begin", "", nil)
|
||||||
if w.Code != http.StatusOK {
|
if w.Code != http.StatusOK {
|
||||||
t.Fatalf("begin: code = %d, want 200 (%s)", w.Code, w.Body.String())
|
t.Fatalf("begin: code = %d, want 200 (%s)", w.Code, w.Body.String())
|
||||||
}
|
}
|
||||||
@@ -157,7 +157,7 @@ func TestPasskeyBeginPassesExistingCredentials(t *testing.T) {
|
|||||||
v := &fakePasskeyVerifier{}
|
v := &fakePasskeyVerifier{}
|
||||||
eh := newPasskeyAPI(repo, v, user)
|
eh := newPasskeyAPI(repo, v, user)
|
||||||
|
|
||||||
if w := do(eh, "POST", "/api/v1/account/passkey/register/begin", `{}`, nil); w.Code != http.StatusOK {
|
if w := do(eh, "POST", "/api/v1/account/passkey/register/begin", "", nil); w.Code != http.StatusOK {
|
||||||
t.Fatalf("begin: code = %d, want 200 (%s)", w.Code, w.Body.String())
|
t.Fatalf("begin: code = %d, want 200 (%s)", w.Code, w.Body.String())
|
||||||
}
|
}
|
||||||
if len(v.lastUser.Credentials) != 1 || v.lastUser.Credentials[0].CredentialID != "existing-cred" {
|
if len(v.lastUser.Credentials) != 1 || v.lastUser.Credentials[0].CredentialID != "existing-cred" {
|
||||||
@@ -173,8 +173,8 @@ func TestPasskeyBeginSupersedes(t *testing.T) {
|
|||||||
repo := newFakeRepo()
|
repo := newFakeRepo()
|
||||||
eh := newPasskeyAPI(repo, &fakePasskeyVerifier{}, user)
|
eh := newPasskeyAPI(repo, &fakePasskeyVerifier{}, user)
|
||||||
|
|
||||||
do(eh, "POST", "/api/v1/account/passkey/register/begin", `{}`, nil)
|
do(eh, "POST", "/api/v1/account/passkey/register/begin", "", nil)
|
||||||
do(eh, "POST", "/api/v1/account/passkey/register/begin", `{}`, nil)
|
do(eh, "POST", "/api/v1/account/passkey/register/begin", "", nil)
|
||||||
if len(repo.passkeyChallenges) != 1 {
|
if len(repo.passkeyChallenges) != 1 {
|
||||||
t.Fatalf("a second begin must supersede the first; stashed challenges = %d, want 1", len(repo.passkeyChallenges))
|
t.Fatalf("a second begin must supersede the first; stashed challenges = %d, want 1", len(repo.passkeyChallenges))
|
||||||
}
|
}
|
||||||
@@ -189,7 +189,7 @@ func TestPasskeyUnavailable(t *testing.T) {
|
|||||||
repo := newFakeRepo()
|
repo := newFakeRepo()
|
||||||
eh := newPasskeyAPI(repo, nil, user) // nil verifier
|
eh := newPasskeyAPI(repo, nil, user) // nil verifier
|
||||||
|
|
||||||
if w := do(eh, "POST", "/api/v1/account/passkey/register/begin", `{}`, nil); w.Code != http.StatusServiceUnavailable || decodeErr(t, w) != "passkey_unavailable" {
|
if w := do(eh, "POST", "/api/v1/account/passkey/register/begin", "", nil); w.Code != http.StatusServiceUnavailable || decodeErr(t, w) != "passkey_unavailable" {
|
||||||
t.Fatalf("begin with no verifier: code = %d body %s, want 503 passkey_unavailable", w.Code, w.Body.String())
|
t.Fatalf("begin with no verifier: code = %d body %s, want 503 passkey_unavailable", w.Code, w.Body.String())
|
||||||
}
|
}
|
||||||
if w := do(eh, "POST", "/api/v1/account/passkey/register/finish", `{"name":"x","attestation":{"a":1}}`, nil); w.Code != http.StatusServiceUnavailable || decodeErr(t, w) != "passkey_unavailable" {
|
if w := do(eh, "POST", "/api/v1/account/passkey/register/finish", `{"name":"x","attestation":{"a":1}}`, nil); w.Code != http.StatusServiceUnavailable || decodeErr(t, w) != "passkey_unavailable" {
|
||||||
@@ -395,7 +395,7 @@ func TestPasskeyFaceSeparation(t *testing.T) {
|
|||||||
ih := api.InternalHandler()
|
ih := api.InternalHandler()
|
||||||
|
|
||||||
for _, tc := range []struct{ method, path, body string }{
|
for _, tc := range []struct{ method, path, body string }{
|
||||||
{"POST", "/api/v1/account/passkey/register/begin", `{}`},
|
{"POST", "/api/v1/account/passkey/register/begin", ""},
|
||||||
{"POST", "/api/v1/account/passkey/register/finish", `{"name":"k","attestation":{"a":1}}`},
|
{"POST", "/api/v1/account/passkey/register/finish", `{"name":"k","attestation":{"a":1}}`},
|
||||||
{"GET", "/api/v1/account/passkey/credentials", ""},
|
{"GET", "/api/v1/account/passkey/credentials", ""},
|
||||||
{"DELETE", "/api/v1/account/passkey/credentials/x", ""},
|
{"DELETE", "/api/v1/account/passkey/credentials/x", ""},
|
||||||
|
|||||||
@@ -29,7 +29,8 @@ import (
|
|||||||
// - the operation must list the status that came back (or a default / 4XX / 5XX);
|
// - the operation must list the status that came back (or a default / 4XX / 5XX);
|
||||||
// - a JSON body must fit the schema documented for that status, and a response
|
// - a JSON body must fit the schema documented for that status, and a response
|
||||||
// object may carry only the properties its schema names;
|
// object may carry only the properties its schema names;
|
||||||
// - the JSON request behind a 2xx must fit the documented requestBody.
|
// - the JSON request behind a 2xx must fit the documented requestBody, naming
|
||||||
|
// only the properties it names.
|
||||||
//
|
//
|
||||||
// Requests to a path the document does not have are left to the route parity test.
|
// Requests to a path the document does not have are left to the route parity test.
|
||||||
|
|
||||||
@@ -163,7 +164,10 @@ func checkContract(path string, calls []contractCall) ([]string, error) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if c.status/100 == 2 && isJSON(c.reqCT) && len(c.reqBody) > 0 {
|
// decodeJSON reads the body whatever Content-Type says, so a JSON body sent
|
||||||
|
// without one (as most handler tests send it) is held to the document too.
|
||||||
|
reqJSON := isJSON(c.reqCT) || (c.reqCT == "" && json.Valid(c.reqBody))
|
||||||
|
if c.status/100 == 2 && reqJSON && len(c.reqBody) > 0 {
|
||||||
rb, ok := op["requestBody"].(map[string]any)
|
rb, ok := op["requestBody"].(map[string]any)
|
||||||
if !ok && c.method == http.MethodGet {
|
if !ok && c.method == http.MethodGet {
|
||||||
continue // a body on a GET is ignored, whatever it holds
|
continue // a body on a GET is ignored, whatever it holds
|
||||||
@@ -175,7 +179,7 @@ func checkContract(path string, calls []contractCall) ([]string, error) {
|
|||||||
if schema, ok := jsonSchemaOf(v.deref(rb)); ok {
|
if schema, ok := jsonSchemaOf(v.deref(rb)); ok {
|
||||||
var body any
|
var body any
|
||||||
if err := json.Unmarshal(c.reqBody, &body); err == nil {
|
if err := json.Unmarshal(c.reqBody, &body); err == nil {
|
||||||
for _, e := range v.check(schema, body, "request", false) {
|
for _, e := range v.check(schema, body, "request", true) {
|
||||||
report(c, "%s", e)
|
report(c, "%s", e)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -318,8 +322,9 @@ func (v *contractValidator) deref(n map[string]any) map[string]any {
|
|||||||
return n
|
return n
|
||||||
}
|
}
|
||||||
|
|
||||||
// check returns where value leaves schema. strict (responses) also refuses object
|
// check returns where value leaves schema. strict also refuses object properties
|
||||||
// properties the schema does not name, unless it allows additional ones.
|
// the schema does not name, unless it allows additional ones: a response field and
|
||||||
|
// a request field the document misnames (display_name for displayName) both show.
|
||||||
func (v *contractValidator) check(schema map[string]any, value any, at string, strict bool) []string {
|
func (v *contractValidator) check(schema map[string]any, value any, at string, strict bool) []string {
|
||||||
schema = v.deref(schema)
|
schema = v.deref(schema)
|
||||||
if ref, ok := schema["x-unresolved"]; ok {
|
if ref, ok := schema["x-unresolved"]; ok {
|
||||||
@@ -491,6 +496,11 @@ func TestContractCheckerCatchesDrift(t *testing.T) {
|
|||||||
// A response field outside its enum.
|
// A response field outside its enum.
|
||||||
{method: "POST", path: "/api/v1/account/link/verify", reqCT: jsonCT, reqBody: []byte(`{"code":"ABC"}`),
|
{method: "POST", path: "/api/v1/account/link/verify", reqCT: jsonCT, reqBody: []byte(`{"code":"ABC"}`),
|
||||||
status: 200, respCT: jsonCT, respBody: []byte(`{"linked":true,"mc_uuid":"u","auth_source":""}`), at: "j"},
|
status: 200, respCT: jsonCT, respBody: []byte(`{"linked":true,"mc_uuid":"u","auth_source":""}`), at: "j"},
|
||||||
|
// A request property the requestBody does not name: the document once had
|
||||||
|
// display_name where the handler reads displayName. Sent with no Content-Type,
|
||||||
|
// which the handler decodes all the same.
|
||||||
|
{method: "POST", path: "/api/v1/servers", reqBody: []byte(`{"name":"x1","subdomain":"x1","display_name":"X"}`),
|
||||||
|
status: 201, respCT: jsonCT, respBody: []byte(`{"name":"x1","subdomain":"x1","desiredState":"Stopped"}`), at: "k"},
|
||||||
}
|
}
|
||||||
got, err := checkContract("../../docs/openapi.yaml", calls)
|
got, err := checkContract("../../docs/openapi.yaml", calls)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@@ -504,6 +514,7 @@ func TestContractCheckerCatchesDrift(t *testing.T) {
|
|||||||
`GET /api/v1/servers → 200 (b): response: property "extra" is not documented`,
|
`GET /api/v1/servers → 200 (b): response: property "extra" is not documented`,
|
||||||
`POST /api/v1/account/link/verify → 200 (i): request.code: is integer, documented as string`,
|
`POST /api/v1/account/link/verify → 200 (i): request.code: is integer, documented as string`,
|
||||||
`POST /api/v1/account/link/verify → 200 (j): response.auth_source: is not one of [mojang thirdparty]`,
|
`POST /api/v1/account/link/verify → 200 (j): response.auth_source: is not one of [mojang thirdparty]`,
|
||||||
|
`POST /api/v1/servers → 201 (k): request: property "display_name" is not documented`,
|
||||||
}
|
}
|
||||||
if strings.Join(got, "\n") != strings.Join(want, "\n") {
|
if strings.Join(got, "\n") != strings.Join(want, "\n") {
|
||||||
t.Fatalf("violations:\n%s\nwant:\n%s", strings.Join(got, "\n"), strings.Join(want, "\n"))
|
t.Fatalf("violations:\n%s\nwant:\n%s", strings.Join(got, "\n"), strings.Join(want, "\n"))
|
||||||
|
|||||||
@@ -8463,7 +8463,14 @@ export interface operations {
|
|||||||
};
|
};
|
||||||
cookie?: never;
|
cookie?: never;
|
||||||
};
|
};
|
||||||
requestBody?: never;
|
requestBody: {
|
||||||
|
content: {
|
||||||
|
"application/json": {
|
||||||
|
/** @description The context_sha256 the admin reviewed: the X-Felis-Context-Sha256 header of the context they downloaded, or the listed one. A context uploaded again since answers 409 context_changed. */
|
||||||
|
expected_digest: string;
|
||||||
|
};
|
||||||
|
};
|
||||||
|
};
|
||||||
responses: {
|
responses: {
|
||||||
/** @description The approved submission, with the linked build id. */
|
/** @description The approved submission, with the linked build id. */
|
||||||
200: {
|
200: {
|
||||||
@@ -8478,7 +8485,7 @@ export interface operations {
|
|||||||
401: components["responses"]["Unauthorized"];
|
401: components["responses"]["Unauthorized"];
|
||||||
403: components["responses"]["Forbidden"];
|
403: components["responses"]["Forbidden"];
|
||||||
404: components["responses"]["NotFound"];
|
404: components["responses"]["NotFound"];
|
||||||
/** @description Submission has already been reviewed. */
|
/** @description The submission has already been reviewed (already_reviewed), or its context was uploaded again after the reviewed digest (context_changed). */
|
||||||
409: {
|
409: {
|
||||||
headers: {
|
headers: {
|
||||||
[name: string]: unknown;
|
[name: string]: unknown;
|
||||||
|
|||||||
Reference in new issue
Block a user