Feature/OIDC groups management by dev-cti · Pull Request #1147 · CERT-Polska/mwdb-core

dev-cti · 2026-03-06T09:47:04Z

Your checklist for this pull request

I've read the contributing guideline.
I've tested my changes by building and running the project, and testing changed functionality (if applicable)
I've added automated tests for my change (if applicable, optional)
I've updated documentation to reflect my change (if applicable)

What is the current behaviour?
The user groups potentially returned during the OIDC login are not used

What is the new behaviour?
The user groups are automatically managed from the groups listed in the OIDC Token.
If these groups don't exist yet, there are automatically created.

The OIDC groups can be filtered using regex to be able to target specific groups if needed
And the group names can be modfied between OIDC and MWDB

Two modes are defined to control how the user groups are finally defined.
It allow "mixed" mode where you can manage user groups with OIDC groups and MWDB groups without conflict

The modes are defined as follow:

MIXED:
The user is added to OIDC groups, default groups, private group, specific provider group and previously added MWDB groups
FULL:
The user is added to OIDC groups, default groups, private group, specific provider group and removed from previously added MWDB groups

Test plan
Enable the oidc_groups feature and configure your keycloak to return the groups into the OIDC Token.

Test1
Configuration:
oidc_groups: 1
oidc_groups_mode: MIXED (default)
oidc_groups_match_pattern: (.*) (default)
oidc_groups_replace_pattern: \1 (default)

Login with a user with specific OIDC groups

Check that all the OIDC groups have been created with the same names in MWDB

Check that the user has been added to all these OIDC groups

Check that the user has been added to all default MWDB groups

Check that the user has been added to the specific OIDC provider group

Remove the user from one OIDC group

Login with the user

Check that the user has been remove from the OIDC group specified above

Add the user to a MWDB group

Login with the user

Check that the user has been added to all OIDC groups

Check that the user has been added to all default MWDB groups

Check that the user has been added to the specific OIDC provider group

Check that the user has been added to the new MWDB Group

Test 2
Configuration:
oidc_groups: 1
oidc_groups_mode: FULL (default)
oidc_groups_match_pattern: (.*) (default)
oidc_groups_replace_pattern: \1 (default)

Login with a user with specific OIDC groups

Check that all the OIDC groups have been created with the same names in MWDB

Check that the user has been added to all these OIDC groups

Check that the user has been added to all default MWDB groups

Check that the user has been added to the specific OIDC provider group

Remove the user from one OIDC group

Login with the user

Check that the user has been remove from the OIDC group specified above

Add the user to a MWDB group

Login with the user

Check that the user has been added to all OIDC groups

Check that the user has been added to all default MWDB groups

Check that the user has been added to the specific OIDC provider group

Check that the user has been removed MWDB Group

The regex filtering and mapping feature using the configuration variables oidc_groups_match_pattern and oidc_groups_replace_pattern has also been tested

Closing issues

closes #1029

psrok1 · 2026-03-24T11:48:41Z

Hello! Thanks for contribution, it looks like a very nice improvement to the OpenID-based user management and I'm going to check it out soon.

psrok1

Hello,

In addition to the specific comments, I have a few general notes:

MWDB Core supports multiple OpenID Providers. In the case of our public instance (mwdb.cert.pl), not all providers should be allowed to manage groups. Currently, these settings are global and apply to all providers. Would it be possible to make this configurable per provider?
It would also be helpful to clearly indicate to the MWDB administrator when a group has been created by an OpenID Provider and is therefore managed under the MIXED setting.

If you’re not comfortable working on the frontend part, I would still greatly appreciate implementing the necessary backend changes.

psrok1 · 2026-03-24T17:05:09Z

    workspace = db.Column(db.Boolean, nullable=False, default=True)
    immutable = db.Column(db.Boolean, nullable=False, default=False)

+    # External Group comming from openid provider


Suggested change

# External Group comming from openid provider

# External group created by OpenID Provider

psrok1 · 2026-03-24T18:41:27Z

+        db.ForeignKey("openid_provider.name"),
+        nullable=True,
+        default=None,
+    )


It would be better if FK is set to openid_provider.id because id is the PK (even if name has unique constraint). I see why name was chosen - only name is passed to mwdb.core.oauth.provider.OpenIDProvider object and create_user_groups requires reference to the provider entity. I think it would be better to change the interface of OpenIDProvider object interface instead.

psrok1 · 2026-03-24T18:45:06Z

        """
        return "Registered via OpenID Connect protocol"

+    def create_user_groups(self, group_names: list[str]) -> list["Group"]:


If we need reference to OpenIDProvider model object to provide a proper foreign key to the group, maybe it would be better to pass it as an argument to this function?

I move all the group creations (provider specifc group and user groups) into the oauth resource

psrok1 · 2026-03-24T18:47:17Z

                "is_oidc_registration_enabled": (
                    app_config.mwdb.enable_oidc_registration
                ),
+                "is_oidc_groups_enabled": (app_config.mwdb.enable_oidc_groups),


This change is not required. /api/server is used for passing configuration values that are meaningful for client (Web UI or CLI). Right now client doesn't need that information so I would not add it. The change in scheme itself is also missing.

psrok1 · 2026-03-24T19:06:26Z

+- MIXED: The OIDC users are only added and removed to the MWDB groups linked to the current OIDC provider.
+  It means that a user will keep his local groups whatever the content of his OIDC Token.


If I understand the code correctly - in MIXED mode, group is synchronized with OpenID only when it was created by OpenID because the linking is only in OpenIDProvider.create_user_groups. If that's the case, I think that the word "created" better than "linked" describes what is actually managed by MIXED setting.

That behavior is simple but is also a bit limiting because there is no other way to add user to existing groups other than setting FULL mode. Maybe we should allow adding to the groups not created by OpenID if they're matching the regex filter but without removing and without "linking"?

If we want to change that, I think that removing the linking requirement in this part would be enough, but I'm not 100% sure for now.

for new_user_group in new_user_groups: # Mixed mode # Add only user to openid groups linked to the current provider if ( app_config.mwdb.oidc_groups_mode == OIDCGroupManagementMode.MIXED and new_user_group.openid_provider_name != provider_settings.name ): continue

It's also better to left it unlinked as it is.

I didn't want to allow external users to be automatically added to "sensitive" local groups.
For example if the external group names are directly mapped to the internal names it can be a problem.
If the user is in the external admin group it will be automatically added to the admin group of mwdb.

But I guess that the in most of the cases the external groups will be prefixed by something (provider name?)

dev-cti · 2026-04-07T07:55:10Z

Hello
Thanks for your feedbacks
I’ve had a lot of work lately, but I’m taking your feedback into account quickly.

dev-cti · 2026-04-16T14:54:23Z

Hello,

In addition to the specific comments, I have a few general notes:
* MWDB Core supports multiple OpenID Providers. In the case of our public instance (mwdb.cert.pl), not all providers should be allowed to manage groups. Currently, these settings are global and apply to all providers. Would it be possible to make this configurable per provider?

* It would also be helpful to clearly indicate to the MWDB administrator when a group has been created by an OpenID Provider and is therefore managed under the MIXED setting.
If you’re not comfortable working on the frontend part, I would still greatly appreciate implementing the necessary backend changes.

Yes it is really better
So I have removed the global settings and implemented a per provider configuration.
The OIDC group management parameters can now be configured in the provider view.
There are now 3 modes (None, Full and mixed). By default the management mode is None (disable)
I have also added the provider of origin in the group list view and group detail view

psrok1 self-requested a review March 9, 2026 13:12

psrok1 requested changes Mar 24, 2026

View reviewed changes

dev-cti added 8 commits April 17, 2026 10:04

Automatic management of OIDC groups

3a95c38

Bump version to 2.18.0

663f045

fix: Import order

e990542

fix linter

1bb5370

fix: Remove OIDC Group configuration from server info response

bf59aaa

feat: Automatc management of OIDC groups (Per provider configuration)

442b149

feat Update documentation

1782602

feat Update database migration script following rebase on master

6f172ed

dev-cti force-pushed the feature/oidc_groups_management branch from 3058ba2 to 6f172ed Compare April 17, 2026 08:44

Web sources linting

619e755

	# External Group comming from openid provider
	# External group created by OpenID Provider

		- MIXED: The OIDC users are only added and removed to the MWDB groups linked to the current OIDC provider.
		It means that a user will keep his local groups whatever the content of his OIDC Token.

Conversation

dev-cti commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

psrok1 commented Mar 24, 2026

Uh oh!

psrok1 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dev-cti commented Apr 7, 2026

Uh oh!

dev-cti commented Apr 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dev-cti commented Mar 6, 2026 •

edited

Loading