From 5aa7c18756633d6ec421731f814676679ddd6db9 Mon Sep 17 00:00:00 2001
From: Patrick Radtke <patrick@cirrusidentity.com>
Date: Wed, 20 Jan 2021 09:58:28 -0800
Subject: [PATCH] Ignore invalid hook names
Sometimes we patch or edit a hook to meet our specific needs.
Somtimes this leaves behind a file like FILENAME.orig. The hook
detection login still runs these other files, making for confusing
debugging issues. Use a stricter match on what makes a valid hook name
to avoid these issues.
---
lib/SimpleSAML/Module.php | 2 +-
tests/lib/SimpleSAML/ModuleTest.php | 22 +++++++++++++++++++
tests/modules/unittest/README.md | 1 +
.../unittest/hooks/hook_invalid.php.orig | 3 +++
tests/modules/unittest/hooks/hook_valid.php | 14 ++++++++++++
.../unittest/hooks/invalid_hook_prefix.php | 3 +++
6 files changed, 44 insertions(+), 1 deletion(-)
create mode 100644 tests/modules/unittest/README.md
create mode 100644 tests/modules/unittest/hooks/hook_invalid.php.orig
create mode 100644 tests/modules/unittest/hooks/hook_valid.php
create mode 100644 tests/modules/unittest/hooks/invalid_hook_prefix.php
diff --git a/lib/SimpleSAML/Module.php b/lib/SimpleSAML/Module.php
index ef6222319..425eb13b9 100644
--- a/lib/SimpleSAML/Module.php
+++ b/lib/SimpleSAML/Module.php
@@ -496,7 +496,7 @@ class Module
continue;
}
- if (!preg_match('/hook_(\w+)\.php/', $file, $matches)) {
+ if (!preg_match('/^hook_(\w+)\.php$/', $file, $matches)) {
continue;
}
$hook_name = $matches[1];
diff --git a/tests/lib/SimpleSAML/ModuleTest.php b/tests/lib/SimpleSAML/ModuleTest.php
index 0a4b69941..543e30709 100644
--- a/tests/lib/SimpleSAML/ModuleTest.php
+++ b/tests/lib/SimpleSAML/ModuleTest.php
@@ -112,4 +112,26 @@ class ModuleTest extends TestCase
'\SimpleSAML\Auth\ProcessingFilter'
));
}
+
+ /**
+ * Test for SimpleSAML\Module::getModuleHooks(). It covers happy path.
+ */
+ public function testGetModuleHooks(): void
+ {
+ $hooks = Module::getModuleHooks('saml');
+ $this->assertArrayHasKey('metadata_hosted', $hooks);
+ $this->assertEquals('saml_hook_metadata_hosted', $hooks['metadata_hosted']['func']);
+ $expectedFile = dirname(__DIR__, 3) . '/modules/saml/hooks/hook_metadata_hosted.php';
+ $this->assertEquals($expectedFile, $hooks['metadata_hosted']['file']);
+ }
+
+ /**
+ * Test for SimpleSAML\Module::getModuleHooks(). It covers invalid hook names
+ */
+ public function testGetModuleHooksIgnoresInvalidHooks(): void
+ {
+ $hooks = Module::getModuleHooks('../tests/modules/unittest');
+ $this->assertArrayHasKey('valid', $hooks, 'hooks=' . var_export($hooks, true));
+ $this->assertCount(1, $hooks, "Invalid hooks should be ignored");
+ }
}
diff --git a/tests/modules/unittest/README.md b/tests/modules/unittest/README.md
new file mode 100644
index 000000000..52b5ff5a2
--- /dev/null
+++ b/tests/modules/unittest/README.md
@@ -0,0 +1 @@
+For some unit tests we need a module that has a specific setup or issues.
diff --git a/tests/modules/unittest/hooks/hook_invalid.php.orig b/tests/modules/unittest/hooks/hook_invalid.php.orig
new file mode 100644
index 000000000..d09ba0cd5
--- /dev/null
+++ b/tests/modules/unittest/hooks/hook_invalid.php.orig
@@ -0,0 +1,3 @@
+<?php
+
+// Hook detection should not load this file since it does not conform to the hook naming scheme
diff --git a/tests/modules/unittest/hooks/hook_valid.php b/tests/modules/unittest/hooks/hook_valid.php
new file mode 100644
index 000000000..b45dcd25e
--- /dev/null
+++ b/tests/modules/unittest/hooks/hook_valid.php
@@ -0,0 +1,14 @@
+<?php
+
+use SimpleSAML\Assert\Assert;
+use SimpleSAML\Auth;
+
+/**
+ * Hook to for use with unit tests
+ *
+ * @param array &$data Some data
+ */
+function unittest_hook_valid(array &$data)
+{
+ $data['summary'][] = 'success';
+}
diff --git a/tests/modules/unittest/hooks/invalid_hook_prefix.php b/tests/modules/unittest/hooks/invalid_hook_prefix.php
new file mode 100644
index 000000000..d09ba0cd5
--- /dev/null
+++ b/tests/modules/unittest/hooks/invalid_hook_prefix.php
@@ -0,0 +1,3 @@
+<?php
+
+// Hook detection should not load this file since it does not conform to the hook naming scheme
--
GitLab