search

EnCiv / undebate-ssp

Undebate Self Service Portal- Web portal where election administrators of democratically run organizations create undebates for their candidates and voters
Other
6 stars 9 forks source link

send-password api #154

Closed ddfridley closed 1 year ago

ddfridley commented 2 years ago

The send password socket-api call is missing. This should be added to the civil-server repo.
This is the code, taken from the original synaccord repo: 

'use strict';

import sequencer          from 'promise-sequencer';
import User               from '../models/user';
import sendEmail          from '../server/util/send-email';
import secret             from '../../secret.json';

function sendPassword (email, return_to, cb) {
  const { host } = this.handshake.headers;

  function emailBody (key, token) {
    const tail=token+(return_to||'');
    return secret['forgot password email']
      .replace(/\{key\}/g, key)
      .replace(/\{url\}/g, `http://${host}/page/reset-password/${tail}`);
  }

  new Promise((pass, fail) => {
    User
      .findOne({ email })

      .then(user => new Promise((pass, fail) => {
        if ( ! user ) {
          fail(new Error('User not found'));
        }

        else {
          sequencer(
            ()          =>    user.reactivate(),
            user        =>    sendEmail({
              from      :   secret.email.user,
              to        :   email,
              subject   :   'Reset password',
              text      :   emailBody(user.activation_key, user.activation_token)
            })
          ).then(pass, fail);
        }

      }).then(pass, fail))

      .catch(fail);
  })
    .then(cb)
    .catch(error => cb({ error : error.message }));
}

export default sendPassword;

In the above secret['forgot password email'] is: "Hey,\r\n\r\nYou are receiving this email because you requested a password reset.\r\n\r\nYour reset key is:\r\n\r\n\t{key}\r\n\r\nTo reset your password, copy the reset key above and go to {url}\r\n\n\nThank you,\nThe Synaccord Team",

I didn't really need to be a secret. I would just include this text in the new send-password.js. In the future, or as a stretch goal, we can use SendInBlue to send the email, and create an HTML template for it.

It sends a link, that the users clicks on, that will take them to a page that will allow them to change their password. The link has a code in it, that is looked up in the database to find the user. Here is the code for that route:

  resetPassword(){
    this.app.get(['/page/reset-password/:token','/page/reset-password/:token/*'], (req, res, next) => {
      try{
        if(req.params.token){
          User.findOne({activation_token: req.params.token}).then(user=>{
              if(user && user._id){
                req.user=user.toJSON();
                req.cookies.synuser={id: req.user._id, email: req.user.email} // passing the activation key also
                req.activation_key=user.activation_key;
                this.setUserCookie(req,res,next);
              }else
                next();
            },
            (error)=>{console.info("resetPassord found error",error); next(error)}
          )
        }
      }
      catch(error){
        next(error);
      }
    },serverReactRender.bind(this))
  }
ice1080 commented 1 year ago

@ddfridley is the original synaccord repo you mentioned a public repo? I did some searching and couldn't find it. The send-email file is missing from the above example. I'll eventually work on switching the above to SendInBlue, just figured I'd ask if you have it handy.

ice1080 commented 1 year ago

Also, I noticed that join.jsx in civil-client is currently unused. From what I can tell, I have to update loginForm.jsx in that project to add the resetPassword flow, would you like me to delete the unused Join file? Same question for join.js in civil-server.

ice1080 commented 1 year ago

@ddfridley you mentioned using send-in-blue on civil-server. I noticed that there is currently neither send-in-blue nor nodemailer are setup in civil-server; just confirming that we do for sure want to add send-in-blue to this?

ice1080 commented 1 year ago

For historical tracking, I am copying the candidate-invitation template for the reset password template. I am removing the unsubscribe section of the email since that's obviously not applicable here.

ice1080 commented 1 year ago

Hey @ddfridley, you mentioned a cypress test for this one in the description. I added a cypress test that creates a user, logs in as the user, and then sends the reset password email. I'm not sure it's possible to actually intercept the email request from the backend to send-in-blue (cypress only lets you intercept frontend to backend calls as far as I know). If there is some way to seed the db with a user before running, then we should be able to insert a fake activationToken etc, and then test that the reset password link works and we can login with the new password after.

Alternatively, I can add some scripts to the package.json that perform what I mentioned using mongodb cli commands. I'll pursue that for now unless I hear back from you on a simpler way to seed and reset the db.

ddfridley commented 1 year ago

I have no way to preppopulate users for cypress. A script sounds fine.

⁣Get BlueMail for Android ​

On Oct 11, 2022, 8:02 AM, at 8:02 AM, ice1080 @.***> wrote:

Hey @ddfridley, you mentioned a cypress test for this one in the description. I added a cypress test that creates a user, logs in as the user, and then sends the reset password email. I'm not sure it's possible to actually intercept the email request from the backend to send-in-blue (cypress only lets you intercept frontend to backend calls as far as I know). If there is some way to seed the db with a user before running, then we should be able to insert a fake activationToken etc, and then test that the reset password link works and we can login with the new password after.

Alternatively, I can add some scripts to the package.json that perform what I mentioned using mongodb cli commands. I'll pursue that for now unless I hear back from you on a simpler way to seed and reset the db.

-- Reply to this email directly or view it on GitHub: https://github.com/EnCiv/undebate-ssp/issues/154#issuecomment-1274091463 You are receiving this because you were mentioned.

Message ID: @.***>

ddfridley commented 1 year ago

For testing emails sendinblue has this that might be useful: https://github.com/sendinblue/APIv3-nodejs-library/blob/master/docs/InboundParsingApi.md#getInboundEmailEvents

I've never used it.

⁣Get BlueMail for Android ​

On Oct 11, 2022, 8:02 AM, at 8:02 AM, ice1080 @.***> wrote:

Hey @ddfridley, you mentioned a cypress test for this one in the description. I added a cypress test that creates a user, logs in as the user, and then sends the reset password email. I'm not sure it's possible to actually intercept the email request from the backend to send-in-blue (cypress only lets you intercept frontend to backend calls as far as I know). If there is some way to seed the db with a user before running, then we should be able to insert a fake activationToken etc, and then test that the reset password link works and we can login with the new password after.

Alternatively, I can add some scripts to the package.json that perform what I mentioned using mongodb cli commands. I'll pursue that for now unless I hear back from you on a simpler way to seed and reset the db.

-- Reply to this email directly or view it on GitHub: https://github.com/EnCiv/undebate-ssp/issues/154#issuecomment-1274091463 You are receiving this because you were mentioned.

Message ID: @.***>

ice1080 commented 1 year ago

Just about have this issue complete. I see it working when running civil-server on its own, and just saw it working inside undebate-ssp too. Remaining work:

ddfridley commented 1 year ago

Sorry. My mind is on vacation and only working in the background. About testing for this. I just realized that I had removed cypress because it was causing trouble when pushing undebate-ssp to heroku. Probably also causing problems for others building the repo too.

Did you install cypress?

I have two thoughts. One is to abandon cypress. We are doing a lot more with jest than I had previously thought possible.  The other is to create a separate repo that imports cypress and civil-server and run the tests there.

I wish npm had a testDependencies  section that is only installed at the top level.

Thoughts?

David.

⁣Get BlueMail for Android ​

On Oct 13, 2022, 4:56 AM, at 4:56 AM, ice1080 @.***> wrote:

Just about have this issue complete. I see it working when running civil-server on its own, and just saw it working inside undebate-ssp too. Remaining work:

  • update undebate-ssp to correctly handle the response from the reset-password socket
  • create custom reset password page for undebate-ssp to match undebate-ssp signup/login popup
  • stretch goal - rewrite authForm.jsx in civil-client to use the useAuth custom use-method

-- Reply to this email directly or view it on GitHub: https://github.com/EnCiv/undebate-ssp/issues/154#issuecomment-1276925018 You are receiving this because you were mentioned.

Message ID: @.***>

ice1080 commented 1 year ago

Cypress is currently in the optional dependencies of civil-server and there is already an existing test in there. I was able to write some pretty good cypress tests for this flow in that repo so I was planning on leaving it at that. When you mention testDependencies, isn't that pretty much what the devDependencies are for? They don't get distributed with the prod build.

ddfridley commented 1 year ago

Ok. My memory is faulty. I'm glad you didn't have to do a lot extra for cypress.

When you npm install a package like civil-server it's devDependencies are installed and built before building the package itself. But test dependences like cypress, jest, and storybook are not needed to build civil-server when it is installed in undebate-ssp.

See the bottom of:

https://github.com/npm/cli/issues/3902

⁣Get BlueMail for Android ​

On Oct 13, 2022, 5:03 PM, at 5:03 PM, ice1080 @.***> wrote:

Cypress is currently in the optional dependencies of civil-server and there is already an existing test in there. I was able to write some pretty good cypress tests for this flow in that repo so I was planning on leaving it at that. When you mention testDependencies, isn't that pretty much what the devDependencies are for? They don't get distributed with the prod build.

-- Reply to this email directly or view it on GitHub: https://github.com/EnCiv/undebate-ssp/issues/154#issuecomment-1277666175 You are receiving this because you were mentioned.

Message ID: @.***>

ice1080 commented 1 year ago

Ah ya that makes sense. Fyi though, most contributors of projects don't look at closed issues, so your comment will most likely stay ignored. I'd recommend opening a new issue and explaining why it's different than the other issue, or why it's not solved by the closing of the other issue.

ice1080 commented 1 year ago

Just got back from vacation so trying to get this one finished up. Our jest tests don't currently test any frontend interactions and it would take quite a bit to get to that point. As mentioned before, cypress is currently already installed in civil-server so I was using that to test many of these interactions. Is that alright to leave in there? If we added cypress to undebate-ssp in the dev dependencies, would that still get packaged inside heroku? I know you said it would for dependencies' dev dependencies, but wasn't sure for direct dev dependencies. Let me know how you'd like to proceed with this.

ice1080 commented 1 year ago

Another thing I just noticed was this error message code in civil-server:

function route() {
  const apiLimiter = expressRateLimit({
    windowMs: 60 * 1000,
    max: 2,
    message: 'Too many attempts logging in, please try again after 24 hours.',
  })  
  this.app.post('/sign/in', apiLimiter, signIn, this.setUserCookie, sendUserId)
}

The error message says to try again after 24 hours, but it really only checks that you don't try 3 times in a row within 60 seconds. Should I update the message or was this intentional? I waited 60 seconds instead of 24 hours and was able to sign in again.

ddfridley commented 1 year ago

What if we do something based on NODE_ENV being development or production. For production it should be 24 hours. for development it should be 60 seconds.

ice1080 commented 1 year ago

@ddfridley Phew this one was a doozy. Here are all three pull requests for this issue:

On civil-server you can run npm run test to see the jest tests passing, and you can run the dev server and run npm run cypress:headless or npm run cypress to see the cypress tests passing. I also confirmed that the jest tests on undebate-ssp were still passing.

In order to truly test all of this hooked together prior to merging the PRs, you would have to use temp branches. Here are the dependencies I was using in undebate-ssp's package.json, which should all be up to date. If you go this route, manually change them in package.json, then remove your node_modules and reinstall. Sometimes you have to use npm uninstall <dependency> to remove it from the npm cache too, and then reinstall it and move it back into the peerDependencies section.

Let me know if you have any questions or changes!