Add keycloak support for multple instances from openid_connect 3.x

Created on 28 September 2023, 9 months ago
Updated 24 May 2024, about 1 month ago

Problem/Motivation

openid_connect has added suport for multiple clients of every provider. This means that there can be multiple keycloak configurations on a single site. Since the keycloak module provides the keycloak_sso functionality that alters the entire sites behavious it doesn't make sense to provide that setting on each keycloak client configuration.

Steps to reproduce

Install the keycloak 2.x-dev and the openid_connect 3.x modules on a site. Then go to the openid_connect list page and notice that it is possible to create multiple clients of the keycloak type.

Proposed resolution

I propose that we remove the hard coded client konfiguration static that determines that the machine name of the keycloak client must be keycloak and also that we lift out the keycloak_sso setting to a single keycloak settings page. On this page it should also be possible to select which of the keycloak clients on the site that should be used for the /user/login page.

πŸ“Œ Task
Status

Needs review

Version

2.2

Component

Code

Created by

πŸ‡ΈπŸ‡ͺSweden auth

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

Comments & Activities

  • Issue created by @auth
  • @auth opened merge request.
  • Status changed to Needs review 9 months ago
  • πŸ‡ΈπŸ‡ͺSweden auth

    The functionality mentioned in the proposed resolution is implemented in the linked issue fork

  • πŸ‡©πŸ‡ͺGermany J-Lee πŸ‡©πŸ‡ͺπŸ‡ͺπŸ‡Ί

    I noticed the same problem when I tried a Keykloak implementation. The MR looks good so far in a quick review.

  • πŸ‡ΈπŸ‡ͺSweden auth

    Would it be possible for one of the maintainers to review this issue and see if this can be accepted?

  • Status changed to Needs work 8 months ago
  • πŸ‡§πŸ‡ͺBelgium BramDriesen Belgium πŸ‡§πŸ‡ͺ

    Didn't test it out yet. Quickly went through the code and added a few remarks

  • Status changed to Needs review 8 months ago
  • πŸ‡ΈπŸ‡ͺSweden auth

    Thank you for the feedback.

    I have provided a updatedb hook for the schema change and fixed the issue with settings.keycloak_i18n_mapping that should be settings.keycloak_i18n.mapping.

    I left the initial config as it was since I belive that to be a reasonable default value.

  • πŸ‡ΈπŸ‡ͺSweden auth

    If a maintainer have the chance to look at this I will try to be quick in my response time to move this patch forward towards acceptance.

  • πŸ‡ΊπŸ‡¦Ukraine _tarik_ Lutsk

    Here diff from the MR 32 for those who won't have a list from commits in composer.json

    Also, thanks @auth for his contribution it works like a charm for me.

Production build 0.69.0 2024