cal.com icon indicating copy to clipboard operation
cal.com copied to clipboard

Published 20 hours ago • verified publisher icon calcom

disables signups via env variable

Open phishy opened this issue 2 years ago • 5 comments

What does this PR do?

Adds a new environment variable that disables signups:

  • remove link on login page
  • returns a 404 for /signup
  • returns a 403 for /api/auth/signup

Fixes #6192

Environment: Staging(main branch)

Type of change

  • [x] New feature (non-breaking change which adds functionality)

How should this be tested?

  • [ ] set env var NEXT_PUBLIC_DISABLE_SIGNUP to true
  • [ ] ensure signup link is disabled
  • [ ] ensure visiting /signup returns a 404
  • [ ] ensure trying to make a new user via /api/auth/signup fails

Checklist

  • I haven't added tests that prove my fix is effective or that my feature works

phishy avatar Dec 29 '22 01:12 phishy

@phishy is attempting to deploy a commit to the cal Team on Vercel.

A member of the Team first needs to authorize it.

vercel[bot] avatar Dec 29 '22 01:12 vercel[bot]

we don't wanna overload our .env, instead we're moving most modification settings to a database entry

PeerRich avatar Jan 01 '23 12:01 PeerRich

Yeah I saw you hit the 4K AWS limit, and the config table is in RFC. Is that right?

phishy avatar Jan 02 '23 15:01 phishy

Yeah I saw you hit the 4K AWS limit, and the config table is in RFC. Is that right?

correct, in general its a much better UX to have a config table + UI + admin manager to make non-code changes to the product than having a convoluted .env file

PeerRich avatar Jan 02 '23 17:01 PeerRich

Yeah I saw you hit the 4K AWS limit, and the config table is in RFC. Is that right?

correct, in general its a much better UX to have a config table + UI + admin manager to make non-code changes to the product than having a convoluted .env file

Agreed. Env is for secrets.

phishy avatar Jan 02 '23 17:01 phishy

@phishy What's the status of this request?

ojengwa avatar Jan 05 '23 11:01 ojengwa

@ojengwa its currently blocked because we dont want to overload .env.

however, the database implementation is a few weeks away, @zomars should we merge this and later move the .env to a database / admin setup?

PeerRich avatar Jan 05 '23 11:01 PeerRich

@PeerRich I understand you might be having issues with your env size, but this feature should be opt-in, meaning you should not need to change your env variables in order to keep the app working as is. Only users intending to block signups should need to add NEXT_PUBLIC_DISABLE_SIGNUP to their envs. Thus, in my point of view, I don't see why this is blocked.

tofran avatar Jan 09 '23 15:01 tofran

we will likely end up merging this and later extract this setting into an admin database @zomars @joeauyeung

PeerRich avatar Jan 09 '23 18:01 PeerRich