guanacone / fullstack_app

BSD Zero Clause License
0 stars 0 forks source link

Email confirmation via mailgun-js #41

Closed guanacone closed 3 years ago

guanacone commented 3 years ago

PR is obviously not ready for merge. But if you could check if I'm going in the right direction.

guanacone commented 3 years ago

First try at implementing email confirmation. A really wired thing is that locally, when a unregistered user tries to log in he get's redirected, but not in the review app... It's still pretty messy but do you think it's going in the right direction?

guanacone commented 3 years ago

Not much change since yesterday. I think those are the necessary first steps for the email confirmation implementation.

  1. When user is registered, a new document is created in the db with a field isActivated that is set to false and an email is sent to the provided email (hard coded to my email for now) with an JWT activation token which is valid 24h. User is redirected to /activateAcccount where he's reminded to activate his account.
  2. If an not yet activated user tries to log in, he's redirected to /activateAcccount where he's reminded to activate his account.

This works locally but when deployed to heroku, an not yet activated is redirected to the same url (/user) as an activated one. I did check heroku logs --tail during that process, but I couldn't find anything strange.

guanacone commented 3 years ago

Still same issues in deployed app, unactivated user gets not redirected...

guanacone commented 3 years ago

I think this is pretty much good to be merged with master. One last thing I'd like to implement is to delete an user if he hasn't activated his account when the activationToken expires. In order to do so I thought about the following implementation:

  1. When a new user is created a field expireAt is populated with the activationToken exp time.
  2. When user activates his account expireAt field is updated with a value in the future (1 year?).
  3. With every user login expireAt field is updates with value in the future (1 year?).

It seams this is the only way that the user doesn't get deleted afer the account has been activated when a TTL is defined in the model. What do you think?

edwmurph commented 3 years ago

One last thing I'd like to implement is to delete an user if he hasn't activated his account when the activationToken expires.

i'd recommend doing a little research to cross reference how other sites handle this but off the top of my head, i think a user should be able to not login to a site for a year and expect their account to still exist. you could mitigate that by sending out email warnings if their account is going to be deleted but i don't think thats standard for other sites? i might be wrong about this but i think it's worth a little research to see how other sites handle this.

assuming you dont want to delete an activated user, when a user creates an account, you could still set a TTL with the user record, but then when the user activates their account, you could just delete the TTL on their user record? i think if the indexed field in a document is null, the document wont expire but im not positive there

guanacone commented 3 years ago

After the unsuccessful heroku specific solutions I looked at the gasby issue and someone in the thread pointed to the gatsby-plugins-express which I neither could implement successfully... Any hints would be greatly appreciated! https://github.com/wille/gatsby-plugin-express/issues/16

edwmurph commented 3 years ago

here's the issue i linked to you in our last zoom chat: https://github.com/gatsbyjs/gatsby/issues/12576

i dont think you need gatsby plugin express, this is a separate issue. gatsby plugin express is just a way to run express with gatsby. it on it's own doesn't solve the client only route problem

basically i think it's worth trying to update express to detect whenever a client side only route is requested from the server, and then have express send back the corresponding gatsby page

edwmurph commented 3 years ago

this is a good explanation of the problem: https://stackoverflow.com/questions/27928372/react-router-urls-dont-work-when-refreshing-or-writing-manually

i think the "hybrid" solution is the ideal solution worth pursuing in this case but there are other solutions you could explore posted in that thread

guanacone commented 3 years ago

this is a good explanation of the problem: https://stackoverflow.com/questions/27928372/react-router-urls-dont-work-when-refreshing-or-writing-manually

This was a very helpful link and therefore gatsbyjs/gatsby#12576 starts to be get a little bit more understandable. I roughly understand whats going on but module.exports = async program => {...} is very confusing to me. I guess this whole module gets exported to be used somewhere else. I think the most important part of this for my problem are the following lines:

const root = path.join(program.directory, `public`)
const pages = await getPages(program.directory)

Where or what the hell is program???

edwmurph commented 3 years ago

this is the important part https://github.com/gatsbyjs/gatsby/blob/3b3153d410800890c87af25df192136e2a4af97b/packages/gatsby/src/commands/serve.js#L28-L36

conceptually i think you need to do the same thing as the heroku solution was doing, but with express. e.g. when a user requests /user/new hijack the request and send back the /user/index.html file. and you'll have to do that for each client-only route

im not sure exactly what program is in that repo, they're probably using some weird tooling but that tooling isn't in your project so your express-based solution shouldn't need that part