Schema requires authTemplates to have `name` field, but none of the given examples have it

[grebnetiew]

In the schema, omitting lines not relevant to the issue,

properties:
  authTemplate:
    description: "Optional authentication information template. This is used to avoid duplication in the auth list. \
                 Entries defined in the template will be applied to all elements in the auth list that do not specify them."
    allOf:
    - $ref: "#/$defs/AuthenticationInfo"
    - type: object

# ...
  AuthenticationInfo:
    type: object
    properties:
      name:
        type: string
    required: ["name"]

To me, this suggests the authTemplate must be an AuthenticationInfo, which requires a name field.

However, in the Dataset repo, we find the blogapi example:

auth:
  - name: admin
    loginEndpointAuth:
      payloadRaw: "{\"usernameOrEmail\": \"admin\", \"password\": \"bar123\"}"
  - name: user
    loginEndpointAuth:
      payloadRaw: "{\"usernameOrEmail\": \"user\", \"password\": \"bar123\"}"

authTemplate:
    loginEndpointAuth:
        endpoint: /api/auth/signin
        verb: POST
        contentType: application/json
        token:
            extractFrom: body
            extractSelector: /accessToken
            sendName: Authorization
            sendIn: header
            sendTemplate: "Bearer {token}"

Incidentally, the same holds for LoginEndpointAuth, which requires the verb, but no verbs are specified in the auth array in the example.

The comments suggest that each AuthenticationInfo and its sub-objects may omit required fields at will, as long as the merged variant satisfies the required constraints. However, I don't think that's what the specification actually specifies. Or am I misunderstanding how this kind of specification works?

[arcuri82]

hi @grebnetiew ,

thanks for pointing this out... you are right ;)

The idea behind authTemplate was to avoid repetitions, by just specifying a subset of fields that are common to all AuthenticationInfo entries under auth. but, once we have - $ref: "#/$defs/AuthenticationInfo", indeed we are importing all its constraints, including name being required. This is not what we want.

Indeed, this can be considered as a "bug" in the schema...

We need to think what would be the easiest/most-elegant approach in JSON Schema to specify this. Worst case, we could just have a new AuthenticationInfoNoConstraint object for the authTemplate.
Suggestions are welcome ;)

[arcuri82]

and indeed it is an issue for the entries in auth, as they might miss "required" entries which are specified in the authTemplate.

ideally, the validation should be done after the "merge", but not sure if there is any way to express something like this with a JSON Schema.
Regardless, current version of schema needs to be fixed.
An option could be to remove all required constraints, but better if we can find some other alternatives.

@Stranger6667 @en-milie do you have any opinion/suggestion on this?

[arcuri82]

after digging a bit more into it, doesn't seem feasible to support it natively with JSON Schema :(
I see 2 options:

1. we remove the required constraints, and replace them with x-required. As anyway the merge of data (ie, from authTemplate fields into auth objects) would be done internally in the fuzzers, validation of x-required could be done programmatically there as well

2. we remove authTemplate from the schema. We create a second schema/template-document with authTemplate that enables applying transformations (eg something like Jsonnet or CUE) to resolve the merge, creating a valid JSON/YAML output that would satisfy the new schema without authTemplate.

Between the 2 options, I think (1) might be the easiest. Option (2) is more precise, but I am bit worried about tool support for template engines in different programming languages (should work at least for Java, Python and Rust).

@grebnetiew What do you think?

[grebnetiew]

It seems YAML has anchors and aliases, which natively support the structure merging you now leave for the fuzzers to perform.

Before:

auth:
  - name: ROLE_ADMIN
    loginEndpointAuth:
      payloadRaw: "username=admin&password=test"
  - name: ROLE_EMP
    loginEndpointAuth:
      payloadRaw: "username=selimhorri&password=test"
  - name: ROLE_MGR
    loginEndpointAuth:
      payloadRaw: "username=soumayahajjem&password=test"

authTemplate:
  loginEndpointAuth:
    endpoint: /app/login
    verb: POST
    contentType: application/x-www-form-urlencoded
    expectCookies: true

After:

endpointTemplate: &template
  endpoint: /app/login
  verb: POST
  contentType: application/x-www-form-urlencoded
  expectCookies: true

auth:
  - name: ROLE_ADMIN
    loginEndpointAuth:
      <<: *template
      payloadRaw: "username=admin&password=test"
  - name: ROLE_EMP
    loginEndpointAuth:
      <<: *template
      payloadRaw: "username=selimhorri&password=test"
  - name: ROLE_MGR
    loginEndpointAuth:
      <<: *template
      payloadRaw: "username=soumayahajjem&password=test"

You can then specify required for the auth structure and children (and omit specification of the authTemplate altogether).

(I am not sure if our YAML parsing library supports this, so I might be taking aim at my own foot by suggesting this ;) )

[arcuri82]

hi @grebnetiew ,

thanks for the pointer! I didn't know about templates in YAML...

It would had been perfect, but, digging a bit more into it, it seems like they were an optional feature in YAML 1.1, removed in YAML 1.2, and no specific plan to introduce templating mechanism in the coming YAML 1.3 specs :(

Currently Jackson (which I use) does not support them (but I could change to another library if needed).

Another potential issue is that there is no templating mechanism is TOML (which would be another possible way to define auth configuration files)

[grebnetiew]

In that case I think your option (1) is best. Having two separate schemas/documents kind of reduces usability for the end user, and if that is acceptable, you might as well just remove the "template" idea and require people to repeat values in all auth elements to begin with :)

[arcuri82]

@grebnetiew good ;)
unless there is any objection (or other suggestions) from @Stranger6667 @en-milie , i will replace the required with x-required (in a new PR) in the next coming days