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

Support for pattern based origin cors configuration #25016

Conversation

korektur
Copy link
Contributor

@korektur korektur commented May 5, 2020

Fix #24763

I have modified CorsConfiguration to support pattern based origins.

Obviously that can be done in a different ways, like using somehing else instead of standard java Pattern or creating a new delegatePatternCorsConfiguration instead of modifying existing one, so please let me know if the approach I've chosen is not the most suitable.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label May 5, 2020
@korektur
Copy link
Contributor Author

@sdeleuze @rstoyanchev can you review please?

@rstoyanchev rstoyanchev self-assigned this Jul 6, 2020
@rstoyanchev rstoyanchev added in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jul 6, 2020
@rstoyanchev rstoyanchev added this to the 5.3 M2 milestone Jul 6, 2020
korektur added 2 commits July 6, 2020 19:19
# Conflicts:
#	spring-webmvc/src/test/java/org/springframework/web/servlet/handler/CorsAbstractHandlerMappingTests.java
#	spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/CrossOriginTests.java
More tests added.
Combininig CorsConfiguration fixed.

Closes spring-projectsgh-24763
@rstoyanchev
Copy link
Contributor

@korektur, on a first pass this looks okay to me and I can take it from here.

One thing I'd like to refine is the pattern syntax to be just wildcards, as in your original #24763 (comment) "*.company.com" rather than a full regex expression ".*\\.company\\.com". Internally it can still be converted to a regex so this is more about keeping the syntax simpler and more readable. Does that work for your cases?

Note also that typically it is better to rebase your PR rather than doing merges. I have corrected that locally, so please don't make any further changes.

@korektur
Copy link
Contributor Author

korektur commented Jul 7, 2020

@rstoyanchev yes, that makes sense.

@bb-deepak
Copy link

@korektur curious if this is now available. Couldn't find any documentation for this

@korektur
Copy link
Contributor Author

korektur commented Oct 8, 2021

@bb-deepak
Copy link

thank you, it's indeed mentioned in the spring cors config docs, instead of mentioned as a configurable property in spring cloud gateway docs example hence a bit difficult to find - https://cloud.spring.io/spring-cloud-gateway/reference/html/#cors-configuration
@korektur .

@michael-o
Copy link

Can someone for the stupid explain why this violates CORS specs with "*" and allowCredentials: true? From an external point of view it does not matter how you match internally, this should just work like with previous fixed strings allowedOrigins.

FTR: https://stackoverflow.com/a/19744754/696632

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CorsConfiguration based on pattern matching
5 participants