On Oct 26, 10:16 pm, Brian Rosner <[email protected]> wrote:
> On Oct 26, 2009, at 8:17 PM, Nick Retallack wrote:
>
> > 1) Wouldn't it be nice if verifying your email address logged you in
> > automatically?
>
> -1. I definitely don't like this. As you mention, it isn't even
> possible unless we modify authentication. I want the action taken by
> emailconfirmation to be atomic. Letting emailconfirmation handle
> authentication too sounds like a really bad idea not only from a
> security standpoint, but from an implementation standpoint too.
Yeah but it's the expected behavior. Having to verify your email
address is annoying enough. It should be able to log you in and
complete the registration.
I agree that it shouldn't be part of the django-email-confirmation app
though. It should be part of the pinax account app. It should just
not route to that view in django-email-confirmation, and use a custom
view in the pinax account app to log you in.
Also, this could be made into a non-default configuration setting, so
you can warn people about the security risks in some nearby comments.
> > Let assume you have enabled the ACCOUNT_EMAIL_VERIFICATION setting.
> > That means that when a new user tries to sign up, they are asked to
> > check their email for the confirmation code / link. When they click
> > the link, it would be nice if that logged them in, instead of just
> > dropping them on an "Email Address Confirmed" page and hoping they
> > think to click Login in the top right at this point.
>
> This can be easily solved (if not already) by giving the user a link
> to login after completing the confirmation.
How is that any different security-wise? I'd rather not even see the
intermediate page. Just log me in.
> > 2) If #1, then all email confirmation keys for this email should be
> > consumed when used.
>
> I think this is a reasonable request. Not because of #1 as I disagree,
> but for the fact it becomes useless after the confirmation. Can you
> file a task on this particular issue?
Sure.
> > If you do go ahead and make the email confirmation log you in with the
> > associated account, the confirmations become dangerous. You can't
> > leave stale ones around. Right now, I can keep refreshing this email
> > confirmed page with the same confirmation key and it just confirms it
> > repeatedly. I can even pull out old confirmation keys and they work
> > too. I suppose the keys expire eventually, but I think it would be
> > best if they were just deleted when they are no longer useful. When
> > an email is successfully confirmed, it should delete all the
> > associated confirmation keys.
>
> Yep, I agree that upon confirmation that all keys associated to the
> confirmed e-mail address should be deleted. The current behavior is
> that you are allowed to re-send confirmation e-mails. A very likely
> situation where a user doesn't get them, but each time you re-send a
> new key is generated. This should be noted in the task you'll create
> for this issue.
I'm not sure how those things are related. There's nothing wrong with
having multiple keys lying around while you're attempting to verify
your email address, because it's always possible that some of them
don't make it. They could also arrive in the wrong order, so new keys
shouldn't invalidate old ones.
> > 3) Two users shouldn't be allowed to verify the same email address.
> > [...]
>
> I'll comment on what you've said in #3 in one go.
>
> I think you are digging yourself into a hole here. You are still
> attached to the notion that each case must be able to work with #1.
> Since I've disagreed with you there all those problems go away. I
> definitely agree there is a bug here, but your solution is just wrong
> IMO. We should not be generating the same key for each account. That
> can be fixed by introducing another value in the hash that is account
> specific. We should also be indicating in the e-mail *which* account
> that link is valid for. The code you point out that is wrong in
> ResetPasswordKeyForm works under the assumption there can only be one
> key per account. Which was the intention, but wasn't written that way.
> That is a bug which deserves a new task so we can fix it :-)
The problems don't go away though. You'd have to change the
ResetPasswordForm ("Forgot Password" feature) to make this work. It
just has one field, "Email Address", which does not identify the user
who needs a password reset. Maybe we should have you type in a
username instead of an email address. Or it could just send one email
containing reset keys for all of the accounts associated with this
email address. Which would you prefer?
> IIRC, there are concrete use cases for the single e-mail address
> across multiple accounts. I personally haven't had to deal with that,
> but I am pretty positive James Tauber has which was why the code was
> designed the way it is (though not without bugs as you've pointed
> out :-) )
And there are reasons why webmasters might want to disallow this
behavior. I think there should be a configuration option to disallow
having multiple accounts with the same email address, which should
make the app behave as I described. This is useful for people who
want to control registration on their site just a little by requiring
unique email addresses. I know it's easily subverted by using name
[email protected], but not everybody knows that. Also, we could check
for that.
> > 4) You shouldn't be allowed to just jam on that resend-verification
> > link forever.
>
> > Currently, I could add someone else's email address and then click
> > resend-verification a million times to flood their inbox with spam.
> > Or if they're using gmail which collapses these all into one message,
> > I could just hit it once a day to be sure the message never leaves
> > their sight. Eventually, messages from the origin website might get
> > marked as spam if people get annoyed at this. So, maybe there should
> > be some kind of timeout or cap on this behavior. Or maybe it should
> > notify the administrators if it catches you using this feature an
> > unrealistically large number of times.
>
> You surely are persistent on annoying people (I recall you doing this
> to a couple core developers on CPC) ;-)
>
> Okay, I am not opposed to adding this functionality. I feel that we
> can allow a cap on the number of times a user can re-send a
> confirmation. Sounds reasonable to me. Write it up as a task.
It was someone else's suggestion. I just did it because I was
surprised you guys were ignoring this blatantly broken forgotten
password feature.
> > 5) The notification that a confirmation was sent is kind of dumb.
>
> > Once you log in after confirming your email, you see a message
> > indicating that the email was sent. No duh, I just confirmed it.
> > However, since I wasn't logged in at the time, it couldn't show me
> > this user message before. I think what we need here is a session-
> > based flash message, like the kind in rails, so we can display the
> > message when it is generated. There are a bunch of django snippets
> > out there that do this. We should use one.
>
> I wasn't able to reproduce this as you describe. What notification is
> this? We create a user message once the user adds an e-mail address in
> the account page. Though I don't see how this would then be shown
> after the user logs in as they were logged in when adding the e-mail
> and would have seen it after making the POST for adding the new e-mail.
If you set ACCOUNT_EMAIL_VERIFICATION=True, you couldn't possibly be
logged in when the message is generated. You have to actually verify
your email address first. After you do that, you can log in, and at
that point you get this useless message that an email confirmation has
been sent.
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Pinax Core Development" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to
[email protected]
For more options, visit this group at
http://groups.google.com/group/pinax-core-dev?hl=en
-~----------~----~----~----~------~----~------~--~---
