codeigniter4 / shield

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

Bug: can't use Auth config in different namespace than App #761

Closed jozefrebjak closed 1 year ago

jozefrebjak commented 1 year ago

PHP Version

8.1.16

CodeIgniter4 Version

4.3.6

Shield Version

dev-develop

Which operating systems have you tested for this bug?

macOS

Which server did you use?

fpm-fcgi

Database

MariaDB 10.2

Did you customize Shield?

No.

What happened?

After merge of https://github.com/codeigniter4/shield/pull/748 we cant use Auth config in different namespace than App.

Steps to Reproduce

We are storing Auth as a module in ROOTPATH/modules/Auth folder. There is Config folder with Auth.php config. This worked before merge https://github.com/codeigniter4/shield/pull/748, after merge our App is broken. We recently updated to latest dev-develop commit hash and go back commit to commit to find when something changed.

We are using App/Config/Autoload.php to load module like:

        'Auth'           => ROOTPATH . 'modules/Auth/',

Expected Output

We would like to store Auth as module in our App.

Anything else?

Workaround is to move Auth.php to App/Config.

datamweb commented 1 year ago

We are storing Auth as a module in ROOTPATH/modules/Auth folder.

@jozefrebjak do you mean that you installed the shield without composer? To be honest, I don't understand exactly what you mean.

Can you explain the reproduction step by step?

jozefrebjak commented 1 year ago

@datamweb Initialise new CodeIgniter4 project

composer create-project codeigniter4/appstarter shield-auth-config-poc && cd appstarter shield-auth-config-poc

Change minimum-stability in your project

composer config minimum-stability dev
composer config prefer-stable true

Install latest dev-develop shield

composer require codeigniter4/shield:dev-develop

Create .env with content:

database.default.database = ../writable/database.db
database.default.DBDriver = SQLite3

Migrate

php spark migrate -all

Update security, we are using Registrar.php in modules/Auth/Config folder with content:

<?php

namespace Auth\Config;

class Registrar
{
    public static function Security(): array
    {
        return [
            'csrfProtection' => 'session',
        ];
    }
}

Create Auth module folders

mkdir modules && cd modules && mkdir Auth && cd Auth && mkdir Config

Back to root

cd ...

Now manually copy Auth.php to modules/Auth/Config/

cp vendor/codeigniter4/shield/src/Config/Auth.php modules/Auth/Config/

Update Auth.php

namespace CodeIgniter\Shield\Config; -> namespace Auth\Config;

use CodeIgniter\Config\BaseConfig; -> use CodeIgniter\Shield\Config\Auth as ShieldAuth;

class Auth extends BaseConfig-> class Auth extends ShieldAuth

Then in app/Config/Autoload.php add:

    public $psr4 = [
        APP_NAMESPACE => APPPATH, // For custom app namespace
        'Config'      => APPPATH . 'Config',
+       'Auth'        => ROOTPATH . '/modules/Auth',
    ];

Then add Routes.php with content:

<?php

namespace Auth\Config;

use Config\Services;

$routes = Services::routes();

service('auth')->routes($routes);

to modules/Auth/Config folder.

Now we can run:

php spark serve

and go to http://localhost:8080/login and it works, but when we change someting in /modules/Auth/Config/Auth.php it's ignored.

For example we want to use our view for login, so we can create folder Views in modules/Auth/Views and copy default login and change

public array $views = [
        - 'login'                       => '\CodeIgniter\Shield\Views\login',
        + 'login'                       => '\Auth\Views\login',
        'register'                    => '\CodeIgniter\Shield\Views\register',
        'layout'                      => '\CodeIgniter\Shield\Views\layout',
        'action_email_2fa'            => '\CodeIgniter\Shield\Views\email_2fa_show',
        'action_email_2fa_verify'     => '\CodeIgniter\Shield\Views\email_2fa_verify',
        'action_email_2fa_email'      => '\CodeIgniter\Shield\Views\Email\email_2fa_email',
        'action_email_activate_show'  => '\CodeIgniter\Shield\Views\email_activate_show',
        'action_email_activate_email' => '\CodeIgniter\Shield\Views\Email\email_activate_email',
        'magic-link-login'            => '\CodeIgniter\Shield\Views\magic_link_form',
        'magic-link-message'          => '\CodeIgniter\Shield\Views\magic_link_message',
        'magic-link-email'            => '\CodeIgniter\Shield\Views\Email\magic_link_email',
    ];

it's not work. Whole Auth config in our module is ignored.

CleanShot 2023-07-04 at 09 23 29

I believe that issue is with change of logic like config('AuthRoutes') become config(AuthRoutes::class) and when is config(AuthRoutes::class) used that it cant find config in different namespaces than App.

Here is repo to test it.

jozefrebjak commented 1 year ago

@kenjis can you take a look ? I think it's a bug in framework or config(SOMETHING::class) should find configs only in App namespace ? It's correct ? Or it's a bug? I have the same issue in codeigniter4-assets library where it's the same in line.

kenjis commented 1 year ago

I thought config('App') and config(App::class) are exactly the same, but it seems not. I will take a look tomorrow.

Update: No, they are different.

kenjis commented 1 year ago

@codeigniter4/core-team Should the following code load the Auth\Config\Auth if it is available?

config(\CodeIgniter\Shield\Config\Auth::class)
  1. We would like to use config(\CodeIgniter\Shield\Config\Auth::class), not config('Auth'), because it can tell the type of the config object.
  2. The current Factories do not load Auth\Config\Auth even if it is available, when we specify CodeIgniter\Shield\Config\Auth.
  3. It is not intuitive that Auth\Config\Auth is loaded when we specify CodeIgniter\Shield\Config\Auth.

It seems better to revert #748 ?

paulbalandan commented 1 year ago

Looking at the class loading part of Factories it seems supplying the fully qualified class name of the config will satisfy immediately the class_exists check. If we supply Auth then Factories will load first Config\Auth if available since preferApp is true.

MGatner commented 1 year ago

Right. We use the generic 'Auth' to signal to Factories that we want any Config class with that name, and it will look in Config namespace first and then work through third-party modules to the framework. All third-party modules have the same priority so supplying the FQCN skips the lookup.

The only workaround I can think of (I use this in my Handlers package I think) is to locate all matching Config classes and then give priority to one that extends another (e.g. Auth\Modules\Auth extends CodeIgniter\Shield\Config\Auth would use the modules version).

EDIT: https://github.com/tattersoftware/codeigniter4-handlers/blob/develop/src/BaseFactory.php

kenjis commented 1 year ago

Created PR to revert: #762

kenjis commented 1 year ago

I have the same issue in codeigniter4-assets library where it's the same in line.

It is a bug in codeigniter4-assets.