[aur-dev] [PATCH] Redirect at previous page after a successful login

[Lukas Fleischer]

On Thu, 18 Jun 2015 at 21:28:17, Gordian Edenhofer wrote:
> After the user was authenticated a redirect to the site which
> linked the user to the login page is done. This fixes FS#32481.
> ---
>  web/html/login.php        |  1 +
>  web/lib/acctfuncs.inc.php | 15 ++++++++++++++-
>  2 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/web/html/login.php b/web/html/login.php
> index f898a57..1b3a589 100644
> --- a/web/html/login.php
> +++ b/web/html/login.php
> @@ -42,6 +42,7 @@ html_header('AUR ' . __("Login"));
>                         <p>
>                                 <input type="submit" class="button" value="<?php  print __("Login"); ?>" />
>                                 <a href="<?= get_uri('/passreset/') ?>">[<?= __('Forgot Password') ?>]</a>
> +                               <input id="id_referer" type="hidden" name="referer" value="<?= !empty($_SERVER['HTTP_REFERER']) ? $_SERVER['HTTP_REFERER'] : '/'; ?>" />

Please use urlencode() to escape the value of $_SERVER['HTTP_REFERER'].

Also, I would prefer not setting the referer field at all if the HTTP
header is not available:

    <?php if (isset($_SERVER['HTTP_REFERER'])): ?>
    <input id="id_referer" type="hidden" name="referer" value="<?= urlencode($_SERVER['HTTP_REFERER']) ?>" />
    <?php endif; ?>

It would be nice if we could avoid the use of HTTP referers but it seems
like a good temporary solution. Thank you for implementing this, some
more comments below.

>                         </p>
>                 </fieldset>
>         </form>
> diff --git a/web/lib/acctfuncs.inc.php b/web/lib/acctfuncs.inc.php
> index 20ac081..127a991 100644
> --- a/web/lib/acctfuncs.inc.php
> +++ b/web/lib/acctfuncs.inc.php
> @@ -544,7 +544,20 @@ function try_login() {
>         }
>  
>         setcookie("AURSID", $new_sid, $cookie_time, "/", null, !empty($_SERVER['HTTPS']), true);
> -       header("Location: " . get_uri('/'));
> +
> +       /**
> +        * Check whether the site itself refered here and if so refer back to its origin
> +        *
> +        * One major drawback is that POST request are not handled properly, the only possible
> +        * solution I could think of is to use JavaScript to auto submit a hidden form, though
> +        * it would slow down the page load time and would require js for a successful redirect.
> +        * This hard dependcy is not somehtings I want to implement since this problem is too
> +        * minor for such an agressive approach IMHO.
> +        */

This comment doesn't really belong here. You could have added it below
the "---" marker that follows the commit message.

I am also not quite sure which issue you are referring to here. Are you
talking about the problem described in FS#31882 [1]?

> +       $referer = !empty($_REQUEST['referer']) ? $_REQUEST['referer'] : '/';

You can use the in_request() helper function here.

> +       $aur_location = aur_location();

No need for an extra variable here. Just call aur_location() in the line
below.

> +       $referer = strpos($referer, $aur_location) === 0 ? $referer : '/';

I'd prefer parenthesis around the comparison here. Maybe even use an
if-block for improved readability?

> +       header("Location: " . get_uri( $referer ));

Style: No spaces before and after the parameter list, please.

>         $login_error = "";
>  }
>  
> -- 
> 2.4.4

[1] https://bugs.archlinux.org/task/31882