Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JsonOverlay should not remove security: [] #235

Open
chris-brace opened this issue Apr 12, 2019 · 6 comments
Open

JsonOverlay should not remove security: [] #235

chris-brace opened this issue Apr 12, 2019 · 6 comments

Comments

@chris-brace
Copy link

chris-brace commented Apr 12, 2019

I am serializing a OAS3 model to YAML using the following boilerplate described in the docs

final JsonNode serial = Overlay.of(model).toJson(SerializationOptions.Option.FOLLOW_REFS);
final String raw = new YAMLMapper().writerWithDefaultPrettyPrinter().writeValueAsString(serial);

My API has a default security scheme applied to all routes except one, which defines security: [] to override the root security object as is (extremely poorly) described in "step two" of this document https://swagger.io/docs/specification/authentication/ . The serializers remove this, so right now i am doing some gross string substitution to fix it.

The point is, that empty list has significance.

Also, I am aware that I can disable removal of all empty lists and objects using serialization options, but this is not what i want to do as the output generates tons of errors and warnings from parsers and editors.


In case anyone else ends up here my gross solution is, for the following spec:

security:
  - JWTAuth: []
paths:
  /:
    get:
      security:
        - JWTAuth: ["DISABLE"]

I do the following to the final, serialized, prettyprinted YAML file:

private static String fixupYAML(final String in)
{
    // This is what happens when you do - Foo: []
    return in.replaceAll("JWTAuth: null", "JWTAuth: []")
             // Because the serializer removes all empty objects and arrays
             .replaceAll("security:\n {6}- JWTAuth:\n {8}- \"DISABLE\"", "security: []");
}
@tedepstein
Copy link
Contributor

@chris-brace , not sure whether it's helpful to point this out, but it seems what you want is an empty array, not an empty object. Is there a serialization option to remove empty objects, but preserve empty arrays? If so, would that still give you too much noise in the serialized OpenAPI doc, and ensuing errors, warnings, etc.?

Is there a way to explicitly set the value of the security property to empty array, and would that change how it serializes?

Sorry that I don't know the answers to these questions, and I'm not set up to look at it right now. But if my musings here don't lead to a solution, I will take a closer look, hopefully next week.

@tedepstein
Copy link
Contributor

tedepstein commented Apr 12, 2019

A hunch: SecurityRequirementImpl seems to need a fix similar to the one we see in SecurityParameterImpl.

Security Requirement Object is a map of Security Scheme names to an array of scope requirements for that security scheme. In KZOP, SecurityParameter represents the string scopes array, i.e. the map value in the Security Requirement Object. It's defined here in types3.yaml, the config file for the JSON Overlay generator.

- name: SecurityParameter
  imports:
    impl:
    - JsonNodeFactory
  fields:
    parameter:
      type: String
      parentPath: ''
      structure: collection

I think the JsonNodeFactory import is needed for this code, which I think is what provides the empty array if the property value is missing.

	@Override
	protected JsonNode _fixJson(JsonNode json) {
		return json.isMissingNode() ? _jsonArray() : json;
	}

At first glance, it seems like we need a similar solution for SecurityRequirements.

We know this works for SecurityParameter, because OpenAPI requires empty scope arrays on Security Requirement Objects for schemes of any type other than oauth2 or openIdConnect. The requirement for empty scope array is explicit in the spec, and is very commonly used in lots of API documents.

Empty security requirements, allowing anonymous access, are not as common. It's not even clear (enough) in the OpenAPI spec that it's permitted. See the open PR #1886 here and the referenced issues.

It seems that the proposed syntax for this is security: [ {} ], not security: [].

Empty security requirements array as a security value is less common. But the spec explicitly allows this as an operation-level override of a top-level security requirement:

To remove a top-level security declaration, an empty array can be used.

The open pull request #1886 adds explicit support for an empty Security Requirement Object in the array. It's intended for cases where security is optional, as in this example, showing the empty object in an array with other, non-empty Security Requirement Objects.

I'm guessing you'll hit the same problem, trying to create an array with a single, empty Security Requirement object.

This needs a little more thought...

@chris-brace
Copy link
Author

Sorry, I edited my issue to be specific that what i want is, in the case that it is present, an empty list for security to persist through serialization because it is meaningful.

@tedepstein
Copy link
Contributor

tedepstein commented Apr 12, 2019

@chris-brace , just to cover the bases, it would help to know:

  • Is it possible to create an empty SecurityRequirement object, and add this to the security array value?
  • If so, does it serialize as "security" : [ {} ] or is the security property just omitted?

@tedepstein
Copy link
Contributor

@chris-brace , I took some time this weekend to experiment with serialization, focusing on several cases of empty objects and empty arrays that are meaningful in Security Scheme and Security Requirement objects. You can see an OpenAPI YAML test input and failing JUnit test in the task/#235 branch.

It seems that the problems with serialization are pretty extensive. There is the big problem of empty objects and empty arrays described in the docs:

N.B. Neither treatment distinguishes between empty structures that were created automatically during the elaboration process, and empty structures that were present in the source model (or added via the mutation API). This is especially problematic in certain cases where empty structures actually mean something different from missing structures in the OpenAPI specification. For example, an empty servers list in an Operation Object overrides servers objects at the path or root level in the model, but a missing servers object does not. Until this issue is addressed properly, toJson() can yield models that differ semantically from their intended meaning. Round-tripping through the parser and the serializer can change the meaning of a model.

Also described in issue #114.

I am also seeing that flows are completely missing from the serialized JSON, which may or may not be explained by the known issue with empty objects. It looks like it might be a separate bug, but I haven't really tried to analyze it yet.

So it seems that serialization is incomplete, not ready for real use at this point. You can use the parser to read, traverse, and inspect an OpenAPI model (which is what our application code does), even modify the OpenAPI model in-memory, but not to write changes back to a file.

The fix for these problems is going to require some design changes that are beyond what we can do right now. I'd like to keep this open as a back-burner issue, and try to rework the design to handle serialization properly. But I can't offer a time frame for this.

@chris-brace
Copy link
Author

chris-brace commented Apr 14, 2019 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants