Does the SAML reply URL need to include the registration ID?

structurizr / onpremises

Structurizr on-premises installation

https://docs.structurizr.com/onpremises

MIT License

140 stars 54 forks source link

Does the SAML reply URL need to include the registration ID? #104

Closed andrey-dubnik closed 7 months ago

andrey-dubnik commented 7 months ago

Description

The reply URL changed in the latest versions From '/login/saml2/sso' To '/login/saml2/sso/structurizr'

This is either a bug or some doc update may be required

Steps to reproduce

Configure SAML

Screenshot

No response

Code sample

No response

Configuration

No response

Severity

Minor

Priority

I have no budget and there's no rush, please fix this for free

More information

No response

andrey-dubnik commented 7 months ago

latest version which have SSO reply in line with documentation is 3263

simonbrowndotje commented 7 months ago

Correct, the SAML configuration changed in 2024.01.02 - see release notes, and https://docs.structurizr.com/onpremises/authentication/saml has the new configuration instructions. Feel free to open a docs PR if you feel more is required.

andrey-dubnik commented 7 months ago

Thanks @simonbrowndotje, current version of the documentation states

Register the Structurizr on-premises application with your Identity Provider. When doing this, you will need a “Reply URL”, which is of the form {structurizr.url}/login/saml2/sso

Where it actually expects {structurizr.url}/login/saml2/sso/structurizr now, at least once I have updated a version I have started to get the error message about the new reply URL from my IDP (AzureAD).

Hence I thought it is a bug which introduced extra to the SSO reply or a doc update. I'm happy to do a PR if you confirm the doc needs a change from /sso to /sso/structurizr

andrey-dubnik commented 7 months ago

Doc update would include following

Register the Structurizr on-premises application with your Identity Provider. When doing this, you will need a “Reply URL”, which is of the form {structurizr.url}/login/saml2/sso/{registrationId}

simonbrowndotje commented 7 months ago

So just to clarify:

Build 3263 uses the old Spring SAML library, which required a reply URL of the form {structurizr.url}/saml/SSO.
Build 2024.01.02 uses the new Spring SAML library, which requires a reply URL of the form {structurizr.url}/login/saml2/sso, but you're saying that this may be {structurizr.url}/login/saml2/sso/{registrationId} instead.

You may be correct, but this doesn't seem to be the case with any of the test apps I have on various identity providers; for example (this is Azure AD):

If I set the reply URL to {structurizr.url}/login/saml2/sso/example, then the structurizr.saml.registrationId property does need to be set to example in the structurizr.properties file, otherwise I do see an Azure AD error. But setting the reply URL to {structurizr.url}/login/saml2/sso seems to work for me irrespective of what the registration ID is set to. I think this requires more investigation before updating the docs.

andrey-dubnik commented 7 months ago

Found the misconfiguration on my end (need to use glasses more often) - I had /saml/SSO setup on my SSO application which now needs a change to a different URL in line with the new requirements.

Although the change may have a positive impact in theory my IDP was suggesting that I need a ReplyURL set as sso/structurizr which makes sense as accordingly to the Spring documentation the default endpoint is /login/saml2/sso/{registrationId} but it can be changed.

I guess either /sso or /sso/{registrationId} may be valid reply URLs but I'm yet to test that and it may take few days to come through the change requests.