simpliko / wpadverts

WordPress Classifieds Plugin
https://wpadverts.com/
GNU General Public License v2.0
20 stars 11 forks source link

Account creation ability should be based on WP Setting. #142

Closed erikdemarco closed 2 years ago

erikdemarco commented 2 years ago

In "add listing" page, there is a feature for user to sign up

(Create an account for me so I can manage all my ads from one place). This text should not exist if admin disable "Anyone can register" in wordpress general setting.

gwin commented 2 years ago

The admin setting should not influence the [adverts_add] account field, because the user might not want users to register with the default WP login but still register via third-party plugin or the [adverts_add].

That said if you do not want to have "Create account" field in the [adverts_add] you can hide it either with Custom Fields extension or by adding the code below in your theme functions.php file

add_filter( "adverts_form_load", "hide_adverts_account_field" );
function hide_adverts_account_field( $form ) {
  if( $form['name'] != "advert" ) {
    return $form;
  }
  foreach( $form["field"] as $key => $field ) {
    if( $field["name"] == "_adverts_account" ) {
        unset( $form["field"][$key] );
    }
  }
  return $form;
}
erikdemarco commented 2 years ago

@gwin I'm a bit not agree because:

  1. There is Wordpress Philosophy "Decisions, not Options" here https://wordpress.org/about/philosophy/ We should not give admin too many options, its more better if our system intelligently decide the optimal option for them based on site's current settings, moreover this option is already built in in wordpress core setting. Just for example imagine if all plugin developers adopt your philosophy. And all this plugins allow this account creation from each of their plugin. Then if Admin happens to have multiple of this kind of plugins installed, and want to disable 'user registration', they need to disable it multiple times (even wpadverts doesnt have option to disable 'account creation'). You know most of security bug is happens to site with login capabilities.

  2. When we just remove the field, hacker can just tamper the form to include '_adverts_account' field again.

I think the win-win solution here is: To have another option for this 'Account creation' for wpadverts. And this setting intelligently decide the initial setting value during the installation.

gwin commented 2 years ago
  1. this sounds great in theory but does not work in practice, there are multiple users who want to allow users to register via [adverts_add] or some other registration plugin but do not want users to use the wp-login.php page for this, so the second I make this update I will get users asking how to restore the previous functionality.
  2. once adverts_account is removed with adverts_form_load filter the only way to restore it for a user is to hack into the server and modify the PHP code tampering with the form data will not do anything, unless you identified a bug that allows doing that? If so, please post how to reproduce it, thanks.