Add admin option to disable SSO registrations #604
Conversation
fix function type return and if statements add persist to keycloak on existing email
| @@ -75,11 +78,16 @@ public function authenticate(Request $request): Passport | |||
| if ($user) { | |||
| $user->oauthKeycloakId = $keycloakUser->getId(); | |||
|
|
|||
| $this->entityManager->persist($user); | |||
There was a problem hiding this comment.
this was missing from keycloak but appears in all the other authenticators, so seemed like it might be needed
e-five256
changed the title
|
This will remove (or at least handicap) a vector currently being used by spammers to bypass our email verification and captcha checks while giving legit users the ability to still use SSO after normal registration... a much needed feature for those of us with SSO enabled. |
ghost
self-requested a review
ghost
added
enhancement
|
@BentiGorlich @thepaperpilot since you both were involved with the discussion on the issue wanted to make sure this made sense to both of you for how it was implemented and ofc don't worry about getting this in the zitadel stuff, I'll make sure to do that if it comes before or after |
|
Does this new setting default to true or will it have to be enabled for instances that have some SSO providers enabled? Also @nobodyatroot can you explain what you mean exactly? |
|
@BentiGorlich When register with SSO is enabled (or just having Google/Facebook/Github SSO enabled before this PR), there is no email verification or captcha check... I've been hammered multiple times with spammers signing up using this feature because it bypasses our simple spam "checks", I would turn SSO off but I have legit users using Google to signin. |
|
Ah ok. Yeah I have noticed that most spam accounts on mastodon use gmail addresses. I would have expected that Hoogle would do a better job at preventing spam accounts |
Me too, but I guess if they're not actively using the Gmail account to spam emails, then they don't care.... |
Yep! (to it defaulting to enabled) mbin/src/Service/SettingsManager.php Line 69 in bfa4ea9 should be no unexpected changes for admins pulling this in. I could see it being a little confusing for admins that don't use SSO at all, since it saying sso registration enabled (which it is by default) when there is none configured has no effect. There isn't a great way to check whether SSO is enabled without checking every single provider, but I could add that and hide the admin option if none are set, or just change the option to have a description that says only has an effect when having an sso provider enabled |
|
I think it is fine as is, but a description doesn't burt nobody 😁 |
Add admin option to disable SSO registrations while still allowing SSO logins
Right now as described in the linked issue, disabling site registration does not stop new account creation via SSO methods. This adds a new overall admin setting to disable all sso options new account registration, while allowing logins to still function for existing users
the authenticators have 3 stages:
I chose to implement the check between options 2 and 3, as if the user was already able to make a native account with the email, it seems like there's no harm in allowing to remap to the SSO option, but I could move it up between 1 and 2 instead if people think that is more correct
suggest hiding whitespace changes when looking at the diff
when attempting to register with an account that does not exist, users get redirected to login and shown this message
Fixes #518