-
Notifications
You must be signed in to change notification settings - Fork 143
docs: ADR for universal drive app without user domain #4652
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
base: master
Are you sure you want to change the base?
Conversation
Crash--
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
didn't read all the ADR yet.
| | URL Pattern | Description | Routing | Auth Required | | ||
| |-------------|-------------|---------|---------------| | ||
| | `/files/{file-id}` | Personal files | Route to authenticated user's instance | Yes | | ||
| | `/shares/{sharing-id}/files/{file-id}` | Shared drive files | Query user's instance for sharing info | Yes | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we are doing the old way to share aka duplication, do we have the same sharing-id / file-id on both instances ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This URL /shares/{sharing-id}/files{file-id} is it the one displayed in the user's browser?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we have the same sharing_id, but not file_id, and there is nothing in this ADR about "old" shares, there's migration or anything, I assumed that they continue to work as is
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, it's API call
| |-------------|-------------|---------|---------------| | ||
| | `/files/{file-id}` | Personal files | Route to authenticated user's instance | Yes | | ||
| | `/shares/{sharing-id}/files/{file-id}` | Shared drive files | Query user's instance for sharing info | Yes | | ||
| | `/public/{encoded-sharecode}` | Public link files | Decode sharecode to find instance | No | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to define new routes? Why can't we re use all the existing routes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's actually "old" routes, but if you use the same cozy-stack and not a new service, we will need to have a new middleware to detect that we are in "universal" mode, resolve an instance, and delegate further
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't we create a new DNS entry:
universaldrive.twake.app that point to this "service".
So depending on the DNS you're targeting, you arrive or not on this universal mode.
And if you arrive on this new "service", we can accept "", resolve to an instance, and then deletage this "" to the right instance.
Maybe this is what you plan to do, but having listed only these 5 or 6 routes got my confused
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, exactly, I thought it was written somewhere there, I'll double-check
| | `/shares/{sharing-id}/files/{file-id}` | Shared drive files | Query user's instance for sharing info | Yes | | ||
| | `/public/{encoded-sharecode}` | Public link files | Decode sharecode to find instance | No | | ||
|
|
||
| **Note:** For public links, `encoded-sharecode` contains instance information as `base64(instance-slug:original-sharecode)`. For authenticated shared drive access, the user's instance is queried to resolve the sharing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is is already the casE?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, I'll check, but I don't think that we have an instance in the share-code right now, I didn't make any sense
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, it depends of the sharecodes. Long sharecodes are JWT, with the instance URL in a field. But, what we use are shortcodes, and we need an instance to query the database to fetch the permissions document for them.
| window.cozyData = { | ||
| "token": "eyJhbG...", | ||
| "domain": "drive.org.com", // API calls via Universal Drive | ||
| "realtimeDomain": "usera.org.com", // WebSocket direct to user's instance |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @paultranvan @zatteo @rezk2ll @doubleface since it means that we'll have things to change on cozy-client side.
|
|
||
| The frontend uses `cozyData.domain` for all API calls: | ||
| ```javascript | ||
| // cozy-client-js reads domain from cozyData |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cozy-client please, cozy-client-js is dead since 8 years :p
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How can it be dead, if it's on all our apps?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nop. https://github.com/cozy/cozy-client-js is deprecated since few years in favor of https://github.com/linagora/cozy-client .
@taratatach Maybe we can archive the cozy-client-js repo? Or at least put a big banner saying deprecated on this repo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be fair, it was supposed to be dead 8 years ago, but still managed to painfully live until quite recently :)
The deprecated notice sounds good.
| ``` | ||
| ┌──────────────────────────────────────────────────────────────────┐ | ||
| │ User's Browser │ | ||
| │ URL: https://drive.org.com/files/{file-id} │ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Today Drive app generates app as:
https://xxx-drive.twake.app/#/folder/io.cozy.files.root-dir
we'll never have:
https://drive.org.com/files/{file-id}
But I assume it will work with hash navigation too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, it's just an example
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it won't work with current URLs. For a public sharing, the first request the browser will send is https://drive.org.com/public/ (the sharecode is in the hash), and the html response from the stack must contain in the token of the app, which requires to know the instance, and we don't have any clue for that.
| │ │ │ | ||
| │ 2. 302 Redirect to Registration │ │ | ||
| │ /auth?client_id=drive& │ │ | ||
| │ redirect_uri=drive.org/cb │ │ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in fact, no, maybe it'll not work with the #hash navigation because server will never see that, and we'll lose the information during the redirection, no?
|
|
||
| | Component | Changes Needed | | ||
| |-----------|----------------| | ||
| | **cozy-drive frontend** | Minor: Support `realtimeDomain` for WebSocket connections | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to change the way we handle the token from publish shared code too (from the front-end side)
|
|
||
| **Option A: Federated OIDC Authentication** | ||
|
|
||
| Allow users to authenticate with their home organization's IdP when accessing another organization's Universal Drive. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will authenticate with the external OIDC but we'll have a Lemon between the external OIDC and our app.
Our app will communication with this Lemon OIDC. And Lemon will communicate with the external OIDC. Like that, we can still have workplaceFQDN and everything. So, yes, it kinda a centralized stuff
| Current: usera.org.com/drive ← UserA's drive | ||
| userb.org.com/drive ← UserB's drive | ||
| Proposed: drive.org.com ← Universal Drive for all users in org |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the current version, we have both usera.org.com (API calls), and usera-drive.org.com, with the drive assets (html, js, css, favicons, etc.). In the stack, it's two routers called router and appsHandler in web/routing.go.
Using the new domain for both the drive URLs and API calls may be a bit tricky:
- looking at assets and then API calls: we don't want to check if an asset is present in Swift for every API call (too slow)
- looking at API calls and then at assets: we may have some bad icons (I think both respond to
/and/robots.txt
But if I understand correctly, you want to use drive.org.com for the drive assets/URLs, not API calls. Correct?
|
|
||
| WebSocket connections for realtime events (file changes, notifications) connect **directly to the user's instance domain** rather than through the Universal Drive domain. This avoids the complexity of WebSocket proxying and provides better performance. | ||
|
|
||
| **Approach:** Inject a separate `realtimeDomain` in cozyData: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand why we are doing something different for realtime websockets and API calls.
| | `/shares/{sharing-id}/files/{file-id}` | Shared drive files | Query user's instance for sharing info | Yes | | ||
| | `/public/{encoded-sharecode}` | Public link files | Decode sharecode to find instance | No | | ||
|
|
||
| **Note:** For public links, `encoded-sharecode` contains instance information as `base64(instance-slug:original-sharecode)`. For authenticated shared drive access, the user's instance is queried to resolve the sharing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, it depends of the sharecodes. Long sharecodes are JWT, with the instance URL in a field. But, what we use are shortcodes, and we need an instance to query the database to fetch the permissions document for them.
| ``` | ||
| ┌──────────────────────────────────────────────────────────────────┐ | ||
| │ User's Browser │ | ||
| │ URL: https://drive.org.com/files/{file-id} │ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it won't work with current URLs. For a public sharing, the first request the browser will send is https://drive.org.com/public/ (the sharecode is in the hash), and the html response from the stack must contain in the token of the app, which requires to know the instance, and we don't have any clue for that.
| Original: sharing-id = "abc123def" owned by usera.org.com | ||
| Encoded: sharing-id = "dXNlcmE6YWJjMTIzZGVm" (base64 of "usera:abc123def") | ||
| URL: /shares/dXNlcmE6YWJjMTIzZGVm/files/{file-id} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will need to update cozy-drive (or cozy-libs?) to use the new sharing-id encoding in public link: this URL is built on the front side, not on the server.
| Example: | ||
| Original: ?sharecode=abc123 on usera.org.com | ||
| Encoded: /public/dXNlcmE6YWJjMTIz (base64 of "usera:abc123") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
idem
|
|
||
| **Required changes:** | ||
| 1. **cozy-client/cozy-realtime:** Check for `realtimeDomain` in cozyData, use it for WebSocket URL | ||
| 2. **CORS:** Configure cozy-stack to accept WebSocket connections with Universal Drive origin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think CORS will work with the current code. But, we will need to update the CSP rules for that to work.
| **Required changes:** | ||
| 1. **cozy-client/cozy-realtime:** Check for `realtimeDomain` in cozyData, use it for WebSocket URL | ||
| 2. **CORS:** Configure cozy-stack to accept WebSocket connections with Universal Drive origin | ||
| 3. **Auth:** WebSocket AUTH message works with existing token mechanism |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the tricky part is that the token for a user (not for a public link) requires that a cookie is present for a session for that user. It should work as drive.org.com and usera.org.com are both subdomains of .org.com, and that the cookie should be set on .org.com.
No description provided.