Cors headers #2021
Cors headers #2021
Conversation
… are allowed to make cross-origin requests.
…. Added supporting code and funcs.
|
Thanks for submitting this. As I said in #796, it needs to be determined whether this should be a value configured via Vault's API and stored internally or set in the config file. |
|
@jefferai : Yep, and once that's determined I'll be happy to refactors things as needed. I figured though that getting things this far would be helpful. You know that and I like to write Go. So any chance I get to do that is a win. Cheers. |
|
Hi @naunga, We had a discussion internally -- we'd like to have CORS configured via the API so it is authenticated and lives in Vault's barrier, with the configuration endpoint living at sys/config/cors. If you're willing to change your code to behave that way it'd be great! |
|
Absolutely. I'm going to be traveling a bit this month. So I'll be slower than I was the first time, but definitely will get it refactored. |
|
Great! |
… the system backend, but accessible from the core.
|
@moolshankar 0.7.2 does not have these changes. The PR is not yet merged. |
|
@vishalnayak .. Oh okay, May I know when this PR is expected to be merged? If it is soon then we can wait or if it is going to take time, then I might need to add a server side layer as a workaround which I was trying to avoid actually. Thanks for quick responses :) |
|
@moolshankar The PR is in good shape. It needs review from other team members as well and all are pretty swamped at the moment. Hopefully this should get through in the next release, which is not very far. Can't promise a precise time frame though. |
|
@vishalnayak .... Cool. Thanks a lot Vishal for your help. |
|
@moolshankar You could always build your own Vault binary with this PR merged in locally. |
|
@jefferai .... Thanks for the tip Jeff. I will try that. |
|
|
||
| // IsEnabled returns the value of CORSConfig.isEnabled | ||
| func (c *CORSConfig) IsEnabled() bool { | ||
| c.RLock() |
There was a problem hiding this comment.
Can we use atomic.(Load,Save)Uint32 instead of locking here? The uints in use can be const values to make for meaningful comparisons.
I'm not sure for AllowedOrigins if it'd be faster to keep the lock or to use atomic.Value.
| @@ -62,6 +62,7 @@ func NewSystemBackend(core *Core, config *logical.BackendConfig) (logical.Backen | |||
| "replication/primary/secondary-token", | |||
| "replication/reindex", | |||
| "rotate", | |||
| "config/*", | |||
There was a problem hiding this comment.
This is too broad. Arguably config/cors*, or nothing.
|
I'm going to make these changes post-merge. Thanks for all the hard work on this! |
|
Cool. It was my pleasure. Now just gotta figure out what to work on next. 🤔 |
* oss/master: (161 commits) update gitignore changelog++ Exclude /sys/leases/renew from registering with expiration manager (#2891) More cleanup Clarify/fix some configuration info. Add a convenience function for copying a client (#2887) Better error messages using ListObjects than using HeadBucket. Might be a bigger request but messages are better than BadRequest, how this changes effect the messages are in the issue (#2892) Add ACL info to Consul configuration page Return error on bad CORS and add Header specification to API request primitive Add Zyborg.Vault PowerShell module to libs list (#2869) changelog++ CouchDB physical backend (#2880) Fix root paths test Add missing datadog vendored lib changelog++ Fix up CORS. Cors headers (#2021) Address review feedback Fix the test error message Added utility on router to fetch mount entry using its ID ...
This addresses issue #796.
Adds two new top-level configuration options:
I made the decision to make this a top-level option, because at the end of the day all CORs does is signal to a browser that the server is willing to communicate with it and which HTTP methods it will accept. It will not have an effect on whether or not stored secrets can be accessed.