[aur-dev] [PATCH v3 1/3] Use username from the database if one is provided by the user
This fixes a bug where the new user name input by the user was invalid, causing the account deletion link and the form action to be wrong. Signed-off-by: Marcel Korpel <marcel.korpel@gmail.com> --- web/html/account.php | 4 ++-- web/lib/acctfuncs.inc.php | 8 +++++--- web/template/account_edit_form.php | 4 ++-- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/web/html/account.php b/web/html/account.php index c447de3..f5e6c19 100644 --- a/web/html/account.php +++ b/web/html/account.php @@ -61,7 +61,7 @@ if (isset($_COOKIE["AURSID"])) { $row["AccountTypeID"], $row["Suspended"], $row["Email"], "", "", $row["RealName"], $row["LangPreference"], $row["IRCNick"], $row["PGPKey"], $PK, - $row["InactivityTS"] ? 1 : 0, $row["ID"]); + $row["InactivityTS"] ? 1 : 0, $row["ID"], $row["Username"]); } else { print __("You do not have permission to edit this account."); } @@ -100,7 +100,7 @@ if (isset($_COOKIE["AURSID"])) { in_request("E"), in_request("P"), in_request("C"), in_request("R"), in_request("L"), in_request("I"), in_request("K"), in_request("PK"), in_request("J"), - in_request("ID")); + in_request("ID"), $row["Username"]); } } else { if (has_credential(CRED_ACCOUNT_SEARCH)) { diff --git a/web/lib/acctfuncs.inc.php b/web/lib/acctfuncs.inc.php index 2b57b2d..9d6f5ee 100644 --- a/web/lib/acctfuncs.inc.php +++ b/web/lib/acctfuncs.inc.php @@ -56,11 +56,12 @@ function html_format_pgp_fingerprint($fingerprint) { * @param string $PK The list of SSH public keys * @param string $J The inactivity status of the displayed user * @param string $UID The user ID of the displayed user + * @param string $N The username as present in the database * * @return void */ function display_account_form($A,$U="",$T="",$S="",$E="",$P="",$C="",$R="", - $L="",$I="",$K="",$PK="",$J="", $UID=0) { + $L="",$I="",$K="",$PK="",$J="",$UID=0,$N="") { global $SUPPORTED_LANGS; include("account_edit_form.php"); @@ -86,11 +87,12 @@ function display_account_form($A,$U="",$T="",$S="",$E="",$P="",$C="",$R="", * @param string $PK The list of public SSH keys * @param string $J The inactivity status of the user * @param string $UID The user ID of the modified account + * @param string $N The username as present in the database * * @return string|void Return void if successful, otherwise return error */ function process_account_form($TYPE,$A,$U="",$T="",$S="",$E="",$P="",$C="", - $R="",$L="",$I="",$K="",$PK="",$J="",$UID=0) { + $R="",$L="",$I="",$K="",$PK="",$J="",$UID=0,$N="") { global $SUPPORTED_LANGS; $error = ''; @@ -247,7 +249,7 @@ function process_account_form($TYPE,$A,$U="",$T="",$S="",$E="",$P="",$C="", if ($error) { print "<ul class='errorlist'><li>".$error."</li></ul>\n"; display_account_form($A, $U, $T, $S, $E, "", "", - $R, $L, $I, $K, $PK, $J, $UID); + $R, $L, $I, $K, $PK, $J, $UID, $N); return; } diff --git a/web/template/account_edit_form.php b/web/template/account_edit_form.php index 56bdd45..0aadb9d 100644 --- a/web/template/account_edit_form.php +++ b/web/template/account_edit_form.php @@ -1,9 +1,9 @@ <?php if ($A == "UpdateAccount"): ?> <p> - <?= __('Click %shere%s if you want to permanently delete this account.', '<a href="' . get_user_uri($U) . 'delete/' . '">', '</a>') ?> + <?= __('Click %shere%s if you want to permanently delete this account.', '<a href="' . get_user_uri($N) . 'delete/' . '">', '</a>') ?> </p> -<form id="edit-profile-form" action="<?= get_user_uri($U) . 'update/'; ?>" method="post"> +<form id="edit-profile-form" action="<?= get_user_uri($N) . 'update/'; ?>" method="post"> <?php else: ?> <form id="edit-profile-form" action="<?= get_uri('/register/'); ?>" method="post"> <?php endif; ?> -- 2.4.5
Signed-off-by: Marcel Korpel <marcel.korpel@gmail.com> --- web/html/register.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/html/register.php b/web/html/register.php index 014d802..cb3e8dd 100644 --- a/web/html/register.php +++ b/web/html/register.php @@ -25,7 +25,7 @@ if (in_request("Action") == "NewAccount") { in_request("PK")); } else { - print __("Use this form to create an account."); + print '<p>' . __("Use this form to create an account.") . '</p>'; display_account_form("NewAccount", "", "", "", "", "", "", "", $LANG); } -- 2.4.5
Don't print messages (and the account form) in process_account_form() anymore, but return them to the caller. When updating accounts, this function will be called before the headers are written. Signed-off-by: Marcel Korpel <marcel.korpel@gmail.com> --- Change since v2: * Don't buffer output anymore and send a new (GET) request, but process process the data before outputting headers. web/html/account.php | 29 ++++++++++++++++++----------- web/html/register.php | 11 ++++++++++- web/lib/acctfuncs.inc.php | 31 ++++++++++++++++--------------- 3 files changed, 44 insertions(+), 27 deletions(-) diff --git a/web/html/account.php b/web/html/account.php index f5e6c19..9aa48a3 100644 --- a/web/html/account.php +++ b/web/html/account.php @@ -19,6 +19,23 @@ if (in_array($action, $need_userinfo)) { $PK = implode("\n", account_get_ssh_keys($row["ID"])); } +/* This has to be done before the navigation headers are written */ +if ($action == "UpdateAccount") { + $updateAccountMessage = ''; + /* Details for account being updated */ + /* Verify user permissions and that the request is a valid POST */ + if (can_edit_account($row) && check_token()) { + /* Update the details for the existing account */ + list($success, $updateAccountMessage) = process_account_form( + "edit", "UpdateAccount", + in_request("U"), in_request("T"), in_request("S"), + in_request("E"), in_request("P"), in_request("C"), + in_request("R"), in_request("L"), in_request("I"), + in_request("K"), in_request("PK"), in_request("J"), + in_request("ID"), $row["Username"]); + } +} + if ($action == "AccountInfo") { html_header(__('Account') . ' ' . $row['Username']); } else { @@ -91,17 +108,7 @@ if (isset($_COOKIE["AURSID"])) { } } elseif ($action == "UpdateAccount") { - /* Details for account being updated */ - /* Verify user permissions and that the request is a valid POST */ - if (can_edit_account($row) && check_token()) { - /* Update the details for the existing account */ - process_account_form("edit", "UpdateAccount", - in_request("U"), in_request("T"), in_request("S"), - in_request("E"), in_request("P"), in_request("C"), - in_request("R"), in_request("L"), in_request("I"), - in_request("K"), in_request("PK"), in_request("J"), - in_request("ID"), $row["Username"]); - } + print $updateAccountMessage; } else { if (has_credential(CRED_ACCOUNT_SEARCH)) { # display the search page if they're a TU/dev diff --git a/web/html/register.php b/web/html/register.php index cb3e8dd..9c5c1cc 100644 --- a/web/html/register.php +++ b/web/html/register.php @@ -19,11 +19,20 @@ echo '<div class="box">'; echo '<h2>' . __('Register') . '</h2>'; if (in_request("Action") == "NewAccount") { - process_account_form("new", "NewAccount", in_request("U"), 1, 0, + list($success, $message) = process_account_form( + "new", "NewAccount", in_request("U"), 1, 0, in_request("E"), '', '', in_request("R"), in_request("L"), in_request("I"), in_request("K"), in_request("PK")); + print $message; + + if (!$success) { + display_account_form("NewAccount", in_request("U"), 1, 0, + in_request("E"), '', '', in_request("R"), + in_request("L"), in_request("I"), in_request("K"), + in_request("PK")); + } } else { print '<p>' . __("Use this form to create an account.") . '</p>'; display_account_form("NewAccount", "", "", "", "", "", "", "", $LANG); diff --git a/web/lib/acctfuncs.inc.php b/web/lib/acctfuncs.inc.php index 9d6f5ee..8dd26c7 100644 --- a/web/lib/acctfuncs.inc.php +++ b/web/lib/acctfuncs.inc.php @@ -89,13 +89,14 @@ function display_account_form($A,$U="",$T="",$S="",$E="",$P="",$C="",$R="", * @param string $UID The user ID of the modified account * @param string $N The username as present in the database * - * @return string|void Return void if successful, otherwise return error + * @return array boolean indication success and message to be printed */ function process_account_form($TYPE,$A,$U="",$T="",$S="",$E="",$P="",$C="", $R="",$L="",$I="",$K="",$PK="",$J="",$UID=0,$N="") { global $SUPPORTED_LANGS; $error = ''; + $message = ''; if (is_ipbanned()) { $error = __('Account registration has been disabled ' . @@ -247,10 +248,8 @@ function process_account_form($TYPE,$A,$U="",$T="",$S="",$E="",$P="",$C="", } if ($error) { - print "<ul class='errorlist'><li>".$error."</li></ul>\n"; - display_account_form($A, $U, $T, $S, $E, "", "", - $R, $L, $I, $K, $PK, $J, $UID, $N); - return; + $message = "<ul class='errorlist'><li>".$error."</li></ul>\n"; + return array(false, $message); } if ($TYPE == "new") { @@ -278,25 +277,25 @@ function process_account_form($TYPE,$A,$U="",$T="",$S="",$E="",$P="",$C="", $q.= "$I, $K)"; $result = $dbh->exec($q); if (!$result) { - print __("Error trying to create account, %s%s%s.", + $message = __("Error trying to create account, %s%s%s.", "<strong>", htmlspecialchars($U,ENT_QUOTES), "</strong>"); - return; + return array(false, $message); } $uid = $dbh->lastInsertId(); account_set_ssh_keys($uid, $ssh_keys, $ssh_fingerprints); - print __("The account, %s%s%s, has been successfully created.", + $message = __("The account, %s%s%s, has been successfully created.", "<strong>", htmlspecialchars($U,ENT_QUOTES), "</strong>"); - print "<p>\n"; + $message .= "<p>\n"; if ($send_resetkey) { send_resetkey($email, true); - print __("A password reset key has been sent to your e-mail address."); - print "</p>\n"; + $message .= __("A password reset key has been sent to your e-mail address."); + $message .= "</p>\n"; } else { - print __("Click on the Login link above to use your account."); - print "</p>\n"; + $message .= __("Click on the Login link above to use your account."); + $message .= "</p>\n"; } } else { /* Modify an existing account. */ @@ -341,13 +340,15 @@ function process_account_form($TYPE,$A,$U="",$T="",$S="",$E="",$P="",$C="", $ssh_key_result = account_set_ssh_keys($UID, $ssh_keys, $ssh_fingerprints); if ($result === false || $ssh_key_result === false) { - print __("No changes were made to the account, %s%s%s.", + $message = __("No changes were made to the account, %s%s%s.", "<strong>", htmlspecialchars($U,ENT_QUOTES), "</strong>"); } else { - print __("The account, %s%s%s, has been successfully modified.", + $message = __("The account, %s%s%s, has been successfully modified.", "<strong>", htmlspecialchars($U,ENT_QUOTES), "</strong>"); } } + + return array(true, $message); } /** -- 2.4.5
On Thu, 16 Jul 2015 at 00:56:32, Marcel Korpel wrote:
Don't print messages (and the account form) in process_account_form() anymore, but return them to the caller. When updating accounts, this function will be called before the headers are written.
There is a lot of information missing here. Why does moving the process_account_form() invocation help? You should mention that the header template retrieves the user name from the database itself and will obtain updated information if the perform the UPDATE query before including that template. I had a hard time understanding this change and had to browse the code to check why this works.
Signed-off-by: Marcel Korpel <marcel.korpel@gmail.com> --- Change since v2: * Don't buffer output anymore and send a new (GET) request, but process process the data before outputting headers.
web/html/account.php | 29 ++++++++++++++++++----------- web/html/register.php | 11 ++++++++++- web/lib/acctfuncs.inc.php | 31 ++++++++++++++++--------------- 3 files changed, 44 insertions(+), 27 deletions(-)
diff --git a/web/html/account.php b/web/html/account.php index f5e6c19..9aa48a3 100644 --- a/web/html/account.php +++ b/web/html/account.php @@ -19,6 +19,23 @@ if (in_array($action, $need_userinfo)) { $PK = implode("\n", account_get_ssh_keys($row["ID"])); }
+/* This has to be done before the navigation headers are written */ +if ($action == "UpdateAccount") { + $updateAccountMessage = '';
We use underscores ($update_account_message) instead of camelCase.
+ /* Details for account being updated */ + /* Verify user permissions and that the request is a valid POST */ + if (can_edit_account($row) && check_token()) { + /* Update the details for the existing account */ + list($success, $updateAccountMessage) = process_account_form( + "edit", "UpdateAccount", + in_request("U"), in_request("T"), in_request("S"), + in_request("E"), in_request("P"), in_request("C"), + in_request("R"), in_request("L"), in_request("I"), + in_request("K"), in_request("PK"), in_request("J"), + in_request("ID"), $row["Username"]); + } +} + if ($action == "AccountInfo") { html_header(__('Account') . ' ' . $row['Username']); } else { @@ -91,17 +108,7 @@ if (isset($_COOKIE["AURSID"])) { }
} elseif ($action == "UpdateAccount") { - /* Details for account being updated */ - /* Verify user permissions and that the request is a valid POST */ - if (can_edit_account($row) && check_token()) { - /* Update the details for the existing account */ - process_account_form("edit", "UpdateAccount", - in_request("U"), in_request("T"), in_request("S"), - in_request("E"), in_request("P"), in_request("C"), - in_request("R"), in_request("L"), in_request("I"), - in_request("K"), in_request("PK"), in_request("J"), - in_request("ID"), $row["Username"]); - } + print $updateAccountMessage; } else { if (has_credential(CRED_ACCOUNT_SEARCH)) { # display the search page if they're a TU/dev diff --git a/web/html/register.php b/web/html/register.php index cb3e8dd..9c5c1cc 100644 --- a/web/html/register.php +++ b/web/html/register.php @@ -19,11 +19,20 @@ echo '<div class="box">'; echo '<h2>' . __('Register') . '</h2>';
if (in_request("Action") == "NewAccount") { - process_account_form("new", "NewAccount", in_request("U"), 1, 0, + list($success, $message) = process_account_form( + "new", "NewAccount", in_request("U"), 1, 0, in_request("E"), '', '', in_request("R"), in_request("L"), in_request("I"), in_request("K"), in_request("PK"));
+ print $message; + + if (!$success) { + display_account_form("NewAccount", in_request("U"), 1, 0, + in_request("E"), '', '', in_request("R"), + in_request("L"), in_request("I"), in_request("K"), + in_request("PK")); + } } else { print '<p>' . __("Use this form to create an account.") . '</p>'; display_account_form("NewAccount", "", "", "", "", "", "", "", $LANG); diff --git a/web/lib/acctfuncs.inc.php b/web/lib/acctfuncs.inc.php index 9d6f5ee..8dd26c7 100644 --- a/web/lib/acctfuncs.inc.php +++ b/web/lib/acctfuncs.inc.php @@ -89,13 +89,14 @@ function display_account_form($A,$U="",$T="",$S="",$E="",$P="",$C="",$R="", * @param string $UID The user ID of the modified account * @param string $N The username as present in the database * - * @return string|void Return void if successful, otherwise return error + * @return array boolean indication success and message to be printed
s/boolean indication/Boolean indicating/
*/ function process_account_form($TYPE,$A,$U="",$T="",$S="",$E="",$P="",$C="", $R="",$L="",$I="",$K="",$PK="",$J="",$UID=0,$N="") { global $SUPPORTED_LANGS;
$error = ''; + $message = '';
if (is_ipbanned()) { $error = __('Account registration has been disabled ' . @@ -247,10 +248,8 @@ function process_account_form($TYPE,$A,$U="",$T="",$S="",$E="",$P="",$C="", }
if ($error) { - print "<ul class='errorlist'><li>".$error."</li></ul>\n"; - display_account_form($A, $U, $T, $S, $E, "", "", - $R, $L, $I, $K, $PK, $J, $UID, $N);
You dropped this here but only added it to one of the call sites (account registration). Which part of the code includes the account form when an error occurs during an update? Either needs a fix or clarification (in the commit message and in the form of a comment). Thanks!
- return; + $message = "<ul class='errorlist'><li>".$error."</li></ul>\n"; + return array(false, $message); }
if ($TYPE == "new") { @@ -278,25 +277,25 @@ function process_account_form($TYPE,$A,$U="",$T="",$S="",$E="",$P="",$C="", $q.= "$I, $K)"; $result = $dbh->exec($q); if (!$result) { - print __("Error trying to create account, %s%s%s.", + $message = __("Error trying to create account, %s%s%s.", "<strong>", htmlspecialchars($U,ENT_QUOTES), "</strong>"); - return; + return array(false, $message); }
$uid = $dbh->lastInsertId(); account_set_ssh_keys($uid, $ssh_keys, $ssh_fingerprints);
- print __("The account, %s%s%s, has been successfully created.", + $message = __("The account, %s%s%s, has been successfully created.", "<strong>", htmlspecialchars($U,ENT_QUOTES), "</strong>"); - print "<p>\n"; + $message .= "<p>\n";
if ($send_resetkey) { send_resetkey($email, true); - print __("A password reset key has been sent to your e-mail address."); - print "</p>\n"; + $message .= __("A password reset key has been sent to your e-mail address."); + $message .= "</p>\n"; } else { - print __("Click on the Login link above to use your account."); - print "</p>\n"; + $message .= __("Click on the Login link above to use your account."); + $message .= "</p>\n"; } } else { /* Modify an existing account. */ @@ -341,13 +340,15 @@ function process_account_form($TYPE,$A,$U="",$T="",$S="",$E="",$P="",$C="", $ssh_key_result = account_set_ssh_keys($UID, $ssh_keys, $ssh_fingerprints);
if ($result === false || $ssh_key_result === false) { - print __("No changes were made to the account, %s%s%s.", + $message = __("No changes were made to the account, %s%s%s.", "<strong>", htmlspecialchars($U,ENT_QUOTES), "</strong>"); } else { - print __("The account, %s%s%s, has been successfully modified.", + $message = __("The account, %s%s%s, has been successfully modified.", "<strong>", htmlspecialchars($U,ENT_QUOTES), "</strong>"); } } + + return array(true, $message); }
/** -- 2.4.5
participants (2)
-
Lukas Fleischer
-
Marcel Korpel