jhthorsen / mojolicious-plugin-openapi

OpenAPI / Swagger plugin for Mojolicious
54 stars 44 forks source link

Security: missing handler defaults to authorize #254

Closed afhp2018 closed 2 months ago

afhp2018 commented 3 months ago

Security plugin does not handle missing security callback correctly:

(./Plugin/OpenAPI/Security.pm:68)

  for my $security_and (@security_or) {
      for my $name (sort keys %$security_and) {
        my $security_cb = $handlers->{$name};

        if (!$security_cb) {
          $res{$name} = {message => "No security callback for $name."} unless exists $res{$name};
        }
        elsif (!exists $res{$name}) {
          $res{$name} = undef;
          $n_checks++;

          # $security_cb is obviously called synchronously, but the callback
          # might also be called synchronously. We need the $sync_mode guard
          # to make sure that we do not call continue() if that is the case.
          $c->$security_cb(
            $definitions->{$name},
            $security_and->{$name},
            sub {
              $res{$name} //= $_[1];
              $security_completed->() if --$n_checks == 0;
            }
          );
        }
      }
    }

If there is no handler defined in the code, $security_completed->() is never called and does not deny the request.

Correct would be:

   for my $security_and (@security_or) {
      for my $name (sort keys %$security_and) {
        my $security_cb = $handlers->{$name};

        if (!$security_cb) {
          $res{$name} = {message => "No security callback for $name."} unless exists $res{$name};
          $security_completed->();
        }
        elsif (!exists $res{$name}) {
          $res{$name} = undef;
          $n_checks++;

          # $security_cb is obviously called synchronously, but the callback
          # might also be called synchronously. We need the $sync_mode guard
          # to make sure that we do not call continue() if that is the case.
          $c->$security_cb(
            $definitions->{$name},
            $security_and->{$name},
            sub {
              $res{$name} //= $_[1];
              $security_completed->() if --$n_checks == 0;
            }
          );
        }
      }
    }
jhthorsen commented 2 months ago

Thanks for noticing 👍 Can you open a PR with a test as well?

jhthorsen commented 2 months ago

Oh, you already did 😸

Going to be closed by #255