Skip to content

Commit

Permalink
Decouple the OpenID module from the Users/Roles modules
Browse files Browse the repository at this point in the history
  • Loading branch information
kevinchalet committed May 11, 2018
1 parent 0d0a9f0 commit 1cc0772
Show file tree
Hide file tree
Showing 13 changed files with 213 additions and 185 deletions.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,10 @@
using System.Linq;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.Identity;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.Localization;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Localization;
using Microsoft.Extensions.Options;
using OpenIddict.Core;
using OrchardCore.Admin;
using OrchardCore.DisplayManagement;
Expand All @@ -24,7 +22,6 @@
using OrchardCore.OpenId.ViewModels;
using OrchardCore.Security.Services;
using OrchardCore.Settings;
using OrchardCore.Users;

namespace OrchardCore.OpenId.Controllers
{
Expand All @@ -40,8 +37,6 @@ public class ApplicationController : Controller
private readonly OpenIdApplicationManager _applicationManager;
private readonly INotifier _notifier;
private readonly ShellDescriptor _shellDescriptor;
private readonly UserManager<IUser> _userManager;
private readonly IOptions<IdentityOptions> _identityOptions;

public ApplicationController(
IShapeFactory shapeFactory,
Expand All @@ -50,8 +45,6 @@ public ApplicationController(
IAuthorizationService authorizationService,
IRoleProvider roleProvider,
OpenIdApplicationManager applicationManager,
UserManager<IUser> userManager,
IOptions<IdentityOptions> identityOptions,
IHtmlLocalizer<ApplicationController> htmlLocalizer,
INotifier notifier,
ShellDescriptor shellDescriptor)
Expand All @@ -65,8 +58,6 @@ public ApplicationController(
_roleProvider = roleProvider;
_notifier = notifier;
_shellDescriptor = shellDescriptor;
_userManager = userManager;
_identityOptions = identityOptions;
}

public async Task<ActionResult> Index(PagerParameters pagerParameters)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,32 +1,20 @@
using System.Linq;
using System.Security.Claims;
using System.Threading.Tasks;
using AspNet.Security.OpenIdConnect.Primitives;
using Microsoft.AspNetCore.Authentication;
using Microsoft.AspNetCore.Identity;
using Microsoft.AspNetCore.Mvc;
using Microsoft.Extensions.Localization;
using Newtonsoft.Json.Linq;
using OpenIddict.Core;
using OrchardCore.Modules;
using OrchardCore.OpenId.Filters;
using OrchardCore.Users;

namespace OrchardCore.OpenId.Controllers
{
[Feature(OpenIdConstants.Features.Server)]
[OpenIdController, SkipStatusCodePages]
public class UserInfoController : Controller
{
private readonly IStringLocalizer<UserInfoController> T;
private readonly UserManager<IUser> _userManager;

public UserInfoController(
IStringLocalizer<UserInfoController> localizer,
UserManager<IUser> userManager)
{
T = localizer;
_userManager = userManager;
}

// GET/POST: /OrchardCore.OpenId/UserInfo/Me
[AcceptVerbs("GET", "POST")]
[IgnoreAntiforgeryToken]
Expand All @@ -42,41 +30,64 @@ public async Task<IActionResult> Me()
// Note: this controller doesn't use [Authorize] to prevent MVC Core from throwing
// an exception if the JWT/validation handler was not registered (e.g because the
// OpenID server feature was not enabled or because the configuration was invalid).
var result = await HttpContext.AuthenticateAsync(OpenIdConstants.Schemes.Userinfo);
if (result?.Principal == null)
{
return Challenge(OpenIdConstants.Schemes.Userinfo);
}

var user = await _userManager.GetUserAsync(result.Principal);
if (user == null)
var principal = (await HttpContext.AuthenticateAsync(OpenIdConstants.Schemes.Userinfo))?.Principal;
if (principal == null)
{
return Challenge(OpenIdConstants.Schemes.Userinfo);
}

var claims = new JObject();

// Note: the "sub" claim is a mandatory claim and must be included in the JSON response.
claims[OpenIdConnectConstants.Claims.Subject] = await _userManager.GetUserIdAsync(user);
claims[OpenIdConnectConstants.Claims.Subject] = principal.FindFirst(OpenIdConnectConstants.Claims.Subject)?.Value;

if (_userManager.SupportsUserEmail &&
result.Principal.HasClaim(OpenIdConnectConstants.Claims.Scope, OpenIdConnectConstants.Scopes.Email))
if (principal.HasClaim(OpenIdConnectConstants.Claims.Scope, OpenIdConnectConstants.Scopes.Email))
{
claims[OpenIdConnectConstants.Claims.Email] = await _userManager.GetEmailAsync(user);
claims[OpenIdConnectConstants.Claims.EmailVerified] = await _userManager.IsEmailConfirmedAsync(user);
var address = principal.FindFirst(OpenIdConnectConstants.Claims.Email)?.Value ??
principal.FindFirst(ClaimTypes.Email)?.Value;

if (!string.IsNullOrEmpty(address))
{
claims[OpenIdConnectConstants.Claims.Email] = address;

var status = principal.FindFirst(OpenIdConnectConstants.Claims.EmailVerified)?.Value;
if (!string.IsNullOrEmpty(status))
{
claims[OpenIdConnectConstants.Claims.EmailVerified] = bool.Parse(status);
}
}
}

if (_userManager.SupportsUserPhoneNumber &&
result.Principal.HasClaim(OpenIdConnectConstants.Claims.Scope, OpenIdConnectConstants.Scopes.Phone))
if (principal.HasClaim(OpenIdConnectConstants.Claims.Scope, OpenIdConnectConstants.Scopes.Phone))
{
claims[OpenIdConnectConstants.Claims.PhoneNumber] = await _userManager.GetPhoneNumberAsync(user);
claims[OpenIdConnectConstants.Claims.PhoneNumberVerified] = await _userManager.IsPhoneNumberConfirmedAsync(user);
var phone = principal.FindFirst(OpenIdConnectConstants.Claims.PhoneNumber)?.Value ??
principal.FindFirst(ClaimTypes.MobilePhone)?.Value ??
principal.FindFirst(ClaimTypes.HomePhone)?.Value ??
principal.FindFirst(ClaimTypes.OtherPhone)?.Value;

if (!string.IsNullOrEmpty(phone))
{
claims[OpenIdConnectConstants.Claims.PhoneNumber] = phone;

var status = principal.FindFirst(OpenIdConnectConstants.Claims.PhoneNumberVerified)?.Value;
if (!string.IsNullOrEmpty(status))
{
claims[OpenIdConnectConstants.Claims.PhoneNumberVerified] = bool.Parse(status);
}
}
}

if (_userManager.SupportsUserRole &&
result.Principal.HasClaim(OpenIdConnectConstants.Claims.Scope, OpenIddictConstants.Scopes.Roles))
if (principal.HasClaim(OpenIdConnectConstants.Claims.Scope, OpenIddictConstants.Scopes.Roles))
{
claims[OpenIddictConstants.Claims.Roles] = JArray.FromObject(await _userManager.GetRolesAsync(user));
var roles = principal.FindAll(OpenIdConnectConstants.Claims.Role)
.Concat(principal.FindAll(ClaimTypes.Role))
.Select(claim => claim.Value)
.ToArray<object>();

if (roles.Length != 0)
{
claims[OpenIddictConstants.Claims.Roles] = new JArray(roles);
}
}

// Note: the complete list of standard claims supported by the OpenID Connect specification
Expand Down
4 changes: 1 addition & 3 deletions src/OrchardCore.Modules/OrchardCore.OpenId/Manifest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,7 @@
OpenIdConstants.Features.Core,
OpenIdConstants.Features.Management,
"OrchardCore.Authentication",
"OrchardCore.DataProtection",
"OrchardCore.Users",
"OrchardCore.Roles"
"OrchardCore.DataProtection"
}
)]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,8 @@
<ProjectReference Include="..\..\OrchardCore\OrchardCore.Environment.Navigation\OrchardCore.Environment.Navigation.csproj" />
<ProjectReference Include="..\..\OrchardCore\OrchardCore.Recipes.Abstractions\OrchardCore.Recipes.Abstractions.csproj" />
<ProjectReference Include="..\..\OrchardCore\OrchardCore.OpenId.Core\OrchardCore.OpenId.Core.csproj" />
<ProjectReference Include="..\OrchardCore.Roles\OrchardCore.Roles.csproj" />
<ProjectReference Include="..\..\OrchardCore\OrchardCore.Users.Abstractions\OrchardCore.Users.Abstractions.csproj" />
<ProjectReference Include="..\OrchardCore.Settings\OrchardCore.Settings.csproj" />
<ProjectReference Include="..\OrchardCore.Users\OrchardCore.Users.csproj" />
</ItemGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,6 @@ public Task<ImmutableArray<ValidationResult>> ValidateSettingsAsync(OpenIdServer

var results = ImmutableArray.CreateBuilder<ValidationResult>();

if (!settings.AllowAuthorizationCodeFlow && !settings.AllowClientCredentialsFlow &&
!settings.AllowImplicitFlow && !settings.AllowPasswordFlow)
{
results.Add(new ValidationResult(T["At least one OpenID Connect flow must be enabled."]));
}

if (!string.IsNullOrEmpty(settings.Authority))
{
if (!Uri.TryCreate(settings.Authority, UriKind.Absolute, out Uri uri) || !uri.IsWellFormedOriginalString())
Expand Down Expand Up @@ -94,15 +88,15 @@ public Task<ImmutableArray<ValidationResult>> ValidateSettingsAsync(OpenIdServer
{
if (settings.CertificateStoreName == null)
{
results.Add(new ValidationResult(T["A Certificate Store Name is required."], new[]
results.Add(new ValidationResult(T["The certificate store name is required."], new[]
{
nameof(settings.CertificateStoreName)
}));
}

if (settings.CertificateStoreLocation == null)
{
results.Add(new ValidationResult(T["A Certificate Store Location is required."], new[]
results.Add(new ValidationResult(T["The certificate store location is required."], new[]
{
nameof(settings.CertificateStoreLocation)
}));
Expand Down Expand Up @@ -157,25 +151,31 @@ public Task<ImmutableArray<ValidationResult>> ValidateSettingsAsync(OpenIdServer
}
}

if (!settings.AllowAuthorizationCodeFlow && !settings.AllowClientCredentialsFlow &&
!settings.AllowImplicitFlow && !settings.AllowPasswordFlow)
{
results.Add(new ValidationResult(T["At least one OpenID Connect flow must be enabled."]));
}

if (settings.AllowPasswordFlow && !settings.EnableTokenEndpoint)
{
results.Add(new ValidationResult(T["Password Flow cannot be enabled if Token Endpoint is disabled"], new[]
results.Add(new ValidationResult(T["The password flow cannot be enabled wheb the token endpoint is disabled."], new[]
{
nameof(settings.AllowPasswordFlow)
}));
}

if (settings.AllowClientCredentialsFlow && !settings.EnableTokenEndpoint)
{
results.Add(new ValidationResult(T["Client Credentials Flow cannot be enabled if Token Endpoint is disabled"], new[]
results.Add(new ValidationResult(T["The client credentials flow cannot be enabled when the token endpoint is disabled."], new[]
{
nameof(settings.AllowClientCredentialsFlow)
}));
}

if (settings.AllowAuthorizationCodeFlow && (!settings.EnableAuthorizationEndpoint || !settings.EnableTokenEndpoint))
{
results.Add(new ValidationResult(T["Authorization Code Flow cannot be enabled if Authorization Endpoint and Token Endpoint are disabled"], new[]
results.Add(new ValidationResult(T["The authorization code flow cannot be enabled when the authorization and/or token endpoints are disabled."], new[]
{
nameof(settings.AllowAuthorizationCodeFlow)
}));
Expand All @@ -185,15 +185,15 @@ public Task<ImmutableArray<ValidationResult>> ValidateSettingsAsync(OpenIdServer
{
if (!settings.EnableTokenEndpoint)
{
results.Add(new ValidationResult(T["Refresh Token Flow cannot be enabled if Token Endpoint is disabled"], new[]
results.Add(new ValidationResult(T["The refresh token flow cannot be enabled when the token endpoint is disabled."], new[]
{
nameof(settings.AllowRefreshTokenFlow)
}));
}

if (!settings.AllowPasswordFlow && !settings.AllowAuthorizationCodeFlow)
{
results.Add(new ValidationResult(T["Refresh Token Flow only can be enabled if Password Flow, Authorization Code Flow or Hybrid Flow are enabled"], new[]
results.Add(new ValidationResult(T["The refresh token flow cannot be enabled when the password, authorization code and implicit flows are disabled."], new[]
{
nameof(settings.AllowRefreshTokenFlow)
}));
Expand All @@ -202,7 +202,7 @@ public Task<ImmutableArray<ValidationResult>> ValidateSettingsAsync(OpenIdServer

if (settings.AllowImplicitFlow && !settings.EnableAuthorizationEndpoint)
{
results.Add(new ValidationResult(T["Allow Implicit Flow cannot be enabled if Authorization Endpoint is disabled"], new[]
results.Add(new ValidationResult(T["The implicit flow cannot be enabled when the authorization endpoint is disabled."], new[]
{
nameof(settings.AllowImplicitFlow)
}));
Expand Down
3 changes: 0 additions & 3 deletions src/OrchardCore.Modules/OrchardCore.OpenId/Startup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,6 @@ public override void ConfigureServices(IServiceCollection services)
ServiceDescriptor.Transient<IPostConfigureOptions<OpenIddictOptions>, OpenIddictInitializer>(),
ServiceDescriptor.Transient<IPostConfigureOptions<OpenIddictOptions>, OpenIdConnectServerInitializer>()
});

// Disabling same-site is required for OpenID's module prompt=none support to work correctly.
services.ConfigureApplicationCookie(options => options.Cookie.SameSite = SameSiteMode.None);
}
}

Expand Down
16 changes: 16 additions & 0 deletions src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleStore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,22 @@ public async Task<IEnumerable<string>> GetRoleNamesAsync()
return roles.Roles.Select(x => x.RoleName).OrderBy(x => x).ToList();
}

public async Task<IEnumerable<Claim>> GetRoleClaimsAsync(string role, CancellationToken cancellationToken = default)
{
if (string.IsNullOrEmpty(role))
{
throw new ArgumentException("The role name cannot be null or empty.", nameof(role));
}

var entity = await FindByNameAsync(role, cancellationToken);
if (entity == null)
{
return Array.Empty<Claim>();
}

return await GetClaimsAsync(entity, cancellationToken);
}

#region IRoleStore<IRole>
public async Task<IdentityResult> CreateAsync(IRole role, CancellationToken cancellationToken)
{
Expand Down
5 changes: 5 additions & 0 deletions src/OrchardCore.Modules/OrchardCore.Users/Startup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,11 @@ public override void ConfigureServices(IServiceCollection services)
await SecurityStampValidator.ValidatePrincipalAsync(context);
}
};

// Disabling same-site is required for OpenID's module prompt=none support to work correctly.
// Note: it has no practical impact on the security of the site since all endpoints are always
// protected by antiforgery checks, that are enforced with or without this setting being changed.
options.Cookie.SameSite = SameSiteMode.None;
})
.AddCookie(IdentityConstants.ExternalScheme, options =>
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,8 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>netstandard2.0</TargetFramework>
<RootNamespace>OrchardCore.Security</RootNamespace>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Microsoft.AspNetCore.Identity" />
</ItemGroup>

</Project>
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
using System.Collections.Generic;
using System.Collections.Generic;
using System.Security.Claims;
using System.Threading;
using System.Threading.Tasks;

namespace OrchardCore.Security.Services
{
public interface IRoleProvider
{
Task<IEnumerable<string>> GetRoleNamesAsync();
Task<IEnumerable<Claim>> GetRoleClaimsAsync(string role, CancellationToken cancellationToken = default);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@ namespace OrchardCore.Users.Services
{
public interface IUserService
{
Task<IUser> AuthenticateAsync(string userName, string password, Action<string, string> reportError);
Task<IUser> CreateUserAsync(string userName, string email, string[] roleNames, string password, Action<string, string> reportError);
Task<bool> ChangePasswordAsync(IUser user, string currentPassword, string newPassword, Action<string, string> reportError);
Task<IUser> GetAuthenticatedUserAsync(ClaimsPrincipal principal);
Task<IUser> GetForgotPasswordUserAsync(string userIdentifier);
Task<bool> ResetPasswordAsync(string userIdentifier, string resetToken, string newPassword, Action<string, string> reportError);
Task<ClaimsPrincipal> CreatePrincipalAsync(IUser user);
}
}
Loading

0 comments on commit 1cc0772

Please sign in to comment.