implement read-only auth #373

rolyatmax · 2025-01-15T21:31:18Z

Have left a few comments on this PR to give more context. Probably easiest to review this PR commit-by-commit, leveraging the context given in each commit message.

related: #307

vercel · 2025-01-15T21:31:23Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

3 Skipped Deployments

Name	Status	Preview	Updated (UTC)
y-sweet-debugger	⬜️ Ignored (Inspect)	Visit Preview	Jan 17, 2025 5:13pm
y-sweet-demos	⬜️ Ignored (Inspect)	Visit Preview	Jan 17, 2025 5:13pm
y-sweet-gendocs	⬜️ Ignored (Inspect)	Visit Preview	Jan 17, 2025 5:13pm

rolyatmax · 2025-01-15T21:32:10Z

crates/y-sweet-core/src/api_types.rs

-    #[serde(rename = "none")]
-    Nothing,
-    #[serde(rename = "readonly")]
+    #[serde(rename = "read-only")]


This matches the types in the JS client library

crates/y-sweet-core/src/api_types.rs

rolyatmax · 2025-01-15T21:34:49Z

crates/y-sweet-core/src/api_types.rs

 pub struct AuthDocRequest {
    #[serde(default = "Authorization::full")]
    pub authorization: Authorization,
    #[serde(rename = "userId")]
    pub user_id: Option<String>,
-    #[serde(default)]
-    pub metadata: HashMap<String, Value>,


We don't use this at present. And if we did, we'd likely need to encode it into the token which we'd like to avoid since its length is unbounded. Let's just drop this field altogether.

rolyatmax · 2025-01-15T21:39:00Z

crates/y-sweet/src/server.rs

@@ -440,37 +431,50 @@ async fn get_doc_as_update(
 async fn get_doc_as_update_deprecated(
    Path(doc_id): Path<String>,
    State(server_state): State<Arc<Server>>,
-    authorization: Option<TypedHeader<headers::Authorization<headers::authorization::Bearer>>>,
+    auth_header: Option<TypedHeader<headers::Authorization<headers::authorization::Bearer>>>,


A lot of these changes are just renaming vars/args to better differentiate between an Authorization (an enum that indicates read/write access) and an auth header.

jakelazaroff · 2025-01-15T23:07:43Z

crates/y-sweet-core/src/doc_connection.rs

+                        Err(sync::Error::PermissionDenied {
+                            reason: "Token does not have write access".to_string(),
+                        })


Do we plan to make the clients aware of the authorization level as well? Not sure how these errors are logged/captured/etc but as of now this is an expected code path, since clients will send updates as though they have full write access.

Yeah, this just drops out before writing, and the resulting error gets logged in the y-sweet server. It shouldn't interrupt the websocket connection. I think we'll want to include the level of Authorization (full or read-only) in the ClientToken so that clients can use it when rendering the UI. I'm not sure how easy it is for the YSweetProvider to interrupt local changes to the YDoc, but we could at least have the provider subscribe to changes on the YDoc and log warnings when in read-only mode.

rolyatmax · 2025-01-16T21:38:19Z

crates/y-sweet-core/src/api_types.rs

+
+    /// The authorization level of the client.
+    #[serde(rename = "authorization")]
+    pub authorization: Authorization,


We're now returning authorization as part of the ClientToken so that clients can limit front-end functionality (or just render differently) when the token is read-only.

…tion::Nothing variant

…and to pull it from a token in full-server mode

paulgb · 2025-01-17T16:48:50Z

crates/y-sweet/src/server.rs

@@ -385,7 +385,7 @@ impl Server {
            if let Some(token) = token {
                let authorization = authenticator
                    .verify_doc_token(token, doc, current_time_epoch_millis())
-                    .map_err(|e| (StatusCode::FORBIDDEN, e))?;
+                    .map_err(|e| (StatusCode::UNAUTHORIZED, e))?;


is this a case where the test is wrong rather than the implementation? I think if the token is provided which it seems like the guard checks here), but the token is invalid, it's a forbidden rather than unauthorized.

@ellipsis-dev do you know?

I think if verify_doc_token fails, it's because the token is invalid - which should return a 401 - a failure to authenticate. (I hate that the spec calls it 401 Unauthorized since 403s should be used when authorization fails while 401s should be used when authentication fails. 🤷‍♂️)

From MDN docs:

The HTTP 401 Unauthorized client error response status code indicates that a request was not successful because it lacks valid authentication credentials for the requested resource.

A 401 Unauthorized is similar to the 403 Forbidden response, except that a 403 is returned when a request contains valid credentials, but the client does not have permissions to perform a certain action.

ellipsis-dev · 2025-01-17T16:50:25Z

The read-only auth implementation adds a ReadOnly permission level alongside Full access. It uses the Authorization enum for permission levels (api_types.rs) and enforces them through token-based authentication. Read-only tokens are generated with Authorization::ReadOnly and verified during API requests. The implementation includes proper serialization with "read-only" and "full" string representations (auth.rs).

ellipsis-dev · 2025-01-17T16:50:30Z

Read-only auth uses a token-based system with ReadOnly and Full levels (Authorization enum in api_types.rs). Tokens are generated with gen_doc_token() and verified during access. The ClientToken structure includes the token and authorization level (api_types.rs). Read-only auth is tested (auth.rs).

paulgb

Thanks for this! LGTM

crates/y-sweet-core/src/api_types.rs

rolyatmax requested review from paulgb and jakelazaroff January 15, 2025 21:31

rolyatmax force-pushed the taylor/dis-3111-y-sweet-server-implement-read-only-auth branch from 957959c to 6c570c8 Compare January 15, 2025 21:31

rolyatmax commented Jan 15, 2025

View reviewed changes

crates/y-sweet-core/src/api_types.rs Outdated Show resolved Hide resolved

rolyatmax commented Jan 15, 2025

View reviewed changes

jakelazaroff reviewed Jan 15, 2025

View reviewed changes

rolyatmax force-pushed the taylor/dis-3111-y-sweet-server-implement-read-only-auth branch 4 times, most recently from 0ad45b0 to 0fd37a9 Compare January 16, 2025 20:51

rolyatmax marked this pull request as ready for review January 16, 2025 21:17

vercel bot deployed to Preview – y-sweet-gendocs January 16, 2025 21:33 View deployment

vercel bot deployed to Preview – y-sweet-demos January 16, 2025 21:35 View deployment

vercel bot deployed to Preview – y-sweet-debugger January 16, 2025 21:36 View deployment

rolyatmax commented Jan 16, 2025

View reviewed changes

rolyatmax added 12 commits January 17, 2025 11:18

drop metadata support from y-sweet server

48f5629

make read-only JSON match the client library types and drop Authoriza…

96fb2f6

…tion::Nothing variant

encode the authorization type into the token payload

81a9bb6

check authorization when handling message

b66933c

refactor to pull authorization from plane headers in single-doc mode …

63dba80

…and to pull it from a token in full-server mode

drop y-sweet-worker clippy check from github workflows

b77b55b

reordering tests for better organization

f0cba4e

fix broken test

02214e0

drop support for testing y-sweet-worker

88f9c99

add tests for read-only token functionality

0cdb6af

add tests for single-doc-server

0c23798

return authorization field as part of ClientToken

e80aea0

rolyatmax force-pushed the taylor/dis-3111-y-sweet-server-implement-read-only-auth branch from 45d6f00 to e80aea0 Compare January 17, 2025 16:18

vercel bot deployed to Preview – y-sweet-debugger January 17, 2025 16:19 View deployment

vercel bot deployed to Preview – y-sweet-demos January 17, 2025 16:20 View deployment

vercel bot deployed to Preview – y-sweet-gendocs January 17, 2025 16:21 View deployment

paulgb reviewed Jan 17, 2025

View reviewed changes

paulgb approved these changes Jan 17, 2025

View reviewed changes

crates/y-sweet-core/src/api_types.rs Outdated Show resolved Hide resolved

crates/y-sweet-core/src/api_types.rs Outdated Show resolved Hide resolved

changes per comments: drop unnecessary code

fc50b41

rolyatmax merged commit 5032427 into main Jan 17, 2025
11 checks passed

rolyatmax deleted the taylor/dis-3111-y-sweet-server-implement-read-only-auth branch January 17, 2025 17:17

rolyatmax mentioned this pull request Jan 17, 2025

Read-only client tokens #307

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

implement read-only auth #373

implement read-only auth #373

rolyatmax commented Jan 15, 2025 •

edited

Loading

vercel bot commented Jan 15, 2025 •

edited

Loading

rolyatmax Jan 15, 2025

rolyatmax Jan 15, 2025

rolyatmax Jan 15, 2025

jakelazaroff Jan 15, 2025

rolyatmax Jan 16, 2025

rolyatmax Jan 16, 2025

paulgb Jan 17, 2025

rolyatmax Jan 17, 2025

rolyatmax Jan 17, 2025 •

edited

Loading

ellipsis-dev bot commented Jan 17, 2025

ellipsis-dev bot commented Jan 17, 2025

paulgb left a comment

implement read-only auth #373

implement read-only auth #373

Conversation

rolyatmax commented Jan 15, 2025 • edited Loading

vercel bot commented Jan 15, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rolyatmax Jan 17, 2025 • edited Loading

Choose a reason for hiding this comment

ellipsis-dev bot commented Jan 17, 2025

ellipsis-dev bot commented Jan 17, 2025

paulgb left a comment

Choose a reason for hiding this comment

rolyatmax commented Jan 15, 2025 •

edited

Loading

vercel bot commented Jan 15, 2025 •

edited

Loading

rolyatmax Jan 17, 2025 •

edited

Loading