Skip to content

[Security] Facilitate the management of user sessions #52412

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

Spomky
Copy link
Contributor

@Spomky Spomky commented Nov 1, 2023

Q A
Branch? 7.1
Bug fix? One point of #30914
New feature? yes
Deprecations? no
Issues none
License MIT
  • Feature
    • Interfaces and classes
    • Configuration
  • Tests
  • Documentation

Storing user sessions in databases, such as MariaDB, MySQL or PosgreSQL, is not that easy.
When reading the documentation page for the Sessions management, it appears the DB structure is quite rigid. In addition, the developers who use Doctrine and Doctrine Migrations will feel frustrated as there are some actions to be done by hand: the DB structure is not related to any entity.
Relationships between the user and the current session is not that easy as well.
In addition, the bullet "Simultaneous session limiting (e.g. each user can login only from one device at the same time)" mentioned in #30914 is hard to achieve.

In one project, session mgmt is a strong requirement and finally I used a custom handler and entities/repositories to achieve this very simply. I think it could be a good idea to integrate it directly in Symfony so that anybody could use it.

The feature implementation is much simpler:

  • An object that implements UserSessionInterface: represents the user session and the relationship with the user object
  • A repository that implements UserSessionRepositoryInterface: manages the UserSession objects
  • A UserSessionSubscriber: make sure the user is correctly associated to the session on each request (including login) and removes the session on logout
  • A UserSessionHandler: it extends AbstractSessionHandler and uses the repository for the sessions lifecycle

This is a very simple proposal at the moment. Namespace and concept to be discussed.

Example of concrete classes below.

//App/Entity/UserSession
<?php

declare(strict_types=1);

namespace App\Entity;

use App\Repository\UserSessionRepository;
use Symfony\Bundle\SecurityBundle\UserSession\UserSessionInterface;
use Doctrine\DBAL\Types\Types;
use Doctrine\ORM\Mapping as ORM;
use LogicException;
use Symfony\Component\Security\Core\User\UserInterface;

#[ORM\Entity(repositoryClass: UserSessionRepository::class)]
#[ORM\Table(name: 'users__sessions')]
class UserSession implements UserSessionInterface
{
    public function __construct(
        #[ORM\Id] #[ORM\Column] #[ORM\GeneratedValue(strategy: 'NONE')] public string $id,
        #[ORM\ManyToOne(targetEntity: User::class, fetch: 'EAGER', inversedBy: 'sessions')]
        #[ORM\JoinColumn(nullable: true)]
        public null|UserInterface $user,
        #[ORM\Column(type: Types::BLOB)] public $data,
        #[ORM\Column(type: Types::INTEGER)] public int $lifetime,
        #[ORM\Column(type: Types::INTEGER)] public int $time,
    ) {
    }

    public function getId(): string
    {
        return $this->id;
    }

    public function getData()
    {
        return $this->data;
    }

    public function setData(string $data): void
    {
        $this->data = $data;
    }

    public function getLifetime(): int
    {
        return $this->lifetime;
    }

    public function setLifetime(int $lifetime): void
    {
        $this->lifetime = $lifetime;
    }

    public function getTime(): int
    {
        return $this->time;
    }

    public function getUser(): ?UserInterface
    {
        return $this->user;
    }

    public function setUser(UserInterface $user): void
    {
        if ($this->user !== null) {
            throw new LogicException('User already set.');
        }
        $this->user = $user;
    }
}
// App\Repository\UserSessionRepository
<?php

declare(strict_types=1);

namespace App\Repository;

use App\Entity\UserSession;
use Symfony\Bundle\SecurityBundle\UserSession\UserSessionInterface;
use Symfony\Bundle\SecurityBundle\UserSession\UserSessionRepositoryInterface;
use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository;
use Doctrine\Persistence\ManagerRegistry;
use Psr\Clock\ClockInterface;
use Symfony\Component\Security\Core\User\UserInterface;

/**
 * @extends ServiceEntityRepository<UserSession>
 *
 * @method UserSession|null find($id, $lockMode = null, $lockVersion = null)
 * @method UserSession|null findOneBy(array $criteria, array $orderBy = null)
 * @method UserSession[]    findAll()
 * @method UserSession[]    findBy(array $criteria, array $orderBy = null, $limit = null, $offset = null)
 */
final class UserSessionRepository extends ServiceEntityRepository implements UserSessionRepositoryInterface
{
    public function __construct(
        ManagerRegistry $registry,
        private readonly ClockInterface $clock
    ) {
        parent::__construct($registry, UserSession::class);
    }

    public function save(UserSessionInterface $userSession): void
    {
        $this->getEntityManager()
            ->persist($userSession);

        $this->getEntityManager()
            ->flush();
    }

    public function remove(UserSessionInterface $userSession): void
    {
        $this->getEntityManager()
            ->remove($userSession);

        $this->getEntityManager()
            ->flush();
    }

    public function findOneById(string $sessionId): ?UserSessionInterface
    {
        return $this->findOneBy([
            'id' => $sessionId,
        ]);
    }

    public function removeExpired(): void
    {
        $this->createQueryBuilder('user_session')
            ->delete()
            ->where('user_session.lifetime < :currentTime')
            ->setParameter('currentTime', $this->clock->now()->getTimestamp())
            ->getQuery()
            ->execute()
        ;
    }

    public function create(string $sessionId, string $data, int $maxLifetime, int $getTimestamp): UserSessionInterface
    {
        return new UserSession($sessionId, null, $data, $maxLifetime, $getTimestamp);
    }

    /**
     * @return array<UserSessionInterface>
     */
    public function findAllUserSessions(UserInterface $user): array
    {
        return $this->createQueryBuilder('user_session')
            ->where('user_session.user = :user')
            ->setParameter('user', $user)
            ->getQuery()
            ->execute()
        ;
    }
}
//config/packags/framework.yaml
framework:
    session:
        handler_id: Symfony\Bundle\SecurityBundle\UserSession\UserSessionHandler
        ....

@carsonbot
Copy link

Hey!

Thanks for your PR. You are targeting branch "7.0" but it seems your PR description refers to branch "7.1".
Could you update the PR description or change target branch? This helps core maintainers a lot.

Cheers!

Carsonbot

@Spomky Spomky force-pushed the features/seesion-mgmt branch 2 times, most recently from a5d1225 to 0614fef Compare November 1, 2023 21:23
@Spomky Spomky force-pushed the features/seesion-mgmt branch from 0614fef to f3249f0 Compare November 1, 2023 21:25
@wouterj wouterj modified the milestones: 7.0, 7.1 Nov 1, 2023
@nicolas-grekas nicolas-grekas changed the base branch from 7.0 to 7.1 November 20, 2023 14:30
@nicolas-grekas
Copy link
Member

Doctrine Migrations will feel frustrated as there are some actions to be done by hand

Not anymore, isn't it? A migration is now auto-generated when you use Doctrine as a backend.

Would it make sense to decouple the session storage from the relationship to the session? E.g. I'd like to store my sessions in redis, but I could keep the session id in the DB so that I can do the reverse mapping from user to all it's session id. That link would also contain useful info such as the last IP address, etc.

I didn't fully grasp your proposal so that's maybe already how it works :)

@Spomky
Copy link
Contributor Author

Spomky commented Feb 22, 2024

Hi,

Due to lack of time, I did not take care to answer the questions here.

Not anymore, isn't it? A migration is now auto-generated when you use Doctrine as a backend.

Yes indeed. However this PR allows more than just have sessions migration but also stores the user ID if any. This allows to make relationship between users and their active/past sessions and manage them with ease.

Would it make sense to decouple the session storage from the relationship to the session? E.g. I'd like to store my sessions in redis, but I could keep the session id in the DB so that I can do the reverse mapping from user to all it's session id.

My first intent was to use an ORM as session storage. Because the new service relies on an interface, it is possible to store sessions whereever you want, Redis or a filesystem for exemple. I will try, but I think it is also possible to store data in Redis and the ID in the DB.

That link would also contain useful info such as the last IP address, etc.
There is no Session object here, only an Interface with the bare minimum. Technically, it seems possible to implement a method + an event listener for associating the IP address to the Session.

Also, I am wondering if it could be interesting to have the Token or at least the firewall name when an authenticated user is associated to a session. WDYT?

@Spomky Spomky marked this pull request as draft March 21, 2024 12:59
@xabbuh xabbuh modified the milestones: 7.1, 7.2 May 15, 2024
@Spomky
Copy link
Contributor Author

Spomky commented Jul 31, 2024

Not mature enough

@Spomky Spomky closed this Jul 31, 2024
@Spomky Spomky deleted the features/seesion-mgmt branch July 31, 2024 07:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants