Repository navigation
Preserve requested URLs through login - #1053
hartikainen wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request implements a mechanism to preserve and restore the user's requested destination path, query, and fragment across the OAuth2 login flow. On the frontend, the Angular authGuard and HomePage components are updated to pass the requested URL as a return_to query parameter to the login endpoint. On the backend, the Go OAuth2 package encodes this path into the OAuth state, validates that it is a safe local path, and redirects the user to it upon successful authentication. Feedback on the changes highlights a potential open redirect vulnerability in isLocalReturnPath where multi-slash paths (e.g., ///example.com) could bypass the local path check, and suggests adding corresponding test cases to prevent this security issue.
Reject raw return paths beginning with `//` before parsing. Cover triple, quadruple, and percent-encoded leading slashes in the validator and login callback tests. The decoded-path check also rejects these inputs; the raw check makes the restriction explicit at the input boundary. Comment link: #1053 (comment) Comment link: #1053 (comment)
|
@hartikainen - thank you for the fix. ready for review? |
Reject raw return paths beginning with `//` before parsing. Cover triple, quadruple, and percent-encoded leading slashes in the validator and login callback tests. The decoded-path check also rejects these inputs; the raw check makes the restriction explicit at the input boundary. Comment link: #1053 (comment) Comment link: #1053 (comment)
bc8f921 to
b99ce9f
Compare
b99ce9f to
e66fc1d
Compare
e66fc1d to
14f34cb
Compare
|
This is now ready to go. @andrzejfudro, mind giving it another look? (I just updated the branch off of |
Carry the requested route through OAuth state so sign-in returns to the original page, including its query and fragment. Accept only local absolute paths and bind the destination to the state cookie. Fall back to `/experiments` when no valid destination is supplied.
14f34cb to
202def9
Compare
I'm putting this out here as a proposed solution for #1052. This is fully Astra-coded and I haven't yet verified the changes myself beyond the unit tests.
The changes here wire the requested route through OAuth state so sign-in returns to the originally-requested page (including its query and fragment). It accepts only local absolute paths and bind the destination to the state cookie and falls back to
/experimentswhen no valid destination is supplied.Closes #1052.