Closed wkloucek closed 3 years ago
@glpatcern something to discuss next week if you like. OnlyOffice works just fine with the WOPI server and the AppProvider if we limit the userid to 128 characters for now (they have plans to raise the limit)
This pull request introduces 1 alert when merging d0f298aa2cc1d0773d54aaf4818d0d202152fc9d into 05444c713cdcbb1b28535372bf3be9f1ccb4cd1e - view on LGTM.com
new alerts:
As discussed on the chat, let's bring this up next week.
A concern I have is more "conceptual": if we allow wopi to peek into the Reva token, then we could do that early on so the WOPI access token is also much smaller - there's no need to carry over the whole thing... But then, we should rather have the AppProvider in Reva mint a more scoped token, so there's no need to inject in WOPI this knowledge of the Reva token's structure. The scoped token does not need to carry most of the user-specific info (in particular groups, which can make 20-30kb of payload for us), and could also be scoped in terms of permissions (yet granting full write access to the folder containing the target file, as that's the minimum requirement for WOPI to create lock files, support renames, etc.).
This is a priori superseded by https://github.com/cs3org/reva/pull/2085. We could re-discuss if that's not sufficient, yet WOPI should not assume any structure of the Reva token. Closing for now.