Skip to content

Fix auth redirects - #161

Open
uniejo wants to merge 5 commits into
Yancy-app:masterfrom
uniejo:fix-auth-redirects
Open

uniejo wants to merge 5 commits into
Yancy-app:masterfrom
uniejo:fix-auth-redirects

Conversation

@uniejo

@uniejo uniejo commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

only redirect to local paths after login and logout

The return_to check only rejected URLs starting with a scheme or //,
so /\evil.example (which browsers treat as //evil.example) could
redirect users to another site after logging in. An unsafe return_to
also caused a 500 error instead of being ignored.

Add Yancy::Auth->is_local_path, which only allows paths starting with a
single / without backslashes or control characters. return_to and
start_session skip unsafe values and fall back to a safe default.

Also add a /auth/logout route that ends the session and redirects to
a local return_to path, or /.

Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com

uniejo and others added 5 commits October 1, 2026 10:10
Yancy::Controller::Yancy was renamed to Yancy::Controller::Model, and
Yancy::Plugin::File was removed in favor of Yancy::Storage. Update the
controller test and give the default editor a Yancy::Storage rooted at
public/uploads instead of loading the missing file plugin.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The new password auth template now uses the `l` helper for its labels,
and Yancy::Auth loads the I18N plugin if the app does not have it yet.
This lets t/i18n.t use Yancy::Auth::Password instead of the deleted
auth plugin roles.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Yancy::Auth->start_session only redirected, so logging in did nothing.
Add start_session, current_user, and end_session to Yancy::User, using
the sessions table with the session ID stored in the Mojolicious
session. Yancy::Auth now starts a session for the user after a
successful login, and Yancy::User keeps the auths registered with it.

Also fix two bugs in Yancy::Auth::Password's login check: logging in
as an unknown user no longer looks up attributes for an undefined user
ID, and upgrading a password digest calls set_password instead of the
missing _set_password.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The editor API and storage routes were open to anyone, giving full
read/write access to every schema and to file storage.

Yancy::Plugin::Editor now requires either a `users` model (a
Yancy::User) or an explicit `allow_anonymous => 1`. With a users model,
requests from anyone who is not logged in get a 401 from the API and
storage routes, and the editor page redirects to the first registered
auth's login page.

Mojolicious::Plugin::Yancy only adds its default editor when one of
those options is given in `editor`, and logs a warning otherwise.

The development apps set `allow_anonymous => 1`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The return_to check only rejected URLs starting with a scheme or `//`,
so `/\evil.example` (which browsers treat as `//evil.example`) could
redirect users to another site after logging in. An unsafe return_to
also caused a 500 error instead of being ignored.

Add Yancy::Auth->is_local_path, which only allows paths starting with a
single `/` without backslashes or control characters. return_to and
start_session skip unsafe values and fall back to a safe default.

Also add a `/auth/logout` route that ends the session and redirects to
a local return_to path, or `/`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@preaction

Copy link
Copy Markdown
Collaborator

I'll have to port this change over to the Yancy-Auth repository as well, but thanks for closing this security hole!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants