Hey sorry to pester the channel, but could someone take a look at this PR please: <https://github.co...

# http4k

[Ricardo]

Hey sorry to pester the channel, but could someone take a look at this PR please: https://github.com/http4k/http4k/pull/277

[dave]

The reason that it hasn't been merged is that I'm not sure that it's actually the right thing to do.

[dave]

I need to get some time to take a look at it, compare it against the actual swagger specs then then decide where to go from there.

[Ricardo]

What are your reservations?

[Ricardo]

Maybe it's something I could fix?

[dave]

For a start, we're not currently compliant with the spec as per OA:

After you have defined the security schemes in the securitySchemes section, you can apply them to the whole API or individual operations by adding the security section on the root level or operation level, respectively. When used on the root level, security applies the specified security schemes globally to all API operations, unless overridden on the operation level. In the following example, the API calls can be authenticated using either an API key or OAuth 2. The ApiKeyAuth and OAuth2 names refer to the schemes previously defined in securitySchemes.

[dave]

on 2 counts.
1. I think we currently do apply both securities (global and local) to every endpoint instead of overriding them
2. We need to work out a way to auth with "any" of the securities that are on each endpoint. We can possibly do this by applying each filter one by one and then "selecting" the one that passes, but I'm not sure about that as it probably has side-effects

[dave]

we will probably need to rework the ContractRoutingHttpHandler code entirely to make it work with many securities being applied

[Ricardo]

Yes, we did take a look at this. It's tricky: if all security definitions filtered with predicates this would be simple, but because the OAuth security definitions use filters, you can't OR them together easily.

[Ricardo]

But we decided not to fix this because http4k-contract already incorrectly composes security definition filters and considered it a separate issue.

[Ricardo]

Also, Google Cloud Endpoints handles the actual security so we just use NoOp filters locally.

[Ricardo]

Do you think the filter composition is something you want to fix for this PR?

[dave]

I want to fix both - I've just merged a change to handle the override behaviour, so endpoint security now trumps the global. Next is the multi security. Just to confirm, are you needing an OR operation to be supplied for each endpoint, or an AND operation (so all securities are applied). OA3 supports both, but for the moment I think we should attack OR as it's a more difficult problem (and also AND can be synthesised out of multiple filters)

[dave]

Thinking about this, a better way to do it would be to keep the a single "Security" definition for each contract and route (and hence an single filter), but to combine them from multiple other Security instances (using a supplied OrSecurity()/AndSecurity() construct) for both Filter construction and rendering. This would mean that we're not locking away the implementation in way which is inaccessible to the API user, and thus the retry/combination look can be externalised and tweaked as required.

[dave]

One open question is that although OpenaApi supports multiple auth schemes (in both OR and AND combinations), I can't see how AND schemes are represented in the JSON version from this doc: https://swagger.io/docs/specification/authentication/

[Ricardo]

@dave I like the idea of an OrSecurity and AndSecurity construct, very readable.

[Ricardo]

We don't need either, we use NoOp for our filtering in http4k and rely on Google Cloud Endpoints for auth, but if we switched we would want to OR security filters together.

[Ricardo]

Does this mean my PR is no longer required? Should I close it?

[dave]

Yeah, you can do as it's just going to clash. I've got a branch multiple-securities going so you can track progress.

[Ricardo]

Cool, I need to open a new branch, my branch also contains a minor bug fix we should probably get into http4k.

[Ricardo]

I'll cherry-pick and send a new PR

[dave]

There are 2 open questions still which I haven't answered before we can finish this off on the branch -
1. how are AND and OR securities represented in the JSON (for both OA2 and OA3)
2. The OA2 renderer seems only to render a single security instead of all of them. The OA3 one works fine 🤔

[Ricardo]

I think OR securities are listed at the same level, AND ones are nested.

[Ricardo]

Not sure about number 2.

[dave]

I've fixed 2 - there was a problem with the overriding

[Ricardo]

I have another proposal for changing the http4k-contract to support some of the other Google Cloud Endpoints.

[Ricardo]

Rather than push another PR, should I propose my change here first?

[dave]

that's probably wise 🙂