Feature/#840 JWT cookie authentication #849
Conversation
This package isn't strictly necessary since a possibly more performant custom implementation could be written for the intended singular cookie parsing use case.
Also rectifies the missing 'mediaToken' documentation part and does some very minor refactoring.
This is now possible because of the JWT-cookie layer. Through this change the mediaToken system becomes obsolete.
Note that there seems to be some issue with the chat system that already existed prior to the transition to cookie-based authentication.
Also deactivates extractJwtFromAuthHeaderWithScheme by default, so that only extractJwtFromCookie is active, and removes the mediaTokenInsufficient error code.
This should be the last mediaToken remnant on the API-side.
Removes the obsolete localStorage tokens (token & mediaToken) as well as the obsolete authHeader method and adjusts isLoggedIn accordingly.
Since there still is an old "## [NEXT]" section I added a new one with an identifier for the next "Release [08.11.2018]".
|
While this should be complete, please don't merge this to |
# Conflicts: # CHANGELOG.md # api/package-lock.json # api/package.json
# Conflicts: # CHANGELOG.md # api/package-lock.json # api/package.json
|
@torss i merged latest develop and resolved merge conflicts. Could you have another look at |
Pull Request Test Coverage Report for Build 408
💛 - Coveralls |
|
@danielkesselberg Probably yes! At least iirc I had no specific reason not to install that under (Also a sidenote re. the |
|
I have no objections to |
There was a problem hiding this comment.
LGTM 👍 Its pretty common to pass a jwt token as header but you made strong arguments for the cookie approach. I assume you did a lot of research about this topic so i'm fine with it. Big plus we need much less code than before and access to images / videos is much easier (no need for jwtpipe anymore 🎉)
|
@danielkesselberg Well, I don't know if I would call it "a lot" of research, but hopefully it was enough. xD |
|
I guess |
…840-jwt-cookie-auth # Conflicts: # CHANGELOG.md # api/package.json
Description
Closes #840
Switches the entire authentication system to use a single JWT stored and transmitted via a cookie.
The cookie is
HttpOnlyand hasSameSiteset to'Strict'to protect against CSRF attacks.Previously we had two tokens: A primary
'token'and a'mediaToken', both stored inlocalStorage. The'mediaToken'was a relatively recent addition in #807 to secure the static'uploads'route for images and such. In contrast to a cookie this'mediaToken'had to be sent via URL query parameter, see the #807 PR for details.Improvements
localStoragetokens could (only) be read via JavaScript. Now the newHttpOnlycookie token can't be accessed via JavaScript at all, it is (re)set via HTTP requests.localStoragetokens had to be handled via JavaScript.'mediaToken'system: Since the'mediaToken'system was only implemented due to browser API limitations Bugfix/#729 secure static uploads #807 it was now possible to removed it completely, including the'JwtPipe'.'uploads'route securiy: A user now has to be logged-in to access files of the'uploads'route, making it impossible to accidentially share/leak a URL with attached'mediaToken'.Known Issues
Secureattribute (i.e. HTTPS-only) set - maybe this should be configurable to implicitly prevent accidental non-HTTP hosting.'uploads'route security also implies that it is now impossible for users to quickly share a non-public file URL with people who don't have an account / aren't logged-in. This should probably be solved by providing proper, explicit public URL creation facilities. (This isn't really a new issue because it was only possible to share such URLs due to the lax / non-existent security.)