AspNet.Security.OAuth.Providers icon indicating copy to clipboard operation
AspNet.Security.OAuth.Providers copied to clipboard

Denying Consent for PayPal Provider Appears to Invoke OnRemoteFailure Event Rather Than OnAccessDenied Event

Open Mike-E-angelo opened this issue 2 years ago • 9 comments

Describe the bug

I am testing out my Oauth2 providers for my application. Microsoft, Google, Reddit, and Twitter all have workflows that gracefully return to the home page and/or invoke the RemoteAuthenticationOptions.Events.OnAccessDenied event (which I have configured to simply end response and return to home page).

However, when using PayPal, the result is a 500 error. The error thrown is 'RemoteAuthenticationFailed error in my logs. Additionally, the URL that is in the browser is the following:

https://<site>/signin-paypal?error_uri=https%3A%2F%2F<site>%2Fsignin-paypal&error_description=Consent%20denied&error=access_denied

It appears that access_denied is there, but the OnRemoteFailure event is being called instead for some reason.

Steps To reproduce

  1. Log into a PayPal-enabled Oauth2 site
  2. On the consent screen, click one of the two links here:
  3. This should produce the error

Expected behaviour

If the user does not consent, it should invoke the OnAccessDenied event like other providers do.

Actual behaviour

OnRemoteFailure is called. I see two errors in my logs.

One is from AspNet.Security.OAuth.Paypal.PaypalAuthenticationHandler: {Id: 4, Name: 'RemoteAuthenticationFailed'}

The other is Microsoft.AspNetCore.Server.IIS.Core.IISHttpServer and has a stack trace:

System.Exception: An error was encountered while handling the remote login.
 ---> System.Exception: The oauth state was missing or invalid.
   --- End of inner exception stack trace ---
   at async Task<bool> Microsoft.AspNetCore.Authentication.RemoteAuthenticationHandler<TOptions>.HandleRequestAsync()
   at async Task Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
   at async Task<bool> Microsoft.AspNetCore.Server.IIS.Core.IISHttpContextOfT<TContext>.ProcessRequestAsync()

System information

  • Azure App Service
  • 6.0.6
.NET SDK (reflecting any global.json):
 Version:   6.0.201
 Commit:    ef40e6aa06

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.14393
 OS Platform: Windows
 RID:         win10-x86
 Base Path:   C:\Program Files (x86)\dotnet\sdk\6.0.201\

Host (useful for support):
  Version: 6.0.3
  Commit:  c24d9a9c91

.NET SDKs installed:
  1.1.14 [C:\Program Files (x86)\dotnet\sdk]
  2.1.526 [C:\Program Files (x86)\dotnet\sdk]
  2.2.109 [C:\Program Files (x86)\dotnet\sdk]
  3.1.118 [C:\Program Files (x86)\dotnet\sdk]
  3.1.417 [C:\Program Files (x86)\dotnet\sdk]
  5.0.405 [C:\Program Files (x86)\dotnet\sdk]
  5.0.406 [C:\Program Files (x86)\dotnet\sdk]
  6.0.102 [C:\Program Files (x86)\dotnet\sdk]
  6.0.201 [C:\Program Files (x86)\dotnet\sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.All 2.1.30 [C:\Program Files (x86)\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.2.14 [C:\Program Files (x86)\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.App 2.1.30 [C:\Program Files (x86)\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.2.14 [C:\Program Files (x86)\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.0.3 [C:\Program Files (x86)\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.18 [C:\Program Files (x86)\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.23 [C:\Program Files (x86)\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 5.0.14 [C:\Program Files (x86)\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 5.0.15 [C:\Program Files (x86)\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 6.0.2 [C:\Program Files (x86)\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 6.0.3 [C:\Program Files (x86)\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 1.0.16 [C:\Program Files (x86)\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 1.1.13 [C:\Program Files (x86)\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.0.9 [C:\Program Files (x86)\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.30 [C:\Program Files (x86)\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.2.14 [C:\Program Files (x86)\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.0.3 [C:\Program Files (x86)\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.18 [C:\Program Files (x86)\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.23 [C:\Program Files (x86)\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 5.0.14 [C:\Program Files (x86)\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 5.0.15 [C:\Program Files (x86)\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 6.0.2 [C:\Program Files (x86)\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 6.0.3 [C:\Program Files (x86)\dotnet\shared\Microsoft.NETCore.App]

To install additional .NET runtimes or SDKs:
  https://aka.ms/dotnet-download

Additional context

Everything else works great :) Thank you for all your efforts out there. 🙏

Mike-E-angelo avatar Jun 08 '22 16:06 Mike-E-angelo

Have you tried to set AccessDeniedPath to a non-empty value in the options?

kevinchalet avatar Jun 08 '22 16:06 kevinchalet

It's possible that PayPal uses some non-standard error code for this flow.

We've had this problem before with LinkedIn, see #480.

If the error code isn't handled by the base OAuth handler in the dotnet/aspnetcore repo, then it would need custom code to handle it. Alternatively, you could write your own event handler for OnRemoteFailure to deal with it appropriately (this is what I've done in some of my own code to handle non-standard stuff, in my use case it was Microsoft Account provider which returned consent_required).

martincostello avatar Jun 08 '22 16:06 martincostello

Have you tried to set AccessDeniedPath to a non-empty value in the options?

I have not, @kevinchalet. I have been making use of the OnAccessDenied event to cause a redirect. Am I correct in understanding that this is basically a simpler way of doing that? If so, I have some changes to make!

Alternatively, you could write your own event handler for OnRemoteFailure

Yeah @martincostello that's what I am looking into now. Basically looks like I will be checking for error=access_denied?

Mike-E-angelo avatar Jun 08 '22 16:06 Mike-E-angelo

I have not, @kevinchalet. I have been making use of the OnAccessDenied event to cause a redirect. Am I correct in understanding that this is basically a simpler way of doing that? If so, I have some changes to make!

Yeah, basically. It should handle access_denied without any modification as it's a standard error code.

kevinchalet avatar Jun 08 '22 16:06 kevinchalet

Yeah, basically. It should handle access_denied without any modification as it's a standard error code.

So this works exactly the same with the providers I was using with OnAccessDenied with and have removed that event. The PayPal provider still seems to call OnRemoteFailure even though error=access_denied is in the URL. The OnAccessDenied event is not called (I checked before removing it from the others :)). This is what I am having to assign to the OnRemoteFailure event ATM and works as a workaround for now:

sealed class RemoteFailure : IAllocated<RemoteFailureContext>
{
	public static RemoteFailure Default { get; } = new();

	RemoteFailure() : this("/") {}

	readonly string _return;

	public RemoteFailure(string @return) => _return = @return;

	public Task Get(RemoteFailureContext parameter)
	{
		switch (parameter.Scheme.Name)
		{
			case PaypalAuthenticationDefaults.AuthenticationScheme:
				if (parameter.Request.Query.TryGetValue("error", out var error))
				{
					switch (error)
					{
						case "access_denied":
							parameter.Response.Redirect(_return);
							parameter.HandleResponse();
							break;
					}
				}
				break;
		}

		return Task.CompletedTask;
	}
}

Mike-E-angelo avatar Jun 08 '22 16:06 Mike-E-angelo

The code for access_denied is here. Before that it checks the OAuth state, and throws an exception if it's missing.

If you have The oauth state was missing or invalid. in your logs, that suggests an issue with the state parameter not being set back from the provider.

martincostello avatar Jun 08 '22 16:06 martincostello

If you have The oauth state was missing or invalid. in your logs, that suggests an issue with the state parameter not being set back from the provider.

Great. Another one. 😅😭

I've simplified the steps to reproduce this: you don't need to actually use the PayPal workflow, but use the following URL (mentioned in the original post) on your local server with the PayPal provider installed:

https://<site>/signin-paypal?error_uri=https%3A%2F%2F<site>%2Fsignin-paypal&error_description=Consent%20denied&error=access_denied

You should get the same result. So it would seem you are correct and PayPal is not sending back the correct information to the caller. :(

Mike-E-angelo avatar Jun 08 '22 17:06 Mike-E-angelo

For this issue, from my side, I am set. I have my own workaround/configuration per above. If this is a matter to contact PayPal to ensure they are passing correct variables, I will not be volunteering for that. In addition to the grief that we've been experiencing with #572, I have not had too much luck with PayPal support. If you are up for contacting PayPal to get this resolved, we can keep this issue open. Otherwise, I am OK with closing it. ✌

Mike-E-angelo avatar Jun 20 '22 08:06 Mike-E-angelo

Paypal now supports OpenID Connect discovery/OAuth 2.0 authorization server metadata (https://www.paypal.com/.well-known/openid-configuration) and the authorization endpoint they return as part of the discovery document differs from the one we currently use in the aspnet-contrib provider. It may be worth giving a try with the newer endpoint to see if it makes any difference.

kevinchalet avatar Jun 20 '22 15:06 kevinchalet

It's confirmed: the OIDC implementation has a bunch of compliance issues, but it supports returning the state parameter even for errored requests. I was only able to test it against the sandbox environment, but it's likely the production endpoints behave the same way.

https://github.com/openiddict/openiddict-core/pull/1537

kevinchalet avatar Oct 15 '22 14:10 kevinchalet

Adding the help wanted label in case someone would be interested in updating the PayPal provider to use the newer endpoints.

kevinchalet avatar Oct 15 '22 14:10 kevinchalet

Looks like no one is interested. Closing 😄

kevinchalet avatar Jan 19 '24 11:01 kevinchalet