ValidateController::v3ValidateAction() - Code Metrics - Inspection of "add more tests" - leo108/laravel_cas_server - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( 59d827...fe270b )

by leo

created 2016-10-12 13:50 UTC

ValidateController::v3ValidateAction() A

↳ Parent: ValidateController

Complexity

Conditions	1
Paths	1

Size

Total Lines	4
Code Lines	2

Duplication

Lines	0
Ratio	0 %

Code Coverage

Tests	2
CRAP Score	1

Importance

Changes

Metric	Value
cc	1
eloc	2
nc	1
nop	1
dl	0
loc	4
ccs	2
cts	2
cp	1
crap	1
rs	10
c	0
b	0
f	0

<?php
/**
 * Created by PhpStorm.
 * User: chenyihong
 * Date: 16/8/1
 * Time: 14:52
 */

namespace Leo108\CAS\Http\Controllers;

use Illuminate\Support\Str;
use Leo108\CAS\Contracts\TicketLocker;
use Leo108\CAS\Repositories\TicketRepository;
use Leo108\CAS\Exceptions\CAS\CasException;
use Leo108\CAS\Models\Ticket;
use Illuminate\Http\Request;
use Illuminate\Http\Response;
use SimpleXMLElement;

class ValidateController extends Controller
{
    /**
     * @var TicketLocker
     */
    protected $ticketLocker;
    /**
     * @var TicketRepository
     */
    protected $ticketRepository;

    const BASE_XML = '<cas:serviceResponse xmlns:cas="http://www.yale.edu/tp/cas"></cas:serviceResponse>';

    /**
     * ValidateController constructor.
     * @param TicketLocker     $ticketLocker
     * @param TicketRepository $ticketRepository
     */
    public function __construct(TicketLocker $ticketLocker, TicketRepository $ticketRepository)
    {
        $this->ticketLocker     = $ticketLocker;
        $this->ticketRepository = $ticketRepository;
    }

    public function v1ValidateAction(Request $request)
    {
        $service = $request->get('service', '');
        $ticket  = $request->get('ticket', '');
        if (empty($service) || empty($ticket)) {
            return new Response('no');
        }

        if (!$this->lockTicket($ticket)) {
            return new Response('no');
        }
        $record = $this->ticketRepository->getByTicket($ticket);
        if (!$record || $record->service_url != $service) {
            $this->unlockTicket($ticket);

            return new Response('no');
        }
        $this->ticketRepository->invalidTicket($record);
/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}

        $this->unlockTicket($ticket);

        return new Response('yes');
    }

    public function v2ValidateAction(Request $request)
    {
        return $this->casValidate($request, false);
    }

    public function v3ValidateAction(Request $request)
    {
        return $this->casValidate($request, true);
    }

    /**
     * @param Request $request
     * @param bool    $returnAttr
     * @return Response
     */
    protected function casValidate(Request $request, $returnAttr)
    {
        $service = $request->get('service', '');
        $ticket  = $request->get('ticket', '');
        $format  = strtoupper($request->get('format', 'XML'));
        if (empty($service) || empty($ticket)) {
            return $this->failureResponse(
                CasException::INVALID_REQUEST,
                'param service and ticket can not be empty',
                $format
            );
        }

        if (!$this->lockTicket($ticket)) {
            return $this->failureResponse(CasException::INTERNAL_ERROR, 'try to lock ticket failed', $format);
        }

        $record = $this->ticketRepository->getByTicket($ticket);
        try {
            if (!$record) {
                throw new CasException(CasException::INVALID_TICKET, 'ticket is not valid');
            }

            if ($record->service_url != $service) {
                throw new CasException(CasException::INVALID_SERVICE, 'service is not valid');
            }
        } catch (CasException $e) {
            //invalid ticket if error occur
            $record instanceof Ticket && $this->ticketRepository->invalidTicket($record);
            $this->unlockTicket($ticket);

            return $this->failureResponse($e->getCasErrorCode(), $e->getMessage(), $format);
        }
        $this->ticketRepository->invalidTicket($record);
/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}
        $this->unlockTicket($ticket);

        $attr = $returnAttr ? $record->user->getCASAttributes() : [];

        return $this->successResponse($record->user->getName(), $attr, $format);
    }

    /**
     * @param string $username
     * @param array  $attrs
     * @param string $format
     * @return Response
     */
    protected function successResponse($username, $attrs, $format)
    {
        if (strtoupper($format) === 'JSON') {
            $data = [
                'serviceResponse' => [
                    'authenticationSuccess' => [
                        'user' => $username,
                    ],
                ],
            ];

            if (!empty($attrs)) {
                $data['serviceResponse']['authenticationSuccess']['attributes'] = $attrs;
            }

            return new Response($data);
        } else {
            $xml          = simplexml_load_string(self::BASE_XML);
            $childSuccess = $xml->addChild('cas:authenticationSuccess');
            $childSuccess->addChild('cas:user', $username);

            if (!empty($attrs)) {
                $childAttrs = $childSuccess->addChild('cas:attributes');
                foreach ($attrs as $key => $value) {
                    $childAttrs->addChild('cas:'.$key, $value);
                }
            }

            return $this->returnXML($xml);
        }
    }

    /**
     * @param string $code
     * @param string $desc
     * @param string $format
     * @return Response
     */
    protected function failureResponse($code, $desc, $format)
    {
        if (strtoupper($format) === 'JSON') {
            return new Response(
                [
                    'serviceResponse' => [
                        'authenticationFailure' => [
                            'code'        => $code,
                            'description' => $desc,
                        ],
                    ],
                ]
            );
        } else {
            $xml          = simplexml_load_string(self::BASE_XML);
            $childFailure = $xml->addChild('cas:authenticationFailure', $desc);
            $childFailure->addAttribute('code', $code);

            return $this->returnXML($xml);
        }
    }

    /**
     * @param string $ticket
     * @return bool
     */
    protected function lockTicket($ticket)
    {
        return $this->ticketLocker->acquireLock($ticket, config('cas.lock_timeout'));
    }

    /**
     * @param string $ticket
     * @return bool
     */
    protected function unlockTicket($ticket)
    {
        return $this->ticketLocker->releaseLock($ticket);
    }

    /**
     * remove the first line of xml string
     * @param string $str
     * @return string
     */
    protected function removeXmlFirstLine($str)
    {
        $first = '<?xml version="1.0"?>';
        if (Str::startsWith($str, $first)) {
            return trim(substr($str, strlen($first)));
        }

        return $str;
    }

    /**
     * @param SimpleXMLElement $xml
     * @return Response
     */
    protected function returnXML(SimpleXMLElement $xml)
    {
        return new Response($this->removeXmlFirstLine($xml->asXML()), 200, array('Content-Type' => 'application/xml'));

    }
}

1		<?php
2		/**
3		* Created by PhpStorm.
4		* User: chenyihong
5		* Date: 16/8/1
6		* Time: 14:52
7		*/
8
9		namespace Leo108\CAS\Http\Controllers;
10
11		use Illuminate\Support\Str;
12		use Leo108\CAS\Contracts\TicketLocker;
13		use Leo108\CAS\Repositories\TicketRepository;
14		use Leo108\CAS\Exceptions\CAS\CasException;
15		use Leo108\CAS\Models\Ticket;
16		use Illuminate\Http\Request;
17		use Illuminate\Http\Response;
18		use SimpleXMLElement;
19
20		class ValidateController extends Controller
21		{
22		/**
23		* @var TicketLocker
24		*/
25		protected $ticketLocker;
26		/**
27		* @var TicketRepository
28		*/
29		protected $ticketRepository;
30
31		const BASE_XML = '<cas:serviceResponse xmlns:cas="http://www.yale.edu/tp/cas"></cas:serviceResponse>';
32
33		/**
34		* ValidateController constructor.
35		* @param TicketLocker $ticketLocker
36		* @param TicketRepository $ticketRepository
37		*/
38	4	public function __construct(TicketLocker $ticketLocker, TicketRepository $ticketRepository)
39		{
40	4	$this->ticketLocker = $ticketLocker;
41	4	$this->ticketRepository = $ticketRepository;
42	4	}
43
44	1	public function v1ValidateAction(Request $request)
45		{
46	1	$service = $request->get('service', '');
47	1	$ticket = $request->get('ticket', '');
48	1	if (empty($service) \|\| empty($ticket)) {
49	1	return new Response('no');
50		}
51
52	1	if (!$this->lockTicket($ticket)) {
53	1	return new Response('no');
54		}
55	1	$record = $this->ticketRepository->getByTicket($ticket);
56	1	if (!$record \|\| $record->service_url != $service) {
57	1	$this->unlockTicket($ticket);
58
59	1	return new Response('no');
60		}
61	1	$this->ticketRepository->invalidTicket($record);
		0 ignored issues – show Bug introduced 2016-10-12 05:26 UTC by Report Bug Copy Issue Report It seems like `$record` defined by `$this->ticketRepository->getByTicket($ticket)` on line `55` can also be of type `boolean`; however, `Leo108\CAS\Repositories\...sitory::invalidTicket()` does only seem to accept `object<Leo108\CAS\Models\Ticket>`, maybe add an additional type check? If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check: /** * @return array\|string */ function returnsDifferentValues($x) { if ($x) { return 'foo'; } return array(); } $x = returnsDifferentValues($y); if (is_array($x)) { // $x is an array. } If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue. Loading history...
62
63	1	$this->unlockTicket($ticket);
64
65	1	return new Response('yes');
66		}
67
68	1	public function v2ValidateAction(Request $request)
69		{
70	1	return $this->casValidate($request, false);
71		}
72
73	1	public function v3ValidateAction(Request $request)
74		{
75	1	return $this->casValidate($request, true);
76		}
77
78		/**
79		* @param Request $request
80		* @param bool $returnAttr
81		* @return Response
82		*/
83	1	protected function casValidate(Request $request, $returnAttr)
84		{
85	1	$service = $request->get('service', '');
86	1	$ticket = $request->get('ticket', '');
87	1	$format = strtoupper($request->get('format', 'XML'));
88	1	if (empty($service) \|\| empty($ticket)) {
89	1	return $this->failureResponse(
90	1	CasException::INVALID_REQUEST,
91	1	'param service and ticket can not be empty',
92		$format
93	1	);
94		}
95
96	1	if (!$this->lockTicket($ticket)) {
97	1	return $this->failureResponse(CasException::INTERNAL_ERROR, 'try to lock ticket failed', $format);
98		}
99
100	1	$record = $this->ticketRepository->getByTicket($ticket);
101		try {
102	1	if (!$record) {
103	1	throw new CasException(CasException::INVALID_TICKET, 'ticket is not valid');
104		}
105
106	1	if ($record->service_url != $service) {
107	1	throw new CasException(CasException::INVALID_SERVICE, 'service is not valid');
108		}
109	1	} catch (CasException $e) {
110		//invalid ticket if error occur
111	1	$record instanceof Ticket && $this->ticketRepository->invalidTicket($record);
112	1	$this->unlockTicket($ticket);
113
114	1	return $this->failureResponse($e->getCasErrorCode(), $e->getMessage(), $format);
115		}
116	1	$this->ticketRepository->invalidTicket($record);
		0 ignored issues – show Bug introduced 2016-10-12 05:26 UTC by Report Bug Copy Issue Report It seems like `$record` defined by `$this->ticketRepository->getByTicket($ticket)` on line `100` can also be of type `boolean`; however, `Leo108\CAS\Repositories\...sitory::invalidTicket()` does only seem to accept `object<Leo108\CAS\Models\Ticket>`, maybe add an additional type check? If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check: /** * @return array\|string */ function returnsDifferentValues($x) { if ($x) { return 'foo'; } return array(); } $x = returnsDifferentValues($y); if (is_array($x)) { // $x is an array. } If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue. Loading history...
117	1	$this->unlockTicket($ticket);
118
119	1	$attr = $returnAttr ? $record->user->getCASAttributes() : [];
120
121	1	return $this->successResponse($record->user->getName(), $attr, $format);
122		}
123
124		/**
125		* @param string $username
126		* @param array $attrs
127		* @param string $format
128		* @return Response
129		*/
130	1	protected function successResponse($username, $attrs, $format)
131		{
132	1	if (strtoupper($format) === 'JSON') {
133		$data = [
134		'serviceResponse' => [
135		'authenticationSuccess' => [
136	1	'user' => $username,
137	1	],
138	1	],
139	1	];
140
141	1	if (!empty($attrs)) {
142	1	$data['serviceResponse']['authenticationSuccess']['attributes'] = $attrs;
143	1	}
144
145	1	return new Response($data);
146		} else {
147	1	$xml = simplexml_load_string(self::BASE_XML);
148	1	$childSuccess = $xml->addChild('cas:authenticationSuccess');
149	1	$childSuccess->addChild('cas:user', $username);
150
151	1	if (!empty($attrs)) {
152	1	$childAttrs = $childSuccess->addChild('cas:attributes');
153	1	foreach ($attrs as $key => $value) {
154	1	$childAttrs->addChild('cas:'.$key, $value);
155	1	}
156	1	}
157
158	1	return $this->returnXML($xml);
159		}
160		}
161
162		/**
163		* @param string $code
164		* @param string $desc
165		* @param string $format
166		* @return Response
167		*/
168	1	protected function failureResponse($code, $desc, $format)
169		{
170	1	if (strtoupper($format) === 'JSON') {
171	1	return new Response(
172		[
173		'serviceResponse' => [
174		'authenticationFailure' => [
175	1	'code' => $code,
176	1	'description' => $desc,
177	1	],
178	1	],
179		]
180	1	);
181		} else {
182	1	$xml = simplexml_load_string(self::BASE_XML);
183	1	$childFailure = $xml->addChild('cas:authenticationFailure', $desc);
184	1	$childFailure->addAttribute('code', $code);
185
186	1	return $this->returnXML($xml);
187		}
188		}
189
190		/**
191		* @param string $ticket
192		* @return bool
193		*/
194	1	protected function lockTicket($ticket)
195		{
196	1	return $this->ticketLocker->acquireLock($ticket, config('cas.lock_timeout'));
197		}
198
199		/**
200		* @param string $ticket
201		* @return bool
202		*/
203	1	protected function unlockTicket($ticket)
204		{
205	1	return $this->ticketLocker->releaseLock($ticket);
206		}
207
208		/**
209		* remove the first line of xml string
210		* @param string $str
211		* @return string
212		*/
213		protected function removeXmlFirstLine($str)
214		{
215		$first = '<?xml version="1.0"?>';
216		if (Str::startsWith($str, $first)) {
217		return trim(substr($str, strlen($first)));
218		}
219
220		return $str;
221		}
222
223		/**
224		* @param SimpleXMLElement $xml
225		* @return Response
226		*/
227		protected function returnXML(SimpleXMLElement $xml)
228		{
229		return new Response($this->removeXmlFirstLine($xml->asXML()), 200, array('Content-Type' => 'application/xml'));
		0 ignored issues – show Security Bug introduced 2016-10-12 05:26 UTC by Report Bug Copy Issue Report It seems like `$xml->asXML()` targeting `SimpleXMLElement::asXML()` can also be of type `false`; however, `Leo108\CAS\Http\Controll...r::removeXmlFirstLine()` does only seem to accept `string`, did you maybe forget to handle an error condition? Loading history...
230		}
231		}

leo108 / laravel_cas_server

Push — master ( 59d827...fe270b )

ValidateController::v3ValidateAction() A

Complexity

Size

Duplication

Code Coverage

Importance

Duplication Side-by-Side

Filter issues like