1

My understanding has always been that security attributes on methods will override security attributes on class, but that doesn't seem to be the case any more as the simple code below demonstrates:

class Program
{
    [PrincipalPermission(SecurityAction.Demand, Authenticated = true)] //<-- this passes
    class DumbClass
    {
        [PrincipalPermission(SecurityAction.Demand, Role = "ffff")] //<-- this passes (but shouldn't)
        public string EchoMethod(string input)
        {
            return input;
        }
    }

    static void Main(string[] args)
    {
        Thread.CurrentPrincipal = new ClaimsPrincipal(new ClaimsIdentity("manual"));

        //this should throw becuase the principal is not in the role "ffff"
        //BUT DOESN'T
        Console.WriteLine(new DumbClass().EchoMethod("this"));
    }
}

If I remove the declaration on the class then I get the expected security exception. Am I missing something really obvious. I'm using .Net 4.5

  • I believe that multiple `PrincipalPermissionAttributes` are combined using OR logic, whether on a class or on a member. Which would explain what you're seeing: no SecurityException for a user who is EITHER in the ffff role OR authenticated, which means any authenticated user. – Joe Oct 22 '15 at 21:28
  • Did you ever find a solution to this? I'm having the exact same problem. – BRW Jun 05 '18 at 18:40

2 Answers2

0

Because PrincipalPermissionAttribute Demands are combined using OR, and a class attribute is essentially the same as adding the attribute to each method, your example is equivalent to:

[PrincipalPermission(SecurityAction.Demand, Authenticated = true)] 
class DumbClass
{
    [PrincipalPermission(SecurityAction.Demand, Authenticated = true)] 
    public DumbClass()
    {
    }

    [PrincipalPermission(SecurityAction.Demand, Authenticated = true)] 
    [PrincipalPermission(SecurityAction.Demand, Role = "ffff")]
    public string EchoMethod(string input)
    {
        return input;
    }
}

and because of OR logic, your demand for Role="ffff" is redundant.

If you want to restrict EchoMethod to role "ffff", and allow authenticated users for all other methods, change your code to:

class DumbClass
{
    [PrincipalPermission(SecurityAction.Demand, Authenticated = true)] 
    public DumbClass()
    {
    }

    [PrincipalPermission(SecurityAction.Demand, Role = "ffff")]
    public string EchoMethod(string input)
    {
        return input;
    }

    [PrincipalPermission(SecurityAction.Demand, Authenticated = true)] 
    public string OtherMethod(string input)
    {
        return input;
    }

}
Joe
  • 122,218
  • 32
  • 205
  • 338
  • So Scott Guthrie is wrong? :-) "I have also then added two additional security demands on the 'LookupEmployee' and 'AddEmployee' methods." https://weblogs.asp.net/scottgu/Tip_2F00_Trick_3A00_-Adding-Authorization-Rules-to-Business-and-Data-Layers-using-PrincipalPermissionAttributes – Eric J. Oct 23 '19 at 21:26
-1

Change your code as so:

[PrincipalPermission(SecurityAction.Demand)] //<-- REMOVE Authenticated = true
class DumbClass
{
    [PrincipalPermission(SecurityAction.Demand, Role = "ffff")] //<-- this passes (but shouldn't)
    public string EchoMethod(string input)
    {
        return input;
    }
}

By setting Authenticated = true you are explicitly indicating that the user has already been authenticated when they may or may not have been.

DVK
  • 2,726
  • 1
  • 17
  • 20
  • Setting Authenticated=true doesn't authenticate anything, it just indicates Authentication is demanded. The scenario is they have to be authenticated to use the class, but they need to be authorised in a particular role to actually call the method. There may be other methods for which meerely being Authenticated is enough. See this old blog post from Scott Gu: http://weblogs.asp.net/scottgu/Tip_2F00_Trick_3A00_-Adding-Authorization-Rules-to-Business-and-Data-Layers-using-PrincipalPermissionAttributes – Paul Johnston Oct 22 '15 at 20:11
  • This is incorrect. SecurityAction.Demand is what specifies that authentication is demanded. Setting Authenticated = true explicitly specifies that principal has been authenticated already. That is directly from the MSDN documentation. https://msdn.microsoft.com/en-us/library/system.security.permissions.principalpermissionattribute.authenticated(v=vs.110).aspx – DVK Oct 22 '15 at 20:26
  • It depends on context- if asserting a permission that you may well be saying the current principal is authenicated, but if you are demanding, then you are requireing the current principal to be authenticated. If you protect a class or method with [PrincipalPermission(SecurityAction.Demand, Authenticated = true)] and call it with an UnAuthenticated principal it will FAIL with a security exception becuase there was demand to prove the identity has been Authenticated If you were correct, then how would you declare a security attribute that tested whether the identity was authenticated? – Paul Johnston Oct 22 '15 at 20:37
  • btw the code changes you suggest still result in the method being called when the user isn't in the role. Authenticated=true is the default setting for PrincipalPermissionAttribute. – Paul Johnston Oct 22 '15 at 20:45
  • The authenticated=true on the class demands the user be authenticated to instantiate the class. The PrincipalPermission attribute on the method is asking the user to be a part of that domain group or else they cant execute the method – David Dec 07 '16 at 00:52