users should be able to change their passwords on the identity service. this diff introduces the action that will be dispatched from native and web. it also includes the changes to the grpc-web client.
Details
Details
successfully dispatched the new action from native and web. then successfully logged in to identity with the new password.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
Requesting review to get help resolving the issue described in the inline comment
| web/grpc/identity-service-client-wrapper.js | ||
|---|---|---|
| 107 ↗ | (On Diff #35563) | this is giving me a flow error because the Error constructor only expects one argument. I'm trying to follow this: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/Error#rethrowing_an_error_with_a_cause |
| web/grpc/identity-service-client-wrapper.js | ||
|---|---|---|
| 79–81 ↗ | (On Diff #35563) | i'm not sure why, but flow didn't like this, so i had to declare a new variable client in changePassword |
Comment Actions
JS looks great!! Didn't review the specifics of the OPAQUE stuff but trust you have that covered
| native/identity-service/identity-service-context-provider.react.js | ||
|---|---|---|
| 67 ↗ | (On Diff #35567) | This await doesn't change the logic:
So it might be unnoticeably more efficient to not have the await. But I guess it might cause some confusion, so up to you. (I think eslint should warn us about a promise that is left without being awaited or returned, but I might be wrong) |
| native/identity-service/identity-service-context-provider.react.js | ||
|---|---|---|
| 67 ↗ | (On Diff #35567) | Thanks for explaining this... didn't realize that's how it worked. @varun can probably leave the logic as-is and undo these changes. Personally I think either way is fine, but if perf is better in your variant then that's probably best
It's actually Flow that will warn us about that |
| lib/actions/user-actions.js | ||
|---|---|---|
| 508 ↗ | (On Diff #35567) | The latest release of the whitepaper (viewable on Overleaf or GitHub) defines a protocol for password updates in Section 3.2.1.4. As part of that protocol, the user's current password is supposed to be verified. The whitepaper update happened earlier this month... sorry for not noticing earlier. I think this will require some identity service changes, and the client will need to make at least two gRPC calls... |
| native/identity-service/identity-service-context-provider.react.js | ||
|---|---|---|
| 708 ↗ | (On Diff #41456) | i'll remove this await |