-
Notifications
You must be signed in to change notification settings - Fork 573
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
Email confirmation/update #1568
Changes from all commits
5ad3a90
f97e3ab
ea1e6cc
c4cf9fa
57aec22
5dfd97f
a80966f
96d0f01
b718599
81a3f56
e2f7a9a
b796ad2
b223271
4ec7c01
626ac31
009db47
ce2e196
4fc3b5f
f7a5db3
ddb79cb
6ac232c
95e64cb
fc63115
2b8157f
9db8c35
d7dd136
d976bba
b15974c
4db5770
d7eff88
2919e44
54a1184
0874edb
0b7439c
3de42df
1ca0ca8
50d6760
bb3f94c
c9c8d40
0982846
3bd26b1
58dc1f5
803beae
12b7a60
40f29f9
51b03dc
6027d36
91e24ff
93fea5f
607d94f
8a62d6e
e67c2d7
4e98627
0e8fb99
6348cfa
b2be947
2b0790a
df63b05
f8a9946
1db3d72
692f541
76a014d
ac913fb
09ec731
a0d467a
3ee4573
1e62fef
91c05fe
0508ae7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
--- | ||
'@atproto/dev-env': patch | ||
'@atproto/api': patch | ||
'@atproto/pds': patch | ||
--- | ||
|
||
Added email verification and update flows |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'@atproto/common-web': patch | ||
--- | ||
|
||
Added lessThanAgoMs utility |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
{ | ||
"lexicon": 1, | ||
"id": "com.atproto.server.confirmEmail", | ||
"defs": { | ||
"main": { | ||
"type": "procedure", | ||
"description": "Confirm an email using a token from com.atproto.server.requestEmailConfirmation.", | ||
"input": { | ||
"encoding": "application/json", | ||
"schema": { | ||
"type": "object", | ||
"required": ["email", "token"], | ||
"properties": { | ||
"email": { "type": "string" }, | ||
"token": { "type": "string" } | ||
} | ||
} | ||
}, | ||
"errors": [ | ||
{ "name": "AccountNotFound" }, | ||
{ "name": "ExpiredToken" }, | ||
{ "name": "InvalidToken" }, | ||
{ "name": "InvalidEmail" } | ||
] | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,7 +29,8 @@ | |
"refreshJwt": { "type": "string" }, | ||
"handle": { "type": "string", "format": "handle" }, | ||
"did": { "type": "string", "format": "did" }, | ||
"email": { "type": "string" } | ||
"email": { "type": "string" }, | ||
"emailConfirmed": { "type": "boolean" } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a gut thing, but these email-specific methods and params feel a little bit narrow to me. At the same time, I do think we should probably just roll with it. Maybe for fun worth scanning over this old approach, which includes confirmation/verification: https://github.com/bluesky-social/atproto/compare/2fa There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah. I see what you're saying 🤔 This would be fairly easy to switch over and may save us from some future deprecation |
||
} | ||
} | ||
}, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
{ | ||
"lexicon": 1, | ||
"id": "com.atproto.server.requestEmailConfirmation", | ||
"defs": { | ||
"main": { | ||
"type": "procedure", | ||
"description": "Request an email with a code to confirm ownership of email" | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
{ | ||
"lexicon": 1, | ||
"id": "com.atproto.server.requestEmailUpdate", | ||
"defs": { | ||
"main": { | ||
"type": "procedure", | ||
"description": "Request a token in order to update email.", | ||
"output": { | ||
"encoding": "application/json", | ||
"schema": { | ||
"type": "object", | ||
"required": ["tokenRequired"], | ||
"properties": { | ||
"tokenRequired": { "type": "boolean" } | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
{ | ||
"lexicon": 1, | ||
"id": "com.atproto.server.updateEmail", | ||
"defs": { | ||
"main": { | ||
"type": "procedure", | ||
"description": "Update an account's email.", | ||
"input": { | ||
"encoding": "application/json", | ||
"schema": { | ||
"type": "object", | ||
"required": ["email"], | ||
"properties": { | ||
"email": { "type": "string" }, | ||
"token": { | ||
"type": "string", | ||
"description": "Requires a token from com.atproto.sever.requestEmailUpdate if the account's email has been confirmed." | ||
} | ||
} | ||
} | ||
}, | ||
"errors": [ | ||
{ "name": "ExpiredToken" }, | ||
{ "name": "InvalidToken" }, | ||
{ "name": "TokenRequired" } | ||
] | ||
} | ||
} | ||
} |
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.
Two questions here:
createSession
endpoint by making it an opaqueidentifier
?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.
My thinking here is that it's an extra integrity check on email.
Imagine the following flow:
[email protected]
[email protected]
[email protected]
This is very unlikely. I'm not sure we need to defend against it and we may have a better mechanism by which to do it (storing that state in the db?)