Fix: No token (query) or authclient (session) found

[ArchBlood]

What kind of change does this PR introduce? (check at least one)

• [x] Bugfix
• [ ] Feature
• [ ] Code style update
• [ ] Refactor
• [ ] Build-related changes
• [ ] Other, please describe:

Does this PR introduce a breaking change? (check one)

• [ ] Yes
• [x] No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

• [x] It's submitted to the develop branch, not the master branch if no hotfix
• [x] When resolving a specific issue, it's referenced in the PR's description (e.g. Fix #xxx[,#xxx], where "xxx" is the Github issue number)
• [x] All tests are passing
• [ ] New/updated tests are included
• [x] Changelog was modified

If adding a new feature, the PR's description includes:

• [x] A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Other information: Registering with an OAuth client can lead to token session issues, this addresses the issue. fixes https://github.com/humhub-contrib/auth-facebook/issues/4

[what-the-diff[bot]]

PR Summary

• Fix to Handle Absence of Token or Auth Client The update includes a solution for situations where no token or authentication client is found, reducing the chances of unexpected errors and ensuring smoother operation.

• User Validation Update in Registration Process A verification routine has been added to the registration process, preventing the creation of duplicate accounts. This leads to a more efficient use of resources by avoiding unnecessary data storage.

• Login Function for Registered Users A login functionality for existing users has been established within the registration module. This allows already registered users to access the system more conveniently, enhancing user experience.

• Mandatory Token or Auth Client Check A mandatory check for the availability of a token or an authentication client has been added. This increases security, as it ensures only authenticated users have access to the system.

[martincodeinc]

Good morning .

When using the updated registrationcontroller i get the following error message after submitting the registration form. This is with a normal registration through the login screen.

yii\base\UnknownPropertyException: Getting unknown property: humhub\modules\user\Module::showRegistrationForm in /var/www/vhosts/commotio.nl/protected/vendor/yiisoft/yii2/base/Component.php:154 Stack trace: #0 /var/www/vhosts/commotio.nl/protected/vendor/yiisoft/yii2/di/ServiceLocator.php(77): yii\base\Component->__get('showRegistratio...') #1 /var/www/vhosts/commotio.nl/protected/humhub/modules/user/controllers/RegistrationController.php(134): yii\di\ServiceLocator->__get('showRegistratio...') #2 [internal function]: humhub\modules\user\controllers\RegistrationController->actionIndex() #3 /var/www/vhosts/commotio.nl/protected/vendor/yiisoft/yii2/base/InlineAction.php(57): call_user_func_array(Array, Array) #4 /var/www/vhosts/commotio.nl/protected/vendor/yiisoft/yii2/base/Controller.php(178): yii\base\InlineAction->runWithParams(Array) #5 /var/www/vhosts/commotio.nl/protected/vendor/yiisoft/yii2/base/Module.php(552): yii\base\Controller->runAction('', Array) #6 /var/www/vhosts/commotio.nl/protected/vendor/yiisoft/yii2/web/Application.php(103): yii\base\Module->runAction('user/registrati...', Array) #7 /var/www/vhosts/commotio.nl/protected/vendor/yiisoft/yii2/base/Application.php(384): yii\web\Application->handleRequest(Object(humhub\components\Request)) #8 /var/www/vhosts/commotio.nl/index.php(24): yii\base\Application->run() #9 {main}

Martin.

[martincodeinc]

HumHub 1.15.4 community edition

[martincodeinc]

I guess this is a fix on your branch; mentioned code snippets do not appear in the main branch code of humhub; e.g $existingUser = User::findByEmail

[ArchBlood]

Please note that this is using the develop branch which is setup for v1.16.x

[ArchBlood]

See the following variables that were added in v1.16.x; https://github.com/humhub/humhub/blob/d216e221ae4c288439af6794bf5bdc9e9e02173d/protected/humhub/modules/user/Module.php#L162-L194

[ArchBlood]

As for a test I've ran one personally with the latest changes in the develop branch and see no issues currently; The issue here was the else statement was always returning true on https://github.com/humhub-contrib/auth-facebook/issues/4 which is indeed fixed by this P/R.

I guess this is a fix on your branch; mentioned code snippets do not appear in the main branch code of humhub; e.g $existingUser = User::findByEmail

$existingUser = User::findByEmail isn't used in the core User model this was an error on my part, so I fixed this by changing it to $existingUser = User::findIdentity($registration->getUser()->email);.

[ArchBlood]

@yurabakhtin when you have the time can you look over the P/R in case I made any mistakes?

[luke-]

@ArchBlood Unfortunately, I have only now had time to look into the error in more detail.

The error 'Registration failed: No token (query) or authclient (session) found!' could also occur in the context of AuthClients if:

• The AuthClient does not return enough attributes for a complete automatic registration (e.g. user name or surname missing) and need to provided by the user
• and: The class of the AuthClient is not serializable

---

If, for example, an AuthClient cannot be serialized because of an API, the following method can be implemented to remove non-serializable properties beforehand. https://github.com/humhub/humhub/blob/master/protected/humhub/modules/user/authclient/BaseClient.php#L42-L49 Ideally, of course, the AuthClient would not have to be serializable, but there is currently no other solution.

---

Unfortunately, your current PR is difficult to review & accept because it involves major changes and the process as a whole is very complex.

Due to the removed else conditions, registrations without token or previously set AuthClient are now also possible.

What specific problem are you trying to solve? Or what is the problem with the current implementation?

[ArchBlood]

@ArchBlood Unfortunately, I have only now had time to look into the error in more detail.

The error 'Registration failed: No token (query) or authclient (session) found!' could also occur in the context of AuthClients if:

• The AuthClient does not return enough attributes for a complete automatic registration (e.g. user name or surname missing) and need to provided by the user
• and: The class of the AuthClient is not serializable

---

If, for example, an AuthClient cannot be serialized because of an API, the following method can be implemented to remove non-serializable properties beforehand. https://github.com/humhub/humhub/blob/master/protected/humhub/modules/user/authclient/BaseClient.php#L42-L49 Ideally, of course, the AuthClient would not have to be serializable, but there is currently no other solution.

---

Unfortunately, your current PR is difficult to review & accept because it involves major changes and the process as a whole is very complex.

Due to the removed else conditions, registrations without token or previously set AuthClient are now also possible.

What specific problem are you trying to solve? Or what is the problem with the current implementation?

The else statement was just relocated within the handleAuthClientRegistration() method directly to remove the condition where it's returning true even though the token & AuthClient are present.

As for the possibility of the issue being because of attributes, this would require some looking into as this issue shouldn't affect authclient modules that are handling attributes correctly, and if they're not then that means all authclient modules currently in the marketplace are affected which needs placed on a priority list. :thinking:

The only reason I say that it probably isn't related to attributes is that then authclients that are still in the core would be affected as well which doesn't seem to be the case.

[luke-]

@ArchBlood Unfortunately, I have only now had time to look into the error in more detail. The error 'Registration failed: No token (query) or authclient (session) found!' could also occur in the context of AuthClients if:

• The AuthClient does not return enough attributes for a complete automatic registration (e.g. user name or surname missing) and need to provided by the user
• and: The class of the AuthClient is not serializable

If, for example, an AuthClient cannot be serialized because of an API, the following method can be implemented to remove non-serializable properties beforehand. https://github.com/humhub/humhub/blob/master/protected/humhub/modules/user/authclient/BaseClient.php#L42-L49 Ideally, of course, the AuthClient would not have to be serializable, but there is currently no other solution.

Unfortunately, your current PR is difficult to review & accept because it involves major changes and the process as a whole is very complex. Due to the removed else conditions, registrations without token or previously set AuthClient are now also possible. What specific problem are you trying to solve? Or what is the problem with the current implementation?

The else statement was just relocated within the handleAuthClientRegistration() method directly to remove the condition where it's returning true even though the token & AuthClient are present.

I understand that the else has been moved, but with that it now allows registration without prior set AuthClients. The else condition prevented this case before.

We really need to understand why the AuthClient was not set before.

As for the possibility of the issue being because of attributes, this would require some looking into as this issue shouldn't affect authclient modules that are handling attributes correctly, and if they're not then that means all authclient modules currently in the marketplace are affected which needs placed on a priority list. 🤔

The only reason I say that it probably isn't related to attributes is that then authclients that are still in the core would be affected as well which doesn't seem to be the case.

The non-serializability of AuthClient does not affect all AuthClients. The only one I am currently aware of is SAML, as it works with anonymous functions that cannot be serialized.