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

Add support for CORS origins defined by regular expressions #99

Conversation

devin-brenton
Copy link

Currently, the config is defined in a JSON file. Therefore, all entries to the cors.origins array must be strings. However, the CORS npm package used in blueoak-server supports regular expressions as CORS origins. In order to utilize that functionality, this PR converts any string in the origin array into a RegExp if it starts with ^.

Currently, the config is defined in a JSON file. Therefore, all entries to the cors.origins array must be strings. However, the cors npm package used in blueoak-server supports regular expressions as cors origins. In order to utilize that functionality, this PR converts any string in the origin array into a RegExp if it starts with a leading slash.
@seanpk seanpk self-assigned this Jan 24, 2018
Copy link
Member

@seanpk seanpk left a comment

Choose a reason for hiding this comment

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

Why not use the inline RegExp format for this?
i.e. keying off the string starting with /:
/.+\.foobar\.net/

Generally, there are many options that can be passed to the cors middleware that can't be expressed in JSON.
What if we provide support such that if config.get('cors') is a string instead of an object, that we treat it as a file path and try to require it and use it as the options parameter?

Tests are required.

// Convert any CORS origin defined with regex (i.e. starts with '^')
// from JSON string into an actual RegExp object.
for (var i = 0; i < cfg.origin.length; i++) {
if (cfg.origin[i].startsWith('^')) {
Copy link
Member

Choose a reason for hiding this comment

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

String.prototype.startsWith is not supported by older versions of node.js which BOS supports

@seanpk
Copy link
Member

seanpk commented Feb 20, 2018

In the case the led to the PR, the project switched to using a .js config file and this is no longer needed.
If support is to be added along these lines, it will happen somehow related to #100.

@seanpk seanpk closed this Feb 20, 2018
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.

2 participants