search

aspnet-contrib / AspNet.Security.OAuth.Providers

OAuth 2.0 social authentication providers for ASP.NET Core
Apache License 2.0
2.38k stars 538 forks source link

Added WorldId provider #819

Closed sandrohanea closed 8 months ago

sandrohanea commented 9 months ago

Hello, Added a provider for World ID.

I hope I didn't miss anything as it's my first time adding a provider here.

sandrohanea commented 9 months ago

Some initial comments, but if this is a very new service provider and still in beta then I don't think we'd actually want to have to integrate and support this at this stage.

The provider is not in beta, but already have production applications and support and more than 2.5 million unique humans are already in the platform.

Indeed, those 2 properties (likely_human and credential_type) are still under beta object so I understand why you might not want them in the package as they might change soon, so I removed them + all references for those.

kevinchalet commented 9 months ago

Thanks for your PR, @sandrohanea!

To match the naming rules used by the .NET team (e.g Microsoft.AspNetCore.Authentication.OpenIdConnect), shouldn't we name this provider AspNet.Security.OAuth.WorldId (i.e without the capital D)?

sandrohanea commented 9 months ago

Thanks for your PR, @sandrohanea!

To match the naming rules used by the .NET team (e.g Microsoft.AspNetCore.Authentication.OpenIdConnect), shouldn't we name this provider AspNet.Security.OAuth.WorldId (i.e without the capital D)?

They are using the name World ID, but I understand that in this package, it would be better alligned if WorldId is used (without the capital D), so I renamed it.

kevinchalet commented 9 months ago

@sandrohanea did you try the provider? I tried adding an OpenIddict web integration for WorldID and all I get is a 500 response at the token request stage.

Apparently, WorldID seems to crash when you specify a redirect_uri parameter in the token request (yet, it's a mandatory parameter when you define a redirect_uri in the authorization request... and for OIDC, it's always required).

kevinchalet commented 9 months ago

2 other remarks:

sandrohanea commented 9 months ago

2 other remarks:

  • WorldID supports PKCE, we'll want to enable it by default in the aspnet-contrib provider.
  • WorldID doesn't flow the nonce parameter in the identity token, which is a serious protocol violation. We don't have id_token validation in place in any of the aspnet-contrib providers so it's not blocking here, but it sucks WorldID is one of those non-standard implementations...

I don't think it supports PKCE currently: https://docs.worldcoin.org/reference/sign-in#exchange-code That endpoint is just accepting basic auth and not accepting code_verifier or code_challenge, or am I missing something?

Regarding:

@sandrohanea did you try the provider? I tried adding an OpenIddict web integration for WorldID and all I get is a 500 response at the token request stage.

Apparently, WorldID seems to crash when you specify a redirect_uri parameter in the token request (yet, it's a mandatory parameter when you define a redirect_uri in the authorization request... and for OIDC, it's always required).

Yes, I tried the provider with both staging app and production app, worked correctly for this OAuth2 approach.

kevinchalet commented 9 months ago

I don't think it supports PKCE currently: https://docs.worldcoin.org/reference/sign-in#exchange-code That endpoint is just accepting basic auth and not accepting code_verifier or code_challenge, or am I missing something?

It's not documented but the OIDC discovery document (https://id.worldcoin.org/.well-known/openid-configuration) lists S256 as a supported code_challenge_method, which means PKCE is supported:

"code_challenge_methods_supported":["S256"]

Yes, I tried the provider with both staging app and production app, worked correctly for this OAuth2 approach.

Weird. I'll give it another try: it's not normal to get a 500 error when the redirect_uri is sent.

BTW, how do you authenticate when using a staging app? Do you need a special iOS/Android app or is there a setting somewhere?

kevinchalet commented 9 months ago

I was curious so I tried your provider and I get exactly the same error without changing anything to the code:

HTTP/1.1 500 Internal Server Error
Date: Thu, 21 Dec 2023 16:18:12 GMT
Content-Length: 0
Connection: keep-alive
Cache-Control: public, max-age=0, must-revalidate
Content-Security-Policy: default-src 'self'; script-src 'self' 'nonce-7f4aa660-c53d-4374-80d7-c69df8393d88' 'strict-dynamic'; font-src 'self' https://world-id-public.s3.amazonaws.com; style-src 'self' 'unsafe-inline' fonts.googleapis.com; connect-src 'self' https://app.posthog.com https://docs.worldcoin.org https://status.worldcoin.org https://developer.worldcoin.org https://rum.browser-intake-datadoghq.com https://bridge.worldcoin.org; img-src 'self' https://worldcoin.org https://world-id-public.s3.amazonaws.com
Strict-Transport-Security: max-age=63072000
Vary: RSC, Next-Router-State-Tree, Next-Router-Prefetch, Next-Url
X-Matched-Path: /oidc-route
X-Vercel-Cache: MISS
X-Vercel-Execution-Region: iad1
X-Vercel-Id: cdg1::iad1::9v64t-1703175491891-796b4fd8448b
CF-Cache-Status: DYNAMIC
Server: cloudflare
CF-RAY: 839170882c1f7028-CDG

Which manifests as a JSON exception with your provider since the body is empty:

Microsoft.AspNetCore.Authentication.AuthenticationFailureException: An error was encountered while handling the remote login.
 ---> System.Text.Json.JsonReaderException: The input does not contain any JSON tokens. Expected the input to start with a valid JSON token, when isFinalBlock is true. LineNumber: 0 | BytePositionInLine: 0.
   at System.Text.Json.ThrowHelper.ThrowJsonReaderException(Utf8JsonReader& json, ExceptionResource resource, Byte nextByte, ReadOnlySpan`1 bytes)
   at System.Text.Json.Utf8JsonReader.Read()
   at System.Text.Json.JsonDocument.Parse(ReadOnlySpan`1 utf8JsonSpan, JsonReaderOptions readerOptions, MetadataDb& database, StackRowStack& stack)
   at System.Text.Json.JsonDocument.Parse(ReadOnlyMemory`1 utf8Json, JsonReaderOptions readerOptions, Byte[] extraRentedArrayPoolBytes, PooledByteBufferWriter extraPooledByteBufferWriter)
   at System.Text.Json.JsonDocument.Parse(ReadOnlyMemory`1 json, JsonDocumentOptions options)
   at System.Text.Json.JsonDocument.Parse(String json, JsonDocumentOptions options)
   at Microsoft.AspNetCore.Authentication.OAuth.OAuthHandler`1.PrepareFailedOAuthTokenReponse(HttpResponseMessage response, String body)
   at Microsoft.AspNetCore.Authentication.OAuth.OAuthHandler`1.ExchangeCodeAsync(OAuthCodeExchangeContext context)
   at Microsoft.AspNetCore.Authentication.OAuth.OAuthHandler`1.HandleRemoteAuthenticateAsync()
   at Microsoft.AspNetCore.Authentication.RemoteAuthenticationHandler`1.HandleRequestAsync()
   --- End of inner exception stack trace ---
   at Microsoft.AspNetCore.Authentication.RemoteAuthenticationHandler`1.HandleRequestAsync()
   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
   at Microsoft.WebTools.BrowserLink.Net.BrowserLinkMiddleware.InvokeAsync(HttpContext context)
   at Microsoft.AspNetCore.Watch.BrowserRefresh.BrowserRefreshMiddleware.InvokeAsync(HttpContext context)
   at Microsoft.AspNetCore.Server.IIS.Core.IISHttpContextOfT`1.ProcessRequestAsync()

If I replay the same request without the redirect_uri parameter attached, I get a normal response so there's definitely something very funky with this service.

@martincostello I suggest we wait to merge this provider, at least until Worldcoin fixes this issue on their end.

kevinchalet commented 9 months ago

After re-reading the documentation, it seems they just don't support the standard redirect_uri parameter in grant_type=authorization_code token requests. That said, it doesn't explain why it didn't work for me - I just tried again and I'm getting the same result - but worked for you 🤣

If we want to merge this provider, we'll need to override the token request phase to avoid sending the redirect_uri parameter that causes 500 errors.

kevinchalet commented 8 months ago

Doing some housecleaning but feel free to reopen if you're still interested in working on this provider.