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

Generate and validate Reva tokens from Reva #2525

Closed
labkode opened this issue Sep 17, 2021 · 5 comments · Fixed by #2528
Closed

Generate and validate Reva tokens from Reva #2525

labkode opened this issue Sep 17, 2021 · 5 comments · Fixed by #2528

Comments

@labkode
Copy link
Contributor

labkode commented Sep 17, 2021

OCIS hacks the ways tokens are supposed to be used by relying on the internal implementation of Reva, which currently uses JWT, this makes it impossible to change underlying session storage.

Plus, the defaults are not the same in Reva, which provides different expiration times depending what authentication method you use (basic vs bearer (oidc)).

These are all the places that must be cleaned:

[18:17][root@cbox-ocisdev-hugo (ocis_dev:box/ocis) ocis]# grep -ri github.com/cs3org/reva/pkg/token/manager/jwt . | grep go
./accounts/pkg/storage/cs3.go:  "github.com/cs3org/reva/pkg/token/manager/jwt"
./ocis-pkg/indexer/index/cs3/autoincrement.go:  "github.com/cs3org/reva/pkg/token/manager/jwt"
./ocis-pkg/indexer/index/cs3/non_unique.go:     "github.com/cs3org/reva/pkg/token/manager/jwt"
./ocis-pkg/indexer/index/cs3/unique.go: "github.com/cs3org/reva/pkg/token/manager/jwt"
./ocis-pkg/middleware/account.go:       "github.com/cs3org/reva/pkg/token/manager/jwt"
./ocs/pkg/server/http/svc_test.go:      "github.com/cs3org/reva/pkg/token/manager/jwt"
./ocs/pkg/service/v0/users.go:  "github.com/cs3org/reva/pkg/token/manager/jwt"
./proxy/pkg/middleware/create_home.go:  "github.com/cs3org/reva/pkg/token/manager/jwt"
./proxy/pkg/middleware/account_resolver.go:     "github.com/cs3org/reva/pkg/token/manager/jwt"
./graph/pkg/middleware/auth.go: "github.com/cs3org/reva/pkg/token/manager/jwt"

The reason why the WOPI tokens had an expiration date of 1 minute is because this value is hardcoded in:
https://github.com/owncloud/ocis/blob/master/proxy/pkg/middleware/account_resolver.go#L26
@wkloucek

Fixing this value avoids this workaround:
cs3org/reva#2058

@C0rby can you take this on and call the Authenticate call in Reva rather than doing this workaround?

@wkloucek
Copy link
Contributor

@labkode my idea was also to add a scope to the token in cs3org/reva#2058. The token shouldn't have that much permissions for just opening one file and writing two lock files. And in the app provider we have that information to limit the scope.

@C0rby
Copy link
Contributor

C0rby commented Sep 20, 2021

@labkode, I totally agree and would love to tackle this. 👍

@labkode
Copy link
Contributor Author

labkode commented Sep 20, 2021

@C0rby can you jump into this from the OCIS side? @ishank011 will provide a new implementation for tokens in Reva to unblock the integration with some apps but we need this development in OCIS to have the full workflow working.

@glpatcern
Copy link

The token shouldn't have that much permissions for just opening one file and writing two lock files. And in the app provider we have that information to limit the scope.

Just to be sure, the token used by WOPI can definitely be scoped down but it shall have more permissions than the above: essentially full access to the container/folder where the file is located. This in order to support "Save as" operations (PutRelative in WOPI parlance), as well as the ability to check existing lock files created by other apps (MS Office, LibreOffice).

@ishank011
Copy link
Contributor

@C0rby I think the machine auth mechanism might be the most straightforward way for this. It will delegate the GetUserByClaim call that ocis currently makes to reva and my PR will make sure that the token size is limited cs3org/reva#2085

Ref cs3org/reva#2028

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

Successfully merging a pull request may close this issue.

5 participants