Custom URL Help - ref #347

[bjoern-js]

The feature from two weeks ago is very nice, however, this does not entirely solve the dynamic URL implementation.

https://github.com/grafana/django-saml2-auth/pull/348/files

The code in question:

def get_custom_acs_url() -> Optional[str]:
    get_custom_acs_url_hook = dictor(settings.SAML2_AUTH, "TRIGGER.GET_CUSTOM_ASSERTION_URL")
    return run_hook(get_custom_acs_url_hook) if get_custom_acs_url_hook else None

The hook never gets the request object passed. In fact, no args or kwargs are passed.

How would one access the mentioned "tenant-1" or "tenant-2" URL parameter?

[mostafa]

@henryh9n is a much better person to answer this, yet I suppose you can have them as path parameter, as suggested in the original issue:

For example, let's say the base ACS URL is https://example.com/api/acs. Since we support multiple tenants, https://example.com/api/tenant-1/acs and https://example.com/api/tenant-2/acs are also valid ACS URLs, however Django will only find the base one (due to the way we set it up).

[henryh9n]

@bjoern-js The reason I made this is that previously the ASC URL was computed as follows:

        acs_url = domain + get_reverse([acs, "acs", "django_saml2_auth:acs"])  # type: ignore

this's not always correct, in some cases (local testing, dynamic routing, etc.) the domain might be different or get_reverse function might not work properly.

I agree that it would be even more useful to pass the request/assertion to this function.

We use the trigger in get_saml_client function, which has the SAML assertion as an optional parameter (as a string). I'm guessing it would be more useful to pass the Django response here tho? Or maybe both? Let me know what you think!

In either case, feel free to make a PR to add it, or I can have a look at this the next week.

[bjoern-js]

Thank you @henryh9n.

We are working through it. Even though we have quite a good understanding of the entire flow, we are still thinking of the best way.

For real multi tenancy, as we require it, we would also need to make another change.

We will work through the entire flow and then create a PR soon. Interesting to see the feedback.

Effectively, we decided to pass a "tenant id" to the signin request and pass it forward to the get_custom_acs function, that way people can construct the ACS return URL anyway they want.

There must be a hook for the entity id. To make it dynamic as well, given that a different clients/users have different Azure etc accounts.

Our setup requires that a customer can set their own SSO configuration on their account. We follow the approach that Bitwarden shows their users basically needing the customer to supply some key configurations, while providing them with some dynamic urls, i.e https://bitwarden.com/help/saml-microsoft-entra-id/

[mostafa]

@bjoern-js Just note that new features and changes must not break backward compatibility. 🙏

[bjoern-js]

main...j0x539:django-saml2-auth:django-saml2-auth/issue351

[mostafa]

@bjoern-js Had a quick look and it looks good to me in general. Please send over a PR for review.