fix: Signup error redirect to wrong path #31179
Conversation
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/7927931183. |
|
Deploy-Preview-URL: https://ce-31179.dp.appsmith.com |
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/7928032791. |
|
Deploy-Preview-URL: https://ce-31179.dp.appsmith.com |
WalkthroughWalkthroughThe recent update introduces a modification in the user signup and login process, wherein the redirect path is dynamically determined based on the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| String path = "/user/signup"; | ||
|
|
||
| String referer = exchange.getRequest().getHeaders().getFirst("referer"); | ||
| if (referer != null) { | ||
| try { | ||
| path = URI.create(referer).getPath(); | ||
| } catch (IllegalArgumentException ex) { | ||
| // This is okay, we just use the default value for `path`. | ||
| } | ||
| } |
There was a problem hiding this comment.
The logic for determining the redirect path based on the referer header is a good approach to ensure users are redirected back to the signup page they came from. However, there are a few considerations:
- Security: Ensure that the
refererheader is validated to prevent open redirect vulnerabilities. Consider checking if therefereris a valid URL within your application's domain. - Fallback Path: The default fallback path is set to
"/user/signup". Confirm that this path is universally applicable for all signup scenarios in your application. - Error Handling: The catch for
IllegalArgumentExceptionis appropriate, but consider logging the exception for debugging purposes, especially if therefererheader is malformed.
Consider adding validation for the referer header to ensure it belongs to your application's domain to prevent open redirect vulnerabilities.
| URI redirectUri; | ||
| try { | ||
| redirectUri = new URIBuilder() | ||
| .setPath("/") | ||
| .setPath(path) |
There was a problem hiding this comment.
When constructing the redirectUri with the error parameter, it's a good practice to ensure that the error message is URL-encoded to prevent issues with URL parsing, especially if the error message contains special characters.
Ensure the error message is URL-encoded when setting it as a query parameter to prevent potential issues with URL parsing.
On signup failure, we need to redirect the client to same signup page they were on, for the error message to show up. So instead of redirecting to the homepage, we get the path from the incoming request and use that.
Summary by CodeRabbit