Skip to content

Removed access management flags and code validating it(AST-103296) #1211

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions internal/commands/groups_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import (
"github.com/checkmarx/ast-cli/internal/wrappers/mock"
)

//todo : need to modify these test cases

func TestCreateScanAndProjectWithGroupFFTrue(t *testing.T) {
mock.Flags = wrappers.FeatureFlagsResponseModel{{Name: "ACCESS_MANAGEMENT_ENABLED", Status: true}}
execCmdNilAssertion(
Expand Down
13 changes: 5 additions & 8 deletions internal/commands/project.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ var (
)

func NewProjectCommand(applicationsWrapper wrappers.ApplicationsWrapper, projectsWrapper wrappers.ProjectsWrapper, groupsWrapper wrappers.GroupsWrapper,
accessManagementWrapper wrappers.AccessManagementWrapper, featureFlagsWrapper wrappers.FeatureFlagsWrapper) *cobra.Command {
accessManagementWrapper wrappers.AccessManagementWrapper, featureFlagsWrapper wrappers.FeatureFlagsWrapper,
) *cobra.Command {
projCmd := &cobra.Command{
Use: "project",
Short: "Manage projects",
Expand Down Expand Up @@ -249,17 +250,11 @@ func runCreateProjectCommand(
if err != nil {
return err
}

groups, err := updateGroupValues(&input, cmd, groupsWrapper)
if err != nil {
return err
}
// Validate groups access before creating the project.
// This validation will only be performed if the ACCESS_MANAGEMENT_PHASE2 flag is ON.
err = services.ValidateGroupsAccessPhase2(groups, accessManagementWrapper, featureFlagsWrapper)
if err != nil {
return err
}

setupScanTags(&input, cmd)
err = validateConfiguration(cmd)
if err != nil {
Expand Down Expand Up @@ -291,7 +286,9 @@ func runCreateProjectCommand(
return errors.Wrapf(err, "%s", services.FailedCreatingProj)
}
}

err = services.AssignGroupsToProjectNewAccessManagement(projResponseModel.ID, projResponseModel.Name, groups, accessManagementWrapper, featureFlagsWrapper)

if err != nil {
return err
}
Expand Down
16 changes: 8 additions & 8 deletions internal/commands/util/import_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ func TestImport_ImportSarifFileWithCorrectFlags_CreateImportSuccessfully(t *test
&mock.ProjectsMockWrapper{},
&mock.UploadsMockWrapper{},
&mock.GroupsMockWrapper{},
mock.AccessManagementMockWrapper{},
&mock.AccessManagementMockWrapper{},
&mock.ByorMockWrapper{},
mock.ApplicationsMockWrapper{},
&mock.FeatureFlagsMockWrapper{},
Expand All @@ -32,7 +32,7 @@ func TestImport_ImportSarifFileProjectDoesntExist_CreateImportWithProvidedNewNam
&mock.ProjectsMockWrapper{},
&mock.UploadsMockWrapper{},
&mock.GroupsMockWrapper{},
mock.AccessManagementMockWrapper{},
&mock.AccessManagementMockWrapper{},
&mock.ByorMockWrapper{},
mock.ApplicationsMockWrapper{},
&mock.FeatureFlagsMockWrapper{},
Expand All @@ -48,7 +48,7 @@ func TestImport_ImportSarifFileMissingImportFilePath_CreateImportReturnsErrorWit
&mock.ProjectsMockWrapper{},
&mock.UploadsMockWrapper{},
&mock.GroupsMockWrapper{},
mock.AccessManagementMockWrapper{},
&mock.AccessManagementMockWrapper{},
&mock.ByorMockWrapper{},
mock.ApplicationsMockWrapper{},
&mock.FeatureFlagsMockWrapper{},
Expand All @@ -63,7 +63,7 @@ func TestImport_ImportSarifFileEmptyImportFilePathValue_CreateImportReturnsError
&mock.ProjectsMockWrapper{},
&mock.UploadsMockWrapper{},
&mock.GroupsMockWrapper{},
mock.AccessManagementMockWrapper{},
&mock.AccessManagementMockWrapper{},
&mock.ByorMockWrapper{},
mock.ApplicationsMockWrapper{},
&mock.FeatureFlagsMockWrapper{},
Expand All @@ -78,7 +78,7 @@ func TestImport_ImportSarifFileMissingImportProjectName_CreateImportReturnsError
&mock.ProjectsMockWrapper{},
&mock.UploadsMockWrapper{},
&mock.GroupsMockWrapper{},
mock.AccessManagementMockWrapper{},
&mock.AccessManagementMockWrapper{},
&mock.ByorMockWrapper{},
mock.ApplicationsMockWrapper{},
&mock.FeatureFlagsMockWrapper{},
Expand All @@ -93,7 +93,7 @@ func TestImport_ImportSarifFileProjectNameNotProvided_CreateImportWithProvidedNe
&mock.ProjectsMockWrapper{},
&mock.UploadsMockWrapper{},
&mock.GroupsMockWrapper{},
mock.AccessManagementMockWrapper{},
&mock.AccessManagementMockWrapper{},
&mock.ByorMockWrapper{},
mock.ApplicationsMockWrapper{},
&mock.FeatureFlagsMockWrapper{},
Expand All @@ -108,7 +108,7 @@ func TestImport_ImportSarifFileUnacceptedFileExtension_CreateImportReturnsErrorW
&mock.ProjectsMockWrapper{},
&mock.UploadsMockWrapper{},
&mock.GroupsMockWrapper{},
mock.AccessManagementMockWrapper{},
&mock.AccessManagementMockWrapper{},
&mock.ByorMockWrapper{},
mock.ApplicationsMockWrapper{},
&mock.FeatureFlagsMockWrapper{},
Expand All @@ -123,7 +123,7 @@ func TestImport_ImportSarifFileMissingExtension_CreateImportReturnsErrorWithCorr
&mock.ProjectsMockWrapper{},
&mock.UploadsMockWrapper{},
&mock.GroupsMockWrapper{},
mock.AccessManagementMockWrapper{},
&mock.AccessManagementMockWrapper{},
&mock.ByorMockWrapper{},
mock.ApplicationsMockWrapper{},
&mock.FeatureFlagsMockWrapper{},
Expand Down
1 change: 1 addition & 0 deletions internal/constants/feature-flags/feature-flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@ package featureflags
const (
AccessManagementEnabled = "ACCESS_MANAGEMENT_ENABLED"
AccessManagementPhase2 = "ACCESS_MANAGEMENT_PHASE_2"
GroupValidationEnabled = "GROUPS_VALIDATION_ENABLED"
)
37 changes: 19 additions & 18 deletions internal/services/groups.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,18 +47,34 @@ func CreateGroupsMap(groupsStr string, groupsWrapper wrappers.GroupsWrapper) ([]
}
return groupsMap, nil
}
func getGroupsToAssign(receivedGroups, existingGroups []*wrappers.Group) []*wrappers.Group {
var groupsToAssign []*wrappers.Group
var groupsMap = make(map[string]bool)
for _, existingGroup := range existingGroups {
groupsMap[existingGroup.ID] = true
}
for _, receivedGroup := range receivedGroups {
find := groupsMap[receivedGroup.ID]
if !find {
groupsToAssign = append(groupsToAssign, receivedGroup)
}
}
return groupsToAssign
}

func AssignGroupsToProjectNewAccessManagement(projectID string, projectName string, groups []*wrappers.Group,
accessManagement wrappers.AccessManagementWrapper, featureFlagsWrapper wrappers.FeatureFlagsWrapper) error {

amEnabledFlag, _ := wrappers.GetSpecificFeatureFlag(featureFlagsWrapper, featureFlagsConstants.AccessManagementEnabled)
amPhase2Flag, _ := wrappers.GetSpecificFeatureFlag(featureFlagsWrapper, featureFlagsConstants.AccessManagementPhase2)
groupValidationEnabledFlag, _ := wrappers.GetSpecificFeatureFlag(featureFlagsWrapper, featureFlagsConstants.GroupValidationEnabled)

// If ACCESS_MANAGEMENT_PHASE2 flag is ON and if the ACCESS_MANAGEMENT_ENABLED flag is OFF
// If ACCESS_MANAGEMENT_ENABLED flag is OFF or (groupValidation is on and ACCESS_MANAGEMENT_ENABLED is also on )
// In both cases, we do not need to assign groups through the CreateGroupsAssignment call.
if !amEnabledFlag.Status || amPhase2Flag.Status {

if !amEnabledFlag.Status || (amEnabledFlag.Status && groupValidationEnabledFlag.Status) {
return nil
}

groupsAssignedToTheProject, err := accessManagement.GetGroups(projectID)
if err != nil {
return err
Expand All @@ -75,21 +91,6 @@ func AssignGroupsToProjectNewAccessManagement(projectID string, projectName stri
return nil
}

func getGroupsToAssign(receivedGroups, existingGroups []*wrappers.Group) []*wrappers.Group {
var groupsToAssign []*wrappers.Group
var groupsMap = make(map[string]bool)
for _, existingGroup := range existingGroups {
groupsMap[existingGroup.ID] = true
}
for _, receivedGroup := range receivedGroups {
find := groupsMap[receivedGroup.ID]
if !find {
groupsToAssign = append(groupsToAssign, receivedGroup)
}
}
return groupsToAssign
}

func GetGroupIds(groups []*wrappers.Group) []string {
var groupIds []string
for _, group := range groups {
Expand Down
42 changes: 0 additions & 42 deletions internal/services/groups_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,55 +4,13 @@ import (
"reflect"
"testing"

featureFlagsConstants "github.com/checkmarx/ast-cli/internal/constants/feature-flags"

"github.com/checkmarx/ast-cli/internal/wrappers"
"github.com/checkmarx/ast-cli/internal/wrappers/mock"
)

func setup() {
wrappers.ClearCache()
}

func TestAssignGroupsToProject(t *testing.T) {
setup() // Clear the map before starting this test
type args struct {
projectID string
projectName string
groups []*wrappers.Group
accessManagement wrappers.AccessManagementWrapper
featureFlagsWrapper wrappers.FeatureFlagsWrapper
}
tests := []struct {
name string
args args
wantErr bool
}{
{
name: "When assigning group to project, no error should be returned",
args: args{
projectID: "project-id",
projectName: "project-name",
groups: []*wrappers.Group{{
ID: "group-id-to-assign",
Name: "group-name-to-assign",
}},
accessManagement: &mock.AccessManagementMockWrapper{},
featureFlagsWrapper: &mock.FeatureFlagsMockWrapper{},
},
wantErr: false,
},
}
for _, tt := range tests {
ttt := tt
mock.Flag = wrappers.FeatureFlagResponseModel{Name: featureFlagsConstants.AccessManagementEnabled, Status: true}
t.Run(tt.name, func(t *testing.T) {
if err := AssignGroupsToProjectNewAccessManagement(ttt.args.projectID, ttt.args.projectName, ttt.args.groups,
ttt.args.accessManagement, ttt.args.featureFlagsWrapper); (err != nil) != ttt.wantErr {
t.Errorf("AssignGroupsToProjectNewAccessManagement() error = %v, wantErr %v", err, ttt.wantErr)
}
})
}
}

func TestCreateGroupsMap(t *testing.T) {
Expand Down
41 changes: 2 additions & 39 deletions internal/services/projects.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"strings"
"time"

featureFlagsConstants "github.com/checkmarx/ast-cli/internal/constants/feature-flags"
"github.com/checkmarx/ast-cli/internal/logger"
commonParams "github.com/checkmarx/ast-cli/internal/params"
"github.com/checkmarx/ast-cli/internal/wrappers"
Expand Down Expand Up @@ -110,6 +109,7 @@ func createProject(
var projModel = wrappers.Project{}
projModel.Name = projectName
projModel.ApplicationIds = applicationID

if isBranchPrimary {
logger.PrintIfVerbose(fmt.Sprintf("Setting the branch in project : %s", branchName))
projModel.MainBranch = branchName
Expand All @@ -118,16 +118,10 @@ func createProject(
if projectGroups != "" {
var groups []string
var groupErr error
groupsMap, groups, groupErr = GetGroupMap(groupsWrapper, projectGroups, nil)
_, groups, groupErr = GetGroupMap(groupsWrapper, projectGroups, nil)
if groupErr != nil {
return "", groupErr
}
// Validate groups access before assigning them to the project.
// This validation will only be performed if the ACCESS_MANAGEMENT_PHASE2 flag is ON.
err := ValidateGroupsAccessPhase2(groupsMap, accessManagementWrapper, featureFlagsWrapper)
if err != nil {
return "", err
}
projModel.Groups = groups
}

Expand Down Expand Up @@ -234,7 +228,6 @@ func updateProject(project *wrappers.ProjectResponseModel,

return projectID, nil
}

func UpsertProjectGroups(projModel *wrappers.Project, projectsWrapper wrappers.ProjectsWrapper,
accessManagementWrapper wrappers.AccessManagementWrapper, projectID string, projectName string,
featureFlagsWrapper wrappers.FeatureFlagsWrapper, groupsMap []*wrappers.Group) error {
Expand All @@ -250,33 +243,3 @@ func UpsertProjectGroups(projModel *wrappers.Project, projectsWrapper wrappers.P
}
return nil
}

func ValidateGroupsAccessPhase2(groups []*wrappers.Group, accessManagementWrapper wrappers.AccessManagementWrapper, featureFlagsWrapper wrappers.FeatureFlagsWrapper) error {
// If no groups to validate, return
if len(groups) == 0 {
return nil
}

amPhase2Flag, _ := wrappers.GetSpecificFeatureFlag(featureFlagsWrapper, featureFlagsConstants.AccessManagementPhase2)
if !amPhase2Flag.Status {
return nil
}

// Extract group IDs
var groupIDs []string
for _, group := range groups {
groupIDs = append(groupIDs, group.ID)
}

// Validate groups access
hasAccess, err := accessManagementWrapper.HasEntityAccessToGroups(groupIDs)
if err != nil {
return errors.Wrap(err, "Failed to validate groups access")
}

if !hasAccess {
return errors.New("One or more groups are not authorized for assignment")
}

return nil
}
Loading
Loading