On Oct 27, 2009, at 12:27 AM, Nick Retallack wrote:
> 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. You apparently are the one with the requirement to require e-mail confirmation. Pinax by default doesn't enforce that. I disagree that it is expected behavior. I simply do not want to support this use case in Pinax. > Also, this could be made into a non-default configuration setting, so > you can warn people about the security risks in some nearby comments. Having to document potential security risks is something I'd prefer to avoid. If this functionality is important to your requirements there isn't much stopping you from implementing it yourself. It isn't anything *I* want to support in Pinax. >>> 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. The link doesn't literally log you in. It is a link to the login page where you provide your credentials for authentication. >>> 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. Confirmation is only tied to one account. Invalidation of the old keys won't matter since you already performed the confirmation for the account you requested it for. I'm confused how you see invalidation of old ones an issue? > >>> 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? Keep in mind Pinax won't always depend on a username. Since we will be generating a unique hash for each account I don't mind producing a single e-mail as you suggest. >> 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. This is fine. All I am trying to get at is that we don't need to implement this feature to solve other bugs in Pinax. They are unrelated. Supporting [email protected] syntax will be a site specific requirement. If you want that, add it yourself. >>> 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. No, we were not ignoring anything. This is a strictly volunteer based project. We are spending our free time to work on Pinax. A little patience from the community is all we ask for. Yes, there are bugs in Pinax. I am glad you are helping us identify them and solve them. >>> 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. Oddly enough I still wasn't able to reproduce. However, I see in the code what you mean. It doesn't make much sense unless you are authenticated after sign-up which is the case when e-mail is optional. This can definitely be fixed. Brian Rosner http://oebfare.com http://twitter.com/brosner --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
