Issues in SessionSecurityListener.php - All Issues - Inspection of "Merge pull request #2335 from acrobat/fixed-remain..." - Kunstmaan/KunstmaanBundlesCMS - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( 1de9b7...830752 )

by Kristof

created 2019-02-06 14:09 UTC

EventListener/SessionSecurityListener.php (1 issue)

Showing only issues like (Show All)

possible assignment of non-compatible types.

Bug Documentation Minor

Kristof Jochmans 1

Upgrade to new PHP Analysis Engine

These results are based on our legacy PHP analysis, consider migrating to our new PHP analysis engine instead. Learn more

<?php

namespace Kunstmaan\AdminBundle\EventListener;

use Psr\Log\LoggerInterface;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Session\SessionInterface;
use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
use Symfony\Component\HttpKernel\HttpKernelInterface;

class SessionSecurityListener
{
    /**
     * @var LoggerInterface
     */
    private $logger;

    /**
     * @var bool
     */
    private $ipCheck;

    /**
     * @var bool
     */
    private $userAgentCheck;

    /**
     * @var string
     */
    private $ip;

    /**
     * @var string
     */
    private $userAgent;

    /**
     * @param bool            $ipCheck
     * @param bool            $userAgentCheck
     * @param LoggerInterface $logger
     */
    public function __construct($ipCheck, $userAgentCheck, LoggerInterface $logger)
    {
        $this->ipCheck = $ipCheck;
        $this->userAgentCheck = $userAgentCheck;
        $this->logger = $logger;
    }

    /**
     * @param FilterResponseEvent $event
     */
    public function onKernelResponse(FilterResponseEvent $event)
    {
        if (HttpKernelInterface::MASTER_REQUEST !== $event->getRequestType()) {
            return;
        }

        // Make sure the ip and user agent is stored in the session
        $request = $event->getRequest();
        if ($request->hasSession() && $request->getSession()->isStarted()) {
            $session = $request->getSession();
            if ($this->ipCheck && !$session->has('kuma_ip')) {
                $session->set('kuma_ip', $this->getIp($request));
            }
            if ($this->userAgentCheck && !$session->has('kuma_ua')) {
                $session->set('kuma_ua', $this->getUserAgent($request));
            }
        }
    }

    /**
     * @param GetResponseEvent $event
     */
    public function onKernelRequest(GetResponseEvent $event)
    {
        if (HttpKernelInterface::MASTER_REQUEST !== $event->getRequestType()) {
            return;
        }

        $request = $event->getRequest();
        if ($request->hasSession() && $request->getSession()->isStarted()) {
            $session = $request->getSession();

            // Check that the ip matches
            if ($this->ipCheck && $session->has('kuma_ip') && $session->get('kuma_ip') != $this->getIp($request)) {
                $this->logger->error(sprintf(
                    "Session ip '%s' does not match with request ip '%s', invalidating the current session",
                    $session->get('kuma_ip'),
                    $this->getIp($request)
                ));
                $this->invalidateSession($session, $request);
            }

            // Check that the user agent matches
            if ($this->userAgentCheck && $session->has('kuma_ua') && $session->get('kuma_ua') != $this->getUserAgent($request)) {
                $this->logger->error(sprintf(
                    "Session user agent '%s' does not match with request user agent '%s', invalidating the current session",
                    $session->get('kuma_ua'),
                    $this->getUserAgent($request)
                ));
                $this->invalidateSession($session, $request);
            }
        }
    }

    /**
     * @param SessionInterface $session
     * @param Request          $request
     */
    private function invalidateSession(SessionInterface $session, Request $request)
    {
        $session->invalidate();
        $session->set('kuma_ip', $this->getIp($request));
        $session->set('kuma_ua', $this->getUserAgent($request));
    }

    /**
     * @param Request $request
     *
     * @return string
     */
    private function getIp(Request $request)
    {
        if (!$this->ip) {
            $forwarded = $request->server->get('HTTP_X_FORWARDED_FOR');
            if (strlen($forwarded) > 0) {
                $parts = explode(',', $forwarded);
                $parts = array_map('trim', $parts);
                $parts = array_filter($parts);
                if (count($parts) > 0) {
                    $ip = $parts[0];
                }
            }
            if (empty($ip)) {
                $ip = $request->getClientIp();
            }
            $this->ip = $ip;
        }

        return $this->ip;
    }

    /**
     * @param Request $request
     *
     * @return array|string
     */
    private function getUserAgent(Request $request)
    {
        if (!$this->userAgent) {
            $this->userAgent = $request->headers->get('User-Agent');
class Id
{
    public $id;

    public function __construct($id)
    {
        $this->id = $id;
    }

}

class Account
{
    /** @var  Id $id */
    public $id;
}

$account_id = false;

if (starsAreRight()) {
    $account_id = new Id(42);
}

$account = new Account();
if ($account instanceof Id)
{
    $account->id = $account_id;
}
        }

        return $this->userAgent;
    }
}


1		<?php
2
3		namespace Kunstmaan\AdminBundle\EventListener;
4
5		use Psr\Log\LoggerInterface;
6		use Symfony\Component\HttpFoundation\Request;
7		use Symfony\Component\HttpFoundation\Session\SessionInterface;
8		use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
9		use Symfony\Component\HttpKernel\Event\GetResponseEvent;
10		use Symfony\Component\HttpKernel\HttpKernelInterface;
11
12		class SessionSecurityListener
13		{
14		/**
15		* @var LoggerInterface
16		*/
17		private $logger;
18
19		/**
20		* @var bool
21		*/
22		private $ipCheck;
23
24		/**
25		* @var bool
26		*/
27		private $userAgentCheck;
28
29		/**
30		* @var string
31		*/
32		private $ip;
33
34		/**
35		* @var string
36		*/
37		private $userAgent;
38
39		/**
40		* @param bool $ipCheck
41		* @param bool $userAgentCheck
42		* @param LoggerInterface $logger
43		*/
44		public function __construct($ipCheck, $userAgentCheck, LoggerInterface $logger)
45		{
46		$this->ipCheck = $ipCheck;
47		$this->userAgentCheck = $userAgentCheck;
48		$this->logger = $logger;
49		}
50
51		/**
52		* @param FilterResponseEvent $event
53		*/
54		public function onKernelResponse(FilterResponseEvent $event)
55		{
56		if (HttpKernelInterface::MASTER_REQUEST !== $event->getRequestType()) {
57		return;
58		}
59
60		// Make sure the ip and user agent is stored in the session
61		$request = $event->getRequest();
62		if ($request->hasSession() && $request->getSession()->isStarted()) {
63		$session = $request->getSession();
64		if ($this->ipCheck && !$session->has('kuma_ip')) {
65		$session->set('kuma_ip', $this->getIp($request));
66		}
67		if ($this->userAgentCheck && !$session->has('kuma_ua')) {
68		$session->set('kuma_ua', $this->getUserAgent($request));
69		}
70		}
71		}
72
73		/**
74		* @param GetResponseEvent $event
75		*/
76		public function onKernelRequest(GetResponseEvent $event)
77		{
78		if (HttpKernelInterface::MASTER_REQUEST !== $event->getRequestType()) {
79		return;
80		}
81
82		$request = $event->getRequest();
83		if ($request->hasSession() && $request->getSession()->isStarted()) {
84		$session = $request->getSession();
85
86		// Check that the ip matches
87	View Code Duplication	if ($this->ipCheck && $session->has('kuma_ip') && $session->get('kuma_ip') != $this->getIp($request)) {
88		$this->logger->error(sprintf(
89		"Session ip '%s' does not match with request ip '%s', invalidating the current session",
90		$session->get('kuma_ip'),
91		$this->getIp($request)
92		));
93		$this->invalidateSession($session, $request);
94		}
95
96		// Check that the user agent matches
97	View Code Duplication	if ($this->userAgentCheck && $session->has('kuma_ua') && $session->get('kuma_ua') != $this->getUserAgent($request)) {
98		$this->logger->error(sprintf(
99		"Session user agent '%s' does not match with request user agent '%s', invalidating the current session",
100		$session->get('kuma_ua'),
101		$this->getUserAgent($request)
102		));
103		$this->invalidateSession($session, $request);
104		}
105		}
106		}
107
108		/**
109		* @param SessionInterface $session
110		* @param Request $request
111		*/
112		private function invalidateSession(SessionInterface $session, Request $request)
113		{
114		$session->invalidate();
115		$session->set('kuma_ip', $this->getIp($request));
116		$session->set('kuma_ua', $this->getUserAgent($request));
117		}
118
119		/**
120		* @param Request $request
121		*
122		* @return string
123		*/
124		private function getIp(Request $request)
125		{
126		if (!$this->ip) {
127		$forwarded = $request->server->get('HTTP_X_FORWARDED_FOR');
128		if (strlen($forwarded) > 0) {
129		$parts = explode(',', $forwarded);
130		$parts = array_map('trim', $parts);
131		$parts = array_filter($parts);
132		if (count($parts) > 0) {
133		$ip = $parts[0];
134		}
135		}
136		if (empty($ip)) {
137		$ip = $request->getClientIp();
138		}
139		$this->ip = $ip;
140		}
141
142		return $this->ip;
143		}
144
145		/**
146		* @param Request $request
147		*
148		* @return array\|string
149		*/
150		private function getUserAgent(Request $request)
151		{
152		if (!$this->userAgent) {
153		$this->userAgent = $request->headers->get('User-Agent');
		0 ignored issues – show Documentation Bug introduced 2018-04-18 13:05 UTC by Report Bug Copy Issue Report Show Similar Issues like this It seems like `$request->headers->get('User-Agent')` can also be of type `array<integer,string>`. However, the property `$userAgent` is declared as type `string`. Maybe add an additional type check? Our type inference engine has found a suspicous assignment of a value to a property. This check raises an issue when a value that can be of a mixed type is assigned to a property that is type hinted more strictly. For example, imagine you have a variable `$accountId` that can either hold an Id object or false (if there is no account id yet). Your code now assigns that value to the `id` property of an instance of the `Account` class. This class holds a proper account, so the id value must no longer be false. Either this assignment is in error or a type check should be added for that assignment. class Id { public $id; public function __construct($id) { $this->id = $id; } } class Account { /** @var Id $id */ public $id; } $account_id = false; if (starsAreRight()) { $account_id = new Id(42); } $account = new Account(); if ($account instanceof Id) { $account->id = $account_id; } Loading history...
154		}
155
156		return $this->userAgent;
157		}
158		}
159

Kunstmaan / KunstmaanBundlesCMS

Push — master ( 1de9b7...830752 )

EventListener/SessionSecurityListener.php (1 issue)

Showing only issues like (Show All)

Introduced By

Upgrade to new PHP Analysis Engine

Duplication Side-by-Side

Filter issues like