Skip to content

Fix/18552 cors wildcard credentials #19020

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.

Sign up for GitHub

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

Open
Bellambharath wants to merge 4 commits into
base: main
Choose a base branch
from
+73 −64
Open

Fix/18552 cors wildcard credentials #19020

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ac99102
Enhance IsAnyOriginAllowed method for safety
Bellambharath Mar 18, 2026
919f95f
Refactor CORS options configuration logic
Bellambharath Mar 18, 2026
43a1d2a
Refactor CORS options configuration logic
Bellambharath Mar 18, 2026
6f0f75c
Merge branch 'OrchardCMS:main' into fix/18552-cors-wildcard-credentials
Bellambharath Apr 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/OrchardCore.Modules/OrchardCore.Cors/Controllers/AdminController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,5 +147,7 @@ public async Task<ActionResult> IndexPOST()
}

private static bool IsAnyOriginAllowed(CorsPolicyViewModel corsPolicyViewModel)
=> corsPolicyViewModel.AllowAnyOrigin || corsPolicyViewModel.AllowedOrigins.Any(origin => origin == CorsConstants.AnyOrigin);
=> corsPolicyViewModel.AllowAnyOrigin
|| corsPolicyViewModel.AllowedOrigins?.Any(origin =>
string.Equals(origin?.Trim(), CorsConstants.AnyOrigin, StringComparison.Ordinal)) == true;
Comment on lines 149 to +152
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method now treats an origin entry of "" (including whitespace-trimmed) as "any origin". Since the UI warning shown on save is framed in terms of "AllowAnyOrigin", it can become confusing when the checkbox is off but "" is present in the list. Consider adjusting the user-facing warning text to reference "any origin / '*'" rather than only the checkbox name.

Copilot uses AI. Check for mistakes.
}
133 changes: 70 additions & 63 deletions src/OrchardCore.Modules/OrchardCore.Cors/Services/CorsOptionsConfiguration.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using Microsoft.AspNetCore.Cors.Infrastructure;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using CorsConstants = Microsoft.AspNetCore.Cors.Infrastructure.CorsConstants;

namespace OrchardCore.Cors.Services;

Expand All @@ -16,71 +17,77 @@ public CorsOptionsConfiguration(CorsService corsService, ILogger<CorsOptionsConf
}

public void Configure(CorsOptions options)
{
var corsSettings = _corsService.GetSettingsAsync().GetAwaiter().GetResult();
if (corsSettings?.Policies == null || !corsSettings.Policies.Any())
{
return;
}

foreach (var corsPolicy in corsSettings.Policies)
{
if (corsPolicy.AllowCredentials && corsPolicy.AllowAnyOrigin)
{
_logger.LogWarning("Using AllowCredentials and AllowAnyOrigin at the same time is considered a security risk, the {PolicyName} policy will not be loaded.", corsPolicy.Name);
continue;
}

options.AddPolicy(corsPolicy.Name, configurePolicy =>
{
if (corsPolicy.AllowAnyHeader)
{
configurePolicy.AllowAnyHeader();
}
else
{
configurePolicy.WithHeaders(corsPolicy.AllowedHeaders);
}
{
var corsSettings = _corsService.GetSettingsAsync().GetAwaiter().GetResult();
if (corsSettings?.Policies == null || !corsSettings.Policies.Any())
{
return;
}
Comment on lines 19 to +25
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation for the method opening brace/body is inconsistent with the surrounding 4-space indentation in this file (e.g., the { under Configure is currently offset). Please align formatting to match the rest of the module to avoid noisy diffs going forward.

Copilot uses AI. Check for mistakes.

if (corsPolicy.AllowAnyMethod)
{
configurePolicy.AllowAnyMethod();
}
else
{
configurePolicy.WithMethods(corsPolicy.AllowedMethods);
}
foreach (var corsPolicy in corsSettings.Policies)
{
var allowAnyOrigin = corsPolicy.AllowAnyOrigin
|| corsPolicy.AllowedOrigins?.Any(origin =>
string.Equals(origin?.Trim(), CorsConstants.AnyOrigin, StringComparison.Ordinal)) == true;
Comment on lines +29 to +31
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "any origin" detection logic is duplicated here and in AdminController.IsAnyOriginAllowed. To avoid the two drifting (e.g., different trimming/comparison rules), consider extracting a shared helper (e.g., on the settings model or a small internal utility) and reusing it in both places.

Copilot uses AI. Check for mistakes.

if (corsPolicy.AllowAnyOrigin)
{
configurePolicy.AllowAnyOrigin();
}
else
{
configurePolicy.WithOrigins(corsPolicy.AllowedOrigins);
}
if (corsPolicy.AllowCredentials && allowAnyOrigin)
{
_logger.LogWarning(
"Using AllowCredentials and AllowAnyOrigin at the same time is considered a security risk, the {PolicyName} policy will not be loaded.",
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The warning text mentions "AllowAnyOrigin" but this branch is now also triggered when AllowedOrigins contains the wildcard "" (even if AllowAnyOrigin is false). Consider rewording to refer to "any origin (including '')" so logs accurately reflect the configuration being rejected.

Suggested change
"Using AllowCredentials and AllowAnyOrigin at the same time is considered a security risk, the {PolicyName} policy will not be loaded.",
"Using AllowCredentials with any origin (including '*') is considered a security risk, the {PolicyName} policy will not be loaded.",

Copilot uses AI. Check for mistakes.
corsPolicy.Name);
continue;
}

if (corsPolicy.AllowCredentials)
{
configurePolicy.AllowCredentials();
}
else
{
configurePolicy.DisallowCredentials();
}
options.AddPolicy(corsPolicy.Name, configurePolicy =>
{
if (corsPolicy.AllowAnyHeader)
{
configurePolicy.AllowAnyHeader();
}
else
{
configurePolicy.WithHeaders(corsPolicy.AllowedHeaders);
}

if (corsPolicy.AllowAnyMethod)
{
configurePolicy.AllowAnyMethod();
}
else
{
configurePolicy.WithMethods(corsPolicy.AllowedMethods);
}

if (allowAnyOrigin)
{
configurePolicy.AllowAnyOrigin();
}
else
{
configurePolicy.WithOrigins(corsPolicy.AllowedOrigins);
}

if (corsPolicy.AllowCredentials)
{
configurePolicy.AllowCredentials();
}
else
{
configurePolicy.DisallowCredentials();
}

if (corsPolicy.ExposedHeaders?.Length > 0)
{
configurePolicy.WithExposedHeaders(corsPolicy.ExposedHeaders);
}
});

if (corsPolicy.IsDefaultPolicy)
{
options.DefaultPolicyName = corsPolicy.Name;
}
}

if (corsPolicy.ExposedHeaders?.Length > 0)
{
configurePolicy.WithExposedHeaders(corsPolicy.ExposedHeaders);
}
});

if (corsPolicy.IsDefaultPolicy)
{
options.DefaultPolicyName = corsPolicy.Name;
}
}

options.DefaultPolicyName ??= corsSettings.Policies.FirstOrDefault()?.Name;
}
options.DefaultPolicyName ??= corsSettings.Policies.FirstOrDefault()?.Name;
}
Comment on lines +89 to +92
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

options.DefaultPolicyName ??= corsSettings.Policies.FirstOrDefault()?.Name; can set the default policy name to a policy that was skipped above (e.g., invalid AllowCredentials + any-origin). That leaves DefaultPolicyName pointing at a policy that was never added via AddPolicy, which can cause runtime failures when the pipeline calls UseCors() without a policy name. Prefer selecting the first successfully-added policy name (track it during the loop), or leave DefaultPolicyName unset when no policies were loaded.

Copilot uses AI. Check for mistakes.
}
Loading