Skip to content

Commit d2ac965

Browse files
committed
Revert "adding logs whenever client uses token which is security best practice"
This reverts commit 9e37185.
1 parent 54151f7 commit d2ac965

File tree

10 files changed

+5
-36
lines changed

10 files changed

+5
-36
lines changed

pkg/provider/bitbucketcloud/bitbucket.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -203,16 +203,12 @@ func (v *Provider) SetClient(_ context.Context, run *params.Run, event *info.Eve
203203
return fmt.Errorf("no git_provider.user has been in repo crd")
204204
}
205205
v.bbClient = bitbucket.NewBasicAuth(event.Provider.User, event.Provider.Token)
206-
// Added log for security audit purposes to log client access when a token is used
207-
v.Logger.Infof("bitbucket-cloud: initialized client with provided token for user=%s", event.Provider.User)
208-
209206
v.Token = &event.Provider.Token
210207
v.Username = &event.Provider.User
211208
v.run = run
212209
v.eventEmitter = eventEmitter
213210
v.repo = repo
214211
v.triggerEvent = event.EventType
215-
216212
return nil
217213
}
218214

pkg/provider/bitbucketcloud/bitbucket_test.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -137,9 +137,7 @@ func TestSetClient(t *testing.T) {
137137
for _, tt := range tests {
138138
t.Run(tt.name, func(t *testing.T) {
139139
ctx, _ := rtesting.SetupFakeContext(t)
140-
observer, _ := zapobserver.New(zap.InfoLevel)
141-
logger := zap.New(observer).Sugar()
142-
v := Provider{Logger: logger}
140+
v := Provider{}
143141
err := v.SetClient(ctx, nil, tt.event, nil, nil)
144142
if tt.wantErrSubstr != "" {
145143
assert.ErrorContains(t, err, tt.wantErrSubstr)

pkg/provider/bitbucketdatacenter/bitbucketdatacenter.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -307,9 +307,6 @@ func (v *Provider) SetClient(ctx context.Context, run *params.Run, event *info.E
307307
},
308308
}
309309
v.client = client
310-
311-
// Added for security audit purposes to log client access when a token is used
312-
v.Logger.Infof("bitbucket-datacenter: initialized client with provided token for user=%s providerURL=%s", event.Provider.User, event.Provider.URL)
313310
}
314311
v.run = run
315312
v.repo = repo

pkg/provider/bitbucketdatacenter/bitbucketdatacenter_test.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -362,15 +362,13 @@ func TestSetClient(t *testing.T) {
362362
}
363363
for _, tt := range tests {
364364
t.Run(tt.name, func(t *testing.T) {
365-
observer, _ := zapobserver.New(zap.InfoLevel)
366-
logger := zap.New(observer).Sugar()
367365
ctx, _ := rtesting.SetupFakeContext(t)
368366
client, mux, tearDown, tURL := bbtest.SetupBBDataCenterClient()
369367
defer tearDown()
370368
if tt.muxUser != nil {
371369
mux.HandleFunc("/users/foo", tt.muxUser)
372370
}
373-
v := &Provider{client: client, baseURL: tURL, Logger: logger}
371+
v := &Provider{client: client, baseURL: tURL}
374372
err := v.SetClient(ctx, nil, tt.opts, nil, nil)
375373
if tt.wantErrSubstr != "" {
376374
assert.ErrorContains(t, err, tt.wantErrSubstr)

pkg/provider/gitea/gitea.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -159,10 +159,6 @@ func (v *Provider) SetClient(_ context.Context, run *params.Run, runevent *info.
159159
if err != nil {
160160
return err
161161
}
162-
163-
// Added log for security audit purposes to log client access when a token is used
164-
v.Logger.Infof("gitea: initialized API client with provided credentials user=%s providerURL=%s", runevent.Provider.User, apiURL)
165-
166162
v.giteaInstanceURL = runevent.Provider.URL
167163
v.eventEmitter = emitter
168164
v.repo = repo

pkg/provider/github/github.go

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -301,13 +301,6 @@ func (v *Provider) SetClient(ctx context.Context, run *params.Run, event *info.E
301301
return fmt.Errorf("no github client has been initialized")
302302
}
303303

304-
// Added log for security audit purposes to log client access when a token is used
305-
integration := "github-webhook"
306-
if event.InstallationID != 0 {
307-
integration = "github-app"
308-
}
309-
v.Logger.Infof(integration+": initialized OAuth2 client for providerName=%s providerURL=%s", v.providerName, event.Provider.URL)
310-
311304
v.APIURL = apiURL
312305

313306
if event.Provider.WebhookSecretFromRepo {

pkg/provider/github/github_test.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -663,9 +663,7 @@ func TestGithubSetClient(t *testing.T) {
663663
for _, tt := range tests {
664664
t.Run(tt.name, func(t *testing.T) {
665665
ctx, _ := rtesting.SetupFakeContext(t)
666-
observer, _ := zapobserver.New(zap.InfoLevel)
667-
logger := zap.New(observer).Sugar()
668-
v := Provider{Logger: logger}
666+
v := Provider{}
669667
err := v.SetClient(ctx, nil, tt.event, nil, nil)
670668
assert.NilError(t, err)
671669
assert.Equal(t, tt.expectedURL, *v.APIURL)

pkg/provider/gitlab/gitlab.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,6 @@ func (v *Provider) SetClient(_ context.Context, run *params.Run, runevent *info.
201201
}
202202
v.Token = &runevent.Provider.Token
203203

204-
v.Logger.Infof("gitlab: initialized for client with token for apiURL=%s, org=%s, repo=%s)", apiURL, runevent.Organization, runevent.Repository)
205204
// In a scenario where the source repository is forked and a merge request (MR) is created on the upstream
206205
// repository, runevent.SourceProjectID will not be 0 when SetClient is called from the pac-watcher code.
207206
// This is because, in the controller, SourceProjectID is set in the annotation of the pull request,

pkg/provider/gitlab/gitlab_test.go

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -260,14 +260,12 @@ func TestGetConfig(t *testing.T) {
260260

261261
func TestSetClient(t *testing.T) {
262262
ctx, _ := rtesting.SetupFakeContext(t)
263-
observer, _ := zapobserver.New(zap.InfoLevel)
264-
fakelogger := zap.New(observer).Sugar()
265263
v := &Provider{}
266264
assert.Assert(t, v.SetClient(ctx, nil, info.NewEvent(), nil, nil) != nil)
267265

268266
client, _, tearDown := thelp.Setup(t)
269267
defer tearDown()
270-
vv := &Provider{gitlabClient: client, Logger: fakelogger}
268+
vv := &Provider{gitlabClient: client}
271269
err := vv.SetClient(ctx, nil, &info.Event{
272270
Provider: &info.Provider{
273271
Token: "hello",
@@ -279,8 +277,6 @@ func TestSetClient(t *testing.T) {
279277

280278
func TestSetClientDetectAPIURL(t *testing.T) {
281279
ctx, _ := rtesting.SetupFakeContext(t)
282-
observer, _ := zapobserver.New(zap.InfoLevel)
283-
fakelogger := zap.New(observer).Sugar()
284280
mockClient, _, tearDown := thelp.Setup(t)
285281
defer tearDown()
286282

@@ -385,7 +381,6 @@ func TestSetClientDetectAPIURL(t *testing.T) {
385381
gitlabClient: mockClient, // Use the shared mock client
386382
repoURL: tc.repoURL,
387383
pathWithNamespace: tc.pathWithNamespace,
388-
Logger: fakelogger,
389384
}
390385
event := info.NewEvent()
391386
event.Provider.Token = tc.providerToken

pkg/reconciler/reconciler_test.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,7 @@ func TestReconciler_ReconcileKind(t *testing.T) {
6666
defer teardown()
6767

6868
vcx := &ghprovider.Provider{
69-
Token: github.Ptr("None"),
70-
Logger: fakelogger,
69+
Token: github.Ptr("None"),
7170
}
7271

7372
vcx.SetGithubClient(fakeclient)

0 commit comments

Comments
 (0)