Add env-configurable option to disable public signups #2035
Conversation
kevinmcconnell
left a comment
kevinmcconnell
left a comment
There was a problem hiding this comment.
This looks great @harisadam, thanks for getting it started. I've left a few suggestions, all minor things though!
I also think we might want to flip this around so that the env is MULTI_TENANT, and by default the app runs as single tenant. I think that is actually the behavior that people self-hosting the app are mostly likely to want. Because if you're running the app for yourself, you probably don't want to let random strangers come along and sign up for accounts on your server :) So MULTI_TENANT seems like it should be opt-in. Having the default this way means one less ENV for people to think about in the common case.
If we do that, we will also need to be able to have our fizzy-saas gem opt in to MULTI_TENANT as well. We could use a config value for that (similar to what's happening here).
| def single_tenant? | ||
| ENV.fetch("SINGLE_TENANT", "false") == "true" | ||
| end |
There was a problem hiding this comment.
I'd consider making this a model concern. Something like Account.accepting_signups? which takes into account both the ENV and the number of accounts.
I'd also base that logic on Account.any? rather than User.any?. They should be equivalent in practice, but a tenant is really an account, so it's a better fit there.
There was a problem hiding this comment.
makes sense, I created a MultiTenant concern for the Account model, it could hold other multitenant specific stuff in the future
| allow_unauthenticated_access | ||
| rate_limit to: 10, within: 3.minutes, only: :create, with: -> { redirect_to new_signup_path, alert: "Try again later." } | ||
| before_action :redirect_authenticated_user | ||
| before_action :prevent_signup_when_users_exist, if: -> { single_tenant? && User.any? } |
There was a problem hiding this comment.
If we have the model concern we can simplify the conditional a bit here too, so it's just checking Account.accepting_signups?
Could also put the conditional inside the action, similar to the style of redirect_authenticated_user that's nearby.
| end | ||
|
|
||
| def prevent_signup_when_users_exist | ||
| redirect_to new_session_url, alert: "Your account hasn’t been set up yet. Please request an invitation from one of your coworkers." |
There was a problem hiding this comment.
Don't think we need the alert here. Nothing will link to the signup page in this state, right? In which case, I think silently redirecting is fine.
…troller (#accepting_signups?)
|
@kevinmcconnell Thanks for the suggestions, they were spot-on, I’ve applied all of them. Now |
Also use a `configure` block to set it up.
|
Thanks again @harisadam. I've made a few more small changes, and I'll get this shipped shortly! |
| - `proxy/ssl` and `proxy/host`: Kamal can set up SSL certificates for you automatically. To enable that, set the hostname again as `host`. If you don't want SSL for some reason, you can set `ssl: false` to turn it off. | ||
| - `env/clear/MAILER_FROM_ADDRESS`: This is the email address that Fizzy will send emails from. It should usually be an address from the same domain where you're running Fizzy. | ||
| - `env/clear/SMTP_ADDRESS`: The address of an SMTP server that you can send email through. You can use a 3rd-party service for this, like Sendgrid or Postmark, in which case their documentation will tell you what to use for this. | ||
| - `env/clear/MULTI_TENANT`: Set to `false` to enable single-tenant mode (disable multi-account signups). |
There was a problem hiding this comment.
Shouldn't this be something along the lines of "Set to true to enable multi-tenant mode [...]" now that MULTI_TENANT explicitly has to be set to true? Or am I interpreting it wrong?
There was a problem hiding this comment.
Yes, good call, we can clarify this a bit 👍
There was a problem hiding this comment.
Make sure to update config/deploy.yml when you do! 🙌
6 participants
This PR adds a simple single-tenant (“no public signup”) mode controlled by the
SINGLE_TENANTenvironment variable.When
SINGLE_TENANT=trueand at least one user already exists(similarly to once-campfire):To avoid exposing whether an email exists, the page always displays the generic message:
“Check your inbox if your email belongs to an existing account.”
screenrecording-2025-12-09_17-01-06.mp4