Conversation
aduth
requested review from
jmhooper,
mitchellhenke,
nprimak,
peggles2 and
solipet
aduth
commented
Apr 11, 2022
aduth
commented
Apr 11, 2022
aduth
force-pushed
the
aduth-verify-api-route
branch
from
6a81f95 to
bee7044
Compare
aduth
force-pushed
the
aduth-secret-store
branch
from
c7229dc to
ee19f04
Compare
aduth
commented
Apr 12, 2022
aduth
force-pushed
the
aduth-verify-api-route
branch
2 times, most recently
from
fc7fb1b to
446e4b5
Compare
aduth
added a commit
that referenced
this pull request
Apr 14, 2022
To avoid magic number and make it clearer what's happening #6183 (comment)
aduth
force-pushed
the
aduth-secret-store
branch
from
80aa9f2 to
4ebaf09
Compare
jmhooper
reviewed
Apr 21, 2022
aduth
force-pushed
the
aduth-secret-store
branch
from
4ebaf09 to
1d7409b
Compare
aduth
added a commit
that referenced
this pull request
Apr 21, 2022
To avoid magic number and make it clearer what's happening #6183 (comment)
aduth
added a commit
that referenced
this pull request
Apr 21, 2022
aduth
force-pushed
the
aduth-secret-store
branch
from
1d7409b to
2848802
Compare
aduth
changed the title
**Why**: As a demonstration of secure client-side storage decrypted with key provided by server per session. changelog: Upcoming Features, Identity Proofing, Add client-side encrypted storage
To avoid magic number and make it clearer what's happening #6183 (comment)
- Avoid waiting to render the app - Manage subscribers automatically via context value change
Consistency #6183 (comment)
Avoid dependency cycle, make room for more index-exported
Route now authenticated
Contributor
Author
|
Marking this ready for review. We don't currently store anything in it, so it's a bit difficult to review in its current form. The plan would be to use this for "Password Entry" -> "Personal Key" submission so that we can store and restore the personal key received from the API response. Also, originally I planned to move away from using the session to store the encryption key, but ultimately I think it should be fine as it is? Open to feedback. |
zachmargolis
approved these changes
May 4, 2022
Comment on lines
+82
to
+86
| <StepIndicatorStep title="Getting Started" status={StepStatus.COMPLETE} /> | ||
| <StepIndicatorStep title="Verify your ID" status={StepStatus.COMPLETE} /> | ||
| <StepIndicatorStep title="Verify your personal details" status={StepStatus.COMPLETE} /> | ||
| <StepIndicatorStep title="Verify phone or address" status={StepStatus.COMPLETE} /> | ||
| <StepIndicatorStep title="Secure your account" status={StepStatus.CURRENT} /> |
Contributor
There was a problem hiding this comment.
Is translating these titles is a "later" item?
Contributor
Author
There was a problem hiding this comment.
😬
While it wasn't really intended as part of the changes here (mostly moved file verbatim), and while it was probably a "later" item at the time of original implementation in #6187, admittedly I kinda forgot about it, and probably should have translated it already.
I'll do a quick follow-on after this is merged.
See: https://github.com/18F/identity-idp/pull/6183/files#r865355166 Co-Authored-By: Zach Margolis <zbmargolis@gmail.com>
peggles2
pushed a commit
that referenced
this pull request
May 5, 2022
* Implement client session secret store **Why**: As a demonstration of secure client-side storage decrypted with key provided by server per session. changelog: Upcoming Features, Identity Proofing, Add client-side encrypted storage * Collapse readStorage try blocks * Clarify AES cipher key/iv generation To avoid magic number and make it clearer what's happening #6183 (comment) * Simplify cipher assignment logic * Refactor SecretsContextProvider as observable initializer - Avoid waiting to render the app - Manage subscribers automatically via context value change * Rename encode as s2ab Consistency #6183 (comment) * iv per encrypt #6183 (comment) * Make setItem await-able * Reference crypto consistently from window object * Add SecretSessionStorage inline comment docs * Add SecretSessionStorage specs * Merge useSecretValue to context implementation * Remove demo value from SecretValues * Add docs for SecretsContext * Use flow values as secrets * Split VerifyFlow from index Avoid dependency cycle, make room for more index-exported * Destructure storeKey in same way as other data attributes * Use user_session instead of session Route now authenticated * Inline encryption cipher initialization to memoized session assignment See: https://github.com/18F/identity-idp/pull/6183/files#r865355166 Co-Authored-By: Zach Margolis <zbmargolis@gmail.com> Co-authored-by: Zach Margolis <zbmargolis@gmail.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Why: As a demonstration of secure client-side storage decrypted with key provided by server per session.
What:
sessionStorageFor demonstration, current implementation usesEdit: Review-ready version still usessession, though a final implementation could likely store this as associated with the user in thedocument_capture_sessionstableuser_session. We could revisit this later, but session should suffice.