Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

requires seemingly adds conditions to all proceeding routes. #2092

Open
rawleyfowler opened this issue Aug 12, 2023 · 3 comments
Open

requires seemingly adds conditions to all proceeding routes. #2092

rawleyfowler opened this issue Aug 12, 2023 · 3 comments

Comments

@rawleyfowler
Copy link
Contributor

rawleyfowler commented Aug 12, 2023

  • Mojolicious version: 9.33
  • Perl version: 5.36
  • Operating system: Linux 6.4.8

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.

@rawleyfowler rawleyfowler changed the title Weird behaviour with requires and routes. 'requires' seemingly adds to all routes. Aug 12, 2023
@rawleyfowler rawleyfowler changed the title 'requires' seemingly adds to all routes. requires seemingly adds to all proceeding routes. Aug 12, 2023
@rawleyfowler rawleyfowler changed the title requires seemingly adds to all proceeding routes. requires seemingly adds conditions to all proceeding routes. Aug 12, 2023
@daleif
Copy link

daleif commented Aug 14, 2023

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
Copy link
Contributor Author

rawleyfowler commented Aug 15, 2023

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

@rawleyfowler
Copy link
Contributor Author

rawleyfowler commented Aug 15, 2023

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

Seems to fix this.

Instead of:

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

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

No branches or pull requests

2 participants