| Message ID | 20150311213935.GA11007@mx.loc |
|---|---|
| State | Superseded |
| Delegated to: | Stephen Finucane |
| Headers | show |
On Wed, Mar 11, 2015 at 10:39:35PM +0100, Bernhard Reutner-Fischer wrote: > Hi, > > I am attaching an old patch to add a password-reset knob to the login > page. If anyone thinks this is a good idea then it should be thoroughly > tested against current django, i do remember that it worked like > intended back then. > > Apparently i never managed to send this one? > > thanks, This doesn't apply cleanly to the current HEAD. Mind rebasing it and we can get it merged/close of the TODO item? Cheers, Stephen
On 17 October 2015 at 00:57, Finucane, Stephen <stephen.finucane@intel.com> wrote: > On Wed, Mar 11, 2015 at 10:39:35PM +0100, Bernhard Reutner-Fischer > wrote: >> Hi, >> >> I am attaching an old patch to add a password-reset knob to the login >> page. If anyone thinks this is a good idea then it should be >> thoroughly >> tested against current django, i do remember that it worked like >> intended back then. >> >> Apparently i never managed to send this one? >> >> thanks, > > This doesn't apply cleanly to the current HEAD. Mind rebasing it and > we can get it > merged/close of the TODO item? Eh, sneaky mention of TODO ;) Ok, so i refurbished the "password reset" link, including pretty patchwork templates instead of the default django admin ones. Then i wanted to drop it off the docs/TODO list, but alas there was no such item! What i found, however, was that somebody asked for being able to change their email address which sounds like a sane thing to do. Likewise for your last name (not sure about the first name but it comes for free). One thing that i wonder is if the patchwork_emailconfirmation is supposed to be able to hold duplicate user_id/email entries like it currently does. I do not think this makes sense and should IMHO be forbidden unless it serves a purpose i cannot see? Note that the html-ui does not display duplicate entries although they can be entered in the html-ui and do end up in the database. I wouldn't allow that. Some more todos: * docs/requirements-base.txt could mention psycopg2 along MySQL-python==1.2.5, thus patchwork/settings/dev.py should add a + #'ENGINE': 'django.db.backends.postgresql_psycopg2', thanks, > > Cheers, > Stephen Bernhard Reutner-Fischer (5): login: add link to password reset Allow changing primary email address for accounts tests/test_user: Remove duplicate test TODO: cleanup deprecated unittest aliases .gitignore: patchwork/settings/production.py .gitignore | 3 ++ docs/TODO | 3 +- patchwork/forms.py | 14 +++++ patchwork/models.py | 3 ++ patchwork/templates/patchwork/login.html | 7 ++- patchwork/templates/patchwork/profile.html | 18 ++++++- patchwork/templates/patchwork/user-data.html | 34 ++++++++++++ patchwork/tests/test_user.py | 61 +++++++++++++++++++++- patchwork/urls.py | 10 ++++ patchwork/views/user.py | 42 ++++++++++++++- .../registration/password_reset_complete.html | 9 ++++ templates/registration/password_reset_confirm.html | 51 ++++++++++++++++++ templates/registration/password_reset_done.html | 13 +++++ templates/registration/password_reset_email.html | 12 +++++ templates/registration/password_reset_form.html | 45 ++++++++++++++++ 15 files changed, 320 insertions(+), 5 deletions(-) create mode 100644 patchwork/templates/patchwork/user-data.html create mode 100644 templates/registration/password_reset_complete.html create mode 100644 templates/registration/password_reset_confirm.html create mode 100644 templates/registration/password_reset_done.html create mode 100644 templates/registration/password_reset_email.html create mode 100644 templates/registration/password_reset_form.html
> On 17 October 2015 at 00:57, Finucane, Stephen <stephen.finucane@intel.com> > wrote: > > On Wed, Mar 11, 2015 at 10:39:35PM +0100, Bernhard Reutner-Fischer > > wrote: > >> Hi, > >> > >> I am attaching an old patch to add a password-reset knob to the login > >> page. If anyone thinks this is a good idea then it should be > >> thoroughly > >> tested against current django, i do remember that it worked like > >> intended back then. > >> > >> Apparently i never managed to send this one? > >> > >> thanks, > > > > This doesn't apply cleanly to the current HEAD. Mind rebasing it and > > we can get it > > merged/close of the TODO item? > > Eh, sneaky mention of TODO ;) A non-existent TODO, no less! Damn, I'm good :P > Ok, so i refurbished the "password reset" link, including pretty > patchwork templates instead of the default django admin ones. > Then i wanted to drop it off the docs/TODO list, but alas there was no > such item! > What i found, however, was that somebody asked for being able to change > their email address which sounds like a sane thing to do. Likewise for > your last name (not sure about the first name but it comes for free). Perfect. I'll review as soon as I can and merge if I'm happy and post feedback if I've any concerns. Appreciate the resubmission here. > One thing that i wonder is if the patchwork_emailconfirmation is > supposed to be able to hold duplicate user_id/email entries like it > currently does. I do not think this makes sense and should IMHO be > forbidden unless it serves a purpose i cannot see? > Note that the html-ui does not display duplicate entries although they > can be entered in the html-ui and do end up in the database. I wouldn't > allow that. If I understand you correctly (and I'm not sure if I do...), you're saying there is no check done to ensure a user signing up does not use a username or email that has already been taken? If so, that's a big issue and needs to be fixed. > Some more todos: > * docs/requirements-base.txt could mention psycopg2 along > MySQL-python==1.2.5, thus patchwork/settings/dev.py should add a > + #'ENGINE': 'django.db.backends.postgresql_psycopg2', Yeah, we really should make this easier. This is part of a bigger question over how to specify dependencies and spin up a development environment in general. You could add a load of new requirements files but that's not very pretty - who wants to see eight different requirements files for all the combinations of Django and DB we support, after all? I'm working on an Ansible+Vagrant setup which will automatically use PostgreSQL and as such minimize this issue (I'll submit soon as I complete the series feature) but I'm open to ideas. Stephen PS: Just to clarify, we now support at least following: * Django: 1.6, 1.7 and 1.8. We'll add 1.9 support soon as it's out. * DB: MySQL and PostgreSQL. SQLite is possible but you shouldn't use it for development (test using what you deploy against - this is highlighted by the failing of some unit tests due to some case sensitivity issues, iirc)
On November 11, 2015 5:24:36 AM GMT+01:00, "Finucane, Stephen" <stephen.finucane@intel.com> wrote: >> On 17 October 2015 at 00:57, Finucane, Stephen ><stephen.finucane@intel.com> >> wrote: >> > On Wed, Mar 11, 2015 at 10:39:35PM +0100, Bernhard Reutner-Fischer >> > wrote: >> One thing that i wonder is if the patchwork_emailconfirmation is >> supposed to be able to hold duplicate user_id/email entries like it >> currently does. I do not think this makes sense and should IMHO be >> forbidden unless it serves a purpose i cannot see? >> Note that the html-ui does not display duplicate entries although >they >> can be entered in the html-ui and do end up in the database. I >wouldn't >> allow that. > >If I understand you correctly (and I'm not sure if I do...), you're >saying >there is no check done to ensure a user signing up does not use a >username >or email that has already been taken? If so, that's a big issue and >needs >to be fixed. No, I mean the notification email addresses, you can enter dups and use the address of another user-account there (results in a conformation mail being sent). See?
From 9b454613dcf4a30a04ab290c0275f5d04bdbf7e0 Mon Sep 17 00:00:00 2001 From: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> Date: Wed, 1 Dec 2010 22:30:58 +0100 Subject: [PATCH 2/2] login: add link to password reset Signed-off-by: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> --- templates/registration/login.html | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/templates/registration/login.html b/templates/registration/login.html index 2dfc2a7..662675c 100644 --- a/templates/registration/login.html +++ b/templates/registration/login.html @@ -18,9 +18,12 @@ {% endif %} {{ form }} <tr> - <td colspan="2" class="submitrow"> + <td class="submitrow"> <input type="submit" value="Login"/> </td> + <td class="submitrow"> + <a href="{% url django.contrib.auth.views.password_reset %}">Forgot password?</a> + </td> </tr> </table> </form> -- 1.7.2.3