Test Failed
Branch develop (db5506)
by Felipe
03:46
created

UsersController   D

Complexity

Total Complexity 58

Size/Duplication

Total Lines 477
Duplicated Lines 35.22 %

Importance

Changes 0
Metric Value
dl 168
loc 477
rs 4.8387
c 0
b 0
f 0
wmc 58

9 Methods

Rating   Name   Duplication   Size   Complexity  
B doAccount() 0 45 4
A doSaveCreate() 19 19 4
B doChangePassword() 52 52 7
B doDefault() 0 88 2
B doSaveEdit() 0 23 6
C doEdit() 0 61 11
B doDrop() 26 26 3
C doCreate() 0 49 7
C render() 59 59 14

How to fix   Duplicated Code    Complexity   

Duplicated Code

Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.

Common duplication problems, and corresponding solutions are:

Complex Class

 Tip:   Before tackling complexity, make sure that you eliminate any duplication first. This often can reduce the size of classes significantly.

Complex classes like UsersController often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes.

Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.

While breaking up the class, it is a good idea to analyze how other classes use UsersController, and based on these observations, apply Extract Interface, too.

1
<?php
2
3
namespace PHPPgAdmin\Controller;
4
5
use \PHPPgAdmin\Decorators\Decorator;
6
7
/**
8
 * Base controller class
9
 */
10
class UsersController extends BaseController
11
{
12
    public $_name = 'UsersController';
13
14 View Code Duplication
    public function render()
0 ignored issues
show
Duplication introduced by
This method seems to be duplicated in your project.

Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.

You can also find more detailed suggestions in the “Code” section of your repository.

Loading history...
Coding Style introduced by
render uses the super-global variable $_REQUEST which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
15
    {
16
        $this->printHeader($lang['strusers']);
0 ignored issues
show
Comprehensibility Best Practice introduced by
The variable $lang seems to be never defined.
Loading history...
17
        $this->printBody();
18
19
        switch ($action) {
0 ignored issues
show
Comprehensibility Best Practice introduced by
The variable $action seems to be never defined.
Loading history...
20
            case 'changepassword':
21
                if (isset($_REQUEST['ok'])) {
22
                    $this->doChangePassword(false);
23
                } else {
24
                    $this->doAccount();
25
                }
26
27
                break;
28
            case 'confchangepassword':
29
                $this->doChangePassword(true);
30
                break;
31
            case 'account':
32
                $this->doAccount();
33
                break;
34
            case 'save_create':
35
                if (isset($_REQUEST['cancel'])) {
36
                    $this->doDefault();
37
                } else {
38
                    $this->doSaveCreate();
39
                }
40
41
                break;
42
            case 'create':
43
                $this->doCreate();
44
                break;
45
            case 'drop':
46
                if (isset($_REQUEST['cancel'])) {
47
                    $this->doDefault();
48
                } else {
49
                    $this->doDrop(false);
50
                }
51
52
                break;
53
            case 'confirm_drop':
54
                $this->doDrop(true);
55
                break;
56
            case 'save_edit':
57
                if (isset($_REQUEST['cancel'])) {
58
                    $this->doDefault();
59
                } else {
60
                    $this->doSaveEdit();
61
                }
62
63
                break;
64
            case 'edit':
65
                $this->doEdit();
66
                break;
67
            default:
68
                $this->doDefault();
69
                break;
70
        }
71
72
        $this->printFooter();
73
    }
74
75
    /**
76
     * Show default list of users in the database
77
     */
78
    public function doDefault($msg = '')
0 ignored issues
show
Coding Style introduced by
doDefault uses the super-global variable $_REQUEST which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
79
    {
80
        $conf = $this->conf;
81
        $misc = $this->misc;
82
        $lang = $this->lang;
83
        $data = $misc->getDatabaseAccessor();
84
85
        $renderUseExpires = function ($val) use ($lang) {
86
            return $val == 'infinity' ? $lang['strnever'] : htmlspecialchars($val);
87
        };
88
89
        $this->printTrail('server');
90
        $this->printTabs('server', 'users');
91
        $this->printMsg($msg);
92
93
        $users = $data->getUsers();
94
95
        $columns = [
96
            'user'      => [
97
                'title' => $lang['strusername'],
98
                'field' => Decorator::field('usename'),
99
            ],
100
            'superuser' => [
101
                'title' => $lang['strsuper'],
102
                'field' => Decorator::field('usesuper'),
103
                'type'  => 'yesno',
104
            ],
105
            'createdb'  => [
106
                'title' => $lang['strcreatedb'],
107
                'field' => Decorator::field('usecreatedb'),
108
                'type'  => 'yesno',
109
            ],
110
            'expires'   => [
111
                'title'  => $lang['strexpires'],
112
                'field'  => Decorator::field('useexpires'),
113
                'type'   => 'callback',
114
                'params' => ['function' => $renderUseExpires, 'null' => $lang['strnever']],
115
            ],
116
            'defaults'  => [
117
                'title' => $lang['strsessiondefaults'],
118
                'field' => Decorator::field('useconfig'),
119
            ],
120
            'actions'   => [
121
                'title' => $lang['stractions'],
122
            ],
123
        ];
124
125
        $actions = [
126
            'alter' => [
127
                'content' => $lang['stralter'],
128
                'attr'    => [
129
                    'href' => [
130
                        'url'     => 'users.php',
131
                        'urlvars' => [
132
                            'action'   => 'edit',
133
                            'username' => Decorator::field('usename'),
134
                        ],
135
                    ],
136
                ],
137
            ],
138
            'drop'  => [
139
                'content' => $lang['strdrop'],
140
                'attr'    => [
141
                    'href' => [
142
                        'url'     => 'users.php',
143
                        'urlvars' => [
144
                            'action'   => 'confirm_drop',
145
                            'username' => Decorator::field('usename'),
146
                        ],
147
                    ],
148
                ],
149
            ],
150
        ];
151
152
        echo $this->printTable($users, $columns, $actions, 'users-users', $lang['strnousers']);
153
154
        $this->printNavLinks(['create' => [
155
            'attr'    => [
156
                'href' => [
157
                    'url'     => 'users.php',
158
                    'urlvars' => [
159
                        'action' => 'create',
160
                        'server' => $_REQUEST['server'],
161
                    ],
162
                ],
163
            ],
164
            'content' => $lang['strcreateuser'],
165
        ]], 'users-users', get_defined_vars());
166
    }
167
168
    /**
169
     * If a user is not a superuser, then we have an 'account management' page
170
     * where they can change their password, etc.  We don't prevent them from
171
     * messing with the URL to gain access to other user admin stuff, because
172
     * the PostgreSQL permissions will prevent them changing anything anyway.
173
     */
174
    public function doAccount($msg = '')
0 ignored issues
show
Coding Style introduced by
doAccount uses the super-global variable $_REQUEST which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
175
    {
176
        $conf = $this->conf;
177
        $misc = $this->misc;
178
        $lang = $this->lang;
179
        $data = $misc->getDatabaseAccessor();
180
181
        $server_info = $misc->getServerInfo();
182
183
        $userdata         = $data->getUser($server_info['username']);
184
        $_REQUEST['user'] = $server_info['username'];
185
186
        $this->printTrail('user');
187
        $this->printTabs('server', 'account');
188
        $this->printMsg($msg);
189
190
        if ($userdata->recordCount() > 0) {
191
            $userdata->fields['usesuper']    = $data->phpBool($userdata->fields['usesuper']);
192
            $userdata->fields['usecreatedb'] = $data->phpBool($userdata->fields['usecreatedb']);
193
            echo "<table>\n";
194
            echo "<tr><th class=\"data\">{$lang['strusername']}</th><th class=\"data\">{$lang['strsuper']}</th><th class=\"data\">{$lang['strcreatedb']}</th><th class=\"data\">{$lang['strexpires']}</th>";
195
            echo "<th class=\"data\">{$lang['strsessiondefaults']}</th>";
196
            echo "</tr>\n";
197
            echo "<tr>\n\t<td class=\"data1\">", $misc->printVal($userdata->fields['usename']), "</td>\n";
198
            echo "\t<td class=\"data1\">", $misc->printVal($userdata->fields['usesuper'], 'yesno'), "</td>\n";
199
            echo "\t<td class=\"data1\">", $misc->printVal($userdata->fields['usecreatedb'], 'yesno'), "</td>\n";
200
            echo "\t<td class=\"data1\">", ($userdata->fields['useexpires'] == 'infinity' || is_null($userdata->fields['useexpires']) ? $lang['strnever'] : $misc->printVal($userdata->fields['useexpires'])), "</td>\n";
201
            echo "\t<td class=\"data1\">", $misc->printVal($userdata->fields['useconfig']), "</td>\n";
202
            echo "</tr>\n</table>\n";
203
        } else {
204
            echo "<p>{$lang['strnodata']}</p>\n";
205
        }
206
207
        $this->printNavLinks(['changepassword' => [
208
            'attr'    => [
209
                'href' => [
210
                    'url'     => 'users.php',
211
                    'urlvars' => [
212
                        'action' => 'confchangepassword',
213
                        'server' => $_REQUEST['server'],
214
                    ],
215
                ],
216
            ],
217
            'content' => $lang['strchangepassword'],
218
        ]], 'users-account', get_defined_vars());
219
    }
220
221
    /**
222
     * Show confirmation of change password and actually change password
223
     */
224 View Code Duplication
    public function doChangePassword($confirm, $msg = '')
0 ignored issues
show
Duplication introduced by
This method seems to be duplicated in your project.

Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.

You can also find more detailed suggestions in the “Code” section of your repository.

Loading history...
Coding Style introduced by
doChangePassword uses the super-global variable $_REQUEST which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
Coding Style introduced by
doChangePassword uses the super-global variable $_POST which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
225
    {
226
        $conf = $this->conf;
227
        $misc = $this->misc;
228
        $lang = $this->lang;
229
        $data = $misc->getDatabaseAccessor();
230
231
        $server_info = $misc->getServerInfo();
232
233
        if ($confirm) {
234
            $_REQUEST['user'] = $server_info['username'];
235
            $this->printTrail('user');
236
            $this->printTitle($lang['strchangepassword'], 'pg.user.alter');
237
            $this->printMsg($msg);
238
239
            if (!isset($_POST['password'])) {
240
                $_POST['password'] = '';
241
            }
242
243
            if (!isset($_POST['confirm'])) {
244
                $_POST['confirm'] = '';
245
            }
246
247
            echo '<form action="' . SUBFOLDER . "/src/views/users.php\" method=\"post\">\n";
248
            echo "<table>\n";
249
            echo "\t<tr>\n\t\t<th class=\"data left required\">{$lang['strpassword']}</th>\n";
250
            echo "\t\t<td><input type=\"password\" name=\"password\" size=\"32\" value=\"",
251
            htmlspecialchars($_POST['password']), "\" /></td>\n\t</tr>\n";
252
            echo "\t<tr>\n\t\t<th class=\"data left required\">{$lang['strconfirm']}</th>\n";
253
            echo "\t\t<td><input type=\"password\" name=\"confirm\" size=\"32\" value=\"\" /></td>\n\t</tr>\n";
254
            echo "</table>\n";
255
            echo "<p><input type=\"hidden\" name=\"action\" value=\"changepassword\" />\n";
256
            echo $misc->form;
257
            echo "<input type=\"submit\" name=\"ok\" value=\"{$lang['strok']}\" />\n";
258
            echo "<input type=\"submit\" name=\"cancel\" value=\"{$lang['strcancel']}\" />\n";
259
            echo "</p></form>\n";
260
        } else {
261
            // Check that password is minimum length
262
            if (strlen($_POST['password']) < $conf['min_password_length']) {
263
                $this->doChangePassword(true, $lang['strpasswordshort']);
264
            }
265
266
            // Check that password matches confirmation password
267
            elseif ($_POST['password'] != $_POST['confirm']) {
268
                $this->doChangePassword(true, $lang['strpasswordconfirm']);
269
            } else {
270
                $status = $data->changePassword($server_info['username'],
271
                    $_POST['password']);
272
                if ($status == 0) {
273
                    $this->doAccount($lang['strpasswordchanged']);
274
                } else {
275
                    $this->doAccount($lang['strpasswordchangedbad']);
276
                }
277
            }
278
        }
279
    }
280
281
    /**
282
     * Function to allow editing of a user
283
     */
284
    public function doEdit($msg = '')
0 ignored issues
show
Coding Style introduced by
doEdit uses the super-global variable $_REQUEST which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
Coding Style introduced by
doEdit uses the super-global variable $_POST which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
285
    {
286
        $conf = $this->conf;
0 ignored issues
show
Unused Code introduced by
The assignment to $conf is dead and can be removed.
Loading history...
287
        $misc = $this->misc;
288
        $lang = $this->lang;
289
        $data = $misc->getDatabaseAccessor();
290
291
        $this->printTrail('user');
292
        $this->printTitle($lang['stralter'], 'pg.user.alter');
293
        $this->printMsg($msg);
294
295
        $userdata = $data->getUser($_REQUEST['username']);
296
297
        if ($userdata->recordCount() > 0) {
298
            $server_info                     = $misc->getServerInfo();
299
            $canRename                       = $data->hasUserRename() && ($_REQUEST['username'] != $server_info['username']);
300
            $userdata->fields['usesuper']    = $data->phpBool($userdata->fields['usesuper']);
301
            $userdata->fields['usecreatedb'] = $data->phpBool($userdata->fields['usecreatedb']);
302
303
            if (!isset($_POST['formExpires'])) {
304
                if ($canRename) {
305
                    $_POST['newname'] = $userdata->fields['usename'];
306
                }
307
308
                if ($userdata->fields['usesuper']) {
309
                    $_POST['formSuper'] = '';
310
                }
311
312
                if ($userdata->fields['usecreatedb']) {
313
                    $_POST['formCreateDB'] = '';
314
                }
315
316
                $_POST['formExpires']  = $userdata->fields['useexpires'] == 'infinity' ? '' : $userdata->fields['useexpires'];
317
                $_POST['formPassword'] = '';
318
            }
319
320
            echo '<form action="' . SUBFOLDER . "/src/views/users.php\" method=\"post\">\n";
321
            echo "<table>\n";
322
            echo "\t<tr>\n\t\t<th class=\"data left\">{$lang['strusername']}</th>\n";
323
            echo "\t\t<td class=\"data1\">", ($canRename ? "<input name=\"newname\" size=\"15\" maxlength=\"{$data->_maxNameLen}\" value=\"" . htmlspecialchars($_POST['newname']) . '" />' : $misc->printVal($userdata->fields['usename'])), "</td>\n\t</tr>\n";
324
            echo "\t<tr>\n\t\t<th class=\"data left\"><label for=\"formSuper\">{$lang['strsuper']}</label></th>\n";
325
            echo "\t\t<td class=\"data1\"><input type=\"checkbox\" id=\"formSuper\" name=\"formSuper\"",
326
            (isset($_POST['formSuper'])) ? ' checked="checked"' : '', " /></td>\n\t</tr>\n";
327
            echo "\t<tr>\n\t\t<th class=\"data left\"><label for=\"formCreateDB\">{$lang['strcreatedb']}</label></th>\n";
328
            echo "\t\t<td class=\"data1\"><input type=\"checkbox\" id=\"formCreateDB\" name=\"formCreateDB\"",
329
            (isset($_POST['formCreateDB'])) ? ' checked="checked"' : '', " /></td>\n\t</tr>\n";
330
            echo "\t<tr>\n\t\t<th class=\"data left\">{$lang['strexpires']}</th>\n";
331
            echo "\t\t<td class=\"data1\"><input size=\"16\" name=\"formExpires\" value=\"", htmlspecialchars($_POST['formExpires']), "\" /></td>\n\t</tr>\n";
332
            echo "\t<tr>\n\t\t<th class=\"data left\">{$lang['strpassword']}</th>\n";
333
            echo "\t\t<td class=\"data1\"><input type=\"password\" size=\"16\" name=\"formPassword\" value=\"", htmlspecialchars($_POST['formPassword']), "\" /></td>\n\t</tr>\n";
334
            echo "\t<tr>\n\t\t<th class=\"data left\">{$lang['strconfirm']}</th>\n";
335
            echo "\t\t<td class=\"data1\"><input type=\"password\" size=\"16\" name=\"formConfirm\" value=\"\" /></td>\n\t</tr>\n";
336
            echo "</table>\n";
337
            echo "<p><input type=\"hidden\" name=\"action\" value=\"save_edit\" />\n";
338
            echo '<input type="hidden" name="username" value="', htmlspecialchars($_REQUEST['username']), "\" />\n";
339
            echo $misc->form;
340
            echo "<input type=\"submit\" name=\"alter\" value=\"{$lang['stralter']}\" />\n";
341
            echo "<input type=\"submit\" name=\"cancel\" value=\"{$lang['strcancel']}\" /></p>\n";
342
            echo "</form>\n";
343
        } else {
344
            echo "<p>{$lang['strnodata']}</p>\n";
345
        }
346
    }
347
348
    /**
349
     * Function to save after editing a user
350
     */
351
    public function doSaveEdit()
0 ignored issues
show
Coding Style introduced by
doSaveEdit uses the super-global variable $_POST which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
352
    {
353
        $conf = $this->conf;
0 ignored issues
show
Unused Code introduced by
The assignment to $conf is dead and can be removed.
Loading history...
354
        $misc = $this->misc;
355
        $lang = $this->lang;
356
        $data = $misc->getDatabaseAccessor();
357
358
        // Check name and password
359
        if (isset($_POST['newname']) && $_POST['newname'] == '') {
360
            $this->doEdit($lang['struserneedsname']);
361
        } elseif ($_POST['formPassword'] != $_POST['formConfirm']) {
362
            $this->doEdit($lang['strpasswordconfirm']);
363
        } else {
364
            if (isset($_POST['newname'])) {
365
                $status = $data->setRenameUser($_POST['username'], $_POST['formPassword'], isset($_POST['formCreateDB']), isset($_POST['formSuper']), $_POST['formExpires'], $_POST['newname']);
366
            } else {
367
                $status = $data->setUser($_POST['username'], $_POST['formPassword'], isset($_POST['formCreateDB']), isset($_POST['formSuper']), $_POST['formExpires']);
368
            }
369
370
            if ($status == 0) {
371
                $this->doDefault($lang['struserupdated']);
372
            } else {
373
                $this->doEdit($lang['struserupdatedbad']);
374
            }
375
        }
376
    }
377
378
    /**
379
     * Show confirmation of drop and perform actual drop
380
     */
381 View Code Duplication
    public function doDrop($confirm)
0 ignored issues
show
Duplication introduced by
This method seems to be duplicated in your project.

Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.

You can also find more detailed suggestions in the “Code” section of your repository.

Loading history...
Coding Style introduced by
doDrop uses the super-global variable $_REQUEST which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
382
    {
383
        $conf = $this->conf;
0 ignored issues
show
Unused Code introduced by
The assignment to $conf is dead and can be removed.
Loading history...
384
        $misc = $this->misc;
385
        $lang = $this->lang;
386
        $data = $misc->getDatabaseAccessor();
387
388
        if ($confirm) {
389
            $this->printTrail('user');
390
            $this->printTitle($lang['strdrop'], 'pg.user.drop');
391
392
            echo '<p>', sprintf($lang['strconfdropuser'], $misc->printVal($_REQUEST['username'])), "</p>\n";
393
394
            echo '<form action="' . SUBFOLDER . "/src/views/users.php\" method=\"post\">\n";
395
            echo "<p><input type=\"hidden\" name=\"action\" value=\"drop\" />\n";
396
            echo '<input type="hidden" name="username" value="', htmlspecialchars($_REQUEST['username']), "\" />\n";
397
            echo $misc->form;
398
            echo "<input type=\"submit\" name=\"drop\" value=\"{$lang['strdrop']}\" />\n";
399
            echo "<input type=\"submit\" name=\"cancel\" value=\"{$lang['strcancel']}\" /></p>\n";
400
            echo "</form>\n";
401
        } else {
402
            $status = $data->dropUser($_REQUEST['username']);
403
            if ($status == 0) {
404
                $this->doDefault($lang['struserdropped']);
405
            } else {
406
                $this->doDefault($lang['struserdroppedbad']);
407
            }
408
        }
409
    }
410
411
    /**
412
     * Displays a screen where they can enter a new user
413
     */
414
    public function doCreate($msg = '')
0 ignored issues
show
Coding Style introduced by
doCreate uses the super-global variable $_POST which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
415
    {
416
        $conf = $this->conf;
0 ignored issues
show
Unused Code introduced by
The assignment to $conf is dead and can be removed.
Loading history...
417
        $misc = $this->misc;
418
        $lang = $this->lang;
419
        $data = $misc->getDatabaseAccessor();
420
421
        if (!isset($_POST['formUsername'])) {
422
            $_POST['formUsername'] = '';
423
        }
424
425
        if (!isset($_POST['formPassword'])) {
426
            $_POST['formPassword'] = '';
427
        }
428
429
        if (!isset($_POST['formConfirm'])) {
430
            $_POST['formConfirm'] = '';
431
        }
432
433
        if (!isset($_POST['formExpires'])) {
434
            $_POST['formExpires'] = '';
435
        }
436
437
        $this->printTrail('server');
438
        $this->printTitle($lang['strcreateuser'], 'pg.user.create');
439
        $this->printMsg($msg);
440
441
        echo '<form action="' . SUBFOLDER . "/src/views/users.php\" method=\"post\">\n";
442
        echo "<table>\n";
443
        echo "\t<tr>\n\t\t<th class=\"data left required\">{$lang['strusername']}</th>\n";
444
        echo "\t\t<td class=\"data1\"><input size=\"15\" maxlength=\"{$data->_maxNameLen}\" name=\"formUsername\" value=\"", htmlspecialchars($_POST['formUsername']), "\" /></td>\n\t</tr>\n";
445
        echo "\t<tr>\n\t\t<th class=\"data left\">{$lang['strpassword']}</th>\n";
446
        echo "\t\t<td class=\"data1\"><input size=\"15\" type=\"password\" name=\"formPassword\" value=\"", htmlspecialchars($_POST['formPassword']), "\" /></td>\n\t</tr>\n";
447
        echo "\t<tr>\n\t\t<th class=\"data left\">{$lang['strconfirm']}</th>\n";
448
        echo "\t\t<td class=\"data1\"><input size=\"15\" type=\"password\" name=\"formConfirm\" value=\"", htmlspecialchars($_POST['formConfirm']), "\" /></td>\n\t</tr>\n";
449
        echo "\t<tr>\n\t\t<th class=\"data left\"><label for=\"formSuper\">{$lang['strsuper']}</label></th>\n";
450
        echo "\t\t<td class=\"data1\"><input type=\"checkbox\" id=\"formSuper\" name=\"formSuper\"",
451
        (isset($_POST['formSuper'])) ? ' checked="checked"' : '', " /></td>\n\t</tr>\n";
452
        echo "\t<tr>\n\t\t<th class=\"data left\"><label for=\"formCreateDB\">{$lang['strcreatedb']}</label></th>\n";
453
        echo "\t\t<td class=\"data1\"><input type=\"checkbox\" id=\"formCreateDB\" name=\"formCreateDB\"",
454
        (isset($_POST['formCreateDB'])) ? ' checked="checked"' : '', " /></td>\n\t</tr>\n";
455
        echo "\t<tr>\n\t\t<th class=\"data left\">{$lang['strexpires']}</th>\n";
456
        echo "\t\t<td class=\"data1\"><input size=\"30\" name=\"formExpires\" value=\"", htmlspecialchars($_POST['formExpires']), "\" /></td>\n\t</tr>\n";
457
        echo "</table>\n";
458
        echo "<p><input type=\"hidden\" name=\"action\" value=\"save_create\" />\n";
459
        echo $misc->form;
460
        echo "<input type=\"submit\" name=\"create\" value=\"{$lang['strcreate']}\" />\n";
461
        echo "<input type=\"submit\" name=\"cancel\" value=\"{$lang['strcancel']}\" /></p>\n";
462
        echo "</form>\n";
463
    }
464
465
    /**
466
     * Actually creates the new user in the database
467
     */
468 View Code Duplication
    public function doSaveCreate()
0 ignored issues
show
Duplication introduced by
This method seems to be duplicated in your project.

Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.

You can also find more detailed suggestions in the “Code” section of your repository.

Loading history...
Coding Style introduced by
doSaveCreate uses the super-global variable $_POST which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
469
    {
470
        $conf = $this->conf;
0 ignored issues
show
Unused Code introduced by
The assignment to $conf is dead and can be removed.
Loading history...
471
        $misc = $this->misc;
472
        $lang = $this->lang;
473
        $data = $misc->getDatabaseAccessor();
474
475
        // Check data
476
        if ($_POST['formUsername'] == '') {
477
            $this->doCreate($lang['struserneedsname']);
478
        } elseif ($_POST['formPassword'] != $_POST['formConfirm']) {
479
            $this->doCreate($lang['strpasswordconfirm']);
480
        } else {
481
            $status = $data->createUser($_POST['formUsername'], $_POST['formPassword'],
482
                isset($_POST['formCreateDB']), isset($_POST['formSuper']), $_POST['formExpires'], []);
483
            if ($status == 0) {
484
                $this->doDefault($lang['strusercreated']);
485
            } else {
486
                $this->doCreate($lang['strusercreatedbad']);
487
            }
488
        }
489
    }
490
}
491