New 3rd-party PHP dependency manager for e107 core #4099
Conversation
Deltik
changed the title
Actually it does. subfolders of e107_handlers (phpmailer, phpthumb etc) ARE the vendor folders. |
- MOD: Replaced e107::getPref('social_login') with
SocialLoginConfigManager::getValidConfiguredProviderConfigs()
- FIX: signup_shortcodes updated with new social login providers
- MOD: e107::filter_request() code de-duplication: HTTP 400 exits
- MOD: Deprecated e107::getHybridAuth() to discourage direct access to
third-party dependency Hybridauth
- FIX: Updated e_user_provider for Hybridauth 3
- FIX: e_user::tryProviderSession() and Hybridauth 3
- NEW: Dynamic auth provider support in social_adminarea
- NEW: Database migration for social plugin's social_login pref
In PHP 7.0.12 and earlier, a class could not be loaded with the same name as another class in a different namespace. This commit patches the Hybridauth dependency to avoid PHP bug 66773. Unfortunately, the fix will be rolled back any time Hybridauth is updated or overwritten. PHP bug link: https://bugs.php.net/bug.php?id=66773
- FIX: Duplicate invalid login messages in userlogin::login() - NEW: e_user_provider: Return URL passthrough to go back where intended - MOD: Cleanup of some confusing APIs in e_user_provider - MOD: Return URL passthrough in system/xup/* - MOD: system/xup/test: Logout test renamed into something clearer
Now guests can't snoop there unless the admin allows it. Documentation has been improved accordingly.
Also fixed variable passing weirdness in API of e_user_provider
|
@Deltik - While updating the code, Social Plugin did not save the Keys/ID code, (I used Steam Login on a site, so idk if the secrets, and scopes are saved or not). Also notice once updated, I can no longer login to Steam - get Error: [6]Couldn't resolve host 'XXX' |
|
@LaocheXe: This pull request changes the |
The site-wide social login system is now backwards-compatible with how the social_login_active core pref worked since commit 3b2d833. social_login_active's least significant bit is now treated as a global bit, so if it's not set, no other bits are allowed to be set. This un-breaks all existing checks for whether social_login_active is empty. Except in themes, the social_login_active check has been replaced with an e_user_provider API to check if social login is enabled site-wide.
|
@Deltik alright, I removed the key/id that I need for the old method, try to log in and get an Error Code 5 - SQL Insert Error. In System Log I see XUP_SIGNUP Failure, under that I see ADMINUI_04: Admin-UI DB Error: user The testing site I have set up uses the same method for logging in as the current main site (which uses the current e107 social plugin), I have it set just to Login, and it never ask for email address because Steam doesn't give out that type of information. I dont know, but do all OpenID's not require a key/id? if not then we could remove that from them so users would only have to hit the on or off switch |
(Not utilized in the admin frontend yet)
|
@LaocheXe: The SQL insert error has been fixed in daa31be. f03e60d is my ridiculous solution to figuring out the odd fields in some social login providers (namely For example, Another example is that Some social login providers have weird nuances that can't be expressed in the three existing input columns, "Key/ID", "Secret", and "Scope". |
During e_user_provider::signup()
|
@Deltik with your version I see that message, however, with current e107 - it allows the user to login (and add's their account to the database). That's is what I would like it to do, it's a way to combat spam bots. |
|
@LaocheXe: I am still not following. The error message didn't show before due to a bug that was fixed in c260152. If you use The "Test signup/login with Steam" button will actually register and log in the new user. What "login only" setting are you talking about? |
|
@Deltik Preferences > User Registration/Login > User registration system - "Login Only". https://www.501stlegion-a3.com - using master branch of e107. "3rd. I have it set up to login only, Which allowed new users to just use Steam to login, so even with the first two changes, it would login me in as a new account." What I mean is I failed to login, and it threw up an error that it couldn't find "Steam_xxx" but in the database it was listed as "steam_https://xxx" - it should have created that account in the database since it couldn't find it? Both sites use the same setting as listed above, on the main site, new users just login, account is created. Testing site, new user login's but fails, because their account is not on the database. |
|
@LaocheXe: Your staging site is using the login-only endpoint ( The bug with |
system/xup/signup is now system/xup/login
|
Code Climate has analyzed commit 4d7ce7e and detected 23 issues on this pull request. Here's the issue category breakdown:
The test coverage on the diff in this pull request is 12.9% (80% is the threshold). This pull request will bring the total coverage in the repository to 6.2% (0.2% change). View more on Code Climate. |
App root files introduced in e107inc#4099
App root files introduced in e107inc#4099
App root files introduced in #4099
App root files introduced in #4099
App root files introduced in #4099
App root files introduced in #4099
App root files introduced in #4099
App root files introduced in #4099
App root files introduced in #4099
App root files introduced in #4099
App root files introduced in #4099
App root files introduced in #4099
App root files introduced in #4099
App root files introduced in #4099
Motivation and Context
3rd-party dependencies are put into the e107 core repository but periodically need updating, which is a bit of a hassle without a dependency manager.
The e107 core also does not have a standard structure for where 3rd-party libraries are stored. Examples:
./e107_handlers/pclzip.lib.php./e107_handlers/jsshrink/These external libraries are mixed in with core handlers/interfaces, which can make it confusing for plugin authors to know which public interfaces are stable and part of e107. One notable case is the removal of the calendar library, which required all plugins that used it to be modified to use the equivalent stable e107 core interface (thanks @Jimmi08).
Description
This pull request introduces Composer as the external dependency manager for the e107 core. External dependencies now go into a dedicated folder:
./e107_handler/vendor/.The public interfaces in
./e107_handlers/vendor/are not considered stable.Plugins should not call code inside
./e107_handlers/vendor/directly.They should only use interfaces (handlers) provided by the e107 framework. In e107 v2, the handler factory is the
e107class.Because this change fundamentally changes how third-party libraries are included in e107, there is a transition plan.
e107 has historically bundled the full source code of external dependencies in the core repository.
Some code, particularly syncing from the GitHub remote, expects dependencies to be included in the core source.
This behavior will be maintained until all existing code depending on the behavior is updated to support resolving dependencies with Composer.
Transition plan:
<2.3./e107_handlers/^2.3./e107_handlers/vendor/config.platform.phpoption in./composer.json).^3./e107_handlers/vendor/./e107_handlers/vendor/folder. The e107 installer runscomposer installat the beginning of the install process. The e107 self-updater runscomposer installafter deploying the desired e107 version. Only e107 releases may have dependencies bundled in the release package for offline/Intranet/firewalled installations.To demonstrate these new changes, the Hybridauth (formerly known as HybridAuth) library has been updated from version 2.9.6 to version 3.1.1 in this pull request. Fixes: #3492
How Has This Been Tested?
Tests have not been written for the updated Hybridauth because I haven't figured out how Hybridauth works yet…
Types of Changes
Checklist