Skip to content

Commit cec7dc7

Browse files
committed
Ensure token is valid before proceeding (#1803)
Make sure the token is valid before proceeding with the rest of the code. Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com>
1 parent c02d5d2 commit cec7dc7

File tree

4 files changed

+71
-19
lines changed

4 files changed

+71
-19
lines changed

pkg/provider/bitbucketserver/acl_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,14 +203,15 @@ func TestIsAllowed(t *testing.T) {
203203
for _, tt := range tests {
204204
t.Run(tt.name, func(t *testing.T) {
205205
ctx, _ := rtesting.SetupFakeContext(t)
206-
bbclient, mux, tearDown := bbv1test.SetupBBServerClient(ctx)
206+
bbclient, mux, tearDown, tURL := bbv1test.SetupBBServerClient(ctx)
207207
defer tearDown()
208208
bbv1test.MuxProjectMemberShip(t, mux, tt.event, tt.fields.projectMembers)
209209
bbv1test.MuxRepoMemberShip(t, mux, tt.event, tt.fields.repoMembers)
210210
bbv1test.MuxPullRequestActivities(t, mux, tt.event, tt.fields.pullRequestNumber, tt.fields.activities)
211211
bbv1test.MuxFiles(t, mux, tt.event, tt.fields.defaultBranchLatestCommit, "", tt.fields.filescontents)
212212

213213
v := &Provider{
214+
baseURL: tURL,
214215
Client: bbclient,
215216
defaultBranchLatestCommit: tt.fields.defaultBranchLatestCommit,
216217
pullRequestNumber: tt.fields.pullRequestNumber,

pkg/provider/bitbucketserver/bitbucketserver.go

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package bitbucketserver
33
import (
44
"context"
55
"fmt"
6+
"net/http"
67
"path/filepath"
78
"strings"
89

@@ -215,13 +216,13 @@ func (v *Provider) GetFileInsideRepo(_ context.Context, event *info.Event, path,
215216

216217
func (v *Provider) SetClient(ctx context.Context, run *params.Run, event *info.Event, _ *v1alpha1.Repository, _ *events.EventEmitter) error {
217218
if event.Provider.User == "" {
218-
return fmt.Errorf("no provider.user has been set in the repo crd")
219+
return fmt.Errorf("no spec.git_provider.user has been set in the repo crd")
219220
}
220221
if event.Provider.Token == "" {
221-
return fmt.Errorf("no provider.secret has been set in the repo crd")
222+
return fmt.Errorf("no spec.git_provider.secret has been set in the repo crd")
222223
}
223224
if event.Provider.URL == "" {
224-
return fmt.Errorf("no provider.url has been set in the repo crd")
225+
return fmt.Errorf("no spec.git_provider.url has been set in the repo crd")
225226
}
226227

227228
// make sure we have /rest at the end of the url
@@ -237,8 +238,17 @@ func (v *Provider) SetClient(ctx context.Context, run *params.Run, event *info.E
237238

238239
ctx = context.WithValue(ctx, bbv1.ContextBasicAuth, basicAuth)
239240
cfg := bbv1.NewConfiguration(event.Provider.URL)
240-
v.Client = bbv1.NewAPIClient(ctx, cfg)
241+
if v.Client == nil {
242+
v.Client = bbv1.NewAPIClient(ctx, cfg)
243+
}
241244
v.run = run
245+
resp, err := v.Client.DefaultApi.GetUser(event.Provider.User)
246+
if resp.Response != nil && resp.StatusCode == http.StatusUnauthorized {
247+
return fmt.Errorf("cannot get user %s with token: %w", event.Provider.User, err)
248+
}
249+
if err != nil {
250+
return fmt.Errorf("cannot get user %s: %w", event.Provider.User, err)
251+
}
242252

243253
return nil
244254
}

pkg/provider/bitbucketserver/bitbucketserver_test.go

Lines changed: 53 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,9 @@ func TestGetTektonDir(t *testing.T) {
5555
observer, _ := zapobserver.New(zap.InfoLevel)
5656
logger := zap.New(observer).Sugar()
5757
ctx, _ := rtesting.SetupFakeContext(t)
58-
client, mux, tearDown := bbtest.SetupBBServerClient(ctx)
58+
client, mux, tearDown, tURL := bbtest.SetupBBServerClient(ctx)
5959
defer tearDown()
60-
v := &Provider{Logger: logger, Client: client, projectKey: tt.event.Organization}
60+
v := &Provider{Logger: logger, baseURL: tURL, Client: client, projectKey: tt.event.Organization}
6161
bbtest.MuxDirContent(t, mux, tt.event, tt.testDirPath, tt.path)
6262
content, err := v.GetTektonDir(ctx, tt.event, tt.path, "")
6363
if tt.wantErr {
@@ -168,7 +168,7 @@ func TestCreateStatus(t *testing.T) {
168168
for _, tt := range tests {
169169
t.Run(tt.name, func(t *testing.T) {
170170
ctx, _ := rtesting.SetupFakeContext(t)
171-
client, mux, tearDown := bbtest.SetupBBServerClient(ctx)
171+
client, mux, tearDown, tURL := bbtest.SetupBBServerClient(ctx)
172172
defer tearDown()
173173
if tt.nilClient {
174174
client = nil
@@ -177,6 +177,7 @@ func TestCreateStatus(t *testing.T) {
177177
event.EventType = "pull_request"
178178
event.Provider.Token = "token"
179179
v := &Provider{
180+
baseURL: tURL,
180181
Client: client,
181182
pullRequestNumber: pullRequestNumber,
182183
projectKey: event.Organization,
@@ -229,9 +230,9 @@ func TestGetFileInsideRepo(t *testing.T) {
229230
for _, tt := range tests {
230231
t.Run(tt.name, func(t *testing.T) {
231232
ctx, _ := rtesting.SetupFakeContext(t)
232-
client, mux, tearDown := bbtest.SetupBBServerClient(ctx)
233+
client, mux, tearDown, tURL := bbtest.SetupBBServerClient(ctx)
233234
defer tearDown()
234-
v := &Provider{Client: client, defaultBranchLatestCommit: "1234", projectKey: tt.event.Organization}
235+
v := &Provider{Client: client, baseURL: tURL, defaultBranchLatestCommit: "1234", projectKey: tt.event.Organization}
235236
bbtest.MuxFiles(t, mux, tt.event, tt.targetbranch, filepath.Dir(tt.path), tt.filescontents)
236237
fc, err := v.GetFileInsideRepo(ctx, tt.event, tt.path, tt.targetbranch)
237238
assert.NilError(t, err)
@@ -246,11 +247,12 @@ func TestSetClient(t *testing.T) {
246247
apiURL string
247248
opts *info.Event
248249
wantErrSubstr string
250+
muxUser func(w http.ResponseWriter, r *http.Request)
249251
}{
250252
{
251253
name: "bad/no username",
252254
opts: info.NewEvent(),
253-
wantErrSubstr: "no provider.user",
255+
wantErrSubstr: "no spec.git_provider.user",
254256
},
255257
{
256258
name: "bad/no secret",
@@ -259,7 +261,7 @@ func TestSetClient(t *testing.T) {
259261
User: "foo",
260262
},
261263
},
262-
wantErrSubstr: "no provider.secret",
264+
wantErrSubstr: "no spec.git_provider.secret",
263265
},
264266
{
265267
name: "bad/no url",
@@ -269,7 +271,39 @@ func TestSetClient(t *testing.T) {
269271
Token: "bar",
270272
},
271273
},
272-
wantErrSubstr: "no provider.url",
274+
wantErrSubstr: "no spec.git_provider.url",
275+
},
276+
{
277+
name: "bad/invalid user",
278+
opts: &info.Event{
279+
Provider: &info.Provider{
280+
User: "foo",
281+
Token: "bar",
282+
URL: "https://foo.bar",
283+
},
284+
},
285+
muxUser: func(w http.ResponseWriter, _ *http.Request) {
286+
w.WriteHeader(http.StatusUnauthorized)
287+
_, _ = w.Write([]byte(`{"errors": [{"message": "Unauthorized"}]}`))
288+
},
289+
apiURL: "https://foo.bar/rest",
290+
wantErrSubstr: "cannot get user foo with token",
291+
},
292+
{
293+
name: "bad/unknown error",
294+
opts: &info.Event{
295+
Provider: &info.Provider{
296+
User: "foo",
297+
Token: "bar",
298+
URL: "https://foo.bar",
299+
},
300+
},
301+
muxUser: func(w http.ResponseWriter, _ *http.Request) {
302+
w.WriteHeader(http.StatusInternalServerError)
303+
_, _ = w.Write([]byte(`{"errors": [{"message": "Internal Server Error"}]}`))
304+
},
305+
apiURL: "https://foo.bar/rest",
306+
wantErrSubstr: "cannot get user foo: Status: 500",
273307
},
274308
{
275309
name: "good/url append /rest",
@@ -280,13 +314,21 @@ func TestSetClient(t *testing.T) {
280314
URL: "https://foo.bar",
281315
},
282316
},
317+
muxUser: func(w http.ResponseWriter, _ *http.Request) {
318+
fmt.Fprint(w, `{"name": "foo"}`)
319+
},
283320
apiURL: "https://foo.bar/rest",
284321
},
285322
}
286323
for _, tt := range tests {
287324
t.Run(tt.name, func(t *testing.T) {
288325
ctx, _ := rtesting.SetupFakeContext(t)
289-
v := &Provider{}
326+
bbclient, mux, tearDown, tURL := bbtest.SetupBBServerClient(ctx)
327+
defer tearDown()
328+
if tt.muxUser != nil {
329+
mux.HandleFunc("/users/foo", tt.muxUser)
330+
}
331+
v := &Provider{Client: bbclient, baseURL: tURL}
290332
err := v.SetClient(ctx, nil, tt.opts, nil, nil)
291333
if tt.wantErrSubstr != "" {
292334
assert.ErrorContains(t, err, tt.wantErrSubstr)
@@ -299,7 +341,6 @@ func TestSetClient(t *testing.T) {
299341
}
300342

301343
func TestGetCommitInfo(t *testing.T) {
302-
defaultBaseURL := "https://base"
303344
tests := []struct {
304345
name string
305346
event *info.Event
@@ -325,11 +366,11 @@ func TestGetCommitInfo(t *testing.T) {
325366
for _, tt := range tests {
326367
t.Run(tt.name, func(t *testing.T) {
327368
ctx, _ := rtesting.SetupFakeContext(t)
328-
bbclient, mux, tearDown := bbtest.SetupBBServerClient(ctx)
369+
bbclient, mux, tearDown, tURL := bbtest.SetupBBServerClient(ctx)
329370
bbtest.MuxCommitInfo(t, mux, tt.event, tt.commit)
330371
bbtest.MuxDefaultBranch(t, mux, tt.event, tt.defaultBranch, tt.latestCommit)
331372
defer tearDown()
332-
v := &Provider{Client: bbclient, baseURL: defaultBaseURL, projectKey: tt.event.Organization}
373+
v := &Provider{Client: bbclient, baseURL: tURL, projectKey: tt.event.Organization}
333374
err := v.GetCommitInfo(ctx, tt.event)
334375
assert.NilError(t, err)
335376
assert.Equal(t, tt.defaultBranch, tt.event.DefaultBranch)

pkg/provider/bitbucketserver/test/test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ var (
2525
buildAPIURL = "/build-status/1.0"
2626
)
2727

28-
func SetupBBServerClient(ctx context.Context) (*bbv1.APIClient, *http.ServeMux, func()) {
28+
func SetupBBServerClient(ctx context.Context) (*bbv1.APIClient, *http.ServeMux, func(), string) {
2929
mux := http.NewServeMux()
3030
apiHandler := http.NewServeMux()
3131
apiHandler.Handle(defaultAPIURL+"/", http.StripPrefix(defaultAPIURL, mux))
@@ -49,7 +49,7 @@ func SetupBBServerClient(ctx context.Context) (*bbv1.APIClient, *http.ServeMux,
4949
cfg := bbv1.NewConfiguration(server.URL)
5050
cfg.HTTPClient = server.Client()
5151
client := bbv1.NewAPIClient(ctx, cfg)
52-
return client, mux, tearDown
52+
return client, mux, tearDown, server.URL
5353
}
5454

5555
func MuxCreateComment(t *testing.T, mux *http.ServeMux, event *info.Event, expectedCommentSubstr string, prID int) {

0 commit comments

Comments
 (0)