wwwlicious / servicestack-authentication-identityserver

A plugin for ServiceStack and IdentityServer that provides OpenIDConnect / OAuth 2.0 Single Sign-On Authentication
Other
29 stars 15 forks source link

CallbackUrl throws ArgumentNullException #12

Open stackedbitz opened 6 years ago

stackedbitz commented 6 years ago

When running the ServiceAuthProvider sample, the API throws an ArgumentNullException when a secure service request from the client tries to authenticate:

serviceClient.Post(new Authenticate { provider = IdentityServerAuthProvider.Name });

The null value is being thrown by the referrerUrl variable in the CallbackUrl property get() method:

get { return appSettings.Get(ConfigKeys.CallbackUrl, referrerUrl.AppendUrlPaths("auth", "IdentityServer")); }

I notice the ServiceAuthProvider is excluded from the list of providers that require callback URL validation in the ValidateCallbackRequirements method which would appear to set the referrerUrl. Do service auth providers not require callback validation? If so, then why would the CallbackUrl need to be accessed as observed in the call stack below (just before the exception is thrown)?

ServiceStack.Authentication.IdentityServer.IdentityServerAuthFeature.CallbackUrl.get() Line 225 C# ServiceStack.Authentication.IdentityServer.Providers.IdentityServerAuthProvider.RefreshSettings() Line 166 C# ServiceStack.Authentication.IdentityServer.Providers.ServiceAuthProvider.Authenticate(ServiceStack.IServiceBase authService, ServiceStack.Auth.IAuthSession session, ServiceStack.Authenticate request) Line 30 C#

Any help would be great, thanks!

richardbartley commented 5 years ago

Trying to authenticate with a clientid and secret key, do I set the AuthProviderType ot ServiceProvider? This is giving me the ArgumentNullException (I think on the callback Url which is not needed for this???).

this.Plugins.Add(new IdentityServerAuthFeature { AuthProviderType = IdentityServerAuthProviderType.ServiceProvider, AuthRealm = serviceUrl, // The URL of the IdentityServer instance ClientId = "client", // The Client Identifier so that IdentityServer can identify the service ClientSecret = "secret", // The Client Secret so that IdentityServer can authorize the service Scopes = "apiMB offline_access" // See {{urlIdvSvr}}/.well-known/openid-configuration for available scopes });

stackedbitz commented 5 years ago

Cause If you're using AuthProviderType of either ServiceAuthProvider and ImpersonationAuthProvider, then referrerUrl is never set in ValidateCallbackRequirements(IAppHost appHost). This is problematic since RefreshSettings() tries to update CallbackUrl regardless of AuthProviderType, resulting in an ArgumentNullException.

Solution Either CallbackUrl should be ignored in RefreshSettings() for ServiceAuthProvider and ImpersonationAuthProvider

OR

referrerUrl should just return null when attempting to get the CallbackUrl like so:

Before: referrerUrl.AppendUrlPaths("auth", "IdentityServer");

After: referrerUrl?.AppendUrlPaths("auth", "IdentityServer");

Personally, the latter looks cleaner. Happy to submit a PR if this fix is approved.

stackedbitz commented 5 years ago

Better yet, why not just remove the line:

if (!providersThatShouldValidateCallbackUrl.Contains(providerType)) return;

This way the CallbackUrl is always valid, whether or not it's used.