Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Null effective policy causing exception. #7809

Closed
grantmcdade opened this issue May 22, 2018 · 21 comments
Closed

Null effective policy causing exception. #7809

grantmcdade opened this issue May 22, 2018 · 21 comments
Assignees
Labels
3 - Done bug PRI: 1 - Required Must be handled in a reasonable time

Comments

@grantmcdade
Copy link

grantmcdade commented May 22, 2018

Authorization Policy Exception

Using PolicyServer.Local causes the exception but I can't reprodure the exception in a test project.

Functional impact

The page fails to load. It seems to happen wherever the [Authorize] attribute is present. I have a global AuthorizeFilter as well for a default policy of require authenticated user.

Minimal repro steps

Browse to any page with the [Authorize] attribute (Test project works though)

Expected result

No exception.

Actual result

Exception is thrown.

Further technical details

[Check this line of core]

builder = builder ?? new AuthorizationPolicyBuilder(effectivePolicy);

ArgumentNullException: Value cannot be null.
 Parameter name: policy
Microsoft.AspNetCore.Authorization.AuthorizationPolicyBuilder.Combine(AuthorizationPolicy policy)
Microsoft.AspNetCore.Authorization.AuthorizationPolicyBuilder..ctor(AuthorizationPolicy policy)
Microsoft.AspNetCore.Mvc.Authorization.AuthorizeFilter.GetEffectivePolicyAsync(AuthorizationFilterContext context)
Microsoft.AspNetCore.Mvc.Authorization.AuthorizeFilter.OnAuthorizationAsync(AuthorizationFilterContext context)
Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.InvokeFilterPipelineAsync()
Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.InvokeAsync()
Microsoft.AspNetCore.Builder.RouterMiddleware.Invoke(HttpContext httpContext)
Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
Microsoft.AspNetCore.StaticFiles.StaticFileMiddleware.Invoke(HttpContext context)
Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.Invoke(HttpContext context)

Does anyone have any idea what might be causing this? I'm using ASP.NET Core 2.1 release candidate.

My package references are as follows:

    <PackageReference Include="Microsoft.AspNetCore.HttpsPolicy" Version="2.1.0-rc1-final" />
    <PackageReference Include="Microsoft.AspNetCore.Identity.UI" Version="2.1.0-rc1-final" />
    <PackageReference Include="Serilog" Version="2.7.1" />
    <PackageReference Include="Serilog.Extensions.Logging" Version="2.0.2" />
    <PackageReference Include="Serilog.Sinks.Console" Version="3.1.1" />
    <PackageReference Include="Serilog.Sinks.File" Version="4.0.0" />
    <PackageReference Include="StructureMap" Version="4.6.1" />
    <PackageReference Include="StructureMap.Microsoft.DependencyInjection" Version="1.4.0" />

My OS is Windows 10 and I'm using IIS Express as web server.

@grantmcdade
Copy link
Author

My configured roles and premissions are:

  "Policy": {
    "roles": [
        {
            "name": "Admin",
            "subjects": [],
            "identityRoles": [ "Admin" ]
        },
        {
            "name": "ToolsAdmin",
            "subjects": [],
            "identityRoles": [ "ToolsAdmin" ]
        },
        {
            "name": "Tester",
            "subjects": [],
            "identityRoles": [ "Tester" ]
        }
    ],
    "permissions": [
      {
        "name": "ManageUsers",
        "roles": [ "Admin" ]
      },
      {
        "name": "ManageRoles",
        "roles": [ "Admin" ]
      },
      {
        "name": "ManageTools",
        "roles": [ "Admin", "ToolsAdmin" ]
      },
      {
        "name": "ManageMedia",
        "roles": [ "Admin" ]
      },
      {
        "name": "ManageReports",
        "roles": [ "Tester" ]
      },
      {
        "name": "ViewStartQuestions",
        "roles": [ "Admin", "Tester" ]
      },
      {
        "name": "MakeTestChanges",
        "roles": [ "Tester" ]
      }
    ]
  }

@grantmcdade
Copy link
Author

I just logged an issue in AspNet/MVC but it is related to PolicyServer.Local so I would appreciate any input you migh have. cc @leastprivilege @brockallen

@mkArtakMSFT
Copy link
Member

Thanks for reporting this issue.

@javiercn, can you please look into this?

@javiercn
Copy link
Member

@HaoK As he's the expert in this area.

@grantmcdade I can't repro this.

Can you provide a minimal repro project that showcases the issue?

Thanks.

@grantmcdade
Copy link
Author

thanks @javiercn. I have already spent the last two days trying to do exactly that but as you found out it's not an easy problem to reproduce. I don't have this setup to debug this and since I'm no expert I don't know what might be causing the problem. I will continue to try and reproduce this in a smaller Project.

@HaoK
Copy link
Member

HaoK commented May 22, 2018

How do you get a null effective policy?

@HaoK
Copy link
Member

HaoK commented May 22, 2018

Startup.cs is a good place to start

@HaoK
Copy link
Member

HaoK commented May 22, 2018

This bug might be the same issue with a repro: aspnet/Security#1764

@HaoK
Copy link
Member

HaoK commented May 22, 2018

Ok so the issue appears to be when AuthorizeFilter is created with a null effective policy, Combine does not allow passing in a null policy, GetEffectivePolicy should be checking for null policy and combining with a empty policy rather than passing in null.

@HaoK
Copy link
Member

HaoK commented May 22, 2018

Ok so the workaround is to turn off AllowCombiningAuthorizeFilters in mvcOptions

@javiercn
Copy link
Member

@HaoK Can taking it from here?

@HaoK
Copy link
Member

HaoK commented May 22, 2018

Sure

@HaoK HaoK assigned HaoK and unassigned javiercn May 22, 2018
@HaoK
Copy link
Member

HaoK commented May 22, 2018

fyi @blowdart @ajcvickers

@grantmcdade
Copy link
Author

Thanks for looking into this so quickly. I have created the project to help with reproduction of the issue.

https://github.com/grantmcdade/CTG.Web

It is basically the original project with as much as possible stripped out. If you try to view "Account -> Details" while already logged in then the exception occurs.

@javiercn
Copy link
Member

@grantmcdade Thanks for the repor.

@HaoK Can you take a look to ensure your fix covers @grantmcdade issue?

@mkArtakMSFT mkArtakMSFT added this to the 2.2.0-preview1 milestone May 23, 2018
@mkArtakMSFT mkArtakMSFT modified the milestones: 2.2.0-preview1, 2.1.1 May 23, 2018
@mkArtakMSFT mkArtakMSFT added the servicing-consider Shiproom approval is required for the issue label May 23, 2018
@HaoK
Copy link
Member

HaoK commented May 23, 2018

Yep its definitely the same issue, so policy server uses a custom IAuthorizationPolicyProvider, so this is going to hit everyone who uses policy server + 2.1 :(

@HaoK
Copy link
Member

HaoK commented May 23, 2018

@grantmcdade have you confirmed that the workaround works for you as well? Turning off the combination behavior should avoid hitting this code path.

mvcOptions.AllowCombiningAuthorizeFilters = false

@jo-ninja
Copy link
Contributor

I am using PolicyServer.Local and also hitting this issue after upgrading from ASP.NET Core 2.0 to 2.1 RC.

mvcOptions.AllowCombiningAuthorizeFilters = false works for me. Thanks!

@grantmcdade
Copy link
Author

grantmcdade commented May 24, 2018 via email

@mkArtakMSFT
Copy link
Member

Moved this out to 2.2, as the workaround is pretty cheap and this doesn't meet the bar for a patch.

@mkArtakMSFT mkArtakMSFT added PRI: 1 - Required Must be handled in a reasonable time and removed servicing-consider Shiproom approval is required for the issue patch-proposed labels May 24, 2018
tatarincev added a commit to VirtoCommerce/vc-storefront that referenced this issue Jun 8, 2018
Workaround to avoid 'Null effective policy causing exception' (on logout)
aspnet/Mvc#7809
@HaoK HaoK added 3 - Done and removed 1 - Ready labels Jul 12, 2018
@HaoK
Copy link
Member

HaoK commented Jul 12, 2018

Fixed via #8068

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
3 - Done bug PRI: 1 - Required Must be handled in a reasonable time
Projects
None yet
Development

No branches or pull requests

5 participants