Open jub0bs opened 13 hours ago
I feel like this belongs in x/oauth2. We shouldn't encourage ad-hoc auth handling.
Bearer token auth was introduced as part of oauth2, but it isn't strictly tied to oauth2.
The x/oauth2 package already provides the "set token" side in a way via https://pkg.go.dev/golang.org/x/oauth2#Config.Client, but in some cases a client needs more flexibility than that, for example when writing integration tests where one needs to be fully in control of which token is used.
And, particularly for the server side, there is no API in net/http nor x/oauth2 to extract a bearer token from a request.
you can control the token directly with a static token and https://pkg.go.dev/golang.org/x/oauth2#Token.SetAuthHeader
the proposal points to the oauth2 spec saying bearer is case insensitive, but that doesn't mean any other usage of bearer is similarly case insensitive. There's no reason why we can't have the API to extract the token in x/oauth2
I guess oauth2.Token.SetAuthHeader
is OK as a setter, although as a method hanging off of Token
it's not exactly as flexible as what is proposed here. But it does support different token types, which is nice.
We would still need the getter method, which IMO is what is most important about this proposal, given that it's non-trivial to get right. If we want to double down on the oauth2.Token
API I'd be fine for that func or method to live there as well. How about:
ParseAuthHeader(r *http.Request) (*Token, error)
I feel like I have seen bearer tokens in the wild that weren't OAuth tokens. Refusing to set a token that isn't base32 is surprising behavior. Moving it to x/oauth makes sense if we want to enforce that the tokens are valid OAuth tokens.
Proposal Details
I propose the addition of the following two methods:
Those methods parallel ones related to HTTP Basic Authentication already exported by net/http:
At first, you may think that the logic for getting/setting a Bearer token is so trivial that it doesn't deserve its own methods in the standard library. However, I've come to realise that many implementations out there suffer from correctness issues and/or performance issues.
strings.Split
, thereby facilitating denial-of-service attacks; see also this tangentially related issue (now resolved) in github.com/rs/cors.Moreover, and despite my lack of data to back up the following claim, I believe that Bearer is one of the most popular authentication scheme nowadays (likely even more popular than Basic), given the prominent role it plays in OAuth 2.x and OpenID Connect. Therefore, the logic required for parsing a request's Authorization header that uses Bearer arguably deserves to be enshrined in the standard library.
This playground contains a standalone implementation as well as a test suite. For convenience,
SetBearerAuth
andBearerAuth
are presented there as package-level functions rather than as*Request
methods.