Suggestions for reliability and security improvements

Question

Suggestions for reliability and security improvements

Closed this issue 7 years ago · 3 comments

I am adapting a portion of A3M (the CI3 version under development). I want to share some suggestions that are not urgent, let them be on your "dream map".

I suppose that "Remember me" option would be re-implemented some day.

When a user logs-in, this fact is stored within the session. But there is no information how the user logged-in. Three cases are possible:

Using the local for site username and password.
Using the activated option "Remember me".
Using an external account in - OpenId, Facebook, etc.

Suggestions:

If a user signs-up by using an external account (Facebook, etc.) there should be always a possibility the same user to login alternatively by his local for the site account. In this case a local username and a password should be generated and passed through email. If for some technical reason the external authentication is impossible, user may choose to login directly using his/her local account. Maybe this has been implemented, but I don't recognize code for serving this way. Here an email address is needed, at the moment it is unclear to me how it could be obtained.
If a user signs-in using the activated option "Remember me", or using an external account, this fact should be remembered. In this case, for some security sensitive pages (profile edition, ordering products, payments, etc.) the user should be temporary redirected to a new login page, that will ask username and password, then, on success, the user will return automatically to the previous security sensitive page for continuation.
Within the documentation should be added some guidance for developers about logging within a separate administration panel. In this case "Remember me" and external account should not be enabled, only local username and password should be used.

Answer 1 · 2014-07-17T09:21:55.000Z

We'll get e-mail address from the provider, so generating password for them is really nice idea. (consider it being now on the feature list for 2.0 release)
I'm actually for getting rid of the "remember me" functionality, but if people want it back I will look into it.
Currently having a separate administration accounts from all the other would require some serious changes and I think it would be useful only to very few. On the other hand I think we could implement here the need to confirm your credentials like you suggested in point 2, which then could be adapted by sites that have payments, etc.

Great ideas @ivantcholakov !

Answer 2 · 2014-08-12T00:38:59.000Z

2 . I thing, a method $this->authentication->is_fully_authenticated() will do the job. And I can see its implementation is easy.

New things:

4 . Authentication: Making it no so dependent on the Session library. Then a CLI script may login by ID of a "system" or "robot" account, and when it changes data it would place within the fields "updated_by" its id. People will know whether data is touched automatically.

5 . A field like 'previous_login_at' would be nice. If a user logs in and sees its own profile this information would be useful.

Here is a piece of unfinished conceptual code for now, specific to a platform of mine:

<?php defined('BASEPATH') OR exit('No direct script access allowed');

/**
 * @author Ivan Tcholakov <ivantcholakov@gmail.com>, 2014
 * @license The MIT License, http://opensource.org/licenses/MIT
 */

defined('LOGIN_NO_ERROR') OR define('LOGIN_NO_ERROR', 0);
defined('LOGIN_NO_USER_FOUND') OR define('LOGIN_NO_USER_FOUND', 1);
defined('LOGIN_NO_PASSWORD_MATCH') OR define('LOGIN_NO_PASSWORD_MATCH', 2);
defined('LOGIN_NO_PASSWORD_SET') OR define('LOGIN_NO_PASSWORD_SET', 3);
defined('LOGIN_USER_SUSPENDED') OR define('LOGIN_USER_SUSPENDED', 4);
defined('LOGIN_USER_UNAUTHORIZED') OR define('LOGIN_USER_UNAUTHORIZED', 5);
defined('LOGIN_USER_UNVERIFIED') OR define('LOGIN_USER_UNVERIFIED', 6);
defined('LOGIN_NO_USERNAME_PROVIDED') OR define('LOGIN_NO_USERNAME_PROVIDED', 7);
defined('LOGIN_NO_PASSWORD_PROVIDED') OR define('LOGIN_NO_PASSWORD_PROVIDED', 8);
defined('LOGIN_FORCE_PASSWORD_CHANGE') OR define('LOGIN_FORCE_PASSWORD_CHANGE', 9);

class Current_user extends CI_Model {

    protected $user_id;
    protected $data;
    protected $last_login_error = LOGIN_NO_ERROR;
    protected $logout_info;
    protected $is_fully_authenticated = false;

    public function __construct() {

        parent::__construct();

        $this->load->config('users');
        $this->load->model('users');

        if (is_cli()) {

            $this->set(null);

        } else {

            // On http-request set the stored in session account_id.
            // Empty/zero value sets the Annonymous user_id = 0.
            $this->set($this->session->userdata('user_id'));
        }
    }

    public function refresh() {

        if (!$this->is_logged_in()) {

            $this->data = null;

        } else {

            $this->data = $this->users->get($this->id());

            if (empty($this->data)) {

                $this->set(null);

                return false;
            }
        }

        return true;
    }

    protected function set($id) {

        $id = $id === null ? null : (int) $id;

        $this->user_id = $id;

        if (!is_cli()) {

            if ($id !== null) {
                $this->session->set_userdata('user_id', $id);
            } else {
                $this->session->unset_userdata('user_id');
            }
        }

        return $this->refresh();
    }

    public function get() {

        return $this->data;
    }

    public function id() {

        return $this->user_id;
    }

    public function is_logged_in() {

        return $this->id() !== null;
    }

    public function is_fully_authenticated() {

        return $this->is_logged_in() && (bool) $this->is_fully_authenticated;
    }

    public function set_fully_authenticated($value = true) {

        $this->is_fully_authenticated = (bool) $value;
    }

    public function username() {

        $data = $this->get();

        if (empty($data)) {
            return null;
        }

        return $data['username'];
    }

    public function email() {

        $data = $this->get();

        if (empty($data)) {
            return null;
        }

        return $data['email'];
    }

    public function login($username, $password, $remember_me = false) {

        $logged_in = $this->_login($username, $password, false);

        // TODO: Implement "Remember me" feature.

        return $logged_in;
    }

    public function login_admin_panel($username, $password) {

        return $this->_login($username, $password, true);
    }

    public function login_by_id($id) {

        return $this->_login_by_id($id, false);
    }

    protected function _login($username, $password, $admin_panel) {

        $username = (string) $username;
        $password = (string) $password;

        $this->set(null);
        $this->last_login_error = LOGIN_NO_ERROR;
        $this->set_fully_authenticated(false);

        if ($username == '') {
            return $this->login_failed(LOGIN_NO_USERNAME_PROVIDED);
        }

        if ($password == '') {
            return $this->login_failed(LOGIN_NO_PASSWORD_PROVIDED);
        }

        $user = $this->users->get_by_username_or_email($username);

        if (empty($user)) {
            return $this->login_failed(LOGIN_NO_USER_FOUND);
        }

        $id = (int) $user['id'];
        $password_derivate = (string) $user['password'];

        if ($password_derivate == '') {
            return $this->login_failed(LOGIN_NO_PASSWORD_SET);
        }

        if (!$this->users->verify_password($password, $password_derivate)) {
            return $this->login_failed(LOGIN_NO_PASSWORD_MATCH);
        }

        $result = $this->_login_by_id($id, $admin_panel);
        $this->set_fully_authenticated($result);

        return $result;
    }

    protected function _login_by_id($id, $admin_panel) {

        $id = (int) $id;
        $admin_panel = !empty($admin_panel);

        $this->set(null);
        $this->last_login_error = LOGIN_NO_ERROR;
        $this->set_fully_authenticated(false);

        $user = $this->users->get($id);

        if (empty($user)) {
            return $this->login_failed(LOGIN_NO_USER_FOUND);
        }

        if ($user['verified_at'] == '') {
            return $this->login_failed(LOGIN_USER_UNVERIFIED);
        }

        if (empty($user['active'])) {
            return $this->login_failed(LOGIN_USER_SUSPENDED);
        }

        if ($admin_panel) {

            // A specific check for login within the admin panel.

            $group_id = (int) $user['group_id'];

            // 1 == Administrators.

            if ($group_id != 1) {
                return $this->login_failed(LOGIN_USER_UNAUTHORIZED);
            }
        }

        if (!$this->set($id)) {
            return $this->login_failed(LOGIN_NO_USER_FOUND);
        }

        $this->users->skip_observers()->update($id, array('previous_login_at' => $user['last_login_at'], 'last_login_at' => date('Y-m-d H:i:s')));

        return $this->login_successfull(LOGIN_NO_ERROR);
    }

    protected function login_successfull($login_result) {

        $this->last_login_error = $login_result;

        return true;
    }

    protected function login_failed($login_error) {

        $this->last_login_error = $login_error;

        return false;
    }

    public function last_login_error() {

        return $this->last_login_error;
    }

    public function logout() {

        $this->last_login_error = LOGIN_NO_ERROR;
        $this->logout_info = $this->get();

        $this->set(null);
        $this->set_fully_authenticated(false);

        if (!is_cli() && $this->config->item('logout_destroys_session')) {
            $this->session->sess_destroy();
        }
    }

    public function logout_info() {

        if ($this->is_logged_in()) {
            return $this->get();
        }

        return $this->logout_info;
    }

}

Answer 3 · 2014-08-16T20:44:08.000Z

I got a working autologin feature in CI3, implemented as a separate model. It has dependencies on other stuff of mine, but it could be reworked for clean CI.

While playing with it, I realized there is another easy thing to be added:

6 . $this->authentication->new_session_started() - returns boolean result.

This is why: I have autologin ("Remember Me") that expires after 60 days for example. My native PHP session expires after 4 hours. After session expiration, the autologin feature authenticates the user again, but except the current user id, all other previous session data is to be gone.

The new method informs that logged-in user has lost his/her previous session. It helps redirections to be made, for example to site's homepage or to the authenticated user private landing page.