Skip to content

Commit 5def11f

Browse files
migmartrimigmartri
authored
fix: do not create org during auto-onboarding (#986)
Signed-off-by: Miguel Martinez Trivino <[email protected]>
1 parent 408f623 commit 5def11f

File tree

6 files changed

+95
-110
lines changed

6 files changed

+95
-110
lines changed

‎app/controlplane/configs/samples/config.yaml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,6 @@ referrer_shared_index:
6565

6666
onboarding:
6767
- name: "read-only-demo"
68-
role: "viewer"
68+
role: MEMBERSHIP_ROLE_ORG_VIEWER
6969
- name: "read-write-demo"
70-
role: "owner"
70+
role: MEMBERSHIP_ROLE_ORG_OWNER

‎app/controlplane/internal/conf/controlplane/config/v1/conf.pb.go‎

Lines changed: 11 additions & 10 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎app/controlplane/internal/conf/controlplane/config/v1/conf.proto‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,5 +156,10 @@ message OnboardingSpec {
156156
// Name of the organization
157157
string name = 1 [(buf.validate.field).string.min_len = 1];
158158
// Role to assign to the user
159-
controlplane.v1.MembershipRole role = 2;
159+
controlplane.v1.MembershipRole role = 2 [
160+
(buf.validate.field).enum = {
161+
not_in: [0]
162+
},
163+
(buf.validate.field).enum.defined_only = true
164+
];
160165
}

‎app/controlplane/pkg/biz/organization.go‎

Lines changed: 19 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,12 @@ import (
1919
"context"
2020
"errors"
2121
"fmt"
22+
"io"
2223
"time"
2324

25+
"github.com/chainloop-dev/chainloop/app/controlplane/internal/authz"
2426
conf "github.com/chainloop-dev/chainloop/app/controlplane/internal/conf/controlplane/config/v1"
27+
"github.com/chainloop-dev/chainloop/pkg/servicelogger"
2528
"github.com/go-kratos/kratos/v2/log"
2629
"github.com/google/uuid"
2730
"github.com/moby/moby/pkg/namesgenerator"
@@ -49,9 +52,13 @@ type OrganizationUseCase struct {
4952
onboardingConfig []*conf.OnboardingSpec
5053
}
5154

52-
func NewOrganizationUseCase(repo OrganizationRepo, repoUC *CASBackendUseCase, iUC *IntegrationUseCase, mRepo MembershipRepo, onboardingConfig []*conf.OnboardingSpec, logger log.Logger) *OrganizationUseCase {
55+
func NewOrganizationUseCase(repo OrganizationRepo, repoUC *CASBackendUseCase, iUC *IntegrationUseCase, mRepo MembershipRepo, onboardingConfig []*conf.OnboardingSpec, l log.Logger) *OrganizationUseCase {
56+
if l == nil {
57+
l = log.NewStdLogger(io.Discard)
58+
}
59+
5360
return &OrganizationUseCase{orgRepo: repo,
54-
logger: log.NewHelper(logger),
61+
logger: servicelogger.ScopedHelper(l, "biz/organization"),
5562
casBackendUseCase: repoUC,
5663
integrationUC: iUC,
5764
membershipRepo: mRepo,
@@ -259,10 +266,13 @@ func (uc *OrganizationUseCase) AutoOnboardOrganizations(ctx context.Context, use
259266
}
260267

261268
for _, spec := range uc.onboardingConfig {
262-
// Ensure the organization exists or create it if it doesn't
263-
org, err := uc.ensureOrganizationExists(ctx, spec)
269+
// Find organization or skip onboarding if it doesn't exist
270+
org, err := uc.orgRepo.FindByName(ctx, spec.GetName())
264271
if err != nil {
265-
return fmt.Errorf("failed to ensure organization exists: %w", err)
272+
return fmt.Errorf("failed to find organization: %w", err)
273+
} else if org == nil {
274+
uc.logger.Infow("msg", "Organization not found", "name", spec.GetName())
275+
continue
266276
}
267277

268278
// Parse organization UUID
@@ -272,40 +282,17 @@ func (uc *OrganizationUseCase) AutoOnboardOrganizations(ctx context.Context, use
272282
}
273283

274284
// Ensure user membership
275-
if err := uc.ensureUserMembership(ctx, orgUUID, userUUID, spec); err != nil {
285+
if err := uc.ensureUserMembership(ctx, orgUUID, userUUID, PbRoleToBiz(spec.GetRole())); err != nil {
276286
return fmt.Errorf("failed to ensure user membership: %w", err)
277287
}
278288
}
279289

280290
return nil
281291
}
282292

283-
// ensureOrganizationExists ensures that an organization specified by the onboarding configuration exists.
284-
// If the organization does not exist, it creates it.
285-
func (uc *OrganizationUseCase) ensureOrganizationExists(ctx context.Context, spec *conf.OnboardingSpec) (*Organization, error) {
286-
// Ensure the organization exists or create it if it doesn't
287-
org, err := uc.orgRepo.FindByName(ctx, spec.GetName())
288-
if err != nil {
289-
return nil, fmt.Errorf("failed to find organization: %w", err)
290-
} else if org == nil {
291-
// Create the organization since it does not exist
292-
org, err = uc.orgRepo.Create(ctx, spec.GetName())
293-
if err != nil {
294-
return nil, fmt.Errorf("failed to create organization: %w", err)
295-
}
296-
297-
// Create default inline CAS-backend
298-
if _, err := uc.casBackendUseCase.CreateInlineFallbackBackend(ctx, org.ID); err != nil {
299-
return nil, fmt.Errorf("failed to create fallback backend: %w", err)
300-
}
301-
}
302-
303-
return org, nil
304-
}
305-
306293
// ensureUserMembership ensures that a user is a member of the specified organization with the appropriate role.
307294
// If the membership does not exist, it creates it.
308-
func (uc *OrganizationUseCase) ensureUserMembership(ctx context.Context, orgUUID, userUUID uuid.UUID, spec *conf.OnboardingSpec) error {
295+
func (uc *OrganizationUseCase) ensureUserMembership(ctx context.Context, orgUUID, userUUID uuid.UUID, role authz.Role) error {
309296
m, err := uc.membershipRepo.FindByOrgAndUser(ctx, orgUUID, userUUID)
310297
if err != nil {
311298
return fmt.Errorf("failed to find membership: %w", err)
@@ -317,10 +304,11 @@ func (uc *OrganizationUseCase) ensureUserMembership(ctx context.Context, orgUUID
317304
}
318305

319306
// Create the membership with the specified role
320-
_, err = uc.membershipRepo.Create(ctx, orgUUID, userUUID, true, PbRoleToBiz(spec.GetRole()))
307+
_, err = uc.membershipRepo.Create(ctx, orgUUID, userUUID, true, role)
321308
if err != nil {
322309
return fmt.Errorf("failed to create membership: %w", err)
323310
}
324311

312+
uc.logger.Infow("msg", "User auto-onboarded to organization", "org", orgUUID, "user", userUUID, "role", role)
325313
return nil
326314
}

‎app/controlplane/pkg/biz/organization_integration_test.go‎

Lines changed: 49 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -275,9 +275,9 @@ func (s *OrgIntegrationTestSuite) SetupTest() {
275275

276276
type AuthOnboardingTestSuite struct {
277277
testhelpers.UseCasesEachTestSuite
278-
usr, usr1 *biz.User
279-
org *biz.Organization
280-
m *biz.Membership
278+
userWithoutOrg, userInOrg *biz.User
279+
existingOrg *biz.Organization
280+
m *biz.Membership
281281
}
282282

283283
func (s *AuthOnboardingTestSuite) SetupTest() {
@@ -286,109 +286,100 @@ func (s *AuthOnboardingTestSuite) SetupTest() {
286286

287287
s.TestingUseCases = testhelpers.NewTestingUseCases(t, testhelpers.WithOnboardingConfiguration([]*conf.OnboardingSpec{
288288
{
289-
Name: "testing-org",
289+
Name: "non-existing-org",
290290
Role: v1.MembershipRole_MEMBERSHIP_ROLE_ORG_VIEWER,
291291
},
292+
{
293+
Name: "existing-org",
294+
Role: v1.MembershipRole_MEMBERSHIP_ROLE_ORG_OWNER,
295+
},
292296
}))
293297

294-
s.setupUsersAndOrganization(ctx)
295-
s.setupMembership(ctx)
296-
}
297-
298-
func (s *AuthOnboardingTestSuite) setupUsersAndOrganization(ctx context.Context) {
298+
// Create org and two users
299299
var err error
300-
s.org, err = s.Organization.Create(ctx, "onboarded-org")
300+
s.existingOrg, err = s.Organization.Create(ctx, "existing-org")
301301
require.NoError(s.T(), err)
302302

303-
s.usr, err = s.User.FindOrCreateByEmail(ctx, "foo@bar", true)
303+
s.userWithoutOrg, err = s.User.FindOrCreateByEmail(ctx, "foo@bar", true)
304304
require.NoError(s.T(), err)
305305

306-
s.usr1, err = s.User.FindOrCreateByEmail(ctx, "bar@foo", true)
306+
s.userInOrg, err = s.User.FindOrCreateByEmail(ctx, "bar@foo", true)
307307
require.NoError(s.T(), err)
308-
}
309308

310-
func (s *AuthOnboardingTestSuite) setupMembership(ctx context.Context) {
311-
var err error
312-
s.m, err = s.Membership.Create(ctx, s.org.ID, s.usr1.ID, biz.WithMembershipRole(authz.RoleViewer))
309+
// usr1 is already a member of the org
310+
s.m, err = s.Membership.Create(ctx, s.existingOrg.ID, s.userInOrg.ID, biz.WithMembershipRole(authz.RoleViewer))
313311
s.NoError(err)
314312
}
315313

314+
// User without org only gets attached to the existing org
316315
func (s *AuthOnboardingTestSuite) TestAutoOnboardOrganizations() {
317316
ctx := context.Background()
318317

319-
org, err := s.Repos.OrganizationRepo.FindByName(ctx, "testing-org")
320-
s.Nil(err)
321-
s.Nil(org)
322-
323-
err = s.Organization.AutoOnboardOrganizations(ctx, s.usr.ID)
318+
// User has no memberships
319+
memberships, err := s.Membership.ByUser(ctx, s.userWithoutOrg.ID)
324320
s.NoError(err)
321+
s.Len(memberships, 0)
325322

326-
org, err = s.Repos.OrganizationRepo.FindByName(ctx, "testing-org")
323+
// Auto onboard
324+
err = s.Organization.AutoOnboardOrganizations(ctx, s.userWithoutOrg.ID)
327325
s.NoError(err)
328-
s.NotNil(org)
329326

330-
m, err := s.Membership.FindByOrgAndUser(ctx, org.ID, s.usr.ID)
327+
// User has now 1 membership that points to the existing org
328+
memberships, err = s.Membership.ByUser(ctx, s.userWithoutOrg.ID)
331329
s.NoError(err)
332-
s.NotNil(m)
330+
s.Len(memberships, 1)
331+
s.Equal(s.existingOrg.ID, memberships[0].OrganizationID.String())
332+
s.Equal(authz.RoleOwner, memberships[0].Role)
333333
}
334334

335-
func (s *AuthOnboardingTestSuite) TestOnboardOrganizationsTwice() {
335+
func (s *AuthOnboardingTestSuite) TestAutoOnboardNoOrganizationsCreated() {
336336
ctx := context.Background()
337337

338-
org, err := s.Repos.OrganizationRepo.FindByName(ctx, "testing-org")
339-
s.Nil(err)
338+
org, err := s.Repos.OrganizationRepo.FindByName(ctx, "non-existing-org")
339+
s.NoError(err)
340340
s.Nil(org)
341341

342-
// Call it once
343-
err = s.Organization.AutoOnboardOrganizations(ctx, s.usr.ID)
342+
// Auto onboard
343+
err = s.Organization.AutoOnboardOrganizations(ctx, s.userWithoutOrg.ID)
344344
s.NoError(err)
345345

346-
// Call it twice
347-
err = s.Organization.AutoOnboardOrganizations(ctx, s.usr.ID)
346+
// The org has not been created
347+
org, err = s.Repos.OrganizationRepo.FindByName(ctx, "non-existing-org")
348348
s.NoError(err)
349+
s.Nil(org)
350+
}
349351

350-
org, err = s.Repos.OrganizationRepo.FindByName(ctx, "testing-org")
352+
func (s *AuthOnboardingTestSuite) TestOnboardOrganizationsTwice() {
353+
ctx := context.Background()
354+
355+
// Auto onboard
356+
err := s.Organization.AutoOnboardOrganizations(ctx, s.userWithoutOrg.ID)
357+
s.NoError(err)
358+
// Auto onboard again
359+
err = s.Organization.AutoOnboardOrganizations(ctx, s.userWithoutOrg.ID)
351360
s.NoError(err)
352-
s.NotNil(org)
353361

354-
m, err := s.Membership.FindByOrgAndUser(ctx, org.ID, s.usr.ID)
362+
// User has now 1 membership that points to the existing org
363+
memberships, err := s.Membership.ByUser(ctx, s.userWithoutOrg.ID)
355364
s.NoError(err)
356-
s.NotNil(m)
365+
s.Len(memberships, 1)
357366
}
358367

359368
func (s *AuthOnboardingTestSuite) TestAutoOnboardWithExistingMemberships() {
360369
ctx := context.Background()
361370

362-
org, err := s.Repos.OrganizationRepo.FindByName(ctx, s.org.Name)
363-
s.Nil(err)
364-
s.NotNil(org)
365-
366-
m, err := s.Membership.FindByOrgAndUser(ctx, org.ID, s.usr1.ID)
371+
err := s.Organization.AutoOnboardOrganizations(ctx, s.userInOrg.ID)
367372
s.NoError(err)
368-
s.NotNil(m)
369-
s.Equal(s.m.Role, m.Role)
370373

371-
err = s.Organization.AutoOnboardOrganizations(ctx, s.usr1.ID)
374+
got, err := s.Membership.FindByOrgAndUser(ctx, s.existingOrg.ID, s.userInOrg.ID)
372375
s.NoError(err)
373-
374-
newM, err := s.Membership.FindByOrgAndUser(ctx, org.ID, s.usr1.ID)
375-
s.NoError(err)
376-
s.NotNil(newM)
377-
s.Equal(s.m.Role, newM.Role)
376+
s.Equal(s.m, got)
378377
}
379378

380379
func (s *AuthOnboardingTestSuite) TestAutoOnboardWithoutConfiguration() {
381380
ctx := context.Background()
382381
s.TestingUseCases = testhelpers.NewTestingUseCases(s.T())
383382

384-
org, err := s.Repos.OrganizationRepo.FindByName(ctx, "testing-org")
385-
s.Nil(err)
386-
s.Nil(org)
387-
388-
err = s.Organization.AutoOnboardOrganizations(ctx, s.usr.ID)
383+
err := s.Organization.AutoOnboardOrganizations(ctx, s.userWithoutOrg.ID)
389384
s.NoError(err)
390-
391-
org, err = s.Repos.OrganizationRepo.FindByName(ctx, "testing-org")
392-
s.Nil(err)
393-
s.Nil(org)
394385
}

‎app/controlplane/pkg/biz/user_integration_test.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"github.com/chainloop-dev/chainloop/app/controlplane/pkg/biz/testhelpers"
2727

2828
"github.com/stretchr/testify/assert"
29+
"github.com/stretchr/testify/require"
2930
"github.com/stretchr/testify/suite"
3031
)
3132

@@ -137,26 +138,25 @@ func (s *userOnboardingTestSuite) TestAutoOnboardOrganizationsNoConfiguration()
137138

138139
func (s *userOnboardingTestSuite) TestAutoOnboardOrganizationsWithConfiguration() {
139140
ctx := context.Background()
140-
// Create a user with no orgs
141-
141+
const orgName = "existing-org"
142142
s.TestingUseCases = testhelpers.NewTestingUseCases(s.T(), testhelpers.WithOnboardingConfiguration([]*conf.OnboardingSpec{
143143
{
144-
Name: "testing-org",
144+
Name: orgName,
145145
Role: v1.MembershipRole_MEMBERSHIP_ROLE_ORG_VIEWER,
146146
},
147147
}))
148148

149-
org, err := s.Repos.OrganizationRepo.FindByName(ctx, "testing-org")
150-
s.Nil(err)
151-
s.Nil(org)
149+
// The user got onboarded in the existing org
150+
org, err := s.Organization.Create(ctx, orgName)
151+
require.NoError(s.T(), err)
152152

153153
user, err := s.User.FindOrCreateByEmail(ctx, "[email protected]")
154154
s.NoError(err)
155155
s.NotNil(user)
156156

157-
org, err = s.Repos.OrganizationRepo.FindByName(ctx, "testing-org")
157+
m, err := s.Membership.FindByOrgAndUser(ctx, org.ID, user.ID)
158158
s.NoError(err)
159-
s.NotNil(org)
159+
s.NotNil(m)
160160
}
161161

162162
// Utility struct to hold the test suite

0 commit comments

Comments
 (0)