Skip to content

Commit

Permalink
Ensure token is valid before proceeding (#1803)
Browse files Browse the repository at this point in the history
Make sure the token is valid before proceeding with the rest of the
code.

Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com>
  • Loading branch information
chmouel authored Oct 31, 2024
1 parent e4648f2 commit 549b2d8
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 19 deletions.
3 changes: 2 additions & 1 deletion pkg/provider/bitbucketserver/acl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,14 +203,15 @@ func TestIsAllowed(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ctx, _ := rtesting.SetupFakeContext(t)
bbclient, mux, tearDown := bbv1test.SetupBBServerClient(ctx)
bbclient, mux, tearDown, tURL := bbv1test.SetupBBServerClient(ctx)
defer tearDown()
bbv1test.MuxProjectMemberShip(t, mux, tt.event, tt.fields.projectMembers)
bbv1test.MuxRepoMemberShip(t, mux, tt.event, tt.fields.repoMembers)
bbv1test.MuxPullRequestActivities(t, mux, tt.event, tt.fields.pullRequestNumber, tt.fields.activities)
bbv1test.MuxFiles(t, mux, tt.event, tt.fields.defaultBranchLatestCommit, "", tt.fields.filescontents)

v := &Provider{
baseURL: tURL,
Client: bbclient,
defaultBranchLatestCommit: tt.fields.defaultBranchLatestCommit,
pullRequestNumber: tt.fields.pullRequestNumber,
Expand Down
18 changes: 14 additions & 4 deletions pkg/provider/bitbucketserver/bitbucketserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package bitbucketserver
import (
"context"
"fmt"
"net/http"
"path/filepath"
"strings"

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

func (v *Provider) SetClient(ctx context.Context, run *params.Run, event *info.Event, _ *v1alpha1.Repository, _ *events.EventEmitter) error {
if event.Provider.User == "" {
return fmt.Errorf("no provider.user has been set in the repo crd")
return fmt.Errorf("no spec.git_provider.user has been set in the repo crd")
}
if event.Provider.Token == "" {
return fmt.Errorf("no provider.secret has been set in the repo crd")
return fmt.Errorf("no spec.git_provider.secret has been set in the repo crd")
}
if event.Provider.URL == "" {
return fmt.Errorf("no provider.url has been set in the repo crd")
return fmt.Errorf("no spec.git_provider.url has been set in the repo crd")
}

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

ctx = context.WithValue(ctx, bbv1.ContextBasicAuth, basicAuth)
cfg := bbv1.NewConfiguration(event.Provider.URL)
v.Client = bbv1.NewAPIClient(ctx, cfg)
if v.Client == nil {
v.Client = bbv1.NewAPIClient(ctx, cfg)
}
v.run = run
resp, err := v.Client.DefaultApi.GetUser(event.Provider.User)
if resp.Response != nil && resp.StatusCode == http.StatusUnauthorized {
return fmt.Errorf("cannot get user %s with token: %w", event.Provider.User, err)
}
if err != nil {
return fmt.Errorf("cannot get user %s: %w", event.Provider.User, err)
}

return nil
}
Expand Down
65 changes: 53 additions & 12 deletions pkg/provider/bitbucketserver/bitbucketserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ func TestGetTektonDir(t *testing.T) {
observer, _ := zapobserver.New(zap.InfoLevel)
logger := zap.New(observer).Sugar()
ctx, _ := rtesting.SetupFakeContext(t)
client, mux, tearDown := bbtest.SetupBBServerClient(ctx)
client, mux, tearDown, tURL := bbtest.SetupBBServerClient(ctx)
defer tearDown()
v := &Provider{Logger: logger, Client: client, projectKey: tt.event.Organization}
v := &Provider{Logger: logger, baseURL: tURL, Client: client, projectKey: tt.event.Organization}
bbtest.MuxDirContent(t, mux, tt.event, tt.testDirPath, tt.path)
content, err := v.GetTektonDir(ctx, tt.event, tt.path, "")
if tt.wantErr {
Expand Down Expand Up @@ -168,7 +168,7 @@ func TestCreateStatus(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ctx, _ := rtesting.SetupFakeContext(t)
client, mux, tearDown := bbtest.SetupBBServerClient(ctx)
client, mux, tearDown, tURL := bbtest.SetupBBServerClient(ctx)
defer tearDown()
if tt.nilClient {
client = nil
Expand All @@ -177,6 +177,7 @@ func TestCreateStatus(t *testing.T) {
event.EventType = "pull_request"
event.Provider.Token = "token"
v := &Provider{
baseURL: tURL,
Client: client,
pullRequestNumber: pullRequestNumber,
projectKey: event.Organization,
Expand Down Expand Up @@ -229,9 +230,9 @@ func TestGetFileInsideRepo(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ctx, _ := rtesting.SetupFakeContext(t)
client, mux, tearDown := bbtest.SetupBBServerClient(ctx)
client, mux, tearDown, tURL := bbtest.SetupBBServerClient(ctx)
defer tearDown()
v := &Provider{Client: client, defaultBranchLatestCommit: "1234", projectKey: tt.event.Organization}
v := &Provider{Client: client, baseURL: tURL, defaultBranchLatestCommit: "1234", projectKey: tt.event.Organization}
bbtest.MuxFiles(t, mux, tt.event, tt.targetbranch, filepath.Dir(tt.path), tt.filescontents)
fc, err := v.GetFileInsideRepo(ctx, tt.event, tt.path, tt.targetbranch)
assert.NilError(t, err)
Expand All @@ -246,11 +247,12 @@ func TestSetClient(t *testing.T) {
apiURL string
opts *info.Event
wantErrSubstr string
muxUser func(w http.ResponseWriter, r *http.Request)
}{
{
name: "bad/no username",
opts: info.NewEvent(),
wantErrSubstr: "no provider.user",
wantErrSubstr: "no spec.git_provider.user",
},
{
name: "bad/no secret",
Expand All @@ -259,7 +261,7 @@ func TestSetClient(t *testing.T) {
User: "foo",
},
},
wantErrSubstr: "no provider.secret",
wantErrSubstr: "no spec.git_provider.secret",
},
{
name: "bad/no url",
Expand All @@ -269,7 +271,39 @@ func TestSetClient(t *testing.T) {
Token: "bar",
},
},
wantErrSubstr: "no provider.url",
wantErrSubstr: "no spec.git_provider.url",
},
{
name: "bad/invalid user",
opts: &info.Event{
Provider: &info.Provider{
User: "foo",
Token: "bar",
URL: "https://foo.bar",
},
},
muxUser: func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusUnauthorized)
_, _ = w.Write([]byte(`{"errors": [{"message": "Unauthorized"}]}`))
},
apiURL: "https://foo.bar/rest",
wantErrSubstr: "cannot get user foo with token",
},
{
name: "bad/unknown error",
opts: &info.Event{
Provider: &info.Provider{
User: "foo",
Token: "bar",
URL: "https://foo.bar",
},
},
muxUser: func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusInternalServerError)
_, _ = w.Write([]byte(`{"errors": [{"message": "Internal Server Error"}]}`))
},
apiURL: "https://foo.bar/rest",
wantErrSubstr: "cannot get user foo: Status: 500",
},
{
name: "good/url append /rest",
Expand All @@ -280,13 +314,21 @@ func TestSetClient(t *testing.T) {
URL: "https://foo.bar",
},
},
muxUser: func(w http.ResponseWriter, _ *http.Request) {
fmt.Fprint(w, `{"name": "foo"}`)
},
apiURL: "https://foo.bar/rest",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ctx, _ := rtesting.SetupFakeContext(t)
v := &Provider{}
bbclient, mux, tearDown, tURL := bbtest.SetupBBServerClient(ctx)
defer tearDown()
if tt.muxUser != nil {
mux.HandleFunc("/users/foo", tt.muxUser)
}
v := &Provider{Client: bbclient, baseURL: tURL}
err := v.SetClient(ctx, nil, tt.opts, nil, nil)
if tt.wantErrSubstr != "" {
assert.ErrorContains(t, err, tt.wantErrSubstr)
Expand All @@ -299,7 +341,6 @@ func TestSetClient(t *testing.T) {
}

func TestGetCommitInfo(t *testing.T) {
defaultBaseURL := "https://base"
tests := []struct {
name string
event *info.Event
Expand All @@ -325,11 +366,11 @@ func TestGetCommitInfo(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ctx, _ := rtesting.SetupFakeContext(t)
bbclient, mux, tearDown := bbtest.SetupBBServerClient(ctx)
bbclient, mux, tearDown, tURL := bbtest.SetupBBServerClient(ctx)
bbtest.MuxCommitInfo(t, mux, tt.event, tt.commit)
bbtest.MuxDefaultBranch(t, mux, tt.event, tt.defaultBranch, tt.latestCommit)
defer tearDown()
v := &Provider{Client: bbclient, baseURL: defaultBaseURL, projectKey: tt.event.Organization}
v := &Provider{Client: bbclient, baseURL: tURL, projectKey: tt.event.Organization}
err := v.GetCommitInfo(ctx, tt.event)
assert.NilError(t, err)
assert.Equal(t, tt.defaultBranch, tt.event.DefaultBranch)
Expand Down
4 changes: 2 additions & 2 deletions pkg/provider/bitbucketserver/test/test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ var (
buildAPIURL = "/build-status/1.0"
)

func SetupBBServerClient(ctx context.Context) (*bbv1.APIClient, *http.ServeMux, func()) {
func SetupBBServerClient(ctx context.Context) (*bbv1.APIClient, *http.ServeMux, func(), string) {
mux := http.NewServeMux()
apiHandler := http.NewServeMux()
apiHandler.Handle(defaultAPIURL+"/", http.StripPrefix(defaultAPIURL, mux))
Expand All @@ -49,7 +49,7 @@ func SetupBBServerClient(ctx context.Context) (*bbv1.APIClient, *http.ServeMux,
cfg := bbv1.NewConfiguration(server.URL)
cfg.HTTPClient = server.Client()
client := bbv1.NewAPIClient(ctx, cfg)
return client, mux, tearDown
return client, mux, tearDown, server.URL
}

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

0 comments on commit 549b2d8

Please sign in to comment.