+260
-37
bun.lock
+2
-1
docs/package.json
+3
docs/src/lib/oauth-client.ts
+2
-1
docs/src/lib/session.ts
+3
-3
docs/src/routes/auth.ts
+145
-16
docs/src/routes/subscribe.ts
+151
-46
packages/cli/src/components/sequoia-subscribe.js
History
3 rounds
10 comments
3 commits
expand
collapse
Also added wrangler to the devDependencies so you can just run `bun run dev:api` and everything just works.
Stores as a cookie and in local storage as a fallback. Passes from origin to sequoia.pub.
Co-Authored-By: @stevedylan.dev
expand 2 comments
Awesome!! Thanks for this work; sick feature :)
expand 7 comments
Made a separate route to check credentials. Cleaner and doesn't radically alter the behavior of the existing route. Note, however, that this doesn't work. Seems CORS is preventing it despite setting SameSite=None; Secure on the session_id cookie, which seems like it'd be safe in this case.
Using both curl and the ngrok traffic inspector (reverse-proxied for https endpoints as needed) confirm the right CORS headers are being sent from the Api endpoint, and I made sure to change my cookie settings for that endpoint but the cookie just isn't being sent it seems. I had some console.log statements in there which confirmed the DID wasn't found i.e., no session_id cookie.
I think the only reasonable way this could work is to host the script on the same origin as the /oauth and /subscribe APIs, which means either self-hosting (separate issue) or centralized hosting via sequoia.pub or at least a CDN.
I might have a solution that's not perfect, but I guess better than nothing. I realized that we could probably fetch this state client side since we are storing DID as part of the session cookie. However since we can't generally access those specific cookies, we can still store the state in local storage. An approach I took was to use the query params to store the DID as well as the subscribe state. With the DID we can make a client side call to the PDS for the site.standard.graph.subscription record, and if it matches the publication of the current site, then we show the unsubscribe button.
The downside is that since we're using local storage, clearing browser data or using another device means it will show the subscribe button, and worse cast scenario they try to create a record that already exists. I did try it out and it does work, so if you're interested I've created a diff patch file that can be applied to your branch:
curl -o unsubscribe.patch https://files.stevedylan.dev/unsubscribe.patch
git apply unsubscribe.patch
(git patches slap and I'm glad I messed around with dwm in linux to learn them lol)
Feel free to take it or leave it! Also we will need to update the OAuth scopes again to include the delete functionality for unsubscribing.
scope: "atproto repo:site.standard.graph.subscription?action=create&action=delete",
I would commit directly with the updates but since you're currently working off a fork I thought that might get messy. To that end I've added you as a collaborator on Tangled for Sequoia, so next time you need to make a PR feel free to make a branch directly on the repo and push that way :)
Thanks! I'll take a look at this tomorrow. Good call on the scopes: totally forgot about that.
The local storage idea came up when I was looking for solutions as well. That said, it at least roams with online profiles like Apple IDs, Chrome and Edge profiles, etc., right? I think so. So not terrible but, yeah, there are holes. not any worse than not having it, though. At least the flow tells you you're already sub'd so it doesn't hurt anything.
Was there a reason you deleted the /subscribe/check route? Don't we still need that for just the check, or is that the part that can't possibly work anyway? Given that, the routing to return to the original post actually was already working so I don't think modifying the URL is necessary further.
Or was the point to send those back to the publication itself for the sequoia-subscribe.js component to store in local storage?
I made that change - actually store it in a cookie via JS - and it works. New update incoming.
I think I deleted it since we switched to the client side call, but I was under the impression that was the only use case. Totally fine keeping it if we need it / want to use it later!
expand 1 comment
This includes the other PR only because I needed some of the changes therein to make the diff less messy. That other PR should be taken first then this one. Even if we make other changes to the other PR, this should keep this PR more easily mergable.
Not sure if storing it in localStorage is worth it assuming document.cookie always works. This is what Opus 4.6 wanted to do so maybe there's some good reason? If not, we could certainly clean up the code some.
Tested it with reverse-proxied sites and works well. Record gets deleted as expected and the button changes state as expected.