Checkmate icon indicating copy to clipboard operation
Checkmate copied to clipboard

fix: error resolve for signup user when password contains <> sign

Open Darshan98Solanki opened this issue 2 months ago • 12 comments

Hey @gorkemcetin,

Fixes #3010

Please ensure all items are checked off before requesting a review. "Checked off" means you need to add an "x" character between brackets so they turn into checkmarks.

  • [x] (Do not skip this or your PR will be closed) I deployed the application locally.
  • [x] (Do not skip this or your PR will be closed) I have performed a self-review and testing of my code.
  • [x] I have included the issue # in the PR.
  • [x] I have not included any files that are not related to my pull request, including package-lock and package-json if dependencies have not changed
  • [x] I didn't use any hardcoded values (otherwise it will not scale, and will make it difficult to maintain consistency across the application).
  • [x] I ran npm run format in server and client directories, which automatically formats your code.
  • [x] I took a screenshot or a video and attached to this PR if there is a UI change.

I just have encoded the password before hit request (cause it is reserved keywords) and decode that on backend before validating and insert option in db.

Note:- from frontend we saw that password is there in payload but when we log password in backend we see empty string like ''. So this method I have added.

Have given the SS for validating the changed on real time.

Signup form:- image

Login form:- image

After login:- image

Please check the pr and let me know is there is any updates.

Summary by CodeRabbit

  • Refactor
    • Updated authentication data handling to enhance password transmission security during register and login processes.

Darshan98Solanki avatar Nov 10 '25 10:11 Darshan98Solanki

[!NOTE]

.coderabbit.yml has unrecognized properties

CodeRabbit is using all valid settings from your configuration. Unrecognized properties (listed below) have been ignored and may indicate typos or deprecated fields that can be removed.

⚠️ Parsing warnings (1)
Validation error: Unrecognized key(s) in object: 'release_notes'
⚙️ Configuration instructions
  • Please see the configuration documentation for more information.
  • You can also validate your configuration using the online YAML validator.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Walkthrough

Client and server now synchronize password handling through URL encoding and decoding. The client encodes passwords via encodeURIComponent before transmission, and the server decodes them via decodeURIComponent upon receipt in both authentication endpoints, preserving special characters during transmission.

Changes

Cohort / File(s) Change Summary
Password URL encoding on client
client/src/Features/Auth/authSlice.js
Added encodeURIComponent to password before sending in both register and login thunks
Password URL decoding on server
server/src/controllers/v1/authController.js
Added decodeURIComponent to req.body.password in both registerUser and loginUser handlers prior to validation

Sequence Diagram

sequenceDiagram
    participant User
    participant Client as Client (authSlice.js)
    participant Network as HTTP Request
    participant Server as Server (authController.js)
    participant Validator

    User->>Client: Enter password with special chars (e.g., "<BIBpIiDXbgdp6M0vr6SO8T7")
    Client->>Client: encodeURIComponent(password)
    Client->>Network: Send encoded password
    Network->>Server: Receive encoded password
    Server->>Server: decodeURIComponent(req.body.password)
    Server->>Validator: Validate decoded password
    Validator-->>Server: ✓ Valid
    Server-->>Client: ✓ Success
    Client-->>User: Account created

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Homogeneous, repetitive pattern applied consistently across two files
  • No control flow changes or complex logic
  • Critical consideration: Verify that encoding/decoding roundtrip preserves all special characters without data loss
  • Confirm no other password handling paths bypass this encoding/decoding mechanism

Poem

A rabbit hops with glee, encoding as it goes, The password wrapped in URL's protective throes, Special chars like < now safely pass through space, No more phantom empties—decoded with grace! 🐰✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main fix: resolving signup errors when passwords contain reserved characters like '<>'.
Description check ✅ Passed The description includes issue reference, completed checklist items, clear explanation of the solution, and supporting screenshots for validation.
Linked Issues check ✅ Passed The PR code changes directly address issue #3010 by URL-encoding passwords client-side and decoding server-side, preventing '<' and other reserved characters from causing validation failures.
Out of Scope Changes check ✅ Passed Changes are limited to auth-related files (authSlice.js and authController.js) and directly target the password encoding issue without introducing unrelated modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • [ ] 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot] avatar Nov 10 '25 10:11 coderabbitai[bot]

Hey @Darshan98Solanki ,

While it works, this seems like an overly complex solution to the problem.

Moreover, it creates a tight dependency between the front and back ends, which is to be avoided.

Is there some reason the validation on the frontend can't be modified to allow for the password in question?

ajhollid avatar Nov 10 '25 17:11 ajhollid

hey, @ajhollid

I understood your concern give me some time I will find some solution with decoupled for same.

Darshan98Solanki avatar Nov 11 '25 07:11 Darshan98Solanki

Hey @Darshan98Solanki ,

While it works, this seems like an overly complex solution to the problem.

Moreover, it creates a tight dependency between the front and back ends, which is to be avoided.

Is there some reason the validation on the frontend can't be modified to allow for the password in question?

I think we need to think in terms of regex here, let me know what solution you have thought of.

Owaiseimdad avatar Nov 11 '25 09:11 Owaiseimdad

@ajhollid @Owaiseimdad

can we restrict < > this two letter for password or else pass the payload with stringify version but that is also tightly coupled.

I think restrict this letter will be good solution ?

Darshan98Solanki avatar Nov 12 '25 09:11 Darshan98Solanki

@ajhollid @Owaiseimdad

can we restrict < > this two letter for password or else pass the payload with stringify version but that is also tightly coupled.

I think restrict this letter will be good solution ?

Stringifying or otherwise transforming the input defeats the purpose of input validation, so let's avoid that.

The input vaidation schema should be modified to allow for the < and > characters. Regex is probably a reasonable solution as @Owaiseimdad mentioned, why not give that a shot?

ajhollid avatar Nov 12 '25 17:11 ajhollid

New Regex:- /^(?=.[a-z])(?=.[A-Z])(?=.\d)(?=.[!?@#$%^&*()-_=+[]{};:'",.~|\\/])[A-Za-z\d!?@#$%^&*()\-_=+\[\]{};:'",.~|\/]{8,}$/;

Old Regex:- /^(?=.[a-z])(?=.[A-Z])(?=.[0-9])(?=.[!?@#$%^&*()-_=+[]{};:'",.<>~|\\/])[A-Za-z0-9!?@#$%^&*()\-_=+[\]{};:'",.<>~|\/]+$/;

Difference Note :-

  •     More secure (blocks < and >)
    
  •     Stronger (minimum 8 characters)
    

I think this regex is good for validation should I change this for validation of password ? please give me you feedback @ajhollid @Owaiseimdad

Darshan98Solanki avatar Nov 13 '25 03:11 Darshan98Solanki

@ajhollid @Owaiseimdad @gorkemcetin

as per we discussed last any inputs ? should I remove <> sign from the password regex to prevent XXS attack and make it more secure also change the frontend relevant if needed.

Darshan98Solanki avatar Nov 14 '25 05:11 Darshan98Solanki

@ajhollid @Owaiseimdad @gorkemcetin

i'm looking inputs from your what should do further?

Darshan98Solanki avatar Nov 15 '25 15:11 Darshan98Solanki

@Darshan98Solanki the new regex must be only more premissive than the original one, it cannot be any more restrictive or it could possibly disallow existing users. So you can remove the min 8 char and it should be good to go.

ajhollid avatar Nov 16 '25 03:11 ajhollid

@ajhollid we also need to change the frontend as well. and give the list of special char list so the user will have clear idea about it.

Darshan98Solanki avatar Nov 20 '25 12:11 Darshan98Solanki

@ajhollid we also need to change the frontend as well. and give the list of special char list so the user will have clear idea about it.

We can do that in another PR.

gorkem-bwl avatar Nov 20 '25 14:11 gorkem-bwl