Skip to content
This repository was archived by the owner on Oct 10, 2025. It is now read-only.

Commit d064a2f

Browse files
authored
fix: do not redirect to invalid redirectTo on error to (#664)
If an attacker requests an invalid redirectTo the service correctly validates the value and throws on error. However, in the case of the /verify endpoint we redirect to the very same redirectTo that we just invalidated indicating the redirectTo was invalid. ### **PR Type** Bug fix ___ ### **Description** - Fix redirect URL validation in ticket verification - Prevent redirection to invalid/malicious URLs on error - Use client URL instead of invalid redirectTo parameter - Add test case for wrong redirect URL scenario ___ ### Diagram Walkthrough ```mermaid flowchart LR A["Invalid redirectTo"] --> B["Validation Error"] B --> C["Use ClientURL instead"] C --> D["Safe Redirect"] ``` <details> <summary><h3> File Walkthrough</h3></summary> <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Bug fix</strong></td><td><table> <tr> <td> <details> <summary><strong>verify_ticket.go</strong><dd><code>Fix redirect URL validation error handling</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary> <hr> go/controller/verify_ticket.go <ul><li>Replace invalid <code>redirectTo</code> with <code>ctrl.config.ClientURL</code> on validation <br>error<br> <li> Prevent potential security vulnerability from malicious redirect URLs</ul> </details> </td> <td><a href="https://github.com/nhost/hasura-auth/pull/664/files#diff-7a36d63db0b247e393d4e7a9a2ff52be673c33aa2a4e40af06f1a4445edb3c32">+1/-1</a>&nbsp; &nbsp; &nbsp; </td> </tr> </table></td></tr><tr><td><strong>Tests</strong></td><td><table> <tr> <td> <details> <summary><strong>verify_ticket_test.go</strong><dd><code>Add test for invalid redirect URL handling</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary> <hr> go/controller/verify_ticket_test.go <ul><li>Add test case for "wrong redirect URL" scenario<br> <li> Verify proper error handling with malicious redirect URL<br> <li> Test expected redirect to client URL with appropriate error message</ul> </details> </td> <td><a href="https://github.com/nhost/hasura-auth/pull/664/files#diff-0c15b83d87b39bbbb4175ee401fef063bd686677d110c89822ddc287c5f0e57e">+31/-0</a>&nbsp; &nbsp; </td> </tr> </table></td></tr></tr></tbody></table> </details> ___
1 parent c618a7a commit d064a2f

2 files changed

Lines changed: 32 additions & 1 deletion

File tree

go/controller/verify_ticket.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ func (ctrl *Controller) getVerifyValidateRequest(
109109

110110
_, apiErr := ctrl.wf.ValidateOptionsRedirectTo(ctx, options, logger)
111111
if apiErr != nil {
112-
return sql.AuthUser{}, "", redirectTo, apiErr
112+
return sql.AuthUser{}, "", ctrl.config.ClientURL, apiErr
113113
}
114114

115115
ticketType, apiErr := getTicketType(ctx, req.Params.Ticket, logger)

go/controller/verify_ticket_test.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -526,6 +526,37 @@ func TestVerifyTicket(t *testing.T) { //nolint:maintidx
526526
jwtTokenFn: nil,
527527
getControllerOpts: nil,
528528
},
529+
530+
{
531+
name: "wrong redirect URL",
532+
config: func() *controller.Config {
533+
c := getConfig()
534+
c.RequireEmailVerification = true
535+
return c
536+
},
537+
db: func(ctrl *gomock.Controller) controller.DBClient {
538+
mock := mock.NewMockDBClient(ctrl)
539+
540+
return mock
541+
},
542+
request: api.VerifyTicketRequestObject{
543+
Params: api.VerifyTicketParams{
544+
Ticket: "passwordReset:123",
545+
RedirectTo: "http://evil.com:3000/redirect",
546+
Type: nil,
547+
},
548+
},
549+
expectedResponse: controller.ErrorRedirectResponse{
550+
Headers: struct {
551+
Location string
552+
}{
553+
Location: `http://localhost:3000?error=redirectTo-not-allowed&errorDescription=The+value+of+%22options.redirectTo%22+is+not+allowed.`, //nolint:lll
554+
},
555+
},
556+
expectedJWT: nil,
557+
jwtTokenFn: nil,
558+
getControllerOpts: nil,
559+
},
529560
}
530561

531562
for _, tc := range cases {

0 commit comments

Comments
 (0)