Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Setting a custom User class results in fatal error #1325

Closed
schwarzdesign opened this issue Feb 10, 2021 · 8 comments
Closed

Setting a custom User class results in fatal error #1325

schwarzdesign opened this issue Feb 10, 2021 · 8 comments

Comments

@schwarzdesign
Copy link

schwarzdesign commented Feb 10, 2021

Short description of the issue

Using custom page classes with the user template results in a fatal error. As per this blog post, it should be possible to extend the default \ProcessWire\User class with a custom class and configure ProcessWire to use the custom class for $user objects. However, this results in a fatal error when I try it.

Expected behavior

Using a custom class extending the User class for the user template should not cause an error.

In addition, when setting a custom class for the user template and this error occurs, the only way to recover from this is to manually edit the templates database table and reset the pageClass for the user template to User. Maybe this should be caught by ProcessWire, to avoid people accidentally locking themselves out of the site (the error occurs both on the backend and the frontend).

Actual behavior

After saving the user template with a custom page class, I get the following error:

Fatal error: Uncaught TypeError: Argument 1 passed to ProcessWire\Users::setCurrentUser() must be an instance of ProcessWire\User, instance of ProcessWire\Page given, called in [...]/httpdocs/public/wire/core/Session.php on line 214 and defined in [...]/httpdocs/public/wire/core/Users.php on line 68

Steps to reproduce the issue

  1. Set $config->advanced = true; to be able to edit the user template.
  2. Create a custom user class and make sure it's included or autoloaded (see example below).
  3. Go to Setup -> Templates -> user -> System and set the Page class name to MemberUser (I also tried the FQCN \ProcessWire\MemberUser).
  4. Click on save, the error mentioned above will appear.
// example for a custom user class
<?php
namespace ProcessWire;

class MemberUser extends User {}

Setup/Environment

  • ProcessWire: 3.0.168
  • PHP: 7.4.15
  • Webserver: Apache
@Toutouwai
Copy link

I don't get a problem with a custom User class as per the instructions in the blog post: https://processwire.com/blog/posts/pw-3.0.152/#new-ability-to-specify-custom-page-classes

/site/classes/UserPage.php

<?php namespace ProcessWire;

class UserPage extends User {

	public function greeting() {
		return 'hello';
	}

}

2021-02-11_114638

The blog post doesn't say anything about editing the user template and changing its class so I'm not sure why you're doing those steps.

@schwarzdesign
Copy link
Author

schwarzdesign commented Feb 11, 2021

@Toutouwai Fair enough, it works this way. However, for every other template my approach (editing the Page class name setting for the template) works fine, and I prefer it. The approach mentioned in the blog post has some limitations I don't like:

  • I have to put all extending classes in the ProcessWire namespace, and I don't like to clutter existing namespaces with new classes. With the other approach (changing the setting on the template) I can set a FQCN and use my own class, like schwarzdesign\Page\MemberUser.
  • I have to put the class file in the public webroot, I don't like that for security reasons. We use composer to autoload PHP files from a directory one level above the webroot, so they can never be accessed directly, preventing a potential source of code leaks (by the way, this isn't theoretical, I've literally had this happen on a Strato Webhosting package).
  • The class name is fixed, so I have to use UserPage instead of something more descriptive and specific to my use-case.

I'm not saying that the simpler approach using $config->usePageClasses isn't a valid in some situations, but I think ProcessWire should support both ways. Especially since changing the page class works fine for every regular template. And the user template even has a field to edit the class name, which kind of indicates that this should work. Besides, in theory there isn't a difference between both approaches as long as the custom class extends the User class, it's probably just a matter of when/how ProcessWire determines which class to use for users. So this can (and should) be fixed.

@ryancramerdesign
Copy link
Member

@schwarzdesign This is a setting that requires $config->advanced=true; and the hidden System tab because you can break the system it, like other settings in advanced mode. The user template already sets a custom page class of 'User' (to override 'Page'), so this option has already been put to use by the core. It does the same thing with Role, Permission and Language.

But theoretically it should still be possible to override it as long as it's extending the User class. I can see your example does that, but the error message seems to indicate that it was dealing with just a Page instance rather than a User instance. That makes me think that PW just doesn't know about your MemberUser class soon enough in order to use it. This could be because identifying the user happens early in the boot process, before the init() state, and very likely before your custom class is visible to PW. I think that if you made your custom class visible from /site/config.php (by including it from there) then it just might work.

@schwarzdesign
Copy link
Author

@ryancramerdesign Thanks for the reply! My custom classes are autoloaded using Composer's autoloader. However, the autoloader was included in site/init.php, so too late for the user class.

I tried including the autoloader in the index.php instead, before everything else. This worked to an extent, but now I'm getting a different error when trying to set the user template to use my custom class:

Fatal error: Uncaught Error: Class 'ProcessWire\MemberUser' not found in [...]/httpdocs/public/wire/core/Pages.php on line 1422

This error comes from Pages::newPage which uses the wireClassName function. I had a look, this function checks if the string it's given includes a double backslash sequence (\\) and forces the ProcessWire namespace if it doesn't. I tried inserting a var_dump after that function call just to check what the function is returning before the error occurs. Here's the output:

[...]/httpdocs/public/wire/core/Pages.php:1421:string 'ProcessWire\Page' (length=16)
[...]/httpdocs/public/wire/core/Pages.php:1421:string 'ProcessWire\Page' (length=16)
[...]/httpdocs/public/wire/core/Pages.php:1421:string 'ProcessWire\Page' (length=16)
[...]/httpdocs/public/wire/core/Pages.php:1421:string 'ProcessWire\Page' (length=16)
[...]/httpdocs/public/wire/core/Pages.php:1421:string 'ProcessWire\Page' (length=16)
[...]/httpdocs/public/wire/core/Pages.php:1421:string 'ProcessWire\Role' (length=16)
[...]/httpdocs/public/wire/core/Pages.php:1421:string 'ProcessWire\Role' (length=16)
[...]/httpdocs/public/wire/core/Pages.php:1421:string '\schwarzdesign\Page\MemberUser' (length=30)
[...]/httpdocs/public/wire/core/Pages.php:1421:string 'ProcessWire\MemberUser' (length=22)

In the database, the custom class is saved with double backslashes like this: \\schwarzdesign\\Page\\MemberUser
So maybe it's just an encoding issue. Or the function is called twice and returns the wrong result the second time – looks like that, since the var_dump is executed twice with the MemberUser class and wireClassName returns the right result the first time. Maybe wireClassName just needs to be modified to deal with fully qualified classnames correctly?

Could you take a look if this is something that can be fixed? I'm thinking this is the last hurdle, and it would be really great if this use-case could be supported. Extending the User is a really important capability for us. And in theory it should work with all existing typehints and functions, it's only the wireClassName implementation that seems to trip up when given non-ProcessWire namespaced classes.

Thanks!

@ryancramerdesign
Copy link
Member

@schwarzdesign Thanks for the additional info. In the file /wire/core/PagesType.php can you try changing this line (607):

return $this->template->getPageClass(false);

to this (just change the false to true):

return $this->template->getPageClass(true);

Does that fix the issue?

@schwarzdesign
Copy link
Author

@ryancramerdesign Looks like it's working, thanks! With this change the errors are gone and ProcessWire appears to correctly use the extended class:

custom-page-class-test

Source code for the extended class for testing:

<?php
namespace schwarzdesign\Page;

use ProcessWire\User;

class MemberUser extends User {
    public function sayHello() {
        return 'Hello World :)';
    }
}

Can you include this change in the core?

Really glad it's working now. Thanks again!

ryancramerdesign added a commit to processwire/processwire that referenced this issue Mar 23, 2021
@ryancramerdesign
Copy link
Member

@schwarzdesign Thanks for testing that out, glad it fixes it there too. I've pushed the change to the dev branch.

@schwarzdesign
Copy link
Author

Great addition, thanks @ryancramerdesign!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants