First pass at invite-only migration. #1949

dessalines · 2021-11-23T16:05:31Z

No description provided.

dessalines · 2021-11-23T16:06:28Z

migrations/2021-11-23-153753_add_invite_only_columns/up.sql

+  deny_reason text,
+  published timestamp not null default now(),
+  unique(local_user_id)
+);


@Nutomic take a look at these, I changed some of the column names to make more sense. If they look good I can continue on with it.

published is the creation time? That one is a bit unclear, others are fine. Not sure if acceptor_id and, accepted and deny_reason are really necessary. The first doesnt really matter, instead of accepted I would use local_user.activated (which i'm adding right now for email verification), and deny_reason can just be sent by email without storing. Well maybe that one makes sense for other admins to view.

…instances

dessalines · 2021-12-04T02:42:25Z

Still some work to do here, but the difficult part is done.

- Add a DeleteAccount response. - RegistrationApplicationView now has the safe LocalUserSettings. - Adding VerifyEmail to websocket API, added a proper response type.

- Added a check_registration_application function. - Only send a verification email if its been changed. - VerifyEmail now returns LoginResponse. - Deleting the old tokens after a successful email verify. - If port is missing on email config, display a better error message.

Nutomic · 2021-12-11T17:52:33Z

Trying to test it now. It looks like you also made it that login is required to view any content, didnt think that was part of this milestone.

Its probably necessary to add an alternative frontpage which informs that the instance is private (instead of the small popup "this instance is private" which can easily be missed). Right now it just keeps loading forever.
Next problem is that i cant login to my existing account on ds9, it gives error "email not verified". That shouldnt happen as it completely locks people out of their account. I set both email_verified and accepted_application true in the db, but still cant login (same error).
On signup, it should probably show some message like "verify your email now so that you can login".
The verification email is sent by your gmail address for some reason.
The email verification link doesnt seem to work right, after clicking its again loading forever. It should show some confirmation like "your email has been verified", probably alongside the login form.

Nutomic · 2021-12-11T17:59:32Z

crates/api/src/local_user.rs

+          &context.secret().jwt_secret,
+          &context.settings().hostname,
+        )?
+        .into(),


So clicking the verification link should directly log the user in, without password check? Thats not very secure, and besides I dont think it worked for me.

Hrm... yeah I spose that's correct. Dang, I just changed that respose to LoginResponse from an empty VerifyEmailResponse, but I can revert that. In that case, on a successful verify, I'll just redirect them to the login page.

Nutomic · 2021-12-11T18:02:03Z

crates/api/src/local_user.rs

+
+    let email = if let Some(email) = &email_deref {
+      let site = blocking(context.pool(), Site::read_simple).await??;
+      if site.require_email_verification {


This is also how i implemented it, but i dont think it makes sense. If there is an email being set, Lemmy should always send a verification mail, regardless of the require_email_verification setting. Cause its really useless to have an email with a typo in it. What require_email_verification should do is make the email field mandatory on signup (and prevent unsetting it in the user settings), nothing else.

Nutomic · 2021-12-11T18:07:35Z

crates/utils/src/settings/structs.rs

+  #[default(None)]
+  pub application_question: Option<String>,
+  #[default(None)]
+  pub private_instance: Option<bool>,


Worth emphasizing that even with this setting, other instances can view all posts, comments etc from the instance via federation if it is enabled. And because of that, it is easy to iterate through all posts and comments using curl -H 'Accept: application/activity+json' https://ds9.lemmy.ml/post/x.

So maybe it should throw an error if both private_instance and federation.enabled are set (at least until #868 or #1576 are implemented).

I can add that check to main.rs

dessalines · 2021-12-12T02:53:16Z

Trying to test it now. It looks like you also made it that login is required to view any content, didnt think that was part of this milestone.

Only when private instance is set to true.

Its probably necessary to add an alternative frontpage which informs that the instance is private (instead of the small popup "this instance is private" which can easily be missed). Right now it just keeps loading forever.

Hrm, it shouldn't do that, but I'll test again. Right now I have it so that it will redirect you to the login page if you get that error (just tested, yes this is working ). But I can add a "This instance is private" header to the login page.

Next problem is that i cant login to my existing account on ds9, it gives error "email not verified". That shouldnt happen as it completely locks people out of their account. I set both email_verified and accepted_application true in the db, but still cant login (same error).

If a server suddenly turns on require_email_verification, I'm not sure the best way to not lock out existing users. That's a tough one. Maybe when that gets set to true, we need to update that email_verified column to true for every existing user ( even though that isn't actually the case ). I could use some help thinking of the best way to handle that.

On signup, it should probably show some message like "verify your email now so that you can login".

It shows up popup that says "verification email sent", and that field is required when the site requires it, so we should be good there.

The verification email is sent by your gmail address for some reason.

lemmy.ml is blocked by spamhaus so to even be able to test this, I had to use my gmail creds. Unfortunately I think we're gonna have to give up on postfix for lemmy.ml and just pay for an email service once this gets merged.

The email verification link doesnt seem to work right, after clicking its again loading forever. It should show some confirmation like "your email has been verified", probably alongside the login form.

I tested that a few days ago, and it worked fine. It should only load for a second or two. I'll also make that isomorphic on the front end tho just in case. I'd be interested to see your console logs there when it failed.

dessalines · 2021-12-12T19:59:12Z

@Nutomic okay these fixes are deployed to ds9.lemmy.ml, you can test whenever. Note that you'll have to disable federation if you want to test private instances.

Nutomic · 2021-12-14T13:53:45Z

Only when private instance is set to true.

Yes its no problem, i just didnt think we would implement it as part of this milestone.

Hrm, it shouldn't do that, but I'll test again. Right now I have it so that it will redirect you to the login page if you get that error (just tested, yes this is working ). But I can add a "This instance is private" header to the login page.

Okay that works, but only when you initially open the site. When you are on the login page and navigate to another page (eg the main page, or communities page), it shows an error popup and loads forever. Also i noticed the mod log is still publicly visible for a private instance.

If a server suddenly turns on require_email_verification, I'm not sure the best way to not lock out existing users. That's a tough one. Maybe when that gets set to true, we need to update that email_verified column to true for every existing user ( even though that isn't actually the case ). I could use some help thinking of the best way to handle that.

Not sure either. It could be enough to document this in the config, telling admins that they manually need to update this column for existing users.

It shows up popup that says "verification email sent", and that field is required when the site requires it, so we should be good there.

Problem is that those popups are extremely easy to miss, as they are in a corner of the screen and disappear pretty quickly.

lemmy.ml is blocked by spamhaus so to even be able to test this, I had to use my gmail creds. Unfortunately I think we're gonna have to give up on postfix for lemmy.ml and just pay for an email service once this gets merged.

Only if we enable email verification, which i doubt is gonna help much with spam. The registration applications are gonna be a lot more effective in that regard. But if there is an email service with a reasonable price, we can consider it.

I tested that a few days ago, and it worked fine. It should only load for a second or two. I'll also make that isomorphic on the front end tho just in case. I'd be interested to see your console logs there when it failed.

Yes this time it worked fine. But when clicking the verification link for a second time (after already being verified), it again ends up loading forever (just with a small error popup). Maybe in cases like this, it should show the error(s) in place of the loading bar (and no popup).

dessalines · 2021-12-14T16:55:53Z

Okay that works, but only when you initially open the site. When you are on the login page and navigate to another page (eg the main page, or communities page), it shows an error popup and loads forever.

Not much I can do about that. The most important thing is the initial page load puts you in the right place.

Also i noticed the mod log is still publicly visible for a private instance.

Looks like GetModLog doesn't even have an auth field. I'll add that.

Not sure either. It could be enough to document this in the config, telling admins that they manually need to update this column for existing users.

I already added the previous solution to the code. I tested, and it works.

Only if we enable email verification, which i doubt is gonna help much with spam

I agree, I'd rather not enable email verification. But it still would probably be worth it to get an email service, so that other notification emails and password resets can still get sent out without problems. Those are all currently broken now due to spamhaus.

But when clicking the verification link for a second time (after already being verified), it again ends up loading forever (just with a small error popup). Maybe in cases like this, it should show the error(s) in place of the loading bar (and no popup).

I'll check this out on the front end.

Nutomic · 2021-12-15T16:04:16Z

Not much I can do about that. The most important thing is the initial page load puts you in the right place.

You could hide or disable the buttons in that case.

Anyway this seems ready to merge then.

First pass at invite-only migration.

b361849

dessalines commented Nov 23, 2021

View reviewed changes

Nutomic and others added 4 commits November 25, 2021 18:50

Implement email verification (fixes #219)

c199215

remove unwrap

e43bc67

Merge remote-tracking branch 'origin/email-verification' into invite_…

9889d15

…instances

Adding views and functionality to registration application. #209

e28977c

dessalines added 2 commits December 4, 2021 14:40

Add private instance site column, and back end checks.

52b8e73

Adding some message fields to LoginResponse

438efe1

dessalines marked this pull request as ready for review December 4, 2021 20:46

dessalines marked this pull request as draft December 4, 2021 20:47

Adding private instance to site setup.

5507f53

dessalines marked this pull request as ready for review December 4, 2021 20:57

dessalines requested a review from Nutomic December 4, 2021 20:57

dessalines added 4 commits December 6, 2021 09:52

Merge branch 'main' into invite_instances

aa57883

Merge branch 'main' into invite_instances

be8f79d

A few additions:

b038583

- Add a DeleteAccount response. - RegistrationApplicationView now has the safe LocalUserSettings. - Adding VerifyEmail to websocket API, added a proper response type.

Adding and reorganizing some email helpers.

d17470c

dessalines mentioned this pull request Dec 8, 2021

Implement email verification (fixes #219) #1956

Closed

dessalines added 4 commits December 8, 2021 16:48

Merge branch 'main' into invite_instances

b9978cc

Version 0.15.0-rc.3

69cfd7a

Adding published to email_verification table.

f753fb9

Nutomic reviewed Dec 11, 2021

View reviewed changes

dessalines marked this pull request as draft December 12, 2021 03:03

Adding fixes from comments.

d60624a

Version 0.15.0-rc.4

cd78e05

dessalines marked this pull request as ready for review December 12, 2021 21:56

Merge branch 'main' into invite_instances

a90d5a4

dessalines added 2 commits December 14, 2021 12:58

Adding modlog private site check.

86a4d9b

Version 0.15.0-rc.6

5bb620f

Nutomic approved these changes Dec 15, 2021

View reviewed changes

dessalines merged commit c883a49 into main Dec 15, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

First pass at invite-only migration. #1949

First pass at invite-only migration. #1949

dessalines commented Nov 23, 2021

dessalines Nov 23, 2021

Nutomic Nov 24, 2021

dessalines commented Dec 4, 2021

Nutomic commented Dec 11, 2021 •

edited

Loading

Nutomic Dec 11, 2021 •

edited

Loading

dessalines Dec 12, 2021

Nutomic Dec 11, 2021

Nutomic Dec 11, 2021

dessalines Dec 12, 2021

dessalines commented Dec 12, 2021

dessalines commented Dec 12, 2021

Nutomic commented Dec 14, 2021

dessalines commented Dec 14, 2021

Nutomic commented Dec 15, 2021

First pass at invite-only migration. #1949

First pass at invite-only migration. #1949

Conversation

dessalines commented Nov 23, 2021

dessalines Nov 23, 2021

Choose a reason for hiding this comment

Nutomic Nov 24, 2021

Choose a reason for hiding this comment

dessalines commented Dec 4, 2021

Nutomic commented Dec 11, 2021 • edited Loading

Nutomic Dec 11, 2021 • edited Loading

Choose a reason for hiding this comment

dessalines Dec 12, 2021

Choose a reason for hiding this comment

Nutomic Dec 11, 2021

Choose a reason for hiding this comment

Nutomic Dec 11, 2021

Choose a reason for hiding this comment

dessalines Dec 12, 2021

Choose a reason for hiding this comment

dessalines commented Dec 12, 2021

dessalines commented Dec 12, 2021

Nutomic commented Dec 14, 2021

dessalines commented Dec 14, 2021

Nutomic commented Dec 15, 2021

Nutomic commented Dec 11, 2021 •

edited

Loading

Nutomic Dec 11, 2021 •

edited

Loading