Skip to content

getJWTGroupsChanges does not dedupe group names → duplicate JWT groups entries create phantom duplicate groups #6618

Description

@stratosjl

Summary

When the JWT groups claim contains the same group name more than once, NetBird's JWT group-sync creates a separate group row per occurrence instead of treating the duplicates as one group. The result is multiple distinct group IDs sharing an identical name (e.g. two netbird-admins rows with different IDs), and the affected user is auto-joined to all of them. The duplication is not detected or collapsed, so it persists in the store. The root cause is that getJWTGroupsChanges iterates the claimed group names without first de-duplicating them, so each repeated name is processed as if it were a new, distinct group.

Environment

  • NetBird self-hosted, embedded IdP (Dex) brokering an upstream OIDC connector.
  • JWT group-sync enabled (jwt_groups_enabled=1, group claim = groups).

Repro

  1. Configure the IdP so that two property-mappings/claims both emit the groups claim for the same user (a common accident, e.g. an explicit "groups" mapping plus a default profile mapping that also emits the user's groups). The user's resulting token then carries the group names duplicated, e.g.
    "groups": ["Admins", "netbird-admins", "Admins", "netbird-admins"].
  2. Log that user in so JWT group-sync runs.
  3. Inspect the groups store.

Expected: one group row per distinct name; the user is a member of each named group once.

Actual: a separate group row is created for each duplicate occurrence (e.g. two netbird-admins rows with different IDs and two Admins rows), and the user is auto-joined to all of them. The duplicates are not collapsed.

Suggested fix

De-duplicate the claimed group names in getJWTGroupsChanges before the create/diff step: collapse the incoming groupNames to a set (preserving the existing case/normalisation semantics) so a duplicated claim is idempotent. This makes group-sync robust to misconfigured or multi-mapping IdPs without requiring every operator to guarantee exactly one emitter upstream.

Impact

  • Integrity / split-brain: two distinct group IDs share one name. A later policy that binds the group by name-scoped selection may resolve to only one of the duplicate IDs, while the user is a member of both, so the policy silently applies to a subset of the intended membership, or the "same" group means different things in different places. This is a latent authorization correctness hazard, not merely cosmetic clutter.
  • It is also easy to hit by accident (any IdP where a default mapping and an explicit mapping both carry groups), so a single dedupe at the sync boundary removes a whole class of operator foot-guns.

(Related observation: #5399, the embedded Dex connector dropping the groups scope on create, is a separate groups-handling issue in the same area.)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions