Code review comment for lp:~james-w/lava-dashboard/openid

Revision history for this message
Zygmunt Krynicki (zyga) wrote :

I see the following issues:

1) Launchpad SSO does not send an email address, the user should be prompted to give one IMHO. I suspect that this could be simplified if we allow lauchpad to "trust" this application but I don't want to assume we can do that.

2) There should be an option to sign in with username and password for 'model' backend. I don't think we can assume each installation will just use openid for everything.

3) There should be an option to sign in with alternate provider. Or at the very least the log in page should say "sign in with launchpad account" or something like that.

As for testing the project. I don't know if you can test projects, I'll check it out and get back to this. It's actually the same problem as we had with CSRF checks. It's not a part of the application (although that's where we currently placed this).

Other than that it's great to see this running :-)
Great work James!

« Back to merge proposal