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.

Sign up for GitHub

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

LDAP variables can be integers #35701

Closed
wants to merge 2 commits into from
Closed

LDAP variables can be integers #35701

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3efaf50
Addresses #35642: LDAP variables can be integers
alpapan Dec 9, 2022
25ad28c
Changes for #35701
alpapan Dec 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/user_ldap/lib/Configuration.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ protected function setMultiLine(string $varName, $value): void {
} else {
$finalValue = [];
foreach ($value as $key => $val) {
if (is_string($val)) {
if (is_string($val) || is_numeric($int)) {

Check failure

Code scanning / Psalm

UndefinedVariable

Cannot find referenced variable $int
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops, that's my mistake!

$val = trim($val);
if ($val !== '') {
//accidental line breaks are not wanted and can cause
Expand Down
12 changes: 8 additions & 4 deletions apps/user_ldap/lib/Group_LDAP.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -703,7 +703,8 @@ public function getUserGroups($uid) {
if ($userMatch !== false) {
// match found so this user is in this group
$groupName = $this->access->dn2groupname($dynamicGroup['dn'][0]);
if (is_string($groupName)) {
// in case it is just an integer (is_string(int) returns false
if (is_string($groupName) || is_numeric($groupName)) {

Check notice

Code scanning / Psalm

DocblockTypeContradiction

Docblock-defined type false for $groupName is never numeric
// be sure to never return false if the dn could not be
// resolved to a name, for whatever reason.
$groups[] = $groupName;
Expand All @@ -730,7 +731,8 @@ public function getUserGroups($uid) {
$groupDNs = $this->_getGroupDNsFromMemberOf($userDN);
foreach ($groupDNs as $dn) {
$groupName = $this->access->dn2groupname($dn);
if (is_string($groupName)) {
// in case it is just an integer (is_string(int) returns false
if (is_string($groupName) || is_numeric($groupName)) {

Check notice

Code scanning / Psalm

DocblockTypeContradiction

Docblock-defined type false for $groupName is never numeric
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why can't the name of a group be just numbers?

// be sure to never return false if the dn could not be
// resolved to a name, for whatever reason.
$groups[] = $groupName;
Expand Down Expand Up @@ -1138,7 +1140,8 @@ public function groupExists($gid) {
protected function filterValidGroups(array $listOfGroups): array {
$validGroupDNs = [];
foreach ($listOfGroups as $key => $item) {
$dn = is_string($item) ? $item : $item['dn'][0];
// in case it is just an integer (is_string(int) returns false
$dn = !is_array($item) ? $item : $item['dn'][0];
if (is_array($item) && !isset($item[$this->access->connection->ldapGroupDisplayName][0])) {
continue;
}
Expand Down Expand Up @@ -1191,7 +1194,8 @@ public function createGroup($gid) {
if ($dn = $this->groupPluginManager->createGroup($gid)) {
//updates group mapping
$uuid = $this->access->getUUID($dn, false);
if (is_string($uuid)) {
// in case it is just an integer, not sure if this UUID could ever be an int though
if (is_string($uuid) || is_numeric($uuid)) {

Check notice

Code scanning / Psalm

DocblockTypeContradiction

Docblock-defined type false for $uuid is never numeric
$this->access->mapAndAnnounceIfApplicable(
$this->access->getGroupMapper(),
$dn,
Expand Down
3 changes: 2 additions & 1 deletion apps/user_ldap/lib/Mapping/AbstractMapping.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,8 @@ public function setUUIDbyDN($uuid, $fdn): bool {
*/
protected function getDNHash(string $fdn): string {
$hash = hash('sha256', $fdn, false);
if (is_string($hash)) {
// very rare but a hash could just be numbers? is_string(int) would return false
if (is_string($hash) || is_numeric($hash)) {

Check failure

Code scanning / Psalm

TypeDoesNotContainType

Type false for $hash is never numeric
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in theory a sha256 can return just numbers. PHP will parse that as numeric and not a string AFAIK.

return $hash;
} else {
throw new \RuntimeException('hash function did not return a string');
Expand Down
6 changes: 4 additions & 2 deletions apps/user_ldap/lib/User_LDAP.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,8 @@ public function getUsers($search = '', $limit = 10, $offset = 0) {
* @throws \OC\ServerNotAvailableException
*/
public function userExistsOnLDAP($user, bool $ignoreCache = false): bool {
if (is_string($user)) {
// in case it is just an integer (is_string(int) returns false
if (is_string($user) || is_numeric($user)) {

Check notice

Code scanning / Psalm

DocblockTypeContradiction

Docblock-defined type OCA\User_LDAP\User\User for $user is never numeric
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can't the username be just numbers?

$user = $this->access->userManager->get($user);
}
if (is_null($user)) {
Expand Down Expand Up @@ -645,7 +646,8 @@ public function createUser($username, $password) {
if (is_string($dn)) {
// the NC user creation work flow requires a know user id up front
$uuid = $this->access->getUUID($dn, true);
if (is_string($uuid)) {
// not sure if UUID could ever be just a number
if (is_string($uuid) || is_numeric($uuid)) {

Check notice

Code scanning / Psalm

DocblockTypeContradiction

Docblock-defined type false for $uuid is never numeric
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IDK what UUID is but if it can ever be a string of integers, PHP parses that as an integer not a string (no typecasting)

$this->access->mapAndAnnounceIfApplicable(
$this->access->getUserMapper(),
$dn,
Expand Down