search

calcom / cal.com

Scheduling infrastructure for absolutely everyone.
https://cal.com
Other
31.92k stars 7.81k forks source link

[CAL-4038] feature flag for Signup can't be disabled #15758

Closed PeerRich closed 2 months ago

PeerRich commented 3 months ago

Discussed in https://github.com/calcom/cal.com/discussions/15708

Originally posted by **AhmedAburady** July 9, 2024 I found the setting in settings > admin features > disable-signup even when enabling that and i see "flags updated" still users can signup any solution for that? i'm using Cloudflare tunnel so i don't use reverse proxy to disable the link

CAL-4038

Studycode001 commented 3 months ago

Can you assign me this issue @PeerRich

Souptik2001 commented 3 months ago

Hi @PeerRich @ciaranha ,

The actual problem mentioned in the issue, is actually simply a caching problem.

If you see the flagFeatures are cached in-memory for 5 mins. - https://github.com/calcom/cal.com/blob/5becc95fc430d327d4b485246678541210fa06cd/packages/features/flags/server/utils.ts#L34

Therefore when you change toggle any feature flag switch and instantly check the feature (in this case the disable-signup), the cache still may have the old value and the previous behavior will be shown till 5 mins. have not elapsed since last cache rewarm.

Although this is an admin level change and for this level change 5 mins. caching is not a major thing, but still it will be better to have a fix for this. For this we have to do a forceful cache rewarming in the toggleFeatureFlag API, over here - https://github.com/calcom/cal.com/blob/5becc95fc430d327d4b485246678541210fa06cd/packages/trpc/server/routers/viewer/admin/_router.ts#L40 Please let me know if this change is desirable, then I can raise a PR for this.

Apart from this one thing I noticed is that although the front-end page was protected by this disable-signup flag, but the main signup API itself was note blocked if this flag was set. So, anyone can basically singup using a POST request to the API. Therefore I raised a small PR for fixing that only (added rest of the details in the PR).

Souptik2001 commented 2 months ago

@PeerRich @ciaranha

Another thing which I want to point out here is that rewarming the cache here is not at all easy.

Although in the above comment I have mentioned this -

Although this is an admin level change and for this level change 5 mins. caching is not a major thing, but still it will be better to have a fix for this. For this we have to do a forceful cache rewarming in the toggleFeatureFlag API, over here - https://github.com/calcom/cal.com/blob/5becc95fc430d327d4b485246678541210fa06cd/packages/trpc/server/routers/viewer/admin/_router.ts#L40

But this will not be that easy (or I would say even not possible). Because we are using an in-memory (variable based) cache, for reference here - https://github.com/calcom/cal.com/blob/5becc95fc430d327d4b485246678541210fa06cd/packages/features/flags/server/utils.ts#L25

Currently we are only using this cache across our getServerSideProps functions, that's why it is working properly, because it is using the same instance of this cache across multiple requests. But APIs on the other hand run serverless, that is each have their own process, therefore we have no way to access that cache, which we want to rewarm. As far I can think there is no simple non-hacky way to solve this issue.

I would suggest to using some Caching service like Redis or Vercel KV (if its deployed on Vercel). That will be much simpler and efficient. What do you think?