On Fri, 26 Jun 2015 at 21:03:17, Gordian Edenhofer wrote:
> Displaying flag, notify, vote, adopt and file request links for
> users which did not authenticate themselves and letting those fake
> buttons link to the login page.
Forgot to mention it in your previous patches but could you add your
sign-off, please (simply pass -s when running `git commit`)?
> ---
> Agreed, the statements were kind of redundant.
> I hope this patch is more straightforward.
>
Looks much better indeed, thanks! Some comments below.
> web/lib/aur.inc.php | 33 ++++++++++++++++++++++-----------
> web/template/pkgbase_actions.php | 26 ++++++++++++--------------
> 2 files changed, 34 insertions(+), 25 deletions(-)
>
> diff --git a/web/lib/aur.inc.php b/web/lib/aur.inc.php
> index 95f72ce..98ebde4 100644
> --- a/web/lib/aur.inc.php
> +++ b/web/lib/aur.inc.php
> @@ -226,11 +226,16 @@ function html_format_maintainers($maintainer,
> $comaintainers) {
> *
> * @param string $uri The link target
> * @param string $desc The link label
> + * @param string $uid The User ID
> *
> * @return string The generated HTML code for the action link
> */
> -function html_action_link($uri, $desc) {
> - $code = '<a href="' . htmlspecialchars($uri, ENT_QUOTES) . '">';
> +function html_action_link($uri, $desc, $uid="") {
Do we really need to make the $uid parameter optional? Do we skip the
parameter somewhere? If it makes sense, could we please use false as
default value instead of an empty string?
> + if ($uid) {
> + $code = '<a href="' . htmlspecialchars($uri, ENT_QUOTES) .
> '">';
> + } else {
> + $code = '<a href="' . get_uri('/login/', true) . '">';
> + }
> [...]
Cool, this is kind of what I expected! I wonder whether we can directly
set a referer here, though? Shouldn't something like
get_uri('/login/', true) . '?referer=' . urlencode($uri)
work?
Having said that, I now see a potential problem with the GET parameter
approach of implementing FS#32481. You could build a malicious login
link that redirects to a certain page and send that link to a privileged
user (i.e. a TU or a Developer). I am not aware of any action that
cannot be undone and doesn't require any additional confirmation, so
that probably isn't very critical. We should fix it anyway...
Regards,
Lukas
