Skip to content

Make RequestedAuthnContext configurable in SAML2 - #2171

Closed
Ant1x wants to merge 2 commits into
BookStackApp:masterfrom
Ant1x:saml2-authn-context
Closed

Ant1x wants to merge 2 commits into
BookStackApp:masterfrom
Ant1x:saml2-authn-context

Conversation

@Ant1x

@Ant1x Ant1x commented Jul 6, 2020

Copy link
Copy Markdown

When using SAML IDP providers that have different authentication methods available, authentication will occasionally fail since by default the OneLogin SAML library sets requestedAuthnContext to true.

This has the effect of requiring the exact urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport authentication context from the IDP (Refer to README: https://github.com/onelogin/php-saml)

I've been getting SAML to work with Microsoft ADFS that is configured with a range of different authentication mechanisms, and in this configuration ADFS is unable to authenticate a request that asks for PasswordProtectedTransport

In this PR I have made this option configurable by setting SAML2_REQUESTED_AUTHN_CONTEXT in the env config file. I have made it default to true as to not change the current working configuration for already deployed Bookstack instances.

Setting SAML2_REQUESTED_AUTHN_CONTEXT to false will send an authentication request to the IDP without specifying any authentication context, allowing the IDP to select the most appropriate method to authenticate the end user.

Setting SAML2_REQUESTED_AUTHN_CONTEXT to true or not configuring it will keep the same behavior of requesting PasswordProtectedTransport.

(Note: The online GitHub editor has automatically added a newline to the .env.example.complete file)

@ssddanbrown

Copy link
Copy Markdown
Member

Thank you very much for offering this PR @Ant1x and apologies for my very late reply. Also thank you for the clear detailed description for the options involved and what this was intended to cover.

I've just merged in earlier PR #1998 which covers the same case so I'll therefore close this off. The changes will be part of the next patch release.

@ssddanbrown ssddanbrown closed this May 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants