Fix email verification

[LuisaAPF]

It's been a month since I last signed in to my account (locally). It happens that I forgot to confirm my email and that is an active option that forbids my access after 30 days. Is there a way to overcome this?
(I mean, I'm sure that is a manual way to solve this, but maybe it's better to avoid that this happens in production?)

[carpodaster]

@LuisaAPF agreed, if that happens in production, that does seem like a bug. To verify if I got it right: you encounter this problem on your development machine only right now?

If you don't have much data, the easiest way would probably to drop and re-create your development database:

bundle exec rake db:reset

Let me know if that fixed the problem locally. Would you like to work on some form of account recovery to prevent this lock out on production? I'm happy to help of course.

[LuisaAPF]

Hi @carpodaster, I could solve the problem locally. But is it really necessary to forbid the user from accessing her/his account if the email address is not confirmed? I think it would be better to allow access at least to the profile page, in case a change in email is needed. What do you think?

[carpodaster]

@LuisaAPF yes, you are absolutely right, access to the profile page should be possible. Maybe the app should even auto-redirect to the profile page after a login with an unconfirmed account whose grace period has expired? I don't have a way of testing out myself right now: can you log in at all with an unconfirmed-expired account? And if so, are you shown a flash notification that prompts you to validate the email address (this should be shown during the grace period)

[LuisaAPF]

When my account was expired, I wasn't able to login. I got a 401 unauthorized status. There was no notification asking me to validate my email address. But this wouldn't have solved the problem if my email was wrong or inactive. So I think we should give the user the chance to update the email before re-sending the confirmation request.

In my opinion, the best way would be to never block the access to an account. Instead, we should forbid a user with an unconfirmed email to have access to certain functionalities, like creating a team.

[emcoding]

Lately, I have been thinking about pulling the account-part out of the User model, and into a separate UserAccount. I expect that it would be easier to handle all devise-functions in the UserAccount functionality. Is that something to consider? (@carsten, @max)

[emcoding]

Sorry, I mean @carpodaster , @klappradla ^

[klappradla]

I personally wouldn't consider it @F3PiX, don't really see the pay-off (but you can of course always give it a try 👍 ). Is there actually much business logic tied to "accounts"? Isn't it all in the devise gem? We're also only using very very few features of devise 😉

Requiring a verified email to sign in is actually a feature™ @LuisaAPF 😉. User can be assigned as coaches which would e.g. require an email, etc. One could of course also solve this by adding more roles and restricting resources and interactions respectively. But from what I remember, the initial motivation was: verifying the email after sign up is one click away, motivating the user to do it later will probably be a harder task.
☝️ this just as a background, I'm happy for suggestions on how to improve this.

I pretty much like @carpodaster of sending unconfirmed users to a dedicated page right away. In theory, there would even be pages to request a new confirmation token 🙈 however, I don't think they're correctly set up in the Teams App.

In order to have something "actionable" about this bug report. What do you think about turning this in: ???

• fix / fully integrate Devise's confirmable module
• redirect users trying to log in with unconfirmed email and expired tokens to the right place

[LuisaAPF]

Hey @klappradla,
1 - I agree, it is a feature, not a bug 😀
2 - Regarding your comment about motivating the user to do it later, I don't think it would make a difference, because now the user doesn't know that her/his account will expire anyway
3 - I don't really see the purpose of blocking a user's account for not validating the email. In principle, during the 30 days in which they are allowed access to their account, they could be trying to create teams or being assigned to roles, functionalities which should require a valid email.
4 - What would be the right place to redirect users with unconfirmed email and expired tokens?

[klappradla]

Will stick to the number thing 🤓

2 - You're right.
3 - indeed interesting @LuisaAPF - we should of course prevent any of these actions for unconfirmed users and somehow remind them to confirm their email.
4 - including Devise's confirmable module should in theory give us a page where users can request a new token:

The routes are theoretically there, but I don't think there's anything configured to have them working.

❯ be rails routes | grep confirm
  new_user_confirmation GET    /users/confirmation/new(.:format)  devise/confirmations#new
      user_confirmation GET    /users/confirmation(.:format)      devise/confirmations#show
                        POST   /users/confirmation(.:format)      devise/confirmations#create

So to sum up:
the problem is not the 30 days (we could also remove this and have tokens never expire) but that users aren't reminded to confirm their account plus aren't prevented from using their unconfirmed account for basically everything, right?

[LuisaAPF]

Exactly! More important than blocking an account would be making sure a user can't do anything interesting with an unconfirmed email, from day 1.

[klappradla]

👍
So should we turn this issue into

Fix email verification

Submitting an application without having one's account is not possible, so that's a ✅
From what I remember the only "problem" where coaches with unconfirmed email addresses - see #618.

But I guess we could add:

• reminder / warning for Teams that coach's account is not confirmed
• enable the routes / views for resending a token (needs some adaptions due to omniauth)
• add proper redirects for users trying to log in with unconfirmed and expired accounts
• quick idea: with ☝️ in place, set the expiration to less then 30 days (maybe a week?) so that people are reminded earlier

What do you think?

[emcoding]

That's a nice summing up. 💝
I am going to propose to take the UserAccount (with all the log in/log out) out of the User Model. That would make the Devise implementation cleaner and easier to work with. The to do list ^ fits nicely in that plan.
If it is okay with both of you @LuisaAPF, @klappradla , I'd like to work on that - awaiting the outcome of the discussion re: the UserAccount.

[klappradla]

Sure @F3PiX - I'll assign the issue to you.

I'll also remove the bug label, because technically speaking the "buggy" part of this has been discussed away into a "feature" and it makes it move obvious from the outside.

[LuisaAPF]

Hey @klappradla , @F3PiX . I think it is a way of solving the problem. If we want to lock the account, the user should be able to unlock it, to avoid overwhelming the support. But honestly, I think it is an unnecessary piece of extra work. Say a user creates an account today, but forgets to validate the email. She cannot do any harm, because all important actions require a valid email. One week later she tries to log in again and finds out her account has been blocked. It seems to me she's being penalized for nothing.

[carpodaster]

set the expiration to less then 30 days (maybe a week?) so that people are reminded earlier

I don't recall the initial motivation for having this grace period. I would vote for not allowing an unconfirmed user anything but updating their profile. And confirming their email address of course. This is very strict, yes. But browsing projects, the community and teams page as well as the activity stream doesn't require a user account.