From 18dd5f892260360d398a93f10fbdf36671e9f901 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Lallement Date: Fri, 28 Nov 2025 09:59:45 +0100 Subject: [PATCH 1/5] Updated DB Schema with the right type for UGID --- internal/users/db/sql/create_schema.sql | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/users/db/sql/create_schema.sql b/internal/users/db/sql/create_schema.sql index f08c89e399..559892d4b3 100644 --- a/internal/users/db/sql/create_schema.sql +++ b/internal/users/db/sql/create_schema.sql @@ -6,14 +6,14 @@ CREATE TABLE IF NOT EXISTS users ( dir TEXT DEFAULT "", shell TEXT DEFAULT "/bin/bash", broker_id TEXT DEFAULT "", - locked BOOLEAN DEFAULT FALSE + locked BOOLEAN DEFAULT FALSE ); CREATE UNIQUE INDEX "idx_user_name" ON users ("name"); CREATE TABLE IF NOT EXISTS GROUPS ( name TEXT NOT NULL, -- Uniqueness is enforced by the index below gid INT PRIMARY KEY, -- Uniqueness and not NULL is enforced by PRIMARY KEY - ugid INT NOT NULL -- Uniqueness is enforced by the index below + ugid TEXT NOT NULL -- Uniqueness is enforced by the index below ); CREATE UNIQUE INDEX "idx_group_name" ON GROUPS ("name"); CREATE UNIQUE INDEX "idx_group_ugid" ON GROUPS ("ugid"); From 623802d9a89af8fc5f70ab5e9adb369ba3ff4a68 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Lallement Date: Fri, 28 Nov 2025 10:01:34 +0100 Subject: [PATCH 2/5] fix: table names must be lowercase --- internal/users/db/sql/create_schema.sql | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/users/db/sql/create_schema.sql b/internal/users/db/sql/create_schema.sql index 559892d4b3..668fcc6b90 100644 --- a/internal/users/db/sql/create_schema.sql +++ b/internal/users/db/sql/create_schema.sql @@ -10,20 +10,20 @@ CREATE TABLE IF NOT EXISTS users ( ); CREATE UNIQUE INDEX "idx_user_name" ON users ("name"); -CREATE TABLE IF NOT EXISTS GROUPS ( +CREATE TABLE IF NOT EXISTS groups ( name TEXT NOT NULL, -- Uniqueness is enforced by the index below gid INT PRIMARY KEY, -- Uniqueness and not NULL is enforced by PRIMARY KEY ugid TEXT NOT NULL -- Uniqueness is enforced by the index below ); -CREATE UNIQUE INDEX "idx_group_name" ON GROUPS ("name"); -CREATE UNIQUE INDEX "idx_group_ugid" ON GROUPS ("ugid"); +CREATE UNIQUE INDEX "idx_group_name" ON groups ("name"); +CREATE UNIQUE INDEX "idx_group_ugid" ON groups ("ugid"); CREATE TABLE IF NOT EXISTS users_to_groups ( uid INT NOT NULL, gid INT NOT NULL, PRIMARY KEY (uid, gid), FOREIGN KEY (uid) REFERENCES users (uid) ON DELETE CASCADE, - FOREIGN KEY (gid) REFERENCES GROUPS (gid) ON DELETE CASCADE + FOREIGN KEY (gid) REFERENCES groups (gid) ON DELETE CASCADE ); CREATE TABLE IF NOT EXISTS users_to_local_groups ( From 8064294f5791f5c0ffd3ab0c3cbec892af90516e Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Lallement Date: Thu, 27 Nov 2025 15:17:19 +0100 Subject: [PATCH 3/5] Test: ensures db v2 still loads TestLoadOldSchemaV2Database ensures databases created with the old schema (v2, ugid INT) can be opened and read correctly by the current code without additional migrations. --- internal/users/db/db_test.go | 38 +++++++++++++ .../OldSchemaV2/one_user_and_group_v2.sql | 54 +++++++++++++++++++ 2 files changed, 92 insertions(+) create mode 100644 internal/users/db/testdata/OldSchemaV2/one_user_and_group_v2.sql diff --git a/internal/users/db/db_test.go b/internal/users/db/db_test.go index 7dcf6bdf30..7098c5023b 100644 --- a/internal/users/db/db_test.go +++ b/internal/users/db/db_test.go @@ -919,6 +919,44 @@ func TestDeleteUser(t *testing.T) { } } +// TestLoadSchemaV2WithIntUGID ensures databases created with schema v2 with a +// ugid column of type INT can be opened and read correctly by the current code +// without additional migrations. +func TestLoadSchemaV2WithIntUGID(t *testing.T) { + t.Parallel() + + tempDir := t.TempDir() + + // Create a SQLite database from an old-schema SQL dump (ugid as INT, schema_version=2). + dump := filepath.Join("testdata", "OldSchemaV2", "one_user_and_group_v2.sql") + err := db.Z_ForTests_CreateDBFromDump(dump, tempDir) + require.NoError(t, err, "Setup: could not create database from old schema dump") + + // Open using current code path. + m, err := db.New(tempDir) + require.NoError(t, err, "Setup: could not open manager for old schema database") + t.Cleanup(func() { _ = m.Close() }) + + // Validate we can read the user and group correctly. + u, err := m.UserByID(1111) + require.NoError(t, err, "Should read user from old schema DB") + require.Equal(t, "user1", u.Name) + require.EqualValues(t, 11111, u.GID) + + g, err := m.GroupWithMembersByID(11111) + require.NoError(t, err, "Should read group from old schema DB") + require.Equal(t, "group1", g.Name) + // Even though ugid column was INT in old schema, it should scan into string fine. + require.Equal(t, "12345678", g.UGID) + require.Len(t, g.Users, 1) + require.Equal(t, "user1", g.Users[0]) + + // Also ensure lookup by UGID works with string input. + gByUGID, err := m.GroupByUGID("12345678") + require.NoError(t, err, "Should find group by UGID from old schema DB") + require.EqualValues(t, 11111, gByUGID.GID) +} + // initDB returns a new database ready to be used alongside its database directory. func initDB(t *testing.T, dbFile string) *db.Manager { t.Helper() diff --git a/internal/users/db/testdata/OldSchemaV2/one_user_and_group_v2.sql b/internal/users/db/testdata/OldSchemaV2/one_user_and_group_v2.sql new file mode 100644 index 0000000000..9fcd97b672 --- /dev/null +++ b/internal/users/db/testdata/OldSchemaV2/one_user_and_group_v2.sql @@ -0,0 +1,54 @@ +PRAGMA foreign_keys=OFF; +BEGIN TRANSACTION; +CREATE TABLE IF NOT EXISTS users ( + name TEXT NOT NULL, + uid INT PRIMARY KEY, + gid INT NOT NULL, + gecos TEXT DEFAULT "", + dir TEXT DEFAULT "", + shell TEXT DEFAULT "/bin/bash", + broker_id TEXT DEFAULT "", + locked BOOLEAN DEFAULT FALSE +); +CREATE UNIQUE INDEX "idx_user_name" ON users ("name"); + +CREATE TABLE IF NOT EXISTS GROUPS ( + name TEXT NOT NULL, + gid INT PRIMARY KEY, + ugid INT NOT NULL +); +CREATE UNIQUE INDEX "idx_group_name" ON GROUPS ("name"); +CREATE UNIQUE INDEX "idx_group_ugid" ON GROUPS ("ugid"); + +CREATE TABLE IF NOT EXISTS users_to_groups ( + uid INT NOT NULL, + gid INT NOT NULL, + PRIMARY KEY (uid, gid), + FOREIGN KEY (uid) REFERENCES users (uid) ON DELETE CASCADE, + FOREIGN KEY (gid) REFERENCES GROUPS (gid) ON DELETE CASCADE +); + +CREATE TABLE IF NOT EXISTS users_to_local_groups ( + uid INT NOT NULL, + group_name TEXT NOT NULL, + PRIMARY KEY (uid, group_name), + FOREIGN KEY (uid) REFERENCES users (uid) ON DELETE CASCADE +); + +CREATE TABLE IF NOT EXISTS schema_version ( + version INT PRIMARY KEY +); + +-- Seed data using the old schema (ugid as INT) +INSERT INTO users (name, uid, gid, gecos, dir, shell, broker_id, locked) +VALUES ('user1', 1111, 11111, 'User1 gecos', '/home/user1', '/bin/bash', 'broker-id', FALSE); + +INSERT INTO GROUPS (name, gid, ugid) +VALUES ('group1', 11111, 12345678); + +INSERT INTO users_to_groups (uid, gid) +VALUES (1111, 11111); + +-- Old schema v2 +INSERT INTO schema_version VALUES (2); +COMMIT; \ No newline at end of file From 9310833be3737402ec855563dbe275171291d050 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Lallement Date: Thu, 27 Nov 2025 15:21:26 +0100 Subject: [PATCH 4/5] Test: migration from schema v1 supports new UGID type TestMigrateOldSchemaV1 ensures databases created with schema v1 (no 'locked' column) are transparently migrated on open and readable via current code. --- internal/users/db/db_test.go | 38 ++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/internal/users/db/db_test.go b/internal/users/db/db_test.go index 7098c5023b..9d3970241c 100644 --- a/internal/users/db/db_test.go +++ b/internal/users/db/db_test.go @@ -957,6 +957,44 @@ func TestLoadSchemaV2WithIntUGID(t *testing.T) { require.EqualValues(t, 11111, gByUGID.GID) } +// TestMigrateOldSchemaV1 ensures databases created with schema v1 (no 'locked' column) +// are transparently migrated on open and readable via current code. +func TestMigrateOldSchemaV1(t *testing.T) { + t.Parallel() + + tempDir := t.TempDir() + + // Use existing v1 dump lacking 'locked' column; schema_version=1 + dump := filepath.Join("testdata", "TestMigrationAddLockedColumnToUsersTable", "one_user_and_group_without_locked_column.sql") + err := db.Z_ForTests_CreateDBFromDump(dump, tempDir) + require.NoError(t, err, "Setup: could not create database from v1 schema dump") + + // Open using current manager, which should apply the 'locked' column migration as needed. + m, err := db.New(tempDir) + require.NoError(t, err, "Setup: could not open manager for v1 schema database") + t.Cleanup(func() { _ = m.Close() }) + + // Validate user is readable and 'locked' defaults to false after migration. + u, err := m.UserByID(1111) + require.NoError(t, err, "Should read user from migrated DB") + require.Equal(t, "user1", u.Name) + require.EqualValues(t, 11111, u.GID) + require.False(t, u.Locked, "locked should default to false after migration") + + // Validate group and members are readable; ugid scans as string. + g, err := m.GroupWithMembersByID(11111) + require.NoError(t, err, "Should read group from migrated DB") + require.Equal(t, "group1", g.Name) + require.Equal(t, "12345678", g.UGID) + require.Len(t, g.Users, 1) + require.Equal(t, "user1", g.Users[0]) + + // Lookup by UGID should work. + gByUGID, err := m.GroupByUGID("12345678") + require.NoError(t, err) + require.EqualValues(t, 11111, gByUGID.GID) +} + // initDB returns a new database ready to be used alongside its database directory. func initDB(t *testing.T, dbFile string) *db.Manager { t.Helper() From b1c027b42764a45195e67094566866b94896ca7c Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Lallement Date: Fri, 28 Nov 2025 11:32:26 +0100 Subject: [PATCH 5/5] test: use table testing for schema migration tests --- internal/users/db/db_test.go | 113 +++++++----------- .../one_user_and_group_v2.sql | 0 2 files changed, 43 insertions(+), 70 deletions(-) rename internal/users/db/testdata/{OldSchemaV2 => TestLoadSchemaV2WithIntUGID}/one_user_and_group_v2.sql (100%) diff --git a/internal/users/db/db_test.go b/internal/users/db/db_test.go index 9d3970241c..0774627cbb 100644 --- a/internal/users/db/db_test.go +++ b/internal/users/db/db_test.go @@ -919,80 +919,53 @@ func TestDeleteUser(t *testing.T) { } } -// TestLoadSchemaV2WithIntUGID ensures databases created with schema v2 with a -// ugid column of type INT can be opened and read correctly by the current code -// without additional migrations. -func TestLoadSchemaV2WithIntUGID(t *testing.T) { +// TestBackwardCompatibilityAndMigrations covers loading legacy schemas (e.g., v2 with INT ugid) +// and migrating older schemas (e.g., v1 without 'locked' column) to the latest schema. +func TestBackwardCompatibilityAndMigrations(t *testing.T) { t.Parallel() - tempDir := t.TempDir() - - // Create a SQLite database from an old-schema SQL dump (ugid as INT, schema_version=2). - dump := filepath.Join("testdata", "OldSchemaV2", "one_user_and_group_v2.sql") - err := db.Z_ForTests_CreateDBFromDump(dump, tempDir) - require.NoError(t, err, "Setup: could not create database from old schema dump") - - // Open using current code path. - m, err := db.New(tempDir) - require.NoError(t, err, "Setup: could not open manager for old schema database") - t.Cleanup(func() { _ = m.Close() }) - - // Validate we can read the user and group correctly. - u, err := m.UserByID(1111) - require.NoError(t, err, "Should read user from old schema DB") - require.Equal(t, "user1", u.Name) - require.EqualValues(t, 11111, u.GID) - - g, err := m.GroupWithMembersByID(11111) - require.NoError(t, err, "Should read group from old schema DB") - require.Equal(t, "group1", g.Name) - // Even though ugid column was INT in old schema, it should scan into string fine. - require.Equal(t, "12345678", g.UGID) - require.Len(t, g.Users, 1) - require.Equal(t, "user1", g.Users[0]) - - // Also ensure lookup by UGID works with string input. - gByUGID, err := m.GroupByUGID("12345678") - require.NoError(t, err, "Should find group by UGID from old schema DB") - require.EqualValues(t, 11111, gByUGID.GID) -} + tests := map[string]struct { + dump string + }{ + "SchemaV2_IntUGID": {dump: filepath.Join("testdata", "TestLoadSchemaV2WithIntUGID", "one_user_and_group_v2.sql")}, + "SchemaV1_NoLockedColumn": {dump: filepath.Join("testdata", "TestMigrationAddLockedColumnToUsersTable", "one_user_and_group_without_locked_column.sql")}, + } -// TestMigrateOldSchemaV1 ensures databases created with schema v1 (no 'locked' column) -// are transparently migrated on open and readable via current code. -func TestMigrateOldSchemaV1(t *testing.T) { - t.Parallel() + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() - tempDir := t.TempDir() - - // Use existing v1 dump lacking 'locked' column; schema_version=1 - dump := filepath.Join("testdata", "TestMigrationAddLockedColumnToUsersTable", "one_user_and_group_without_locked_column.sql") - err := db.Z_ForTests_CreateDBFromDump(dump, tempDir) - require.NoError(t, err, "Setup: could not create database from v1 schema dump") - - // Open using current manager, which should apply the 'locked' column migration as needed. - m, err := db.New(tempDir) - require.NoError(t, err, "Setup: could not open manager for v1 schema database") - t.Cleanup(func() { _ = m.Close() }) - - // Validate user is readable and 'locked' defaults to false after migration. - u, err := m.UserByID(1111) - require.NoError(t, err, "Should read user from migrated DB") - require.Equal(t, "user1", u.Name) - require.EqualValues(t, 11111, u.GID) - require.False(t, u.Locked, "locked should default to false after migration") - - // Validate group and members are readable; ugid scans as string. - g, err := m.GroupWithMembersByID(11111) - require.NoError(t, err, "Should read group from migrated DB") - require.Equal(t, "group1", g.Name) - require.Equal(t, "12345678", g.UGID) - require.Len(t, g.Users, 1) - require.Equal(t, "user1", g.Users[0]) - - // Lookup by UGID should work. - gByUGID, err := m.GroupByUGID("12345678") - require.NoError(t, err) - require.EqualValues(t, 11111, gByUGID.GID) + tempDir := t.TempDir() + + err := db.Z_ForTests_CreateDBFromDump(tc.dump, tempDir) + require.NoError(t, err, "Setup: could not create database from dump") + + // Open using current manager, it'll trigger a migration depending on schema_version. + m, err := db.New(tempDir) + require.NoError(t, err, "Setup: could not open manager for database") + t.Cleanup(func() { _ = m.Close() }) + + // Validate user can be read and that locked is false (either set or default). + u, err := m.UserByID(1111) + require.NoError(t, err, "Should read user from DB") + require.Equal(t, "user1", u.Name) + require.EqualValues(t, 11111, u.GID) + require.False(t, u.Locked, "locked should be false in both old and migrated schemas") + + // Validate group and members. ugid should read as string regardless of underlying type. + g, err := m.GroupWithMembersByID(11111) + require.NoError(t, err, "Should read group from DB") + require.Equal(t, "group1", g.Name) + require.Equal(t, "12345678", g.UGID) + require.Len(t, g.Users, 1) + require.Equal(t, "user1", g.Users[0]) + + // Also ensure lookup by UGID works with string input. + gByUGID, err := m.GroupByUGID("12345678") + require.NoError(t, err) + require.EqualValues(t, 11111, gByUGID.GID) + }) + } } // initDB returns a new database ready to be used alongside its database directory. diff --git a/internal/users/db/testdata/OldSchemaV2/one_user_and_group_v2.sql b/internal/users/db/testdata/TestLoadSchemaV2WithIntUGID/one_user_and_group_v2.sql similarity index 100% rename from internal/users/db/testdata/OldSchemaV2/one_user_and_group_v2.sql rename to internal/users/db/testdata/TestLoadSchemaV2WithIntUGID/one_user_and_group_v2.sql