mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
feat: add user_secrets table (#19162)
Closes https://github.com/coder/internal/issues/780 ## Summary of changes: - added `user_secrets` table - `user_secrets` table contains `env_name` and `file_path` fields which are not used at the moment, but will be used in later PRs - `user_secrets` table doesn't contain `value_key_id`, I will add it in a separate migration in a dbcrypt PR - on one hand I don't want to add fields which are not used (because it's a risk smth may change in implementation later), on the other hand I don't want to add too many migrations for user secrets table - added unique sql indexes - added sql queries for CRUD operations on user-secrets - introduced new `ResourceUserSecret` resource - basic unit-tests for CRUD ops and authorization behavior - Role updates: - owner: - remove `ResourceUserSecret` from site-wide perms - add `ResourceUserSecret` to user-wide perms - orgAdmin - remove `ResourceUserSecret` from org-wide perms; seems it's not strictly required, because `ResourceUserSecret` is not tied to organization in dbauthz wrappers? - memberRole - no need to change memberRole because it implicitly has access to user-secrets thanks to the `allPermsExcept` - is it enough changes to roles? Main questions: - [ ] We will have 2 migrations for user-secrets: - initial migration (in current PR) - adding `value_key_id` in dbcrypt PR - is this approach reasonable? - [ ] Are changes to roles's permissions are correct? - [ ] Are changes in roles_test.go are correct? --------- Co-authored-by: Steven Masley <Emyrk@users.noreply.github.com>
This commit is contained in:
Yevhenii Shcherbina
co-authored by
Steven Masley
Steven Masley
parent
34c46c0748
commit
c65996a041
@@ -6004,6 +6004,252 @@ func TestGetRunningPrebuiltWorkspaces(t *testing.T) {
|
||||
require.Equal(t, runningPrebuild.ID, runningPrebuilds[0].ID, "expected the running prebuilt workspace to be returned")
|
||||
}
|
||||
|
||||
func TestUserSecretsCRUDOperations(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Use raw database without dbauthz wrapper for this test
|
||||
db, _ := dbtestutil.NewDB(t)
|
||||
ctx := testutil.Context(t, testutil.WaitMedium)
|
||||
|
||||
t.Run("FullCRUDWorkflow", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Create a new user for this test
|
||||
testUser := dbgen.User(t, db, database.User{})
|
||||
|
||||
// 1. CREATE
|
||||
secretID := uuid.New()
|
||||
createParams := database.CreateUserSecretParams{
|
||||
ID: secretID,
|
||||
UserID: testUser.ID,
|
||||
Name: "workflow-secret",
|
||||
Description: "Secret for full CRUD workflow",
|
||||
Value: "workflow-value",
|
||||
EnvName: "WORKFLOW_ENV",
|
||||
FilePath: "/workflow/path",
|
||||
}
|
||||
|
||||
createdSecret, err := db.CreateUserSecret(ctx, createParams)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, secretID, createdSecret.ID)
|
||||
|
||||
// 2. READ by ID
|
||||
readSecret, err := db.GetUserSecret(ctx, createdSecret.ID)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, createdSecret.ID, readSecret.ID)
|
||||
assert.Equal(t, "workflow-secret", readSecret.Name)
|
||||
|
||||
// 3. READ by UserID and Name
|
||||
readByNameParams := database.GetUserSecretByUserIDAndNameParams{
|
||||
UserID: testUser.ID,
|
||||
Name: "workflow-secret",
|
||||
}
|
||||
readByNameSecret, err := db.GetUserSecretByUserIDAndName(ctx, readByNameParams)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, createdSecret.ID, readByNameSecret.ID)
|
||||
|
||||
// 4. LIST
|
||||
secrets, err := db.ListUserSecrets(ctx, testUser.ID)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, secrets, 1)
|
||||
assert.Equal(t, createdSecret.ID, secrets[0].ID)
|
||||
|
||||
// 5. UPDATE
|
||||
updateParams := database.UpdateUserSecretParams{
|
||||
ID: createdSecret.ID,
|
||||
Description: "Updated workflow description",
|
||||
Value: "updated-workflow-value",
|
||||
EnvName: "UPDATED_WORKFLOW_ENV",
|
||||
FilePath: "/updated/workflow/path",
|
||||
}
|
||||
|
||||
updatedSecret, err := db.UpdateUserSecret(ctx, updateParams)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, "Updated workflow description", updatedSecret.Description)
|
||||
assert.Equal(t, "updated-workflow-value", updatedSecret.Value)
|
||||
|
||||
// 6. DELETE
|
||||
err = db.DeleteUserSecret(ctx, createdSecret.ID)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Verify deletion
|
||||
_, err = db.GetUserSecret(ctx, createdSecret.ID)
|
||||
require.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "no rows in result set")
|
||||
|
||||
// Verify list is empty
|
||||
secrets, err = db.ListUserSecrets(ctx, testUser.ID)
|
||||
require.NoError(t, err)
|
||||
assert.Len(t, secrets, 0)
|
||||
})
|
||||
|
||||
t.Run("UniqueConstraints", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Create a new user for this test
|
||||
testUser := dbgen.User(t, db, database.User{})
|
||||
|
||||
// Create first secret
|
||||
secret1 := dbgen.UserSecret(t, db, database.UserSecret{
|
||||
UserID: testUser.ID,
|
||||
Name: "unique-test",
|
||||
Description: "First secret",
|
||||
Value: "value1",
|
||||
EnvName: "UNIQUE_ENV",
|
||||
FilePath: "/unique/path",
|
||||
})
|
||||
|
||||
// Try to create another secret with the same name (should fail)
|
||||
_, err := db.CreateUserSecret(ctx, database.CreateUserSecretParams{
|
||||
UserID: testUser.ID,
|
||||
Name: "unique-test", // Same name
|
||||
Description: "Second secret",
|
||||
Value: "value2",
|
||||
})
|
||||
require.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "duplicate key value")
|
||||
|
||||
// Try to create another secret with the same env_name (should fail)
|
||||
_, err = db.CreateUserSecret(ctx, database.CreateUserSecretParams{
|
||||
UserID: testUser.ID,
|
||||
Name: "unique-test-2",
|
||||
Description: "Second secret",
|
||||
Value: "value2",
|
||||
EnvName: "UNIQUE_ENV", // Same env_name
|
||||
})
|
||||
require.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "duplicate key value")
|
||||
|
||||
// Try to create another secret with the same file_path (should fail)
|
||||
_, err = db.CreateUserSecret(ctx, database.CreateUserSecretParams{
|
||||
UserID: testUser.ID,
|
||||
Name: "unique-test-3",
|
||||
Description: "Second secret",
|
||||
Value: "value2",
|
||||
FilePath: "/unique/path", // Same file_path
|
||||
})
|
||||
require.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "duplicate key value")
|
||||
|
||||
// Create secret with empty env_name and file_path (should succeed)
|
||||
secret2 := dbgen.UserSecret(t, db, database.UserSecret{
|
||||
UserID: testUser.ID,
|
||||
Name: "unique-test-4",
|
||||
Description: "Second secret",
|
||||
Value: "value2",
|
||||
EnvName: "", // Empty env_name
|
||||
FilePath: "", // Empty file_path
|
||||
})
|
||||
|
||||
// Verify both secrets exist
|
||||
_, err = db.GetUserSecret(ctx, secret1.ID)
|
||||
require.NoError(t, err)
|
||||
_, err = db.GetUserSecret(ctx, secret2.ID)
|
||||
require.NoError(t, err)
|
||||
})
|
||||
}
|
||||
|
||||
func TestUserSecretsAuthorization(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Use raw database and wrap with dbauthz for authorization testing
|
||||
db, _ := dbtestutil.NewDB(t)
|
||||
authorizer := rbac.NewStrictCachingAuthorizer(prometheus.NewRegistry())
|
||||
authDB := dbauthz.New(db, authorizer, slogtest.Make(t, &slogtest.Options{}), coderdtest.AccessControlStorePointer())
|
||||
ctx := testutil.Context(t, testutil.WaitMedium)
|
||||
|
||||
// Create test users
|
||||
user1 := dbgen.User(t, db, database.User{})
|
||||
user2 := dbgen.User(t, db, database.User{})
|
||||
owner := dbgen.User(t, db, database.User{})
|
||||
orgAdmin := dbgen.User(t, db, database.User{})
|
||||
|
||||
// Create organization for org-scoped roles
|
||||
org := dbgen.Organization(t, db, database.Organization{})
|
||||
|
||||
// Create secrets for users
|
||||
user1Secret := dbgen.UserSecret(t, db, database.UserSecret{
|
||||
UserID: user1.ID,
|
||||
Name: "user1-secret",
|
||||
Description: "User 1's secret",
|
||||
Value: "user1-value",
|
||||
})
|
||||
|
||||
user2Secret := dbgen.UserSecret(t, db, database.UserSecret{
|
||||
UserID: user2.ID,
|
||||
Name: "user2-secret",
|
||||
Description: "User 2's secret",
|
||||
Value: "user2-value",
|
||||
})
|
||||
|
||||
testCases := []struct {
|
||||
name string
|
||||
subject rbac.Subject
|
||||
secretID uuid.UUID
|
||||
expectedAccess bool
|
||||
}{
|
||||
{
|
||||
name: "UserCanAccessOwnSecrets",
|
||||
subject: rbac.Subject{
|
||||
ID: user1.ID.String(),
|
||||
Roles: rbac.RoleIdentifiers{rbac.RoleMember()},
|
||||
Scope: rbac.ScopeAll,
|
||||
},
|
||||
secretID: user1Secret.ID,
|
||||
expectedAccess: true,
|
||||
},
|
||||
{
|
||||
name: "UserCannotAccessOtherUserSecrets",
|
||||
subject: rbac.Subject{
|
||||
ID: user1.ID.String(),
|
||||
Roles: rbac.RoleIdentifiers{rbac.RoleMember()},
|
||||
Scope: rbac.ScopeAll,
|
||||
},
|
||||
secretID: user2Secret.ID,
|
||||
expectedAccess: false,
|
||||
},
|
||||
{
|
||||
name: "OwnerCannotAccessUserSecrets",
|
||||
subject: rbac.Subject{
|
||||
ID: owner.ID.String(),
|
||||
Roles: rbac.RoleIdentifiers{rbac.RoleOwner()},
|
||||
Scope: rbac.ScopeAll,
|
||||
},
|
||||
secretID: user1Secret.ID,
|
||||
expectedAccess: false,
|
||||
},
|
||||
{
|
||||
name: "OrgAdminCannotAccessUserSecrets",
|
||||
subject: rbac.Subject{
|
||||
ID: orgAdmin.ID.String(),
|
||||
Roles: rbac.RoleIdentifiers{rbac.ScopedRoleOrgAdmin(org.ID)},
|
||||
Scope: rbac.ScopeAll,
|
||||
},
|
||||
secretID: user1Secret.ID,
|
||||
expectedAccess: false,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range testCases {
|
||||
tc := tc // capture range variable
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
authCtx := dbauthz.As(ctx, tc.subject)
|
||||
|
||||
// Test GetUserSecret
|
||||
_, err := authDB.GetUserSecret(authCtx, tc.secretID)
|
||||
|
||||
if tc.expectedAccess {
|
||||
require.NoError(t, err, "expected access to be granted")
|
||||
} else {
|
||||
require.Error(t, err, "expected access to be denied")
|
||||
assert.True(t, dbauthz.IsNotAuthorizedError(err), "expected authorization error")
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestWorkspaceBuildDeadlineConstraint(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user