Multiple IDP Support

Created on 15 October 2019, about 5 years ago
Updated 17 October 2023, about 1 year ago
✨ Feature request
Status

Needs work

Version

3.0

Component

Code

Created by

πŸ‡ΊπŸ‡ΈUnited States RecursiveMeta

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡ΊπŸ‡ΈUnited States jesconstantine

    Hello!

    This is a usecase I'll need to satisfy as well, I was curious if anyone had developed this functionality or if anyone had plans to develop this functionality?

    Thanks!

  • πŸ‡³πŸ‡±Netherlands roderik Amsterdam,NL / Budapest,HU

    Hi jesconstantine,

    The original poster has not replied and noone has made actual plans to develop this. I happily accept either contributed code or sponsorship ;-)

    The saml_sp module β†’ is able to handle multiple IdPs. Per 2021, there were some other incompatibilities that make you choose one or the other -- primarily:

    • the 'unique id' for the logging-in user. saml_sp wants this in the NameID attribute of the SAML message / cannot handle it being in a generic SAML attribute value. Also, it may or may not require the NameID to be the user's email (I don't remember).
    • inversely, samlauth wants the unique ID a generic SAML attribute value, and can only handle the NameID with a patch currently ( #3211380-10: [RFC] NameID support β†’ ).

    (Also I have a feeling this module is more future proof, but I'm biased.)

    My current idea about getting this feature into this module is:

    1)

    I'd appreciate anyone laying out their use case, so I can be sure I'm not misunderstanding it / it gets documented properly.

    (Per #3211419-3: Consider merging with the saml_sp module / differences between modules β†’ , the saml_sp maintainer lays out his reasons, which is the only actual use case I've seen written down. My interpretation of this is: he's not _actually_ using multiple IdPs at once -- but having multiple configurations saved as config entities makes it a lot easier to switch / there are situations where some form of config override would be too hard.

    I'm assuming most people are using some form of config override for making things work in dev/test/live environments.)

    2)

    Implementation:

    Almost all configuration in the config screen (except the "Login / Logout" section) should be configurable per IdP. Having config entities indeed seems the best way, to me. So the config form should be 1) split between "Login / Logout" and the rest, 2) almost all configuration should be saved in an entity.
    (There can be discussion about which configuration values could be shared between IdPs, but at this moment I don't believe a good distinction can be made and I think we'll have to just duplicate and reconfigure all the things for every IdP.)

    Then the locations in the code which currently get the 'samlauth.authentication' configuration, must know which config entity to load (by ID) and load configuration from there instead. I believe this should be doable.

    (And if you need the add-on modules for role mapping and attribute mapping... we'd need to have separate config entities for for those too. Again, maybe those can/should be shareable between different IdPs, but I'm not convinced. May depend on point 1.)

    3)

    Since most sites don't need this, I probably would want to not work with config entities by default, and have this functionality in an add-on module. Where e.g. enabling the module reads the existing 'global' config and creates a config entity from it, and there's some documentation on what to do when uninstalling it.

    But that's for later. A patch/hacked module that would do 2, would be a good step toward getting this functionality included in a stable version eventually.

  • πŸ‡¨πŸ‡¦Canada dylan donkersgoed London, Ontario

    Dylan Donkersgoed β†’ made their first commit to this issue’s fork.

  • Open on Drupal.org β†’
    Core: 9.5.5 + Environment: PHP 7.4 & MySQL 5.7
    last update over 1 year ago
    Not currently mergeable.
  • @dylan-donkersgoed opened merge request.
  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 9.5.5 + Environment: PHP 7.4 & MySQL 5.7
    last update over 1 year ago
    2 fail
  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 9.5.5 + Environment: PHP 7.4 & MySQL 5.7
    last update over 1 year ago
    2 fail
  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 9.5.5 + Environment: PHP 7.4 & MySQL 5.7
    last update over 1 year ago
    2 fail
  • Status changed to Needs work over 1 year ago
  • πŸ‡¨πŸ‡¦Canada dylan donkersgoed London, Ontario

    I've pushed some preliminary code to an MR for this. It still needs some work. Right now this just splits the configuration related to the IdP metadata into a config entity. However, as roderick noted above there are some other attributes that almost certainly should be configurable. E.g. the unique ID, username and e-mail attributes. There are some other attributes arguably could be (e.g. maybe you want to require signed assertions for one IdP but not another?). And the submodules should be integrated with it as well.

    This MR will also need an update hook to move the existing IdP config to the config entities.

    Also, right now this MR doesn't expose any mechanism to actually select which IdP you're using through the UI besides setting a default one nor does it provide, e.g., separate login links for separate IdPs. So it's kind of useless unless you're planning to supplement it with overrides outside the module (which I am). So for some possible enhancements:

    • The module could provide login links per-IdP
    • The module could provide some mechanism for configuring when to use which IdP, e.g. selecting based on domain.

    I'd appreciate anyone laying out their use case, so I can be sure I'm not misunderstanding it / it gets documented properly.

    (Per #3211419-3: Consider merging with the saml_sp module / differences between modules, the saml_sp maintainer lays out his reasons, which is the only actual use case I've seen written down. My interpretation of this is: he's not _actually_ using multiple IdPs at once -- but having multiple configurations saved as config entities makes it a lot easier to switch / there are situations where some form of config override would be too hard.

    I'm assuming most people are using some form of config override for making things work in dev/test/live environments.)

    I can think of two other use cases besides per-environment configuration.

    The one I'm dealing with right now is having different configuration for different site domains. I have a site with two different domains, one English and one French, so they have a different ACS URL etc. which means multiple SPs. The way the IdP owner's system works there's only one IdP per SP so that means multiple IdPs as well. I can kind of work around this with settings.php overrides/custom code, but it would be much more convenient if it was in the module.

    Another case (and one that's harder to work around) would be allowing logins from multiple different IdPs. I haven't worked on any sites that have done this with SAML, but I have personally worked on sites that have done it with OpenID and seen sites that do it with SAML in the wild.

  • πŸ‡¨πŸ‡¦Canada pierre paul lefebvre

    Hey @dylan-donkersgoed, nice work!
    I'm interested in working on custom login link and maybe unique name id per idp.
    Can you provide a bit more details on how you override the idp to be used?

    So it's kind of useless unless you're planning to supplement it with overrides outside the module (which I am).

    The only way I've found, by reading your code, is to override the config variable, default_idp, but if I do that on a high traffic site, the ACS reply might be done on a different IDP than it was first requested. Unless you're overriding per site, which have different domain name?

    Thanks!

  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 9.5.5 + Environment: PHP 7.4 & MySQL 5.7
    last update over 1 year ago
    2 fail
  • @pierre-paul-lefebvre opened merge request.
  • πŸ‡¨πŸ‡¦Canada pierre paul lefebvre

    My merge request is based on Dylan's work. I'm adding a custom event that can override the IDP to be used.
    For my use case, I have a route that sets the IDP in the session, when the event is fired, the value is read from the session and returns the ID of the idp to use.

    It's working fine for my use case, but I just realized that I need to send a different SP id to my IDP. Im not sure if this should be considered a normal use case though. Right now the SP id (and certificates) are general configuration.

  • πŸ‡¨πŸ‡¦Canada pierre paul lefebvre

    Im currently at Drupalcon Lille and I'm planning on moving this task forward as much as possible. If anyone wants to join, I'll be at the contrib room everyday, whenever I have downtime :)

Production build 0.71.5 2024