Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: prevent non work emails #3993

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

Conversation

Mikehrn
Copy link
Contributor

@Mikehrn Mikehrn commented Feb 16, 2025

This blocks signing up with a non-work email, as an experiment to see how it impacts our numbers. @cdriesler I gave blocking it when using single sign-on a shot, but not sure if this is correct. Please roast me if needed :D

I will leave this as a draft for now. Dont merge yet.

Screenshot 2025-02-16 at 21 32 53

@benjaminvo
Copy link
Contributor

  1. Is this blocking only implemented on our server? I couldn't immediately see that from the code. But I guess it should.
  2. Have you tested how it works for invites? Flow: Receive invite on a blocked email domain -> Open signup screen with the email pre-filled -> Here we probably shouldn't block.
  3. Are you also blocking using a Google SSO button?

@Mikehrn
Copy link
Contributor Author

Mikehrn commented Feb 16, 2025

  1. Is this blocking only implemented on our server? I couldn't immediately see that from the code. But I guess it should.
  2. Have you tested how it works for invites? Flow: Receive invite on a blocked email domain -> Open signup screen with the email pre-filled -> Here we probably shouldn't block.
  3. Are you also blocking using a Google SSO button?
  1. Yeah there is no difference now, but you're right, I'm however not sure how we can do that atm, afaik there is no isSpeckle method. Maybe we should do it with a FF?
  2. Good point, should we block invites with blocked domains, or do we still want to allow those? In essence it's the same as signing up
  3. I think it should capture both, but I need @cdriesler to help me validate this, as I'm not very familiar with the server part of the auth

@cdriesler
Copy link
Member

What you've done to block domains on the backend is sound, but I would put the same logic within the body of createUser to capture all pathways (email/password, server-level SSO, workspace SSO, etc). Will push the change to this branch tomorrow.

How this will behave with Google SSO is that they will always be able to complete the sign in flow, but we'll prevent new user registration if the email is on the blacklist. So "chuck@speckle.systems" is ok, but "chuck@gmail.com" is not.

I'll take a look at how we can limit this set of changes to app.speckle.systems. Each server declares its "canonical URL" and I think we're safe to use that.

@Mikehrn
Copy link
Contributor Author

Mikehrn commented Feb 16, 2025

What you've done to block domains on the backend is sound, but I would put the same logic within the body of createUser to capture all pathways (email/password, server-level SSO, workspace SSO, etc). Will push the change to this branch tomorrow.

How this will behave with Google SSO is that they will always be able to complete the sign in flow, but we'll prevent new user registration if the email is on the blacklist. So "chuck@speckle.systems" is ok, but "chuck@gmail.com" is not.

I'll take a look at how we can limit this set of changes to app.speckle.systems. Each server declares its "canonical URL" and I think we're safe to use that.

Checking canonical URL sounds easy, vs a FF which one you think makes most sense? I have local changes to add the FF if we wanna go that route 😄

Copy link
Contributor

And remember that this is an experiment for now. So we should try to be pragmatic if there are ways to cut corners.

@cdriesler
Copy link
Member

What you've done to block domains on the backend is sound, but I would put the same logic within the body of createUser to capture all pathways (email/password, server-level SSO, workspace SSO, etc). Will push the change to this branch tomorrow.
How this will behave with Google SSO is that they will always be able to complete the sign in flow, but we'll prevent new user registration if the email is on the blacklist. So "chuck@speckle.systems" is ok, but "chuck@gmail.com" is not.
I'll take a look at how we can limit this set of changes to app.speckle.systems. Each server declares its "canonical URL" and I think we're safe to use that.

Checking canonical URL sounds easy, vs a FF which one you think makes most sense? I have local changes to add the FF if we wanna go that route 😄

Probably both is good imo!

@Mikehrn
Copy link
Contributor Author

Mikehrn commented Feb 16, 2025

And remember that this is an experiment for now. So we should try to be pragmatic if there are ways to cut corners.

FE-wise the only question is wether to block personal emails in invite or allow them, either isn't much work, it's mostly a product question

@benjaminvo
Copy link
Contributor

And remember that this is an experiment for now. So we should try to be pragmatic if there are ways to cut corners.

FE-wise the only question is wether to block personal emails in invite or allow them, either isn't much work, it's mostly a product question

When blocking in invite would you do it in the actual invite flow or when the user tries to sign up using that email? Or both.

I'm not sure yet if blocking on invite is needed to start. I don't think so right now.

Copy link
Contributor

📸 Preview service has generated an image.

Copy link
Contributor

📸 Preview service has generated an image.

Copy link
Contributor

📸 Preview service has generated an image.

Copy link
Contributor

📸 Preview service has generated an image.

Copy link
Contributor

📸 Preview service has generated an image.

@Mikehrn Mikehrn marked this pull request as ready for review February 18, 2025 10:27
Copy link
Contributor

📸 Preview service has generated an image.

Copy link
Contributor

📸 Preview service has generated an image.

Copy link
Contributor

📸 Preview service has generated an image.

Copy link
Contributor

📸 Preview service has generated an image.

Copy link
Contributor

📸 Preview service has generated an image.

Copy link
Contributor

📸 Preview service has generated an image.

Copy link
Contributor

📸 Preview service has generated an image.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants