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

Accept reva token as a bearer authentication #3315

Merged
merged 5 commits into from
Oct 6, 2022

Conversation

gmgigi96
Copy link
Member

@gmgigi96 gmgigi96 commented Oct 5, 2022

No description provided.

@gmgigi96 gmgigi96 requested review from ishank011 and a team as code owners October 5, 2022 16:14
labkode
labkode previously approved these changes Oct 5, 2022
@gmgigi96 gmgigi96 marked this pull request as draft October 5, 2022 20:25
@micbar
Copy link
Member

micbar commented Oct 6, 2022

@gmgigi96 @labkode @glpatcern

Let me please throw in my 2 cents.

I see some security aspects in this PR. Reva access tokens are very powerful. Most of them are not scoped and could allow access to any resource and all file operations.

IMHO it is not a good practise to use reva access tokens outside of reva and on the client side. Clients should only use short-living OIDC tokens. This is how we do that in the ocis middlewares.

@micbar
Copy link
Member

micbar commented Oct 6, 2022

IMHO it is not a good practise to use reva access tokens outside of reva and on the client side. Clients should only use short-living OIDC tokens. This is how we do that in the ocis middlewares.

Let me explain that a bit better:

As long as you use the reva-CLI or another CERN internal client, this not an issue.

BUT: We are currently sending Reva Tokens via the cs3org Wopi server to the Microsoft Office365 Server. Now the office 365 Server has a valid working reva jwt token. That means it can have access to all the files of a specific user as long as the application has direct access to the reva gateway. (which is not the case when it is not directly exposed)

Enabling that access also via HTTP is like opening the gates.

@micbar
Copy link
Member

micbar commented Oct 6, 2022

@labkode It seems that we have a little disconnect how to handle authentication in reva.

Maybe let us move that discussion to direct communication.

@gmgigi96
Copy link
Member Author

gmgigi96 commented Oct 6, 2022

BUT: We are currently sending Reva Tokens via the cs3org Wopi server to the Microsoft Office365 Server. Now the office 365 Server has a valid working reva jwt token. That means it can have access to all the files of a specific user as long as the application has direct access to the reva gateway. (which is not the case when it is not directly exposed)

I think the problem is more to restrict the scope of the token we are sending to Microsoft to only the resource the app will be using.

@micbar
Copy link
Member

micbar commented Oct 6, 2022

I think there are two issues with reva tokens currently:

  1. the missing scopes -> could be fixed
  2. they are issued once and cannot be invalidated
    Reva tokens are not under the control of an IdP and can therefore not be revoked. In addition to that, they have lifetimes which are very long. So a compromised client can do a lot of damage.

@micbar
Copy link
Member

micbar commented Oct 6, 2022

I think the problem is more to restrict the scope of the token we are sending to Microsoft to only the resource the app will be using.

Yes, or better encrypt them. It is always best practise that any 3rd party which does not need credentials should not get those. see cs3org/wopiserver#90

@labkode
Copy link
Member

labkode commented Oct 6, 2022

Hi @micbar,

The issue today is that we give full scope when passing the token for another purposes (like WOPI or OCM), this obviously needs to be fixed, what @gmgigi96 said.

Today the token we use is the one we get from our IDP, and that is a general token for every service, which has a few issues for us today: they are very short lived and web does not handle them like it should (owncloud/web#7729), giving the user a very bad UX.

We also force users to use 2FA, so asking them to login more than once in a day is painful.

Today, for every request that arrives to Reva, we issue a query to verify the token against our IDP.
We have not invested yet on downloading the JWK keys and verify them locally, if you have done so, what is your experience?

To provide the best usability to the users (like Google and co do), we'll use longer tokens, up to a full working week.
We're adding a mechanism to actually invalidate a user in case the token is compromised, is like a blacklist that will last as far as the maximum token duration (1 week). So by design the blacklist will be short and not live more than the nominal token duration.

This direction also align us with the support for application passwords that will be needed by the sync client, we won't be asking the user to login in the sync client every now and then, the token set on the client must be permanent (this is also the direction that other companies are doing to use 3rd party applications).

I hope this clarifies a bit the intention here.

@gmgigi96 gmgigi96 marked this pull request as ready for review October 6, 2022 13:14
@labkode labkode merged commit e58cd30 into cs3org:master Oct 6, 2022
vascoguita pushed a commit to vascoguita/reva that referenced this pull request Oct 18, 2022
vascoguita pushed a commit to vascoguita/reva that referenced this pull request Oct 18, 2022
vascoguita pushed a commit to vascoguita/reva that referenced this pull request Oct 20, 2022
aduffeck pushed a commit to aduffeck/reva that referenced this pull request Sep 18, 2023
aduffeck pushed a commit to aduffeck/reva that referenced this pull request Sep 22, 2023
aduffeck pushed a commit to aduffeck/reva that referenced this pull request Oct 9, 2023
aduffeck pushed a commit to aduffeck/reva that referenced this pull request Oct 10, 2023
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.

3 participants