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

[Sharing NG] Tokeninfo endpoint #8617

Closed
JammingBen opened this issue Mar 12, 2024 · 12 comments
Closed

[Sharing NG] Tokeninfo endpoint #8617

JammingBen opened this issue Mar 12, 2024 · 12 comments
Assignees

Comments

@JammingBen
Copy link
Contributor

JammingBen commented Mar 12, 2024

Description

Web needs to be able to resolve public links and prompt the users for proper authentication.

  • internal links need a valid user session (authenicated user via OIDC)
  • password protected public links need the user to supply the correct password
  • public links without a password can be access by anybody knowing the link

Current implementation (ocs)

The OCS API provides a tokeninfo endpoint for public links (ocs/v1.php/apps/files_sharing/api/v1/tokeninfo/unprotected/{token}) which can be queried unauthenticated. The server responds with same basic data that the Web client uses to correctly resolve a public link.

Sharing NG

Therefore Web needs something similar with Sharing NG. The relevant information:

  • is the link password protected?
  • is the link an internal link? (current name is alias_link)
  • the id of the resource

Requirements

  • unauthenticated users accessing a password-less public link access the resource via the public-files
  • unauthenticated users accessing a password-protected public link need to provider their password
  • authenticated User accessing a public link for a resource that they already have access to by other means, access the file via it's original id (not via the public share provider). This is true independent for password protected and non-protected links.
  • authenticated User accessing an internal link are also directed to the original resource id
  • unauthenticated Users accessing an internal link are directed to the login page and can access the resource after authentication

cc @micbar @kobergj @butonic

@JammingBen
Copy link
Contributor Author

JammingBen commented Mar 26, 2024

To add a bit more detail on how Web uses the tokeninfo endpoint: When resolving a public link, Web needs to know if the link is internal or external to properly redirect the user to either the resolved public page or the internal location (after user login).

We decided to leave this untackled for now because we're not sure if a tokeninfo endpoint is the best approach to solve this issue.

Web will continue to use the OCS API for the tokeninfo for now.

@fschade
Copy link
Contributor

fschade commented Mar 26, 2024

I have a graph implementation ready, currently just the logic, no tests, no bells and whistles… we can use it as basis for discussion.

not at my desk right now but I’ll add the pr tomorrow. Thanks to everyone that was part of the brainstorming!

@rhafer
Copy link
Contributor

rhafer commented Apr 3, 2024

Apart from just re-implementing the tokeninfo logic for sharing-ng, we could try to find a cleaner way to get the same functionality. After some some discussion we came up with this, which should not need a tokeninfo endpoint at all and could purely work with webdav. It is still somewhat rough and we would have to prove that it does actually work:

The client should basically always sent a PROPFIND to /dav/spaces/{publicstorageid}${publicspaceid}!{sharetoken}

  • authenticated clients accessing an internal link are redirected to the "real" resource (`/dav/spaces/{target-resource-id}
  • authenticated clients accessing a pubic link (password protected or not) for a resource they already have access to are also redirected to the "real" resource. (and always need to supply the password)
  • unauthenticated clients access an internal link get a 401 returned with WWW-Authenticate set to Bearer (so that the client knows that it need to get a token via the IDP login page.
  • unauthenticated clients accessing a password protected link get a 401 returned with WWW-Authenticate set to Basic to indicate the requirement for needing the links password.

This would however require some changes our ocdav service (it does not handled the above request on the publicstorageids yet) and AFAICS also in the way we handle authentication for ocdav. So it requires some more research.

@kobergj
Copy link
Collaborator

kobergj commented Apr 3, 2024

Yes I like that approach more than the old one. But some parts are not really clear to me.

The client should basically always sent a PROPFIND to /dav/spaces/{publicstorageid}${publicspaceid}!{sharetoken}

Where does it get that URL from? It might only have the public link /s/opTtskfWusbUBGy

authenticated clients accessing a pubic link (password protected or not) for a resource they already have access to are also redirected to the "real" resource. (No need to supply the password)

As much as that would be a smooth user experience, I am not sure we can do this. We are giving away information from a password protected link, without knowing the password. Might be better if we avoid this.

@rhafer
Copy link
Contributor

rhafer commented Apr 3, 2024

Yes I like that approach more than the old one. But some parts are not really clear to me.

The client should basically always sent a PROPFIND to /dav/spaces/{publicstorageid}${publicspaceid}!{sharetoken}

Where does it get that URL from? It might only have the public link /s/opTtskfWusbUBGy

The public storage and space ids are currently hardcoded AFAIK, so those would basically come from configuration. Alternatively to /dav/spaces we could also use /dav/public-files/{sharetoken} as the entry point I guess. (The authentication issues we neede to figure out are pretty similar in both cases I guess).

authenticated clients accessing a pubic link (password protected or not) for a resource they already have access to are also redirected to the "real" resource. (No need to supply the password)

As much as that would be a smooth user experience, I am not sure we can do this. We are giving away information from a password protected link, without knowing the password. Might be better if we avoid this.

AFAIK this is already the current behavior of the web client when resolving a password-protected link via the tokeninfo endpoint. And if the user does already have access to that resource we're not giving anymore access that the user would already have.

@kobergj
Copy link
Collaborator

kobergj commented Apr 3, 2024

The public storage and space ids are currently hardcoded AFAIK, so those would basically come from configuration. Alternatively to /dav/spaces we could also use /dav/public-files/{sharetoken} as the entry point I guess. (The authentication issues we neede to figure out are pretty similar in both cases I guess).

Yes better 👍 I would like to avoid clients having to learn server id logic

AFAIK this is already the current behavior of the web client when resolving a password-protected link via the tokeninfo endpoint. And if the user does already have access to that resource we're not giving anymore access that the user would already have.

True, but that doesn't mean that the behaviour is currently secure. We do not disclose access to the file, that is true, but we are giving away information about the link to a user who might not have that information otherwise. I agree the attack vector here is very small, but we should try to avoid such cases completely.

@JammingBen
Copy link
Contributor Author

As much as that would be a smooth user experience, I am not sure we can do this. We are giving away information from a password protected link, without knowing the password. Might be better if we avoid this.

AFAIK this is already the current behavior of the web client when resolving a password-protected link via the tokeninfo endpoint. And if the user does already have access to that resource we're not giving anymore access that the user would already have.

Password protected links always prompt the user for a password currently, no matter if they are authenticated or if they have access to that resource or not. After the password has been successfully entered, the server responds with all necessary information to resolve that link to its internal location (for authenticated users that have access).

IMO this behaviour is fine for the future. I also don't understand the security concern with this approach since Web asks for a password in any case 🤔

@kobergj
Copy link
Collaborator

kobergj commented Apr 3, 2024

Yeah, maybe I'm exaggerating the security impact. My point is only that I'm not happy with giving away information (in this case the fileid) to a user who has not yet provided the password.

@JammingBen
Copy link
Contributor Author

I'm totally with you, hence my suggestion to leave the UX as it already is and not give away anything until the user has entered a valid password. Web can still redirect the user to whatever location necessary afterwards.

@rhafer
Copy link
Contributor

rhafer commented Apr 4, 2024

Password protected links always prompt the user for a password currently

@JammingBen Just for clarification (so that I can update the requirements/spec). When an authenticated user is accessing a password protected public link the user needs to enter the password for that link?

@JammingBen
Copy link
Contributor Author

@JammingBen Just for clarification (so that I can update the requirements/spec). When an authenticated user is accessing a password protected public link the user needs to enter the password for that link?

Correct 👍

@micbar
Copy link
Contributor

micbar commented Apr 15, 2024

Follow Up Ticket #8858

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

Successfully merging a pull request may close this issue.

5 participants