Skip to content

Removed OPTION omission for CORS#189

Open
Meetesh Barua (maverick64) wants to merge 1 commit intoconfluentinc:masterfrom
maverick64:cors_fix
Open

Removed OPTION omission for CORS#189
Meetesh Barua (maverick64) wants to merge 1 commit intoconfluentinc:masterfrom
maverick64:cors_fix

Conversation

@maverick64
Copy link
Contributor

This had to be done for https://confluentinc.atlassian.net/browse/ESCALATION-3168.
When Customer has RBAC/CORS enabled together, anonymous requests run into issue because of jetty/OAuthBearerAuthenticator.
We came to conclusion to get it fixed in rest-utils.
This change will require all HTTP methods (including OPTION) to require authentication if it was enabled.

@ghost
Copy link

Confluent Inc. (@confluentinc) It looks like Meetesh Barua (@maverick64) just signed our Contributor License Agreement. 👍

Always at your service,

clabot

Copy link
Member

@rayokota Robert Yokota (rayokota) left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@maverick64 Meetesh Barua (maverick64) changed the base branch from master to 5.3.x July 9, 2020 20:39
@maverick64 Meetesh Barua (maverick64) changed the base branch from 5.3.x to master July 9, 2020 20:40
Copy link
Member

@xvrl Xavier Léauté (xvrl) left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will likely break browser CORS preflight requests, which require OPTIONS methods to not authenticate.

@maverick64
Copy link
Contributor Author

this will likely break browser CORS preflight requests, which require OPTIONS methods to not authenticate.

Added this flag through which we can disable preflight option check for the servers. The components can just add this flag to true and it will behave as before.
access.control.skip.options

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

Successfully merging this pull request may close these issues.

4 participants