Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use opaque id from user if available and limit userid to 128 characters #47

Closed
wants to merge 2 commits into from
Closed

use opaque id from user if available and limit userid to 128 characters #47

wants to merge 2 commits into from

Conversation

wkloucek
Copy link
Contributor

  • uses the opaque id from users if available, because the userid should be stable and represent an user in an unique way
  • ensures that the userid is not longer than 128 characters to be compatible with onlyoffice via wopi

@wkloucek
Copy link
Contributor Author

@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)

@lgtm-com
Copy link
Contributor

lgtm-com bot commented Sep 10, 2021

This pull request introduces 1 alert when merging d0f298a into 05444c7 - view on LGTM.com

new alerts:

  • 1 for Except block handles 'BaseException'

@glpatcern
Copy link
Member

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.).

@glpatcern
Copy link
Member

This is a priori superseded by cs3org/reva#2085. We could re-discuss if that's not sufficient, yet WOPI should not assume any structure of the Reva token. Closing for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants