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

CorsConfiguration based on pattern matching #24763

Closed
korektur opened this issue Mar 22, 2020 · 7 comments
Closed

CorsConfiguration based on pattern matching #24763

korektur opened this issue Mar 22, 2020 · 7 comments
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: superseded An issue that has been superseded by another type: enhancement A general enhancement

Comments

@korektur
Copy link
Contributor

Affects: Spring Web 5.2.4.RELEASE


Initial issue opened in Spring Cloud Gateway: spring-cloud/spring-cloud-gateway#1607

Application APIs can be called by multiple origins. For applications that represent some kind of platform within a company it often makes sense to accept all request from orings coming from company domain, basically accept all request with origin matching a pattern *.company.com. Currently default spring CorsConfiguration supports only list of origins, I think this might not be flexible enough, as it will require configuration change every time new application within a company wants to call that REST api (especially in case of an API Gateway).

Proposal: add support for CORS configuration based on a pattern matching.
One option would'be, I guess, to extend existing CorsConfiguration class with additional fields allowedOriginPatterns and modify checkOrigin to use this property.
Or have separate CorsConfiguration implementation that is using pattern matching.

While current solution is extendible and this approach can be implemented within the application itself, I think this should be a part of framework because it's a widely used functionality and is supported by other frameworks that are used for API implementation.

For example vert.x CorsHandler suipports that: https://github.com/vert-x3/vertx-web/blob/5cd7ecaa6bead1b246b5327537ee9b82c22187bc/vertx-web/src/main/java/io/vertx/ext/web/handler/CorsHandler.java#L42
Also, Kong supports pattern based origins as well.

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

Historically, we have built CorsConfiguration capabilities by staying pretty close to CORS specification. It currently supports plain list of origins and *. Notice that CorsConfiguration#checkOrigin is more flexible than the specification since it supports * also when allowCredentials is true.

But indeed patterns like *.company.com or regexp are not supported yet, and I tend to agree that could be a useful feature, maybe worth to consider for 5.3. We could extend * support patterns like *.company.com or support regexp.

Any thoughts @rstoyanchev?

@rstoyanchev
Copy link
Contributor

Yes, I think it would be useful to support that.

@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 Mar 23, 2020
@rstoyanchev rstoyanchev added this to the 5.x Backlog milestone Mar 23, 2020
@korektur
Copy link
Contributor Author

@rstoyanchev can I do the change myself and open a merge request?

@rstoyanchev
Copy link
Contributor

@korektur yes if you submit a pull request, we'll review it.

korektur added a commit to korektur/spring-framework that referenced this issue May 1, 2020
korektur added a commit to korektur/spring-framework that referenced this issue May 1, 2020
korektur added a commit to korektur/spring-framework that referenced this issue May 2, 2020
More tests added.
Combininig CorsConfiguration fixed.

Closes spring-projectsgh-24763
korektur added a commit to korektur/spring-framework that referenced this issue May 4, 2020
More tests added.
Combininig CorsConfiguration fixed.

Closes spring-projectsgh-24763
@korektur
Copy link
Contributor Author

korektur commented May 5, 2020

@rstoyanchev I have opened a pull request - #25016
Can you please review and let me know if that approach is suitable.
Thanks.

@korektur
Copy link
Contributor Author

@rstoyanchev @sdeleuze can you review the merge request please?
#25016
It's been open for almost a month now without any comments, unfortunatelly.

@rstoyanchev
Copy link
Contributor

Superseded by #25016.

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) status: superseded An issue that has been superseded by another type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants