Resource Server Package Name Inconsistencies

[jzheaux]

Some of the classes in oauth2-resource-server could be better aligned with the whole.

For example, if XYZ represents the root package for that module, then:

• authentication filters tend to be in XYZ.web.authentication, but BearerTokenAuthenticationFilter is only in XYZ.web
• authentication tokens tend to be in XYZ.authentication, but BearerTokenAuthenticationToken is in XYZ

Since these have both GA'd already, they likely won't be adjusted on the 5.x release train, but I'm recording this for future reference.

[jgrandja]

@jzheaux

BearerTokenAuthenticationFilter is in ...web sub-package, which is correct. I don't believe it should be moved to ...web.authentication?

[jzheaux]

@jgrandja, I see that AnonymousAuthenticationFilter, DefaultLoginPageGeneratingFilter, DefaultLogoutPageGeneratingFilter, and UsernamePasswordAuthenticationFilter are all in web.authentication.

BasicAuthenticationFilter is in web.authentication.www, AbstractPreAuthenticatedProcessingFilter is in web.authentication.preauth, and X509AuthenticationFilter is in web.authentication.preauth.x509.

By these it seems to me that authentication filters go into web.authentication or a subpackage of that, which is why I'm thinking BearerTokenAuthenticationFilter should go into web.authentication or a subpackage.

What am I missing regarding the packaging conventions?

[jgrandja]

@jzheaux Thanks for providing references to existing authn Filter's. It looks like there is some inconsistency since OpenIDAuthenticationFilter, CasAuthenticationFilter and OAuth2LoginAuthenticationFilter are not in *.web.authentication package. Maybe @rwinch can confirm which sub-package it should be in?

[rwinch]

I'd have to give this some thought. At first glance I'd say that they should be in web.authentication. I'm trying to remember if there were any conscious decisions to make the packages less sparse by avoiding an authentication package.

It could be argued that cas in itself implies authentication, so perhaps that is reason enough to avoid another authentication package.