mojolicious / mojo

:sparkles: Mojolicious - Perl real-time web framework
https://mojolicious.org
Artistic License 2.0
2.66k stars 576 forks source link

`requires` seemingly adds conditions to all proceeding routes. #2092

Open rawleyfowler opened 10 months ago

rawleyfowler commented 10 months ago

Steps to reproduce the behavior

# Authenticated condition
sub authenticated {
    my ( $r, $c, $captures, $required ) = @_;
    my $res = ( !$required || $c->logged_in );

    if ( !$res ) {
        $c->flash('You need to login to continue.');
        $c->redirect_with_referrer('/login');
    }

    return $res;
}
# Logged_in helper
sub logged_in {
    return exists shift->session->{user_id};
}
# Redirect with referrer helper
sub redirect_with_referrer {
    my $c        = shift;
    my $location = shift;
    my $referrer = $c->req->url->path->to_string;

    return $c->redirect_to("$location?referrer=$referrer");
}
my $router = $self->routes->namespaces( ['App::Controller'] );

$router->add_condition( authenticated => \&authenticated );

$router->get('/')->requires( authenticated => 1 )
      ->to('app#index')->name('index');
$router->get('/login')->to('auth#login_view')->name('auth_login_view');

Expected behavior

I expect that adding the authenticated condition to / will not cause it to be added to /login.

Actual behavior

authenticated is added to both routes. Causing a redirect loop when accessing /, or /login.

If, however I move the order of the routes around, it works as expected.

my $router = $self->routes->namespaces( ['App::Controller'] );

$router->add_condition( authenticated => \&authenticated );

$router->get('/login')->to('auth#login_view')->name('auth_login_view');
$router->get('/')->requires( authenticated => 1 )
      ->to('app#index')->name('index');

Thanks.

daleif commented 10 months ago

Might be an idea to provide a full copiable Lite app example.

I never use requires like that. I do something similar to

my $auth = $router->under('/')->requires( authenticated => 1);
$auth -> get('/')->->to('app#index')->name('index');

note sure if that makes a difference.

rawleyfowler commented 10 months ago

@daleif Thats not very ergonomic if I need more than 1 condition

rawleyfowler commented 10 months ago

$self->routes->add_condition( authenticated => \&authenticated );

Seems to fix this.

Instead of:

$routes->add_condition( authenticated => \&authenticated );