fix: error resolve for signup user when password contains <> sign
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 formatin 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:-
Login form:-
After login:-
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.
[!NOTE]
.coderabbit.ymlhas unrecognized propertiesCodeRabbit 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.
Comment @coderabbitai help to get the list of available commands and usage tips.
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?
hey, @ajhollid
I understood your concern give me some time I will find some solution with decoupled for same.
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.
@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 ?
@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?
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
@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.
@ajhollid @Owaiseimdad @gorkemcetin
i'm looking inputs from your what should do further?
@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 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.
@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.