Fix AuthorizeViewCore.cs invalid rendering logic

[juho-hanhimaki]

Component render would occur between lines 95 and 96 if the AuthenticationState task had already been completed before it was awaited on line 95.

aspnetcore/src/Components/Authorization/src/AuthorizeViewCore.cs

Lines 73 to 97 in d70439c

	protected override async Task OnParametersSetAsync()
	{
	// We allow 'ChildContent' for convenience in basic cases, and 'Authorized' for symmetry
	// with 'NotAuthorized' in other cases. Besides naming, they are equivalent. To avoid
	// confusion, explicitly prevent the case where both are supplied.
	if (ChildContent != null && Authorized != null)
	{
	throw new InvalidOperationException($"Do not specify both '{nameof(Authorized)}' and '{nameof(ChildContent)}'.");
	}
	if (AuthenticationState == null)
	{
	throw new InvalidOperationException($"Authorization requires a cascading parameter of type Task<{nameof(AuthenticationState)}>. Consider using {typeof(CascadingAuthenticationState).Name} to supply this.");
	}
	// First render in pending state
	// If the task has already completed, this render will be skipped
	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);
	}

That leads to rendering of NotAuthorized state in BuildRenderTree function which is unexpected behaviour (currentAuthenticationState is set but authorization is still in progress). NotAuthorized state should only ever be rendered after IsAuthorizedAsync has been completed.

aspnetcore/src/Components/Authorization/src/AuthorizeViewCore.cs

Lines 53 to 70 in d70439c

	protected override void BuildRenderTree(RenderTreeBuilder builder)
	{
	// We're using the same sequence number for each of the content items here
	// so that we can update existing instances if they are the same shape
	if (currentAuthenticationState == null)
	{
	builder.AddContent(0, Authorizing);
	}
	else if (isAuthorized)
	{
	var authorized = Authorized ?? ChildContent;
	builder.AddContent(0, authorized?.Invoke(currentAuthenticationState));
	}
	else
	{
	builder.AddContent(0, NotAuthorized?.Invoke(currentAuthenticationState));
	}
	}

If the AuthenticationState had not been completed before it was awaited the rendering would have occurred before line 95 and no render would occur between lines 95 and 96. In that case the AuthorizeViewCore works as expected.

In this fix the isAuthorized field is made nullable and it is used to determine the state to display in the BuildRenderTree function. Until the authorization has completed the isAuthorized is null the Authorizing state is displayed as expected.

Addresses #24381

[HaoK]

Thanks for the PR @juho-hanhimaki do you think you could add a test to https://github.com/dotnet/aspnetcore/blob/d70439cc82253ac32cc3eaf94ce965bab1ac9d37/src/Components/Authorization/test/AuthorizeViewTest.cs to verify the new behavior?

[juho-hanhimaki]

@HaoK I'll take a look at adding the test.

Took some effort to get the thing building locally.

[dnfadmin]

All CLA requirements met.

[juho-hanhimaki]

I added the test that covers the PR/issue.

Because the TestAuthorizationService that is used in other tests completes the authorization synchronously I had to make async version of it. Otherwise authorization would just complete synchronously and component would just render the Authorized state skipping the Authorizing state completely.

[HaoK]

Awesome that test looks great, on last small thing, can you confirm locally that without your changes your test fails

[juho-hanhimaki]

@HaoK I did, and the test fails as it should without the changes to AuthorizeViewCore.cs.

[juho-hanhimaki]

[HaoK]

Thanks for doing this work @juho-hanhimaki !

[juho-hanhimaki]

Thank you too for taking the PR!

[ghost]

Hi @juho-hanhimaki. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.