Am 2019-12-28 um 17:17 schrieb Gary Gregory:
On Sat, Dec 28, 2019 at 7:40 AM Michael Osipov <[email protected]>
wrote:
Folks,
this has been bugging me for some time lately and I'd like to know your
opinion about it. I am also convinced that this may confuse some of your
client users:
We have:
* interface AuthScheme with String #getName()
* enum AuthSchemes which should be singular and maybe StandardAuthScheme
with the auth schemes we support out of the box. Or similar to
StandardCharsets have "final class StandardAuthSchemes" which has only
"final static Sting SYMBOLIC_NAME = "AuthSchemeName".
For me the enum implies that this is an exhaustive list of schemes we
support, nothing else more possible.
Thanks for bringing all of this up Michael :-)
It feels like there is a disconnect indeed and that the code is not
complete or at least missing Javadoc:
- org.apache.hc.client5.http.impl.auth.SystemDefaultCredentialsProvider
uses hard coded strings instead of AuthSchemes id strings.
- All references to AuthSchemes use the AuthSchemes.id String instead of
an
AuthSchemes itself (writing about this really shows the plural name as
lame.) Always using the id field seems to defeat the purpose of having an
enum in the first place.
Exactly!
Do we intend for user pluggable AuthSchemes id String? It's confusing:
-
org.apache.hc.client5.http.impl.DefaultAuthenticationStrategy.DEFAULT_SCHEME_PRIORITY
is a List<String> instead of a List<AuthSchemes>
- This List is used with:
-
org.apache.hc.client5.http.config.RequestConfig.getProxyPreferredAuthSchemes()
returns List<String>
-
org.apache.hc.client5.http.config.RequestConfig.getTargetPreferredAuthSchemes()
returns List<String>
That tells me I can provide whatever I want in RequestConfig in these two
scheme lists. Is that the intent or do these list must contain only valid
AuthSchemes ids? The Javadoc say nothing about the supported value range
(RequestConfig.Builder.setTargetPreferredAuthSchemes(Collection<String>))
Yes, the usage of strings implies that you can put in whatever you want.
This makes sense because of two reasons:
* You can swap your default impls against your own
* You can add you own auth schemes which operate over WWW-Authenticate,
e.g., Bearer, HOBA, Mutual [1].
We ran into a similar-ish issue over at Log4j 2 with levels (INFO, WARN,
and so on) and I've had to deal with the same pattern elsewhere, so TL;DR
is if you want to define a type with known instances AND allow for custom
instances you cannot use an enum, you have to use a class and/or
interface.
I am not sure of the intent here.
Absolutely correct!
The fact that we have both an enum and an interface with effectively the
same name screams out for help ;-) java.nio.charset.StandardCharsets is a
good compromise and we could follow suit but let's address the above
first
IMO.
* class AuthScope with String #getAuthScheme(). Logically, I would
expect here to be returned an instance of AuthScheme and not a string.
Otherise it should be "String #getSchemeName()"
Indeed, if we have an enum, let's use it, otherwise, do we need it at
all?
If not we can go the java.nio.charset.StandardCharsets route. Maybe.
Tangent:
- org.apache.hc.client5.http.auth.AuthSchemeProvider is Javadoc'd as a
factory class so why no call it AuthSchemeFactory? I think of providers
as
larger scale components (like a security provider) where a factory is
smaller scale and creates instances. In this case, you cannot create
an AuthScheme since it is an enum, but no, wait, this factory is typed to
the interface so it does indeed create instances... we need to fix the
names of these types...
I share this. For me a provider holds a collection a factory
implementations for names entities.
Please create a JIRA issue to track this.
I have a few ideas, I will provide a PR soon for this.
Michael
* HTTPCLIENT-2041, HTTPCLIENT-2042
* class AuthChallenge with "String #getScheme()" same with AuthScope
* class AuthExchange with "AuthSheme #getAuthScheme"
* RequestConfig "Collection<String
#(target|proxy)PreferredAuthSchemes()" same with AuthScope
and maybe other spots I have missed.
CookieSpec and friends have the same issue.
Am I just paranoid or do you consider them to be inconsistent as well?
Michael
[1]
https://developer.mozilla.org/en-US/docs/Web/HTTP/Authentication#Basic_authentication_scheme
