From 17c486c5e668da641d2ed6db6dc1884eb921e15f Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 5 Mar 2024 14:06:35 -0600 Subject: [PATCH] chore: ensure default org always exists (#12412) * chore: ensure default org always exists First user just joins the org created by the migration --- cli/templatelist_test.go | 6 ++- coderd/database/dbauthz/dbauthz.go | 2 +- coderd/database/dbauthz/dbauthz_test.go | 5 ++- coderd/database/dbmem/dbmem.go | 11 +++++ .../000198_ensure_default_org.down.sql | 1 + .../000198_ensure_default_org.up.sql | 16 +++++++ coderd/database/querier_test.go | 15 ++----- coderd/users.go | 43 +++++++++++++------ coderd/users_test.go | 4 +- 9 files changed, 71 insertions(+), 32 deletions(-) create mode 100644 coderd/database/migrations/000198_ensure_default_org.down.sql create mode 100644 coderd/database/migrations/000198_ensure_default_org.up.sql diff --git a/cli/templatelist_test.go b/cli/templatelist_test.go index 98796a3906..3ce91da91b 100644 --- a/cli/templatelist_test.go +++ b/cli/templatelist_test.go @@ -87,6 +87,10 @@ func TestTemplateList(t *testing.T) { t.Parallel() client := coderdtest.New(t, &coderdtest.Options{}) owner := coderdtest.CreateFirstUser(t, client) + + org, err := client.Organization(context.Background(), owner.OrganizationID) + require.NoError(t, err) + templateAdmin, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID, rbac.RoleTemplateAdmin()) inv, root := clitest.New(t, "templates", "list") @@ -107,7 +111,7 @@ func TestTemplateList(t *testing.T) { require.NoError(t, <-errC) pty.ExpectMatch("No templates found in") - pty.ExpectMatch(coderdtest.FirstUserParams.Username) + pty.ExpectMatch(org.Name) pty.ExpectMatch("Create one:") }) } diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 9584b3b338..76f497e1e3 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -232,7 +232,7 @@ var ( rbac.ResourceGroup.Type: {rbac.ActionCreate, rbac.ActionUpdate}, rbac.ResourceRoleAssignment.Type: {rbac.ActionCreate, rbac.ActionDelete}, rbac.ResourceSystem.Type: {rbac.WildcardSymbol}, - rbac.ResourceOrganization.Type: {rbac.ActionCreate}, + rbac.ResourceOrganization.Type: {rbac.ActionCreate, rbac.ActionRead}, rbac.ResourceOrganizationMember.Type: {rbac.ActionCreate}, rbac.ResourceOrgRoleAssignment.Type: {rbac.ActionCreate}, rbac.ResourceProvisionerDaemon.Type: {rbac.ActionCreate, rbac.ActionUpdate}, diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index b6da423078..f457edde8e 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -568,7 +568,7 @@ func (s *MethodTestSuite) TestOrganization() { check.Args(o.ID).Asserts(o, rbac.ActionRead).Returns(o) })) s.Run("GetDefaultOrganization", s.Subtest(func(db database.Store, check *expects) { - o := dbgen.Organization(s.T(), db, database.Organization{}) + o, _ := db.GetDefaultOrganization(context.Background()) check.Args().Asserts(o, rbac.ActionRead).Returns(o) })) s.Run("GetOrganizationByName", s.Subtest(func(db database.Store, check *expects) { @@ -597,9 +597,10 @@ func (s *MethodTestSuite) TestOrganization() { check.Args(u.ID).Asserts(a, rbac.ActionRead, b, rbac.ActionRead).Returns(slice.New(a, b)) })) s.Run("GetOrganizations", s.Subtest(func(db database.Store, check *expects) { + def, _ := db.GetDefaultOrganization(context.Background()) a := dbgen.Organization(s.T(), db, database.Organization{}) b := dbgen.Organization(s.T(), db, database.Organization{}) - check.Args().Asserts(a, rbac.ActionRead, b, rbac.ActionRead).Returns(slice.New(a, b)) + check.Args().Asserts(def, rbac.ActionRead, a, rbac.ActionRead, b, rbac.ActionRead).Returns(slice.New(def, a, b)) })) s.Run("GetOrganizationsByUserID", s.Subtest(func(db database.Store, check *expects) { u := dbgen.User(s.T(), db, database.User{}) diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index cc4edf90c2..1753362c30 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -77,6 +77,17 @@ func New() database.Store { locks: map[int64]struct{}{}, }, } + // Always start with a default org. Matching migration 198. + _, err := q.InsertOrganization(context.Background(), database.InsertOrganizationParams{ + ID: uuid.New(), + Name: "first-organization", + Description: "Builtin default organization.", + CreatedAt: dbtime.Now(), + UpdatedAt: dbtime.Now(), + }) + if err != nil { + panic(fmt.Errorf("failed to create default organization: %w", err)) + } q.defaultProxyDisplayName = "Default" q.defaultProxyIconURL = "/emojis/1f3e1.png" return q diff --git a/coderd/database/migrations/000198_ensure_default_org.down.sql b/coderd/database/migrations/000198_ensure_default_org.down.sql new file mode 100644 index 0000000000..ad2c1fbca0 --- /dev/null +++ b/coderd/database/migrations/000198_ensure_default_org.down.sql @@ -0,0 +1 @@ +-- There is no down. If the org is created, just let it be. Deleting an org feels dangerous in a migration. diff --git a/coderd/database/migrations/000198_ensure_default_org.up.sql b/coderd/database/migrations/000198_ensure_default_org.up.sql new file mode 100644 index 0000000000..4a773753fc --- /dev/null +++ b/coderd/database/migrations/000198_ensure_default_org.up.sql @@ -0,0 +1,16 @@ +-- This ensures a default organization always exists. +INSERT INTO + organizations(id, name, description, created_at, updated_at, is_default) +SELECT + -- Avoid calling it "default" as we are reserving that word as a keyword to fetch + -- the default org regardless of the name. + gen_random_uuid(), + 'first-organization', + 'Builtin default organization.', + now(), + now(), + true +WHERE + -- Only insert if no organizations exist. + NOT EXISTS (SELECT * FROM organizations); + diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index 0f7b0cd95d..ae64260db3 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -506,20 +506,11 @@ func TestDefaultOrg(t *testing.T) { db := database.New(sqlDB) ctx := context.Background() - // Should start with 0 orgs + // Should start with the default org all, err := db.GetOrganizations(ctx) require.NoError(t, err) - require.Len(t, all, 0) - - org, err := db.InsertOrganization(ctx, database.InsertOrganizationParams{ - ID: uuid.New(), - Name: "default", - Description: "", - CreatedAt: dbtime.Now(), - UpdatedAt: dbtime.Now(), - }) - require.NoError(t, err) - require.True(t, org.IsDefault, "first org should always be default") + require.Len(t, all, 1) + require.True(t, all[0].IsDefault, "first org should always be default") } type tvArgs struct { diff --git a/coderd/users.go b/coderd/users.go index cbc9a75059..efdd351b51 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -171,16 +171,37 @@ func (api *API) postFirstUser(rw http.ResponseWriter, r *http.Request) { } } + //nolint:gocritic // needed to create first user + defaultOrg, err := api.Database.GetDefaultOrganization(dbauthz.AsSystemRestricted(ctx)) + if err != nil { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error fetching default organization. If you are encountering this error, you will have to restart the Coder deployment.", + Detail: err.Error(), + }) + return + } + + //nolint:gocritic // ensure everyone group + _, err = api.Database.InsertAllUsersGroup(dbauthz.AsSystemRestricted(ctx), defaultOrg.ID) + // A unique constraint violation just means the group already exists. + // This should not happen, but is ok if it does. + if err != nil && !database.IsUniqueViolation(err) { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error creating all users group.", + Detail: err.Error(), + }) + return + } + //nolint:gocritic // needed to create first user user, organizationID, err := api.CreateUser(dbauthz.AsSystemRestricted(ctx), api.Database, CreateUserRequest{ CreateUserRequest: codersdk.CreateUserRequest{ - Email: createUser.Email, - Username: createUser.Username, - Password: createUser.Password, - // Create an org for the first user. - OrganizationID: uuid.Nil, + Email: createUser.Email, + Username: createUser.Username, + Password: createUser.Password, + OrganizationID: defaultOrg.ID, }, - CreateOrganization: true, + CreateOrganization: false, LoginType: database.LoginTypePassword, }) if err != nil { @@ -1033,10 +1054,7 @@ func (api *API) userRoles(rw http.ResponseWriter, r *http.Request) { } for _, mem := range memberships { - // If we can read the org member, include the roles. - if err == nil { - resp.OrganizationRoles[mem.OrganizationID] = mem.Roles - } + resp.OrganizationRoles[mem.OrganizationID] = mem.Roles } httpapi.Write(ctx, rw, http.StatusOK, resp) @@ -1247,9 +1265,8 @@ func (api *API) CreateUser(ctx context.Context, store database.Store, req Create // TODO: When organizations are allowed to be created, we should // come back to determining the default role of the person who // creates the org. Until that happens, all users in an organization - // should be just regular members. - orgRoles = append(orgRoles, rbac.RoleOrgMember(req.OrganizationID)) - + // should be just regular members. Membership role is implied, and + // not required to be explicit. _, err = tx.InsertAllUsersGroup(ctx, organization.ID) if err != nil { return xerrors.Errorf("create %q group: %w", database.EveryoneGroup, err) diff --git a/coderd/users_test.go b/coderd/users_test.go index 1c962a50b9..6ddbf089cd 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -1093,9 +1093,7 @@ func TestInitialRoles(t *testing.T) { rbac.RoleOwner(), }, "should be a member and admin") - require.ElementsMatch(t, roles.OrganizationRoles[first.OrganizationID], []string{ - rbac.RoleOrgMember(first.OrganizationID), - }, "should be a member and admin") + require.ElementsMatch(t, roles.OrganizationRoles[first.OrganizationID], []string{}, "should be a member") } func TestPutUserSuspend(t *testing.T) {