feat(key-auth) skip authenticating preflight OPTIONS requests

[ecmendenhall]

Summary

Preflight OPTIONS requests explicitly exclude user credentials and custom headers. The key-auth plugin should not attempt to authenticate OPTIONS requests, since browsers will strip Kong's apiKey header.

(Worked on this one with @nicksamps)

Full changelog

• Check request method in KeyAuthHandler:access, short-circuit auth if type is "OPTIONS."
• Add unit test.

Issues resolved

Fix #1292

[Tieske]

I'm not an expert on this, but this completely disables the key-auth for options requests. Shouldn't this at least be configurable?

[ecmendenhall]

In principle, I don't think OPTIONS requests should be authenticated. In practice, upstream APIs probably handle OPTIONS differently, so maybe there's a case for making this configurable. I'd be interested in other opinions here, I'm not sure!

[thibaultcha]

I don't think there is ever a point in authenticating OPTIONS requests... Could be wrong, would love to see a use-case for it.
It would be worth adding a debug log when this plugin receives an OPTIONS request maybe?

[Tieske]

Considering this comment and linked question I'd prefer a defensive approach; make it an option and default to authenticating OPTIONS requests.

[thibaultcha]

Marked as incomplete for now until comments are addressed + configuration capability for this behavior, as discussed.

[Tieske]

Just a thought; instead of calling the option authenticate_preflight, wouldn't it be more flexible to provide a list of methods not to be authenticated.

As in; skip_authentication_on = { "OPTIONS", "GET" }

or; authenticate_methods = {} which then defaults to all methods if empty

[subnetmarco]

@ecmendenhall if we can get this PR updated by today we can include it in 0.9.1

[patrickml]

any updates?

[p0pr0ck5]

@ecmendenhall can you please rebase your PR? There are a number of merge conflicts here. Thanks!

[p0pr0ck5]

Going to close this, as the PR is stale and cannot be merged currently. Thanks for the initial contribution and discussion!

In the future, Kong's plugin architecture will likely be adjusted and expanded such that authentication plugins will already have this sort of logic built in, so that individual plugins do not need to worry about handling this logic. For now, one could workaround this by creating a separate API that forwards to the same upstream, handling only OPTIONS requests, that does not have this plugin enabled.

Thank you again for the initial thought for the patch here!

[ecmendenhall]

@p0pr0ck5 Hi Robert! Just want to say that upgrading to 0.10.x and setting up a separate API for OPTIONS requests worked perfectly for us. I think it's a much better solution anyways. Thanks for the suggestion!

[p0pr0ck5]

Awesome, thanks for the report!

[yoavi2]

Hi guys, is setting up a seperate api still the way to go? Should the cors plugin be used instead?