diff --git a/coderd/database/db.go b/coderd/database/db.go index 0f923a861efb4..23ee5028e3a12 100644 --- a/coderd/database/db.go +++ b/coderd/database/db.go @@ -3,9 +3,8 @@ // Query functions are generated using sqlc. // // To modify the database schema: -// 1. Add a new migration using "create_migration.sh" in database/migrations/ -// 2. Run "make coderd/database/generate" in the root to generate models. -// 3. Add/Edit queries in "query.sql" and run "make coderd/database/generate" to create Go code. +// 1. Add a new migration using "create_migration.sh" in database/migrations/ and run "make gen" to generate models. +// 2. Add/Edit queries in "query.sql" and run "make gen" to create Go code. package database import ( diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 9e616dd79dcbc..5c558aaa0d28c 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -1302,10 +1302,6 @@ func (q *querier) DeleteOldWorkspaceAgentStats(ctx context.Context) error { return q.db.DeleteOldWorkspaceAgentStats(ctx) } -func (q *querier) DeleteOrganization(ctx context.Context, id uuid.UUID) error { - return deleteQ(q.log, q.auth, q.db.GetOrganizationByID, q.db.DeleteOrganization)(ctx, id) -} - func (q *querier) DeleteOrganizationMember(ctx context.Context, arg database.DeleteOrganizationMemberParams) error { return deleteQ[database.OrganizationMember](q.log, q.auth, func(ctx context.Context, arg database.DeleteOrganizationMemberParams) (database.OrganizationMember, error) { member, err := database.ExpectOne(q.OrganizationMembers(ctx, database.OrganizationMembersParams(arg))) @@ -1926,7 +1922,7 @@ func (q *querier) GetOrganizationByID(ctx context.Context, id uuid.UUID) (databa return fetch(q.log, q.auth, q.db.GetOrganizationByID)(ctx, id) } -func (q *querier) GetOrganizationByName(ctx context.Context, name string) (database.Organization, error) { +func (q *querier) GetOrganizationByName(ctx context.Context, name database.GetOrganizationByNameParams) (database.Organization, error) { return fetch(q.log, q.auth, q.db.GetOrganizationByName)(ctx, name) } @@ -1943,7 +1939,7 @@ func (q *querier) GetOrganizations(ctx context.Context, args database.GetOrganiz return fetchWithPostFilter(q.auth, policy.ActionRead, fetch)(ctx, nil) } -func (q *querier) GetOrganizationsByUserID(ctx context.Context, userID uuid.UUID) ([]database.Organization, error) { +func (q *querier) GetOrganizationsByUserID(ctx context.Context, userID database.GetOrganizationsByUserIDParams) ([]database.Organization, error) { return fetchWithPostFilter(q.auth, policy.ActionRead, q.db.GetOrganizationsByUserID)(ctx, userID) } @@ -3737,6 +3733,16 @@ func (q *querier) UpdateOrganization(ctx context.Context, arg database.UpdateOrg return updateWithReturn(q.log, q.auth, fetch, q.db.UpdateOrganization)(ctx, arg) } +func (q *querier) UpdateOrganizationDeletedByID(ctx context.Context, arg database.UpdateOrganizationDeletedByIDParams) error { + deleteF := func(ctx context.Context, id uuid.UUID) error { + return q.db.UpdateOrganizationDeletedByID(ctx, database.UpdateOrganizationDeletedByIDParams{ + ID: id, + UpdatedAt: dbtime.Now(), + }) + } + return deleteQ(q.log, q.auth, q.db.GetOrganizationByID, deleteF)(ctx, arg.ID) +} + func (q *querier) UpdateProvisionerDaemonLastSeenAt(ctx context.Context, arg database.UpdateProvisionerDaemonLastSeenAtParams) error { if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceProvisionerDaemon); err != nil { return err diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index c960f06c65f1b..db4e68721538d 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -815,7 +815,7 @@ func (s *MethodTestSuite) TestOrganization() { })) s.Run("GetOrganizationByName", s.Subtest(func(db database.Store, check *expects) { o := dbgen.Organization(s.T(), db, database.Organization{}) - check.Args(o.Name).Asserts(o, policy.ActionRead).Returns(o) + check.Args(database.GetOrganizationByNameParams{Name: o.Name, Deleted: o.Deleted}).Asserts(o, policy.ActionRead).Returns(o) })) s.Run("GetOrganizationIDsByMemberIDs", s.Subtest(func(db database.Store, check *expects) { oa := dbgen.Organization(s.T(), db, database.Organization{}) @@ -839,7 +839,7 @@ func (s *MethodTestSuite) TestOrganization() { _ = dbgen.OrganizationMember(s.T(), db, database.OrganizationMember{UserID: u.ID, OrganizationID: a.ID}) b := dbgen.Organization(s.T(), db, database.Organization{}) _ = dbgen.OrganizationMember(s.T(), db, database.OrganizationMember{UserID: u.ID, OrganizationID: b.ID}) - check.Args(u.ID).Asserts(a, policy.ActionRead, b, policy.ActionRead).Returns(slice.New(a, b)) + check.Args(database.GetOrganizationsByUserIDParams{UserID: u.ID, Deleted: false}).Asserts(a, policy.ActionRead, b, policy.ActionRead).Returns(slice.New(a, b)) })) s.Run("InsertOrganization", s.Subtest(func(db database.Store, check *expects) { check.Args(database.InsertOrganizationParams{ @@ -960,13 +960,14 @@ func (s *MethodTestSuite) TestOrganization() { Name: "something-different", }).Asserts(o, policy.ActionUpdate) })) - s.Run("DeleteOrganization", s.Subtest(func(db database.Store, check *expects) { + s.Run("UpdateOrganizationDeletedByID", s.Subtest(func(db database.Store, check *expects) { o := dbgen.Organization(s.T(), db, database.Organization{ Name: "doomed", }) - check.Args( - o.ID, - ).Asserts(o, policy.ActionDelete) + check.Args(database.UpdateOrganizationDeletedByIDParams{ + ID: o.ID, + UpdatedAt: o.UpdatedAt, + }).Asserts(o, policy.ActionDelete).Returns() })) s.Run("OrganizationMembers", s.Subtest(func(db database.Store, check *expects) { o := dbgen.Organization(s.T(), db, database.Organization{}) diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 7f56ea5f463e5..9488577edca17 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -2157,19 +2157,6 @@ func (q *FakeQuerier) DeleteOldWorkspaceAgentStats(_ context.Context) error { return nil } -func (q *FakeQuerier) DeleteOrganization(_ context.Context, id uuid.UUID) error { - q.mutex.Lock() - defer q.mutex.Unlock() - - for i, org := range q.organizations { - if org.ID == id && !org.IsDefault { - q.organizations = append(q.organizations[:i], q.organizations[i+1:]...) - return nil - } - } - return sql.ErrNoRows -} - func (q *FakeQuerier) DeleteOrganizationMember(ctx context.Context, arg database.DeleteOrganizationMemberParams) error { err := validateDatabaseType(arg) if err != nil { @@ -3688,12 +3675,12 @@ func (q *FakeQuerier) GetOrganizationByID(_ context.Context, id uuid.UUID) (data return q.getOrganizationByIDNoLock(id) } -func (q *FakeQuerier) GetOrganizationByName(_ context.Context, name string) (database.Organization, error) { +func (q *FakeQuerier) GetOrganizationByName(_ context.Context, params database.GetOrganizationByNameParams) (database.Organization, error) { q.mutex.RLock() defer q.mutex.RUnlock() for _, organization := range q.organizations { - if organization.Name == name { + if organization.Name == params.Name && organization.Deleted == params.Deleted { return organization, nil } } @@ -3740,17 +3727,17 @@ func (q *FakeQuerier) GetOrganizations(_ context.Context, args database.GetOrgan return tmp, nil } -func (q *FakeQuerier) GetOrganizationsByUserID(_ context.Context, userID uuid.UUID) ([]database.Organization, error) { +func (q *FakeQuerier) GetOrganizationsByUserID(_ context.Context, arg database.GetOrganizationsByUserIDParams) ([]database.Organization, error) { q.mutex.RLock() defer q.mutex.RUnlock() organizations := make([]database.Organization, 0) for _, organizationMember := range q.organizationMembers { - if organizationMember.UserID != userID { + if organizationMember.UserID != arg.UserID { continue } for _, organization := range q.organizations { - if organization.ID != organizationMember.OrganizationID { + if organization.ID != organizationMember.OrganizationID || organization.Deleted != arg.Deleted { continue } organizations = append(organizations, organization) @@ -9822,6 +9809,26 @@ func (q *FakeQuerier) UpdateOrganization(_ context.Context, arg database.UpdateO return database.Organization{}, sql.ErrNoRows } +func (q *FakeQuerier) UpdateOrganizationDeletedByID(_ context.Context, arg database.UpdateOrganizationDeletedByIDParams) error { + if err := validateDatabaseType(arg); err != nil { + return err + } + + q.mutex.Lock() + defer q.mutex.Unlock() + + for index, organization := range q.organizations { + if organization.ID != arg.ID || organization.IsDefault { + continue + } + organization.Deleted = true + organization.UpdatedAt = arg.UpdatedAt + q.organizations[index] = organization + return nil + } + return sql.ErrNoRows +} + func (q *FakeQuerier) UpdateProvisionerDaemonLastSeenAt(_ context.Context, arg database.UpdateProvisionerDaemonLastSeenAtParams) error { err := validateDatabaseType(arg) if err != nil { diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index 665c10658a5bc..90ea140d0505c 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -77,6 +77,16 @@ func (m queryMetricsStore) InTx(f func(database.Store) error, options *database. return m.dbMetrics.InTx(f, options) } +func (m queryMetricsStore) DeleteOrganization(ctx context.Context, id uuid.UUID) error { + start := time.Now() + r0 := m.s.UpdateOrganizationDeletedByID(ctx, database.UpdateOrganizationDeletedByIDParams{ + ID: id, + UpdatedAt: time.Now(), + }) + m.queryLatencies.WithLabelValues("DeleteOrganization").Observe(time.Since(start).Seconds()) + return r0 +} + func (m queryMetricsStore) AcquireLock(ctx context.Context, pgAdvisoryXactLock int64) error { start := time.Now() err := m.s.AcquireLock(ctx, pgAdvisoryXactLock) @@ -329,13 +339,6 @@ func (m queryMetricsStore) DeleteOldWorkspaceAgentStats(ctx context.Context) err return err } -func (m queryMetricsStore) DeleteOrganization(ctx context.Context, id uuid.UUID) error { - start := time.Now() - r0 := m.s.DeleteOrganization(ctx, id) - m.queryLatencies.WithLabelValues("DeleteOrganization").Observe(time.Since(start).Seconds()) - return r0 -} - func (m queryMetricsStore) DeleteOrganizationMember(ctx context.Context, arg database.DeleteOrganizationMemberParams) error { start := time.Now() r0 := m.s.DeleteOrganizationMember(ctx, arg) @@ -945,7 +948,7 @@ func (m queryMetricsStore) GetOrganizationByID(ctx context.Context, id uuid.UUID return organization, err } -func (m queryMetricsStore) GetOrganizationByName(ctx context.Context, name string) (database.Organization, error) { +func (m queryMetricsStore) GetOrganizationByName(ctx context.Context, name database.GetOrganizationByNameParams) (database.Organization, error) { start := time.Now() organization, err := m.s.GetOrganizationByName(ctx, name) m.queryLatencies.WithLabelValues("GetOrganizationByName").Observe(time.Since(start).Seconds()) @@ -966,7 +969,7 @@ func (m queryMetricsStore) GetOrganizations(ctx context.Context, args database.G return organizations, err } -func (m queryMetricsStore) GetOrganizationsByUserID(ctx context.Context, userID uuid.UUID) ([]database.Organization, error) { +func (m queryMetricsStore) GetOrganizationsByUserID(ctx context.Context, userID database.GetOrganizationsByUserIDParams) ([]database.Organization, error) { start := time.Now() organizations, err := m.s.GetOrganizationsByUserID(ctx, userID) m.queryLatencies.WithLabelValues("GetOrganizationsByUserID").Observe(time.Since(start).Seconds()) @@ -2366,6 +2369,13 @@ func (m queryMetricsStore) UpdateOrganization(ctx context.Context, arg database. return r0, r1 } +func (m queryMetricsStore) UpdateOrganizationDeletedByID(ctx context.Context, arg database.UpdateOrganizationDeletedByIDParams) error { + start := time.Now() + r0 := m.s.UpdateOrganizationDeletedByID(ctx, arg) + m.queryLatencies.WithLabelValues("UpdateOrganizationDeletedByID").Observe(time.Since(start).Seconds()) + return r0 +} + func (m queryMetricsStore) UpdateProvisionerDaemonLastSeenAt(ctx context.Context, arg database.UpdateProvisionerDaemonLastSeenAtParams) error { start := time.Now() r0 := m.s.UpdateProvisionerDaemonLastSeenAt(ctx, arg) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index c7711505d7d51..38ee52aa76bbd 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -557,20 +557,6 @@ func (mr *MockStoreMockRecorder) DeleteOldWorkspaceAgentStats(ctx any) *gomock.C return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteOldWorkspaceAgentStats", reflect.TypeOf((*MockStore)(nil).DeleteOldWorkspaceAgentStats), ctx) } -// DeleteOrganization mocks base method. -func (m *MockStore) DeleteOrganization(ctx context.Context, id uuid.UUID) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DeleteOrganization", ctx, id) - ret0, _ := ret[0].(error) - return ret0 -} - -// DeleteOrganization indicates an expected call of DeleteOrganization. -func (mr *MockStoreMockRecorder) DeleteOrganization(ctx, id any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteOrganization", reflect.TypeOf((*MockStore)(nil).DeleteOrganization), ctx, id) -} - // DeleteOrganizationMember mocks base method. func (m *MockStore) DeleteOrganizationMember(ctx context.Context, arg database.DeleteOrganizationMemberParams) error { m.ctrl.T.Helper() @@ -1942,18 +1928,18 @@ func (mr *MockStoreMockRecorder) GetOrganizationByID(ctx, id any) *gomock.Call { } // GetOrganizationByName mocks base method. -func (m *MockStore) GetOrganizationByName(ctx context.Context, name string) (database.Organization, error) { +func (m *MockStore) GetOrganizationByName(ctx context.Context, arg database.GetOrganizationByNameParams) (database.Organization, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetOrganizationByName", ctx, name) + ret := m.ctrl.Call(m, "GetOrganizationByName", ctx, arg) ret0, _ := ret[0].(database.Organization) ret1, _ := ret[1].(error) return ret0, ret1 } // GetOrganizationByName indicates an expected call of GetOrganizationByName. -func (mr *MockStoreMockRecorder) GetOrganizationByName(ctx, name any) *gomock.Call { +func (mr *MockStoreMockRecorder) GetOrganizationByName(ctx, arg any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetOrganizationByName", reflect.TypeOf((*MockStore)(nil).GetOrganizationByName), ctx, name) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetOrganizationByName", reflect.TypeOf((*MockStore)(nil).GetOrganizationByName), ctx, arg) } // GetOrganizationIDsByMemberIDs mocks base method. @@ -1987,18 +1973,18 @@ func (mr *MockStoreMockRecorder) GetOrganizations(ctx, arg any) *gomock.Call { } // GetOrganizationsByUserID mocks base method. -func (m *MockStore) GetOrganizationsByUserID(ctx context.Context, userID uuid.UUID) ([]database.Organization, error) { +func (m *MockStore) GetOrganizationsByUserID(ctx context.Context, arg database.GetOrganizationsByUserIDParams) ([]database.Organization, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetOrganizationsByUserID", ctx, userID) + ret := m.ctrl.Call(m, "GetOrganizationsByUserID", ctx, arg) ret0, _ := ret[0].([]database.Organization) ret1, _ := ret[1].(error) return ret0, ret1 } // GetOrganizationsByUserID indicates an expected call of GetOrganizationsByUserID. -func (mr *MockStoreMockRecorder) GetOrganizationsByUserID(ctx, userID any) *gomock.Call { +func (mr *MockStoreMockRecorder) GetOrganizationsByUserID(ctx, arg any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetOrganizationsByUserID", reflect.TypeOf((*MockStore)(nil).GetOrganizationsByUserID), ctx, userID) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetOrganizationsByUserID", reflect.TypeOf((*MockStore)(nil).GetOrganizationsByUserID), ctx, arg) } // GetParameterSchemasByJobID mocks base method. @@ -5039,6 +5025,20 @@ func (mr *MockStoreMockRecorder) UpdateOrganization(ctx, arg any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateOrganization", reflect.TypeOf((*MockStore)(nil).UpdateOrganization), ctx, arg) } +// UpdateOrganizationDeletedByID mocks base method. +func (m *MockStore) UpdateOrganizationDeletedByID(ctx context.Context, arg database.UpdateOrganizationDeletedByIDParams) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpdateOrganizationDeletedByID", ctx, arg) + ret0, _ := ret[0].(error) + return ret0 +} + +// UpdateOrganizationDeletedByID indicates an expected call of UpdateOrganizationDeletedByID. +func (mr *MockStoreMockRecorder) UpdateOrganizationDeletedByID(ctx, arg any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateOrganizationDeletedByID", reflect.TypeOf((*MockStore)(nil).UpdateOrganizationDeletedByID), ctx, arg) +} + // UpdateProvisionerDaemonLastSeenAt mocks base method. func (m *MockStore) UpdateProvisionerDaemonLastSeenAt(ctx context.Context, arg database.UpdateProvisionerDaemonLastSeenAtParams) error { m.ctrl.T.Helper() diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index e699b34bd5433..e05d3a06d31f5 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -438,6 +438,74 @@ BEGIN END; $$; +CREATE FUNCTION protect_deleting_organizations() RETURNS trigger + LANGUAGE plpgsql + AS $$ +DECLARE + workspace_count int; + template_count int; + group_count int; + member_count int; + provisioner_keys_count int; +BEGIN + workspace_count := ( + SELECT count(*) as count FROM workspaces + WHERE + workspaces.organization_id = OLD.id + AND workspaces.deleted = false + ); + + template_count := ( + SELECT count(*) as count FROM templates + WHERE + templates.organization_id = OLD.id + AND templates.deleted = false + ); + + group_count := ( + SELECT count(*) as count FROM groups + WHERE + groups.organization_id = OLD.id + ); + + member_count := ( + SELECT count(*) as count FROM organization_members + WHERE + organization_members.organization_id = OLD.id + ); + + provisioner_keys_count := ( + Select count(*) as count FROM provisioner_keys + WHERE + provisioner_keys.organization_id = OLD.id + ); + + -- Fail the deletion if one of the following: + -- * the organization has 1 or more workspaces + -- * the organization has 1 or more templates + -- * the organization has 1 or more groups other than "Everyone" group + -- * the organization has 1 or more members other than the organization owner + -- * the organization has 1 or more provisioner keys + + IF (workspace_count + template_count + provisioner_keys_count) > 0 THEN + RAISE EXCEPTION 'cannot delete organization: organization has % workspaces, % templates, and % provisioner keys that must be deleted first', workspace_count, template_count, provisioner_keys_count; + END IF; + + IF (group_count) > 1 THEN + RAISE EXCEPTION 'cannot delete organization: organization has % groups that must be deleted first', group_count - 1; + END IF; + + -- Allow 1 member to exist, because you cannot remove yourself. You can + -- remove everyone else. Ideally, we only omit the member that matches + -- the user_id of the caller, however in a trigger, the caller is unknown. + IF (member_count) > 1 THEN + RAISE EXCEPTION 'cannot delete organization: organization has % members that must be deleted first', member_count - 1; + END IF; + + RETURN NEW; +END; +$$; + CREATE FUNCTION provisioner_tagset_contains(provisioner_tags tagset, job_tags tagset) RETURNS boolean LANGUAGE plpgsql AS $$ @@ -967,7 +1035,8 @@ CREATE TABLE organizations ( updated_at timestamp with time zone NOT NULL, is_default boolean DEFAULT false NOT NULL, display_name text NOT NULL, - icon text DEFAULT ''::text NOT NULL + icon text DEFAULT ''::text NOT NULL, + deleted boolean DEFAULT false NOT NULL ); CREATE TABLE parameter_schemas ( @@ -2030,9 +2099,6 @@ ALTER TABLE ONLY oauth2_provider_apps ALTER TABLE ONLY organization_members ADD CONSTRAINT organization_members_pkey PRIMARY KEY (organization_id, user_id); -ALTER TABLE ONLY organizations - ADD CONSTRAINT organizations_name UNIQUE (name); - ALTER TABLE ONLY organizations ADD CONSTRAINT organizations_pkey PRIMARY KEY (id); @@ -2218,9 +2284,7 @@ CREATE INDEX idx_organization_member_organization_id_uuid ON organization_member CREATE INDEX idx_organization_member_user_id_uuid ON organization_members USING btree (user_id); -CREATE UNIQUE INDEX idx_organization_name ON organizations USING btree (name); - -CREATE UNIQUE INDEX idx_organization_name_lower ON organizations USING btree (lower(name)); +CREATE UNIQUE INDEX idx_organization_name_lower ON organizations USING btree (lower(name)) WHERE (deleted = false); CREATE UNIQUE INDEX idx_provisioner_daemons_org_name_owner_key ON provisioner_daemons USING btree (organization_id, name, lower(COALESCE((tags ->> 'owner'::text), ''::text))); @@ -2352,6 +2416,8 @@ CREATE OR REPLACE VIEW provisioner_job_stats AS CREATE TRIGGER inhibit_enqueue_if_disabled BEFORE INSERT ON notification_messages FOR EACH ROW EXECUTE FUNCTION inhibit_enqueue_if_disabled(); +CREATE TRIGGER protect_deleting_organizations BEFORE UPDATE ON organizations FOR EACH ROW WHEN (((new.deleted = true) AND (old.deleted = false))) EXECUTE FUNCTION protect_deleting_organizations(); + CREATE TRIGGER remove_organization_member_custom_role BEFORE DELETE ON custom_roles FOR EACH ROW EXECUTE FUNCTION remove_organization_member_role(); COMMENT ON TRIGGER remove_organization_member_custom_role ON custom_roles IS 'When a custom_role is deleted, this trigger removes the role from all organization members.'; diff --git a/coderd/database/migrations/000296_organization_soft_delete.down.sql b/coderd/database/migrations/000296_organization_soft_delete.down.sql new file mode 100644 index 0000000000000..3db107e8a79f5 --- /dev/null +++ b/coderd/database/migrations/000296_organization_soft_delete.down.sql @@ -0,0 +1,12 @@ +DROP INDEX IF EXISTS idx_organization_name_lower; + +CREATE UNIQUE INDEX IF NOT EXISTS idx_organization_name ON organizations USING btree (name); +CREATE UNIQUE INDEX IF NOT EXISTS idx_organization_name_lower ON organizations USING btree (lower(name)); + +ALTER TABLE ONLY organizations + ADD CONSTRAINT organizations_name UNIQUE (name); + +DROP TRIGGER IF EXISTS protect_deleting_organizations ON organizations; +DROP FUNCTION IF EXISTS protect_deleting_organizations; + +ALTER TABLE organizations DROP COLUMN deleted; diff --git a/coderd/database/migrations/000296_organization_soft_delete.up.sql b/coderd/database/migrations/000296_organization_soft_delete.up.sql new file mode 100644 index 0000000000000..34b25139c950a --- /dev/null +++ b/coderd/database/migrations/000296_organization_soft_delete.up.sql @@ -0,0 +1,85 @@ +ALTER TABLE organizations ADD COLUMN deleted boolean DEFAULT FALSE NOT NULL; + +DROP INDEX IF EXISTS idx_organization_name; +DROP INDEX IF EXISTS idx_organization_name_lower; + +CREATE UNIQUE INDEX IF NOT EXISTS idx_organization_name_lower ON organizations USING btree (lower(name)) + where deleted = false; + +ALTER TABLE ONLY organizations + DROP CONSTRAINT IF EXISTS organizations_name; + +CREATE FUNCTION protect_deleting_organizations() + RETURNS TRIGGER AS +$$ +DECLARE + workspace_count int; + template_count int; + group_count int; + member_count int; + provisioner_keys_count int; +BEGIN + workspace_count := ( + SELECT count(*) as count FROM workspaces + WHERE + workspaces.organization_id = OLD.id + AND workspaces.deleted = false + ); + + template_count := ( + SELECT count(*) as count FROM templates + WHERE + templates.organization_id = OLD.id + AND templates.deleted = false + ); + + group_count := ( + SELECT count(*) as count FROM groups + WHERE + groups.organization_id = OLD.id + ); + + member_count := ( + SELECT count(*) as count FROM organization_members + WHERE + organization_members.organization_id = OLD.id + ); + + provisioner_keys_count := ( + Select count(*) as count FROM provisioner_keys + WHERE + provisioner_keys.organization_id = OLD.id + ); + + -- Fail the deletion if one of the following: + -- * the organization has 1 or more workspaces + -- * the organization has 1 or more templates + -- * the organization has 1 or more groups other than "Everyone" group + -- * the organization has 1 or more members other than the organization owner + -- * the organization has 1 or more provisioner keys + + IF (workspace_count + template_count + provisioner_keys_count) > 0 THEN + RAISE EXCEPTION 'cannot delete organization: organization has % workspaces, % templates, and % provisioner keys that must be deleted first', workspace_count, template_count, provisioner_keys_count; + END IF; + + IF (group_count) > 1 THEN + RAISE EXCEPTION 'cannot delete organization: organization has % groups that must be deleted first', group_count - 1; + END IF; + + -- Allow 1 member to exist, because you cannot remove yourself. You can + -- remove everyone else. Ideally, we only omit the member that matches + -- the user_id of the caller, however in a trigger, the caller is unknown. + IF (member_count) > 1 THEN + RAISE EXCEPTION 'cannot delete organization: organization has % members that must be deleted first', member_count - 1; + END IF; + + RETURN NEW; +END; +$$ LANGUAGE plpgsql; + +-- Trigger to protect organizations from being soft deleted with existing resources +CREATE TRIGGER protect_deleting_organizations + BEFORE UPDATE ON organizations + FOR EACH ROW + WHEN (NEW.deleted = true AND OLD.deleted = false) + EXECUTE FUNCTION protect_deleting_organizations(); diff --git a/coderd/database/models.go b/coderd/database/models.go index 5411591eed51c..4e3353f844a02 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -2675,6 +2675,7 @@ type Organization struct { IsDefault bool `db:"is_default" json:"is_default"` DisplayName string `db:"display_name" json:"display_name"` Icon string `db:"icon" json:"icon"` + Deleted bool `db:"deleted" json:"deleted"` } type OrganizationMember struct { diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 42b88d855e4c3..a5cedde6c4a73 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -94,7 +94,6 @@ type sqlcQuerier interface { // Logs can take up a lot of space, so it's important we clean up frequently. DeleteOldWorkspaceAgentLogs(ctx context.Context, threshold time.Time) error DeleteOldWorkspaceAgentStats(ctx context.Context) error - DeleteOrganization(ctx context.Context, id uuid.UUID) error DeleteOrganizationMember(ctx context.Context, arg DeleteOrganizationMemberParams) error DeleteProvisionerKey(ctx context.Context, id uuid.UUID) error DeleteReplicasUpdatedBefore(ctx context.Context, updatedAt time.Time) error @@ -197,10 +196,10 @@ type sqlcQuerier interface { GetOAuth2ProviderAppsByUserID(ctx context.Context, userID uuid.UUID) ([]GetOAuth2ProviderAppsByUserIDRow, error) GetOAuthSigningKey(ctx context.Context) (string, error) GetOrganizationByID(ctx context.Context, id uuid.UUID) (Organization, error) - GetOrganizationByName(ctx context.Context, name string) (Organization, error) + GetOrganizationByName(ctx context.Context, arg GetOrganizationByNameParams) (Organization, error) GetOrganizationIDsByMemberIDs(ctx context.Context, ids []uuid.UUID) ([]GetOrganizationIDsByMemberIDsRow, error) GetOrganizations(ctx context.Context, arg GetOrganizationsParams) ([]Organization, error) - GetOrganizationsByUserID(ctx context.Context, userID uuid.UUID) ([]Organization, error) + GetOrganizationsByUserID(ctx context.Context, arg GetOrganizationsByUserIDParams) ([]Organization, error) GetParameterSchemasByJobID(ctx context.Context, jobID uuid.UUID) ([]ParameterSchema, error) GetPresetByWorkspaceBuildID(ctx context.Context, workspaceBuildID uuid.UUID) (TemplateVersionPreset, error) GetPresetParametersByTemplateVersionID(ctx context.Context, templateVersionID uuid.UUID) ([]TemplateVersionPresetParameter, error) @@ -485,6 +484,7 @@ type sqlcQuerier interface { UpdateOAuth2ProviderAppByID(ctx context.Context, arg UpdateOAuth2ProviderAppByIDParams) (OAuth2ProviderApp, error) UpdateOAuth2ProviderAppSecretByID(ctx context.Context, arg UpdateOAuth2ProviderAppSecretByIDParams) (OAuth2ProviderAppSecret, error) UpdateOrganization(ctx context.Context, arg UpdateOrganizationParams) (Organization, error) + UpdateOrganizationDeletedByID(ctx context.Context, arg UpdateOrganizationDeletedByIDParams) error UpdateProvisionerDaemonLastSeenAt(ctx context.Context, arg UpdateProvisionerDaemonLastSeenAtParams) error UpdateProvisionerJobByID(ctx context.Context, arg UpdateProvisionerJobByIDParams) error UpdateProvisionerJobWithCancelByID(ctx context.Context, arg UpdateProvisionerJobWithCancelByIDParams) error diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index 00b189967f5a6..b60554de75359 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -21,6 +21,7 @@ import ( "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/db2sdk" "github.com/coder/coder/v2/coderd/database/dbauthz" + "github.com/coder/coder/v2/coderd/database/dbfake" "github.com/coder/coder/v2/coderd/database/dbgen" "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/database/dbtime" @@ -2916,6 +2917,136 @@ func TestGetUserStatusCounts(t *testing.T) { } } +func TestOrganizationDeleteTrigger(t *testing.T) { + t.Parallel() + + if !dbtestutil.WillUsePostgres() { + t.SkipNow() + } + + t.Run("WorkspaceExists", func(t *testing.T) { + t.Parallel() + db, _ := dbtestutil.NewDB(t) + + orgA := dbfake.Organization(t, db).Do() + + user := dbgen.User(t, db, database.User{}) + + dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + OrganizationID: orgA.Org.ID, + OwnerID: user.ID, + }).Do() + + ctx := testutil.Context(t, testutil.WaitShort) + err := db.UpdateOrganizationDeletedByID(ctx, database.UpdateOrganizationDeletedByIDParams{ + UpdatedAt: dbtime.Now(), + ID: orgA.Org.ID, + }) + require.Error(t, err) + // cannot delete organization: organization has 1 workspaces and 1 templates that must be deleted first + require.ErrorContains(t, err, "cannot delete organization") + require.ErrorContains(t, err, "has 1 workspaces") + require.ErrorContains(t, err, "1 templates") + }) + + t.Run("TemplateExists", func(t *testing.T) { + t.Parallel() + db, _ := dbtestutil.NewDB(t) + + orgA := dbfake.Organization(t, db).Do() + + user := dbgen.User(t, db, database.User{}) + + dbgen.Template(t, db, database.Template{ + OrganizationID: orgA.Org.ID, + CreatedBy: user.ID, + }) + + ctx := testutil.Context(t, testutil.WaitShort) + err := db.UpdateOrganizationDeletedByID(ctx, database.UpdateOrganizationDeletedByIDParams{ + UpdatedAt: dbtime.Now(), + ID: orgA.Org.ID, + }) + require.Error(t, err) + // cannot delete organization: organization has 0 workspaces and 1 templates that must be deleted first + require.ErrorContains(t, err, "cannot delete organization") + require.ErrorContains(t, err, "has 0 workspaces") + require.ErrorContains(t, err, "1 templates") + }) + + t.Run("ProvisionerKeyExists", func(t *testing.T) { + t.Parallel() + db, _ := dbtestutil.NewDB(t) + + orgA := dbfake.Organization(t, db).Do() + + dbgen.ProvisionerKey(t, db, database.ProvisionerKey{ + OrganizationID: orgA.Org.ID, + }) + + ctx := testutil.Context(t, testutil.WaitShort) + err := db.UpdateOrganizationDeletedByID(ctx, database.UpdateOrganizationDeletedByIDParams{ + UpdatedAt: dbtime.Now(), + ID: orgA.Org.ID, + }) + require.Error(t, err) + // cannot delete organization: organization has 1 provisioner keys that must be deleted first + require.ErrorContains(t, err, "cannot delete organization") + require.ErrorContains(t, err, "1 provisioner keys") + }) + + t.Run("GroupExists", func(t *testing.T) { + t.Parallel() + db, _ := dbtestutil.NewDB(t) + + orgA := dbfake.Organization(t, db).Do() + + dbgen.Group(t, db, database.Group{ + OrganizationID: orgA.Org.ID, + }) + + ctx := testutil.Context(t, testutil.WaitShort) + err := db.UpdateOrganizationDeletedByID(ctx, database.UpdateOrganizationDeletedByIDParams{ + UpdatedAt: dbtime.Now(), + ID: orgA.Org.ID, + }) + require.Error(t, err) + // cannot delete organization: organization has 1 groups that must be deleted first + require.ErrorContains(t, err, "cannot delete organization") + require.ErrorContains(t, err, "has 1 groups") + }) + + t.Run("MemberExists", func(t *testing.T) { + t.Parallel() + db, _ := dbtestutil.NewDB(t) + + orgA := dbfake.Organization(t, db).Do() + + userA := dbgen.User(t, db, database.User{}) + userB := dbgen.User(t, db, database.User{}) + + dbgen.OrganizationMember(t, db, database.OrganizationMember{ + OrganizationID: orgA.Org.ID, + UserID: userA.ID, + }) + + dbgen.OrganizationMember(t, db, database.OrganizationMember{ + OrganizationID: orgA.Org.ID, + UserID: userB.ID, + }) + + ctx := testutil.Context(t, testutil.WaitShort) + err := db.UpdateOrganizationDeletedByID(ctx, database.UpdateOrganizationDeletedByIDParams{ + UpdatedAt: dbtime.Now(), + ID: orgA.Org.ID, + }) + require.Error(t, err) + // cannot delete organization: organization has 1 members that must be deleted first + require.ErrorContains(t, err, "cannot delete organization") + require.ErrorContains(t, err, "has 1 members") + }) +} + func requireUsersMatch(t testing.TB, expected []database.User, found []database.GetUsersRow, msg string) { t.Helper() require.ElementsMatch(t, expected, database.ConvertUserRows(found), msg) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 58722dc152005..ea4124d8fca94 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -5066,28 +5066,15 @@ func (q *sqlQuerier) UpdateMemberRoles(ctx context.Context, arg UpdateMemberRole return i, err } -const deleteOrganization = `-- name: DeleteOrganization :exec -DELETE FROM - organizations -WHERE - id = $1 AND - is_default = false -` - -func (q *sqlQuerier) DeleteOrganization(ctx context.Context, id uuid.UUID) error { - _, err := q.db.ExecContext(ctx, deleteOrganization, id) - return err -} - const getDefaultOrganization = `-- name: GetDefaultOrganization :one SELECT - id, name, description, created_at, updated_at, is_default, display_name, icon + id, name, description, created_at, updated_at, is_default, display_name, icon, deleted FROM - organizations + organizations WHERE - is_default = true + is_default = true LIMIT - 1 + 1 ` func (q *sqlQuerier) GetDefaultOrganization(ctx context.Context) (Organization, error) { @@ -5102,17 +5089,18 @@ func (q *sqlQuerier) GetDefaultOrganization(ctx context.Context) (Organization, &i.IsDefault, &i.DisplayName, &i.Icon, + &i.Deleted, ) return i, err } const getOrganizationByID = `-- name: GetOrganizationByID :one SELECT - id, name, description, created_at, updated_at, is_default, display_name, icon + id, name, description, created_at, updated_at, is_default, display_name, icon, deleted FROM - organizations + organizations WHERE - id = $1 + id = $1 ` func (q *sqlQuerier) GetOrganizationByID(ctx context.Context, id uuid.UUID) (Organization, error) { @@ -5127,23 +5115,31 @@ func (q *sqlQuerier) GetOrganizationByID(ctx context.Context, id uuid.UUID) (Org &i.IsDefault, &i.DisplayName, &i.Icon, + &i.Deleted, ) return i, err } const getOrganizationByName = `-- name: GetOrganizationByName :one SELECT - id, name, description, created_at, updated_at, is_default, display_name, icon + id, name, description, created_at, updated_at, is_default, display_name, icon, deleted FROM - organizations + organizations WHERE - LOWER("name") = LOWER($1) + -- Optionally include deleted organizations + deleted = $1 AND + LOWER("name") = LOWER($2) LIMIT - 1 + 1 ` -func (q *sqlQuerier) GetOrganizationByName(ctx context.Context, name string) (Organization, error) { - row := q.db.QueryRowContext(ctx, getOrganizationByName, name) +type GetOrganizationByNameParams struct { + Deleted bool `db:"deleted" json:"deleted"` + Name string `db:"name" json:"name"` +} + +func (q *sqlQuerier) GetOrganizationByName(ctx context.Context, arg GetOrganizationByNameParams) (Organization, error) { + row := q.db.QueryRowContext(ctx, getOrganizationByName, arg.Deleted, arg.Name) var i Organization err := row.Scan( &i.ID, @@ -5154,37 +5150,40 @@ func (q *sqlQuerier) GetOrganizationByName(ctx context.Context, name string) (Or &i.IsDefault, &i.DisplayName, &i.Icon, + &i.Deleted, ) return i, err } const getOrganizations = `-- name: GetOrganizations :many SELECT - id, name, description, created_at, updated_at, is_default, display_name, icon + id, name, description, created_at, updated_at, is_default, display_name, icon, deleted FROM - organizations + organizations WHERE - true - -- Filter by ids - AND CASE - WHEN array_length($1 :: uuid[], 1) > 0 THEN - id = ANY($1) - ELSE true - END - AND CASE - WHEN $2::text != '' THEN - LOWER("name") = LOWER($2) - ELSE true - END + -- Optionally include deleted organizations + deleted = $1 + -- Filter by ids + AND CASE + WHEN array_length($2 :: uuid[], 1) > 0 THEN + id = ANY($2) + ELSE true + END + AND CASE + WHEN $3::text != '' THEN + LOWER("name") = LOWER($3) + ELSE true + END ` type GetOrganizationsParams struct { - IDs []uuid.UUID `db:"ids" json:"ids"` - Name string `db:"name" json:"name"` + Deleted bool `db:"deleted" json:"deleted"` + IDs []uuid.UUID `db:"ids" json:"ids"` + Name string `db:"name" json:"name"` } func (q *sqlQuerier) GetOrganizations(ctx context.Context, arg GetOrganizationsParams) ([]Organization, error) { - rows, err := q.db.QueryContext(ctx, getOrganizations, pq.Array(arg.IDs), arg.Name) + rows, err := q.db.QueryContext(ctx, getOrganizations, arg.Deleted, pq.Array(arg.IDs), arg.Name) if err != nil { return nil, err } @@ -5201,6 +5200,7 @@ func (q *sqlQuerier) GetOrganizations(ctx context.Context, arg GetOrganizationsP &i.IsDefault, &i.DisplayName, &i.Icon, + &i.Deleted, ); err != nil { return nil, err } @@ -5217,22 +5217,29 @@ func (q *sqlQuerier) GetOrganizations(ctx context.Context, arg GetOrganizationsP const getOrganizationsByUserID = `-- name: GetOrganizationsByUserID :many SELECT - id, name, description, created_at, updated_at, is_default, display_name, icon + id, name, description, created_at, updated_at, is_default, display_name, icon, deleted FROM - organizations + organizations WHERE - id = ANY( - SELECT - organization_id - FROM - organization_members - WHERE - user_id = $1 - ) + -- Optionally include deleted organizations + deleted = $2 AND + id = ANY( + SELECT + organization_id + FROM + organization_members + WHERE + user_id = $1 + ) ` -func (q *sqlQuerier) GetOrganizationsByUserID(ctx context.Context, userID uuid.UUID) ([]Organization, error) { - rows, err := q.db.QueryContext(ctx, getOrganizationsByUserID, userID) +type GetOrganizationsByUserIDParams struct { + UserID uuid.UUID `db:"user_id" json:"user_id"` + Deleted bool `db:"deleted" json:"deleted"` +} + +func (q *sqlQuerier) GetOrganizationsByUserID(ctx context.Context, arg GetOrganizationsByUserIDParams) ([]Organization, error) { + rows, err := q.db.QueryContext(ctx, getOrganizationsByUserID, arg.UserID, arg.Deleted) if err != nil { return nil, err } @@ -5249,6 +5256,7 @@ func (q *sqlQuerier) GetOrganizationsByUserID(ctx context.Context, userID uuid.U &i.IsDefault, &i.DisplayName, &i.Icon, + &i.Deleted, ); err != nil { return nil, err } @@ -5265,10 +5273,10 @@ func (q *sqlQuerier) GetOrganizationsByUserID(ctx context.Context, userID uuid.U const insertOrganization = `-- name: InsertOrganization :one INSERT INTO - organizations (id, "name", display_name, description, icon, created_at, updated_at, is_default) + organizations (id, "name", display_name, description, icon, created_at, updated_at, is_default) VALUES - -- If no organizations exist, and this is the first, make it the default. - ($1, $2, $3, $4, $5, $6, $7, (SELECT TRUE FROM organizations LIMIT 1) IS NULL) RETURNING id, name, description, created_at, updated_at, is_default, display_name, icon + -- If no organizations exist, and this is the first, make it the default. + ($1, $2, $3, $4, $5, $6, $7, (SELECT TRUE FROM organizations LIMIT 1) IS NULL) RETURNING id, name, description, created_at, updated_at, is_default, display_name, icon, deleted ` type InsertOrganizationParams struct { @@ -5301,22 +5309,23 @@ func (q *sqlQuerier) InsertOrganization(ctx context.Context, arg InsertOrganizat &i.IsDefault, &i.DisplayName, &i.Icon, + &i.Deleted, ) return i, err } const updateOrganization = `-- name: UpdateOrganization :one UPDATE - organizations + organizations SET - updated_at = $1, - name = $2, - display_name = $3, - description = $4, - icon = $5 + updated_at = $1, + name = $2, + display_name = $3, + description = $4, + icon = $5 WHERE - id = $6 -RETURNING id, name, description, created_at, updated_at, is_default, display_name, icon + id = $6 +RETURNING id, name, description, created_at, updated_at, is_default, display_name, icon, deleted ` type UpdateOrganizationParams struct { @@ -5347,10 +5356,31 @@ func (q *sqlQuerier) UpdateOrganization(ctx context.Context, arg UpdateOrganizat &i.IsDefault, &i.DisplayName, &i.Icon, + &i.Deleted, ) return i, err } +const updateOrganizationDeletedByID = `-- name: UpdateOrganizationDeletedByID :exec +UPDATE organizations +SET + deleted = true, + updated_at = $1 +WHERE + id = $2 AND + is_default = false +` + +type UpdateOrganizationDeletedByIDParams struct { + UpdatedAt time.Time `db:"updated_at" json:"updated_at"` + ID uuid.UUID `db:"id" json:"id"` +} + +func (q *sqlQuerier) UpdateOrganizationDeletedByID(ctx context.Context, arg UpdateOrganizationDeletedByIDParams) error { + _, err := q.db.ExecContext(ctx, updateOrganizationDeletedByID, arg.UpdatedAt, arg.ID) + return err +} + const getParameterSchemasByJobID = `-- name: GetParameterSchemasByJobID :many SELECT id, created_at, job_id, name, description, default_source_scheme, default_source_value, allow_override_source, default_destination_scheme, allow_override_destination, default_refresh, redisplay_value, validation_error, validation_condition, validation_type_system, validation_value_type, index diff --git a/coderd/database/queries/organizations.sql b/coderd/database/queries/organizations.sql index 3a74170a913e1..822b51c0aa8ba 100644 --- a/coderd/database/queries/organizations.sql +++ b/coderd/database/queries/organizations.sql @@ -1,89 +1,97 @@ -- name: GetDefaultOrganization :one SELECT - * + * FROM - organizations + organizations WHERE - is_default = true + is_default = true LIMIT - 1; + 1; -- name: GetOrganizations :many SELECT - * + * FROM - organizations + organizations WHERE - true - -- Filter by ids - AND CASE - WHEN array_length(@ids :: uuid[], 1) > 0 THEN - id = ANY(@ids) - ELSE true - END - AND CASE - WHEN @name::text != '' THEN - LOWER("name") = LOWER(@name) - ELSE true - END + -- Optionally include deleted organizations + deleted = @deleted + -- Filter by ids + AND CASE + WHEN array_length(@ids :: uuid[], 1) > 0 THEN + id = ANY(@ids) + ELSE true + END + AND CASE + WHEN @name::text != '' THEN + LOWER("name") = LOWER(@name) + ELSE true + END ; -- name: GetOrganizationByID :one SELECT - * + * FROM - organizations + organizations WHERE - id = $1; + id = $1; -- name: GetOrganizationByName :one SELECT - * + * FROM - organizations + organizations WHERE - LOWER("name") = LOWER(@name) + -- Optionally include deleted organizations + deleted = @deleted AND + LOWER("name") = LOWER(@name) LIMIT - 1; + 1; -- name: GetOrganizationsByUserID :many SELECT - * + * FROM - organizations + organizations WHERE - id = ANY( - SELECT - organization_id - FROM - organization_members - WHERE - user_id = $1 - ); + -- Optionally include deleted organizations + deleted = @deleted AND + id = ANY( + SELECT + organization_id + FROM + organization_members + WHERE + user_id = $1 + ); -- name: InsertOrganization :one INSERT INTO - organizations (id, "name", display_name, description, icon, created_at, updated_at, is_default) + organizations (id, "name", display_name, description, icon, created_at, updated_at, is_default) VALUES - -- If no organizations exist, and this is the first, make it the default. - (@id, @name, @display_name, @description, @icon, @created_at, @updated_at, (SELECT TRUE FROM organizations LIMIT 1) IS NULL) RETURNING *; + -- If no organizations exist, and this is the first, make it the default. + (@id, @name, @display_name, @description, @icon, @created_at, @updated_at, (SELECT TRUE FROM organizations LIMIT 1) IS NULL) RETURNING *; -- name: UpdateOrganization :one UPDATE - organizations + organizations SET - updated_at = @updated_at, - name = @name, - display_name = @display_name, - description = @description, - icon = @icon + updated_at = @updated_at, + name = @name, + display_name = @display_name, + description = @description, + icon = @icon WHERE - id = @id + id = @id RETURNING *; --- name: DeleteOrganization :exec -DELETE FROM - organizations +-- name: UpdateOrganizationDeletedByID :exec +UPDATE organizations +SET + deleted = true, + updated_at = @updated_at WHERE - id = $1 AND - is_default = false; + id = @id AND + is_default = false; + diff --git a/coderd/database/unique_constraint.go b/coderd/database/unique_constraint.go index ce427cf97c3bc..db68849777247 100644 --- a/coderd/database/unique_constraint.go +++ b/coderd/database/unique_constraint.go @@ -38,7 +38,6 @@ const ( UniqueOauth2ProviderAppsNameKey UniqueConstraint = "oauth2_provider_apps_name_key" // ALTER TABLE ONLY oauth2_provider_apps ADD CONSTRAINT oauth2_provider_apps_name_key UNIQUE (name); UniqueOauth2ProviderAppsPkey UniqueConstraint = "oauth2_provider_apps_pkey" // ALTER TABLE ONLY oauth2_provider_apps ADD CONSTRAINT oauth2_provider_apps_pkey PRIMARY KEY (id); UniqueOrganizationMembersPkey UniqueConstraint = "organization_members_pkey" // ALTER TABLE ONLY organization_members ADD CONSTRAINT organization_members_pkey PRIMARY KEY (organization_id, user_id); - UniqueOrganizationsName UniqueConstraint = "organizations_name" // ALTER TABLE ONLY organizations ADD CONSTRAINT organizations_name UNIQUE (name); UniqueOrganizationsPkey UniqueConstraint = "organizations_pkey" // ALTER TABLE ONLY organizations ADD CONSTRAINT organizations_pkey PRIMARY KEY (id); UniqueParameterSchemasJobIDNameKey UniqueConstraint = "parameter_schemas_job_id_name_key" // ALTER TABLE ONLY parameter_schemas ADD CONSTRAINT parameter_schemas_job_id_name_key UNIQUE (job_id, name); UniqueParameterSchemasPkey UniqueConstraint = "parameter_schemas_pkey" // ALTER TABLE ONLY parameter_schemas ADD CONSTRAINT parameter_schemas_pkey PRIMARY KEY (id); @@ -94,8 +93,7 @@ const ( UniqueWorkspacesPkey UniqueConstraint = "workspaces_pkey" // ALTER TABLE ONLY workspaces ADD CONSTRAINT workspaces_pkey PRIMARY KEY (id); UniqueIndexAPIKeyName UniqueConstraint = "idx_api_key_name" // CREATE UNIQUE INDEX idx_api_key_name ON api_keys USING btree (user_id, token_name) WHERE (login_type = 'token'::login_type); UniqueIndexCustomRolesNameLower UniqueConstraint = "idx_custom_roles_name_lower" // CREATE UNIQUE INDEX idx_custom_roles_name_lower ON custom_roles USING btree (lower(name)); - UniqueIndexOrganizationName UniqueConstraint = "idx_organization_name" // CREATE UNIQUE INDEX idx_organization_name ON organizations USING btree (name); - UniqueIndexOrganizationNameLower UniqueConstraint = "idx_organization_name_lower" // CREATE UNIQUE INDEX idx_organization_name_lower ON organizations USING btree (lower(name)); + UniqueIndexOrganizationNameLower UniqueConstraint = "idx_organization_name_lower" // CREATE UNIQUE INDEX idx_organization_name_lower ON organizations USING btree (lower(name)) WHERE (deleted = false); UniqueIndexProvisionerDaemonsOrgNameOwnerKey UniqueConstraint = "idx_provisioner_daemons_org_name_owner_key" // CREATE UNIQUE INDEX idx_provisioner_daemons_org_name_owner_key ON provisioner_daemons USING btree (organization_id, name, lower(COALESCE((tags ->> 'owner'::text), ''::text))); UniqueIndexUsersEmail UniqueConstraint = "idx_users_email" // CREATE UNIQUE INDEX idx_users_email ON users USING btree (email) WHERE (deleted = false); UniqueIndexUsersUsername UniqueConstraint = "idx_users_username" // CREATE UNIQUE INDEX idx_users_username ON users USING btree (username) WHERE (deleted = false); diff --git a/coderd/httpmw/organizationparam.go b/coderd/httpmw/organizationparam.go index a72b361b90d71..2eba0dcedf5b8 100644 --- a/coderd/httpmw/organizationparam.go +++ b/coderd/httpmw/organizationparam.go @@ -73,7 +73,10 @@ func ExtractOrganizationParam(db database.Store) func(http.Handler) http.Handler if err == nil { organization, dbErr = db.GetOrganizationByID(ctx, id) } else { - organization, dbErr = db.GetOrganizationByName(ctx, arg) + organization, dbErr = db.GetOrganizationByName(ctx, database.GetOrganizationByNameParams{ + Name: arg, + Deleted: false, + }) } } if httpapi.Is404Error(dbErr) { diff --git a/coderd/idpsync/organization.go b/coderd/idpsync/organization.go index 6f755529cdde7..87fd9af5e935d 100644 --- a/coderd/idpsync/organization.go +++ b/coderd/idpsync/organization.go @@ -97,7 +97,10 @@ func (s AGPLIDPSync) SyncOrganizations(ctx context.Context, tx database.Store, u return xerrors.Errorf("organization claims: %w", err) } - existingOrgs, err := tx.GetOrganizationsByUserID(ctx, user.ID) + existingOrgs, err := tx.GetOrganizationsByUserID(ctx, database.GetOrganizationsByUserIDParams{ + UserID: user.ID, + Deleted: false, + }) if err != nil { return xerrors.Errorf("failed to get user organizations: %w", err) } diff --git a/coderd/searchquery/search.go b/coderd/searchquery/search.go index 849dd7f584947..103dc80601ad9 100644 --- a/coderd/searchquery/search.go +++ b/coderd/searchquery/search.go @@ -258,7 +258,9 @@ func parseOrganization(ctx context.Context, db database.Store, parser *httpapi.Q if err == nil { return organizationID, nil } - organization, err := db.GetOrganizationByName(ctx, v) + organization, err := db.GetOrganizationByName(ctx, database.GetOrganizationByNameParams{ + Name: v, Deleted: false, + }) if err != nil { return uuid.Nil, xerrors.Errorf("organization %q either does not exist, or you are unauthorized to view it", v) } diff --git a/coderd/users.go b/coderd/users.go index 964f18724449a..5f8866903bc6f 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -1286,7 +1286,10 @@ func (api *API) organizationsByUser(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() user := httpmw.UserParam(r) - organizations, err := api.Database.GetOrganizationsByUserID(ctx, user.ID) + organizations, err := api.Database.GetOrganizationsByUserID(ctx, database.GetOrganizationsByUserIDParams{ + UserID: user.ID, + Deleted: false, + }) if errors.Is(err, sql.ErrNoRows) { err = nil organizations = []database.Organization{} @@ -1324,7 +1327,10 @@ func (api *API) organizationsByUser(rw http.ResponseWriter, r *http.Request) { func (api *API) organizationByUserAndName(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() organizationName := chi.URLParam(r, "organizationname") - organization, err := api.Database.GetOrganizationByName(ctx, organizationName) + organization, err := api.Database.GetOrganizationByName(ctx, database.GetOrganizationByNameParams{ + Name: organizationName, + Deleted: false, + }) if httpapi.Is404Error(err) { httpapi.ResourceNotFound(rw) return diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md index fdc372c034903..4ec303b388d49 100644 --- a/docs/CONTRIBUTING.md +++ b/docs/CONTRIBUTING.md @@ -159,17 +159,17 @@ Database migrations are managed with To add new migrations, use the following command: ```shell -./coderd/database/migrations/create_fixture.sh my name +./coderd/database/migrations/create_migration.sh my name /home/coder/src/coder/coderd/database/migrations/000070_my_name.up.sql /home/coder/src/coder/coderd/database/migrations/000070_my_name.down.sql ``` -Run "make gen" to generate models. - Then write queries into the generated `.up.sql` and `.down.sql` files and commit them into the repository. The down script should make a best-effort to retain as much data as possible. +Run `make gen` to generate models. + #### Database fixtures (for testing migrations) There are two types of fixtures that are used to test that migrations don't diff --git a/docs/admin/security/audit-logs.md b/docs/admin/security/audit-logs.md index 5c6a6e6a802a1..4817ea03f4bc5 100644 --- a/docs/admin/security/audit-logs.md +++ b/docs/admin/security/audit-logs.md @@ -23,7 +23,7 @@ We track the following resources: | NotificationsSettings
| |
FieldTracked
idfalse
notifier_pausedtrue
| | OAuth2ProviderApp
| |
FieldTracked
callback_urltrue
created_atfalse
icontrue
idfalse
nametrue
updated_atfalse
| | OAuth2ProviderAppSecret
| |
FieldTracked
app_idfalse
created_atfalse
display_secretfalse
hashed_secretfalse
idfalse
last_used_atfalse
secret_prefixfalse
| -| Organization
| |
FieldTracked
created_atfalse
descriptiontrue
display_nametrue
icontrue
idfalse
is_defaulttrue
nametrue
updated_attrue
| +| Organization
| |
FieldTracked
created_atfalse
deletedtrue
descriptiontrue
display_nametrue
icontrue
idfalse
is_defaulttrue
nametrue
updated_attrue
| | OrganizationSyncSettings
| |
FieldTracked
assign_defaulttrue
fieldtrue
mappingtrue
| | RoleSyncSettings
| |
FieldTracked
fieldtrue
mappingtrue
| | Template
write, delete | |
FieldTracked
active_version_idtrue
activity_bumptrue
allow_user_autostarttrue
allow_user_autostoptrue
allow_user_cancel_workspace_jobstrue
autostart_block_days_of_weektrue
autostop_requirement_days_of_weektrue
autostop_requirement_weekstrue
created_atfalse
created_bytrue
created_by_avatar_urlfalse
created_by_usernamefalse
default_ttltrue
deletedfalse
deprecatedtrue
descriptiontrue
display_nametrue
failure_ttltrue
group_acltrue
icontrue
idtrue
max_port_sharing_leveltrue
nametrue
organization_display_namefalse
organization_iconfalse
organization_idfalse
organization_namefalse
provisionertrue
require_active_versiontrue
time_til_dormanttrue
time_til_dormant_autodeletetrue
updated_atfalse
user_acltrue
| diff --git a/enterprise/audit/table.go b/enterprise/audit/table.go index b9367a6038e85..53f03dd60ae63 100644 --- a/enterprise/audit/table.go +++ b/enterprise/audit/table.go @@ -275,6 +275,7 @@ var auditableResourcesTypes = map[any]map[string]Action{ "id": ActionIgnore, "name": ActionTrack, "description": ActionTrack, + "deleted": ActionTrack, "created_at": ActionIgnore, "updated_at": ActionTrack, "is_default": ActionTrack, diff --git a/enterprise/coderd/audit_test.go b/enterprise/coderd/audit_test.go index d5616ea3888b9..271671491860d 100644 --- a/enterprise/coderd/audit_test.go +++ b/enterprise/coderd/audit_test.go @@ -75,10 +75,6 @@ func TestEnterpriseAuditLogs(t *testing.T) { require.Equal(t, int64(1), alogs.Count) require.Len(t, alogs.AuditLogs, 1) - require.Equal(t, &codersdk.MinimalOrganization{ - ID: o.ID, - }, alogs.AuditLogs[0].Organization) - // OrganizationID is deprecated, but make sure it is set. require.Equal(t, o.ID, alogs.AuditLogs[0].OrganizationID) diff --git a/enterprise/coderd/groups.go b/enterprise/coderd/groups.go index 8d5a7fceefaec..9771dd9800bb0 100644 --- a/enterprise/coderd/groups.go +++ b/enterprise/coderd/groups.go @@ -440,7 +440,10 @@ func (api *API) groups(rw http.ResponseWriter, r *http.Request) { parser := httpapi.NewQueryParamParser() // Organization selector can be an org ID or name filter.OrganizationID = parser.UUIDorName(r.URL.Query(), uuid.Nil, "organization", func(orgName string) (uuid.UUID, error) { - org, err := api.Database.GetOrganizationByName(ctx, orgName) + org, err := api.Database.GetOrganizationByName(ctx, database.GetOrganizationByNameParams{ + Name: orgName, + Deleted: false, + }) if err != nil { return uuid.Nil, xerrors.Errorf("organization %q not found", orgName) } diff --git a/enterprise/coderd/organizations.go b/enterprise/coderd/organizations.go index a7ec4050ee654..6cf91ec5b856a 100644 --- a/enterprise/coderd/organizations.go +++ b/enterprise/coderd/organizations.go @@ -150,7 +150,16 @@ func (api *API) deleteOrganization(rw http.ResponseWriter, r *http.Request) { return } - err := api.Database.DeleteOrganization(ctx, organization.ID) + err := api.Database.InTx(func(tx database.Store) error { + err := tx.UpdateOrganizationDeletedByID(ctx, database.UpdateOrganizationDeletedByIDParams{ + ID: organization.ID, + UpdatedAt: dbtime.Now(), + }) + if err != nil { + return xerrors.Errorf("delete organization: %w", err) + } + return nil + }, nil) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Internal error deleting organization.", @@ -204,7 +213,10 @@ func (api *API) postOrganizations(rw http.ResponseWriter, r *http.Request) { return } - _, err := api.Database.GetOrganizationByName(ctx, req.Name) + _, err := api.Database.GetOrganizationByName(ctx, database.GetOrganizationByNameParams{ + Name: req.Name, + Deleted: false, + }) if err == nil { httpapi.Write(ctx, rw, http.StatusConflict, codersdk.Response{ Message: "Organization already exists with that name.", diff --git a/site/e2e/tests/organizations.spec.ts b/site/e2e/tests/organizations.spec.ts index 5a1cf4ba82c0c..ff4f5ad993f19 100644 --- a/site/e2e/tests/organizations.spec.ts +++ b/site/e2e/tests/organizations.spec.ts @@ -52,5 +52,6 @@ test("create and delete organization", async ({ page }) => { const dialog = page.getByTestId("dialog"); await dialog.getByLabel("Name").fill(newName); await dialog.getByRole("button", { name: "Delete" }).click(); - await expect(page.getByText("Organization deleted.")).toBeVisible(); + await page.waitForTimeout(1000); + await expect(page.getByText("Organization deleted")).toBeVisible(); }); diff --git a/site/src/pages/OrganizationSettingsPage/OrganizationSettingsPage.tsx b/site/src/pages/OrganizationSettingsPage/OrganizationSettingsPage.tsx index 13c339dcc3c09..3ae72b701c851 100644 --- a/site/src/pages/OrganizationSettingsPage/OrganizationSettingsPage.tsx +++ b/site/src/pages/OrganizationSettingsPage/OrganizationSettingsPage.tsx @@ -1,9 +1,11 @@ +import { getErrorMessage } from "api/errors"; import { deleteOrganization, updateOrganization, } from "api/queries/organizations"; import { EmptyState } from "components/EmptyState/EmptyState"; import { displaySuccess } from "components/GlobalSnackbar/utils"; +import { displayError } from "components/GlobalSnackbar/utils"; import { useOrganizationSettings } from "modules/management/OrganizationSettingsLayout"; import type { FC } from "react"; import { useMutation, useQueryClient } from "react-query"; @@ -42,10 +44,14 @@ const OrganizationSettingsPage: FC = () => { navigate(`/organizations/${updatedOrganization.name}/settings`); displaySuccess("Organization settings updated."); }} - onDeleteOrganization={() => { - deleteOrganizationMutation.mutate(organization.id); - displaySuccess("Organization deleted."); - navigate("/organizations"); + onDeleteOrganization={async () => { + try { + await deleteOrganizationMutation.mutateAsync(organization.id); + displaySuccess("Organization deleted"); + navigate("/organizations"); + } catch (error) { + displayError(getErrorMessage(error, "Failed to delete organization")); + } }} /> ); diff --git a/site/src/pages/OrganizationSettingsPage/OrganizationSettingsPageView.tsx b/site/src/pages/OrganizationSettingsPage/OrganizationSettingsPageView.tsx index 08199c0d65f4f..8ca6c517b251e 100644 --- a/site/src/pages/OrganizationSettingsPage/OrganizationSettingsPageView.tsx +++ b/site/src/pages/OrganizationSettingsPage/OrganizationSettingsPageView.tsx @@ -146,7 +146,10 @@ export const OrganizationSettingsPageView: FC< { + await onDeleteOrganization(); + setIsDeleting(false); + }} onCancel={() => setIsDeleting(false)} entity="organization" name={organization.name}