Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support passing tokens with HTTP Basic Auth for API and WebSocket #4330

Closed
wants to merge 2 commits into from

Conversation

chdorner
Copy link
Contributor

@robertknight mentioned that the WebSocket JavaScript API does not support passing any custom headers to the request. The way we initially thought we would authenticate websocket connections is the same way as we do API requests, with a Authorization: Bearer {token} header.

What does work with the WebSocket JavaScript API is passing the HTTP basic auth username and password in the URL (wss://username:password@hypothes.is/ws), which lead us to implement another way of authenticating requests by passing the token as the HTTP basic auth password.
Note: Some implementations (Firefox, curl) do not support an empty username, in this case the client should just send a dummy string in the username (e.g. wss://X:{token}@hypothes.is/ws).

We first check for the access token in the `Authorization: Bearer
{token}` header, if we can't find it there we fall back and check if it
has been supplied as the HTTP basic auth password. We ignore the
username, but it needs to be set in some environments, especially when
the username and password is part of the url
(ie. `https://:password@host.com` doesn't seem to work in Firefox, or
curl).
The audience check was a protection to ensure that we don't use a token
destined for someone else, but we only ever generate these JWT tokens
for ourselves. So there is no need for checking the audience anymore.
This should not compromise security, because as long as the server-side
client secret is not compromised, there is no way to tamper with the JWT
token.

This will allow us to use the legacy JWT tokens for authenticating
websocket connections when the web service and the websocket service run
on different hosts (in localhost we have: `http://localhost:5000` for
the web service, and `http://localhost:5001` for the websocket service).
@chdorner chdorner requested a review from nickstenning January 26, 2017 15:33
@codecov-io
Copy link

Current coverage is 93.84% (diff: 100%)

Merging #4330 into master will increase coverage by 0.01%

@@             master      #4330   diff @@
==========================================
  Files           311        311          
  Lines         17110      17142    +32   
  Methods           0          0          
  Messages          0          0          
  Branches       1019       1021     +2   
==========================================
+ Hits          16055      16087    +32   
  Misses          950        950          
  Partials        105        105          

Powered by Codecov. Last update 9254821...41b9c9c

@chdorner
Copy link
Contributor Author

Marking WIP as browsers apparently don't support this, we should have investigated more yesterday before jumping on this solution. Anyway, @robertknight is doing some research first.

@chdorner chdorner added the WIP label Jan 27, 2017
@chdorner chdorner removed the request for review from nickstenning January 27, 2017 10:45
@robertknight
Copy link
Member

After some additional testing of different browsers using a tiny Go server, it looks like basic auth isn't going to work for us.

  1. Current versions of Firefox and Chrome will send an Authorization header with a base64-encoded version of the user and password embedded in the URL but not in the initial HTTP handshake request. Instead the server must respond with a 401 status and WWW-Authenticate field in the header if no auth is present and the browser will then retry with HTTP Basic credentials.
  2. IE and Edge do not appear to support sending basic auth credentials derived from the URL at all.

@chdorner
Copy link
Contributor Author

Closing, the solution to the browser issues with basic auth is in #4334.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants