Enh: Configuration Settings #3

ArchBlood · 2023-11-10T11:12:15Z

Resolves: #2

ArchBlood · 2023-11-10T15:06:32Z

This is how it looks after implementation;

ArchBlood · 2023-11-11T00:38:59Z

Seems like it still throws Error: Typed property humhub\modules\sso\jwt\models\Configuration::$settingsManager must not be accessed before initialization 🤔

ArchBlood · 2023-11-11T18:05:45Z

Seems like it still throws Error: Typed property humhub\modules\sso\jwt\models\Configuration::$settingsManager must not be accessed before initialization 🤔

This error only happens for guest when the login modal is activated it seems.

ArchBlood · 2023-11-16T06:14:26Z

Currently still unable to fix this issue, not sure why it's showing up other than autoLogin possibly being the problem.

ArchBlood · 2023-11-23T04:57:55Z

[So after fixing some of the issues it seems that get() throws the following error now;

Error: Call to a member function get() on null in /home/greenmet/public_html/protected/modules/jwt-sso/models/Configuration.php:120

From what I'm getting it seems that get() must be a string type and can't be anything else?

https://github.com/humhub/humhub/blob/ae8f2fa34255a33aff639fb5a5cfde65d39a33c9/protected/humhub/components/SettingsManager.php#L150-L158

ArchBlood · 2023-11-25T02:04:18Z

@luke- from looking at this deeper, it's not possible to implement it in this way, I can implement it through settings but not settingsManager

ArchBlood · 2024-01-09T08:44:57Z

@yurabakhtin could you look over this to see if I made any errors within the code? At the moment I'm completely stumped. 🤔

yurabakhtin · 2024-01-09T13:12:28Z

could you look over this to see if I made any errors within the code? At the moment I'm completely stumped. 🤔

@ArchBlood If you tell about this error:

Seems like it still throws Error: Typed property humhub\modules\sso\jwt\models\Configuration::$settingsManager must not be accessed before initialization 🤔

then please try to update the code public SettingsManager $settingsManager; to public $settingsManager;
or to public ?SettingsManager $settingsManager = null;.

ArchBlood · 2024-01-09T22:09:28Z

could you look over this to see if I made any errors within the code? At the moment I'm completely stumped. 🤔

@ArchBlood If you tell about this error:

Seems like it still throws Error: Typed property humhub\modules\sso\jwt\models\Configuration::$settingsManager must not be accessed before initialization 🤔

then please try to update the code public SettingsManager $settingsManager; to public $settingsManager; or to public ?SettingsManager $settingsManager = null;.

Using public ?SettingsManager $settingsManager = null; provides the following error for enable state when visiting the site;

Error: Call to a member function get() on null in /home/greenmet/public_html/protected/modules/jwt-sso/models/Configuration.php:121

I've changed the loadBySettings() to the following which fixes the null returns;

    public function loadBySettings()
    {
        if ($this->settingsManager !== null) {
            $this->enabled = (bool)$this->settingsManager->get('enabled');
            $this->url = ($this->settingsManager->get('url') ?? '');
            $this->sharedKey = $this->settingsManager->getSerialized('sharedKey') ?? [];
            $this->supportedAlgorithms = ($this->settingsManager->get('supportedAlgorithms') ?? []);
            $this->idAttribute = ($this->settingsManager->get('idAttribute') ?? '');
            $this->leeway = $this->settingsManager->get('leeway') ?? 0;
            $this->allowedIPs = $this->settingsManager->get('allowedIPs') ?? [];
            $this->autoLogin = (bool)$this->settingsManager->get('autoLogin');
        }
    }

But the following error then happens for me;

yii\base\ErrorException: Undefined property: humhub\components\Application::$authClientCollection in /home/greenmet/public_html/protected/modules/jwt-sso/Module.php:53

yurabakhtin · 2024-01-10T08:06:27Z

But the following error then happens for me;

yii\base\ErrorException: Undefined property: humhub\components\Application::$authClientCollection in /home/greenmet/public_html/protected/modules/jwt-sso/Module.php:53

@ArchBlood I think you should either define the property authClientCollection somehow there, or run the code only when if (isset(Yii::$app->authClientCollection)) or if (Yii::$app->authClientCollection instanceof humhub\modules\user\authclient\Collection) .

ArchBlood · 2024-01-10T08:33:39Z

But the following error then happens for me;
yii\base\ErrorException: Undefined property: humhub\components\Application::$authClientCollection in /home/greenmet/public_html/protected/modules/jwt-sso/Module.php:53
@ArchBlood I think you should either define the property authClientCollection somehow there, or run the code only when if (isset(Yii::$app->authClientCollection)) or if (Yii::$app->authClientCollection instanceof humhub\modules\user\authclient\Collection) .

Seems like neither of these options work correctly either, I set like this and the login page is now viewable, but I'm not sure this is the correct way of doing this;

        if (!isset(Yii::$app->authClientCollection)) {
            $jwtAuth = Yii::$app->authClientCollection->getClient('jwt');

            if ($jwtAuth->checkIPAccess()) {
                if ($jwtAuth->autoLogin && $event->action->id === 'login' && empty(Yii::$app->request->get('noJwt'))) {
                    if ($event->isValid) {
                        $event->isValid = false;
                        return $jwtAuth->redirectToBroker();
                    }
                }
            } else {
                // Not allowed, remove authClient
                Yii::$app->authClientCollection->removeClient('jwt');
            }
        }

yurabakhtin · 2024-01-10T09:55:37Z

Seems like neither of these options work correctly either, I set like this and the login page is now viewable, but I'm not sure this is the correct way of doing this;

@ArchBlood I didn't run/test your code but for me it looks incorrectly when you do if (!isset(Yii::$app->authClientCollection)) { and then try access to the undefined Yii::$app->authClientCollection.

Why don't you use the condition as I suggested if (isset(Yii::$app->authClientCollection)) {?

Really I think you should use instead of old code:

if (Yii::$app->authClientCollection->hasClient('jwt')) {

new code like:

if (isset(Yii::$app->authClientCollection) && Yii::$app->authClientCollection->hasClient('jwt')) {

or:

if (Yii::$app->authClientCollection instanceof humhub\modules\user\authclient\Collection && Yii::$app->authClientCollection->hasClient('jwt')) {

ArchBlood · 2024-01-10T16:21:43Z

Seems like neither of these options work correctly either, I set like this and the login page is now viewable, but I'm not sure this is the correct way of doing this;

@ArchBlood I didn't run/test your code but for me it looks incorrectly when you do if (!isset(Yii::$app->authClientCollection)) { and then try access to the undefined Yii::$app->authClientCollection.

Why don't you use the condition as I suggested if (isset(Yii::$app->authClientCollection)) {?

Really I think you should use instead of old code:

if (Yii::$app->authClientCollection->hasClient('jwt')) {

new code like:

if (isset(Yii::$app->authClientCollection) && Yii::$app->authClientCollection->hasClient('jwt')) {

or:

if (Yii::$app->authClientCollection instanceof humhub\modules\user\authclient\Collection && Yii::$app->authClientCollection->hasClient('jwt')) {

Because these throw the following error, using the current code or if (!isset(Yii::$app->authClientCollection) && Yii::$app->authClientCollection->hasClient('jwt')) { doesn't;

yii\base\ErrorException: Undefined property: humhub\components\Application::$authClientCollection in /home/USER/public_html/protected/modules/jwt-sso/Module.php:68
Stack trace:
#0 /home/USER/public_html/protected/modules/jwt-sso/Module.php(68): yii\base\ErrorHandler->handleError()
#1 [internal function]: humhub\modules\sso\jwt\Module::onAuthClientCollectionInit()
#2 /home/USER/public_html/protected/vendor/yiisoft/yii2/base/Event.php(312): call_user_func()
#3 /home/USER/public_html/protected/vendor/yiisoft/yii2/base/Component.php(642): yii\base\Event::trigger()
#4 /home/USER/public_html/protected/humhub/modules/user/authclient/Collection.php(48): yii\base\Component->trigger()
#5 /home/USER/public_html/protected/vendor/yiisoft/yii2/base/Component.php(180): humhub\modules\user\authclient\Collection->setClients()
#6 /home/USER/public_html/protected/vendor/yiisoft/yii2/BaseYii.php(558): yii\base\Component->__set()
#7 /home/USER/public_html/protected/vendor/yiisoft/yii2/base/BaseObject.php(107): yii\BaseYii::configure()
#8 [internal function]: yii\base\BaseObject->__construct()
#9 /home/USER/public_html/protected/vendor/yiisoft/yii2/di/Container.php(419): ReflectionClass->newInstanceArgs()
#10 /home/USER/public_html/protected/vendor/yiisoft/yii2/di/Container.php(170): yii\di\Container->build()
#11 /home/USER/public_html/protected/vendor/yiisoft/yii2/BaseYii.php(365): yii\di\Container->get()
#12 /home/USER/public_html/protected/vendor/yiisoft/yii2/di/ServiceLocator.php(137): yii\BaseYii::createObject()
#13 /home/USER/public_html/protected/vendor/yiisoft/yii2/base/Module.php(766): yii\di\ServiceLocator->get()
#14 /home/USER/public_html/protected/vendor/yiisoft/yii2/di/ServiceLocator.php(74): yii\base\Module->get()
#15 /home/USER/public_html/protected/modules/jwt-sso/Module.php(68): yii\di\ServiceLocator->__get()
#16 [internal function]: humhub\modules\sso\jwt\Module::onAuthClientCollectionInit()
#17 /home/USER/public_html/protected/vendor/yiisoft/yii2/base/Event.php(312): call_user_func()
#18 /home/USER/public_html/protected/vendor/yiisoft/yii2/base/Component.php(642): yii\base\Event::trigger()
#19 /home/USER/public_html/protected/vendor/yiisoft/yii2/base/Controller.php(297): yii\base\Component->trigger()
#20 /home/USER/public_html/protected/vendor/yiisoft/yii2/web/Controller.php(219): yii\base\Controller->beforeAction()
#21 /home/USER/public_html/protected/humhub/components/Controller.php(210): yii\web\Controller->beforeAction()
#22 /home/USER/public_html/protected/humhub/modules/user/controllers/AuthController.php(94): humhub\components\Controller->beforeAction()
#23 /home/USER/public_html/protected/vendor/yiisoft/yii2/base/Controller.php(176): humhub\modules\user\controllers\AuthController->beforeAction()
#24 /home/USER/public_html/protected/vendor/yiisoft/yii2/base/Module.php(552): yii\base\Controller->runAction()
#25 /home/USER/public_html/protected/vendor/yiisoft/yii2/web/Application.php(103): yii\base\Module->runAction()
#26 /home/USER/public_html/protected/vendor/yiisoft/yii2/base/Application.php(384): yii\web\Application->handleRequest()
#27 /home/USER/public_html/index.php(25): yii\base\Application->run()
#28 {main}

yurabakhtin · 2024-01-10T16:34:07Z

Because these throw the following error, using the current code or if (!isset(Yii::$app->authClientCollection) && Yii::$app->authClientCollection->hasClient('jwt')) { doesn't;

@ArchBlood You write here !isset(...) but I advised to use isset(...).

ArchBlood · 2024-01-10T17:13:07Z

Because these throw the following error, using the current code or if (!isset(Yii::$app->authClientCollection) && Yii::$app->authClientCollection->hasClient('jwt')) { doesn't;

@ArchBlood You write here !isset(...) but I advised to use isset(...).

I can switch it to that, but the error will continue to happen, should I apply error handling as well?

ArchBlood · 2024-01-10T17:28:46Z

After testing the following now works, but still throws Error occurred: Undefined property: humhub\components\Application::$authClientCollection just allows access to login even if it errors;

    public static function onAuthClientCollectionInit($event)
    {
        try {
            if (!Yii::$app->user->isGuest) {
                return;
            }

            /** @var Collection $authClientCollection */
            $authClientCollection = $event->sender;

            $config = Configuration::getInstance();

            if (!empty($config->enabled)) {
                $authClientCollection->setClient('jwt', [
                    'class' => authclient\JWT::class,
                    'url' => $config->url,
                    'sharedKey' => $config->sharedKey,
                    'supportedAlgorithms' => $config->supportedAlgorithms,
                    'idAttribute' => $config->idAttribute,
                    'leeway' => $config->leeway,
                    'allowedIPs' => $config->allowedIPs
                ]);
            }

            if (isset(Yii::$app->authClientCollection) && Yii::$app->authClientCollection->hasClient('jwt')) {
                $jwtAuth = Yii::$app->authClientCollection->getClient('jwt');

                if ($jwtAuth->checkIPAccess()) {
                    if ($jwtAuth->autoLogin && $event->action->id === 'login' && empty(Yii::$app->request->get('noJwt'))) {
                        if ($event->isValid) {
                            $event->isValid = false;
                            return $jwtAuth->redirectToBroker();
                        }
                    }
                } else {
                    Yii::$app->authClientCollection->removeClient('jwt');
                }
            }
        } catch (\Exception $e) {
            // Log or handle the exception as needed
            Yii::error('Error occurred: ' . $e->getMessage());
        }
    }

yurabakhtin · 2024-01-11T07:28:27Z

After testing the following now works, but still throws Error occurred: Undefined property: humhub\components\Application::$authClientCollection just allows access to login even if it errors;

@ArchBlood Maybe you should compare the $event->sender is really object of Collection with code like:

if (!($event->sender instanceof Collection)) {
    return;
}

because I see the module config calls the method onAuthClientCollectionInit on two events:

[Collection::class, Collection::EVENT_AFTER_CLIENTS_SET, ['humhub\modules\sso\jwt\Module', 'onAuthClientCollectionInit']],
[AuthController::class, AuthController::EVENT_BEFORE_ACTION, ['humhub\modules\sso\jwt\Module', 'onAuthClientCollectionInit']],

I guess the event AuthController::EVENT_BEFORE_ACTION doesn't have the $event->sender as Collection, I think there we have $event->sender instanceof AuthController, please check, and probably you should remove the second config line completely, but I am not sure because I didn't test this code deeply.

I just see you added the first config line for Collection::EVENT_AFTER_CLIENTS_SET and then added the code with $authClientCollection = $event->sender; there but I think we should use a separate callback method per event, also it would be better to keep more correct names for the callback methods and we prefer to keep them all in separate file Events.php instead of Module.php, I mean the config should looks like this:

[Collection::class, Collection::EVENT_AFTER_CLIENTS_SET, ['humhub\modules\sso\jwt\Events', 'onCollectionAfterClientsSet']],
[AuthController::class, AuthController::EVENT_BEFORE_ACTION, ['humhub\modules\sso\jwt\Events', 'onAuthControllerBeforeAction']],

then you should move the code(or part of the code) from Module::onAuthClientCollectionInit() to Events::onCollectionAfterClientsSet(), and then decide what to do with the second event AuthController::EVENT_BEFORE_ACTION, you should either delete it completely or create new method Events::onAuthControllerBeforeAction()` and decide what code should be there, maybe some part of the code should be kept there.

Thanks.

ArchBlood · 2024-01-11T11:54:28Z

After testing the following now works, but still throws Error occurred: Undefined property: humhub\components\Application::$authClientCollection just allows access to login even if it errors;

@ArchBlood Maybe you should compare the $event->sender is really object of Collection with code like:
if (!($event->sender instanceof Collection)) {
    return;
}
because I see the module config calls the method onAuthClientCollectionInit on two events:
[Collection::class, Collection::EVENT_AFTER_CLIENTS_SET, ['humhub\modules\sso\jwt\Module', 'onAuthClientCollectionInit']],
[AuthController::class, AuthController::EVENT_BEFORE_ACTION, ['humhub\modules\sso\jwt\Module', 'onAuthClientCollectionInit']],
I guess the event AuthController::EVENT_BEFORE_ACTION doesn't have the $event->sender as Collection, I think there we have $event->sender instanceof AuthController, please check, and probably you should remove the second config line completely, but I am not sure because I didn't test this code deeply.

I just see you added the first config line for Collection::EVENT_AFTER_CLIENTS_SET and then added the code with $authClientCollection = $event->sender; there but I think we should use a separate callback method per event, also it would be better to keep more correct names for the callback methods and we prefer to keep them all in separate file Events.php instead of Module.php, I mean the config should looks like this:
[Collection::class, Collection::EVENT_AFTER_CLIENTS_SET, ['humhub\modules\sso\jwt\Events', 'onCollectionAfterClientsSet']],
[AuthController::class, AuthController::EVENT_BEFORE_ACTION, ['humhub\modules\sso\jwt\Events', 'onAuthControllerBeforeAction']],
then you should move the code(or part of the code) from Module::onAuthClientCollectionInit() to Events::onCollectionAfterClientsSet(), and then decide what to do with the second event AuthController::EVENT_BEFORE_ACTION, you should either delete it completely or create new method Events::onAuthControllerBeforeAction()` and decide what code should be there, maybe some part of the code should be kept there.

Thanks.

Yes, you are correct, but in all honesty the whole onAuthClientCollectionInit() should be in the Events.php not Module.php, once I have the time later today to make the changes I'll run some tests then update the code here!

ArchBlood · 2024-01-11T14:05:20Z

Now no errors happen at all! 😄

ArchBlood · 2024-01-11T15:52:34Z

All that's needed now is live testing, which I can't current do as I have no JWT instances setup.

ArchBlood · 2024-01-25T10:13:07Z

If anyone can run some tests for this it would be nice, otherwise I believe that everything looks to be working as intended. 🤔

ArchBlood · 2024-07-28T04:21:32Z

I've updated both the JWT init() & Events to fix some possible redirection errors, while also adding some error handling where needed.

ArchBlood added 5 commits November 10, 2023 05:36

Enh: Configuration Settings

0f7c1f5

Update Configuration.php

9989e84

Update index.php

f753a82

Update Module.php

97e39ab

Update JWT.php

e136c0a

ArchBlood marked this pull request as draft November 10, 2023 11:13

Update index.php

623d8f9

ArchBlood marked this pull request as ready for review November 10, 2023 15:06

ArchBlood added 2 commits November 10, 2023 19:24

Fix: enabled state

375c4b2

Fix: empty fields on save

9a86fd4

ArchBlood added 2 commits November 22, 2023 23:59

Update Configuration.php

19ecfa0

Merge branch 'master' into settings

c887770

ArchBlood marked this pull request as draft January 9, 2024 08:41

ArchBlood added 2 commits January 10, 2024 03:43

Update Configuration.php

5f08322

Update Module.php

d013021

ArchBlood marked this pull request as ready for review January 10, 2024 08:51

Update Module.php

3344030

Update Module.php

d2246f2

ArchBlood added 3 commits January 11, 2024 09:03

Update Events.php

c5bd1dd

Update Module.php

47660ab

Update config.php

d41f254

ArchBlood added 3 commits January 11, 2024 10:39

Fix: Missing models\Configuration class

3954036

Enh: Use onCollectionAfterClientsSet() as suggested

a37af71

Enh: Use onCollectionAfterClientsSet() as suggested

436e54a

ArchBlood mentioned this pull request Jan 30, 2024

Tests #4

Open

ArchBlood added 2 commits July 27, 2024 13:04

Fix: allowedIPs & Error Logging

151e15c

Fix: Redirection Issues

aff29dc

ArchBlood added 4 commits July 28, 2024 00:59

Enh: Extend AuthController

158dda2

Update AdminController.php

6273277

Update Configuration.php

bd80d2e

Update index.php

b0ca009

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enh: Configuration Settings #3

Enh: Configuration Settings #3

ArchBlood commented Nov 10, 2023

ArchBlood commented Nov 10, 2023

ArchBlood commented Nov 11, 2023

ArchBlood commented Nov 11, 2023

ArchBlood commented Nov 16, 2023

ArchBlood commented Nov 23, 2023

ArchBlood commented Nov 25, 2023

ArchBlood commented Jan 9, 2024

yurabakhtin commented Jan 9, 2024

ArchBlood commented Jan 9, 2024

yurabakhtin commented Jan 10, 2024

ArchBlood commented Jan 10, 2024

yurabakhtin commented Jan 10, 2024

ArchBlood commented Jan 10, 2024

yurabakhtin commented Jan 10, 2024

ArchBlood commented Jan 10, 2024

ArchBlood commented Jan 10, 2024 •

edited

Loading

yurabakhtin commented Jan 11, 2024

ArchBlood commented Jan 11, 2024

ArchBlood commented Jan 11, 2024

ArchBlood commented Jan 11, 2024

ArchBlood commented Jan 25, 2024

ArchBlood commented Jul 28, 2024 •

edited

Loading

Enh: Configuration Settings #3

Are you sure you want to change the base?

Enh: Configuration Settings #3

Conversation

ArchBlood commented Nov 10, 2023

ArchBlood commented Nov 10, 2023

ArchBlood commented Nov 11, 2023

ArchBlood commented Nov 11, 2023

ArchBlood commented Nov 16, 2023

ArchBlood commented Nov 23, 2023

ArchBlood commented Nov 25, 2023

ArchBlood commented Jan 9, 2024

yurabakhtin commented Jan 9, 2024

ArchBlood commented Jan 9, 2024

yurabakhtin commented Jan 10, 2024

ArchBlood commented Jan 10, 2024

yurabakhtin commented Jan 10, 2024

ArchBlood commented Jan 10, 2024

yurabakhtin commented Jan 10, 2024

ArchBlood commented Jan 10, 2024

ArchBlood commented Jan 10, 2024 • edited Loading

yurabakhtin commented Jan 11, 2024

ArchBlood commented Jan 11, 2024

ArchBlood commented Jan 11, 2024

ArchBlood commented Jan 11, 2024

ArchBlood commented Jan 25, 2024

ArchBlood commented Jul 28, 2024 • edited Loading

ArchBlood commented Jan 10, 2024 •

edited

Loading

ArchBlood commented Jul 28, 2024 •

edited

Loading