App passwords #826
Merged
App passwords #826
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
devinivy
reviewed
Apr 17, 2023
devinivy
reviewed
Apr 17, 2023
devinivy
reviewed
Apr 17, 2023
| @@ -0,0 +1,10 @@ | |||
| export interface AppPassword { | |||
| accountDid: string | |||
There was a problem hiding this comment.
No issue here at all, but I'm interested to hear whether there's any special reason this is accountDid rather than just did, as you might find in some of the other tables.
There was a problem hiding this comment.
no good reason lol. i'll update it 👍
devinivy
reviewed
Apr 17, 2023
| ) | ||
| }) | ||
|
|
||
| it('restricts certain actions', async () => { |
There was a problem hiding this comment.
Just double-checking that createAppPassword is the only method we want to restrict in this PR.
There was a problem hiding this comment.
I think so? I can't think of any others besides maybe revokeAppPassword
Honestly I'm not even sure we need to restrict createAppPassword 🤔
deleteAccount requires you to add in your account password anyways
There was a problem hiding this comment.
I think it's a good call to restrict createAppPassword 👍
devinivy
reviewed
Apr 17, 2023
| @@ -0,0 +1,103 @@ | |||
| import AtpAgent from '@atproto/api' | |||
There was a problem hiding this comment.
One other case I can think of that might be nice in here is that tokens received after a refresh continue to be restricted as app-password credentials.
|
APIs lgtm from the app perspective |
Co-authored-by: devin ivy <[email protected]>
Co-authored-by: devin ivy <[email protected]>
devinivy
approved these changes
Apr 18, 2023
dholms
added a commit
that referenced
this pull request
Apr 18, 2023
* app password lex & auth chnages * scrypt things * implemented app password refresh tokens * db tidy & migration * revocation + bugfixin * tests, listing passwords & cleanup * Update packages/pds/src/db/scrypt.ts Co-authored-by: devin ivy <[email protected]> * Update packages/pds/src/db/scrypt.ts Co-authored-by: devin ivy <[email protected]> * pr feedback --------- Co-authored-by: devin ivy <[email protected]>
dholms
added a commit
that referenced
this pull request
Apr 23, 2023
* moderation tweks * fix: typo. (#818) * Create invite codes for many accounts (#825) * create invite codes for many accounts * test * Revamp dev env (#796) * fix up a couple of tsc errors in app view merge * wip * use dev-env for appview tests * process all in blob resolver * another test fix * fixed missed merge conflict * fix one more merge conflict * fix popular test * Sync test env with labeler changes (#836) fix test-env * Remove extraneous info on getRecord (#835) * remove extraneous info on getRecord * fix test-env * Fix duplication of constants in the crypto package (#819) * App passwords (#826) * app password lex & auth chnages * scrypt things * implemented app password refresh tokens * db tidy & migration * revocation + bugfixin * tests, listing passwords & cleanup * Update packages/pds/src/db/scrypt.ts Co-authored-by: devin ivy <[email protected]> * Update packages/pds/src/db/scrypt.ts Co-authored-by: devin ivy <[email protected]> * pr feedback --------- Co-authored-by: devin ivy <[email protected]> * remove profile labels * rm temp branch from workflow --------- Co-authored-by: Devin Ivy <[email protected]> Co-authored-by: S. Ota <[email protected]> Co-authored-by: Ilya Siamionau <[email protected]>
mloar
pushed a commit
to mloar/atproto
that referenced
this pull request
Nov 15, 2023
* app password lex & auth chnages * scrypt things * implemented app password refresh tokens * db tidy & migration * revocation + bugfixin * tests, listing passwords & cleanup * Update packages/pds/src/db/scrypt.ts Co-authored-by: devin ivy <[email protected]> * Update packages/pds/src/db/scrypt.ts Co-authored-by: devin ivy <[email protected]> * pr feedback --------- Co-authored-by: devin ivy <[email protected]>
mloar
pushed a commit
to mloar/atproto
that referenced
this pull request
Nov 15, 2023
* moderation tweks * fix: typo. (bluesky-social#818) * Create invite codes for many accounts (bluesky-social#825) * create invite codes for many accounts * test * Revamp dev env (bluesky-social#796) * fix up a couple of tsc errors in app view merge * wip * use dev-env for appview tests * process all in blob resolver * another test fix * fixed missed merge conflict * fix one more merge conflict * fix popular test * Sync test env with labeler changes (bluesky-social#836) fix test-env * Remove extraneous info on getRecord (bluesky-social#835) * remove extraneous info on getRecord * fix test-env * Fix duplication of constants in the crypto package (bluesky-social#819) * App passwords (bluesky-social#826) * app password lex & auth chnages * scrypt things * implemented app password refresh tokens * db tidy & migration * revocation + bugfixin * tests, listing passwords & cleanup * Update packages/pds/src/db/scrypt.ts Co-authored-by: devin ivy <[email protected]> * Update packages/pds/src/db/scrypt.ts Co-authored-by: devin ivy <[email protected]> * pr feedback --------- Co-authored-by: devin ivy <[email protected]> * remove profile labels * rm temp branch from workflow --------- Co-authored-by: Devin Ivy <[email protected]> Co-authored-by: S. Ota <[email protected]> Co-authored-by: Ilya Siamionau <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implement routes for:
Access tokens from app passwords have a different scope on them (which is a subset of the general "access") & the auth verifier for most routes is augmented to allow requests from both scopes. Some routes - such as account deletion or creating new app passwords - do not allow requests from app passwords.
At the DB layer, we scrypt app passwords using the user's did as a salt. This prevents us from having to scrypt a password (costly operation) many times for many different salts. This still protects against rainbow table attacks.