-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Set allowed headers via API instead of defaulting to wildcard. #3023
Merged
jefferai
merged 28 commits into
hashicorp:master
from
naunga:improvement/cors-allowed-headers
Aug 7, 2017
Merged
Changes from all commits
Commits
Show all changes
28 commits
Select commit
Hold shift + click to select a range
728ba85
This comment didn't make a lot of sense.
6c44bcb
Added new field AllowedHeaders. Enable func now accepts a slice of he…
bd4d006
Cleaned up comment.
1ee92b3
No need to have the preflight headers as a package global.
c4ca1e6
Added the headers param.
31770b6
Specifying the actual headers instead of using the wildcard.
8aad9e3
Added list of standard headers that are allowed on CORS requests.
aeae188
Made origin param no longer required to enable CORS. Defaults to wild…
89ef267
Allowed headers defaults to stdAllowedHeaders.
ce6d754
Clearing AllowedHeaders when CORS is disabled.
c1d629b
Added headers to call to Enable.
558e955
Updated comment.
ea6deba
Added list of allowed headers to response.
5d289ed
Added allowed_headers field.
e8a68e8
Initial commit of test.
704e7a3
Persisting the allowed headers.
158f690
Set default allowed headers if they have not been set yet.
5cd956d
allowed_origins is returned as an array.
dafb1f8
Added docs for allowed_headers.
e29e68a
Added allowed_headers to request and expected result.
7006926
Merge branch 'master' into improvement/cors-allowed-headers
0f54b4d
Removing unnecessary locking/unlocking.
0a05154
Fixing logic error.
88c20e1
Each call to Enable should replace AllowedHeaders, not append to the …
094e2a8
An origin to allow must be required when calling Enable.
5abce39
Update test to check that allowed_origins is required.
6aa4591
Indicating that allowed_origins is required.
a0b2fe5
Minor adjustments
jefferai File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
package http | ||
|
||
import ( | ||
"encoding/json" | ||
"net/http" | ||
"reflect" | ||
"testing" | ||
|
||
"github.com/hashicorp/vault/vault" | ||
) | ||
|
||
func TestSysConfigCors(t *testing.T) { | ||
var resp *http.Response | ||
|
||
core, _, token := vault.TestCoreUnsealed(t) | ||
ln, addr := TestServer(t, core) | ||
defer ln.Close() | ||
TestServerAuth(t, addr, token) | ||
|
||
corsConf := core.CORSConfig() | ||
|
||
// Try to enable CORS without providing a value for allowed_origins | ||
resp = testHttpPut(t, token, addr+"/v1/sys/config/cors", map[string]interface{}{ | ||
"allowed_headers": "X-Custom-Header", | ||
}) | ||
|
||
testResponseStatus(t, resp, 500) | ||
|
||
// Enable CORS, but provide an origin this time. | ||
resp = testHttpPut(t, token, addr+"/v1/sys/config/cors", map[string]interface{}{ | ||
"allowed_origins": addr, | ||
"allowed_headers": "X-Custom-Header", | ||
}) | ||
|
||
testResponseStatus(t, resp, 204) | ||
|
||
// Read the CORS configuration | ||
resp = testHttpGet(t, token, addr+"/v1/sys/config/cors") | ||
testResponseStatus(t, resp, 200) | ||
|
||
var actual map[string]interface{} | ||
var expected map[string]interface{} | ||
|
||
lenStdHeaders := len(corsConf.AllowedHeaders) | ||
|
||
expectedHeaders := make([]interface{}, lenStdHeaders) | ||
|
||
for i := range corsConf.AllowedHeaders { | ||
expectedHeaders[i] = corsConf.AllowedHeaders[i] | ||
} | ||
|
||
expected = map[string]interface{}{ | ||
"lease_id": "", | ||
"renewable": false, | ||
"lease_duration": json.Number("0"), | ||
"wrap_info": nil, | ||
"warnings": nil, | ||
"auth": nil, | ||
"data": map[string]interface{}{ | ||
"enabled": true, | ||
"allowed_origins": []interface{}{addr}, | ||
"allowed_headers": expectedHeaders, | ||
}, | ||
"enabled": true, | ||
"allowed_origins": []interface{}{addr}, | ||
"allowed_headers": expectedHeaders, | ||
} | ||
|
||
testResponseStatus(t, resp, 200) | ||
|
||
testResponseBody(t, resp, &actual) | ||
expected["request_id"] = actual["request_id"] | ||
|
||
if !reflect.DeepEqual(actual, expected) { | ||
t.Fatalf("bad: expected: %#v\nactual: %#v", expected, actual) | ||
} | ||
|
||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there ever a chance that Enable will be called multiple times? It seems like the right thing here would be to start fresh every time since this function is (presuambly) being given the canonical set of allowed headers. So rather than the check above and the check here, simply do:
Let me know if I'm missing something here...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enable certainly could be called multiple times, because this code allows new headers to be appended to the existing set. This workflow, however, clashes with the workflow for setting origins. Your suggestion is correct.