What's the point of SecurityDefinitions?

[gromakovsky]

This type is defined in Data.OpenApi.Internal and is not part of any other types. There are some instance defined for it and it's re-exported from Data.OpenApi. Also it's used in tests.

Is this type useful on its own (since it's not part of any other type)? How is it supposed to be used?

I see that in swagger2 there is Swagger data type with _swaggerSecurityDefinitions :: SecurityDefinitions, but apparently it was thrown away in openapi3. Is it intentional (maybe there actually was a change in the specification, i. e. maybe that's the difference between version 2 and version 3)?

[gromakovsky]

So I tried putting something into _openApiSecurity and got the following error in Swagger Editor:

Semantic error at security.0
Security requirements must match a security definition

I didn't read OpenAPI specification carefully, but it looks like I must provide security definitions in order to specify security requirements, but apparently I can't do that in openapi3. Am I missing something?

[gromakovsky]

Update after reading OAS: apparently I should use _componentsSecuritySchemes :: Definitions SecurityScheme field (in the Components type) to provide all security definitions. SecurityDefinitions is just a newtype for Definitions SecurityScheme. So apparently _swaggerSecurityDefinitions :: SecurityDefinitions was changed to _componentsSecuritySchemes :: Definitions SecurityScheme in 821654c which made SecurityDefinitions type useless (? still not sure), but it was forgotten to remove this type.

[maksbotan]

Hi!

apparently I should use _componentsSecuritySchemes :: Definitions SecurityScheme field (in the Components type) to provide all security definitions

Yes. See an example here: https://github.com/biocad/web-template/blob/master/src/Web/Template/Servant/Auth.hs#L295

So apparently _swaggerSecurityDefinitions :: SecurityDefinitions was changed to _componentsSecuritySchemes :: Definitions SecurityScheme in 821654c which made SecurityDefinitions type useless (? still not sure)

Looks like so. However, there is one difference, namely in Semigroup instance for SecurityDefinitions: it uses unionWith (<>), while Definitions SecuritySchem would use the plain union which discards the second version of the conflicting key.

openapi3/src/Data/OpenApi/Internal.hs

Lines 1100 to 1102 in ea20d09

	instance Semigroup SecurityDefinitions where
	(SecurityDefinitions sd1) <> (SecurityDefinitions sd2) =
	SecurityDefinitions $ InsOrdHashMap.unionWith (<>) sd1 sd2

It would only matter for SecuritySchemeOAuth2 type:

openapi3/src/Data/OpenApi/Internal.hs

Lines 1094 to 1097 in ea20d09

	instance Semigroup SecurityScheme where
	SecurityScheme (SecuritySchemeOAuth2 lFlows) lDesc
	<> SecurityScheme (SecuritySchemeOAuth2 rFlows) rDesc =
	SecurityScheme (SecuritySchemeOAuth2 $ lFlows <> rFlows) (swaggerMappend lDesc rDesc)

Semigroup for this type merges two objects, in case they have different filled flow types. I think this is actually a correct behavior and dropping the newtype was a mistake.

Would you make a PR to fix this?

BTW, the newtype is still used in tests.

[gromakovsky]

Oh, I see, thanks.

Would you make a PR to fix this?

Yep, everything is clear now, so I'll probably do it, hopefully within a couple of weeks.

[maksbotan]

Thanks!