Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Allow CORS requests in Secret Store API #10584

Merged
merged 5 commits into from
Apr 20, 2019

Conversation

adetante
Copy link

On the same model as what is done for JSON-RPC, this PR allows to specify the list of domains to allow for cross-domain requests to Secret Store API.

With CLI flag --secretstore-http-cors=http://mydomain or in the toml configuration file.

The default behavior is the same as the current one: all cross-domain requests are rejected.

@parity-cla-bot
Copy link

It looks like @adetante signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

@sorpaas sorpaas added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Apr 11, 2019
Copy link
Collaborator

@svyatonik svyatonik left a comment

Choose a reason for hiding this comment

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

Thanks for contribution! Have you considered using cors module from jsonrpc-server-utils crate? Looks like there are some caveats (like case-insensitive origin matching) that are covered there.

secret-store/src/listener/http_listener.rs Outdated Show resolved Hide resolved
secret-store/src/listener/http_listener.rs Outdated Show resolved Hide resolved
secret-store/src/listener/http_listener.rs Outdated Show resolved Hide resolved
@adetante
Copy link
Author

You're right: I'm going to change my implementation to the jsonrpc-server-utils crate.
Thank you for the feedback.

@svyatonik svyatonik added A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. and removed A8-looksgood 🦄 Pull request is reviewed well. labels Apr 12, 2019
@adetante
Copy link
Author

I updated based on your comments.
Thank you for reviewing

Copy link
Collaborator

@svyatonik svyatonik left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@svyatonik
Copy link
Collaborator

Test failure seems unrelated - SS tests are working fine on my laptop.

@svyatonik svyatonik added A8-looksgood 🦄 Pull request is reviewed well. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Apr 15, 2019
@sorpaas sorpaas merged commit 4cc274e into openethereum:master Apr 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants