Language Change ? Pending

User tests: Successful: Unsuccessful:

avatar SniperSister
SniperSister
29 Mar 2022

Pull Request for Issue #37413

Summary of Changes

Remove the auth binding field and checking code to disable the feature until a proper solution has been implemented see #37405.

Testing Instructions

See #34713

Actual result BEFORE applying this Pull Request

You are NOT logged into the site

Expected result AFTER applying this Pull Request

You are logged into the site

avatar SniperSister SniperSister - open - 29 Mar 2022
avatar SniperSister SniperSister - change - 29 Mar 2022
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 29 Mar 2022
Category Administration com_admin Front End Plugins
avatar SniperSister SniperSister - change - 29 Mar 2022
Labels Added: ?
avatar brianteeman
brianteeman - comment - 29 Mar 2022

Tested the plugin changes and they at least let me log back in again. Have not tested the script.php changes which I assume is to fix upgrades. Still doesn't address the myriad of other issues with this disaster of a code change

avatar zero-24
zero-24 - comment - 29 Mar 2022

I have tested this item successfully on deb4d5f

Confirmed the issue, tested the patch. Confirmed that the script.php checks the values and resets them back to empty when they are invalid


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37415.

avatar zero-24 zero-24 - test_item - 29 Mar 2022 - Tested successfully
avatar bembelimen bembelimen - change - 29 Mar 2022
The description was changed
avatar bembelimen bembelimen - edited - 29 Mar 2022
avatar bayareajenn
bayareajenn - comment - 30 Mar 2022

I have tested this item successfully on deb4d5f

I don't have a CA SSL cert so could not test Webauthn. I tested it the same way as 37416 and was able to duplicate the problem and then load the patch and login again.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37415.

avatar bayareajenn bayareajenn - test_item - 30 Mar 2022 - Tested successfully
avatar joomla-cms-bot joomla-cms-bot - change - 30 Mar 2022
Category Administration com_admin Front End Plugins Administration com_admin com_users Libraries Front End Plugins
avatar joomla-cms-bot joomla-cms-bot - change - 30 Mar 2022
Category Administration com_admin Front End Plugins com_users Libraries Administration com_users Libraries Front End Plugins
avatar SniperSister SniperSister - change - 30 Mar 2022
The description was changed
avatar SniperSister SniperSister - edited - 30 Mar 2022
avatar SniperSister SniperSister - change - 30 Mar 2022
Title
[4.1] Only bind users to primary auth providers
[4.1] Disable auth binding code introduced with 4.1.1
avatar SniperSister SniperSister - edited - 30 Mar 2022
avatar brianteeman
brianteeman - comment - 30 Mar 2022

Canyou remove the new language strings as well please
COM_USERS_USER_FIELD_AUTHPROVIDER_DESC
COM_USERS_USER_FIELD_AUTHPROVIDER_LABEL

avatar joomla-cms-bot joomla-cms-bot - change - 30 Mar 2022
Category Administration Front End Plugins com_users Libraries Administration com_users Language & Strings Front End Libraries Plugins
avatar SniperSister
SniperSister - comment - 30 Mar 2022

Canyou remove the new language strings as well please

Done! Also checked in with @tecpromotion as tt-coordinator.

avatar brianteeman
brianteeman - comment - 30 Mar 2022

and remove
administrator/components/com_users/src/Field/PrimaryauthprovidersField.php

avatar brianteeman
brianteeman - comment - 30 Mar 2022

components/com_users/tmpl/profile/edit.php
<?php if (count($this->twofactormethods) > 1 && !empty($this->twofactorform)) : ?>

not sure if that change is still needed

avatar brianteeman
brianteeman - comment - 30 Mar 2022

libraries/src/Authentication/ProviderAwareAuthenticationPluginInterface.php

avatar brianteeman
brianteeman - comment - 30 Mar 2022

libraries/src/Table/User.php

 * Updates auth provider of a user
avatar SniperSister
SniperSister - comment - 30 Mar 2022

administrator/components/com_users/src/Field/PrimaryauthprovidersField.php
libraries/src/Authentication/ProviderAwareAuthenticationPluginInterface.php

Would leave those two files for now as removing them (and therefore adding them to the deletion list in script.php) would prevent us from re-using the same file names for the future patch.

avatar brianteeman
brianteeman - comment - 30 Mar 2022

No you have to remove them (and come up with new names in future) otherwise you will be breaking semver when you change the contents of the library files.

avatar brianteeman
brianteeman - comment - 30 Mar 2022

libraries/src/User/User.php
public function setAuthProvider($authProvider)

avatar brianteeman
brianteeman - comment - 30 Mar 2022

plugins/authentication/cookie/cookie.php
use Joomla\CMS\Authentication\ProviderAwareAuthenticationPluginInterface;
public static function isPrimaryProvider()

and the same in other plugins

avatar SniperSister SniperSister - change - 30 Mar 2022
Labels Added: Language Change
avatar SniperSister
SniperSister - comment - 30 Mar 2022

No you have to remove them (and come up with new names in future) otherwise you will be breaking semver when you change the contents of the library files.

Ok, fair comment, done!

avatar brianteeman
brianteeman - comment - 30 Mar 2022

Thanks @SniperSister

avatar joomla-cms-bot joomla-cms-bot - change - 30 Mar 2022
Category Administration Front End Plugins com_users Libraries Language & Strings Administration com_admin com_users Language & Strings Front End Libraries Plugins
avatar HLeithner
HLeithner - comment - 30 Mar 2022

semver!!

where do you see a semver violation?

avatar brianteeman
brianteeman - comment - 30 Mar 2022

semver!!

where do you see a semver violation?

it should be properly marked as deprecated so that an ide etc will spot it.

avatar HLeithner
HLeithner - comment - 30 Mar 2022

hat an ide etc will spot it.

The interface is deprecated all other parts get removed because not covered by our semver promise. Also semver doesn't allow us to add deprecations in bugfix releases. that's the reason David added the comment "Please note: might be deprecated with Joomla 4.2"

or do you mean something different?

avatar bembelimen
bembelimen - comment - 30 Mar 2022

Thanks everyone for this good collaboration.
@nikosdion and @brianteeman are you both ok with the solution?

avatar nikosdion
nikosdion - comment - 30 Mar 2022

@bembelimen I am OK with this solution.

avatar brianteeman
brianteeman - comment - 30 Mar 2022

@bembelimen seems ok.

avatar bembelimen bembelimen - close - 30 Mar 2022
avatar bembelimen bembelimen - merge - 30 Mar 2022
avatar bembelimen bembelimen - change - 30 Mar 2022
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2022-03-30 11:56:49
Closed_By bembelimen
avatar bembelimen
bembelimen - comment - 30 Mar 2022

Thank you all!

avatar dgrammatiko
dgrammatiko - comment - 30 Mar 2022

@SniperSister @bembelimen the binding issue has extreme similarities to the Media Manager Adapters, maybe you should take a look how @joomdonation @Fedik @laoneo and me tackled that binding and get some inspiration...

Basically #34069 and #33724 (maybe a couple more)

Add a Comment

Login with GitHub to post a comment