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

Cannot redefine role_checker on newer perls #208

Open
sammakkoinen opened this issue Mar 4, 2024 · 2 comments
Open

Cannot redefine role_checker on newer perls #208

sammakkoinen opened this issue Mar 4, 2024 · 2 comments

Comments

@sammakkoinen
Copy link

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
Copy link
Collaborator

nrdvana commented Mar 7, 2024

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

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.

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