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

When receiving new entities, EB should check attribute manipulations for parse/syntax errors #1778

Open
baszoetekouw opened this issue Dec 13, 2024 · 2 comments

Comments

@baszoetekouw
Copy link
Member

Entities that are pushed to /api/connection may contain an attribute manipulation (AMs). This is configurable PHP code that is eval()ed in the login flow for the specified entity.

This PHP code is typically typed by hand in a plain text web field in Manage, without syntax checking or anything, so AMs regularly contain typos or syntax errors. In the current setup, these are not detected or discovered until someone tries to log in, the code gets eval()ed en an exception is thrown.

To preempt such errors, we would like to introduce a basic syntax check when new entities are received and parsed in /api/connections. There, for all entities that contain an AM, implement a simple check like this:

try {
    @eval($am);
} catch (ParseError $e) {
    echo "Dat ging dus mis. Want: " . $e->getMessage();
    exit(1);
}

(as suggested by @thijskh in OpenConext/OpenConext-manage#136).

If this fails, simply return a 400 with an explanation of which entity failed to validate, and do no import any of the entities.

@baszoetekouw baszoetekouw changed the title WHen receiving new entities, EB should check attribute manipulations for parse/syntax errors When receiving new entities, EB should check attribute manipulations for parse/syntax errors Dec 13, 2024
@baszoetekouw baszoetekouw moved this from New to Backlog in PHP development Jan 22, 2025
@baszoetekouw
Copy link
Member Author

Example of an attribute manipulation:

// Provide the next mapping
// Non standard SAML attribute 'cytricSystemValue' with the CustomerID

# attributes to let through (ARP)
$requiredAttributes = array(
  'urn:mace:dir:attribute-def:uid',
  'urn:mace:terena.org:attribute-def:schacHomeOrganization',
  'cytricSystemValue'
) ;

# set nameid based on UID
if (isset($attributes) and ($attributes !== FALSE)) {
  if (!empty($attributes['urn:mace:dir:attribute-def:uid'][0])) {
    $subjectId = $attributes['urn:mace:dir:attribute-def:uid'][0] ;
  }
};

# Set cytricSystemValue to ApplicationID conditional to schacHomeOrganization
if (isset($attributes) and ($attributes !== FALSE))
{
    if (!empty($attributes['urn:mace:terena.org:attribute-def:schacHomeOrganization'][0]))
    {
        switch ($attributes['urn:mace:terena.org:attribute-def:schacHomeOrganization'][0]) {
        // HU
        case 'cataherijecollege.nl':
            $attributes['cytricSystemValue'] = array("ama-vck-Catharijne-Utrecht");
            break;
        // UFraneker
        case 'uni-franeker.nl':
            $attributes['cytricSystemValue'] = array("ama-vck-UNIVERSITEITFRANEKER_hen");
            break;
        // UniHarderwijk
        case 'uni-Harderwijk.nl':
            $attributes['cytricSystemValue'] = array("ama-vck-harderwijk");
            if (!empty($attributes['urn:mace:dir:attribute-def:uid'][0]) and !empty($attributes['urn:mace:terena.org:attribute-def:schacHomeOrganization'][0])) {
                $subjectId = $attributes['urn:mace:dir:attribute-def:uid'][0] . "@" . $attributes['urn:mace:terena.org:attribute-def:schacHomeOrganization'][0] ;
                }
            break;
        }
    }
}

# Remove all unrequied attributes
foreach ($attributes as $k => $v) {
  if (!in_array($k, $requiredAttributes)) {
    unset($attributes[$k]);
  }
}

@johanib
Copy link
Contributor

johanib commented Jan 29, 2025

Discussed: Step 1: Determine how, estimate time.
Can we use eval? If so, it should be straightforward. Otherwise, use phpstan for example?

Does the development manage have php code?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Backlog
Development

No branches or pull requests

2 participants