Skip to content
This repository has been archived by the owner on Dec 21, 2022. It is now read-only.

Upgrade onelogin/php-saml to v3 to support PHP 7.1+ (WIP) #131

Conversation

maxime-rainville
Copy link
Contributor

@maxime-rainville maxime-rainville commented Aug 14, 2019

I've converted the module to use the latest version of onelogin/php-saml and SS3.7.

However I didn't manage to set up an active directory for testing. So I don't know for a fact that it works and the test coverage is patchy.

Parent issue

Copy link
Contributor

@robbieaverill robbieaverill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good - there were no changes required for our implementation of the library?

@maxime-rainville
Copy link
Contributor Author

I don't actually know without having a proper LDAP set up to test.

Copy link
Contributor

@NightJar NightJar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I expect this will fail. The onelogin library introduced namespaces between v2 and v3.
Fortunately they do not appear to have altered their public API beyond that, and even better the namespaces match the old naming convention of underscores.
e.g. OneLogin_Saml2_Utils becomes OneLogin\Saml2\Utils

See https://github.com/silverstripe/silverstripe-realme/pull/45/files for an example :)

If you wanted to 'cheat' you could probably e.g. use OneLogin\Saml2\Utils as OneLogin_Saml2_Utils; or something :P

madmatt added a commit to madmatt/silverstripe-activedirectory that referenced this pull request Oct 22, 2019
@maxime-rainville
Copy link
Contributor Author

We EOLfing the module.

@maxime-rainville maxime-rainville deleted the pulls/3/make-php7.3-compatible branch December 21, 2022 01:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants