mozilla / PyFxA

Python library for interacting with the Firefox Accounts ecosystem
Other
31 stars 20 forks source link

Support sending and logging in with an unblock code #56

Closed kewisch closed 6 years ago

kewisch commented 7 years ago

This should fix it, I've tested it and it works in practice.

The unit test I have not tested and I would appreciate some guidance on. I don't think it is possible to put the server into a state where the account is blocked so the flow can be tested.

kewisch commented 7 years ago

Ok now, the remaining test failures are probably not mine. Let me know if there is anything I should change or do to get this merged.

rfk commented 7 years ago

Thanks @kewisch! Just to le tyou know, I'm on holiday for the coming week so won't be able to take a proper look until then, but I'll keep this in my inbox to review when I return.

kewisch commented 6 years ago

The commented out test works for me locally when running it directly. I wasn't able to complete a full test run due to timeouts and request errors, I hope travis does a better job. I've also fixed a few flake8 errors and rebased against master.

kewisch commented 6 years ago

The failing tests seem to be something else. I got further locally by replacing restmail.com with restmail.net in code I didn't touch, but another test is timing out trying to contact verifier.login.persona.org. @rfk can you check and see if this is something else?

rfk commented 6 years ago

but another test is timing out trying to contact verifier.login.persona.org

That's definitely unrelated, and due to browserid infra being shut down. I'm going to make a new release of PyBrowserID that will hopefully fix the problem by defaulting to a local verifier rather than the (no longer running) remote verifier.

rfk commented 6 years ago

Kicking off a rebuild, let's see if the new PyBrowserID release helps things...

Natim commented 6 years ago

Kicking off a rebuild, let's see if the new PyBrowserID release helps things...

Yes much better only a mocked tests failed and I guess we can update it since it seems related to that PR.

kewisch commented 6 years ago

Thanks, this was easy enough to fix now that it was not hidden in the other failures. Checks pass now.

rfk commented 6 years ago

LGTM, thanks @kewisch!