Skip to content

Throw if CORS policy is configured to allow credentials and any origin #7751

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

Merged
merged 2 commits into from
Feb 21, 2019
Merged

Throw if CORS policy is configured to allow credentials and any origin #7751

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a7642e2
Throw if CORS policy is configured to allow credentials and any origin
pranavkm Feb 20, 2019
3df5052
Fixup MVC tests
pranavkm Feb 21, 2019
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
3 changes: 1 addition & 2 deletions src/Middleware/CORS/samples/SampleDestination/Startup.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,7 @@ public void ConfigureServices(IServiceCollection services)
options.AddPolicy("AllowAll", policy => policy
.AllowAnyOrigin()
.AllowAnyMethod()
.AllowAnyHeader()
.AllowCredentials());
.AllowAnyHeader());
});
services.AddRouting();
}
Expand Down
3 changes: 1 addition & 2 deletions src/Middleware/CORS/samples/SampleDestination/StartupWithoutEndpointRouting.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,7 @@ public void Configure(IApplicationBuilder app)
innerBuilder.UseCors(policy => policy
.AllowAnyOrigin()
.AllowAnyMethod()
.AllowAnyHeader()
.AllowCredentials());
.AllowAnyHeader());

innerBuilder.UseMiddleware<SampleMiddleware>();
});
Expand Down
5 changes: 5 additions & 0 deletions src/Middleware/CORS/src/Infrastructure/CorsPolicyBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,11 @@ public CorsPolicyBuilder SetIsOriginAllowedToAllowWildcardSubdomains()
/// <returns>The constructed <see cref="CorsPolicy"/>.</returns>
public CorsPolicy Build()
{
if (_policy.AllowAnyOrigin && _policy.SupportsCredentials)
Copy link
Member

Choose a reason for hiding this comment

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

What about Combine?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You have to call Build to get to the the result of Combine.

{
throw new InvalidOperationException(Resources.InsecureConfiguration);
}

return _policy;
}

Expand Down
3 changes: 1 addition & 2 deletions src/Middleware/CORS/src/Infrastructure/CorsService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
using Microsoft.AspNetCore.Cors.Internal;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Logging.Abstractions;
using Microsoft.Extensions.Options;
using Microsoft.Extensions.Primitives;

Expand Down Expand Up @@ -77,7 +76,7 @@ public CorsResult EvaluatePolicy(HttpContext context, CorsPolicy policy)

if (policy.AllowAnyOrigin && policy.SupportsCredentials)
{
_logger.InsecureConfiguration();
throw new ArgumentException(Resources.InsecureConfiguration, nameof(policy));
}

var origin = context.Request.Headers[CorsConstants.Origin];
Expand Down
11 changes: 0 additions & 11 deletions src/Middleware/CORS/src/Internal/CORSLoggerExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ internal static class CORSLoggerExtensions
private static readonly Action<ILogger, string, Exception> _requestHeaderNotAllowed;
private static readonly Action<ILogger, Exception> _failedToSetCorsHeaders;
private static readonly Action<ILogger, Exception> _noCorsPolicyFound;
private static readonly Action<ILogger, Exception> _insecureConfiguration;
private static readonly Action<ILogger, Exception> _isNotPreflightRequest;

static CORSLoggerExtensions()
Expand Down Expand Up @@ -73,11 +72,6 @@ static CORSLoggerExtensions()
new EventId(10, "NoCorsPolicyFound"),
"No CORS policy found for the specified request.");

_insecureConfiguration = LoggerMessage.Define(
LogLevel.Warning,
new EventId(11, "InsecureConfiguration"),
"The CORS protocol does not allow specifying a wildcard (any) origin and credentials at the same time. Configure the policy by listing individual origins if credentials needs to be supported.");

_isNotPreflightRequest = LoggerMessage.Define(
LogLevel.Debug,
new EventId(12, "IsNotPreflightRequest"),
Expand Down Expand Up @@ -134,11 +128,6 @@ public static void NoCorsPolicyFound(this ILogger logger)
_noCorsPolicyFound(logger, null);
}

public static void InsecureConfiguration(this ILogger logger)
{
_insecureConfiguration(logger, null);
}

public static void IsNotPreflightRequest(this ILogger logger)
{
_isNotPreflightRequest(logger, null);
Expand Down
58 changes: 58 additions & 0 deletions src/Middleware/CORS/src/Properties/Resources.Designer.cs
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

71 changes: 0 additions & 71 deletions src/Middleware/CORS/src/Resources.Designer.cs
View file
Open in desktop

This file was deleted.

101 changes: 52 additions & 49 deletions src/Middleware/CORS/src/Resources.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?xml version="1.0" encoding="utf-8"?>
<root>
<!--
<!--
Microsoft ResX Schema

Version 2.0
Expand Down Expand Up @@ -60,63 +60,66 @@
: and then encoded with base64 encoding.
-->
<xsd:schema id="root" xmlns="" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:msdata="urn:schemas-microsoft-com:xml-msdata">
<xsd:import namespace="http://www.w3.org/XML/1998/namespace" />
<xsd:element name="root" msdata:IsDataSet="true">
<xsd:complexType>
<xsd:choice maxOccurs="unbounded">
<xsd:element name="metadata">
<xsd:complexType>
<xsd:sequence>
<xsd:element name="value" type="xsd:string" minOccurs="0" />
</xsd:sequence>
<xsd:attribute name="name" use="required" type="xsd:string" />
<xsd:attribute name="type" type="xsd:string" />
<xsd:attribute name="mimetype" type="xsd:string" />
<xsd:attribute ref="xml:space" />
</xsd:complexType>
</xsd:element>
<xsd:element name="assembly">
<xsd:complexType>
<xsd:attribute name="alias" type="xsd:string" />
<xsd:attribute name="name" type="xsd:string" />
</xsd:complexType>
</xsd:element>
<xsd:element name="data">
<xsd:complexType>
<xsd:sequence>
<xsd:element name="value" type="xsd:string" minOccurs="0" msdata:Ordinal="1" />
<xsd:element name="comment" type="xsd:string" minOccurs="0" msdata:Ordinal="2" />
</xsd:sequence>
<xsd:attribute name="name" type="xsd:string" use="required" msdata:Ordinal="1" />
<xsd:attribute name="type" type="xsd:string" msdata:Ordinal="3" />
<xsd:attribute name="mimetype" type="xsd:string" msdata:Ordinal="4" />
<xsd:attribute ref="xml:space" />
</xsd:complexType>
</xsd:element>
<xsd:element name="resheader">
<xsd:complexType>
<xsd:sequence>
<xsd:element name="value" type="xsd:string" minOccurs="0" msdata:Ordinal="1" />
</xsd:sequence>
<xsd:attribute name="name" type="xsd:string" use="required" />
</xsd:complexType>
</xsd:element>
</xsd:choice>
</xsd:complexType>
</xsd:element>
<xsd:import namespace="http://www.w3.org/XML/1998/namespace" />
<xsd:element name="root" msdata:IsDataSet="true">
<xsd:complexType>
<xsd:choice maxOccurs="unbounded">
<xsd:element name="metadata">
<xsd:complexType>
<xsd:sequence>
<xsd:element name="value" type="xsd:string" minOccurs="0" />
</xsd:sequence>
<xsd:attribute name="name" use="required" type="xsd:string" />
<xsd:attribute name="type" type="xsd:string" />
<xsd:attribute name="mimetype" type="xsd:string" />
<xsd:attribute ref="xml:space" />
</xsd:complexType>
</xsd:element>
<xsd:element name="assembly">
<xsd:complexType>
<xsd:attribute name="alias" type="xsd:string" />
<xsd:attribute name="name" type="xsd:string" />
</xsd:complexType>
</xsd:element>
<xsd:element name="data">
<xsd:complexType>
<xsd:sequence>
<xsd:element name="value" type="xsd:string" minOccurs="0" msdata:Ordinal="1" />
<xsd:element name="comment" type="xsd:string" minOccurs="0" msdata:Ordinal="2" />
</xsd:sequence>
<xsd:attribute name="name" type="xsd:string" use="required" msdata:Ordinal="1" />
<xsd:attribute name="type" type="xsd:string" msdata:Ordinal="3" />
<xsd:attribute name="mimetype" type="xsd:string" msdata:Ordinal="4" />
<xsd:attribute ref="xml:space" />
</xsd:complexType>
</xsd:element>
<xsd:element name="resheader">
<xsd:complexType>
<xsd:sequence>
<xsd:element name="value" type="xsd:string" minOccurs="0" msdata:Ordinal="1" />
</xsd:sequence>
<xsd:attribute name="name" type="xsd:string" use="required" />
</xsd:complexType>
</xsd:element>
</xsd:choice>
</xsd:complexType>
</xsd:element>
</xsd:schema>
<resheader name="resmimetype">
<value>text/microsoft-resx</value>
<value>text/microsoft-resx</value>
</resheader>
<resheader name="version">
<value>2.0</value>
<value>2.0</value>
</resheader>
<resheader name="reader">
<value>System.Resources.ResXResourceReader, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
<value>System.Resources.ResXResourceReader, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
</resheader>
<resheader name="writer">
<value>System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
<value>System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
</resheader>
<data name="InsecureConfiguration" xml:space="preserve">
<value>The CORS protocol does not allow specifying a wildcard (any) origin and credentials at the same time. Configure the CORS policy by listing individual origins if credentials needs to be supported.</value>
</data>
<data name="PreflightMaxAgeOutOfRange" xml:space="preserve">
<value>PreflightMaxAge must be greater than or equal to 0.</value>
</data>
Expand Down
18 changes: 16 additions & 2 deletions src/Middleware/CORS/test/UnitTests/CorsPolicyBuilderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
Expand Down Expand Up @@ -285,7 +285,6 @@ public void AllowCredential_SetsSupportsCredentials_ToTrue()
Assert.True(corsPolicy.SupportsCredentials);
}


[Fact]
public void DisallowCredential_SetsSupportsCredentials_ToFalse()
{
Expand All @@ -300,6 +299,21 @@ public void DisallowCredential_SetsSupportsCredentials_ToFalse()
Assert.False(corsPolicy.SupportsCredentials);
}

[Fact]
public void Build_ThrowsIfConfiguredToAllowAnyOriginWithCredentials()
{
// Arrange
var builder = new CorsPolicyBuilder()
.AllowAnyOrigin()
.AllowCredentials();

// Act
var ex = Assert.Throws<InvalidOperationException>(() => builder.Build());

// Assert
Assert.Equal(Resources.InsecureConfiguration, ex.Message);
}

[Theory]
[InlineData("Some-String", "some-string")]
[InlineData("x:\\Test", "x:\\test")]
Expand Down
Loading