Ensure logs endpoint relies on config rather than the environment

[robmilward]

This fixes a potential vulnerability: In the event that the application config were ever cached, calls to env() return null. This would mean the logs route would not have a username and password restriction set, and could be accessed without any authentication.

In addition, this ensures that if either the username or password are null, the logs endpoint is not registered.

[robmilward]

Just a note, because of the behaviour of env when the application configuration is cached, it is my opinion that all calls to env outside of the config directory are dangerous and should be avoided at all costs. I'll raise an issue in the technologies repo about this for discussion about enforcing it as a rule.

[DivineOmega]

Good catch.

I agree with the policy of using config variables, over directly using environment variables.

[dextermb]

I think it might even be worth to have a validate method in the config. It'd allow for whatever logic you'd like without adding more things into the routes file.

Although I don't think you can access config from within a config file 🤷‍♂️

[
  'logs' => [
    'username' => env('LOG_USERNAME'),
    'password' => env('LOG_PASSWORD'),
    'validate' => function () {
      list('username' => $username, 'password' => $password) = config('laravel-route-restrictor.logs');

      return !empty($username) && !empty($password);
    }
  ]
]

Then in your routes file it would be clearer, as they tend to already get messy.

if (config('laravel-route-restrictor.logs')['validate']()) {
  // Route
}

[AlexCatch]

@dextermb Rather then having that inside of a config file - having the validate method on a service provider might be a bit more laravelly.

LaravelRouteRestrictor::validate()

[DivineOmega]

@dextermb Config files should not contain callables, as they can not be correctly serialised for caching purposes.

[dextermb]

@dextermb Rather then having that inside of a config file - having the validate method on a service provider might be a bit more laravelly.

LaravelRouteRestrictor::validate()

@dextermb Config files should not contain callables, as they can not be correctly serialised for caching purposes.

General idea would be something to keep the routes file less cluttered. Be it any other method 😛

[robmilward]

How about this solution? @DivineOmega @dextermb @AlexCatch

I've moved the route registration into its own service provider. There are no global variables, and a less cluttered route file.

The route is still only registered if username and password are set within the config.

[DivineOmega]

This is now present in v2.1.0.