Art-of-WiFi / UniFi-API-browser

Tool to browse data exposed by Ubiquiti's UniFi Controller API
MIT License
1.1k stars 150 forks source link

Not setting any users in users.php blocks login instead of bypassing auth. #84

Closed scyto closed 3 years ago

scyto commented 3 years ago

In users php it states:

 * IMPORTANT NOTE:
 * If you do not create the users.php file or do not create any user accounts, the API browser tool
 * will be accessible without providing and means of authentication.

I have been testing this with the following changes in my docker development environment

 * IMPORTANT NOTE:
 * If you do not create the users.php file or do not create any user accounts, the API browser tool
 * will be accessible without providing and means of authentication.
 */

if (getenv('NOAPIBROWSERAUTH') != "1" ) {

        $users = [
            [
            'user_name' => getenv('APIBROWSERUSER'), // string, passed in from docker env var 
            'password'  => getenv('APIBROWSERPASS'), // string, the SHA512 hash of the password, passed in from docker env var 
            ],

        #    [
        #       'user_name' => '', // string, the user name
        #       'password'  => '', // string, the SHA512 hash of the password
        #    ],
                ];
}

When the NOAPIBROWSERAUTH env var is set to 0 everything works ok. When the NOAPIBROWSERAUTH env var is set to 1 the login prompt is shown but one can't logon (because no username or password is set) As such i think this means my code is working.... but i am not a coder so not sure....

I am attempting this to service a request I got https://github.com/scyto/docker-UnifiBrowser/issues/5#issue-651082803

Questions

  1. Is the text in users.php wrong or is the function not working as expected?
  2. I note in login.php the only check is to load the file and if not readable it exits login.php
  3. I was expecting to see a check in that same code block for the user / password lines -and if not present exit, is that indeed missing or somewhere else in the code?
/**
 * load the file containing user accounts, if readable
 */
if (is_file('config/users.php') && is_readable('config/users.php')) {
    require_once('config/users.php');
} else {
    exit;
}

I don't need you to change any code, just trying to clarify vs bang my head against wall :-)

scyto commented 3 years ago

hmm i see index.php also has this, i am confused which file is doing the logon that said it still looks just a check if the file exists and is readable - right? it doesn't check to see if a username is set?

/**
 * load the file containing user accounts, if readable
 */
if (is_file('config/users.php') && is_readable('config/users.php')) {
    require_once('config/users.php');
    $user_authentication = true;
} else {
    $user_authentication = false;
    error_log('The file config/users.php does not exist, proceeding without user authentication.');
}
malle-pietje commented 3 years ago

Thanks for spotting this. The intent is for index.php to determine whether we have enabled user auth or not, this doesn’t require including the users.php file though. That line can be removed from the file and as you suggest a check should be added to verify whether the $users array exists and isn’t empty.

I’ll check your PR and merge it or else modify the existing code as needed.

malle-pietje commented 3 years ago

Actually the users.php file needs to be included to allow checking the $users array.

malle-pietje commented 3 years ago

I've added the check with the next commit.

scyto commented 3 years ago

fixed