[management] Skip JWT group evaluation for embedded-IdP local users (#6422)

When JWT group sync is enabled with a restrictive JWTAllowGroups list, the local owner of an embedded-IdP (Dex) deployment can get locked out. The allow-groups check runs account-wide but local password users do not receive
external IdP group claims, so they can't satisfy the allowed list.

This skips JWT group evaluation for local Dex users so the restriction and JWT group sync continue to apply to external-IdP users as intended.
This commit is contained in:
Bethuel Mmbaga
2026-06-15 12:01:54 +03:00
committed by GitHub
parent b19467e3af
commit cd777395f2
6 changed files with 101 additions and 3 deletions

View File

@@ -41,6 +41,8 @@ type Config struct {
GRPCAddr string GRPCAddr string
} }
const localConnectorID = "local"
// Provider wraps a Dex server // Provider wraps a Dex server
type Provider struct { type Provider struct {
config *Config config *Config
@@ -544,7 +546,7 @@ func (p *Provider) CreateUser(ctx context.Context, email, username, password str
// Encode the user ID in Dex's format: base64(protobuf{user_id, connector_id}) // Encode the user ID in Dex's format: base64(protobuf{user_id, connector_id})
// This matches the format Dex uses in JWT tokens // This matches the format Dex uses in JWT tokens
encodedID := EncodeDexUserID(userID, "local") encodedID := EncodeDexUserID(userID, localConnectorID)
return encodedID, nil return encodedID, nil
} }
@@ -619,6 +621,13 @@ func DecodeDexUserID(encodedID string) (userID, connectorID string, err error) {
return userID, connectorID, nil return userID, connectorID, nil
} }
// IsLocalUserID reports whether encodedID is a Dex subject for the built-in
// local password connector.
func IsLocalUserID(encodedID string) bool {
_, connectorID, err := DecodeDexUserID(encodedID)
return err == nil && connectorID == localConnectorID
}
// GetUser returns a user by email // GetUser returns a user by email
func (p *Provider) GetUser(ctx context.Context, email string) (storage.Password, error) { func (p *Provider) GetUser(ctx context.Context, email string) (storage.Password, error) {
return p.storage.GetPassword(ctx, email) return p.storage.GetPassword(ctx, email)

View File

@@ -115,6 +115,26 @@ func TestDecodeDexUserID(t *testing.T) {
} }
} }
func TestIsLocalUserID(t *testing.T) {
tests := []struct {
name string
encodedID string
want bool
}{
{name: "local connector", encodedID: EncodeDexUserID("7aad8c05-3287-473f-b42a-365504bf25e7", "local"), want: true},
{name: "federated connector", encodedID: EncodeDexUserID("entra-user", "entra"), want: false},
{name: "non-dex external IdP id", encodedID: "google-oauth2|1234567890", want: false},
{name: "invalid base64", encodedID: "not-valid-base64!!!", want: false},
{name: "empty", encodedID: "", want: false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.want, IsLocalUserID(tt.encodedID))
})
}
}
func TestEncodeDexUserID(t *testing.T) { func TestEncodeDexUserID(t *testing.T) {
userID := "7aad8c05-3287-473f-b42a-365504bf25e7" userID := "7aad8c05-3287-473f-b42a-365504bf25e7"
connectorID := "local" connectorID := "local"

View File

@@ -28,6 +28,7 @@ import (
nbdns "github.com/netbirdio/netbird/dns" nbdns "github.com/netbirdio/netbird/dns"
"github.com/netbirdio/netbird/formatter/hook" "github.com/netbirdio/netbird/formatter/hook"
"github.com/netbirdio/netbird/idp/dex"
"github.com/netbirdio/netbird/management/internals/controllers/network_map" "github.com/netbirdio/netbird/management/internals/controllers/network_map"
nbconfig "github.com/netbirdio/netbird/management/internals/server/config" nbconfig "github.com/netbirdio/netbird/management/internals/server/config"
"github.com/netbirdio/netbird/management/server/account" "github.com/netbirdio/netbird/management/server/account"
@@ -1588,7 +1589,10 @@ func (am *DefaultAccountManager) updateUserAuthWithSingleMode(ctx context.Contex
// and propagates changes to peers if group propagation is enabled. // and propagates changes to peers if group propagation is enabled.
// requires userAuth to have been ValidateAndParseToken and EnsureUserAccessByJWTGroups by the AuthManager // requires userAuth to have been ValidateAndParseToken and EnsureUserAccessByJWTGroups by the AuthManager
func (am *DefaultAccountManager) SyncUserJWTGroups(ctx context.Context, userAuth auth.UserAuth) error { func (am *DefaultAccountManager) SyncUserJWTGroups(ctx context.Context, userAuth auth.UserAuth) error {
if userAuth.IsChild || userAuth.IsPAT { // Child accounts and PAT-authenticated requests do not sync JWT groups.
// Embedded-Dex local users also skip sync because local password authentication
// does not provide external IdP group claims.
if userAuth.IsChild || userAuth.IsPAT || dex.IsLocalUserID(userAuth.UserId) {
return nil return nil
} }

View File

@@ -26,6 +26,7 @@ import (
"github.com/netbirdio/netbird/shared/management/status" "github.com/netbirdio/netbird/shared/management/status"
nbdns "github.com/netbirdio/netbird/dns" nbdns "github.com/netbirdio/netbird/dns"
"github.com/netbirdio/netbird/idp/dex"
"github.com/netbirdio/netbird/management/internals/controllers/network_map" "github.com/netbirdio/netbird/management/internals/controllers/network_map"
"github.com/netbirdio/netbird/management/internals/controllers/network_map/controller" "github.com/netbirdio/netbird/management/internals/controllers/network_map/controller"
"github.com/netbirdio/netbird/management/internals/controllers/network_map/update_channel" "github.com/netbirdio/netbird/management/internals/controllers/network_map/update_channel"
@@ -723,6 +724,28 @@ func TestDefaultAccountManager_SyncUserJWTGroups(t *testing.T) {
require.Equal(t, g2.Name, "group2", "group2 name should match") require.Equal(t, g2.Name, "group2", "group2 name should match")
require.Equal(t, g2.Issued, types.GroupIssuedJWT, "group2 issued should match") require.Equal(t, g2.Issued, types.GroupIssuedJWT, "group2 issued should match")
}) })
t.Run("local embedded-Dex user is skipped", func(t *testing.T) {
initAccount.Settings.JWTGroupsEnabled = true
initAccount.Settings.JWTGroupsClaimName = "idp-groups"
err := manager.Store.SaveAccount(context.Background(), initAccount)
require.NoError(t, err, "save account failed")
localClaims := auth.UserAuth{
AccountId: accountID,
Domain: domain,
UserId: dex.EncodeDexUserID("local-owner", "local"),
Groups: []string{"group3", "group4"},
}
err = manager.SyncUserJWTGroups(context.Background(), localClaims)
require.NoError(t, err, "sync should be a no-op for local users")
account, err := manager.Store.GetAccount(context.Background(), accountID)
require.NoError(t, err, "get account failed")
for _, g := range account.Groups {
require.NotEqual(t, "group3", g.Name, "local user JWT groups must not be synced")
require.NotEqual(t, "group4", g.Name, "local user JWT groups must not be synced")
}
})
} }
func TestAccountManager_PrivateAccount(t *testing.T) { func TestAccountManager_PrivateAccount(t *testing.T) {

View File

@@ -12,6 +12,7 @@ import (
"github.com/netbirdio/netbird/shared/auth" "github.com/netbirdio/netbird/shared/auth"
"github.com/netbirdio/netbird/base62" "github.com/netbirdio/netbird/base62"
"github.com/netbirdio/netbird/idp/dex"
"github.com/netbirdio/netbird/management/server/store" "github.com/netbirdio/netbird/management/server/store"
"github.com/netbirdio/netbird/management/server/types" "github.com/netbirdio/netbird/management/server/types"
nbjwt "github.com/netbirdio/netbird/shared/auth/jwt" nbjwt "github.com/netbirdio/netbird/shared/auth/jwt"
@@ -74,7 +75,10 @@ func (m *manager) ValidateAndParseToken(ctx context.Context, value string) (auth
} }
func (m *manager) EnsureUserAccessByJWTGroups(ctx context.Context, userAuth auth.UserAuth, token *jwt.Token) (auth.UserAuth, error) { func (m *manager) EnsureUserAccessByJWTGroups(ctx context.Context, userAuth auth.UserAuth, token *jwt.Token) (auth.UserAuth, error) {
if userAuth.IsChild || userAuth.IsPAT { // Child accounts and PAT-authenticated requests do not use JWT group access checks.
// Embedded-Dex local users also skip them because local password authentication
// does not provide external IdP group claims.
if userAuth.IsChild || userAuth.IsPAT || dex.IsLocalUserID(userAuth.UserId) {
return userAuth, nil return userAuth, nil
} }

View File

@@ -16,6 +16,7 @@ import (
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"github.com/netbirdio/netbird/idp/dex"
"github.com/netbirdio/netbird/management/server/auth" "github.com/netbirdio/netbird/management/server/auth"
"github.com/netbirdio/netbird/management/server/store" "github.com/netbirdio/netbird/management/server/store"
"github.com/netbirdio/netbird/management/server/types" "github.com/netbirdio/netbird/management/server/types"
@@ -206,6 +207,43 @@ func TestAuthManager_EnsureUserAccessByJWTGroups(t *testing.T) {
_, err = manager.EnsureUserAccessByJWTGroups(context.Background(), userAuth, token) _, err = manager.EnsureUserAccessByJWTGroups(context.Background(), userAuth, token)
require.Error(t, err, "ensure user access is not in allowed groups") require.Error(t, err, "ensure user access is not in allowed groups")
}) })
t.Run("Local embedded-Dex user is exempt from JWT allow-groups", func(t *testing.T) {
account.Settings.JWTGroupsEnabled = true
account.Settings.JWTGroupsClaimName = "idp-groups"
account.Settings.JWTAllowGroups = []string{"not-a-group"}
err := store.SaveAccount(context.Background(), account)
require.NoError(t, err, "save account failed")
// Local Dex users have a "local" connector encoded in their user ID.
localUserAuth := nbauth.UserAuth{
AccountId: account.Id,
Domain: domain,
UserId: dex.EncodeDexUserID("local-owner", "local"),
}
localUserAuth, err = manager.EnsureUserAccessByJWTGroups(context.Background(), localUserAuth, token)
require.NoError(t, err, "local user must not be locked out by JWT allow-groups (issue #5337)")
require.Len(t, localUserAuth.Groups, 0, "JWT groups must not be evaluated for local users")
})
t.Run("Federated embedded-Dex user is still subject to JWT allow-groups", func(t *testing.T) {
account.Settings.JWTGroupsEnabled = true
account.Settings.JWTGroupsClaimName = "idp-groups"
account.Settings.JWTAllowGroups = []string{"not-a-group"}
err := store.SaveAccount(context.Background(), account)
require.NoError(t, err, "save account failed")
// A federated user (non-"local" connector) must remain restricted.
fedUserAuth := nbauth.UserAuth{
AccountId: account.Id,
Domain: domain,
UserId: dex.EncodeDexUserID("entra-user", "entra"),
}
_, err = manager.EnsureUserAccessByJWTGroups(context.Background(), fedUserAuth, token)
require.Error(t, err, "federated user must still be restricted by JWT allow-groups")
})
} }
func TestAuthManager_ValidateAndParseToken(t *testing.T) { func TestAuthManager_ValidateAndParseToken(t *testing.T) {