Failed Conditions
Push — psr2 ( 957f36...e79ce3 )
by Andreas
03:40
created

auth_plugin_authpdo::selectUser()   D

Complexity

Conditions 10
Paths 34

Size

Total Lines 35
Code Lines 23

Duplication

Lines 0
Ratio 0 %

Importance

Changes 0
Metric Value
cc 10
eloc 23
nc 34
nop 1
dl 0
loc 35
rs 4.8196
c 0
b 0
f 0

How to fix   Complexity   

Long Method

Small methods make your code easier to understand, in particular if combined with a good name. Besides, if your method is small, finding a good name is usually much easier.

For example, if you find yourself adding comments to a method's body, this is usually a good sign to extract the commented part to a new method, and use the comment as a starting point when coming up with a good name for this new method.

Commonly applied refactorings include:

1
<?php
2
/**
3
 * DokuWiki Plugin authpdo (Auth Component)
4
 *
5
 * @license GPL 2 http://www.gnu.org/licenses/gpl-2.0.html
6
 * @author  Andreas Gohr <[email protected]>
7
 */
8
9
/**
10
 * Class auth_plugin_authpdo
11
 */
12
class auth_plugin_authpdo extends DokuWiki_Auth_Plugin
13
{
14
15
    /** @var PDO */
16
    protected $pdo;
17
18
    /** @var null|array The list of all groups */
19
    protected $groupcache = null;
20
21
    /**
22
     * Constructor.
23
     */
24
    public function __construct()
25
    {
26
        parent::__construct(); // for compatibility
27
28
        if (!class_exists('PDO')) {
29
            $this->debugMsg('PDO extension for PHP not found.', -1, __LINE__);
30
            $this->success = false;
31
            return;
32
        }
33
34
        if (!$this->getConf('dsn')) {
35
            $this->debugMsg('No DSN specified', -1, __LINE__);
36
            $this->success = false;
37
            return;
38
        }
39
40
        try {
41
            $this->pdo = new PDO(
42
                $this->getConf('dsn'),
43
                $this->getConf('user'),
44
                conf_decodeString($this->getConf('pass')),
45
                array(
46
                    PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC, // always fetch as array
47
                    PDO::ATTR_EMULATE_PREPARES => true, // emulating prepares allows us to reuse param names
48
                    PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION, // we want exceptions, not error codes
49
                )
50
            );
51
        } catch (PDOException $e) {
52
            $this->debugMsg($e);
53
            msg($this->getLang('connectfail'), -1);
54
            $this->success = false;
55
            return;
56
        }
57
58
        // can Users be created?
59
        $this->cando['addUser'] = $this->checkConfig(
60
            array(
61
                'select-user',
62
                'select-user-groups',
63
                'select-groups',
64
                'insert-user',
65
                'insert-group',
66
                'join-group'
67
            )
68
        );
69
70
        // can Users be deleted?
71
        $this->cando['delUser'] = $this->checkConfig(
72
            array(
73
                'select-user',
74
                'select-user-groups',
75
                'select-groups',
76
                'leave-group',
77
                'delete-user'
78
            )
79
        );
80
81
        // can login names be changed?
82
        $this->cando['modLogin'] = $this->checkConfig(
83
            array(
84
                'select-user',
85
                'select-user-groups',
86
                'update-user-login'
87
            )
88
        );
89
90
        // can passwords be changed?
91
        $this->cando['modPass'] = $this->checkConfig(
92
            array(
93
                'select-user',
94
                'select-user-groups',
95
                'update-user-pass'
96
            )
97
        );
98
99
        // can real names be changed?
100
        $this->cando['modName'] = $this->checkConfig(
101
            array(
102
                'select-user',
103
                'select-user-groups',
104
                'update-user-info:name'
105
            )
106
        );
107
108
        // can real email be changed?
109
        $this->cando['modMail'] = $this->checkConfig(
110
            array(
111
                'select-user',
112
                'select-user-groups',
113
                'update-user-info:mail'
114
            )
115
        );
116
117
        // can groups be changed?
118
        $this->cando['modGroups'] = $this->checkConfig(
119
            array(
120
                'select-user',
121
                'select-user-groups',
122
                'select-groups',
123
                'leave-group',
124
                'join-group',
125
                'insert-group'
126
            )
127
        );
128
129
        // can a filtered list of users be retrieved?
130
        $this->cando['getUsers'] = $this->checkConfig(
131
            array(
132
                'list-users'
133
            )
134
        );
135
136
        // can the number of users be retrieved?
137
        $this->cando['getUserCount'] = $this->checkConfig(
138
            array(
139
                'count-users'
140
            )
141
        );
142
143
        // can a list of available groups be retrieved?
144
        $this->cando['getGroups'] = $this->checkConfig(
145
            array(
146
                'select-groups'
147
            )
148
        );
149
150
        $this->success = true;
151
    }
152
153
    /**
154
     * Check user+password
155
     *
156
     * @param   string $user the user name
157
     * @param   string $pass the clear text password
158
     * @return  bool
159
     */
160
    public function checkPass($user, $pass)
161
    {
162
163
        $userdata = $this->selectUser($user);
164
        if ($userdata == false) return false;
165
166
        // password checking done in SQL?
167
        if ($this->checkConfig(array('check-pass'))) {
168
            $userdata['clear'] = $pass;
169
            $userdata['hash'] = auth_cryptPassword($pass);
170
            $result = $this->query($this->getConf('check-pass'), $userdata);
0 ignored issues
show
Bug introduced by
It seems like $userdata defined by $this->selectUser($user) on line 163 can also be of type boolean; however, auth_plugin_authpdo::query() does only seem to accept array, maybe add an additional type check?

If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check:

/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}

If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue.

Loading history...
171
            if ($result === false) return false;
172
            return (count($result) == 1);
173
        }
174
175
        // we do password checking on our own
176
        if (isset($userdata['hash'])) {
177
            // hashed password
178
            $passhash = new PassHash();
179
            return $passhash->verify_hash($pass, $userdata['hash']);
180
        } else {
181
            // clear text password in the database O_o
182
            return ($pass === $userdata['clear']);
183
        }
184
    }
185
186
    /**
187
     * Return user info
188
     *
189
     * Returns info about the given user needs to contain
190
     * at least these fields:
191
     *
192
     * name string  full name of the user
193
     * mail string  email addres of the user
194
     * grps array   list of groups the user is in
195
     *
196
     * @param   string $user the user name
197
     * @param   bool $requireGroups whether or not the returned data must include groups
198
     * @return array|bool containing user data or false
199
     */
200
    public function getUserData($user, $requireGroups = true)
201
    {
202
        $data = $this->selectUser($user);
0 ignored issues
show
Bug Compatibility introduced by
The expression $this->selectUser($user); of type boolean|array adds the type boolean to the return on line 213 which is incompatible with the return type of the parent method DokuWiki_Auth_Plugin::getUserData of type false|array.
Loading history...
203
        if ($data == false) return false;
204
205
        if (isset($data['hash'])) unset($data['hash']);
206
        if (isset($data['clean'])) unset($data['clean']);
207
208
        if ($requireGroups) {
209
            $data['grps'] = $this->selectUserGroups($data);
0 ignored issues
show
Bug introduced by
It seems like $data defined by $this->selectUser($user) on line 202 can also be of type boolean; however, auth_plugin_authpdo::selectUserGroups() does only seem to accept array, maybe add an additional type check?

If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check:

/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}

If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue.

Loading history...
210
            if ($data['grps'] === false) return false;
211
        }
212
213
        return $data;
214
    }
215
216
    /**
217
     * Create a new User [implement only where required/possible]
218
     *
219
     * Returns false if the user already exists, null when an error
220
     * occurred and true if everything went well.
221
     *
222
     * The new user HAS TO be added to the default group by this
223
     * function!
224
     *
225
     * Set addUser capability when implemented
226
     *
227
     * @param  string $user
228
     * @param  string $clear
229
     * @param  string $name
230
     * @param  string $mail
231
     * @param  null|array $grps
232
     * @return bool|null
233
     */
234
    public function createUser($user, $clear, $name, $mail, $grps = null)
235
    {
236
        global $conf;
237
238
        if (($info = $this->getUserData($user, false)) !== false) {
239
            msg($this->getLang('userexists'), -1);
240
            return false; // user already exists
241
        }
242
243
        // prepare data
244
        if ($grps == null) $grps = array();
245
        array_unshift($grps, $conf['defaultgroup']);
246
        $grps = array_unique($grps);
247
        $hash = auth_cryptPassword($clear);
248
        $userdata = compact('user', 'clear', 'hash', 'name', 'mail');
249
250
        // action protected by transaction
251
        $this->pdo->beginTransaction();
252
        {
253
            // insert the user
254
            $ok = $this->query($this->getConf('insert-user'), $userdata);
255
            if ($ok === false) goto FAIL;
256
            $userdata = $this->getUserData($user, false);
257
            if ($userdata === false) goto FAIL;
258
259
            // create all groups that do not exist, the refetch the groups
260
            $allgroups = $this->selectGroups();
261
        foreach ($grps as $group) {
262
            if (!isset($allgroups[$group])) {
263
                $ok = $this->addGroup($group);
264
                if ($ok === false) goto FAIL;
265
            }
266
        }
267
            $allgroups = $this->selectGroups();
268
269
            // add user to the groups
270
        foreach ($grps as $group) {
271
            $ok = $this->joinGroup($userdata, $allgroups[$group]);
0 ignored issues
show
Bug introduced by
It seems like $userdata defined by $this->getUserData($user, false) on line 256 can also be of type boolean; however, auth_plugin_authpdo::joinGroup() does only seem to accept array, maybe add an additional type check?

If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check:

/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}

If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue.

Loading history...
272
            if ($ok === false) goto FAIL;
273
        }
274
        }
275
        $this->pdo->commit();
276
        return true;
277
278
        // something went wrong, rollback
279
        FAIL:
280
        $this->pdo->rollBack();
281
        $this->debugMsg('Transaction rolled back', 0, __LINE__);
282
        msg($this->getLang('writefail'), -1);
283
        return null; // return error
284
    }
285
286
    /**
287
     * Modify user data
288
     *
289
     * @param   string $user nick of the user to be changed
290
     * @param   array $changes array of field/value pairs to be changed (password will be clear text)
291
     * @return  bool
292
     */
293
    public function modifyUser($user, $changes)
294
    {
295
        // secure everything in transaction
296
        $this->pdo->beginTransaction();
297
        {
298
            $olddata = $this->getUserData($user);
299
            $oldgroups = $olddata['grps'];
300
            unset($olddata['grps']);
301
302
            // changing the user name?
303
        if (isset($changes['user'])) {
304
            if ($this->getUserData($changes['user'], false)) goto FAIL;
305
            $params = $olddata;
306
            $params['newlogin'] = $changes['user'];
307
308
            $ok = $this->query($this->getConf('update-user-login'), $params);
0 ignored issues
show
Bug introduced by
It seems like $params defined by $olddata on line 305 can also be of type boolean; however, auth_plugin_authpdo::query() does only seem to accept array, maybe add an additional type check?

If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check:

/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}

If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue.

Loading history...
309
            if ($ok === false) goto FAIL;
310
        }
311
312
            // changing the password?
313
        if (isset($changes['pass'])) {
314
            $params = $olddata;
315
            $params['clear'] = $changes['pass'];
316
            $params['hash'] = auth_cryptPassword($changes['pass']);
317
318
            $ok = $this->query($this->getConf('update-user-pass'), $params);
0 ignored issues
show
Bug introduced by
It seems like $params defined by $olddata on line 314 can also be of type boolean; however, auth_plugin_authpdo::query() does only seem to accept array, maybe add an additional type check?

If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check:

/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}

If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue.

Loading history...
319
            if ($ok === false) goto FAIL;
320
        }
321
322
            // changing info?
323
        if (isset($changes['mail']) || isset($changes['name'])) {
324
            $params = $olddata;
325
            if (isset($changes['mail'])) $params['mail'] = $changes['mail'];
326
            if (isset($changes['name'])) $params['name'] = $changes['name'];
327
328
            $ok = $this->query($this->getConf('update-user-info'), $params);
0 ignored issues
show
Bug introduced by
It seems like $params defined by $olddata on line 324 can also be of type boolean; however, auth_plugin_authpdo::query() does only seem to accept array, maybe add an additional type check?

If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check:

/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}

If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue.

Loading history...
329
            if ($ok === false) goto FAIL;
330
        }
331
332
            // changing groups?
333
        if (isset($changes['grps'])) {
334
            $allgroups = $this->selectGroups();
335
336
            // remove membership for previous groups
337
            foreach ($oldgroups as $group) {
338
                if (!in_array($group, $changes['grps']) && isset($allgroups[$group])) {
339
                    $ok = $this->leaveGroup($olddata, $allgroups[$group]);
0 ignored issues
show
Bug introduced by
It seems like $olddata defined by $this->getUserData($user) on line 298 can also be of type boolean; however, auth_plugin_authpdo::leaveGroup() does only seem to accept array, maybe add an additional type check?

If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check:

/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}

If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue.

Loading history...
340
                    if ($ok === false) goto FAIL;
341
                }
342
            }
343
344
            // create all new groups that are missing
345
            $added = 0;
346
            foreach ($changes['grps'] as $group) {
347
                if (!isset($allgroups[$group])) {
348
                    $ok = $this->addGroup($group);
349
                    if ($ok === false) goto FAIL;
350
                    $added++;
351
                }
352
            }
353
            // reload group info
354
            if ($added > 0) $allgroups = $this->selectGroups();
355
356
            // add membership for new groups
357
            foreach ($changes['grps'] as $group) {
358
                if (!in_array($group, $oldgroups)) {
359
                    $ok = $this->joinGroup($olddata, $allgroups[$group]);
0 ignored issues
show
Bug introduced by
It seems like $olddata defined by $this->getUserData($user) on line 298 can also be of type boolean; however, auth_plugin_authpdo::joinGroup() does only seem to accept array, maybe add an additional type check?

If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check:

/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}

If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue.

Loading history...
360
                    if ($ok === false) goto FAIL;
361
                }
362
            }
363
        }
364
365
        }
366
        $this->pdo->commit();
367
        return true;
368
369
        // something went wrong, rollback
370
        FAIL:
371
        $this->pdo->rollBack();
372
        $this->debugMsg('Transaction rolled back', 0, __LINE__);
373
        msg($this->getLang('writefail'), -1);
374
        return false; // return error
375
    }
376
377
    /**
378
     * Delete one or more users
379
     *
380
     * Set delUser capability when implemented
381
     *
382
     * @param   array $users
383
     * @return  int    number of users deleted
384
     */
385
    public function deleteUsers($users)
386
    {
387
        $count = 0;
388
        foreach ($users as $user) {
389
            if ($this->deleteUser($user)) $count++;
390
        }
391
        return $count;
392
    }
393
394
    /**
395
     * Bulk retrieval of user data [implement only where required/possible]
396
     *
397
     * Set getUsers capability when implemented
398
     *
399
     * @param   int $start index of first user to be returned
400
     * @param   int $limit max number of users to be returned
401
     * @param   array $filter array of field/pattern pairs, null for no filter
402
     * @return  array list of userinfo (refer getUserData for internal userinfo details)
403
     */
404
    public function retrieveUsers($start = 0, $limit = -1, $filter = null)
405
    {
406
        if ($limit < 0) $limit = 10000; // we don't support no limit
407
        if (is_null($filter)) $filter = array();
408
409
        if (isset($filter['grps'])) $filter['group'] = $filter['grps'];
410
        foreach (array('user', 'name', 'mail', 'group') as $key) {
411
            if (!isset($filter[$key])) {
412
                $filter[$key] = '%';
413
            } else {
414
                $filter[$key] = '%' . $filter[$key] . '%';
415
            }
416
        }
417
        $filter['start'] = (int) $start;
418
        $filter['end'] = (int) $start + $limit;
419
        $filter['limit'] = (int) $limit;
420
421
        $result = $this->query($this->getConf('list-users'), $filter);
422
        if (!$result) return array();
423
        $users = array();
424
        foreach ($result as $row) {
0 ignored issues
show
Bug introduced by
The expression $result of type integer|array is not guaranteed to be traversable. How about adding an additional type check?

There are different options of fixing this problem.

  1. If you want to be on the safe side, you can add an additional type-check:

    $collection = json_decode($data, true);
    if ( ! is_array($collection)) {
        throw new \RuntimeException('$collection must be an array.');
    }
    
    foreach ($collection as $item) { /** ... */ }
    
  2. If you are sure that the expression is traversable, you might want to add a doc comment cast to improve IDE auto-completion and static analysis:

    /** @var array $collection */
    $collection = json_decode($data, true);
    
    foreach ($collection as $item) { /** .. */ }
    
  3. Mark the issue as a false-positive: Just hover the remove button, in the top-right corner of this issue for more options.

Loading history...
425
            if (!isset($row['user'])) {
426
                $this->debugMsg("Statement did not return 'user' attribute", -1, __LINE__);
427
                return array();
428
            }
429
            $users[] = $this->getUserData($row['user']);
430
        }
431
        return $users;
432
    }
433
434
    /**
435
     * Return a count of the number of user which meet $filter criteria
436
     *
437
     * @param  array $filter array of field/pattern pairs, empty array for no filter
438
     * @return int
439
     */
440
    public function getUserCount($filter = array())
441
    {
442
        if (is_null($filter)) $filter = array();
443
444
        if (isset($filter['grps'])) $filter['group'] = $filter['grps'];
445
        foreach (array('user', 'name', 'mail', 'group') as $key) {
446
            if (!isset($filter[$key])) {
447
                $filter[$key] = '%';
448
            } else {
449
                $filter[$key] = '%' . $filter[$key] . '%';
450
            }
451
        }
452
453
        $result = $this->query($this->getConf('count-users'), $filter);
454
        if (!$result || !isset($result[0]['count'])) {
455
            $this->debugMsg("Statement did not return 'count' attribute", -1, __LINE__);
456
        }
457
        return (int) $result[0]['count'];
458
    }
459
460
    /**
461
     * Create a new group with the given name
462
     *
463
     * @param string $group
464
     * @return bool
465
     */
466
    public function addGroup($group)
467
    {
468
        $sql = $this->getConf('insert-group');
469
470
        $result = $this->query($sql, array(':group' => $group));
471
        $this->clearGroupCache();
472
        if ($result === false) return false;
473
        return true;
474
    }
475
476
    /**
477
     * Retrieve groups
478
     *
479
     * Set getGroups capability when implemented
480
     *
481
     * @param   int $start
482
     * @param   int $limit
483
     * @return  array
484
     */
485
    public function retrieveGroups($start = 0, $limit = 0)
486
    {
487
        $groups = array_keys($this->selectGroups());
488
        if ($groups === false) return array();
489
490
        if (!$limit) {
491
            return array_splice($groups, $start);
492
        } else {
493
            return array_splice($groups, $start, $limit);
494
        }
495
    }
496
497
    /**
498
     * Select data of a specified user
499
     *
500
     * @param string $user the user name
501
     * @return bool|array user data, false on error
502
     */
503
    protected function selectUser($user)
504
    {
505
        $sql = $this->getConf('select-user');
506
507
        $result = $this->query($sql, array(':user' => $user));
508
        if (!$result) return false;
509
510
        if (count($result) > 1) {
511
            $this->debugMsg('Found more than one matching user', -1, __LINE__);
512
            return false;
513
        }
514
515
        $data = array_shift($result);
516
        $dataok = true;
517
518
        if (!isset($data['user'])) {
519
            $this->debugMsg("Statement did not return 'user' attribute", -1, __LINE__);
520
            $dataok = false;
521
        }
522
        if (!isset($data['hash']) && !isset($data['clear']) && !$this->checkConfig(array('check-pass'))) {
523
            $this->debugMsg("Statement did not return 'clear' or 'hash' attribute", -1, __LINE__);
524
            $dataok = false;
525
        }
526
        if (!isset($data['name'])) {
527
            $this->debugMsg("Statement did not return 'name' attribute", -1, __LINE__);
528
            $dataok = false;
529
        }
530
        if (!isset($data['mail'])) {
531
            $this->debugMsg("Statement did not return 'mail' attribute", -1, __LINE__);
532
            $dataok = false;
533
        }
534
535
        if (!$dataok) return false;
536
        return $data;
537
    }
538
539
    /**
540
     * Delete a user after removing all their group memberships
541
     *
542
     * @param string $user
543
     * @return bool true when the user was deleted
544
     */
545
    protected function deleteUser($user)
546
    {
547
        $this->pdo->beginTransaction();
548
        {
549
            $userdata = $this->getUserData($user);
550
            if ($userdata === false) goto FAIL;
551
            $allgroups = $this->selectGroups();
552
553
            // remove group memberships (ignore errors)
554
        foreach ($userdata['grps'] as $group) {
555
            if (isset($allgroups[$group])) {
556
                $this->leaveGroup($userdata, $allgroups[$group]);
0 ignored issues
show
Bug introduced by
It seems like $userdata defined by $this->getUserData($user) on line 549 can also be of type boolean; however, auth_plugin_authpdo::leaveGroup() does only seem to accept array, maybe add an additional type check?

If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check:

/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}

If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue.

Loading history...
557
            }
558
        }
559
560
            $ok = $this->query($this->getConf('delete-user'), $userdata);
0 ignored issues
show
Bug introduced by
It seems like $userdata defined by $this->getUserData($user) on line 549 can also be of type boolean; however, auth_plugin_authpdo::query() does only seem to accept array, maybe add an additional type check?

If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check:

/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}

If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue.

Loading history...
561
            if ($ok === false) goto FAIL;
562
        }
563
        $this->pdo->commit();
564
        return true;
565
566
        FAIL:
567
        $this->pdo->rollBack();
568
        return false;
569
    }
570
571
    /**
572
     * Select all groups of a user
573
     *
574
     * @param array $userdata The userdata as returned by _selectUser()
575
     * @return array|bool list of group names, false on error
576
     */
577
    protected function selectUserGroups($userdata)
578
    {
579
        global $conf;
580
        $sql = $this->getConf('select-user-groups');
581
        $result = $this->query($sql, $userdata);
582
        if ($result === false) return false;
583
584
        $groups = array($conf['defaultgroup']); // always add default config
585
        foreach ($result as $row) {
0 ignored issues
show
Bug introduced by
The expression $result of type integer|array is not guaranteed to be traversable. How about adding an additional type check?

There are different options of fixing this problem.

  1. If you want to be on the safe side, you can add an additional type-check:

    $collection = json_decode($data, true);
    if ( ! is_array($collection)) {
        throw new \RuntimeException('$collection must be an array.');
    }
    
    foreach ($collection as $item) { /** ... */ }
    
  2. If you are sure that the expression is traversable, you might want to add a doc comment cast to improve IDE auto-completion and static analysis:

    /** @var array $collection */
    $collection = json_decode($data, true);
    
    foreach ($collection as $item) { /** .. */ }
    
  3. Mark the issue as a false-positive: Just hover the remove button, in the top-right corner of this issue for more options.

Loading history...
586
            if (!isset($row['group'])) {
587
                $this->debugMsg("No 'group' field returned in select-user-groups statement");
588
                return false;
589
            }
590
            $groups[] = $row['group'];
591
        }
592
593
        $groups = array_unique($groups);
594
        sort($groups);
595
        return $groups;
596
    }
597
598
    /**
599
     * Select all available groups
600
     *
601
     * @return array|bool list of all available groups and their properties
602
     */
603
    protected function selectGroups()
604
    {
605
        if ($this->groupcache) return $this->groupcache;
606
607
        $sql = $this->getConf('select-groups');
608
        $result = $this->query($sql);
609
        if ($result === false) return false;
610
611
        $groups = array();
612
        foreach ($result as $row) {
0 ignored issues
show
Bug introduced by
The expression $result of type integer|array is not guaranteed to be traversable. How about adding an additional type check?

There are different options of fixing this problem.

  1. If you want to be on the safe side, you can add an additional type-check:

    $collection = json_decode($data, true);
    if ( ! is_array($collection)) {
        throw new \RuntimeException('$collection must be an array.');
    }
    
    foreach ($collection as $item) { /** ... */ }
    
  2. If you are sure that the expression is traversable, you might want to add a doc comment cast to improve IDE auto-completion and static analysis:

    /** @var array $collection */
    $collection = json_decode($data, true);
    
    foreach ($collection as $item) { /** .. */ }
    
  3. Mark the issue as a false-positive: Just hover the remove button, in the top-right corner of this issue for more options.

Loading history...
613
            if (!isset($row['group'])) {
614
                $this->debugMsg("No 'group' field returned from select-groups statement", -1, __LINE__);
615
                return false;
616
            }
617
618
            // relayout result with group name as key
619
            $group = $row['group'];
620
            $groups[$group] = $row;
621
        }
622
623
        ksort($groups);
624
        return $groups;
625
    }
626
627
    /**
628
     * Remove all entries from the group cache
629
     */
630
    protected function clearGroupCache()
631
    {
632
        $this->groupcache = null;
633
    }
634
635
    /**
636
     * Adds the user to the group
637
     *
638
     * @param array $userdata all the user data
639
     * @param array $groupdata all the group data
640
     * @return bool
641
     */
642
    protected function joinGroup($userdata, $groupdata)
643
    {
644
        $data = array_merge($userdata, $groupdata);
645
        $sql = $this->getConf('join-group');
646
        $result = $this->query($sql, $data);
647
        if ($result === false) return false;
648
        return true;
649
    }
650
651
    /**
652
     * Removes the user from the group
653
     *
654
     * @param array $userdata all the user data
655
     * @param array $groupdata all the group data
656
     * @return bool
657
     */
658
    protected function leaveGroup($userdata, $groupdata)
659
    {
660
        $data = array_merge($userdata, $groupdata);
661
        $sql = $this->getConf('leave-group');
662
        $result = $this->query($sql, $data);
663
        if ($result === false) return false;
664
        return true;
665
    }
666
667
    /**
668
     * Executes a query
669
     *
670
     * @param string $sql The SQL statement to execute
671
     * @param array $arguments Named parameters to be used in the statement
672
     * @return array|int|bool The result as associative array for SELECTs, affected rows for others, false on error
673
     */
674
    protected function query($sql, $arguments = array())
675
    {
676
        $sql = trim($sql);
677
        if (empty($sql)) {
678
            $this->debugMsg('No SQL query given', -1, __LINE__);
679
            return false;
680
        }
681
682
        // execute
683
        $params = array();
684
        $sth = $this->pdo->prepare($sql);
685
        try {
686
            // prepare parameters - we only use those that exist in the SQL
687
            foreach ($arguments as $key => $value) {
688
                if (is_array($value)) continue;
689
                if (is_object($value)) continue;
690
                if ($key[0] != ':') $key = ":$key"; // prefix with colon if needed
691
                if (strpos($sql, $key) === false) continue; // skip if parameter is missing
692
693
                if (is_int($value)) {
694
                    $sth->bindValue($key, $value, PDO::PARAM_INT);
695
                } else {
696
                    $sth->bindValue($key, $value);
697
                }
698
                $params[$key] = $value; //remember for debugging
699
            }
700
701
            $sth->execute();
702
            if (strtolower(substr($sql, 0, 6)) == 'select') {
703
                $result = $sth->fetchAll();
704
            } else {
705
                $result = $sth->rowCount();
706
            }
707
        } catch (Exception $e) {
708
            // report the caller's line
709
            $trace = debug_backtrace();
710
            $line = $trace[0]['line'];
711
            $dsql = $this->debugSQL($sql, $params, !defined('DOKU_UNITTEST'));
712
            $this->debugMsg($e, -1, $line);
713
            $this->debugMsg("SQL: <pre>$dsql</pre>", -1, $line);
714
            $result = false;
715
        }
716
        $sth->closeCursor();
717
        $sth = null;
0 ignored issues
show
Unused Code introduced by
$sth is not used, you could remove the assignment.

This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.

$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}

Both the $myVar assignment in line 1 and the $higher assignment in line 2 are dead. The first because $myVar is never used and the second because $higher is always overwritten for every possible time line.

Loading history...
718
719
        return $result;
720
    }
721
722
    /**
723
     * Wrapper around msg() but outputs only when debug is enabled
724
     *
725
     * @param string|Exception $message
726
     * @param int $err
727
     * @param int $line
728
     */
729
    protected function debugMsg($message, $err = 0, $line = 0)
730
    {
731
        if (!$this->getConf('debug')) return;
732
        if (is_a($message, 'Exception')) {
733
            $err = -1;
734
            $msg = $message->getMessage();
0 ignored issues
show
Bug introduced by
It seems like $message is not always an object, but can also be of type string. Maybe add an additional type check?

If a variable is not always an object, we recommend to add an additional type check to ensure your method call is safe:

function someFunction(A $objectMaybe = null)
{
    if ($objectMaybe instanceof A) {
        $objectMaybe->doSomething();
    }
}
Loading history...
735
            if (!$line) $line = $message->getLine();
736
        } else {
737
            $msg = $message;
738
        }
739
740
        if (defined('DOKU_UNITTEST')) {
741
            printf("\n%s, %s:%d\n", $msg, __FILE__, $line);
742
        } else {
743
            msg('authpdo: ' . $msg, $err, $line, __FILE__);
744
        }
745
    }
746
747
    /**
748
     * Check if the given config strings are set
749
     *
750
     * @author  Matthias Grimm <[email protected]>
751
     *
752
     * @param   string[] $keys
753
     * @return  bool
754
     */
755
    protected function checkConfig($keys)
756
    {
757
        foreach ($keys as $key) {
758
            $params = explode(':', $key);
759
            $key = array_shift($params);
760
            $sql = trim($this->getConf($key));
761
762
            // check if sql is set
763
            if (!$sql) return false;
764
            // check if needed params are there
765
            foreach ($params as $param) {
766
                if (strpos($sql, ":$param") === false) return false;
767
            }
768
        }
769
770
        return true;
771
    }
772
773
    /**
774
     * create an approximation of the SQL string with parameters replaced
775
     *
776
     * @param string $sql
777
     * @param array $params
778
     * @param bool $htmlescape Should the result be escaped for output in HTML?
779
     * @return string
780
     */
781
    protected function debugSQL($sql, $params, $htmlescape = true)
782
    {
783
        foreach ($params as $key => $val) {
784
            if (is_int($val)) {
785
                $val = $this->pdo->quote($val, PDO::PARAM_INT);
786
            } elseif (is_bool($val)) {
787
                $val = $this->pdo->quote($val, PDO::PARAM_BOOL);
788
            } elseif (is_null($val)) {
789
                $val = 'NULL';
790
            } else {
791
                $val = $this->pdo->quote($val);
792
            }
793
            $sql = str_replace($key, $val, $sql);
794
        }
795
        if ($htmlescape) $sql = hsc($sql);
796
        return $sql;
797
    }
798
}
799
800
// vim:ts=4:sw=4:et:
801