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.

Sign up for GitHub

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

Jump to bottom

ADFS IdP loads metadata of disabled SAML IdP #1564

Closed
m0ark opened this issue Jan 19, 2022 · 3 comments · Fixed by #1565 · May be fixed by #1553
Closed

ADFS IdP loads metadata of disabled SAML IdP #1564

m0ark opened this issue Jan 19, 2022 · 3 comments · Fixed by #1565 · May be fixed by #1553

Comments

@m0ark
Copy link
Contributor

m0ark commented Jan 19, 2022

Describe the bug
ADFS IdPs load available SAML IdP metadata even though they are not enabled .
In combination with the install instructions provided in the documentation, which suggests copying the whole metadata-templates/ content to metadata/, probably erroneous configuration (in my case certificates not being available as saml20-idp-hosted.php expects) is loaded.
This will only interfere when using the created associationGroup, especially during SLO requests.

simplesamlphp/lib/SimpleSAML/IdP.php

Lines 94 to 100 in 43c1caf

try {
// this makes the ADFS IdP use the same SP associations as the SAML 2.0 IdP
$saml2EntityId = $metadata->getMetaDataCurrentEntityID('saml20-idp-hosted');
$this->associationGroup = 'saml2:' . $saml2EntityId;
} catch (\Exception $e) {
// probably no SAML 2 IdP configured for this host. Ignore the error
}

To Reproduce
Steps to reproduce the behavior:

  1. Setup SimpleSAMLphp as suggested in https://simplesamlphp.org/docs/stable/simplesamlphp-install (especially copying each and every metadata template to active configuration)
  2. In config/config.php:
'enable.saml20-idp' => false,
'enable.shib13-idp' => false,
'enable.adfs-idp' => true,
  1. Configure adfs-idp-hosted.php (e.g. with customized certificate location)
  2. Sign in using an ADFS RP
  3. Logout leads to an error (unconfigured certificate path of disabled saml20-idp-hosted.php not found)

Expected behavior
SAML IdP metadata should only be loaded if it has been enabled at all ('enable.saml20-idp' => true, ).
@m0ark m0ark mentioned this issue Jan 19, 2022
Open
@thijskh
Copy link
Member

thijskh commented Jan 19, 2022

Maybe I'm not understanding it right, but simply adding an if for the setting around the code you quote would do it?

@m0ark
Copy link
Contributor Author

m0ark commented Jan 19, 2022

It should. I did not test it yet.

I will check later and create a PR.

@m0ark m0ark mentioned this issue Jan 19, 2022
Merged
@tvdijen tvdijen linked a pull request Jan 25, 2022 that will close this issue
Support for IdP initiated proxy SLO #1553
Open
@github-actions
Copy link
Contributor

\n This issue has been automatically locked since there has \n not been any recent activity after it was closed.\n Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@thijskh @m0ark