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

Cannot use the library with SimpleSAML #178

Open
mglaman opened this issue Jul 25, 2019 · 11 comments
Open

Cannot use the library with SimpleSAML #178

mglaman opened this issue Jul 25, 2019 · 11 comments

Comments

@mglaman
Copy link

mglaman commented Jul 25, 2019

When trying to generate a SAMLRequest object from an incoming string, the library crashes.

The following is from the Message constructor: https://github.com/simplesamlphp/saml2/blob/master/src/SAML2/Message.php#L143

    protected function __construct(string $tagName, DOMElement $xml = null)
    {
        $this->tagName = $tagName;
        $this->id = Utils::getContainer()->generateId();

The container is part of the compat layer with SimpleSAML. The generateId method uses the Random library from SimpleSAML

use SimpleSAML\Utils\HTTP;
use SimpleSAML\Utils\Random;
use SimpleSAML\Utils\System;
use SimpleSAML\Utils\XML;

    /**
     * {@inheritdoc}
     * @return string
     */
    public function generateId() : string
    {
        /** @psalm-suppress UndefinedClass */
        return Random::generateID();
    }

It's even marked as supressed in Psalm.

@mglaman
Copy link
Author

mglaman commented Jul 25, 2019

Just found #125, which links to

Provide the required external dependencies by extending and implementing the SAML2\Compat\AbstractContainer then injecting it in the ContainerSingleton (see example below).

So we have to define a container ahead of time to replace the Ssp bridge?

@jaimeperez
Copy link
Member

Yes, you can either use SSP directly or provide your own substitutions.

@tvdijen would it be an idea to move some of the basic things that the SAML2 library uses to another repo, shared between this library and SSP itself? I don't know how much is depending on SSP, or how much of that is not depending on configuration, though.

@tvdijen
Copy link
Member

tvdijen commented Jul 25, 2019

Good suggestion, will investigate! I think it's mainly the Logger

@mglaman
Copy link
Author

mglaman commented Jul 25, 2019

@tvdijen I can give a hand, too. We're building an idP for our Drupal instance.

@tvdijen
Copy link
Member

tvdijen commented Jul 25, 2019

That would be great, cause I'm not able to work on this on the real short-term..
Let me know if you need anything

@tvdijen
Copy link
Member

tvdijen commented Jul 25, 2019

It's actually a bit more than I anticipated on:

$ grep -R "use SimpleSAML" *
src/SAML2/Compat/Ssp/Container.php:use SimpleSAML\Utils\HTTP;
src/SAML2/Compat/Ssp/Container.php:use SimpleSAML\Utils\Random;
src/SAML2/Compat/Ssp/Container.php:use SimpleSAML\Utils\System;
src/SAML2/Compat/Ssp/Container.php:use SimpleSAML\Utils\XML;
src/SAML2/Compat/Ssp/Logger.php:use SimpleSAML\Logger as SspLogger;
src/SAML2/Configuration/SimpleSAMLConverter.php:use SimpleSAML\Configuration;
src/SAML2/HTTPArtifact.php:use SimpleSAML\Configuration;
src/SAML2/HTTPArtifact.php:use SimpleSAML\Metadata\MetaDataStorageHandler;
src/SAML2/HTTPArtifact.php:use SimpleSAML\Module\saml\Message as MSG;
src/SAML2/HTTPArtifact.php:use SimpleSAML\Store;
src/SAML2/HTTPArtifact.php:use SimpleSAML\Utils\HTTP;
src/SAML2/SOAPClient.php:use SimpleSAML\Configuration;
src/SAML2/SOAPClient.php:use SimpleSAML\Utils\Config;
src/SAML2/SOAPClient.php:use SimpleSAML\Utils\Crypto;
tests/SAML2/HTTPPostTest.php:use SimpleSAML\Utils\HTTP;

@mglaman
Copy link
Author

mglaman commented Jul 25, 2019

Is this something feasible for the library? LightSAML is not feasible, either. SAML2 is too tightly coupled to SimpleSAML. We need to have a working idP and I would be willing to help on this effort.

@tvdijen
Copy link
Member

tvdijen commented Jul 25, 2019

If you need an IdP you should just use SimpleSAMLphp.. The library is only useful if you want to develop your own IdP, and trust me, you don't want to reinvent the wheel when it comes to SAML.

@mglaman
Copy link
Author

mglaman commented Jul 25, 2019

I'm not a fan of the library, it's hard to integrate with other systems and has its own response handling and assumptions. I was hoping for a more "lean" library to work with. I do not want to reinvent the wheel, I was hoping to contribute to the libraries.

@tvdijen
Copy link
Member

tvdijen commented Jul 25, 2019

Contributions are always welcome! The library was split off from SimpleSAMLphp years ago and may have been a bit neglected ever since.. I've recently put a lot of effort in bringing it back up to today's standards for a future 4.0 release.. If you feel we need to change things, this would be the time!

@jaimeperez
Copy link
Member

@mglaman SimpleSAMLphp makes assumptions either to make your life easier, or to follow common standards and best practices. Of course, there might be cases where something that should be possible to customise is not, and then contributions are always welcome to improve. But in general, you should use SimpleSAMLphp, not this library. You need to know SAML2 in deep to be on the safe side, and i'ts much more complicated than it might appear. Furthermore, there are lots of security-related issues that SimpleSAMLphp is tackling for you, while you would need to have all those in mind and approach them manually if you use this library on its own.

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

No branches or pull requests

3 participants