Account migration schemas #2170

dholms · 2024-02-12T21:15:54Z

Think there's a good question around how these schemas are grouped. Should they all live under a com.atproto.migration interface? Or spread out as they are to their relevant category?

lexicons/com/atproto/identity/signPlcOp.json

devinivy · 2024-02-13T20:28:13Z

lexicons/com/atproto/repo/checkMissingBlobs.json

@@ -0,0 +1,36 @@
+{
+  "lexicon": 1,
+  "id": "com.atproto.repo.listMissingBlobs",


After seeing this, I could imagine moving migration-specific endpoints like this into their own segment, e.g. com.atproto.migrate.listMissingBlobs.

I actually think this is a useful route in it's own right fwiw. There are some repos now that are missing blobs and this could be useful to figure out what

does this let external folks discover which blobs have been purged for an account? or do purged blobs not show up in this output?

(dholms replied below; I think the reason the github reply box didn't show in my review is that this comment was on the same line that divy commented on previously?)

devinivy · 2024-02-13T20:30:31Z

lexicons/com/atproto/server/checkAccountStatus.json

+            "didMigrated": { "type": "boolean" },
+            "uploadedRepo": { "type": "boolean" },
+            "importedRepo": { "type": "boolean" },
+            "importRepoError": { "type": "string" },


It could be nice to make room for an error message and a code/name here.

I actually had a change of heart and decided against processing the repo async, that way we can just report the error directly in the request

hmm! that is a big change (not being async). it does feel like async would be more general and reliable than keeping a long HTTP request open, and enable a nice dynamic progress bar UI.

but we could add that as an alternative endpoint later.

bnewbold

left a whole lot of comments. mostly just sharing thoughts.

overall pretty excited, this is looking great!

bnewbold · 2024-02-14T07:03:28Z

lexicons/com/atproto/identity/getRecommendedDidCredentials.json

+        "schema": {
+          "type": "object",
+          "properties": {
+            "rotationKeys": {


it is a tiny bit weird having rotationKeys here. it matches the PLC operation syntax/fields, but the other fields here match DID document fields but this one doesn't. I guess it's fine? plcRotationKeys is tempting but probably not worth breaking consistency.

a description on this field (in the Lexicon schema) would be good.

Yeah I agree :/ But I think it makes most sense to return them here & keep it consistent with the other routes.

One thing we can do that may make it go down a bit easier is not return rotationKeys when this route is requested by a did:web. If we add some additional method that requires another field, we can add another property to the response this would be undefined for plc & web dids

nice! that is a good way to handle it.

lexicons/com/atproto/identity/sendPlcOp.json

bnewbold · 2024-02-14T07:08:03Z

lexicons/com/atproto/identity/signPlcOp.json

+  "defs": {
+    "main": {
+      "type": "procedure",
+      "description": "Signs a PLC operation to update some value(s) in the requesting DID's document.",


does this endpoint also the same safety checks as sendPlcOp, or nah?

No it doesn't, because it's used to sign an operation for a possibly different service. Though that reminds me, I do intend to put it behind email verification 👍

bnewbold · 2024-02-14T07:09:44Z

lexicons/com/atproto/identity/sendPlcOp.json

+        "encoding": "application/json",
+        "schema": {
+          "type": "object",
+          "required": ["op"],


I would probably expand all these "op" to "operation". eg the plc directory APIs have full "operation".

but not a strong feeling.

eh yeah probably a good idea. generally we don't shoot for brevity in lexicon 😛

bnewbold · 2024-02-14T07:10:07Z

lexicons/com/atproto/identity/sendPlcOp.json

+  "defs": {
+    "main": {
+      "type": "procedure",
+      "description": "Validates a PLC operation to ensure that it doesn't violate a service's constraints or get the identity into a bad state, then submits it to the PLC registry",


should we allow a param to skip validation, like we do for lexicons?

I'd be fine not making the validation optional to start, but maybe add it in the future.

In my view, if you want to do that, you should just send the operation to PLC yourself. But yeah we can always add it later if we decide we want it 👍

yes! good call

bnewbold · 2024-02-14T07:16:44Z

lexicons/com/atproto/server/deactivateAccount.json

+  "defs": {
+    "main": {
+      "type": "procedure",
+      "description": "Deactivates a currently active account. Used to finalize account migration with the old host after the account has been activated on the new host.",


will we open this up to be the way for folks to temporarily deactivate/reactivate accounts as an account status thing? not sure if described the way it is to set near-term or long-term expectations.

I think yes, but we shouldn't advertise it as a feature just yet

bnewbold · 2024-02-14T07:18:18Z

lexicons/com/atproto/server/getServiceAuth.json

+      "parameters": {
+        "type": "params",
+        "required": ["aud"],
+        "properties": {


what about expiration (exp)? should client be allowed to request longer/shorter tokens within some range?

Makes me a bit uneasy honestly - I don't think we want to offer long versions of these tokens. If you request one, you should really only be using it once

I definitely think the returned token should have some exp; I guess we can just set it to 5-10min in the future or something like that? Single-use in addition is good, I just don't like the idea of a token floating around that could be used (for the first time) the next day or something.

Oh to clarify - the returned token does have an exp and it's set to 60s 👍

We just don't provide optionality around requesting a longer one

bnewbold · 2024-02-14T07:19:32Z

lexicons/com/atproto/server/getServiceAuth.json

+        "properties": {
+          "aud": {
+            "type": "string",
+            "format": "did",


if we make this format: uri it would work with DIDs and leave the door open to other audience URIs/values... but probably best to keep it DID-only.

I like just leaving it DID for now until we see the need for that. I think we can upgrade into format: uri without much trouble

bnewbold · 2024-02-14T07:21:32Z

lexicons/com/atproto/repo/checkMissingBlobs.json

@@ -0,0 +1,36 @@
+{
+  "lexicon": 1,
+  "id": "com.atproto.repo.listMissingBlobs",


does this let external folks discover which blobs have been purged for an account? or do purged blobs not show up in this output?

bnewbold · 2024-02-14T07:23:09Z

lexicons/com/atproto/repo/checkMissingBlobs.json

+            "cursor": { "type": "string" },
+            "cids": {
+              "type": "array",
+              "items": { "type": "string", "format": "cid" }


I don't know if it is easy or worth it, but if this returned blob objects (eg, with size, mimetype, etc, in addition to CID), might be more ergonomic and helpful for humans. and machines could do things like compute total size of missing objects.

hmmm that's not a bad idea 🤔 down to take a stab at it

And to answer the prev question (which GH isn't letting me respond to for some reason), this is not accessible to external accounts, but purged blobs would not show up in the output

Updated it to the blob type (similar to output of uploadBlob) which contains all that info 👍

bnewbold · 2024-02-15T01:55:53Z

all looking good! I left follow-up comments on importRepo and authtoken sections.

devinivy · 2024-02-20T06:00:01Z

lexicons/com/atproto/server/describeServer.json

@@ -9,7 +9,7 @@
        "encoding": "application/json",
        "schema": {
          "type": "object",
-          "required": ["availableUserDomains"],
+          "required": ["did", "availableUserDomains"],


Ooh this is a sneaky little breaking change we need to be a little careful with for rollout.

yup yup - just need to ensure we update all our PDSs before publishing this new agent library

thought it was important to get in before there are other PDSs out in the wild

funny, this bit me too, only just now realized I needed to add it. bsky.app was unhappy with my PDS just now until I did. 😆

bluesky-social/atproto#2170 (review)

dholms added 7 commits February 12, 2024 15:15

draft of account migration lexicons

e737f20

format

268af2e

clean up schemas

dcfd9f7

second pass on schemas

3feb76c

small fix

6be044b

move around checkImportStatus

a51dcae

getServiceAuth

a2f2942

devinivy reviewed Feb 13, 2024

View reviewed changes

lexicons/com/atproto/identity/signPlcOp.json Outdated Show resolved Hide resolved

devinivy reviewed Feb 13, 2024

View reviewed changes

dholms added 2 commits February 13, 2024 18:46

schema tweaks

a6da60a

format

8a985ad

bnewbold reviewed Feb 14, 2024

View reviewed changes

update schemas

5822444

dholms added 2 commits February 14, 2024 20:21

email flow for signed plc operation

40890dc

refactor listMissingBlobs a bit

11b4116

dholms mentioned this pull request Feb 15, 2024

Account migration #2179

Merged

dholms added 2 commits February 15, 2024 17:06

codegen

799ccab

Merge branch 'main' into account-migration-lexicons

64cecbc

devinivy reviewed Feb 20, 2024

View reviewed changes

dholms added 2 commits February 20, 2024 19:14

return did in describeServer

409f7b3

changeset

c7942a0

dholms merged commit 4c511b3 into main Feb 21, 2024
10 checks passed

dholms deleted the account-migration-lexicons branch February 21, 2024 01:23

github-actions bot mentioned this pull request Feb 21, 2024

Version packages #2202

Merged

snarfed added a commit to snarfed/arroba that referenced this pull request Mar 11, 2024

add new required did field to describeServer output

c86334d

bluesky-social/atproto#2170 (review)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Account migration schemas #2170

Account migration schemas #2170

dholms commented Feb 12, 2024 •

edited

Loading

devinivy Feb 13, 2024

dholms Feb 14, 2024

bnewbold Feb 14, 2024

bnewbold Feb 15, 2024

devinivy Feb 13, 2024

dholms Feb 14, 2024

bnewbold Feb 15, 2024

bnewbold left a comment

bnewbold Feb 14, 2024

dholms Feb 14, 2024

bnewbold Feb 15, 2024

bnewbold Feb 14, 2024

dholms Feb 14, 2024

bnewbold Feb 14, 2024

dholms Feb 14, 2024

bnewbold Feb 14, 2024

dholms Feb 14, 2024

bnewbold Feb 15, 2024

bnewbold Feb 14, 2024

dholms Feb 14, 2024

bnewbold Feb 14, 2024

dholms Feb 14, 2024

bnewbold Feb 15, 2024 •

edited

Loading

dholms Feb 15, 2024

bnewbold Feb 14, 2024

dholms Feb 14, 2024

bnewbold Feb 14, 2024

bnewbold Feb 14, 2024

dholms Feb 14, 2024

dholms Feb 14, 2024

bnewbold commented Feb 15, 2024

devinivy Feb 20, 2024

dholms Feb 21, 2024

snarfed Mar 11, 2024

Account migration schemas #2170

Account migration schemas #2170

Conversation

dholms commented Feb 12, 2024 • 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

bnewbold left a 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

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

bnewbold Feb 15, 2024 • 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

bnewbold commented Feb 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dholms commented Feb 12, 2024 •

edited

Loading

bnewbold Feb 15, 2024 •

edited

Loading