rsp2k/headscale
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

oidc groups: fix post-merge compile and migration issues

Browse Source 
Bugs found by running the real test suite after merging upstream:

- types/types_clone.go, types/types_view.go: extend the regeneration
  guard struct literals to include the new Groups field, and add a
  UserView.Groups() accessor. Generated files normally rebuilt via
  cloner / viewer; touched by hand here pending make generate.
- db/db.go: the migration adding the groups column ran after
  202505141324, which calls ListUsers() through the User struct that
  now includes Groups. Move the column-add to 202505141323 so the
  schema is in place before any migration loads users. Register the
  new ID in the FK-disabled migration list.
- db/db.go: 202507021200 recreates all tables from inline SQL during
  the SQLite schema migration; add groups to both the CREATE TABLE
  users statement and the INSERT INTO users ... SELECT FROM users_old
  so the column survives the recreation. Also fix a copy-paste bug
  in the Rollback closure that referenced tx instead of db.
- db/schema.sql: add the groups column to the canonical schema so
  squibble.Validate accepts databases produced by the new migration
  chain. Verified against all 7 historical sqlite dumps in
  hscontrol/db/testdata/sqlite.
- types/users_test.go: the casby-oidc-claim case now exercises group
  storage; update the want to include the JSON-encoded groups column.
- integration/oidc_groups_test.go: replace the aspirational draft
  (which referenced assertNoErr, scenario.usernames, hsic.WithTLS and
  other symbols that do not exist) with a focused test that follows
  the auth_oidc_test.go pattern. Verifies the groups column directly
  via sqlite3 inside the headscale container since the gRPC User
  message does not expose Groups.
This commit is contained in:
Ryan Malloy 2026-05-21 21:14:08 -06:00
parent 2c8640f822
commit 32ea1c1c84
6 changed files with 119 additions and 340 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

49
hscontrol/db/db.go
View File

@ -215,6 +215,27 @@ AND auth_key_id NOT IN (
},
Rollback: func(db *gorm.DB) error { return nil },
},
// Add groups column to users table for OIDC role mapping.
// Must run before any migration that loads users via the User struct
// (e.g., 202505141324), since the User struct now includes Groups.
{
ID: "202505141323",
Migrate: func(tx *gorm.DB) error {
if !tx.Migrator().HasColumn(&types.User{}, "groups") {
err := tx.Migrator().AddColumn(&types.User{}, "groups")
if err != nil {
return fmt.Errorf("adding groups column to users table: %w", err)
}
}
return nil
},
Rollback: func(db *gorm.DB) error {
if db.Migrator().HasColumn(&types.User{}, "groups") {
return db.Migrator().DropColumn(&types.User{}, "groups")
}
return nil
},
},
// Fix the provider identifier for users that have a double slash in the
// provider identifier.
{
@ -315,6 +336,7 @@ AND auth_key_id NOT IN (
provider_identifier text,
provider text,
profile_pic_url text,
groups text,
created_at datetime,
updated_at datetime,
deleted_at datetime
@ -381,8 +403,8 @@ AND auth_key_id NOT IN (
// Copy data directly using SQL
dataCopySQL := []string{
`INSERT INTO users (id, name, display_name, email, provider_identifier, provider, profile_pic_url, created_at, updated_at, deleted_at)
SELECT id, name, display_name, email, provider_identifier, provider, profile_pic_url, created_at, updated_at, deleted_at
`INSERT INTO users (id, name, display_name, email, provider_identifier, provider, profile_pic_url, groups, created_at, updated_at, deleted_at)
SELECT id, name, display_name, email, provider_identifier, provider, profile_pic_url, groups, created_at, updated_at, deleted_at
FROM users_old`,
`INSERT INTO pre_auth_keys (id, key, user_id, reusable, ephemeral, used, tags, expiration, created_at)
@ -447,28 +469,6 @@ AND auth_key_id NOT IN (
},
Rollback: func(db *gorm.DB) error { return nil },
},
// Add Groups column to users table for OIDC role-based access control
{
ID: "202509161200",
Migrate: func(tx *gorm.DB) error {
// Add Groups column to store OIDC group memberships as JSON
if !tx.Migrator().HasColumn(&types.User{}, "groups") {
err := tx.Migrator().AddColumn(&types.User{}, "groups")
if err != nil {
return fmt.Errorf("adding groups column to users table: %w", err)
}
log.Info().Msg("Added Groups column to users table for OIDC role mapping")
}
return nil
},
Rollback: func(db *gorm.DB) error {
// Remove Groups column on rollback
if tx.Migrator().HasColumn(&types.User{}, "groups") {
return tx.Migrator().DropColumn(&types.User{}, "groups")
}
return nil
},
},
// v0.27.1
{
// Drop all tables that are no longer in use and has existed.
@ -1000,6 +1000,7 @@ func runMigrations(cfg types.DatabaseConfig, dbConn *gorm.DB, migrations *gormig
"202502131714",
"202502171819",
"202505091439",
"202505141323",
"202505141324",
// As of 2025-07-02, no new IDs should be added here.

1
hscontrol/db/schema.sql
View File

@ -12,6 +12,7 @@ CREATE TABLE users(
provider_identifier text,
provider text,
profile_pic_url text,
groups text,
created_at datetime,
updated_at datetime,

1
hscontrol/types/types_clone.go
View File

@ -35,6 +35,7 @@ var _UserCloneNeedsRegeneration = User(struct {
ProviderIdentifier sql.NullString
Provider string
ProfilePicURL string
Groups string
}{})
// Clone makes a deep copy of Node.

3
hscontrol/types/types_view.go
View File

@ -124,8 +124,11 @@ var _UserViewNeedsRegeneration = User(struct {
ProviderIdentifier sql.NullString
Provider string
ProfilePicURL string
Groups string
}{})
func (v UserView) Groups() string { return v.ж.Groups }
// View returns a read-only view of Node.
func (p *Node) View() NodeView {
return NodeView{ж: p}

1
hscontrol/types/users_test.go
View File

@ -528,6 +528,7 @@ func TestOIDCClaimsJSONToUser(t *testing.T) {
Valid: true,
},
ProfilePicURL: "https://cdn.casbin.org/img/casbin.svg",
Groups: `["org1/department1","org1/department2"]`,
},
},
}

404
integration/oidc_groups_test.go
View File

@ -2,52 +2,47 @@ package integration
import (
"encoding/json"
"sort"
"strings"
"testing"
"time"
"github.com/google/go-cmp/cmp"
v1 "github.com/juanfont/headscale/gen/go/headscale/v1"
"github.com/juanfont/headscale/integration/hsic"
"github.com/juanfont/headscale/integration/tsic"
"github.com/oauth2-proxy/mockoidc"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// TestOIDCGroupsExtraction tests that OIDC groups are properly extracted and stored
func TestOIDCGroupsExtraction(t *testing.T) {
// TestOIDCGroupsPersisted verifies that the `groups` claim from an OIDC
// provider is persisted into the users.groups column after the user logs in.
//
// The implementation under test:
// - User.Groups column (TEXT, JSON-encoded []string) added by migration
// 202505141323 in hscontrol/db/db.go.
// - User.SetGroups / User.GetGroups in hscontrol/types/users.go.
// - FromClaim() calls SetGroups(claims.Groups) so login populates the column.
// - OIDCClaims.Groups is FlexibleStringSlice so providers like JumpCloud
// that return a single string instead of a one-element array also work.
//
// Verification is done by reading the SQLite database inside the headscale
// container directly, because the gRPC User message does not currently
// expose Groups. Adding groups to the gRPC API is a separate, larger change.
func TestOIDCGroupsPersisted(t *testing.T) {
IntegrationSkip(t)
// Create mock users with different group memberships
// mockoidc serves logins in strict queue order, so keep NodesPerUser=1.
spec := ScenarioSpec{
NodesPerUser: 1,
Users: []string{"admin", "user", "readonly"},
Users: []string{"admin", "dev", "solo"},
OIDCUsers: []mockoidc.MockUser{
// Admin user with multiple groups
{
Subject: "admin@example.com",
Email: "admin@example.com",
PreferredUsername: "admin",
Groups: []string{"admins", "users", "engineering"},
},
// Regular user with single group
{
Subject: "user@example.com",
Email: "user@example.com",
PreferredUsername: "user",
Groups: []string{"users"},
},
// Readonly user with different groups
{
Subject: "readonly@example.com",
Email: "readonly@example.com",
PreferredUsername: "readonly",
Groups: []string{"readonly", "auditors"},
},
oidcMockUserWithGroups("admin", true, []string{"admins", "engineering"}),
oidcMockUserWithGroups("dev", true, []string{"engineering"}),
// User with empty groups — must round-trip as no Groups stored.
oidcMockUserWithGroups("solo", true, nil),
},
}
scenario, err := NewScenario(spec)
assertNoErr(t, err)
require.NoError(t, err)
defer scenario.ShutdownAssertNoPanics(t)
oidcMap := map[string]string{
@ -55,310 +50,87 @@ func TestOIDCGroupsExtraction(t *testing.T) {
"HEADSCALE_OIDC_CLIENT_ID": scenario.mockOIDC.ClientID(),
"CREDENTIALS_DIRECTORY_TEST": "/tmp",
"HEADSCALE_OIDC_CLIENT_SECRET_PATH": "${CREDENTIALS_DIRECTORY_TEST}/hs_client_oidc_secret",
// Enable groups claim extraction
"HEADSCALE_OIDC_EXTRA_PARAMS": `{"groups_claim": "groups"}`,
// Make sure the OIDC scope set includes "groups" so the IdP emits the claim.
"HEADSCALE_OIDC_SCOPE": "openid,profile,email,groups",
}
err = scenario.CreateHeadscaleEnvWithLoginURL(
nil,
hsic.WithTestName("oidcgroups"),
hsic.WithConfigEnv(oidcMap),
hsic.WithTLS(),
hsic.WithHostnameAsServerURL(),
hsic.WithFileInContainer("/tmp/hs_client_oidc_secret", []byte(scenario.mockOIDC.ClientSecret())),
)
assertNoErr(t, err)
requireNoErrHeadscaleEnv(t, err)
// Perform OIDC logins for all users
allClients, err := scenario.ListTailscaleClients()
assertNoErr(t, err)
// Drive the OIDC login flow for every client.
_, err = scenario.ListTailscaleClients()
requireNoErrListClients(t, err)
err = scenario.WaitForTailscaleSync()
requireNoErrSync(t, err)
for _, client := range allClients {
user, ok := scenario.usernames[client.Hostname()]
assertOK(t, ok)
headscale, err := scenario.Headscale()
require.NoError(t, err)
_ = client.Login(scenario.loginWaitGroup, user)
// Wait for login to complete
scenario.loginWaitGroup.Wait()
time.Sleep(5 * time.Second)
}
// Test groups were extracted correctly
t.Run("verify-groups-extracted", func(t *testing.T) {
// Get all users from Headscale
users, err := scenario.ControlServer().ListUsers(&v1.ListUsersRequest{})
assertNoErr(t, err)
// Verify each user has correct groups
expectedGroups := map[string][]string{
"admin": {"admins", "users", "engineering"},
"user": {"users"},
"readonly": {"readonly", "auditors"},
}
for _, user := range users.GetUsers() {
// Parse groups from user
var userGroups []string
if user.GetGroups() != "" {
err := json.Unmarshal([]byte(user.GetGroups()), &userGroups)
assertNoErr(t, err)
}
// Check expected groups
expected, exists := expectedGroups[user.GetName()]
assert.True(t, exists, "Unexpected user: %s", user.GetName())
// Sort both slices for comparison
assert.ElementsMatch(t, expected, userGroups,
"User %s has incorrect groups. Expected: %v, Got: %v",
user.GetName(), expected, userGroups)
}
// Query the SQLite database inside the headscale container for the groups
// column. CLI/gRPC do not expose it yet; this is the authoritative store.
const dbPath = "/tmp/integration_test_db.sqlite3"
out, err := headscale.Execute([]string{
"sqlite3", dbPath,
"-cmd", ".mode tabs",
"SELECT name, COALESCE(groups, '') FROM users WHERE provider = 'oidc' ORDER BY name;",
})
require.NoError(t, err, "querying users.groups from sqlite")
// Test groups persist across logins
t.Run("verify-groups-persistence", func(t *testing.T) {
// Get a client and log it out then back in
client := allClients[0]
user := scenario.usernames[client.Hostname()]
got := parseGroupsRows(t, out)
// Logout
err := client.Logout()
assertNoErr(t, err)
want := map[string][]string{
"admin": {"admins", "engineering"},
"dev": {"engineering"},
"solo": nil,
}
// Login again
_ = client.Login(scenario.loginWaitGroup, user)
scenario.loginWaitGroup.Wait()
time.Sleep(3 * time.Second)
for name, wantGroups := range want {
gotGroups, ok := got[name]
assert.True(t, ok, "user %q not present in users table", name)
assert.ElementsMatch(t, wantGroups, gotGroups,
"groups mismatch for user %q (raw rows: %q)", name, out)
}
}
// Verify groups are still there
users, err := scenario.ControlServer().ListUsers(&v1.ListUsersRequest{})
assertNoErr(t, err)
// Find our user
var targetUser *v1.User
for _, u := range users.GetUsers() {
if u.GetName() == user {
targetUser = u
break
}
// parseGroupsRows parses the tab-separated output of:
//
// SELECT name, COALESCE(groups, '') FROM users ...
//
// Returns a map of username -> decoded groups slice. An empty groups column
// (stored as "" by SetGroups when the input slice is empty) decodes to nil.
func parseGroupsRows(t *testing.T, raw string) map[string][]string {
t.Helper()
rows := map[string][]string{}
for _, line := range strings.Split(strings.TrimSpace(raw), "\n") {
if line == "" {
continue
}
assert.NotNil(t, targetUser, "User not found after re-login")
parts := strings.SplitN(line, "\t", 2)
require.Len(t, parts, 2, "unexpected sqlite row format: %q", line)
name, groupsJSON := parts[0], parts[1]
// Verify groups are preserved
var userGroups []string
if targetUser.GetGroups() != "" {
err := json.Unmarshal([]byte(targetUser.GetGroups()), &userGroups)
assertNoErr(t, err)
if groupsJSON == "" {
rows[name] = nil
continue
}
assert.NotEmpty(t, userGroups, "Groups should persist after re-login")
})
var gs []string
require.NoError(t, json.Unmarshal([]byte(groupsJSON), &gs),
"groups column for %q is not valid JSON: %q", name, groupsJSON)
sort.Strings(gs)
rows[name] = gs
}
return rows
}
// TestOIDCGroupsWithoutClaim tests behavior when groups claim is not configured
func TestOIDCGroupsWithoutClaim(t *testing.T) {
IntegrationSkip(t)
spec := ScenarioSpec{
NodesPerUser: 1,
Users: []string{"user1"},
OIDCUsers: []mockoidc.MockUser{
{
Subject: "user1@example.com",
Email: "user1@example.com",
PreferredUsername: "user1",
Groups: []string{"admins", "users"}, // Groups present but not requested
},
},
}
scenario, err := NewScenario(spec)
assertNoErr(t, err)
defer scenario.ShutdownAssertNoPanics(t)
// OIDC config WITHOUT groups claim
oidcMap := map[string]string{
"HEADSCALE_OIDC_ISSUER": scenario.mockOIDC.Issuer(),
"HEADSCALE_OIDC_CLIENT_ID": scenario.mockOIDC.ClientID(),
"CREDENTIALS_DIRECTORY_TEST": "/tmp",
"HEADSCALE_OIDC_CLIENT_SECRET_PATH": "${CREDENTIALS_DIRECTORY_TEST}/hs_client_oidc_secret",
// No groups claim configured
}
err = scenario.CreateHeadscaleEnvWithLoginURL(
nil,
hsic.WithTestName("oidcnogroups"),
hsic.WithConfigEnv(oidcMap),
hsic.WithTLS(),
hsic.WithHostnameAsServerURL(),
)
assertNoErr(t, err)
// Login user
allClients, err := scenario.ListTailscaleClients()
assertNoErr(t, err)
client := allClients[0]
user := scenario.usernames[client.Hostname()]
_ = client.Login(scenario.loginWaitGroup, user)
scenario.loginWaitGroup.Wait()
time.Sleep(3 * time.Second)
// Verify user exists but has no groups
users, err := scenario.ControlServer().ListUsers(&v1.ListUsersRequest{})
assertNoErr(t, err)
assert.Len(t, users.GetUsers(), 1, "Should have exactly one user")
user1 := users.GetUsers()[0]
assert.Empty(t, user1.GetGroups(), "User should have no groups when claim not configured")
// oidcMockUserWithGroups extends [oidcMockUser] with a Groups claim.
// mockoidc populates the id_token / userinfo from this struct verbatim.
func oidcMockUserWithGroups(username string, emailVerified bool, groups []string) mockoidc.MockUser {
u := oidcMockUser(username, emailVerified)
u.Groups = groups
return u
}
// TestOIDCGroupsEmptyGroups tests behavior when user has no groups
func TestOIDCGroupsEmptyGroups(t *testing.T) {
IntegrationSkip(t)
spec := ScenarioSpec{
NodesPerUser: 1,
Users: []string{"ungrouped"},
OIDCUsers: []mockoidc.MockUser{
{
Subject: "ungrouped@example.com",
Email: "ungrouped@example.com",
PreferredUsername: "ungrouped",
Groups: []string{}, // User has no groups
},
},
}
scenario, err := NewScenario(spec)
assertNoErr(t, err)
defer scenario.ShutdownAssertNoPanics(t)
oidcMap := map[string]string{
"HEADSCALE_OIDC_ISSUER": scenario.mockOIDC.Issuer(),
"HEADSCALE_OIDC_CLIENT_ID": scenario.mockOIDC.ClientID(),
"CREDENTIALS_DIRECTORY_TEST": "/tmp",
"HEADSCALE_OIDC_CLIENT_SECRET_PATH": "${CREDENTIALS_DIRECTORY_TEST}/hs_client_oidc_secret",
"HEADSCALE_OIDC_EXTRA_PARAMS": `{"groups_claim": "groups"}`,
}
err = scenario.CreateHeadscaleEnvWithLoginURL(
nil,
hsic.WithTestName("oidcemptygroups"),
hsic.WithConfigEnv(oidcMap),
hsic.WithTLS(),
hsic.WithHostnameAsServerURL(),
)
assertNoErr(t, err)
// Login user with no groups
allClients, err := scenario.ListTailscaleClients()
assertNoErr(t, err)
client := allClients[0]
user := scenario.usernames[client.Hostname()]
_ = client.Login(scenario.loginWaitGroup, user)
scenario.loginWaitGroup.Wait()
time.Sleep(3 * time.Second)
// Verify user exists with empty groups
users, err := scenario.ControlServer().ListUsers(&v1.ListUsersRequest{})
assertNoErr(t, err)
assert.Len(t, users.GetUsers(), 1, "Should have exactly one user")
ungroupedUser := users.GetUsers()[0]
assert.Empty(t, ungroupedUser.GetGroups(), "User with no groups should have empty groups field")
}
// TestOIDCGroupsUpdatesOnLogin tests that groups are updated when user logs in again
func TestOIDCGroupsUpdatesOnLogin(t *testing.T) {
IntegrationSkip(t)
// Create scenario with user having initial groups
spec := ScenarioSpec{
NodesPerUser: 1,
Users: []string{"dynamic"},
OIDCUsers: []mockoidc.MockUser{
{
Subject: "dynamic@example.com",
Email: "dynamic@example.com",
PreferredUsername: "dynamic",
Groups: []string{"initial-group"},
},
},
}
scenario, err := NewScenario(spec)
assertNoErr(t, err)
defer scenario.ShutdownAssertNoPanics(t)
oidcMap := map[string]string{
"HEADSCALE_OIDC_ISSUER": scenario.mockOIDC.Issuer(),
"HEADSCALE_OIDC_CLIENT_ID": scenario.mockOIDC.ClientID(),
"CREDENTIALS_DIRECTORY_TEST": "/tmp",
"HEADSCALE_OIDC_CLIENT_SECRET_PATH": "${CREDENTIALS_DIRECTORY_TEST}/hs_client_oidc_secret",
"HEADSCALE_OIDC_EXTRA_PARAMS": `{"groups_claim": "groups"}`,
}
err = scenario.CreateHeadscaleEnvWithLoginURL(
nil,
hsic.WithTestName("oidcdynamicgroups"),
hsic.WithConfigEnv(oidcMap),
hsic.WithTLS(),
hsic.WithHostnameAsServerURL(),
)
assertNoErr(t, err)
// Initial login
allClients, err := scenario.ListTailscaleClients()
assertNoErr(t, err)
client := allClients[0]
user := scenario.usernames[client.Hostname()]
_ = client.Login(scenario.loginWaitGroup, user)
scenario.loginWaitGroup.Wait()
time.Sleep(3 * time.Second)
// Verify initial groups
users, err := scenario.ControlServer().ListUsers(&v1.ListUsersRequest{})
assertNoErr(t, err)
userObj := users.GetUsers()[0]
var initialGroups []string
if userObj.GetGroups() != "" {
err := json.Unmarshal([]byte(userObj.GetGroups()), &initialGroups)
assertNoErr(t, err)
}
assert.Equal(t, []string{"initial-group"}, initialGroups)
// Update the mock user to have different groups
// Note: In a real test, this would involve updating the OIDC provider
// For this test, we'll simulate the scenario by modifying the mock
// Logout and login again (simulating groups change in OIDC provider)
err = client.Logout()
assertNoErr(t, err)
// Update mock user groups (this is test-specific, not production code)
scenario.mockOIDC.SetUserGroups("dynamic@example.com", []string{"updated-group", "admin-group"})
_ = client.Login(scenario.loginWaitGroup, user)
scenario.loginWaitGroup.Wait()
time.Sleep(3 * time.Second)
// Verify groups were updated
users, err = scenario.ControlServer().ListUsers(&v1.ListUsersRequest{})
assertNoErr(t, err)
updatedUserObj := users.GetUsers()[0]
var updatedGroups []string
if updatedUserObj.GetGroups() != "" {
err := json.Unmarshal([]byte(updatedUserObj.GetGroups()), &updatedGroups)
assertNoErr(t, err)
}
assert.ElementsMatch(t, []string{"updated-group", "admin-group"}, updatedGroups,
"Groups should be updated on subsequent login")
}