Conversation
|
Not a regular user of Typescript, but will add my thoughts :) |
adyanth
left a comment
adyanth
left a comment
There was a problem hiding this comment.
Using both token and header for authentication might be an anti-pattern (if the reverse proxy sends traffic without the header due to an issue, the token will still be valid and allow access). You can switch between one or the other.
Rest looks good, thanks!
| async (req, res, next) => { | ||
| async (req, res) => { | ||
| if (config.header_auth) { | ||
|
|
There was a problem hiding this comment.
Is this supposed to return here/return something?
There was a problem hiding this comment.
Accidentally left it in. They should never need the /signin or /signup routes.
| await user.save() | ||
| } | ||
|
|
||
| if (!token) { |
There was a problem hiding this comment.
Is there a reason for using both header and token auth? (Unless there is a dependency of the token)
If header auth is selected, there is no need for a token. The middleware can verify the header on each request. If header auth is not enabled, proceed as usual with signing the token on login and validating that in the middleware.
There was a problem hiding this comment.
Good call. I wrapped this in an if-else and added
req.user = user
next()if the user is using header auth
| "signedIn", | ||
| typeof window === "undefined" ? false : !!Cookies.get("drift-token") | ||
| ) | ||
| const token = Cookies.get("drift-token") |
There was a problem hiding this comment.
Point to note, if the header auth is used, this can/will be null. This is fine, since the middleware should ignore the header altogether.
| - `HEADER_AUTH`: if true, enables authenthication via the HTTP header specified in `HEADER_AUTH_KEY` which is generally populated at the reverse-proxy level. | ||
| - `HEADER_AUTH_KEY`: if `HEADER_AUTH` is true, the header to look for the users username (like `Auth-User`) | ||
| - `HEADER_AUTH_ROLE`: if `HEADER_AUTH` is true, the header to look for the users role ("user" | "admin", at the moment) | ||
| - `HEADER_AUTH_WHITELISTED_IPS`: comma-separated list of IPs users can access Drift from using header authentication. Defaults to '127.0.0.1'. |
3 participants
Closes #11