By using an appropriate WHERE clause in the SQL statement,
we can avoid loading the whole dataset in getMetaData(),
which brings an welcome performance improvement with large
Metadata sets.

Otherwise, a theme would not be able to include/embed/extend its own templates.

When opcache.validate_timestamps is disabled, then the new metadata will not be read after a metarefresh.
This can be solved by adding the metadata file to an opcache blacklist, but calling opcache_invalidate()
after writing a file is a nice out-of-the-box solution.

Hopefully, this will enable everybody that is using simplesamlphp to disable opcache.validate_timestamps
without running into problems.

This new interface allows themes to define a class that can be hooked at certain specific points of template initialization/handling, so that they can do stuff like automatically adding variables for all templates, or adding twig extensions. This classes must implement the new TemplateControllerInterface, and be specified in the "theme.controller" configuration option. This way, we avoid the performance hit if we use traditional hooks, and we also avoid hooks from other modules causing trouble.

For now, the interface offers two entry points: setUpTwig(), which allows managing the twig environment after initialization (e.g. to add an extension or define filters); and display(), which offers all the data passed to the template, and allows adding or modifying it.

This makes sense as those should be static values available to every template.

Additionally, add a "templateId" variable that we can use for templates to identify themselves.

Make sure if we are using a theme, its module is added as a valid domain where we can look for translations.

Therefore, it should be accessed using self, not $this.

Instead of one cache, we need to use two: one for the list of modules available, and the other for the details for them. Those caches should be filled independently, so that someone calling getModules() does not trigger the code checking of the modules are enabled or finding their hooks.

It has also an impact in performance, and covers an unlikely scenario. Instead, if you plan to use templates from another module, now you need to call the "addTemplatesFromModule()" method right after creating the template. That way you can register manually what templates you are supposed to use, being much more efficient.

An alternative way to inject data in the templates should be used. This has a terrible impact in performance, and could have undesired side effects.

This allows template users to use their own twig extensions if they want, while also allowing us to remove the "twigInit" hook. Hooks come at a price, and it doesn't make much sense to use them in this case, as they would only be useful if a module wants to add a twig extension even if the code instantiating SimpleSAML_XHTML_Template does not belong to that module. This could lead to unexpected behaviour (i.e. a module adding a hook that creates trouble for the templates defined in another module), so given the lack of use cases supporting the hook and the possible negative consequences implied, it's better to remove it.

The issue here is that every time we need to list the modules or check if they are enabled, we just iterate over the modules directory and subdirectories, which is terribly expensive. Instead of doing so, we build a cache of modules specifying if they are enabled or not. In the end, this is also fixing another issue, given that enabling/disabling a module in the middle of a request being processed could lead to inconsistencies and unexpected behaviour (likely exceptions and horrible crashes). Modules should be checked in the beginning of a request and their state (enabled/disabled) frozen until the request is processed to avoid that, and this is the way to achieve so.

Additionally, we take the chance to check if modules are enabled when we search for them. This reduces the processing time to around a third of the original without this fix.

This allows us to get rid of SHA-1.

The offset is prepended in clear to the token itself, so that we can subtract it from the current time and get the original time slot. However, the time slot, salt and verification data are authenticated by means of the hash function, but not the offset. This means we can take an expired token and make it valid by simply increasing the prepended offset as much as needed to hit the time slot it was generated on. This is an important security issue as the tokens are therefore not bound to the current time at all.

In order to fix it, the offset itself is added to the hash computation, so that a change in the offset produces a new hash that won't match.

s/generateToken/generate/ && s/validateToken/validate/

With the previous implementation, several methods invoked time() themselves. Under certain conditions (basically, when the clock proceeds to the next second between computing the offset and calculating the token value), this could cause a mismatch that could make tokens expire before they are supposed to. Shouldn't be a big issue unless the system is really, really slow, but better safe than sorry.

When unserializing the session fails, the handler should return null instead of false. Additionally, SimpleSAML_Session::load() should make sure that it got an instance of SimpleSAML_Session, to avoid any misbehaving handlers to generate an issue.

This resolves #616.

This also required adding an additional argument to
SimpleSAML\Utils\Crypto::loadPrivateKey to ease in testing. Without
this additional argument, SimpleSAML_Configuration::getBaseDir eventually
gets called to determine the private key location.  This doesn't work
well with vfsstream. This argument shouldn't cause too much trouble, and
seems cohesive enough with the function's purpose.

The reason was the lack of conversion to integer for each character of the strings before applying the XOR operator to them. The operator returns always an empty string when applied to two characters, and applying a binary-wise OR between 0 and an empty string, yields 0. Therefore, $diff is always 0, and the function returns true for every two strings with same length, regardless of their contents.

This reverts commit b1b0d0ef.

This reverts commit c441f9c9.

It looks like mb_substr() doesn’t cope well with NULL as the third parameter in PHP 5.3.