[BUG] User sign up with POST request to the API

[harishkotra]

Description

I've just sent a POST request using Postman to with plain text JSON Body and boom! Account is created.

Request URL: https://app.appsmith.com/api/v1/users
Request: http://prntscr.com/uokz6z

What could go possibly wrong with this approach?

I can simply write a loop that runs probably a 1000 times or 1000...n times and create users with a simple cURL call and random emails and flood the database.

What are the ways to fix this

• Use a CAPTCHA or a reCAPTCHA
• Use a hidden field or honey-pot
• Use a double opt-in form to enable email verification
• Blacklist IPs for repeated submissions
• And more...

Add security to disable API based user creation and also add either a reCAPTCHA or a layer of security to verify if it is an actual user signing up.

Important Details

• Version: Cloud
• OS: Windows 10
• Application: Postman

[pc9795]

We can implement a Rate limiter in the back-end using the token given to the UserPrincipal 🤔

[Nikhil-Nandagopal]

@pc9795 we recently implemented re-captcha for the editor and re-captcha seems like the best way to do this. Would you consider a simple re-captcha implementation?

[pc9795]

Recaptcha will protect us from the in-app requests but what about machine clients (like cURL, Postman etc)?

[Nikhil-Nandagopal]

@pc9795 We actually recently learnt that re-captcha is a backend protection mechanism. It sends the backend a token that is generated by the re-captcha library and the token has to be verified by a google API.
https://developers.google.com/recaptcha/docs/verify

[pc9795]

Oh! If we are implementing the re-captcha token validation in the backend then it works fine. I just had seen this on client-side that's why I was confused 👍

[pc9795]

If we have implemented it then we can close this issue.

[Nikhil-Nandagopal]

@pc9795 it's not implemented yet. Would you like to pick this up?

[pc9795]

I will have a look 👍

[sumitsum]

Hi @Nikhil-Nandagopal, Can I take it up if it is available ?

[monarch0111]

Hey @sumitsum, are you working on this issue? If not, can I take this over?

I have gone through the codebase to understand the flow for this issue.
I have few questions, if you can help me clarify on to get on speed with this issue.

In UserController there are 2 methods which can be used to create new users.
- create calls UserSignup#signupAndLogin
- createFormEncoded, calls UserSignup#signupAndLoginFromFormData which in turn calls UserSignup#signupAndLogin

As per above flow UserSignup#signupAndLogin is the point where both flows converge.

Question: I don't know where UserController#create method is being called from (I guess, it's an API interface for clients other than WebAPP). Where should recaptcha verification step fall in?

If we only want to add recaptcha for signup flow, I feel UserSignup#signupAndLogin should be a good point. But, it voilates Single Responsibility Priciple of method.

Ideally, a middleware (using annotation) should be used to keep it extensible for all methods in future (i.e. login, password change etc).

I am new to Java, please correct me if I get terminologies wrong.

[sharat87]

Hey @monarch0111, you're on the right track. Only change I'd suggest is instead of an annotation, let's just have a service class called CaptchaValidatorService or something that has a method to do the recaptcha validation. Now whatever API endpoints can reach out to this service, if that API needs to be protected by a captcha.

Check out this link: https://www.baeldung.com/spring-security-registration-captcha#Retrieve-ResponseToken. This suggests a way of doing the check in the request handler method and then proceeding to rest of the implementation. It'll be slightly different in our case since Appsmith's codebase is reactive in nature, but this should get you started. The rest of this article can also serve as a starting point for an implementation of the validator service.

Do reach out here if you get stuck somewhere or need a pointer in the codebase itself. Good luck 👍 and thanks for your contributions 🙏!

[monarch0111]

@sharat87 Thanks for the insights. I have done a basic implementation (not tested well, though). monarch0111#1

It needs to be refined and worked on, I will raise a PR on this repo once done.

[mohanarpit]

Will there be a way to bypass/mock the recaptcha as well? I'm asking this because a few deployments (like AWS AMI) create a default admin user. Also our Cypress tests create test user accounts for each test.

I would like to ensure that there's a way to either bypass or mock re-captcha invocations to ensure that we don't end up breaking any of these tests or self-hosted instances.

[monarch0111]

@mohanarpit I have defined an interface, which should allow us to mock this.
e.g. https://github.com/monarch0111/appsmith/pull/1/files#diff-249dac13c37b04dc4c59b32ecb1231dc35862b0c5de50ecb6f7919351ee7f4c0R51

In case secret key is not defined, it will always return true as per current implementation.
I will move it to a flag based configuration to enable/disable captcha verification, so that it's explicit.