Add registering and login with OTP through email

[josepegea]

Add new auth method, using just email and an OTP link.

Based in the work of @phoet in https://github.com/weg-li/weg-li

Both for registering and for logging in, the user just inputs their email and receives a message with a OTP link.

Users registered through this provider lack nickname , name and image on creation.
In order to deal with the validations for these properties, they're initialized as follows:

• nickname is generated by hashing the email address.

• name is filled in with a known marker for "missing name" declared in User::EMPTY_NAME (currently "********"). It doesn't use the email address as a placeholder in order to avoid exposing it, as the user name is displayed in several pages in the application.

• image is set to the Gravatar URL for the email.

In order to urge users registered through email to provide a name that can be used in the app, after log in, a user with such an "empty" name is redirected to the profile edit page and asked to provide one.

As with any other source, if such a user later adds another provider, the name and image coming from those will take over the existing ones.

Should help deal with the Twitter problem described in #1063

[JoschkaSchulz]

@josepegea it looks a bit hacky :) But I understand the problem. Is it really necessary to add the Actionmailer Cop? Couldn't we just solve it, that way we don't have to worry about it in the future?

@salzig what are you thinking?

[josepegea]

@josepegea it looks a bit hacky :) But I understand the problem. Is it really necessary to add the Actionmailer Cop? Couldn't we just solve it, that way we don't have to worry about it in the future?

@salzig what are you thinking?

Indeed, there are many "hacky" parts about this.

Truth be told, I feel the user system is too influenced by Twitter being the first login method supported, and not having email as the main, unique, identifier has made this addition quite more complex than it should. It's only ironic that now it's Twitter, too, who pushes us to change 😅

As for the Metrics/ParameterLists cop, I have my own personal opinion about the overuse of Rubocop and I would gladly remove this one. Making the parameters list shorter here would imply either passing an opaque hash or having the mailer itself deal with getting the data from Whitelabel, and I feel that both options would make the solution more difficult to follow. I was just assuming that the set of cops was already agreed upon and was honoring it. But if we're all fine with removing it, I'll be happy to do that

[salzig]

That's my initial 2cents about it. Sorry for the delay.

[josepegea]

That's my initial 2cents about it. Sorry for the delay.

Thanks for the review! Updated

[phoet]

we do not use "Sie" but "Du" in the rest of our communication

[josepegea]

I speak no German (nor Polish) I'm afraid. I asked ChatGPT to do the translations hoping they would be a good starting point that could be improved by native speakers. Still, I asked again to rewrite the German texts in the colloquial form. Hoping they're not too bad!!

[phoet]

there is probably some matcher for this kind of stuff, but i dont remember, so whatever.

[josepegea]

I think they're available for "request" or "feature" specs, but not for "controller" ones.
Maybe this kind of test belongs more to a Capybara spec, but, I think it fits well here, given the current suite

[josepegea]

Thanks for your review, @phoet !

I applied the changes you suggested.

If there's no more comments, I'll merge this tomorrow in the morning, as I'd like to have it ready for users in preparation of a meetup for next week

[josepegea]

Ok, I merged it and tried to use it. Unfortunately, it turns out that the sending of the email fails, because of missing SMTP auth. @phoet , could you please take a look at the configuration? 🙏

[josepegea]

Ok, I merged it and tried to use it. Unfortunately, it turns out that the sending of the email fails, because of missing SMTP auth. @phoet , could you please take a look at the configuration? 🙏

I confirm that email login works if you manage to get the login link, but that's not possible as long as the app cannot send emails. I managed to make it work for myself and for one user of Madrid.rb by looking at the exception in AppSignal and forwarding the token from there. However, this is far from sustainable.

@phoet mentioned that is quite possible that the Heroku app has lost the Sendgrid configuration. I guess it used to have one, as there's another mailer that was used for usergroup notifications and the app configuration seems to point to Sendgrid. Unfortunately, I don't have access to the Heroku app, so I cannot check. @phoet is on holidays and away from a proper workplace. @salzig could you please check? 🙏

[salzig]

SENDGRID_USERNAME and SENDGRID_PASSWORD, as expected by config/environments/production.rb, aren't configured in Heroku. Only reference I could find is SENDGRID_DOMAIN.

[salzig]

and adding/"unlocking" the twilo SendGrid Heroku Addon ends here. <3

[josepegea]

Thanks for looking at it! 🙌

I was just going to ask if you could enable that addon 😅

[josepegea]

I was looking at the docs and it seems that they generate a unique email address to manage the addon and require SSO from there. Maybe it was already setup?

[phoet]

I deleted and reinstalled the addon. login works, maybe emails too

[josepegea]

Hey, thanks for that. Unfortunately, I just tested and still got SMTP-AUTH requested but missing user name (see AppSignal, under the ArgumentError 2081.

Maybe it's just a matter of setting the user and password provided by the addon on the proper ENV vars: SENDGRID_USERNAME and SENDGRID_PASSWORD

[salzig]

Maybe it's cause no sender was configured. You should have received a verification mail on hello@. Can you confirm?

[josepegea]

I just checked and, yes, there was a verification email from this morning and I just clicked in the confirmation link. I'm afraid I'm in the dark regarding the configuration of the addon or the Sendgrid account as I don't have access to the Heroku dashboard

[salzig]

maybe you where a bit late, I just resend the verification mail.

[josepegea]

Clicked it again. Still same error "550 The from address does not match a verified Sender Identity. Mail"

[salzig]

I'm not surprised by the 550, cause send grid still shows the sender as unverified.

[josepegea]

I verified the address and registered in Sendgrid

I'm really not sure if I should also "Create a new account" 🤔

[salzig]

ah, not sure if it was the right way to create a new account. Cause maybe we can't have it verified for our send grid account now?

[josepegea]

I finalized creating the account and now no error is raised, but emails still don't arrive. Maybe now it's because of lack of SPF or similar setup for the domain, that prevent the email from being delivered. Not even in spam 🤔

[salzig]

I don't think so, cause for Hamburg.onruby.de it works now, but the sender is verified in our account.

[josepegea]

So maybe you need to add the emails for each of the RUGs as verified senders on your account, indeed

[salzig]

Yes, I just send you a message via Discord. Maybe we can have a shortcut for "exchange" of a link there.

[salzig]

And we should introduce a "feature switch", so only Whitelabel where the email is verified/added can use email login.

[salzig]

Madridrb sender mail is now added and verified to our send grid account. Login for HamburgOnRuby and Madridrb via Email works now.

[josepegea]

Thanks for your help, @salzig!

And we should introduce a "feature switch", so only Whitelabel where the email is verified/added can use email login.

I'll work on that later today or tomorrow

[salzig]

Just as you suggested via Discord, maybe it's best to support a "generic" fallback via a "onruby" Global Sender first. So the feature at least works for all Labels. After that we can figure out how to support individual senders (verfied in a single account, different sendgrid account per label, etc)

[josepegea]

Yes, I think that's better. I will do that, then