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

Disable SingleLogoutService for SP #68

Closed
JoeyHoutenbos opened this issue Jan 18, 2017 · 4 comments
Closed

Disable SingleLogoutService for SP #68

JoeyHoutenbos opened this issue Jan 18, 2017 · 4 comments

Comments

@JoeyHoutenbos
Copy link
Contributor

Hi,

I don't want to expose a single-logout service, so I expected to do something like this in the "config/saml2_settings.php":

'sp' => array(
    ...
    'singleLogoutService' => false,
),

But this still sets up the service in the Saml2ServiceProvider@register. When I remove/comment out this part it is working:

if (empty($config['sp']['singleLogoutService']['url'])) {
    $config['sp']['singleLogoutService']['url'] = URL::route('saml_sls');
}

Why isn't this supported? Maybe it's better to check if the singleLogoutService even exists and if it does if the value isn't equal to false before inserting it.

@aacotroneo what do you think about it?

@aacotroneo
Copy link
Owner

yep, that could be a good improvement. It's not supported because no one needed/cared about it, but it does make sense. Something like this to remain the most backwards compatible we can?

 if (!empty($config['sp']['singleLogoutService']) && 
     empty($config['sp']['singleLogoutService']['url']){
            $config['sp']['singleLogoutService']['url'] = URL::route('saml_sls');
 }

@JoeyHoutenbos
Copy link
Contributor Author

I agree with you that would be the most backwards compatible solution.
This means in the settings the whole singleLogoutService part can be removed and it won't be added to the metadata.

Is this something you can include soon?

@aacotroneo
Copy link
Owner

aacotroneo commented Jan 18, 2017 via email

@JoeyHoutenbos
Copy link
Contributor Author

I made a PR #69. Please check it out and include in next release.

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

2 participants