HabitRPG / habitica

A habit tracker app which treats your goals like a Role Playing Game.
https://habitica.com
Other
11.95k stars 4.08k forks source link

Password reset not working for certain accounts #4789

Closed lefnire closed 9 years ago

lefnire commented 9 years ago

I've had various users email that the "Forgot Password" feature isn't sending them an email. I've verified they are trying over multiple days, so it's not due to server downtime. In these events, I double check they don't have the duplicate accounts bug. The weird thing is, during these times when I try reseting my own password it works just fine. I've even tried the following hack:

  1. Change their email address to my address (so that I'll get the email)
  2. Reset password
  3. Nothing...
  4. Change their email back

So it seems to impact only certain users. I've checked logs during these times to see if the code is throwing an error during the process (maybe some undefined field on these users), but it looks clean. Sample UUID 00393aec-e19c-4298-91df-4f6d3e743ee0

paglias commented 9 years ago

You should see if the request module sends the correct data to the e-mail server. If you can't I'll do it next week but I'll need an example email address

lefnire commented 9 years ago

can't for the life of me figure this one out. Thought a couple of my commits might be culprits, due to timing (https://github.com/HabitRPG/habitrpg/commit/8852ac6 & https://github.com/HabitRPG/habitrpg/commit/c3bb42b) but they check out for the tested emails. The server emails me my password just fine, I'm not seeing any NewRelic errors (there are some "email not found" 500s, I made that 404 here for clarity). Any devs down to check this ticket out, I'll send a list of emails that are failing. They're all standard x@y.com, no funny business :/

Alys commented 9 years ago

I'd be interested in at least looking at this. No promises that I'd be able to solve it. Other devs should definitely not feel inhibited from also investigating!

lefnire commented 9 years ago

A thought: TMK the system is using @paglias's Mandrill setup. if a user has clicked "unsubscribe" to opt out of a newsletter email, does that preclude them from forgot passwords? Should I ask users experiencing this issue to chime in on this thread whether or not they've unsubscribed from HabitRPG emails (newsletters, reminders, etc)?

If this ends up being the case, we can just go back to using the old node module for sending emails straight from our main server

evangineer commented 9 years ago

So does this mean that utils.txnEmail() is specific to Mandrill? I've just checked the Mandrill API docs and I can't find /job in them.

That said, I received a HabitRPG email today that was sent using Mandrill. So I'm baffled.

evangineer commented 9 years ago

The relevant bit of code appears to be api.resetPassword() in website/src/controllers/auth.js which makes a call to utils.txnEmail().

I presume the old node module that @lefnire is referring to nodemailer which is used in utils.sendEmail. Where can I find an example of HabitRPG application code that calls utils.sendEmail?

paglias commented 9 years ago

So utils.txnEmail is a standard function that makes a request using request module to our email server where all the texts are stored and the emails are sent using Mandrill's API.

@lefnire the email address linked to that id is not blackilisted or unsubscribed but I just tested too and it doesn't go through

paglias commented 9 years ago

And not a duplicate...

evangineer commented 9 years ago

So the email server code is effectively a wrapper for the Mandrill api? Where is the code for the email server @paglias?

paglias commented 9 years ago

@evangineer yep, basically a wrapper plus it runs a few backgound tasks related to emails. The code is actually private but it's a quite simply job queue backed by Kue. I'm quite sure there are no errors there, it simply seems the request is not getting to the email server at all

paglias commented 9 years ago

@evangineer @lefnire did some debugging: the email server is never hit with password requests from some users

evangineer commented 9 years ago

Thanks @paglias, that makes things clearer. Limits my ability to help with debugging though.

paglias commented 9 years ago

@evangineer basically the code for emails is this one:

  var email = emails[job.data.emailType];

  if(!email){
    return done(new Error('Invalid email type'));
  }

  // If it's an object there is only one email to send, otherwise
  // the same one to multiple users
  var toArr = job.data.to.email ? [job.data.to] : job.data.to;
  mandrillClient.messages.send({
    message: {
      subject: email.data.subject,
      to: toArr,
      'headers': {
        'Reply-To': replyToAddress
      },
      global_merge_vars: job.data.variables,
      from_email: 'messengers@habitrpg.com',
      from_name: 'HabitRPG',
      html: email.htmlTemplate,
      text: email.textTemplate
    }
  }, function(r){
    done(null, r);
  }, function(e){
    done(e);
  });
});

I've removed the non important parts but as you can see it's quite simple. If @lefnire is ok I'm going to connect my local instance to the main database and debug from there but won't have time till tomorrow

lefnire commented 9 years ago

@paglias yeah, do whatever you need. It's weird that it's not hitting the server, since it's working for my email address (is it not working for yours?) and the code is using the email server function call. Thanks for looking into this btw matteo

paglias commented 9 years ago

Yeah for me it's working. With non hitting the server I meant that it doesn't create a job there but since it's not producing an error maybe it's not getting there at all... I don't know Il 12/mar/2015 18:47 "Tyler Renelle" notifications@github.com ha scritto:

@paglias https://github.com/paglias yeah, do whatever you need. It's weird that it's not hitting the server, since it's working for my email address (is it not working for yours?) and the code is using the email server function call. Thanks for looking into this btw matteo

— Reply to this email directly or view it on GitHub https://github.com/HabitRPG/habitrpg/issues/4789#issuecomment-78544946.

lefnire commented 9 years ago

In the mean time, I'll be manually reseting the password for any users who have emailed me about this issue. If you haven't gotten an email from me by the end of today, email me at admin@habitrpg.com

evangineer commented 9 years ago

@paglias Thanks for the code snippet, it's clearer to me now how that part of HabitRPG works. I've been scratching my head for days over that.

paglias commented 9 years ago

I think eventually we could eventually open source it, there isn't anything secret but being very specific to habit I don't know how useful it could be Il 12/mar/2015 20:12 "Mamading Ceesay" notifications@github.com ha scritto:

@paglias https://github.com/paglias Thanks for the code snippet, it's clearer to me now how that part of HabitRPG works. I've been scratching my head for days over that.

— Reply to this email directly or view it on GitHub https://github.com/HabitRPG/habitrpg/issues/4789#issuecomment-78574748.

evangineer commented 9 years ago

@paglias I think anything that makes it easier to test and debug HabitRPG from end to end as a whole system is a good thing. Right now, you're the only person in a position to debug issues relating to the email server code, the current issue being a case in point.

paglias commented 9 years ago

I mostly agree but the current code would need some cleanup before being made public because right now it isn't ready to be published as it is. Also we should get the ok from @lefnire (he too has access to the code if needed) who should have the last word. Il 12/mar/2015 20:40 "Mamading Ceesay" notifications@github.com ha scritto:

@paglias https://github.com/paglias I think anything that makes it easier to test and debug HabitRPG from end to end as a whole system is a good thing. Right now, you're the only person in a position to debug issues relating to the email server code, the current issue being a case in point.

— Reply to this email directly or view it on GitHub https://github.com/HabitRPG/habitrpg/issues/4789#issuecomment-78587653.

lefnire commented 9 years ago

By all means open-source it, as long as there aren't credentials in the repo or anything sensitive (I assume it's using Heroku config vars)

On Thu, Mar 12, 2015 at 4:38 PM, Matteo Pagliazzi notifications@github.com wrote:

I mostly agree but the current code would need some cleanup before being made public because right now it isn't ready to be published as it is. Also we should get the ok from @lefnire (he too has access to the code if needed) who should have the last word. Il 12/mar/2015 20:40 "Mamading Ceesay" notifications@github.com ha scritto:

@paglias https://github.com/paglias I think anything that makes it easier to test and debug HabitRPG from end to end as a whole system is a good thing. Right now, you're the only person in a position to debug issues relating to the email server code, the current issue being a case in point.

— Reply to this email directly or view it on GitHub <https://github.com/HabitRPG/habitrpg/issues/4789#issuecomment-78587653 .

— Reply to this email directly or view it on GitHub https://github.com/HabitRPG/habitrpg/issues/4789#issuecomment-78664369.

paglias commented 9 years ago

Yeah it's using Heroku vars for credentials. I'll do some cleanup and publish it for the weekend Il 13/mar/2015 00:24 "Tyler Renelle" notifications@github.com ha scritto:

By all means open-source it, as long as there aren't credentials in the repo or anything sensitive (I assume it's using Heroku config vars)

On Thu, Mar 12, 2015 at 4:38 PM, Matteo Pagliazzi notifications@github.com wrote:

I mostly agree but the current code would need some cleanup before being made public because right now it isn't ready to be published as it is. Also we should get the ok from @lefnire (he too has access to the code if needed) who should have the last word. Il 12/mar/2015 20:40 "Mamading Ceesay" notifications@github.com ha scritto:

@paglias https://github.com/paglias I think anything that makes it

easier to test and debug HabitRPG from end to end as a whole system is a good thing. Right now, you're the only person in a position to debug issues relating to the email server code, the current issue being a case in point.

— Reply to this email directly or view it on GitHub <https://github.com/HabitRPG/habitrpg/issues/4789#issuecomment-78587653 .

— Reply to this email directly or view it on GitHub https://github.com/HabitRPG/habitrpg/issues/4789#issuecomment-78664369.

— Reply to this email directly or view it on GitHub https://github.com/HabitRPG/habitrpg/issues/4789#issuecomment-78688572.

Alys commented 9 years ago

@lefnire I think you're right about unsubscribing preventing the password emails being sent in some (most?) of the cases. The code that looks for the unsubscribe flag doesn't seem to make any exception for password emails (I believe it should, although we might want to run that past redphoenix). Also, on a test account in production, I was able to receive a password reset email before I selected the unsubscribe from all option, but not after. I'd guess that fixing this would solve the problem for many players.

It seems to not be the whole solution though because one of the three accounts you showed me did not have the unsubscribe preference selected.

lefnire commented 9 years ago

Ok, just to be sure I'll switch us back to nodemailer temporarily when I get a moment today. @evangineer sorry I never answered that question - yeah, the previous solution was nodemailer. Matteo was tasked with building the whole listserv feature, and figured "well we've got a dedicated email server now, let's take some load off prod". Not sure if the nodemailer code is still there sitting around unused, or if it's buried in git history, but I'll scrape it up today

paglias commented 9 years ago

@lefnire @Alys indeed the user id posted here above is unsubscribed from all the emails which is blocking password emails from being sent (user.preferences.emailNotifications.unsubscribeFromAll === true). I'm ok with not considering this setting when sending password reset but I want to be sure it's ok so i'm pinging Vicky about this.

@Alys can you give me the UUID which is not working even though it's not unsubscribed?

paglias commented 9 years ago

@Alys @lefnire Vicky says it's ok so I'm going to remove the check for password emails. But we can't check users who unsubscribed through Mandrill, emails to them we'll be blocked anyway

paglias commented 9 years ago

@Alys @lefnire partial fix here https://github.com/HabitRPG/habitrpg/commit/5752f75c4617c5aec4e77e3f9a430be54a913335 we're just missing users who unsubscribed through Mandrill.

lefnire commented 9 years ago

Is there a way around that? Maybe we should just use nodemailer?

paglias commented 9 years ago

Sendind them using a different email address like (passwords@habitrpg.com insteead of messengers@habitrpg.com) should work. Otherwise just switch from mandrill to our own unsubscription page. I don't know about nodemailer but if it's has worked in the past there shouldn't be problems, at least until we fix it.

Alys commented 9 years ago

@paglias 0b900d5d-a7ee-437d-9fe1-5e7339a1499c

lefnire commented 9 years ago

tried reverting to nodemailer on beta (diff), but it's not sending me a pass. More involved than a revert

paglias commented 9 years ago

@Alys I've been able to send a reset password email to that user so at least that is fixed.

@evangineer the email server is now open sourced at https://github.com/HabitRPG/habitrpg-email-server if you want too give a look :)

evangineer commented 9 years ago

@lefnire Thanks for your response to my question. It's good to have a little insight into the history of email notifications in HabitRPG. I had figured most of it out by looking at the code. The piece that had eluded me was the Kue-based email server middleware. I did find your nodemailer code for resetting passwords instructive. I tried to find such code through looking at the git history but without success. I can see now that I was looking in the wrong place.

@paglias thank you for open sourcing that code and thank you @lefnire for giving your consent to it being open sourced.

Alys commented 9 years ago

It sounds like this was fixed. @paglias Are you aware if there's still any problems?

paglias commented 9 years ago

@Alys it should be fixed. I kept this open just because of the blacklist on Mandrill that is forcing us to use nodemailer, but for the time being we can keep it this way

harielakanti commented 6 years ago

I have another GITHub account with userName: harielakanti. and link: https://github.com/harielakanti I don't the emailId for this account and I forgot the PW. am not able to logIn here, and not able to do ForgotPassword. I was to reActivate or Permanently delete this Account. Please help me.

Alys commented 6 years ago

@harielakantie So you aren't able to log in to a GitHub account? I'm afraid we can't help you here. This is an issue for Habitica accounts (habitica.com). You'll need to contact GitHub for help.