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

share the use of static functions in Mfa authproc #213

Closed
wants to merge 1 commit 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
8 changes: 4 additions & 4 deletions modules/mfa/lib/Auth/Process/Mfa.php
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ public static function getTemplateFor($mfaType)
* @param array $state
* @return string
*/
protected static function getRelayStateUrl($state)
public static function getRelayStateUrl(array $state): string
{
if (array_key_exists('saml:RelayState', $state)) {
$samlRelayState = $state['saml:RelayState'];
Expand Down Expand Up @@ -406,12 +406,12 @@ protected function initComposerAutoloader()
}
}

protected static function isHeadedToMfaSetupUrl($state, $mfaSetupUrl)
public static function isHeadedToUrl(array $state, string $url): bool
{
if (array_key_exists('saml:RelayState', $state)) {
$currentDestination = self::getRelayStateUrl($state);
if (! empty($currentDestination)) {
return (strpos($currentDestination, $mfaSetupUrl) === 0);
return (strpos($currentDestination, $url) === 0);
}
}
return false;
Expand Down Expand Up @@ -578,7 +578,7 @@ public function process(&$state)
// Get the necessary info from the state data.
$employeeId = $this->getAttribute($this->employeeIdAttr, $state);
$mfa = $this->getAttributeAllValues('mfa', $state);
$isHeadedToMfaSetupUrl = self::isHeadedToMfaSetupUrl(
$isHeadedToMfaSetupUrl = self::isHeadedToUrl(
$state,
$this->mfaSetupUrl
);
Expand Down
66 changes: 5 additions & 61 deletions modules/profilereview/lib/Auth/Process/ProfileReview.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use SimpleSAML\Auth\ProcessingFilter;
use SimpleSAML\Auth\State;
use SimpleSAML\Module;
use SimpleSAML\Module\mfa\Auth\Process\Mfa;
use SimpleSAML\Module\profilereview\LoggerFactory;
use SimpleSAML\Session;
use SimpleSAML\Utils\HTTP;
Expand Down Expand Up @@ -69,37 +70,15 @@ protected function loadValuesFromConfig($config, $attributes)
{
foreach ($attributes as $attribute) {
$this->$attribute = $config[$attribute] ?? null;
self::validateConfigValue(

Mfa::validateConfigValue(
$attribute,
$this->$attribute,
$this->logger
);
}
}

/**
* Validate the given config value
*
* @param string $attribute The name of the attribute.
* @param mixed $value The value to check.
* @param LoggerInterface $logger The logger.
* @throws \Exception
*/
public static function validateConfigValue($attribute, $value, $logger)
{
if (empty($value) || !is_string($value)) {
$exception = new \Exception(sprintf(
'The value we have for %s (%s) is empty or is not a string',
$attribute,
var_export($value, true)
), 1507146042);

$logger->critical($exception->getMessage());
throw $exception;
}
}

/**
* Get the specified attribute from the given state data.
*
Expand Down Expand Up @@ -141,48 +120,13 @@ protected function getAttributeAllValues($attributeName, $state)
return is_null($attributeData) ? null : (array)$attributeData;
}

/**
* Return the saml:RelayState if it begins with "http" or "https". Otherwise
* return an empty string.
*
* @param array $state
* @returns string
* @return mixed|string
*/
protected static function getRelayStateUrl($state)
{
if (array_key_exists('saml:RelayState', $state)) {
$samlRelayState = $state['saml:RelayState'];

if (strpos($samlRelayState, "http://") === 0) {
return $samlRelayState;
}

if (strpos($samlRelayState, "https://") === 0) {
return $samlRelayState;
}
}
return '';
}

protected function initComposerAutoloader()
{
$path = __DIR__ . '/../../../vendor/autoload.php';
if (file_exists($path)) {
require_once $path;
}
}

protected static function isHeadedToProfileUrl($state, $ProfileUrl)
{
if (array_key_exists('saml:RelayState', $state)) {
$currentDestination = self::getRelayStateUrl($state);
if (! empty($currentDestination)) {
return (strpos($currentDestination, $ProfileUrl) === 0);
}
}
return false;
}

/**
* Redirect the user to set up profile.
Expand All @@ -193,7 +137,7 @@ public static function redirectToProfile(&$state)
{
$profileUrl = $state['ProfileUrl'];
// Tell the profile-setup URL where the user is ultimately trying to go (if known).
$currentDestination = self::getRelayStateUrl($state);
$currentDestination = Mfa::getRelayStateUrl($state);
if (! empty($currentDestination)) {
$profileUrl = HTTP::addURLParameters(
$profileUrl,
Expand Down Expand Up @@ -223,7 +167,7 @@ public function process(&$state)
{
// Get the necessary info from the state data.
$employeeId = $this->getAttribute($this->employeeIdAttr, $state);
$isHeadedToProfileUrl = self::isHeadedToProfileUrl($state, $this->profileUrl);
$isHeadedToProfileUrl = Mfa::isHeadedToUrl($state, $this->profileUrl);

$mfa = $this->getAttributeAllValues('mfa', $state);
$method = $this->getAttributeAllValues('method', $state);
Expand Down