Skip to content

Commit

Permalink
fixup test failures
Browse files Browse the repository at this point in the history
Signed-off-by: Sarah Funkhouser <147884153+golanglemonade@users.noreply.github.com>
  • Loading branch information
golanglemonade committed Jan 2, 2025
1 parent 860758d commit f922c72
Show file tree
Hide file tree
Showing 15 changed files with 96 additions and 29 deletions.
6 changes: 4 additions & 2 deletions internal/ent/hooks/invite.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/theopenlane/core/internal/ent/generated/hook"
"github.com/theopenlane/core/internal/ent/generated/invite"
"github.com/theopenlane/core/internal/ent/generated/organization"
"github.com/theopenlane/core/internal/ent/generated/privacy"
"github.com/theopenlane/core/pkg/enums"
)

Expand Down Expand Up @@ -126,8 +127,9 @@ func HookInviteAccepted() ent.Hook {
Role: &role,
}

// add user to the inviting org
if _, err := m.Client().OrgMembership.Create().SetInput(input).Save(ctx); err != nil {
// add user to the inviting org, allow the context to bypass privacy checks
allowCtx := privacy.DecisionContext(ctx, privacy.Allow)
if _, err := m.Client().OrgMembership.Create().SetInput(input).Save(allowCtx); err != nil {
log.Error().Err(err).Msg("unable to add user to organization")

return nil, err
Expand Down
10 changes: 8 additions & 2 deletions internal/ent/hooks/managedgroups.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,10 +169,13 @@ func removeFromManagedGroups(ctx context.Context, m *generated.OrgMembershipMuta

// addMemberToManagedGroup adds the user to the system managed groups
func addMemberToManagedGroup(ctx context.Context, m *generated.OrgMembershipMutation, userID, groupName string) error {
orgID, _ := m.OrganizationID()

// get the group to update
groupID, err := m.Client().Group.Query().Where(
group, err := m.Client().Group.Query().Where(
group.IsManaged(true), // grab the managed group
group.Name(groupName),
group.OwnerID(orgID),
).Only(ctx)
if err != nil {
log.Error().Err(err).Msgf("error getting managed group: %s", groupName)
Expand All @@ -182,7 +185,7 @@ func addMemberToManagedGroup(ctx context.Context, m *generated.OrgMembershipMuta
input := generated.CreateGroupMembershipInput{
Role: &enums.RoleMember,
UserID: userID,
GroupID: groupID.ID,
GroupID: group.ID,
}

if err := m.Client().GroupMembership.Create().SetInput(input).Exec(ctx); err != nil {
Expand All @@ -196,10 +199,13 @@ func addMemberToManagedGroup(ctx context.Context, m *generated.OrgMembershipMuta
}

func removeMemberFromManagedGroup(ctx context.Context, m *generated.OrgMembershipMutation, userID, groupName string) error {
orgID, _ := m.OrganizationID()

// get the group to update
group, err := m.Client().Group.Query().Where(
group.IsManaged(true), // grab the managed group
group.Name(groupName),
group.OwnerID(orgID),
).Only(ctx)
if err != nil {
log.Error().Err(err).Msgf("error getting managed group: %s", groupName)
Expand Down
2 changes: 2 additions & 0 deletions internal/ent/interceptors/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package interceptors
var (
// ExistOperation is the operation type for Exist queries
ExistOperation = "Exist"
// OnlyOperation is the operation type for Only queries
OnlyOperation = "Only"
// IDsOperation is the operation type for IDs queries
IDsOperation = "IDs"
)
7 changes: 7 additions & 0 deletions internal/ent/interceptors/group.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@ import (

"github.com/theopenlane/iam/auth"
"github.com/theopenlane/iam/fgax"
"github.com/theopenlane/utils/contextx"

"github.com/theopenlane/core/internal/ent/generated"
"github.com/theopenlane/core/internal/ent/generated/intercept"
"github.com/theopenlane/core/internal/ent/hooks"
)

// InterceptorGroup is middleware to change the Group query
Expand Down Expand Up @@ -66,6 +68,11 @@ func filterGroupsByAccess(ctx context.Context, q *generated.GroupQuery, v ent.Va
}
}

_, managedGroup := contextx.From[hooks.ManagedContextKey](ctx)
if qc.Op == OnlyOperation && managedGroup {
return v.([]*generated.Group), nil
}

// get userID for tuple checks
userID, err := auth.GetUserIDFromContext(ctx)
if err != nil {
Expand Down
7 changes: 7 additions & 0 deletions internal/ent/schema/mixin_orgowned.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@ import (
"entgo.io/ent/dialect/sql"

"github.com/theopenlane/iam/auth"
"github.com/theopenlane/utils/contextx"

"github.com/theopenlane/core/internal/ent/generated"
"github.com/theopenlane/core/internal/ent/generated/intercept"
"github.com/theopenlane/core/internal/ent/hooks"
"github.com/theopenlane/core/internal/ent/interceptors"
"github.com/theopenlane/core/internal/ent/privacy/rule"
)
Expand Down Expand Up @@ -125,6 +127,11 @@ var defaultOrgInterceptorFunc InterceptorFunc = func(o ObjectOwnedMixin) ent.Int
}
}

// skip interceptor if the context has the managed group key
if _, managedGroup := contextx.From[hooks.ManagedContextKey](ctx); managedGroup {
return nil
}

// check query context skips
ctxQuery := ent.QueryFromContext(ctx)

Expand Down
2 changes: 1 addition & 1 deletion internal/graphapi/control_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ func (suite *GraphTestSuite) TestMutationUpdateControl() {
// create another admin user and add them to the same organization and group as testUser1
// this will allow us to test the group editor/viewer permissions
anotherAdminUser := suite.userBuilder(context.Background())
suite.addUserToOrganization(&anotherAdminUser, enums.RoleAdmin, testUser1.OrganizationID)
suite.addUserToOrganization(testUser1.UserCtx, &anotherAdminUser, enums.RoleAdmin, testUser1.OrganizationID)

(&GroupMemberBuilder{client: suite.client, UserID: anotherAdminUser.ID, GroupID: testUser1.GroupID}).MustNew(testUser1.UserCtx, t)

Expand Down
2 changes: 1 addition & 1 deletion internal/graphapi/controlobjective_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,7 @@ func (suite *GraphTestSuite) TestMutationUpdateControlObjective() {
// create another admin user and add them to the same organization and group as testUser1
// this will allow us to test the group editor/viewer permissions
anotherAdminUser := suite.userBuilder(context.Background())
suite.addUserToOrganization(&anotherAdminUser, enums.RoleAdmin, testUser1.OrganizationID)
suite.addUserToOrganization(testUser1.UserCtx, &anotherAdminUser, enums.RoleAdmin, testUser1.OrganizationID)

(&GroupMemberBuilder{client: suite.client, UserID: anotherAdminUser.ID, GroupID: testUser1.GroupID}).MustNew(testUser1.UserCtx, t)

Expand Down
48 changes: 44 additions & 4 deletions internal/graphapi/group_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"testing"

"github.com/brianvoe/gofakeit/v7"
"github.com/samber/lo"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

Expand Down Expand Up @@ -89,6 +90,7 @@ func (suite *GraphTestSuite) TestQueryGroupsByOwner() {
ID: &org1.ID,
},
},
IsManaged: lo.ToPtr(false),
}

resp, err := suite.client.api.GetGroups(reqCtx, whereInput)
Expand Down Expand Up @@ -121,6 +123,7 @@ func (suite *GraphTestSuite) TestQueryGroupsByOwner() {
ID: &org2.ID,
},
},
IsManaged: lo.ToPtr(false),
}

resp, err = suite.client.api.GetGroups(reqCtx2, whereInput)
Expand Down Expand Up @@ -148,8 +151,8 @@ func (suite *GraphTestSuite) TestQueryGroups() {
require.NotNil(t, resp)
require.NotNil(t, resp.Groups.Edges)

// make sure two organizations are returned (group 2 and group 3) and the seeded group
assert.Equal(t, 3, len(resp.Groups.Edges))
// make sure two organizations are returned (group 2 and group 3), the seeded group, and the 3 managed groups
assert.Equal(t, 6, len(resp.Groups.Edges))

group1Found := false
group2Found := false
Expand Down Expand Up @@ -178,8 +181,8 @@ func (suite *GraphTestSuite) TestQueryGroups() {
require.NoError(t, err)
require.NotNil(t, resp)

// make sure only two groups are returned, group 1 and the seeded group
assert.Equal(t, 2, len(resp.Groups.Edges))
// make sure only two groups are returned, group 1 and the seeded group, and the 3 managed groups
assert.Equal(t, 5, len(resp.Groups.Edges))
})
}

Expand Down Expand Up @@ -549,3 +552,40 @@ func (suite *GraphTestSuite) TestMutationDeleteGroup() {
})
}
}

func (suite *GraphTestSuite) TestManagedGroups() {
t := suite.T()
whereInput := &openlaneclient.GroupWhereInput{
IsManaged: lo.ToPtr(true),
}

resp, err := suite.client.api.GetGroups(testUser1.UserCtx, whereInput)
require.NoError(t, err)
require.NotNil(t, resp)

// there should be 3 managed groups created by the system on org creation
assert.Len(t, resp.Groups.Edges, 3)

// you should not be able to update a managed group
groupID := resp.Groups.Edges[0].Node.ID
input := openlaneclient.UpdateGroupInput{
Tags: []string{"test"},
}

_, err = suite.client.api.UpdateGroup(testUser1.UserCtx, groupID, input)
require.Error(t, err)
assert.ErrorContains(t, err, "managed groups cannot be modified")

// you should not be able to add group members to a managed group
_, err = suite.client.api.AddUserToGroupWithRole(testUser1.UserCtx, openlaneclient.CreateGroupMembershipInput{
GroupID: groupID,
UserID: testUser2.ID,
})
require.Error(t, err)
assert.ErrorContains(t, err, "managed groups cannot be modified")

// you should not be able to delete a managed group
_, err = suite.client.api.DeleteGroup(testUser1.UserCtx, groupID)
require.Error(t, err)
assert.ErrorContains(t, err, "managed groups cannot be modified")
}
4 changes: 2 additions & 2 deletions internal/graphapi/internalpolicy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -322,15 +322,15 @@ func (suite *GraphTestSuite) TestMutationUpdateInternalPolicy() {
// create another admin user and add them to the same organization and group as testUser1
// this will allow us to test the group editor permissions
anotherAdminUser := suite.userBuilder(context.Background())
suite.addUserToOrganization(&anotherAdminUser, enums.RoleAdmin, testUser1.OrganizationID)
suite.addUserToOrganization(testUser1.UserCtx, &anotherAdminUser, enums.RoleAdmin, testUser1.OrganizationID)

(&GroupMemberBuilder{client: suite.client, UserID: anotherAdminUser.ID, GroupID: testUser1.GroupID}).MustNew(testUser1.UserCtx, t)

// create a viewer user and add them to the same organization as testUser1
// also add them to the same group as testUser1, this should still allow them to edit the policy
// despite not not being an organization admin
anotherViewerUser := suite.userBuilder(context.Background())
suite.addUserToOrganization(&anotherViewerUser, enums.RoleMember, testUser1.OrganizationID)
suite.addUserToOrganization(testUser1.UserCtx, &anotherViewerUser, enums.RoleMember, testUser1.OrganizationID)

(&GroupMemberBuilder{client: suite.client, UserID: anotherViewerUser.ID, GroupID: testUser1.GroupID}).MustNew(testUser1.UserCtx, t)

Expand Down
2 changes: 1 addition & 1 deletion internal/graphapi/narrative_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ func (suite *GraphTestSuite) TestMutationUpdateNarrative() {
// create another admin user and add them to the same organization and group as testUser1
// this will allow us to test the group editor/viewer permissions
anotherAdminUser := suite.userBuilder(context.Background())
suite.addUserToOrganization(&anotherAdminUser, enums.RoleAdmin, testUser1.OrganizationID)
suite.addUserToOrganization(testUser1.UserCtx, &anotherAdminUser, enums.RoleAdmin, testUser1.OrganizationID)

(&GroupMemberBuilder{client: suite.client, UserID: anotherAdminUser.ID, GroupID: testUser1.GroupID}).MustNew(testUser1.UserCtx, t)

Expand Down
14 changes: 8 additions & 6 deletions internal/graphapi/orgmembers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,8 @@ func (suite *GraphTestSuite) TestMutationCreateOrgMembers() {
require.NoError(t, err)
require.Len(t, orgMember, 1)

userCtx, err := auth.NewTestContextWithOrgID(testUser1.ID, org1.ID)

user1 := (&UserBuilder{client: suite.client}).MustNew(testUser1.UserCtx, t)
user2 := (&UserBuilder{client: suite.client}).MustNew(testUser1.UserCtx, t)

Expand All @@ -136,38 +138,38 @@ func (suite *GraphTestSuite) TestMutationCreateOrgMembers() {
name: "happy path, add admin",
orgID: org1.ID,
userID: user1.ID,
ctx: testUser1.UserCtx,
ctx: userCtx,
role: enums.RoleAdmin,
},
{
name: "happy path, add member",
orgID: org1.ID,
userID: user2.ID,
ctx: testUser1.UserCtx,
ctx: userCtx,
role: enums.RoleMember,
},
{
name: "duplicate user, different role",
orgID: org1.ID,
userID: user1.ID,
role: enums.RoleMember,
ctx: testUser1.UserCtx,
ctx: userCtx,
errMsg: "already exists",
},
{
name: "add user to personal org not allowed",
orgID: testUser1.PersonalOrgID,
userID: user1.ID,
role: enums.RoleMember,
ctx: testUser1.UserCtx,
ctx: userCtx,
errMsg: hooks.ErrPersonalOrgsNoMembers.Error(),
},
{
name: "invalid user",
orgID: org1.ID,
userID: ulids.New().String(),
role: enums.RoleMember,
ctx: testUser1.UserCtx,
ctx: userCtx,
errMsg: "constraint failed, unable to complete the create",
},
{
Expand All @@ -183,7 +185,7 @@ func (suite *GraphTestSuite) TestMutationCreateOrgMembers() {
orgID: org1.ID,
userID: user1.ID,
role: enums.RoleInvalid,
ctx: testUser1.UserCtx,
ctx: userCtx,
errMsg: "not a valid OrgMembershipRole",
},
}
Expand Down
4 changes: 2 additions & 2 deletions internal/graphapi/procedure_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -343,15 +343,15 @@ func (suite *GraphTestSuite) TestMutationUpdateProcedure() {
// create another admin user and add them to the same organization and group as testUser1
// this will allow us to test the group editor permissions
anotherAdminUser := suite.userBuilder(context.Background())
suite.addUserToOrganization(&anotherAdminUser, enums.RoleAdmin, testUser1.OrganizationID)
suite.addUserToOrganization(testUser1.UserCtx, &anotherAdminUser, enums.RoleAdmin, testUser1.OrganizationID)

(&GroupMemberBuilder{client: suite.client, UserID: anotherAdminUser.ID, GroupID: testUser1.GroupID}).MustNew(testUser1.UserCtx, t)

// create a viewer user and add them to the same organization as testUser1
// also add them to the same group as testUser1, this should still allow them to edit the procedure
// despite not not being an organization admin
anotherViewerUser := suite.userBuilder(context.Background())
suite.addUserToOrganization(&anotherViewerUser, enums.RoleMember, testUser1.OrganizationID)
suite.addUserToOrganization(testUser1.UserCtx, &anotherViewerUser, enums.RoleMember, testUser1.OrganizationID)

(&GroupMemberBuilder{client: suite.client, UserID: anotherViewerUser.ID, GroupID: testUser1.GroupID}).MustNew(testUser1.UserCtx, t)

Expand Down
6 changes: 3 additions & 3 deletions internal/graphapi/program_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -403,15 +403,15 @@ func (suite *GraphTestSuite) TestMutationUpdateProgram() {
// create another admin user and add them to the same organization and group as testUser1
// this will allow us to test the group editor permissions
anotherAdminUser := suite.userBuilder(context.Background())
suite.addUserToOrganization(&anotherAdminUser, enums.RoleAdmin, testUser1.OrganizationID)
suite.addUserToOrganization(testUser1.UserCtx, &anotherAdminUser, enums.RoleAdmin, testUser1.OrganizationID)

(&GroupMemberBuilder{client: suite.client, UserID: anotherAdminUser.ID, GroupID: testUser1.GroupID}).MustNew(testUser1.UserCtx, t)

// create a viewer user and add them to the same organization as testUser1
// also add them to the same group as testUser1, this should still allow them to edit the policy
// despite not not being an organization admin
anotherViewerUser := suite.userBuilder(context.Background())
suite.addUserToOrganization(&anotherViewerUser, enums.RoleMember, testUser1.OrganizationID)
suite.addUserToOrganization(testUser1.UserCtx, &anotherViewerUser, enums.RoleMember, testUser1.OrganizationID)

(&GroupMemberBuilder{client: suite.client, UserID: anotherViewerUser.ID, GroupID: testUser1.GroupID}).MustNew(testUser1.UserCtx, t)

Expand All @@ -421,7 +421,7 @@ func (suite *GraphTestSuite) TestMutationUpdateProgram() {

// create a view only user and add them to the same organization as testUser1
meowViewerUser := suite.userBuilder(context.Background())
suite.addUserToOrganization(&meowViewerUser, enums.RoleMember, testUser1.OrganizationID)
suite.addUserToOrganization(testUser1.UserCtx, &meowViewerUser, enums.RoleMember, testUser1.OrganizationID)

// create one more group that will be used to test the blocked group permissions and add anotherViewerUser to it
viewerGroup := (&GroupBuilder{client: suite.client}).MustNew(testUser1.UserCtx, t)
Expand Down
2 changes: 1 addition & 1 deletion internal/graphapi/risk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ func (suite *GraphTestSuite) TestMutationUpdateRisk() {
// create another admin user and add them to the same organization and group as testUser1
// this will allow us to test the group editor/viewer permissions
anotherAdminUser := suite.userBuilder(context.Background())
suite.addUserToOrganization(&anotherAdminUser, enums.RoleAdmin, testUser1.OrganizationID)
suite.addUserToOrganization(testUser1.UserCtx, &anotherAdminUser, enums.RoleAdmin, testUser1.OrganizationID)

(&GroupMemberBuilder{client: suite.client, UserID: anotherAdminUser.ID, GroupID: testUser1.GroupID}).MustNew(testUser1.UserCtx, t)

Expand Down
9 changes: 5 additions & 4 deletions internal/graphapi/seed_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,11 @@ func (suite *GraphTestSuite) setupTestData(ctx context.Context) {
viewOnlyUser = suite.userBuilder(ctx)

// add the user to the organization
suite.addUserToOrganization(&viewOnlyUser, enums.RoleMember, testUser1.OrganizationID)
suite.addUserToOrganization(testUser1.UserCtx, &viewOnlyUser, enums.RoleMember, testUser1.OrganizationID)

// setup a test user that is an admin of an organization
adminUser = suite.userBuilder(ctx)
suite.addUserToOrganization(&adminUser, enums.RoleAdmin, testUser1.OrganizationID)
suite.addUserToOrganization(testUser1.UserCtx, &adminUser, enums.RoleAdmin, testUser1.OrganizationID)

// setup client with a personal access token
pat := (&PersonalAccessTokenBuilder{
Expand Down Expand Up @@ -125,11 +125,12 @@ func (suite *GraphTestSuite) setupTestData(ctx context.Context) {
}

// addUserToOrganization adds a user to an organization with the provided role and set's the user's organization ID and user context
func (suite *GraphTestSuite) addUserToOrganization(userDetails *testUserDetails, role enums.Role, organizationID string) {
// the context passed in is the context that has access to the organization the user is being added to
func (suite *GraphTestSuite) addUserToOrganization(ctx context.Context, userDetails *testUserDetails, role enums.Role, organizationID string) {
t := suite.T()

// update organization to be the read-only member of the first test organization
(&OrgMemberBuilder{client: suite.client, UserID: userDetails.ID, OrgID: organizationID, Role: role.String()}).MustNew(userDetails.UserCtx, t)
(&OrgMemberBuilder{client: suite.client, UserID: userDetails.ID, OrgID: organizationID, Role: role.String()}).MustNew(ctx, t)

userDetails.OrganizationID = organizationID

Expand Down

0 comments on commit f922c72

Please sign in to comment.