Code review comment for lp:~jaboing/canonical-identity-provider/sso

Revision history for this message
Ricardo Kirkner (ricardokirkner) wrote :

A few comments and questions

- why not refactor the if statement into the register_account helper? (the register_account helper would then be 'smart' enough to created the account if needed or log in with the sanctified account if not); this also reduces the changes required

- l.141-142: the double call to delete_device is intentional?

- removing the TWOFACTOR flag will make this run everywhere (dev, staging, prod), so are we sure these tests pass against staging/prod?

- in login_to_isdqa_account you create the account if it's missing; but I don't see how this new account gets the necessary twofactor bits enabled (unless register_account does this by default). Why can't we always call login_to_isdqa_account, both on staging/prod and dev, and get rid of the if statement alltogether?

« Back to merge proposal