CorsPolicyBuilder does not correctly combine policy with DefaultIsOriginAllowed
- Dominant language
- C#
- Stars
- 38.4k
- Forks
- 10.9k
- Avg merge
- 2d 10h
- Merged PRs (30d)
- 281
Description
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Describe the bug
A CorsPolicy has per default a DefaultIsOriginAllowed func which is responsible for evaluating if an origin is allowed. Because this func [encapsulates a member access to `Origins`](https://github.com/dotnet/aspnetcore/blob/7b5da426f9bd42fa515a6b67e0bd1504eacbc6c7/src/Middleware/CORS/src/Infrastructure/CorsPolicy.cs#L177), cors policy builder will not correctly combine two cors policies.
Following sample code demonstrates the problem. It is the Program.cs of a new .net 6 Web API Project.
```
using Microsoft.AspNetCore.Cors.Infrastructure;
var builder = WebApplication.CreateBuilder(args);
// Add services to the container.
builder.Services.AddControllers();
// Learn more about configuring Swagger/OpenAPI at https://aka.ms/aspnetcore/swashbuckle
builder.Services.AddEndpointsApiExplorer();
builder.Services.AddSwaggerGen();
// does not work
var policy1 = new CorsPolicyBuilder(new CorsPolicy())
.AllowAnyHeader()
.AllowAnyMethod()
.WithOrigins("https://www.microsoft.com")
.Build();
//builder.Services.AddCors(options => {
// options.AddDefaultPolicy(policy1);
//});
// works
var policy2 = new CorsPolicyBuilder("https://www.microsoft.com")
.AllowAnyHeader()
.AllowAnyMethod()
.Build();
// will not work if cors request is made for origin https://www.visualstudio.com
//var policy3 = new CorsPolicyBuilder(policy2)
// .WithOrigins("https://www.visualstudio.com")
// .Build();
builder.Services.AddCors(options => {
options.AddDefaultPolicy(policy2);
});
Console.WriteLine(policy1.IsOriginAllowed("https://www.microsoft.com")); // => false
Console.WriteLine(policy2.IsOriginAllowed("https://www.microsoft.com")); // => true
var app = builder.Build();
// Configure the HTTP request pipeline.
if (app.Environment.IsDevelopment()) {
app.UseSwagger();
app.UseSwaggerUI();
}
app.UseHttpsRedirection();
app.UseCors();
app.UseAuthorization();
app.MapControllers();
app.Run();
```
### Expected Behavior
Combining two policies using [`CorsPolicyBuilder`](https://github.com/dotnet/aspnetcore/blob/7b5da426f9bd42fa515a6b67e0bd1504eacbc6c7/src/Middleware/CORS/src/Infrastructure/CorsPolicyBuilder.cs#L29) where each policy has allowed origins should combine correctly.
One solution would be, that if the first policy has `IsDefaultIsOriginAllowed` set to true then the IsOriginAllowed func of the second policy should be kept in the [Combine](https://github.com/dotnet/aspnetcore/blob/7b5da426f9bd42fa515a6b67e0bd1504eacbc6c7/src/Middleware/CORS/src/Infrastructure/CorsPolicyBuilder.cs#L260) function. A prettier solution would probably be to consider changes around the IsDefaultIsOriginAllowed / IsOriginAllowed func approach, but might be more difficult to implement without breaking changes.
### Steps To Reproduce
Run following unit test.
```
[TestClass]
public class UnitTest1 {
[TestMethod]
public void CorsBuilder_should_combine_correctly() {
var policy1 = new CorsPolicyBuilder(new CorsPolicy())
.AllowAnyHeader()
.AllowAnyMethod()
.WithOrigins("https://www.microsoft.com")
.Build();
var policy2 = new CorsPolicyBuilder("https://www.microsoft.com")
.AllowAnyHeader()
.AllowAnyMethod()
.Build();
var policy3 = new CorsPolicyBuilder(policy2)
.WithOrigins("https://www.visualstudio.com")
.Build();
Assert.IsTrue(policy1.IsOriginAllowed("https://www.microsoft.com")); // fails
Assert.IsTrue(policy2.IsOriginAllowed("https://www.microsoft.com")); // works
Assert.IsTrue(policy3.IsOriginAllowed("https://www.microsoft.com")); // works
Assert.IsTrue(policy3.IsOriginAllowed("https://www.visualstudio.com")); // fails
}
}
```
### Exceptions (if any)
_No response_
### .NET Version
6.0.203
### Anything else?
_No response_
Contributor guide
Assessment
This issue has not been assessed yet.