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. |
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!
There was a problem hiding this comment.
That seems like an improvement, but maybe it should always be S::AuthIssues. I think that's what the current code does and it seems more correct to me. WDYT?
Moved the code to RetryingAccount, which should give us this functionality for free. Also, tweaked the fxa-client example a bit.
be1a391 to
2016db7
Compare
skhamis
left a comment
There was a problem hiding this comment.
Thanks for making that change. I agree with the new direction and think its "more correct". LGTM!
|
Dependency checks seem unrelated to this PR, I didn't change |
Moved the code to RetryingAccount, which should give us this functionality for free. Also, tweaked the fxa-client example a bit.
Moved the code to RetryingAccount, which should give us this functionality for free.
Pull Request checklist
[ci full]to the PR title.