Implement OAuth 2.0 Server Metadata (RFC 8414)

[Kehrlann]

See https://tools.ietf.org/html/rfc8414

Closes #54

[Kehrlann]

This is still a draft. Remaining todos are:

• Make the default values common for OIDC Provider Config + OAuth 2.0 Server Metadata
• Integration tests
• Cleanup TODOs
• General polishing
• Make sure test coverage is appropriate
• A pass on Javadoc
• Ensuring consistency with ab591d (polish commit for OIDC Provider Configuration)

[jgrandja]

Thanks for the PR @Kehrlann ! Please see review comments.

In addition to the comments, please ensure copyright year is 2020-2021 and @since is 0.1.1 for all files.

[jgrandja]

I feel like we should merge this into OAuth2AuthorizationServerMetadataClaimAccessor. Then we can remove AuthorizationServerMetadataClaimAccessor, AuthorizationServerMetadataClaimNames and AbstractAuthorizationServerMetadata. This change will simplify the hierarchy.

Although revocation_endpoint, revocation_endpoint_auth_methods_supported and code_challenge_methods_supported are not defined in OIDC Provider Metadata, I believe an OIDC provider will support these configurations either way.

What are your thoughts on this proposed change?

[Kehrlann]

👍 definitely agree with the proposed change.

Do we also the OidcProdiverConfigurationHttpMessageConverter to be a subclass of the OAuth2AuthorizationServerConfigurationHttpMessageConverter ?

Quick research notes:

• Okta supports those three claims in their OIDC configuration
• Google supports revocation_endpoint and code_challenge_methods_supported
• Microsoft supports none of them

[Kehrlann]

One caveat, removing AbstractAuthorizationServerMetadata, using OAuth2AuthorizationServerConfiguration instead in OidcProviderConfiguration extends OAuth2AuthorizationServerConfiguration

Is only possible if we make OAuth2AuthorizationServerConfiguration not final. And I believe we want our model classes to be final?

[jgrandja]

Our typical design process is to make things final (or restrict access) at the start. However, if there is need to open things up than we do only if it makes sense. This is one of those instances so it's ok to make OAuth2AuthorizationServerConfiguration non-final BUT make the constructor protected.

[Kehrlann]

So I've tried this, unfortunately the type hierarchy with generic builders does not allow to have this kind of hierarchy:

- OAuth2AuthorizationServerConfiguration
  - Builder (inner class)
  - static builder() method, returning a Builder
  - OidcProviderConfiguration extends OAuth2AuthorizationServerConfiguration
    - OidcBuilder (inner class, extends Builder) 
    - static builder() method, returning an OidcBuilder

Problems are:

• If OidcBuilder extends the concrete class OAuth2AuthorizationServerConfiguration.Builder, the builder pattern won't work as, for example, builder.issuer("some string") would always return the parent Builder type. I don't think we can get away without a base abstract builder, see this Stackoverflow thread for reference for reference.
• If both Builder and OidcBuilder inherit a common, generic, AbstractBuilder, then OidcProviderConfiguration.builder() return type becomes incompatible with that of OAuth2AuthorizationServerConfiguration.builder().

If we really want to get rid of the AbstractAuthorizationServerConfiguration, we could have the abstract builder and default claims in the AuthorizationServerMetadataClaimAccessor interface. But that feels a bit far-fetched, this is an Accessor interface, sounds very read-only to me.

For now I have just pulled all behavior from OAuth2AuthorizationServerConfiguration and associated builder into AbstractAuthorizationServerConfiguration.

[jgrandja]

Thanks for all the work @Kehrlann ! This is now merged.

I decided to rename OAuth2AuthorizationServerConfiguration back to the original OAuth2AuthorizationServerMetadata. It made more sense.

I also made a few other changes in a follow-up polish commit. Please let me know if you have any questions in regards to the changes. Thanks!