vanstyn / RapidApp

Turnkey ajaxy webapps
http://rapi.io
Other
48 stars 15 forks source link

Cannot redefine role_checker on newer perls #208

Open sammakkoinen opened 4 months ago

sammakkoinen commented 4 months ago

I have several user roles defined in RA::CoreSchema and I need some DB tables and menu items to be accessible to specific roles only, while the administrator role should have access to everything. This behavior is already provided in Catalyst::Plugin::RapidApp::AuthCore:

$config->{role_checker} ||= sub {
    my $ctx = shift; #<-- expects CONTEXT object
    return $ctx->check_any_user_role('administrator',@_);
};

so the administrator is explicitly added to the allowed roles and this is exactly what I want. However this is redefined in RapidApp::Module::ExtComponent:

has 'role_checker', is => 'ro', isa => 'Maybe[CodeRef]', default => sub {
  my $self = shift;
  my $code = try{$self->app->config->{'Model::RapidApp'}{role_checker}} || sub {
    my $c = shift; #<-- expects CONTEXT object
    # ...
    return $c && $c->can('check_any_user_role') ? $c->check_any_user_role(@_) : 1;
  }
};

So when I restrict access to something with require_role => 'user' the administrators lose access (unless I explicitly give them the user role). I don't know whether it is intended. As we see, it should be possible to redefine it via $self->app->config->{'Model::RapidApp'}{role_checker}, so I put the following in __PACKAGE__->config() in the main application module:

'RapidApp' => {
      # ...
      role_checker => sub {
          my $ctx = shift; #<-- expects CONTEXT object
          return $ctx->check_any_user_role('administrator',@_);
      },
      # ...

(I have also tried the following sub to avoid copypaste, but with the same result as described below:

 sub {
          my $c = shift;
          $c->config->{'Plugin::RapidApp::AuthCore'}{role_checker}->($c, @_);
      },

Maybe I'm doing something wrong already at this stage and there's some simpler way to force the behavior of AuthCore.pm, but I could not find it.) It works correctly in the single process mode, i.e. when I run the server as script/myapp_server.pl. However, when I run it as FCGI under FCGI::ProcManager::MaxRequests, the redefined role_checker is usually (but not always!) not applied (try{$self->app->config->{'Model::RapidApp'}{role_checker}} returns false). The administrator does not have access to sections restricted to the 'user' role. Moreover, this happens only on perl 5.24.x, but not on 5.12 and 5.16, which work correctly. I guessed that this might have something to do with hash randomization introduced in 5.18 (like some hash might not be initialized when try{$self->app->config->{'Model::RapidApp'}{role_checker}} is called?), and indeed, running the server with PERL_HASH_SEED=0 (https://perldoc.perl.org/perlrun#PERL_HASH_SEED) seems to fix the issue, although I can't say I have tested enough. A probably better fix (not dependent on perl versions and hidden features) would be adding lazy => 1 to the definition of 'role_checker' in RapidApp::Module::ExtComponent. It helps, but, again, the issue seems to be too complicated to be sure for now.

nrdvana commented 4 months ago

I think the discrepancy between ExtComponent and AuthCore represents that AuthCore is more opinionated about the auth system, where ExtComponent is meant to be more adaptable. It does seem that we have a module load-order bug, but you could work around it by adding your own

sub check_any_user_role {
  my $c= shift;
  $c->next::method('administrator', @_);
}

which will make things consistent across all of Catalyst.

Could you also try

BEGIN { __PACKAGE__->config(...) }

with your overridden role_checker and see if that fixes the load order problem?

sammakkoinen commented 3 months ago
sub check_any_user_role {
  my $c= shift;
  $c->next::method('administrator', @_);
}

This works.

BEGIN { __PACKAGE__->config(...) }

This does not.

At some moment the bug started appearing in the single process mode too. Setting PERL_HASH_SEED=0 definitely helps, so the issue must really have something to do with randomization of hashes in perl.

Although the first workaround does help and is simple enough, yet I think that redefining role_checker in RapidApp::Module::ExtComponent as 'lazy' still makes sense.