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

distinguish between explicit and implicit * #118

Merged

Conversation

cainlevy
Copy link
Contributor

Fixes #117 by distinguishing between an explicit and implicit AllowedOrigins([]string{"*"}). This means that the origin will be reflected when a custom validator says it's okay and there is no contradictory AllowedOrigins(...) configuration.

If there is no custom validator, or the validator is paired with an explicit * (oops), then the allowed origin is a properly opaque * that will disable credentials.

Scrutiny and ideas for alternate implementations are welcome! 😓

@elithrar
Copy link
Contributor

Thanks @cainlevy; will review this weekend.

@ejcx: any qualms?

@cainlevy
Copy link
Contributor Author

cainlevy commented Feb 1, 2018

Anything I can do to move this forward?

@ejcx
Copy link
Contributor

ejcx commented Feb 1, 2018

Whoops. looks like I was the blocker here. Looks like this makes ACAO * always turned on, which is safe? I don't really know why you would want that but I don't see any issues with it.

@cainlevy
Copy link
Contributor Author

cainlevy commented Feb 1, 2018

My intent here is that ACAO * is returned when:

  • CORS is running with all defaults (no custom validator, no specified origins)
  • CORS has been configured with an explicit *

Otherwise, the current origin is reflected on the assumption that it has been previously validated by either a custom validator or a matching origin.

@ejcx
Copy link
Contributor

ejcx commented Feb 1, 2018

SGTM!

@kisielk
Copy link
Contributor

kisielk commented Mar 21, 2018

@elithrar what's the word on this one?

@kisielk kisielk requested a review from elithrar March 21, 2018 16:40
@elithrar elithrar self-assigned this Apr 16, 2018
@elithrar elithrar merged commit c5874fa into gorilla:master Apr 16, 2018
@elithrar
Copy link
Contributor

Sorry for the long delay here! Merged.

@cainlevy cainlevy deleted the reflect_origin_with_custom_validator branch April 16, 2018 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants