Bug 1868418 - Rerun ensure capabilities after logging in#7457
Conversation
|
I think I got the state machine stuff right, but please double-check my work here. |
34eacc8 to
5fd30b9
Compare
Moved the code to RetryingAccount, which should give us this functionality for free. Also, tweaked the fxa-client example a bit.
5fd30b9 to
be1a391
Compare
skhamis
left a comment
There was a problem hiding this comment.
Nice fix! I think we're close but just one question about the error part
| Ok(if active { S::Connected } else { S::AuthIssues }) | ||
| } | ||
| match account.finish_initialize(&device_config.capabilities) { | ||
| Ok(()) => Ok(S::Connected), |
There was a problem hiding this comment.
One thing worth checking: in the old code we land at S::AuthIssues when check_authorization_status comes back false. The new code propagates the original FxaError::Authentication to the Err(cause) arm, which sends us to S::Disconnected?. For someone who is signed-in and whose refresh token is server-invalidated (password-changed elsewhere), I think our AuthIssues preserves the account state and lets consumers show the "re-auth" flow. Disconnected will put them back to full sign-in right?
Should we do something like:
match account.finish_initialize(&device_config.capabilities) {
Ok(()) => Ok(S::Connected),
Err(cause) => {
let target = if matches!(cause.get_error_handling().err, FxaError::Authentication) {
S::AuthIssues
} else {
S::Disconnected
};
Err(StateMachineErr::new(cause, target))
}
}
or something similar? However if i'm missing something here please let me know!
Moved the code to RetryingAccount, which should give us this functionality for free.
Pull Request checklist
[ci full]to the PR title.