Sorunome / mx-puppet-bridge

Puppeting library for matrix
Apache License 2.0
95 stars 29 forks source link

Add support for user-specific management API tokens #50

Closed tulir closed 4 years ago

tulir commented 4 years ago

User-specific tokens are simply HMAC-SHA512 of the user ID and global shared secret

Sorunome commented 4 years ago

Why not use JWTs?

tulir commented 4 years ago

That's a lot more effort

Sorunome commented 4 years ago

Yeah, but then you could easily expand the system in the future without having to re-write much code

auscompgeek commented 4 years ago

Tokens in this scheme don't expire. Wouldn't this become a problem for all users of a bridge if a single user has their token compromised?

tulir commented 4 years ago

Expiration in a sensible way requires stateful tokens. It might make sense to add stateful tokens eventually, but this works fine for now

Sorunome commented 4 years ago

JWT has expiracy

tulir commented 4 years ago

JWT expiration is more or less useless. It either doesn't help for security, makes the UX horrible or is simply a pointless wrapper for stateful tokens.

(http://cryto.net/~joepie91/blog/2016/06/13/stop-using-jwt-for-sessions/ and http://cryto.net/~joepie91/blog/2016/06/19/stop-using-jwt-for-sessions-part-2-why-your-solution-doesnt-work/)

Sorunome commented 4 years ago

You would not use a JWT to store any session data in this case. Instead, you use the JWT for authentication.

tulir commented 4 years ago

Short expiry -> bad UX Long expiry -> doesn't help if a token is compromised Stateful token in JWT -> pointless

Sorunome commented 4 years ago

Still way better than what you currently have.

joepie91 seems to be doing lots of unjustified fear mongering around JWTs, so yeah.

Sorunome commented 4 years ago

Short expiry -> bad UX

it must get the JWT from somewhere, make it seemlessly fetch a new JWT --> good UX

tulir commented 4 years ago

Right now it doesn't fetch the token from anywhere. If I made a new service for that, I could just make it proxy requests instead of giving JWTs.

Sorunome commented 4 years ago

Soru already suggested using JWTs in https://github.com/Sorunome/mx-puppet-bridge/pull/27 and, as an example, said that it could authenticate specific users.

It seems JWTs are more appropriate here than rolling some custom hash method. If you want to re-write this to use JWTs, feel free to also remove the old authentification method, so that we don't have too many things clogged up in the code.

Sorunome commented 4 years ago

It still makes sense to have an API only for fetching JWTs as it is way less work for the server to generate JWTs than to proxy requests.

tulir commented 4 years ago

feel free to also remove the old authentification method, so that we don't have too many things clogged up in the code.

Shared secret auth is definitely still needed, I only needed user-specific auth due to the twitter limitations.

I might switch to proxying, but probably won't bother doing anything other than that.