-
Notifications
You must be signed in to change notification settings - Fork 10.3k
Integrate AuthorizeView with actual authorization #10487
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
Changes from all commits
a8d7459
1d2e19a
b69ed3f
39f84df
e00fc6a
f63cdce
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
// 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; | ||
using Microsoft.Extensions.Logging; | ||
|
||
namespace Microsoft.AspNetCore.Blazor.Services | ||
{ | ||
internal class WebAssemblyConsoleLogger<T> : ILogger<T>, ILogger | ||
{ | ||
public IDisposable BeginScope<TState>(TState state) | ||
{ | ||
return NoOpDisposable.Instance; | ||
} | ||
|
||
public bool IsEnabled(LogLevel logLevel) | ||
{ | ||
return logLevel >= LogLevel.Warning; | ||
} | ||
|
||
public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func<TState, Exception, string> formatter) | ||
{ | ||
var formattedMessage = formatter(state, exception); | ||
Console.WriteLine($"[{logLevel}] {formattedMessage}"); | ||
} | ||
|
||
private class NoOpDisposable : IDisposable | ||
{ | ||
public static NoOpDisposable Instance = new NoOpDisposable(); | ||
|
||
public void Dispose() { } | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
// 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 Microsoft.Extensions.Logging; | ||
|
||
namespace Microsoft.AspNetCore.Blazor.Services | ||
{ | ||
internal class WebAssemblyLoggerFactory : ILoggerFactory | ||
{ | ||
public void AddProvider(ILoggerProvider provider) | ||
{ | ||
// No-op | ||
} | ||
|
||
public ILogger CreateLogger(string categoryName) | ||
=> new WebAssemblyConsoleLogger<object>(); | ||
|
||
public void Dispose() | ||
{ | ||
// No-op | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
// 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; | ||
using Microsoft.AspNetCore.Authorization; | ||
|
||
namespace Microsoft.AspNetCore.Components | ||
{ | ||
// This is so the AuthorizeView can avoid implementing IAuthorizeData (even privately) | ||
internal class AuthorizeDataAdapter : IAuthorizeData | ||
{ | ||
private readonly AuthorizeView _component; | ||
|
||
public AuthorizeDataAdapter(AuthorizeView component) | ||
{ | ||
_component = component ?? throw new ArgumentNullException(nameof(component)); | ||
} | ||
|
||
public string Policy | ||
{ | ||
get => _component.Policy; | ||
set => throw new NotSupportedException(); | ||
} | ||
|
||
public string Roles | ||
{ | ||
get => _component.Roles; | ||
set => throw new NotSupportedException(); | ||
} | ||
|
||
// AuthorizeView doesn't expose any such parameter, as it wouldn't be used anyway, | ||
// since we already have the ClaimsPrincipal by the time AuthorizeView gets involved. | ||
public string AuthenticationSchemes | ||
{ | ||
get => null; | ||
set => throw new NotSupportedException(); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,20 +1,26 @@ | ||
@namespace Microsoft.AspNetCore.Components | ||
@using System.Security.Claims | ||
@using Microsoft.AspNetCore.Authorization | ||
@inject IAuthorizationService AuthorizationService | ||
@inject IAuthorizationPolicyProvider AuthorizationPolicyProvider | ||
|
||
@if (currentAuthenticationState == null) | ||
{ | ||
@Authorizing | ||
} | ||
else if (IsAuthorized()) | ||
else if (isAuthorized) | ||
{ | ||
@((Authorized ?? ChildContent)?.Invoke(currentAuthenticationState)) | ||
} | ||
else | ||
{ | ||
@NotAuthorized | ||
@(NotAuthorized?.Invoke(currentAuthenticationState)) | ||
} | ||
|
||
@functions { | ||
private IAuthorizeData[] selfAsAuthorizeData; | ||
private AuthenticationState currentAuthenticationState; | ||
private bool isAuthorized; | ||
|
||
[CascadingParameter] private Task<AuthenticationState> AuthenticationState { get; set; } | ||
|
||
|
@@ -26,7 +32,7 @@ else | |
/// <summary> | ||
/// The content that will be displayed if the user is not authorized. | ||
/// </summary> | ||
[Parameter] public RenderFragment NotAuthorized { get; private set; } | ||
[Parameter] public RenderFragment<AuthenticationState> NotAuthorized { get; private set; } | ||
|
||
/// <summary> | ||
/// The content that will be displayed if the user is authorized. | ||
|
@@ -39,6 +45,29 @@ else | |
/// </summary> | ||
[Parameter] public RenderFragment Authorizing { get; private set; } | ||
|
||
/// <summary> | ||
/// The policy name that determines whether the content can be displayed. | ||
/// </summary> | ||
[Parameter] public string Policy { get; private set; } | ||
|
||
/// <summary> | ||
/// A comma delimited list of roles that are allowed to display the content. | ||
/// </summary> | ||
[Parameter] public string Roles { get; private set; } | ||
|
||
/// <summary> | ||
/// The resource to which access is being controlled. | ||
/// </summary> | ||
[Parameter] public object Resource { get; private set; } | ||
|
||
protected override void OnInit() | ||
{ | ||
selfAsAuthorizeData = new[] | ||
{ | ||
new AuthorizeDataAdapter((AuthorizeView)(object)this) | ||
}; | ||
} | ||
|
||
protected override async Task OnParametersSetAsync() | ||
{ | ||
// We allow 'ChildContent' for convenience in basic cases, and 'Authorized' for symmetry | ||
|
@@ -54,15 +83,17 @@ else | |
currentAuthenticationState = null; | ||
|
||
// Then render in completed state | ||
// Importantly, we *don't* call StateHasChanged between the following async steps, | ||
// otherwise we'd display an incorrect UI state while waiting for IsAuthorizedAsync | ||
currentAuthenticationState = await AuthenticationState; | ||
isAuthorized = await IsAuthorizedAsync(currentAuthenticationState.User); | ||
} | ||
|
||
private bool IsAuthorized() | ||
private async Task<bool> IsAuthorizedAsync(ClaimsPrincipal user) | ||
{ | ||
// TODO: Support various authorization condition parameters, equivalent to those offered | ||
// by the [Authorize] attribute, e.g., "Roles" and "Policy". This is on hold until we're | ||
// able to reference the policy evaluator APIs from this package. | ||
|
||
return currentAuthenticationState.User?.Identity?.IsAuthenticated == true; | ||
var policy = await AuthorizationPolicy.CombineAsync( | ||
AuthorizationPolicyProvider, selfAsAuthorizeData); | ||
var result = await AuthorizationService.AuthorizeAsync(user, Resource, policy); | ||
return result.Succeeded; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the same as what There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So just to confirm I'm reading this correctly, selfAsAuthorizeData is total set of authorize attributes for this view that you want to use for the authZ check. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes (except this info doesn’t come from attributes in this case- that’s a separate feature on the Blazor router). |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,7 @@ | |
</PropertyGroup> | ||
|
||
<ItemGroup> | ||
<Reference Include="Microsoft.AspNetCore.Authorization" /> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't have a better idea right now, but are we really OK with baking this in at the lowest level. We're saying all components present and future have access to the ASP.NET Authorization library There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm thinking about things like a React native experience, and I think the point is that you'd still want access to things like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, I agree with both the uneasiness and the conclusion that this is the best outcome currently. Considering that all the Components/Blazor packages have There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, this makes me a bit sad on the inside. I'm wondering if we could use some of the infrastructure for single file publish coming in 3.0 to separate the core of the platform from the baked-in components that we give you into two assemblies and simply ILMerge them at publish time. |
||
<Reference Include="Microsoft.JSInterop" /> | ||
<Reference Include="System.ComponentModel.Annotations" /> | ||
</ItemGroup> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you'll also have to support
ILoggerFactory
. You're going to find code in our stack that expects that, and it should be easy to add.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know it is fairly easy to add, but we don't have any code paths that hit it currently, and I'm trying to be minimal with this. We're always free to add more default services later.
If you have reasons to think it's going to be problematic if we don't, let me know, otherwise I'll leave this as-is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't really articulate a specific scenario. This just feels lazy - because we know what we would do if people complain. We could prevent them from having to log and issue and wait for us to fix it by just doing a little thing now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, we want to have a similar set of services in both environments, otherwise when I try to use a component that does logging, then I end up having to register it myself, making the experience different in the two environments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I've added a super-minimal
ILoggerFactory
.I'd be cautious about thinking about it like that. The environments have very different capabilities, goals, and characteristics. Most of the server-side services are for server-side application-server concerns, so a large proportion of them won't be present on WebAssembly by design.