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

NullReferenceException when using a "sealed" subclass of System.Web.Mvc.AuthorizeAttribute #220

Closed
johned3 opened this issue Aug 28, 2013 · 2 comments

Comments

@johned3
Copy link

johned3 commented Aug 28, 2013

MVC 4 running under .Net 4.0.
When using a subclass of System.Web.Mvc.AuthorizeAttribute where the class is "sealed", the following line throws a NullReferenceException:

class: MvcSiteMapProvider.Security.AuthorizeAttributeBuilder
method: public ConstructorInfo Build(Type parentType)
line: 48 - return definedType.GetConstructor(Type.EmptyTypes);

@NightOwl888
Copy link
Collaborator

This is an exact duplicate of #156, however I wasn't able to get enough information from the OP about the problem to understand it entirely.

I think I now understand it, but there doesn't appear to be a good way to fix this. Basically, as pointed out in this post, Microsoft didn't design the AuthorizeAttribute code to be used in this type of scenario. So, we are using "monkey patching" to subclass any custom AuthorizeAttribute and use IL emit to recreate the class with a public method that can be called to check whether we have access.

The only alternatives are as follows:

  1. Call the public OnAuthorization method
  2. Use reflection to call the protected AuthorizeCore method

The first method (as pointed out in the post) attaches a callback every time it is called that cannot be removed, which will cause a resource leak. MvcSiteMapProvider used to do this in a very old version, but was changed some time ago due to the info in that post.

The second method requires a full trust environment. I suppose that would work for some people, and I could potentially set it up to only use reflection in the case where the class is sealed (as long as the check to see if it is sealed doesn't require full trust).

Do you not have control over the declaration of the class? Is there some reason why it has to be sealed?

@NightOwl888
Copy link
Collaborator

I stand corrected. The writer of that post missed an option when calling OnAuthorization - to wrap the HTTP response cache and override the methods that attach the callback and set the response cache duration. We simply provide methods with no implementation, which effectively removes the 2 lines of code from AuthorizeAttribute that we don't want to run in this case, and fixes the problems with messing up the current response cache.

The fix is now available in v4.1.0.

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

No branches or pull requests

2 participants