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

Setting allowed origins via an ICorsPolicyService does not set access-control-allow-origin for OPTIONS requests #1251

Open
TomGathercole opened this issue May 9, 2024 · 6 comments

Comments

@TomGathercole
Copy link

Which version of Duende IdentityServer are you using?
6.0.0

Which version of .NET are you using?
.NET 8.0

Describe the bug
Found when using oidc-client-ts with loadUserInfo: true, which triggers an OPTIONS request to /connect/userinfo (Presumably because it contains an Authorization header).

When the OPTIONS request is sent, origins specified in ICorsPolicyService are not included in the validation process, and the access-control-allow-origin header is not set.

To Reproduce
This only occurs when multiple CORS policies are used. Broadly speaking, application builder config has to look like:

app.UseCors(s => s
    .WithOrigins("https://example.com")
    .AllowAnyHeader()
    .AllowAnyMethod()
);
app.UseIdentityServer();

Then the request below will be returned without an access-control-allow-origin header, even if https://someotherdomain.com is accepted by the ICorsPolicyService:

curl "https://localhost:5000/connect/userinfo" -X OPTIONS ... -H "Origin: https://someotherdomain.com"

Expected behavior
Setting allowed origins in ICorsPolicyService should work for OPTIONS requests.

Additional context
It looks like this is caused because the aspnetcore CORS middleware will short-circuit OPTIONS requests. See:
https://github.com/dotnet/aspnetcore/blob/d6c161969965bfeff71110ea5b3462afacfa9f24/src/Middleware/CORS/src/Infrastructure/CorsMiddleware.cs#L177

Ordinarily, both cors middleware instances will run and either the first or second will set the right headers on the response. For OPTIONS though, only the first instance of the middleware is processed.

Technically I might be able to resolve this by swapping the order of UseCors and UseIdentityServer, but this would give the opposite problem for other OPTIONS requests.

I've been able to trick this into working by creating a dummy endpoint that can teach the CORS middleware which policy to use:

app.UseRouting()
app.UseCors(s => s
    .WithOrigins("https://example.com")
    .AllowAnyHeader()
    .AllowAnyMethod()
);
app.UseIdentityServer();
app.UseEndpoints(endpoints =>
{
    endpoints.MapGet("/connect/userinfo", () => { }).WithMetadata(new EnableCorsAttribute("Duende.IdentityServer"));
});

But this is extremely unpleasant and brittle.

@RolandGuijt
Copy link

The OPTIONS request is a preflight request which basically checks if the actual request that is sent right after the OPTIONS request is allowed by the currently set CORS policy. It is handled by the CORS middleware and isn't intended for the other participants in the pipeline. That's why it is short-circuited.
When there's no access-control-allow-origin header in the response it would mean that access is not allowed. Could it be that there's a mismatch with origins somewhere?
In your example it seems that the origin in the CORS policy is different from the one in your example request.

This doesn't seem to have anything to do with IdentityServer, do you agree? If so maybe the ASP.NET Core issue tracker is the right place to continue the conversation.

@TomGathercole
Copy link
Author

So the origin absolutely does not match - that's why I'm trying to use the provided ICorsPolicyService to support clients accessing the connect/userinfo endpoint. The fact that a preflight OPTIONS request is sent before this request is determined by the browser, and will happen in most scenarios (since it is an authenticated request with credentials).

I do believe this is an identity server issue, because it shows that the ICorsPolicyService extension point does not work as expected. In particular, for clients registered via this service to be able to make a connect/userinfo call, they will also need to be registered in the site-wide default CORs policy (which would render the whole ICorsPolicyService pointless).

@RolandGuijt
Copy link

I misinterpreted your question, my apologies.
I'll investigate this and get back to you.

@RolandGuijt
Copy link

Are you registering the ICorsPolicyService in DI before IdentityServer? Please take a look here, last paragraph.

@TomGathercole
Copy link
Author

I think the last paragraph refers to ICorsPolicyProvider, not ICorsPolicyService, unless I'm misunderstanding? I'm not using a custom ICorsPolicyProvider, and the ICorsPolicyService is registered using:

services.AddIdentityServer(...)
    .AddCorsPolicyService<MyCorsPolicyService>();

@AndersAbel
Copy link
Member

Technically I might be able to resolve this by swapping the order of UseCors and UseIdentityServer, but this would give the opposite problem for other OPTIONS requests.

The IdentityServer Cors handling is designed to work together with any other Cors policies in the hosting Asp.Net Core application. The IdentityServer Cors handling will only be activated for the IdentityServer protocol endpoints where Cors is relevant.

To make these work together it is important that your own app.UseCors(...) is called after app.UseIdentityServer(...). The IdentityServer Cors handling will not interfer with OPTIONS requests that are for non-IdentityServer endpoints. They will be handled by your own Cors policy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants