Skip to content
This repository was archived by the owner on Nov 20, 2018. It is now read-only.

Add defaults + split SignIn/SignOut #873

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ namespace Microsoft.AspNetCore.Authentication
/// </summary>
public class AuthenticateResult
{
private AuthenticateResult() { }
protected AuthenticateResult() { }

/// <summary>
/// If a ticket was produced, authenticate was successful.
Expand All @@ -21,7 +21,7 @@ private AuthenticateResult() { }
/// <summary>
/// The authentication ticket.
/// </summary>
public AuthenticationTicket Ticket { get; private set; }
public AuthenticationTicket Ticket { get; protected set; }

/// <summary>
/// Gets the claims-principal with authenticated user identities.
Expand All @@ -36,18 +36,12 @@ private AuthenticateResult() { }
/// <summary>
/// Holds failure information from the authentication.
/// </summary>
public Exception Failure { get; private set; }

/// <summary>
/// Indicates that stage of authentication was directly handled by user intervention and no
/// further processing should be attempted.
/// </summary>
public bool Handled { get; private set; }
public Exception Failure { get; protected set; }

/// <summary>
/// Indicates that there was no information returned for this authentication scheme.
/// </summary>
public bool Nothing { get; private set; }
public bool None { get; protected set; }

/// <summary>
/// Indicates that authentication was successful.
Expand All @@ -63,23 +57,13 @@ public static AuthenticateResult Success(AuthenticationTicket ticket)
return new AuthenticateResult() { Ticket = ticket };
}

/// <summary>
/// Indicates that stage of authentication was directly handled by user intervention and no
/// further processing should be attempted.
/// </summary>
/// <returns>The result.</returns>
public static AuthenticateResult Handle()
{
return new AuthenticateResult() { Handled = true };
}

/// <summary>
/// Indicates that there was no information returned for this authentication scheme.
/// </summary>
/// <returns>The result.</returns>
public static AuthenticateResult None()
public static AuthenticateResult NoResult()
Copy link
Member Author

Choose a reason for hiding this comment

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

@Tratcher how about we keep this None, but call it NoResult() on the ResultContext's that will get exposed to events?

Copy link
Member

Choose a reason for hiding this comment

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

That would leave you with a method and property that have the same name. NoResult is fine here.

Copy link
Member Author

Choose a reason for hiding this comment

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

These are currently called Nothing and None(), I just don't see much benefit renaming None => NoResult as we do have usages in the code internally already today (i.e. Identity), we can keep NoResult on ResultContext so the public API for events is still the same.

Copy link
Member

Choose a reason for hiding this comment

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

NoResult() and None are better than None() and Nothing.

{
return new AuthenticateResult() { Nothing = true };
return new AuthenticateResult() { None = true };
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ public static Task ForbidAsync(this HttpContext context, string scheme) =>
context.ForbidAsync(scheme, properties: null);

/// <summary>
/// Extension method for Forbid.
/// Extension method for Forbid using the <see cref="AuthenticationOptions.DefaultForbidScheme"/> scheme..
/// </summary>
/// <param name="context">The <see cref="HttpContext"/> context.</param>
/// <returns>The task.</returns>
Expand Down Expand Up @@ -142,6 +142,13 @@ public static Task SignInAsync(this HttpContext context, ClaimsPrincipal princip
public static Task SignInAsync(this HttpContext context, string scheme, ClaimsPrincipal principal, AuthenticationProperties properties) =>
context.RequestServices.GetRequiredService<IAuthenticationService>().SignInAsync(context, scheme, principal, properties);

/// <summary>
/// Extension method for SignOut using the <see cref="AuthenticationOptions.DefaultSignOutScheme"/>.
/// </summary>
/// <param name="context">The <see cref="HttpContext"/> context.</param>
/// <returns>The task.</returns>
public static Task SignOutAsync(this HttpContext context) => context.SignOutAsync(scheme: null, properties: null);

/// <summary>
/// Extension method for SignOut.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// 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.Security.Claims;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Http;

Expand Down Expand Up @@ -39,20 +38,5 @@ public interface IAuthenticationHandler
/// <param name="properties">The <see cref="AuthenticationProperties"/> that contains the extra meta-data arriving with the authentication.</param>
/// <returns>A task.</returns>
Task ForbidAsync(AuthenticationProperties properties);

/// <summary>
/// Handle sign in.
/// </summary>
/// <param name="user">The <see cref="ClaimsPrincipal"/> user.</param>
/// <param name="properties">The <see cref="AuthenticationProperties"/> that contains the extra meta-data arriving with the authentication.</param>
/// <returns>A task.</returns>
Task SignInAsync(ClaimsPrincipal user, AuthenticationProperties properties);

/// <summary>
/// Signout behavior.
/// </summary>
/// <param name="properties">The <see cref="AuthenticationProperties"/> that contains the extra meta-data arriving with the authentication.</param>
/// <returns>A task.</returns>
Task SignOutAsync(AuthenticationProperties properties);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ namespace Microsoft.AspNetCore.Authentication
/// </summary>
public interface IAuthenticationRequestHandler : IAuthenticationHandler
{

/// <summary>
/// Returns true if request processing should stop.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// 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.Security.Claims;
using System.Threading.Tasks;

namespace Microsoft.AspNetCore.Authentication
{
/// <summary>
/// Used to determine if a handler supports SignIn.
/// </summary>
public interface IAuthenticationSignInHandler : IAuthenticationSignOutHandler
{
/// <summary>
/// Handle sign in.
/// </summary>
/// <param name="user">The <see cref="ClaimsPrincipal"/> user.</param>
/// <param name="properties">The <see cref="AuthenticationProperties"/> that contains the extra meta-data arriving with the authentication.</param>
/// <returns>A task.</returns>
Task SignInAsync(ClaimsPrincipal user, AuthenticationProperties properties);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// 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.Threading.Tasks;

namespace Microsoft.AspNetCore.Authentication
{
/// <summary>
/// Used to determine if a handler supports SignOut.
/// </summary>
public interface IAuthenticationSignOutHandler : IAuthenticationHandler
Copy link
Member

Choose a reason for hiding this comment

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

Why the derivation?

Copy link
Member Author

Choose a reason for hiding this comment

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

We require every handler to implement authenticate/challenge/forbid at a minimum, derivation makes this clearer, if we didn't require this, wouldn't that lead ppl to think they could only implement SignOut and use their handler?

{
/// <summary>
/// Signout behavior.
/// </summary>
/// <param name="properties">The <see cref="AuthenticationProperties"/> that contains the extra meta-data arriving with the authentication.</param>
/// <returns>A task.</returns>
Task SignOutAsync(AuthenticationProperties properties);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,4 @@ public interface IClaimsTransformation
/// <returns>The transformed principal.</returns>
Task<ClaimsPrincipal> TransformAsync(ClaimsPrincipal principal);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Options;
Expand Down Expand Up @@ -36,14 +35,16 @@ public AuthenticationSchemeProvider(IOptions<AuthenticationOptions> options)

private IDictionary<string, AuthenticationScheme> _map = new Dictionary<string, AuthenticationScheme>(StringComparer.Ordinal);
private List<AuthenticationScheme> _requestHandlers = new List<AuthenticationScheme>();
private List<AuthenticationScheme> _signOutHandlers = new List<AuthenticationScheme>();
private List<AuthenticationScheme> _signInHandlers = new List<AuthenticationScheme>();

/// <summary>
/// Returns the scheme that will be used by default for <see cref="IAuthenticationService.AuthenticateAsync(HttpContext, string)"/>.
/// This is typically specified via <see cref="AuthenticationOptions.DefaultAuthenticateScheme"/>.
/// Otherwise, if only a single scheme exists, that will be used, if more than one exists, null will be returned.
/// </summary>
/// <returns>The scheme that will be used by default for <see cref="IAuthenticationService.AuthenticateAsync(HttpContext, string)"/>.</returns>
public Task<AuthenticationScheme> GetDefaultAuthenticateSchemeAsync()
public virtual Task<AuthenticationScheme> GetDefaultAuthenticateSchemeAsync()
{
if (_options.DefaultAuthenticateScheme != null)
{
Expand All @@ -59,20 +60,16 @@ public Task<AuthenticationScheme> GetDefaultAuthenticateSchemeAsync()
/// <summary>
/// Returns the scheme that will be used by default for <see cref="IAuthenticationService.ChallengeAsync(HttpContext, string, AuthenticationProperties)"/>.
/// This is typically specified via <see cref="AuthenticationOptions.DefaultChallengeScheme"/>.
/// Otherwise, if only a single scheme exists, that will be used, if more than one exists, null will be returned.
/// Otherwise, this will fallback to <see cref="GetDefaultAuthenticateSchemeAsync"/>.
/// </summary>
/// <returns>The scheme that will be used by default for <see cref="IAuthenticationService.ChallengeAsync(HttpContext, string, AuthenticationProperties)"/>.</returns>
public Task<AuthenticationScheme> GetDefaultChallengeSchemeAsync()
public virtual Task<AuthenticationScheme> GetDefaultChallengeSchemeAsync()
{
if (_options.DefaultChallengeScheme != null)
{
return GetSchemeAsync(_options.DefaultChallengeScheme);
}
if (_map.Count == 1)
{
return Task.FromResult(_map.Values.First());
}
return Task.FromResult<AuthenticationScheme>(null);
return GetDefaultAuthenticateSchemeAsync();
}

/// <summary>
Expand All @@ -81,7 +78,7 @@ public Task<AuthenticationScheme> GetDefaultChallengeSchemeAsync()
/// Otherwise, this will fallback to <see cref="GetDefaultChallengeSchemeAsync"/> .
/// </summary>
/// <returns>The scheme that will be used by default for <see cref="IAuthenticationService.ForbidAsync(HttpContext, string, AuthenticationProperties)"/>.</returns>
public Task<AuthenticationScheme> GetDefaultForbidSchemeAsync()
public virtual Task<AuthenticationScheme> GetDefaultForbidSchemeAsync()
{
if (_options.DefaultForbidScheme != null)
{
Expand All @@ -93,34 +90,40 @@ public Task<AuthenticationScheme> GetDefaultForbidSchemeAsync()
/// <summary>
/// Returns the scheme that will be used by default for <see cref="IAuthenticationService.SignInAsync(HttpContext, string, System.Security.Claims.ClaimsPrincipal, AuthenticationProperties)"/>.
/// This is typically specified via <see cref="AuthenticationOptions.DefaultSignInScheme"/>.
/// Otherwise, if only a single scheme exists, that will be used, if more than one exists, null will be returned.
/// If only a single sign in handler scheme exists, that will be used, if more than one exists,
/// this will fallback to <see cref="GetDefaultAuthenticateSchemeAsync"/>.
/// </summary>
/// <returns>The scheme that will be used by default for <see cref="IAuthenticationService.SignInAsync(HttpContext, string, System.Security.Claims.ClaimsPrincipal, AuthenticationProperties)"/>.</returns>
public Task<AuthenticationScheme> GetDefaultSignInSchemeAsync()
public virtual Task<AuthenticationScheme> GetDefaultSignInSchemeAsync()
{
if (_options.DefaultSignInScheme != null)
{
return GetSchemeAsync(_options.DefaultSignInScheme);
}
if (_map.Count == 1)
if (_signInHandlers.Count == 1)
{
return Task.FromResult(_map.Values.First());
return Task.FromResult(_signInHandlers[0]);
}
return Task.FromResult<AuthenticationScheme>(null);
return GetDefaultAuthenticateSchemeAsync();
}

/// <summary>
/// Returns the scheme that will be used by default for <see cref="IAuthenticationService.SignOutAsync(HttpContext, string, AuthenticationProperties)"/>.
/// This is typically specified via <see cref="AuthenticationOptions.DefaultSignOutScheme"/>.
/// Otherwise, this will fallback to <see cref="GetDefaultSignInSchemeAsync"/> .
/// If only a single sign out handler scheme exists, that will be used, if more than one exists,
/// this will fallback to <see cref="GetDefaultSignInSchemeAsync"/> if that supoorts sign out.
/// </summary>
/// <returns>The scheme that will be used by default for <see cref="IAuthenticationService.SignOutAsync(HttpContext, string, AuthenticationProperties)"/>.</returns>
public Task<AuthenticationScheme> GetDefaultSignOutSchemeAsync()
public virtual Task<AuthenticationScheme> GetDefaultSignOutSchemeAsync()
{
if (_options.DefaultSignOutScheme != null)
{
return GetSchemeAsync(_options.DefaultSignOutScheme);
}
if (_signOutHandlers.Count == 1)
{
return Task.FromResult(_signOutHandlers[0]);
}
return GetDefaultSignInSchemeAsync();
}

Expand All @@ -129,7 +132,7 @@ public Task<AuthenticationScheme> GetDefaultSignOutSchemeAsync()
/// </summary>
/// <param name="name">The name of the authenticationScheme.</param>
/// <returns>The scheme or null if not found.</returns>
public Task<AuthenticationScheme> GetSchemeAsync(string name)
public virtual Task<AuthenticationScheme> GetSchemeAsync(string name)
{
if (_map.ContainsKey(name))
{
Expand All @@ -142,7 +145,7 @@ public Task<AuthenticationScheme> GetSchemeAsync(string name)
/// Returns the schemes in priority order for request handling.
/// </summary>
/// <returns>The schemes in priority order for request handling</returns>
public Task<IEnumerable<AuthenticationScheme>> GetRequestHandlerSchemesAsync()
public virtual Task<IEnumerable<AuthenticationScheme>> GetRequestHandlerSchemesAsync()
{
return Task.FromResult<IEnumerable<AuthenticationScheme>>(_requestHandlers);
}
Expand All @@ -151,7 +154,7 @@ public Task<IEnumerable<AuthenticationScheme>> GetRequestHandlerSchemesAsync()
/// Registers a scheme for use by <see cref="IAuthenticationService"/>.
/// </summary>
/// <param name="scheme">The scheme.</param>
public void AddScheme(AuthenticationScheme scheme)
public virtual void AddScheme(AuthenticationScheme scheme)
{
if (_map.ContainsKey(scheme.Name))
{
Expand All @@ -167,6 +170,14 @@ public void AddScheme(AuthenticationScheme scheme)
{
_requestHandlers.Add(scheme);
}
if (typeof(IAuthenticationSignInHandler).IsAssignableFrom(scheme.HandlerType))
{
_signInHandlers.Add(scheme);
}
if (typeof(IAuthenticationSignOutHandler).IsAssignableFrom(scheme.HandlerType))
{
_signOutHandlers.Add(scheme);
}
_map[scheme.Name] = scheme;
Copy link
Member

@Tratcher Tratcher Jun 17, 2017

Choose a reason for hiding this comment

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

There's no check that the HandlerType actually implements IAuthenticationHandler, which the doc comments say is required.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's done in the scheme itself,

if (!typeof(IAuthenticationHandler).IsAssignableFrom(handlerType))

}
}
Expand All @@ -175,7 +186,7 @@ public void AddScheme(AuthenticationScheme scheme)
/// Removes a scheme, preventing it from being used by <see cref="IAuthenticationService"/>.
/// </summary>
/// <param name="name">The name of the authenticationScheme being removed.</param>
public void RemoveScheme(string name)
public virtual void RemoveScheme(string name)
{
if (!_map.ContainsKey(name))
{
Expand All @@ -186,15 +197,15 @@ public void RemoveScheme(string name)
if (_map.ContainsKey(name))
{
var scheme = _map[name];
_requestHandlers.Remove(_requestHandlers.Where(s => s.Name == name).FirstOrDefault());
_requestHandlers.Remove(scheme);
_signInHandlers.Remove(scheme);
_signOutHandlers.Remove(scheme);
_map.Remove(name);
}
}
}

public Task<IEnumerable<AuthenticationScheme>> GetAllSchemesAsync()
{
return Task.FromResult<IEnumerable<AuthenticationScheme>>(_map.Values);
}
public virtual Task<IEnumerable<AuthenticationScheme>> GetAllSchemesAsync()
=> Task.FromResult<IEnumerable<AuthenticationScheme>>(_map.Values);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,10 @@ public virtual async Task SignInAsync(HttpContext context, string scheme, Claims
}
}

var handler = await Handlers.GetHandlerAsync(context, scheme);
var handler = await Handlers.GetHandlerAsync(context, scheme) as IAuthenticationSignInHandler;
if (handler == null)
{
throw new InvalidOperationException($"No authentication handler is configured to handle the scheme: {scheme}");
throw new InvalidOperationException($"No IAuthenticationSignInHandler is configured to handle sign in for the scheme: {scheme}");
}

await handler.SignInAsync(principal, properties);
Expand All @@ -173,15 +173,20 @@ public virtual async Task SignInAsync(HttpContext context, string scheme, Claims
/// <returns>A task.</returns>
public virtual async Task SignOutAsync(HttpContext context, string scheme, AuthenticationProperties properties)
{
if (string.IsNullOrEmpty(scheme))
if (scheme == null)
{
throw new ArgumentException(nameof(scheme));
var defaultScheme = await Schemes.GetDefaultSignOutSchemeAsync();
scheme = defaultScheme?.Name;
if (scheme == null)
{
throw new InvalidOperationException($"No authenticationScheme was specified, and there was no DefaultSignOutScheme found.");
}
}

var handler = await Handlers.GetHandlerAsync(context, scheme);
var handler = await Handlers.GetHandlerAsync(context, scheme) as IAuthenticationSignOutHandler;
if (handler == null)
{
throw new InvalidOperationException($"No authentication handler is configured to handle the scheme: {scheme}");
throw new InvalidOperationException($"No IAuthenticationSignOutHandler is configured to handle sign out for the scheme: {scheme}");
}

await handler.SignOutAsync(properties);
Expand Down
Loading