Skip to content
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

Fix duplicate date-header #25

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
"php": "^8.1",

"simplesamlphp/assert": "^1.0.0",
"simplesamlphp/simplesamlphp": "^2.2.0",
"simplesamlphp/simplesamlphp": "dev-bugfix/discopwer-duplicate-date-header",
"simplesamlphp/composer-module-installer": "^1.3.4",
"symfony/http-foundation": "^6.4.0"
},
Expand Down
8 changes: 4 additions & 4 deletions src/Controller/DiscoPower.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,15 @@
use SimpleSAML\Error;
use SimpleSAML\Module\discopower\PowerIdPDisco;
use SimpleSAML\Session;
use Symfony\Component\HttpFoundation\{JsonResponse, Request, Response, StreamedResponse};
use Symfony\Component\HttpFoundation\{JsonResponse, Request, Response};

class DiscoPower
{
/**
* @param \Symfony\Component\HttpFoundation\Request $request The current request.
* @return \Symfony\Component\HttpFoundation\StreamedResponse
* @return \Symfony\Component\HttpFoundation\Response
*/
public function main(Request $request): StreamedResponse
public function main(Request $request): Response
{
try {
$discoHandler = new PowerIdPDisco(
Expand All @@ -29,7 +29,7 @@
}

try {
return new StreamedResponse([$discoHandler, 'handleRequest']);
return $discoHandler->handleRequest();
} catch (Exception $exception) {
// An error here should be caused by metadata
throw new Error\Error('METADATA', $exception);
Expand Down Expand Up @@ -58,10 +58,10 @@
// handle JSONP requests
if ($request->query->has('callback')) {
$callback = $request->query->get('callback');
if (!preg_match('/^[a-z0-9_]+$/i', $callback)) {

Check failure on line 61 in src/Controller/DiscoPower.php

View workflow job for this annotation

GitHub Actions / Quality control

ArgumentTypeCoercion

src/Controller/DiscoPower.php:61:48: ArgumentTypeCoercion: Argument 2 of preg_match expects string, but parent type null|scalar provided (see https://psalm.dev/193)

Check failure on line 61 in src/Controller/DiscoPower.php

View workflow job for this annotation

GitHub Actions / Quality control

PossiblyNullArgument

src/Controller/DiscoPower.php:61:48: PossiblyNullArgument: Argument 2 of preg_match cannot be null, possibly null value provided (see https://psalm.dev/078)

Check failure on line 61 in src/Controller/DiscoPower.php

View workflow job for this annotation

GitHub Actions / Quality control

ArgumentTypeCoercion

src/Controller/DiscoPower.php:61:48: ArgumentTypeCoercion: Argument 2 of preg_match expects string, but parent type null|scalar provided (see https://psalm.dev/193)

Check failure on line 61 in src/Controller/DiscoPower.php

View workflow job for this annotation

GitHub Actions / Quality control

PossiblyNullArgument

src/Controller/DiscoPower.php:61:48: PossiblyNullArgument: Argument 2 of preg_match cannot be null, possibly null value provided (see https://psalm.dev/078)
throw new Error\Exception('Unsafe JSONP callback function name ' . var_export($callback, true));
}
$response->setCallback($callback);

Check failure on line 64 in src/Controller/DiscoPower.php

View workflow job for this annotation

GitHub Actions / Quality control

ArgumentTypeCoercion

src/Controller/DiscoPower.php:64:36: ArgumentTypeCoercion: Argument 1 of Symfony\Component\HttpFoundation\JsonResponse::setCallback expects null|string, but parent type null|scalar provided (see https://psalm.dev/193)

Check failure on line 64 in src/Controller/DiscoPower.php

View workflow job for this annotation

GitHub Actions / Quality control

ArgumentTypeCoercion

src/Controller/DiscoPower.php:64:36: ArgumentTypeCoercion: Argument 1 of Symfony\Component\HttpFoundation\JsonResponse::setCallback expects null|string, but parent type null|scalar provided (see https://psalm.dev/193)
}

$response->setData(
Expand Down
11 changes: 9 additions & 2 deletions src/PowerIdPDisco.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use SimpleSAML\Utils;
use SimpleSAML\XHTML\IdPDisco;
use SimpleSAML\XHTML\Template;
use Symfony\Component\HttpFoundation\Response;

/**
* This class implements a generic IdP discovery service, for use in various IdP discovery service pages. This should
Expand Down Expand Up @@ -235,7 +236,7 @@
* and because the SP could bypass all of this anyway by specifying a known IdP in scoping.
*/
try {
parse_str(parse_url($_GET['return'], PHP_URL_QUERY), $returnState);

Check failure on line 239 in src/PowerIdPDisco.php

View workflow job for this annotation

GitHub Actions / Quality control

PossiblyInvalidArgument

src/PowerIdPDisco.php:239:41: PossiblyInvalidArgument: Argument 1 of parse_url expects string, but possibly different type non-empty-array<int|non-empty-string, array<int|non-empty-string, mixed>|string>|string provided (see https://psalm.dev/092)

Check failure on line 239 in src/PowerIdPDisco.php

View workflow job for this annotation

GitHub Actions / Quality control

PossiblyInvalidArgument

src/PowerIdPDisco.php:239:41: PossiblyInvalidArgument: Argument 1 of parse_url expects string, but possibly different type non-empty-array<int|non-empty-string, array<int|non-empty-string, mixed>|string>|string provided (see https://psalm.dev/092)
$state = Auth\State::loadState($returnState['AuthID'], 'saml:sp:sso');
if ($state && array_key_exists('SPMetadata', $state)) {
$spmd = $state['SPMetadata'];
Expand Down Expand Up @@ -289,10 +290,16 @@
* Handles a request to this discovery service.
*
* The IdP disco parameters should be set before calling this function.
*
* @return \Symfony\Component\HttpFoundation\Response
*/
public function handleRequest(): void
public function handleRequest(): Response
{
$this->start();
$result = $this->start();
if ($result !== null) {
return $result;
}

// no choice made. Show discovery service page
$idpList = $this->getIdPList();
Expand All @@ -312,7 +319,7 @@
$t->data['idplist'] = $idpList;
$t->data['faventry'] = null;
foreach ($idpList as $tab => $slist) {
if (!empty($preferredIdP) && array_key_exists($preferredIdP, $slist)) {

Check failure on line 322 in src/PowerIdPDisco.php

View workflow job for this annotation

GitHub Actions / Quality control

RiskyTruthyFalsyComparison

src/PowerIdPDisco.php:322:18: RiskyTruthyFalsyComparison: Operand of type null|string contains type string, which can be falsy and truthy. This can cause possibly unexpected behavior. Use strict comparison instead. (see https://psalm.dev/356)

Check failure on line 322 in src/PowerIdPDisco.php

View workflow job for this annotation

GitHub Actions / Quality control

RiskyTruthyFalsyComparison

src/PowerIdPDisco.php:322:18: RiskyTruthyFalsyComparison: Operand of type null|string contains type string, which can be falsy and truthy. This can cause possibly unexpected behavior. Use strict comparison instead. (see https://psalm.dev/356)
$t->data['faventry'] = $slist[$preferredIdP];
break;
}
Expand Down Expand Up @@ -344,7 +351,7 @@
$t->data['tabNames'][$tab] = $translator::noop($translatableTag);
}
}
$t->send();
return $t;
}


Expand Down Expand Up @@ -389,11 +396,11 @@
return [];
}

$ret = (string) $_COOKIE['_saml_idp'];

Check failure on line 399 in src/PowerIdPDisco.php

View workflow job for this annotation

GitHub Actions / Quality control

RedundantCast

src/PowerIdPDisco.php:399:16: RedundantCast: Redundant cast to string (see https://psalm.dev/262)

Check failure on line 399 in src/PowerIdPDisco.php

View workflow job for this annotation

GitHub Actions / Quality control

RedundantCast

src/PowerIdPDisco.php:399:16: RedundantCast: Redundant cast to string (see https://psalm.dev/262)
$ret = explode(' ', $ret);
foreach ($ret as &$idp) {
$idp = base64_decode($idp);
if ($idp === false) {

Check failure on line 403 in src/PowerIdPDisco.php

View workflow job for this annotation

GitHub Actions / Quality control

DocblockTypeContradiction

src/PowerIdPDisco.php:403:17: DocblockTypeContradiction: string does not contain false (see https://psalm.dev/155)

Check failure on line 403 in src/PowerIdPDisco.php

View workflow job for this annotation

GitHub Actions / Quality control

DocblockTypeContradiction

src/PowerIdPDisco.php:403:17: DocblockTypeContradiction: string does not contain false (see https://psalm.dev/155)
// not properly base64 encoded
return [];
}
Expand Down Expand Up @@ -437,7 +444,7 @@
// we are left with a single entityID whose base64 representation is too long to fit in a cookie
break;
}
$newCookie = $tmp[1];

Check failure on line 447 in src/PowerIdPDisco.php

View workflow job for this annotation

GitHub Actions / Quality control

PossiblyUndefinedArrayOffset

src/PowerIdPDisco.php:447:26: PossiblyUndefinedArrayOffset: Possibly undefined array key $tmp[1] on list{0: string, 1?: string} (see https://psalm.dev/167)

Check failure on line 447 in src/PowerIdPDisco.php

View workflow job for this annotation

GitHub Actions / Quality control

PossiblyUndefinedArrayOffset

src/PowerIdPDisco.php:447:26: PossiblyUndefinedArrayOffset: Possibly undefined array key $tmp[1] on list{0: string, 1?: string} (see https://psalm.dev/167)
}

$params = [
Expand Down
Loading