[Pushed] AdminCP style override loads after show_login() function execution
#1
Line 144:

$default_page->show_login($lang->error_invalid_secret_pin, "error");

This line shows the login page with an error if the PIN entered is invalid. However, when applying custom styling to the style.php file in an AdminCP theme, this page is not completely customized as an eventual custom show_login() function is not fired, and the default show_login() is run instead.

The issue is caused by the following line of code, which is defined @line 420, after that show_login() call:

require_once MYBB_ADMIN_DIR."/styles/{$cp_style}/style.php";

It should be defined before.
[Image: fSGNVQj.png]
Reply
#2
The ACP loads the default style at the top of the file, then checks the global setting but doesn't include it. Later it checks the user setting too and then loads the custom theme. Therefore this does not only apply to the pin page but to all pages before the user is loaded. Not sure whether and how this can be changed easily or whether we even want to change it as we'd need to rethink the theme process for the acp.
Support PMs will be ignored!
Reply
#3
Bump. Any other opinions?
Support PMs will be ignored!
Reply
#4
We should probably move the PIN after the login, just like 2FA. Otherwise I do feel it is right as it is now.
Reply
#5
The PIN should be checked first as there isn't the need to check the user if the PIN was wrong. The 2FA part on the other hand needs to be checked after the user is checked as we don't know his secret before.

Also that wouldn't fix this issue at all as the style is loaded *after every* login check has been made.
Support PMs will be ignored!
Reply
#6
I might be missing something, but I don't see anything against copying (not moving due to https://github.com/mybb/mybb/blob/featur...x.php#L479) the style.php file inclusion here: https://github.com/mybb/mybb/blob/featur...ex.php#L84 if the user is not logged in. Seems to work for me.
Reply
#7
From a quick look you would cause issues if the acp style extends a class and the user has a different style set which extends the same class:

/admin/styles/my_default/style.php:
class Page extends DefaultPage
{
    function show_login()
    {
         // Do something
    }
}

/admin/styles/user/style.php:
class Page extends DefaultPage
{
    function show_login()
    {
         // Do something else
    }
}

You'd include the "my_default" style.php first and after the user checks you'd include the "user" style.php though both have a class named "Page". You'd need to add the style generation to places where it's clear that the user is not logged in. However there are multiple places (wrong username, wrong PIN, wrong 2FA, logged out, lock out, recovery...).
Support PMs will be ignored!
Reply
#8
(2015-07-07, 12:17 PM)Jones H Wrote: You'd need to add the style generation to places where it's clear that the user is not logged in. However there are multiple places (wrong username, wrong PIN, wrong 2FA, logged out, lock out, recovery...).

That's exactly what I meant by "if the user is not logged in". Which seems to be the case only when unlock and login action/do inputs are set, which we can check. Everything you mentioned looks to be covered.
Reply
#9
After reading this is it possible to change the login page instead of it using default mybb for admin login have it use the Acp login style for emerald acp Emerald is already my admin panel theme but login for admin reverts to mybb origional
Reply
#10
Shade, as you were the one reporting this, could you check whether the proposed fix works? If not I'm not sure whether there's much we can do about this.
Support PMs will be ignored!
Reply


Forum Jump:


Users browsing this thread: 1 Guest(s)