Consider allowing more OpenIddict endpoints to be enabled via the UI and via recipes

[kevinchalet]

As a basic authorization server, the OpenID module doesn't use all the features/endpoints offered by OpenIddict, like introspection and revocation. As mentioned in openiddict/openiddict-core#1359 (comment), these features can be configured by amending the OpenIddict settings via a custom plugin, but a simpler story - GUI or recipes-based - may be preferable.

Additional context: openiddict/openiddict-core#1359 (comment)

[kevinchalet]

@mcalasa said he was interested in sending a PR to add introspection/revocation support: openiddict/openiddict-core#1359 (comment) ❤️

[mcalasa]

@kevinchalet I've added Allow Introspection and Revocation Endpoint check boxes to the OpenId Application Settings page and that is saving the settings in the database correctly as I check and uncheck it then save.

I've also added Enable Introspection and Revocation Endpoint check boxes to the OpenId Server settings page and that is saving the settings in the database correctly as I check and uncheck it then save as well.

Now for the server settings I'm not too sure how to actually plug-in the Introspection and Revocation server options in OC and will need your guidance there. I'm guessing I may have to submit a PR in Openiddict to modify OpenIddictServerAspNetCoreOptions, OpenIddictClientOwinOptions and any other files related to it to include those options in there? I'm basing this by looking at how enabling the user info endpoint was implemented.
Let me know your thoughts on this.

Thank you

[kevinchalet]

Now for the server settings I'm not too sure how to actually plug-in the Introspection and Revocation server options in OC and will need your guidance there. I'm guessing I may have to submit a PR in Openiddict to modify OpenIddictServerAspNetCoreOptions, OpenIddictClientOwinOptions and any other files related to it to include those options in there?

Both the ASP.NET Core and OWIN hosts already fully support extracting introspection and revocation requests (and handling such responses) so no change at all should be required in OpenIddict itself.

To enable the endpoints, you'll need to add at least one Uri instance in the OpenIddictServerOptions.IntrospectionEndpointUris and OpenIddictServerOptions.RevocationEndpointUris collection exactly like how it's already done for the other endpoints:

OrchardCore/src/OrchardCore.Modules/OrchardCore.OpenId/Configuration/OpenIdServerConfiguration.cs

Lines 78 to 93 in 8465fbb

	if (settings.AuthorizationEndpointPath.HasValue)
	{
	options.AuthorizationEndpointUris.Add(new Uri(settings.AuthorizationEndpointPath.Value, UriKind.Relative));
	}
	if (settings.LogoutEndpointPath.HasValue)
	{
	options.LogoutEndpointUris.Add(new Uri(settings.LogoutEndpointPath.Value, UriKind.Relative));
	}
	if (settings.TokenEndpointPath.HasValue)
	{
	options.TokenEndpointUris.Add(new Uri(settings.TokenEndpointPath.Value, UriKind.Relative));
	}
	if (settings.UserinfoEndpointPath.HasValue)
	{
	options.UserinfoEndpointUris.Add(new Uri(settings.UserinfoEndpointPath.Value, UriKind.Relative));
	}

Let me know if you need additional guidance 😃

[mcalasa]

Oh man, can't believe I missed that LOL. You the man @kevinchalet
Easy enough change to make and will test this out.

Thank you!

[kevinchalet]

Haha. Let us know how it goes 😄

[mcalasa]

@kevinchalet As I was about to test the Authorization Code Flow I noticed that PKCE was not an option. I saw this issue #7138 (comment) and told myself why not and proceeded to add PKCE to the Server and Application settings pages. I did confirm that it does save the requirement settings correctly in the database. I will be testing and verifying to make sure everything works correctly as well.

Server settings page

Application settings page

[kevinchalet]

Nice!

Should we add a warning message telling users they should ensure their client/client lib supports PKCE before enabling this option?

For the global option, we should at least mention that it will apply to all clients, whether the "require PKCE" flag was set or not on the individual client registrations.

[mcalasa]

Should that warning message be like a dialog that pops up stating that their client lib should support PKCE? Or do you think a label by the check box should state that?

For the global option I'll add a label next to the check box unless you have a better option.