Swagger-core: SecurityRequirements: Mismatch between model & spec

[FossiFoo]

The spec defines "security" as [SecurityRequirement] and those as a Map of names and scopes, so effectively a List>>.

Swagger.java however contains a protected List securityRequirements; with those being just a name and list of scopes, missing one level of the structure.

I would propose to add a "Requirements" object which contains a map of the existing SecurityRequirements.

While this is touched anyway, i also noticed, that the Method to add these is called "addSecurityDefinitions(SecurityRequirements)" instead of "addSecurityRequirements" and a plain "securityRequirements" Helper is missing.

[hgwood]

It is mentioned in #1271 that the fix for this requires a breaking change that will only be considered for the next major version. Is there any idea of the when of said major version?

[webron]

@hgwood - not yet.

[fehguy]

The helper security was added (not securityRequirement per your request) as it follows the JSON representation of attribute names. There is als a securityDefinition helper for building a swagger with the SecurityDefinition object. I'm not quite sure what the need is for a Requirement object though.

[cbornet]

The field is still serialized as "securityRequirement". Do you know if it works with swagger-codegen and swagger-ui ?

[cbornet]

Also I think the @JsonIgnore that was set on setSecurity() should be removed so that swagger-core can deserialize both security and securityRequirement but only serializes as securityRequirement. That would help for transitionning.

[boillodmanuel]

Any news on this topic? The generated swagger is not valid due to 'security' attribute that's named 'securityRequirement'. Do you have an ETA for this issue?

I noticed that there were breaking changes in 1.5.8. Could the fix of this issue be included in 1.5.9?

Best regards

[fehguy]

@boillodmanuel How are you getting securityRequirement? The SwaggerSerializers for jackson should address this. Perhaps you're not using them? They are required.

[boillodmanuel]

I'm not. I' e looked at the unit test and they are using ObjectMapper.
Thanks for pointing this out.

Le ven. 15 avr. 2016 20:02, Tony Tam [email protected] a écrit :

@boillodmanuel https://github.com/boillodmanuel How are you getting
securityRequirement? The SwaggerSerializers for jackson should address
this. Perhaps you're not using them? They are required.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/swagger-api/swagger-core/issues/1258#issuecomment-210566656

[fehguy]

Ok, please report back if you're having troubles with this. You can use the Json.mapper() for convenience.

[boillodmanuel]

I wrote a simple test:

    @Test
    public void testSwaggerExport() throws Exception {
        Swagger swagger = new Swagger();
        swagger.setSecurity(Arrays.asList(new SecurityRequirement().scope("user").scope("movie")));
        System.out.println(Json.mapper().writeValueAsString(swagger));
    }

which produces the following json

{"swagger":"2.0","securityRequirement":[{"scopes":["user","movie"]}]}

which is invalid because the attribute securityRequirement does not exist is spec. It should be security.

I'm doing something wrong?

[cbornet]

To me, this is still not fixed : https://github.com/swagger-api/swagger-core/blob/master/modules/swagger-models/src/main/java/io/swagger/models/Swagger.java#L316

[boillodmanuel]

Is it planned to be fixed soon ? We have to live with a fork only because of this issue which is not ideal.

Thank you

[boillodmanuel]

Hello there,
I've open a pull-request to fix this issue: https://github.com/swagger-api/swagger-core/pull/1783
Hope this helps.

[naab]

I want to bump up this issue, because the latest swagger-ui can't handle "securityRequirement" field generated by this for the authentication feature. (it expects a "security" field, like it should be !)

as per swagger-codegen, its security definitions only work for the "securityRequirement" field, not "security" as it is based on this code (removing the @JsonIgnore on the setters could fix this tho...)

This is a major issue that should be integrated before everyone forks it to fix it... It shouldn't be discussed about for 1 year...
And yes, all swaggers are broken.

The previous PR was closed.
Maybe will you aggree if I do another PR compatible with the existing version but add a security field too ?
(one of them will be ignored depending on the reader)

or maybe at least remove the @JsonIgnore on setSecurity ?
(to be able to use swagger-codegen with Spec compliant json)

[fehguy]

This was fixed by https://github.com/swagger-api/swagger-core/pull/1783

[boillodmanuel]

This fix is not yet in a release. Is it possible to release a new version?

[fehguy]

It should be in 1.5.9. Can you please check and report back?

https://github.com/swagger-api/swagger-core/commit/d24904f9bf36f00a6fad0f66f09eb0472c4bd551

[boillodmanuel]

I confirm that it was not in 1.5.9.
I did a manual test and check the sources too.
Moreover Github commit d24904f says it was only on master, but rebase could create a new sha1.

regards

[webron]

There are a few more PRs I'd like to merge in before pushing 1.5.10 out - hoping to get it all done next week. In the meantime, you can check out 1.5.10-SNAPSHOT.

[boillodmanuel]

I can wait. Thanks for the update.

Le jeu. 11 août 2016 17:57, Ron [email protected] a écrit :

There are a few more PRs I'd like to merge in before pushing 1.5.10 out -
hoping to get it all done next week. In the meantime, you can check out
1.5.10-SNAPSHOT.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/swagger-api/swagger-core/issues/1258#issuecomment-239205606,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAl7j73hp5j-6m_6KD22hlxUoBYhnw7Fks5qe0ZygaJpZM4FXk5b
.

[webron]

@boillodmanuel - it's pushed.

[boillodmanuel]

Thanks a lot.