Skip to content

SAML Auth Provider - #1576

Merged
ssddanbrown merged 4 commits into
BookStackApp:masterfrom
Xiphoseer:feature/saml
Nov 17, 2019
Merged

ssddanbrown merged 4 commits into
BookStackApp:masterfrom
Xiphoseer:feature/saml

Conversation

@Xiphoseer

Copy link
Copy Markdown
Contributor

This PR should be the base of adding SAML as an additional authentication provider for bookstack accounts. Currently with some minimal configuration in .env as specified below, the use should be redirected to the configured identity provide and redirected back to the application. The actual handling of the logged in event remains to be done, as the data sent to the IDP is still somehow broken. Hence the WIP for this PR.

SAML2_ENABLED=true
SAML2_IDP_SSO=https://example.com/idp/SSORedirectLogin
SAML2_IDP_x509=BASE64IDPCERT==
SAML2_IDP_ENTITYID=https://example.com/idp/metadata.xml

@Xiphoseer

Xiphoseer commented Aug 6, 2019

Copy link
Copy Markdown
Contributor Author

Ok, the login should now work, including automatic registration. The SAML provider is implemented in a very similar way to a Socialite plugin. It may be possible to refactor this into an actual socialite plugin. When I checked last week, such a plugin was not available. The configuration now allows for variables like

SAML_USER_NAME_ATTRIBUTE=username
SAML_DISPLAY_NAME_ATTRIBUTE=firstName|lastName
SAML_AUTO_REGISTER=true

I'll address the codeclimate issues in some upcoming commit.

@Xiphoseer Xiphoseer mentioned this pull request Aug 7, 2019
@Xiphoseer Xiphoseer changed the title WIP: Initial work on SAML integration SAML Auth Provider Aug 7, 2019
@Xiphoseer

Copy link
Copy Markdown
Contributor Author

Synchronizing groups now works the same as with the LDAP service. There is now an ExternalAuthService in BookStack\Auth\Access, that contains the common functionality. Ideally, someone would turn this into a proper socialite provider, but that needs to be maintained.

@ssddanbrown

Copy link
Copy Markdown
Member

Thank you very much @Xiphoseer for your work here. Code looks great from a quick scan but I'll have a deeper review at some point soon. I'll probably focus on getting v0.27 done before then coming back to this, Hopefully so this is part of v0.28.

Don't worry too much about codeclimate issues btw, I'm always creating new codeclimate issues 😅

@ssddanbrown ssddanbrown added this to the v0.28.0 milestone Sep 7, 2019
@skoltheus

Copy link
Copy Markdown

Is there an updated timeline for v0.28.0?
Definitely interested in some of the new features.

@ssddanbrown

Copy link
Copy Markdown
Member

@antimatteraccord Nope, not really. It's in progress and will be done when it's ready.

@ssddanbrown ssddanbrown mentioned this pull request Nov 16, 2019
6 tasks
@ssddanbrown

Copy link
Copy Markdown
Member

Continued in #1787

@ssddanbrown
ssddanbrown merged commit 8e723f1 into BookStackApp:master Nov 17, 2019
@ssddanbrown

Copy link
Copy Markdown
Member

As with #1787, This has now been merged ready for the next feature release 🎉

Again, Thank you very much @Xiphoseer.

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.

3 participants