Skip to content

Commit 09342b7

Browse files
Itay PazItay Paz
Itay Paz
authored and
Itay Paz
committed
pull
2 parents 61358ea + bedb736 commit 09342b7

File tree

14 files changed

+198
-123
lines changed

14 files changed

+198
-123
lines changed

‎internal/commands/project.go‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,8 @@ var (
6060
)
6161

6262
func NewProjectCommand(applicationsWrapper wrappers.ApplicationsWrapper, projectsWrapper wrappers.ProjectsWrapper, groupsWrapper wrappers.GroupsWrapper,
63-
accessManagementWrapper wrappers.AccessManagementWrapper, featureFlagsWrapper wrappers.FeatureFlagsWrapper) *cobra.Command {
63+
accessManagementWrapper wrappers.AccessManagementWrapper, featureFlagsWrapper wrappers.FeatureFlagsWrapper,
64+
) *cobra.Command {
6465
projCmd := &cobra.Command{
6566
Use: "project",
6667
Short: "Manage projects",
@@ -249,17 +250,11 @@ func runCreateProjectCommand(
249250
if err != nil {
250251
return err
251252
}
253+
252254
groups, err := updateGroupValues(&input, cmd, groupsWrapper)
253255
if err != nil {
254256
return err
255257
}
256-
// Validate groups access before creating the project.
257-
// This validation will only be performed if the ACCESS_MANAGEMENT_PHASE2 flag is ON.
258-
err = services.ValidateGroupsAccessPhase2(groups, accessManagementWrapper, featureFlagsWrapper)
259-
if err != nil {
260-
return err
261-
}
262-
263258
setupScanTags(&input, cmd)
264259
err = validateConfiguration(cmd)
265260
if err != nil {
@@ -291,7 +286,9 @@ func runCreateProjectCommand(
291286
return errors.Wrapf(err, "%s", services.FailedCreatingProj)
292287
}
293288
}
289+
294290
err = services.AssignGroupsToProjectNewAccessManagement(projResponseModel.ID, projResponseModel.Name, groups, accessManagementWrapper, featureFlagsWrapper)
291+
295292
if err != nil {
296293
return err
297294
}

‎internal/commands/result.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2704,6 +2704,8 @@ func parseSarifResultsSscs(result *wrappers.ScanResult, scanResults []wrappers.S
27042704
var properties wrappers.SarifResultProperties
27052705
properties.Severity = result.Severity
27062706
properties.Validity = result.ScanResultData.Validity
2707+
properties.IsInSource = result.ScanResultData.IsInSource
2708+
properties.CommitURL = result.ScanResultData.CommitURL
27072709
scanResult.Properties = &properties
27082710

27092711
scanResults = append(scanResults, scanResult)

‎internal/commands/util/import_test.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ func TestImport_ImportSarifFileWithCorrectFlags_CreateImportSuccessfully(t *test
1616
&mock.ProjectsMockWrapper{},
1717
&mock.UploadsMockWrapper{},
1818
&mock.GroupsMockWrapper{},
19-
mock.AccessManagementMockWrapper{},
19+
&mock.AccessManagementMockWrapper{},
2020
&mock.ByorMockWrapper{},
2121
mock.ApplicationsMockWrapper{},
2222
&mock.FeatureFlagsMockWrapper{},
@@ -32,7 +32,7 @@ func TestImport_ImportSarifFileProjectDoesntExist_CreateImportWithProvidedNewNam
3232
&mock.ProjectsMockWrapper{},
3333
&mock.UploadsMockWrapper{},
3434
&mock.GroupsMockWrapper{},
35-
mock.AccessManagementMockWrapper{},
35+
&mock.AccessManagementMockWrapper{},
3636
&mock.ByorMockWrapper{},
3737
mock.ApplicationsMockWrapper{},
3838
&mock.FeatureFlagsMockWrapper{},
@@ -48,7 +48,7 @@ func TestImport_ImportSarifFileMissingImportFilePath_CreateImportReturnsErrorWit
4848
&mock.ProjectsMockWrapper{},
4949
&mock.UploadsMockWrapper{},
5050
&mock.GroupsMockWrapper{},
51-
mock.AccessManagementMockWrapper{},
51+
&mock.AccessManagementMockWrapper{},
5252
&mock.ByorMockWrapper{},
5353
mock.ApplicationsMockWrapper{},
5454
&mock.FeatureFlagsMockWrapper{},
@@ -63,7 +63,7 @@ func TestImport_ImportSarifFileEmptyImportFilePathValue_CreateImportReturnsError
6363
&mock.ProjectsMockWrapper{},
6464
&mock.UploadsMockWrapper{},
6565
&mock.GroupsMockWrapper{},
66-
mock.AccessManagementMockWrapper{},
66+
&mock.AccessManagementMockWrapper{},
6767
&mock.ByorMockWrapper{},
6868
mock.ApplicationsMockWrapper{},
6969
&mock.FeatureFlagsMockWrapper{},
@@ -78,7 +78,7 @@ func TestImport_ImportSarifFileMissingImportProjectName_CreateImportReturnsError
7878
&mock.ProjectsMockWrapper{},
7979
&mock.UploadsMockWrapper{},
8080
&mock.GroupsMockWrapper{},
81-
mock.AccessManagementMockWrapper{},
81+
&mock.AccessManagementMockWrapper{},
8282
&mock.ByorMockWrapper{},
8383
mock.ApplicationsMockWrapper{},
8484
&mock.FeatureFlagsMockWrapper{},
@@ -93,7 +93,7 @@ func TestImport_ImportSarifFileProjectNameNotProvided_CreateImportWithProvidedNe
9393
&mock.ProjectsMockWrapper{},
9494
&mock.UploadsMockWrapper{},
9595
&mock.GroupsMockWrapper{},
96-
mock.AccessManagementMockWrapper{},
96+
&mock.AccessManagementMockWrapper{},
9797
&mock.ByorMockWrapper{},
9898
mock.ApplicationsMockWrapper{},
9999
&mock.FeatureFlagsMockWrapper{},
@@ -108,7 +108,7 @@ func TestImport_ImportSarifFileUnacceptedFileExtension_CreateImportReturnsErrorW
108108
&mock.ProjectsMockWrapper{},
109109
&mock.UploadsMockWrapper{},
110110
&mock.GroupsMockWrapper{},
111-
mock.AccessManagementMockWrapper{},
111+
&mock.AccessManagementMockWrapper{},
112112
&mock.ByorMockWrapper{},
113113
mock.ApplicationsMockWrapper{},
114114
&mock.FeatureFlagsMockWrapper{},
@@ -123,7 +123,7 @@ func TestImport_ImportSarifFileMissingExtension_CreateImportReturnsErrorWithCorr
123123
&mock.ProjectsMockWrapper{},
124124
&mock.UploadsMockWrapper{},
125125
&mock.GroupsMockWrapper{},
126-
mock.AccessManagementMockWrapper{},
126+
&mock.AccessManagementMockWrapper{},
127127
&mock.ByorMockWrapper{},
128128
mock.ApplicationsMockWrapper{},
129129
&mock.FeatureFlagsMockWrapper{},

‎internal/constants/feature-flags/feature-flags.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,5 @@ package featureflags
22

33
const (
44
AccessManagementEnabled = "ACCESS_MANAGEMENT_ENABLED"
5-
AccessManagementPhase2 = "ACCESS_MANAGEMENT_PHASE_2"
5+
GroupValidationEnabled = "GROUPS_VALIDATION_ENABLED"
66
)

‎internal/services/groups.go‎

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -47,18 +47,34 @@ func CreateGroupsMap(groupsStr string, groupsWrapper wrappers.GroupsWrapper) ([]
4747
}
4848
return groupsMap, nil
4949
}
50+
func getGroupsToAssign(receivedGroups, existingGroups []*wrappers.Group) []*wrappers.Group {
51+
var groupsToAssign []*wrappers.Group
52+
var groupsMap = make(map[string]bool)
53+
for _, existingGroup := range existingGroups {
54+
groupsMap[existingGroup.ID] = true
55+
}
56+
for _, receivedGroup := range receivedGroups {
57+
find := groupsMap[receivedGroup.ID]
58+
if !find {
59+
groupsToAssign = append(groupsToAssign, receivedGroup)
60+
}
61+
}
62+
return groupsToAssign
63+
}
5064

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

5468
amEnabledFlag, _ := wrappers.GetSpecificFeatureFlag(featureFlagsWrapper, featureFlagsConstants.AccessManagementEnabled)
55-
amPhase2Flag, _ := wrappers.GetSpecificFeatureFlag(featureFlagsWrapper, featureFlagsConstants.AccessManagementPhase2)
69+
groupValidationEnabledFlag, _ := wrappers.GetSpecificFeatureFlag(featureFlagsWrapper, featureFlagsConstants.GroupValidationEnabled)
5670

57-
// If ACCESS_MANAGEMENT_PHASE2 flag is ON and if the ACCESS_MANAGEMENT_ENABLED flag is OFF
71+
// If ACCESS_MANAGEMENT_ENABLED flag is OFF or (GROUP_VALIDATION_ENABLED is on and ACCESS_MANAGEMENT_ENABLED is also on )
5872
// In both cases, we do not need to assign groups through the CreateGroupsAssignment call.
59-
if !amEnabledFlag.Status || amPhase2Flag.Status {
73+
74+
if !amEnabledFlag.Status || (amEnabledFlag.Status && groupValidationEnabledFlag.Status) {
6075
return nil
6176
}
77+
6278
groupsAssignedToTheProject, err := accessManagement.GetGroups(projectID)
6379
if err != nil {
6480
return err
@@ -75,21 +91,6 @@ func AssignGroupsToProjectNewAccessManagement(projectID string, projectName stri
7591
return nil
7692
}
7793

78-
func getGroupsToAssign(receivedGroups, existingGroups []*wrappers.Group) []*wrappers.Group {
79-
var groupsToAssign []*wrappers.Group
80-
var groupsMap = make(map[string]bool)
81-
for _, existingGroup := range existingGroups {
82-
groupsMap[existingGroup.ID] = true
83-
}
84-
for _, receivedGroup := range receivedGroups {
85-
find := groupsMap[receivedGroup.ID]
86-
if !find {
87-
groupsToAssign = append(groupsToAssign, receivedGroup)
88-
}
89-
}
90-
return groupsToAssign
91-
}
92-
9394
func GetGroupIds(groups []*wrappers.Group) []string {
9495
var groupIds []string
9596
for _, group := range groups {

‎internal/services/groups_test.go‎

Lines changed: 90 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,22 @@
11
package services
22

33
import (
4+
"bytes"
5+
"fmt"
6+
"io"
7+
"os"
48
"reflect"
9+
"strings"
510
"testing"
611

712
featureFlagsConstants "github.com/checkmarx/ast-cli/internal/constants/feature-flags"
8-
913
"github.com/checkmarx/ast-cli/internal/wrappers"
1014
"github.com/checkmarx/ast-cli/internal/wrappers/mock"
1115
)
1216

1317
func setup() {
1418
wrappers.ClearCache()
19+
1520
}
1621

1722
func TestAssignGroupsToProject(t *testing.T) {
@@ -29,7 +34,7 @@ func TestAssignGroupsToProject(t *testing.T) {
2934
wantErr bool
3035
}{
3136
{
32-
name: "When assigning group to project, no error should be returned",
37+
name: "When assigning group to project, no error should be returned when AM1 is ON",
3338
args: args{
3439
projectID: "project-id",
3540
projectName: "project-name",
@@ -45,12 +50,74 @@ func TestAssignGroupsToProject(t *testing.T) {
4550
}
4651
for _, tt := range tests {
4752
ttt := tt
48-
mock.Flag = wrappers.FeatureFlagResponseModel{Name: featureFlagsConstants.AccessManagementEnabled, Status: true}
4953
t.Run(tt.name, func(t *testing.T) {
50-
if err := AssignGroupsToProjectNewAccessManagement(ttt.args.projectID, ttt.args.projectName, ttt.args.groups,
51-
ttt.args.accessManagement, ttt.args.featureFlagsWrapper); (err != nil) != ttt.wantErr {
52-
t.Errorf("AssignGroupsToProjectNewAccessManagement() error = %v, wantErr %v", err, ttt.wantErr)
53-
}
54+
mock.Flag = wrappers.FeatureFlagResponseModel{Name: featureFlagsConstants.AccessManagementEnabled, Status: true}
55+
mock.Flag = wrappers.FeatureFlagResponseModel{Name: featureFlagsConstants.GroupValidationEnabled, Status: false}
56+
var output string
57+
output, _ = captureStdout(func() {
58+
if err := AssignGroupsToProjectNewAccessManagement(ttt.args.projectID, ttt.args.projectName, ttt.args.groups,
59+
ttt.args.accessManagement, ttt.args.featureFlagsWrapper); (err != nil) != ttt.wantErr {
60+
t.Errorf("AssignGroupsToProjectNewAccessManagement() error = %v, wantErr %v", err, ttt.wantErr)
61+
if err != nil {
62+
t.Errorf("failed to close file")
63+
}
64+
65+
if output != "" && !strings.Contains(output, "Called CreateGroupsAssignment in AccessManagementMockWrapper") {
66+
t.Errorf("Create GroupAssignment should not get called when GroupValidation Flag is ON")
67+
}
68+
}
69+
})
70+
})
71+
}
72+
}
73+
74+
func TestAssignGroupsToProjectWithGroupValidationFlag(t *testing.T) {
75+
setup() // Clear the map before starting this test
76+
type args struct {
77+
projectID string
78+
projectName string
79+
groups []*wrappers.Group
80+
accessManagement wrappers.AccessManagementWrapper
81+
featureFlagsWrapper wrappers.FeatureFlagsWrapper
82+
}
83+
tests := []struct {
84+
name string
85+
args args
86+
wantErr bool
87+
}{
88+
{
89+
name: "When assigning group to project,error should be returned when Access_management_enabled is ON and GroupValidationFlag is also On",
90+
args: args{
91+
projectID: "project-id",
92+
projectName: "project-name",
93+
groups: []*wrappers.Group{{
94+
ID: "group-id-to-assign",
95+
Name: "group-name-to-assign",
96+
}},
97+
accessManagement: &mock.AccessManagementMockWrapper{},
98+
featureFlagsWrapper: &mock.FeatureFlagsMockWrapper{},
99+
},
100+
wantErr: false,
101+
},
102+
}
103+
for _, tt := range tests {
104+
ttt := tt
105+
t.Run(tt.name, func(t *testing.T) {
106+
mock.Flag = wrappers.FeatureFlagResponseModel{Name: featureFlagsConstants.AccessManagementEnabled, Status: true}
107+
mock.Flag = wrappers.FeatureFlagResponseModel{Name: featureFlagsConstants.GroupValidationEnabled, Status: true}
108+
var output string
109+
output, _ = captureStdout(func() {
110+
if err := AssignGroupsToProjectNewAccessManagement(ttt.args.projectID, ttt.args.projectName, ttt.args.groups,
111+
ttt.args.accessManagement, ttt.args.featureFlagsWrapper); (err != nil) != ttt.wantErr {
112+
t.Errorf("AssignGroupsToProjectNewAccessManagement() error = %v, wantErr %v", err, ttt.wantErr)
113+
if err != nil {
114+
t.Errorf("failed to close file")
115+
}
116+
if output != "" && strings.Contains(output, "Called CreateGroupsAssignment in AccessManagementMockWrapper") {
117+
t.Errorf("Create GroupAssignment should not get called when GroupValidation Flag is ON")
118+
}
119+
}
120+
})
54121
})
55122
}
56123
}
@@ -201,3 +268,19 @@ func Test_getGroupsToAssign(t *testing.T) {
201268
})
202269
}
203270
}
271+
272+
func captureStdout(fn func()) (string, error) {
273+
originalStdout := os.Stdout
274+
r, w, _ := os.Pipe()
275+
os.Stdout = w
276+
fn()
277+
w.Close()
278+
os.Stdout = originalStdout
279+
var buf bytes.Buffer
280+
_, err := io.Copy(&buf, r)
281+
if err != nil {
282+
return "", fmt.Errorf("Cannot copy the pipe ouput into buffer: %v", err)
283+
}
284+
285+
return buf.String(), nil
286+
}

‎internal/services/projects.go‎

Lines changed: 1 addition & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"strings"
88
"time"
99

10-
featureFlagsConstants "github.com/checkmarx/ast-cli/internal/constants/feature-flags"
1110
"github.com/checkmarx/ast-cli/internal/logger"
1211
commonParams "github.com/checkmarx/ast-cli/internal/params"
1312
"github.com/checkmarx/ast-cli/internal/wrappers"
@@ -110,6 +109,7 @@ func createProject(
110109
var projModel = wrappers.Project{}
111110
projModel.Name = projectName
112111
projModel.ApplicationIds = applicationID
112+
113113
if isBranchPrimary {
114114
logger.PrintIfVerbose(fmt.Sprintf("Setting the branch in project : %s", branchName))
115115
projModel.MainBranch = branchName
@@ -122,12 +122,6 @@ func createProject(
122122
if groupErr != nil {
123123
return "", groupErr
124124
}
125-
// Validate groups access before assigning them to the project.
126-
// This validation will only be performed if the ACCESS_MANAGEMENT_PHASE2 flag is ON.
127-
err := ValidateGroupsAccessPhase2(groupsMap, accessManagementWrapper, featureFlagsWrapper)
128-
if err != nil {
129-
return "", err
130-
}
131125
projModel.Groups = groups
132126
}
133127

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

235229
return projectID, nil
236230
}
237-
238231
func UpsertProjectGroups(projModel *wrappers.Project, projectsWrapper wrappers.ProjectsWrapper,
239232
accessManagementWrapper wrappers.AccessManagementWrapper, projectID string, projectName string,
240233
featureFlagsWrapper wrappers.FeatureFlagsWrapper, groupsMap []*wrappers.Group) error {
@@ -250,33 +243,3 @@ func UpsertProjectGroups(projModel *wrappers.Project, projectsWrapper wrappers.P
250243
}
251244
return nil
252245
}
253-
254-
func ValidateGroupsAccessPhase2(groups []*wrappers.Group, accessManagementWrapper wrappers.AccessManagementWrapper, featureFlagsWrapper wrappers.FeatureFlagsWrapper) error {
255-
// If no groups to validate, return
256-
if len(groups) == 0 {
257-
return nil
258-
}
259-
260-
amPhase2Flag, _ := wrappers.GetSpecificFeatureFlag(featureFlagsWrapper, featureFlagsConstants.AccessManagementPhase2)
261-
if !amPhase2Flag.Status {
262-
return nil
263-
}
264-
265-
// Extract group IDs
266-
var groupIDs []string
267-
for _, group := range groups {
268-
groupIDs = append(groupIDs, group.ID)
269-
}
270-
271-
// Validate groups access
272-
hasAccess, err := accessManagementWrapper.HasEntityAccessToGroups(groupIDs)
273-
if err != nil {
274-
return errors.Wrap(err, "Failed to validate groups access")
275-
}
276-
277-
if !hasAccess {
278-
return errors.New("One or more groups are not authorized for assignment")
279-
}
280-
281-
return nil
282-
}

0 commit comments

Comments
 (0)