JWT token mananger now returns the hash of the token #1935
Conversation
ishank011
force-pushed
the
jwt-token-hash
branch
4 times, most recently
from
5a03d01 to
b1a333e
Compare
| if err != nil { | ||
| return nil, nil, err | ||
| } | ||
|
|
There was a problem hiding this comment.
| // TODO fetch token from .. somewhere ? |
This PR would make the tokenmanager only work when all services are running in the same process. Do you plan to make this more flexble? The x-access-token JWT is an internal token that should not leave the boundaries of the microservices. For extarnal access there is the openid connect auth, refresh and id token.
You should never have to encode the x-access-token in a url. We should consider proper API keys for that, IMO.
There was a problem hiding this comment.
I tried with a persistent kv store as well (https://github.com/xujiajun/nutsdb), the problem was that the data wasn't synced until the connection was closed, so I faced the same issue of access from a different process. I'll try to benchmark how much of a drop in performance we face because of opening and closing a new file descriptor every time.
Agreed that the access token shouldn't be present in the URL, currently it's required by wopiserver though. We embed it in an iframe so the user doesn't see it but it can be easily extracted. But the issue is not just that; even adding it as a header might hit the upper limits (in our deployment, my token was 2.9k characters long and the nginx limit is 4-8k)
There was a problem hiding this comment.
For external access OpenID Connect should be used. I'm not sure kow the wopiserver works. Could we treat it like an openid connect client? We could generate a secret key for it ...
There was a problem hiding this comment.
Currently, the wopiserver needs a token with full access rights to create/rename/delete files and folders on behalf of the user. If a different openID connect based token could be used, that's fine a priori. But I understand the issue is to carry over wherever needed the Reva token as it can be too large even for headers...
There was a problem hiding this comment.
The internal access token is used to authenticate requests between micreservices and carry the some user data and the roles of a user. In OCIS we do it that way and the token remains small enough to cause problems. The services can then verify the tokens and fetch the permissions that are attached to the roles the user has. The permissions rarely change, so they can be cached. I haven't had the time to fully digest the roles and scopes concept in reva, but if it causes the token to become so large I think we need a different solution. It might depend on what we actually add to the token. maybe using abbreviations in the json keys and values will shrink it to a reasonable size?
Furthermore, I don't think the internal access token should ever leave the boundaries of the microservices. OpenID Connect has an id token that carries the user data, a refresh token that is longer lived and can be used to retrieve a new access token (which should then be a hash, not a jwt). access tokens should never be part of urls as they will be logged in intermediate proxies.
ishank011
force-pushed
the
jwt-token-hash
branch
from
96b17e2 to
7187832
Compare
|
Superseded by #2085 |
No description provided.