Question about /Security/login page

271 views
Skip to first unread message

Patrick Nelson

unread,
Oct 21, 2015, 3:36:24 PM10/21/15
to silverst...@googlegroups.com
When a user is already logged in and they're redirected to this page, they're shown a simple message like "You're logged in as [first name]."  Is there any particular reason why this doesn't just redirect the user immediately if you're already logged in?

Off hand, I do understand only one use case: If you're using the CMS and your session expires or there's some issue contacting the server, the JavaScript on that page opens this login form in a new window to ensure you can log back in to the CMS. After logging in, instead of redirecting you back to the CMS it'll show you this message, which is fine. 

This is a problem for at least two reasons:
  1. Bookmarking: Even if you have a "BackURL" parameter, it just says you're logged in which is of no benefit to a CMS user who happens to have bookmarked the login page at /Security/login?BackURL=%2Fadmin%2Fpages (easy since a user may start out at going to /admin which ultimately redirects here). This is confusing and forces the user to manually go back to /admin again if they visit the site again when they have a "remember me" cookie set.

  2. Separation of concerns: This is the Security controller with the "login" action. If you end up there and are already logged in, then why is it overlapping functionality/concerns with message delivery? On one hand it makes sense for it to say "you're already logged in" but I think it should at least:
    • Respect the "BackURL" parameter (if one is set), and/or
    • Attempt to close via JavaScript using window.close(), if it was opened by JavaScript (which fails somewhat silently in case it wasn't opened via JavaScript).
    • Offer an option for the user to click to return back to the CMS
Does that make sense? I mention this since I've had several users encounter this issue after bookmarking /Security/login?BackURL=%2Fadmin%2Fpages after explicitly letting them know to bookmark the other /admin URL -- however I do not blame them for being confused.

- Patrick

Patrick Nelson

unread,
Oct 21, 2015, 3:58:12 PM10/21/15
to silverst...@googlegroups.com
I also understand that not all people who are already logged in via /Security/login may have a set destination (e.g. not all may have permissions to access /admin, but do have some other custom implemented permissions). So that leaves other options such as the respect for "BackURL" and, if you check the appropriate permissions first, an option to continue to either the CMS (I think via CMS_ACCESS_CMSMain permission or CMS_ACCESS_LeftAndMain) or the home page.

Patrick Nelson

unread,
Oct 21, 2015, 4:08:28 PM10/21/15
to silverst...@googlegroups.com
Also for any time travelers from the future looking for a solution to the same problem, this is my workaround in the meantime:

Setup the extension:
class SecurityExtension extends Extension {

public function onBeforeSecurityLogin() {
if (!Member::currentUserID()) return;

// Perform a redirect if one is possible. Use built-in capability for security reasons.
$backURL = $this->owner->request->getVar("BackURL");
if ($backURL) $this->owner->redirectBack();
}

}

Then configure it to load via:
Security:
extensions:
- SecurityExtension

Sam Minnée

unread,
Oct 21, 2015, 5:41:01 PM10/21/15
to silverst...@googlegroups.com
The problems come when you have a user who lacks the necessary permissions and so has been sent back to the login form to log in as a different user. Your extension will cause an infinite loop. It's an example of the difficulties of fiddling with this behaviour.

Arguably, users who are already logged in shouldn't be sent to the log-in screen: they should be send to a screen that says "you don't have access" and provides a button to log out and then log in as a new user. This would be a modification to Security::permissionFailure(). If we made that change, then we wouldn't risk infinite loops from the change you propose.

If we wanted to improve this, I'd suggest:

- thinking through all the edge-cases and deciding what the best improvement of the whole system is
- making the change in the master branch, as it's a risky thing to change

Thanks,
Sam

--
You received this message because you are subscribed to the Google Groups "SilverStripe Core Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to silverstripe-d...@googlegroups.com.
To post to this group, send email to silverst...@googlegroups.com.
Visit this group at http://groups.google.com/group/silverstripe-dev.
For more options, visit https://groups.google.com/d/optout.
Reply all
Reply to author
Forward
0 new messages