Skip to content
This repository was archived by the owner on Sep 19, 2022. It is now read-only.

Commit 617153c

Browse files
BaranekDDominik Frantisek Bucik
BaranekD
and
Dominik Frantisek Bucik
authored
fix: Fixed some unchecked potential errors (#204)
* fix: Fixed some unchecked potential errors * refactor: 💡 Refactored pull-request Co-authored-by: Dominik Frantisek Bucik <[email protected]>
1 parent d85bcaf commit 617153c

File tree

4 files changed

+85
-46
lines changed

4 files changed

+85
-46
lines changed

‎lib/Disco.php‎

Lines changed: 60 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -920,54 +920,71 @@ private function fillSpName($t)
920920
$this->adapter = Adapter::getInstance($this->wayfConfiguration->getString(self::INTERFACE, self::RPC));
921921
try {
922922
if (null !== $clientIdWithPrefix) {
923-
$parts = explode(':', $clientIdWithPrefix);
924-
$clientId = end($parts);
923+
$this->fillSpNameForOidc($t, $clientIdWithPrefix);
924+
} else {
925+
$this->fillSpNameForSaml($t);
926+
}
927+
} catch (\Exception $e) {
928+
Logger::debug("Fill SP name - caught exception {$e}");
929+
//OK, we will just display the disco
930+
}
931+
}
925932

926-
$clientIdAttr = $this->wayfConfiguration->getString(self::CLIENT_ID_ATTR, null);
927-
if (null === $clientIdAttr) {
928-
$facility = $this->adapter->getFacilityByClientId($clientId);
929-
} else {
930-
$facility = $this->adapter->getFacilityByClientId($clientId, $clientIdAttr);
931-
}
933+
private function fillSpNameForOidc($t, $clientIdWithPrefix)
934+
{
935+
if (null === $clientIdWithPrefix) {
936+
return;
937+
}
938+
$parts = explode(':', $clientIdWithPrefix);
939+
$clientId = end($parts);
932940

933-
if (null !== $facility) {
934-
$spNameAttrName = $this->wayfConfiguration->getString(
935-
self::SERVICE_NAME_ATTR,
936-
self::SERVICE_NAME_DEFAULT_ATTR_NAME
937-
);
938-
$spNameMap = $this->adapter->getFacilityAttribute($facility, $spNameAttrName);
939-
if (!empty($spNameMap)) {
940-
$this->spName = $t->getTranslation($spNameMap);
941-
}
942-
}
941+
$clientIdAttr = $this->wayfConfiguration->getString(self::CLIENT_ID_ATTR, null);
942+
if (null === $clientIdAttr) {
943+
$facility = $this->adapter->getFacilityByClientId($clientId);
944+
} else {
945+
$facility = $this->adapter->getFacilityByClientId($clientId, $clientIdAttr);
946+
}
947+
948+
if (null !== $facility) {
949+
$spNameAttrName = $this->wayfConfiguration->getString(
950+
self::SERVICE_NAME_ATTR,
951+
self::SERVICE_NAME_DEFAULT_ATTR_NAME
952+
);
953+
$spNameMap = $this->adapter->getFacilityAttribute($facility, $spNameAttrName);
954+
if (!empty($spNameMap)) {
955+
$this->spName = $t->getTranslation($spNameMap);
956+
}
957+
}
958+
}
959+
960+
private function fillSpNameForSaml($t)
961+
{
962+
$this->spName = null;
963+
if (!empty($this->originalsp['entityid'])) {
964+
$entityId = $this->originalsp['entityid'];
965+
$entityIdAttr = $this->wayfConfiguration->getString(self::ENTITY_ID_ATTR, null);
966+
if (null === $entityIdAttr) {
967+
$facility = $this->adapter->getFacilityByEntityId($entityId);
943968
} else {
944-
$entityId = $this->originalsp['entityid'];
945-
$entityIdAttr = $this->wayfConfiguration->getString(self::ENTITY_ID_ATTR, null);
946-
if (null === $entityIdAttr) {
947-
$facility = $this->adapter->getFacilityByEntityId($entityId);
948-
} else {
949-
$facility = $this->adapter->getFacilityByEntityId($entityId, $entityIdAttr);
950-
}
969+
$facility = $this->adapter->getFacilityByEntityId($entityId, $entityIdAttr);
970+
}
951971

952-
if (null !== $facility) {
953-
$spNameAttr = $this->wayfConfiguration->getString(
954-
self::SERVICE_NAME_ATTR,
955-
self::SERVICE_NAME_DEFAULT_ATTR_NAME
956-
);
957-
$spNameMap = $this->adapter->getFacilityAttribute($facility, $spNameAttr);
958-
if (!empty($spNameMap)) {
959-
$this->spName = $t->getTranslation($spNameMap);
960-
}
961-
}
962-
if (empty($entityId)) {
963-
if (!empty($this->originalsp[self::NAME])) {
964-
$this->spName = $t->translate->getTranslation($this->originalsp[self::NAME]);
965-
}
966-
}
972+
if (null === $facility) {
973+
return;
967974
}
968-
} catch (\Exception $e) {
969-
Logger::warning("Fill SP name - caught exception {$e}");
970-
//OK, we will just display the disco
975+
976+
$spNameAttr = $this->wayfConfiguration->getString(
977+
self::SERVICE_NAME_ATTR,
978+
self::SERVICE_NAME_DEFAULT_ATTR_NAME
979+
);
980+
981+
$spNameMap = $this->adapter->getFacilityAttribute($facility, $spNameAttr);
982+
if (!empty($spNameMap)) {
983+
$this->spName = $t->getTranslation($spNameMap);
984+
}
985+
}
986+
if (empty($this->spName) && !empty($this->originalsp[self::NAME])) {
987+
$this->spName = $t->translate->getTranslation($this->originalsp[self::NAME]);
971988
}
972989
}
973990
}

‎lib/Exception.php‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,15 @@ class Exception extends \SimpleSAML\Error\Exception
2626
*/
2727
public function __construct($id, $name, $message)
2828
{
29-
parent::__construct('Perun error: ' . $id . ' - ' . $name . ' - ' . $message);
29+
if (null === $name && null === $message) {
30+
parent::__construct('Perun error: ' . $id);
31+
} elseif (null === $name) {
32+
parent::__construct('Perun error: ' . $id . ' - ' . $message);
33+
} elseif (null === $message) {
34+
parent::__construct('Perun error: ' . $id . ' - ' . $name);
35+
} else {
36+
parent::__construct('Perun error: ' . $id . ' - ' . $name . ' - ' . $message);
37+
}
3038

3139
$this->id = $id;
3240
$this->name = $name;

‎lib/RpcConnector.php‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,10 @@ public function get($manager, $method, $params = [])
8989
);
9090
}
9191
if (isset($result['errorId'])) {
92-
self::error($result['errorId'], $result['name'], $result['message'], $uri, $paramsQuery);
92+
$name = $result['name'] ?? null;
93+
$message = $result['message'] ?? null;
94+
95+
self::error($result['errorId'], $name, $message, $uri, $paramsQuery);
9396
}
9497

9598
return $result;
@@ -140,7 +143,10 @@ public function post($manager, $method, $params = [])
140143
);
141144
}
142145
if (isset($result['errorId'])) {
143-
self::error($result['errorId'], $result['name'], $result['message'], $uri, $paramsJson);
146+
$name = $result['name'] ?? null;
147+
$message = $result['message'] ?? null;
148+
149+
self::error($result['errorId'], $name, $message, $uri, $paramsJson);
144150
}
145151

146152
return $result;

‎www/updateUes.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,14 @@
4242
}
4343

4444
try {
45+
if (empty($attributesFromIdP['sourceIdPEntityID'][0])) {
46+
throw new Exception('perun/www/updateUes.php: Invalid attributes from Idp - sourceIdPEntityID is empty');
47+
}
48+
49+
if (empty($attributesFromIdP['sourceIdPEppn'][0])) {
50+
throw new Exception('perun/www/updateUes.php: Invalid attributes from Idp - sourceIdPEppn is empty');
51+
}
52+
4553
$userExtSource = $adapter->getUserExtSource(
4654
$attributesFromIdP['sourceIdPEntityID'][0],
4755
$attributesFromIdP['sourceIdPEppn'][0]

0 commit comments

Comments
 (0)