[Feature Request] - Redirect login page if already logged in

[ClaraLeigh]

Hey,

Is there a reason the login page doesn't redirect you if you're already logged in?
I can't think of a reason why we shouldn't redirect already logged in users.

Seems like a fairly easy thing to do seeing there is already a "ChooseRedirect" function inside the login controller.

Anyway, this would be my suggestion.

File: /web/concrete/controllers/single_page/login.php

public function view($type = null, $element = 'form')
    {
        // Do default redirect if already logged in
        $u = new User();
        if ($u->isRegistered()) {
            $this->chooseRedirect();
        }

        $this->requireAsset('javascript', 'backstretch');
        $this->set('authTypeParams', $this->getSets());
        if (strlen($type)) {
            $at = AuthenticationType::getByHandle($type);
            $this->set('authType', $at);
            $this->set('authTypeElement', $element);
        }
    }

[joe-meyer]

I agree that this should redirect, or give a 403 permission denied error (and indicate that you're already logged in).

[Remo]

seeing that codes makes we wonder what's the difference between isLoggedIn and isRegistered, but yeah I agree with this change.

[joe-meyer]

seeing that codes makes we wonder what's the difference between isLoggedIn and isRegistered, but yeah I agree with this change.

isRegistered isn't a static function and simply checks that the uID is greater than 0 on the object, whereas isLoggedIn is a static function and checks the $_SESSION for some user information.

[Remo]

Well, this I understand, but why do we need both? isRegistered is also a bit strange because if I fetch another user object (!= current user) and then call isRegistered I get true which kind of obvious but also useless..

[joe-meyer]

Well that's not entirely true. If you attempt to fetch a user object that doesn't exist in the database for instance then this would return false. Also, you'd probably want to make sure that the user isActive as well as registered.

Ultimately I would think we would want to get away from using the current implementation of the isLoggedIn method since it's dependent on sessions and makes stateless applications difficult. As an example, if I wanted to implement a REST API that used auth keys on each requests, I wouldn't want to be depending on those sessions to exist, I would want to have my request instantiate my user object, and then check if it is logged in by checking that user's properties.

[Remo]

Of course, there are a few edge cases like that, just wanted to point out that those method might need some attention, didn't want to hijack this issue..

[aembler]

I think the sad answer is just duplicate crusty code. I think we'd want to keep isRegistered() and alias isLoggedIn() over to isRegistered() (setting aside any concerns about how isRegistered() actually populates its own data – it should be the canonical, proper method.)

[aembler]

Seems like a good addition

[Mnkras]

Another thing, the login page could technically be an editable page, to edit it, you need to be able to get to that page...

[KorvinSzanto]

I have a working branch somewhere that shows "log out" as an option if
you're logged in, I'd like to see if I can dig that up before we merge
this.

On Wed, Apr 15, 2015, 8:46 PM Andrew Embler notifications@github.com
wrote:

Seems like a good addition

—
Reply to this email directly or view it on GitHub
#2249 (comment)
.

[aembler]

We are going to update this page so that the authentication types don't show up when you're logged in - you just get a logout button. This should allow you to still edit the content on the page.