Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Security: Support regular expressions for CORS allow-origin to match against #6923

Merged
merged 1 commit into from Jul 25, 2014

Conversation

Projects
None yet
3 participants
@spinscale
Copy link
Member

spinscale commented Jul 18, 2014

This commit adds regular expression support for the allow-origin
header depending on the value of the request Origin header.

Relates #5601
Closes #6891

@s1monw

View changes

src/main/java/org/elasticsearch/http/netty/NettyHttpChannel.java Outdated
String originHeader = request.header("Origin");
if (Strings.isNullOrEmpty(originHeader) || corsPattern == null) {
if (corsPattern != null) {
resp.headers().add("Access-Control-Allow-Origin", "*");

This comment has been minimized.

Copy link
@s1monw

s1monw Jul 21, 2014

Contributor

can we make the "Access-Control-Allow-Origin" a constant?

@s1monw

View changes

src/main/java/org/elasticsearch/rest/support/RestUtils.java Outdated
* Determine if CORS setting is a regex
*/
public static Pattern getCorsSettingRegex(Settings settings) {
String corsSetting = settings.get("http.cors.allow-origin", "*");

This comment has been minimized.

Copy link
@s1monw

s1monw Jul 21, 2014

Contributor

can this be a constant?

@s1monw

View changes

src/main/java/org/elasticsearch/rest/support/RestUtils.java Outdated
public static Pattern getCorsSettingRegex(Settings settings) {
String corsSetting = settings.get("http.cors.allow-origin", "*");
int len = corsSetting.length();
boolean isRegex = len > 2 && corsSetting.charAt(0) == '/' && corsSetting.charAt(len-1) == '/';

This comment has been minimized.

Copy link
@s1monw

s1monw Jul 21, 2014

Contributor

can this be corsSettings.startsWith / corsSettings.endsWith instead of direct char access?

@s1monw

View changes

src/test/java/org/elasticsearch/rest/CorsRegexTests.java Outdated

@Before
public void init() {
System.setProperty("sun.net.http.allowRestrictedHeaders", "true");

This comment has been minimized.

Copy link
@s1monw

s1monw Jul 21, 2014

Contributor

I think the test framework might complain about this if it's not properly unset. Yet, I we can mute that by extending
private static final String[] IGNORED_INVARIANT_PROPERTIES = { ... in AbstractRandomizedTest#186

@s1monw

View changes

src/test/java/org/elasticsearch/rest/CorsRegexTests.java Outdated
private final Map<String, String> headers = new HashMap<>();

@Override
protected Settings nodeSettings(int nodeOrdinal) {

This comment has been minimized.

Copy link
@s1monw

s1monw Jul 21, 2014

Contributor

This test checks for the manually set value, can we have another test that checks the behavior for the default value? I mean it should be a simple copy of this one I guess no?

@s1monw

This comment has been minimized.

Copy link
Contributor

s1monw commented Jul 21, 2014

@spinscale I reviewed it and left some comments. Overall I think it looks good

@s1monw s1monw removed the review label Jul 21, 2014

@spinscale

This comment has been minimized.

Copy link
Member Author

spinscale commented Jul 21, 2014

@s1monw added tests, incorporated your comments

@s1monw

This comment has been minimized.

Copy link
Contributor

s1monw commented Jul 21, 2014

LGTM

@s1monw s1monw removed the review label Jul 23, 2014

CORS: Support regular expressions for origin to match against
This commit adds regular expression support for the allow-origin
header depending on the value of the request `Origin` header.

The existing HttpRequestBuilder is also extended to support the
OPTIONS HTTP method.

Relates #5601
Closes #6891

@spinscale spinscale merged commit a1e335b into elastic:master Jul 25, 2014

@kimchy kimchy referenced this pull request Aug 4, 2014

Closed

Disable CORS by default #7151

@clintongormley clintongormley changed the title CORS: Support regular expressions for origin to match against Security: Support regular expressions for CORS allow-origin to match against Sep 8, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.