codeigniter4 / shield

Authentication and Authorization for CodeIgniter 4
https://shield.codeigniter.com
MIT License
351 stars 123 forks source link

Bug: Missing loading helper 'auth' in ActionController #352

Closed MitkoIT closed 1 year ago

MitkoIT commented 1 year ago

PHP Version

7.4.16

CodeIgniter4 Version

4.2.3

Shield Version

v1.0.0-beta.2

Which operating systems have you tested for this bug?

Windows

Which server did you use?

apache

Database

mysqlnd 8.1.1

Did you customize Shield?

No

What happened?

We've wanted to enable 2FA authentication in our application, but after enabling it in Auth.php an error occurs.

Steps to Reproduce

Enable 2FA in Auth.php with this code public array $actions = [ 'login' => 'CodeIgniter\Shield\Authentication\Actions\Email2FA', 'register' => null, ];

After user login, system is returning error image

After some reading of Shield code we've seen that the ActionController actually don't load an auth helper. So that's why Codeigniter is trying to run auth() controller, what we see on the screenshot.

Please update ActionController and change protected $helpers = ['setting']; to protected $helpers = ['auth', 'setting'];

Expected Output

image

Anything else?

No response

datamweb commented 1 year ago

Please show us the contents of file app/Controllers/BaseController.php?

MGatner commented 1 year ago

@datamweb it looks like from the screenshot that they are using the native ActionController, which is indeed missing the Auth helper property.

@MitkoIT I agree with your solution; would you please send over a Pull Request? If you are uncomfortable navigating remotes in Git you can make the edit right in GitHub and it will walk you through submitting the PR right in the UI.

MitkoIT commented 1 year ago

Zrzut ekranu 2022-08-09 115238

MitkoIT commented 1 year ago

@MGatner I've maded a pull request by Github UI

kenjis commented 1 year ago

I recommend you do 2. Helper Setup: https://github.com/codeigniter4/shield/blob/develop/docs/install.md#manual-setup

kenjis commented 1 year ago

But loading the helpers is a bit pain. We almost always need them, why don't we load by Composer autoloader?

MGatner commented 1 year ago

I've often thought the framework could use a "global helper" option. I typically add them to Events::on('pre_system') in my projects. Some way to configure them (maybe in Config\Autoload) so they could also be extended via Registrar would be helpful.

MGatner commented 1 year ago

Okay maybe I was wrong on which event, but same idea:

/**
 * Loads helpers we want available globally.
 *
 * @see BaseAction::__construct() and BaseController::$helpers for slightly less global
 */
Events::on('post_controller_constructor', static function () {
    helper(['alerts', 'auth', 'html', 'preferences', 'themes']);
});
kenjis commented 1 year ago

@MitkoIT A simple question. Why did not you run php spark shield:setup? See https://github.com/codeigniter4/shield/blob/develop/docs/install.md#command-setup

Or did you?

MGatner commented 1 year ago

@kenjis I think the fact that we have a controller with a $helpers property set but that uses other helpers not included should be enough to merit a bug fix. As long as we support these controllers without publishing them there will be scenarios where that helper must be loaded internally.

kenjis commented 1 year ago

@MGatner We provide the helpers, and most cases we need to load them. Why don't we load helpers by Composer autoloader?

MGatner commented 1 year ago

@kenjis would that look something like this? composer.json


"autoload": {
    "psr-4": {
        ...
    },
    "files": [
        "src/Helpers/Auth.php"
    ]
}

I haven't used that feature before but if it works how it seems then that would be a great solution.

kenjis commented 1 year ago

@MGatner Yes!

MGatner commented 1 year ago

https://getcomposer.org/doc/04-schema.md#files

Looks good! How do we support non-Composer users?

kenjis commented 1 year ago

Composer is required. https://github.com/codeigniter4/shield#prerequisites

MGatner commented 1 year ago

I'd be in favor of a refactor to autoload helpers via Composer and pull them from controllers (and anywhere else). Just to note that this will prevent developers from overriding the functions with their own versions (e.g. in Common.php or app/Helpers/).

kenjis commented 1 year ago

Oh, if we use Composer autoloader to load helpers, we cannot override the helpers. It is not a CodeIgniter way.

Personally I don't like helpers, because they are global functions, and need to be loaded. But one single benefit is that these are able to be overridden.

kenjis commented 1 year ago

In Common.php, developers can override the helper functions. So I sent a PR #368