403 - error after logging in

[robertillyes]

From commit SHA: be62f3d
To commit SHA: daccc30

[huy-nm]

@robertillyes Do you have any fix for this yet?

[laxmes]

Hey there, just got it and fixed by cleaning all cookies / storage of the browser. Hope it can fix your problem too!

[robertillyes]

Went in an edited the code manually on our installation, just returned false if no children were found for company in line 3588 in company.php as the query in MySql is incorrect for an empty IN clause. Not sure if it's the best solution or I'm missing something but it solved the issue for us.

[joshuapare]

We just hit this as well. we had issues logging in as an admin. I believe the issue may lie in the previous check at 3573 of the is_siteadmin not working, as it shouldn't get to this point in our case of logging in as admin.

[laxmes]

Hey there, just got it and fixed by cleaning all cookies / storage of the browser. Hope it can fix your problem too!

Again, I had the issue, I didn’t want to hard fix by tweaking the code directly and the previous actions worked. I’m sure there is something to do with cookies or whatever. The problem didn’t appear again after cleaning FULLY the website data (all storages, cookies…).

Give it a try and make a feedback please! Maybe deleting one item by one, so you can spot the issue.

[robertillyes]

It is an unhandled situation there. If I have no child companies then the returned value is an empty array. The empty array is an invalid value for the IN operator. Clearing cookies didn't work for me tough.

[laxmes]

Again to be sure about it, did your clear the local storage and session storage?

I didn’t change anything backend side to make it work again, until I cleared everything in the browser.. Worth trying it fully.

[laxmes]

Well I'm facing the same issue again, and cleaning doesn't fix it... Definitely a bug around there!

Edit:
I replaced $USER->id by $userid for the admin check condition, maybe a way to fix it as the condition was false on my side even with an admin user, using $userid is fixing it. Would need a maintainer to validate it though.

[huy-nm]

Well I'm facing the same issue again, and cleaning doesn't fix it... Definitely a bug around there!

Edit: I replaced $USER->id by $userid for the admin check condition, maybe a way to fix it as the condition was false on my side even with an admin user, using $userid is fixing it. Would need a maintainer to validate it though.

`
public static function check_valid_user($companyid, $userid, $deparmentid=0) {
global $DB, $USER;

    // If current user is a site admin or they have appropriate capabilities then they can.
    if (is_siteadmin($userid) ||
        iomad::has_capability('block/iomad_company_admin:company_add', \core\context\company::instance($companyid))) {
        return true;
    }

    if (!empty($departmentid) && $DB->get_record('company_users', array('departmentid' => $departmentid,
                                                                        'companyid' => $companyid,
                                                                        'userid' => $userid))) {
        return true;
    } else if ($DB->get_records('company_users', array('companyid' => $companyid,
                                                       'userid' => $userid))) {
        return true;
    } else {
        // is the user in a child company?
        $company = new company($companyid);
        $children = $company->get_child_companies_recursive();
        if ($DB->get_records_sql("SELECT id FROM {company_users}
                                  WHERE userid = :userid
                                  and companyid IN (" . join(',', array_keys($children)) . ")",
                                  ['userid' => $userid])) {
            return true;
        } else {
            return false;
        }
    }
    // Shouldn't get here.  Return a false in case.
    return false;
}

`
We fixed like you and it works okay for site admin account. But for another, it won't work.
Here it what I found

• I use Client administrator role.
• When you logout and login this role, it will call this function. And somehow, it failed the check of iomad::has_capability('block/iomad_company_admin:company_add'
• And because this role doesn't belong to any company (not record in company_users table) then it go to else condition. And it failed here, because company doesn't have child company.

So the work around here is

• Add that user to all companies that it can access. Then it is okay.
• We need to fix the first if condition, fixing this check has_capability

[laxmes]

I don't think checking role is the issue here: $USER->id is actually returning the id 0 which is not existing (my ids are starting at 1 in the mdl_user table). The global user is somewhere not initialized correctly (maybe not initialized because we are actually at the login step...).

I'll continue to check a bit or maybe just patch it like that for now as the global user is not initialized only for login and the right id is passed in $userid:

// If current user is a site admin or they have appropriate capabilities then they can.
if (is_siteadmin($USER->id ? $USER->id : $userid) ||
iomad::has_capability('block/iomad_company_admin:company_add', \core\context\company::instance($companyid))) {
    return true;
}

[huy-nm]

I don't think checking role is the issue here: $USER->id is actually returning the id 0 which is not existing (my ids are starting at 1 in the mdl_user table). The global user is somewhere not initialized correctly (maybe not initialized because we are actually at the login step...).

I'll continue to check a bit or maybe just patch it like that for now as the global user is not initialized only for login and the right id is passed in $userid:

// If current user is a site admin or they have appropriate capabilities then they can.
if (is_siteadmin($USER->id ? $USER->id : $userid) ||
iomad::has_capability('block/iomad_company_admin:company_add', \core\context\company::instance($companyid))) {
    return true;
}

I change to $userid, work for root admin, but still got error for Client Administrator role. This role has has_capability block/iomad_company_admin:company_add, but still failed. I don't know why.

[laxmes]

I think it's because the has_capability function also rely on global $USER, which is not right in our situation.

That's a situation proving you that pure functions are really solving many issues and headaches :-).