Remove the Mode parameter from AuthorizeAttribute #956

Closed
halter73 opened this Issue Nov 12, 2012 · 2 comments

4 participants

@halter73
SignalR member

Instead of having one AuthorizeAttribute which currently silently does nothing when you apply the Outgoing Mode to a Hub method, have two attributes: AuthorizeAttribute and AuthorizeInvocationAttribute.

Applying the AuthorizeAttribute to a Hub class would work exactly as it does today with the default Mode: Both. When applied to a Hub class, the AuthorizeAttribute would require authentication to both subscribe to the Hub's client-side invocations and to invoke any of the Hub's sever-side methods. It should be illegal to apply this attribute to a Hub method.

The AuthorizeInvocationAttribute would be equivalent to the current AuthorizeAttribute with the Mode: Incoming. When applied to a Hub method, the AuthorizeInvocationAttribute would require authentication to invoke the server-side Hub method, but unauthenticated clients could still subscribe to client-side Hub invocations, even those initiated from inside the method with the attribute. Applying the AuthoriziedInvocationAttribute on a Hub class would be equivalent of applying the attribute to all of the Hub's methods.

This change should make the authorization functionality clearer. Most importantly, developers will be less likely to think they are requiring authorization for things that they are really not, e.g. client-side Hub invocation at the sever-side method level.

This combination of attributes would not provide a way to prevent unauthenticated clients from subscribing to a Hub's client-side invocations while still allowing unauthenticated clients to invoke the Hub's server-side methods. I think this use case is very uncommon and can be worked around by either using multiple hubs or creating a specialized HubPipelineModule.

@halter73 halter73 was assigned Nov 12, 2012
@DamianEdwards
SignalR member

We've settled on changing this to a boolean and a runtime exception for invalid settings:

[Authorize]
public class FooHub
{
    public void Blah()
    {

    }
}

[Authorize(RequireOutgoing=false)]
public class FooHub
{
    public void Blah()
    {

    }
}

public class FooHub
{
    [Authorize]
    public void Blah()
    {

    }
}

public class FooHub
{
    [Authorize(RequireOutgoing=true)]
    public void Blah()
    {
        // throws InvalidOperationException
    }
}
@halter73 halter73 added a commit that referenced this issue Jan 3, 2013
@halter73 halter73 Implements new AuthorizeAttribute API designed in #956.
This gets rid of the old Mode parameter and replaces it with a
RequireOutgoing parameter.

The AuthorizeModule now passes in an appliesToMethod parameter to
AuthorizeHubMethodInvocation so the AuthorizeAttribute can throw if
it is applied to a method with RequireOutgoing set to true.
db1201a
@halter73 halter73 added a commit that referenced this issue Jan 3, 2013
@halter73 halter73 Implements new AuthorizeAttribute API designed in #956.
This gets rid of the old Mode parameter and replaces it with a
RequireOutgoing parameter.

The AuthorizeModule now passes in an appliesToMethod parameter to
AuthorizeHubMethodInvocation so the AuthorizeAttribute can throw if
it is applied to a method with RequireOutgoing set to true.
b81041b
@Xiaohongt
SignalR member

verified

@Xiaohongt Xiaohongt closed this Jan 6, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment