From d530046e8f00e417851978904a7d0d422607510e Mon Sep 17 00:00:00 2001 From: Eoin McAfee Date: Wed, 29 Sep 2021 15:03:02 +0100 Subject: [PATCH] remove auth changes & tech qa changes: --- core/card.go | 27 +--------------- handler/api/acl/check.go | 32 ------------------ handler/api/api.go | 6 ---- handler/api/card/create.go | 11 +++++-- handler/api/card/create_test.go | 57 +++------------------------------ store/card/card.go | 22 +++++++++---- store/card/card_test.go | 7 ++-- store/card/scan.go | 4 +-- 8 files changed, 36 insertions(+), 130 deletions(-) diff --git a/core/card.go b/core/card.go index 0547df169..563b22646 100644 --- a/core/card.go +++ b/core/card.go @@ -25,7 +25,6 @@ var ( errCardStepInvalid = errors.New("No Step ID Provided") errCardBuildInvalid = errors.New("No Build ID Provided") errCardSchemaInvalid = errors.New("No Card Schema Has Been Provided") - errCardDataInvalid = errors.New("No Card Data Has Been Provided") ) type Card struct { @@ -36,15 +35,6 @@ type Card struct { Schema string `json:"schema,omitempty"` } -type CreateCard struct { - Id int64 `json:"id,omitempty"` - Build int64 `json:"build,omitempty"` - Stage int64 `json:"stage,omitempty"` - Step int64 `json:"step,omitempty"` - Schema string `json:"schema,omitempty"` - Data []byte `json:"data,omitempty"` -} - type CardData struct { Id int64 `json:"id,omitempty"` Data []byte `json:"card_data"` @@ -55,7 +45,7 @@ type CardStore interface { FindCardByBuild(ctx context.Context, build int64) ([]*Card, error) FindCard(ctx context.Context, step int64) (*Card, error) FindCardData(ctx context.Context, id int64) (io.ReadCloser, error) - CreateCard(ctx context.Context, card *CreateCard) error + CreateCard(ctx context.Context, card *Card, data io.ReadCloser) error DeleteCard(ctx context.Context, id int64) error } @@ -72,18 +62,3 @@ func (c *Card) Validate() error { return nil } } - -func (c *CreateCard) Validate() error { - switch { - case c.Step == 0: - return errCardStepInvalid - case c.Build == 0: - return errCardBuildInvalid - case len(c.Schema) == 0: - return errCardSchemaInvalid - case len(c.Data) == 0: - return errCardDataInvalid - default: - return nil - } -} diff --git a/handler/api/acl/check.go b/handler/api/acl/check.go index 7a5f0fe97..6033f7759 100644 --- a/handler/api/acl/check.go +++ b/handler/api/acl/check.go @@ -15,9 +15,7 @@ package acl import ( - "github.com/dchest/authcookie" "net/http" - "os" "strings" "github.com/drone/drone/core" @@ -51,36 +49,6 @@ func CheckAdminAccess() func(http.Handler) http.Handler { return CheckAccess(true, true, true) } -func CheckInternalAccess() func(http.Handler) http.Handler { - return checkIAccess() -} - -// CheckIAccess returns an http.Handler middleware that authorizes only -// allows plugins to communicate with the server -func checkIAccess() func(http.Handler) http.Handler { - return func(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - var ( - owner = chi.URLParam(r, "owner") - name = chi.URLParam(r, "name") - ) - log := logger.FromRequest(r). - WithField("namespace", owner). - WithField("name", name) - - secret := []byte(os.Getenv("DRONE_INTERNAL_AUTH_SECRET")) - login := authcookie.Login(extractToken(r), secret) - if login != "" { - log.Debug("api: access granted") - next.ServeHTTP(w, r) - } - render.Unauthorized(w, errors.ErrUnauthorized) - log.Debugln("api: authentication required for access") - return - }) - } -} - // CheckAccess returns an http.Handler middleware that authorizes only // authenticated users with the required read, write or admin access // permissions to the requested repository resource. diff --git a/handler/api/api.go b/handler/api/api.go index 3727076c9..9e7908218 100644 --- a/handler/api/api.go +++ b/handler/api/api.go @@ -170,11 +170,6 @@ func (s Server) Handler() http.Handler { cors := cors.New(corsOpts) r.Use(cors.Handler) - r.Route("/internal/{owner}/{name}", func(r chi.Router) { - r.With( - acl.CheckInternalAccess(), - ).Post("/cards/{build}/{stage}/{step}", card.HandleCreate(s.Builds, s.Card, s.Stages, s.Steps, s.Repos)) - }) r.Route("/repos", func(r chi.Router) { // temporary workaround to enable private mode // for the drone server. @@ -300,7 +295,6 @@ func (s Server) Handler() http.Handler { r.Get("/{build}/{stage}/{step}", card.HandleFind(s.Builds, s.Card, s.Stages, s.Steps, s.Repos)) r.Get("/{build}/{stage}/{step}/json", card.HandleFindData(s.Builds, s.Card, s.Stages, s.Steps, s.Repos)) r.With( - acl.AuthorizeAdmin, acl.CheckAdminAccess(), ).Delete("/{build}/{stage}/{step}", card.HandleDelete(s.Builds, s.Card, s.Stages, s.Steps, s.Repos)) }) diff --git a/handler/api/card/create.go b/handler/api/card/create.go index 7c9aa1f99..849fb41c3 100644 --- a/handler/api/card/create.go +++ b/handler/api/card/create.go @@ -7,7 +7,9 @@ package card import ( + "bytes" "encoding/json" + "io/ioutil" "net/http" "strconv" @@ -83,12 +85,11 @@ func HandleCreate( return } - c := &core.CreateCard{ + c := &core.Card{ Build: build.ID, Stage: stage.ID, Step: step.ID, Schema: in.Schema, - Data: in.Data, } err = c.Validate() @@ -97,7 +98,11 @@ func HandleCreate( return } - err = cardStore.CreateCard(r.Context(), c) + data := ioutil.NopCloser( + bytes.NewBuffer([]byte(in.Data)), + ) + + err = cardStore.CreateCard(r.Context(), c, data) if err != nil { render.InternalError(w, err) return diff --git a/handler/api/card/create_test.go b/handler/api/card/create_test.go index 37760b7f7..0661a5f1b 100644 --- a/handler/api/card/create_test.go +++ b/handler/api/card/create_test.go @@ -43,9 +43,8 @@ var ( ID: 1, StageID: 1, } - dummyCreateCard = &core.CreateCard{ + dummyCreateCard = &core.Card{ Schema: "https://myschema.com", - Data: []byte("{\"type\": \"AdaptiveCard\"}"), } dummyCard = &core.Card{ Id: 1, @@ -54,14 +53,14 @@ var ( Step: 1, Schema: "https://myschema.com", } - dummyCardNoData = &core.CreateCard{ + dummyCardNoData = &core.Card{ Schema: "https://myschema.com", } dummyCardList = []*core.Card{ dummyCard, } dummyCardData = ioutil.NopCloser( - bytes.NewBuffer([]byte(dummyCreateCard.Data)), + bytes.NewBuffer([]byte("{\"type\": \"AdaptiveCard\"}")), ) ) @@ -82,7 +81,7 @@ func TestHandleCreate(t *testing.T) { step.EXPECT().FindNumber(gomock.Any(), dummyStage.ID, gomock.Any()).Return(dummyStep, nil) card := mock.NewMockCardStore(controller) - card.EXPECT().CreateCard(gomock.Any(), gomock.Any()).Return(nil) + card.EXPECT().CreateCard(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) c := new(chi.Context) c.URLParams.Add("owner", "octocat") @@ -106,52 +105,6 @@ func TestHandleCreate(t *testing.T) { } } -func TestHandleCreate_ValidationErrorData(t *testing.T) { - controller := gomock.NewController(t) - defer controller.Finish() - - repos := mock.NewMockRepositoryStore(controller) - repos.EXPECT().FindName(gomock.Any(), "octocat", "hello-world").Return(dummyRepo, nil) - - build := mock.NewMockBuildStore(controller) - build.EXPECT().FindNumber(gomock.Any(), dummyBuild.ID, gomock.Any()).Return(dummyBuild, nil) - - stage := mock.NewMockStageStore(controller) - stage.EXPECT().FindNumber(gomock.Any(), dummyBuild.ID, gomock.Any()).Return(dummyStage, nil) - - step := mock.NewMockStepStore(controller) - step.EXPECT().FindNumber(gomock.Any(), dummyStage.ID, gomock.Any()).Return(dummyStep, nil) - - card := mock.NewMockCardStore(controller) - - c := new(chi.Context) - c.URLParams.Add("owner", "octocat") - c.URLParams.Add("name", "hello-world") - c.URLParams.Add("build", "1") - c.URLParams.Add("stage", "1") - c.URLParams.Add("step", "1") - - in := new(bytes.Buffer) - json.NewEncoder(in).Encode(dummyCardNoData) - - w := httptest.NewRecorder() - r := httptest.NewRequest("POST", "/", in) - r = r.WithContext( - context.WithValue(context.Background(), chi.RouteCtxKey, c), - ) - - HandleCreate(build, card, stage, step, repos).ServeHTTP(w, r) - if got, want := w.Code, http.StatusBadRequest; want != got { - t.Errorf("Want response code %d, got %d", want, got) - } - - got, want := &errors.Error{}, &errors.Error{Message: "No Card Data Has Been Provided"} - json.NewDecoder(w.Body).Decode(got) - if diff := cmp.Diff(got, want); len(diff) != 0 { - t.Errorf(diff) - } -} - func TestHandleCreate_BadRequest(t *testing.T) { controller := gomock.NewController(t) defer controller.Finish() @@ -194,7 +147,7 @@ func TestHandleCreate_CreateError(t *testing.T) { step.EXPECT().FindNumber(gomock.Any(), dummyStage.ID, gomock.Any()).Return(dummyStep, nil) card := mock.NewMockCardStore(controller) - card.EXPECT().CreateCard(gomock.Any(), gomock.Any()).Return(errors.ErrNotFound) + card.EXPECT().CreateCard(gomock.Any(), gomock.Any(), gomock.Any()).Return(errors.ErrNotFound) c := new(chi.Context) c.URLParams.Add("owner", "octocat") diff --git a/store/card/card.go b/store/card/card.go index 52470b382..697b8cacf 100644 --- a/store/card/card.go +++ b/store/card/card.go @@ -81,16 +81,20 @@ func (c cardStore) FindCardData(ctx context.Context, id int64) (io.ReadCloser, e ), err } -func (c cardStore) CreateCard(ctx context.Context, card *core.CreateCard) error { +func (c cardStore) CreateCard(ctx context.Context, card *core.Card, data io.ReadCloser) error { if c.db.Driver() == db.Postgres { - return c.createPostgres(ctx, card) + return c.createPostgres(ctx, card, data) } - return c.create(ctx, card) + return c.create(ctx, card, data) } -func (c *cardStore) create(ctx context.Context, card *core.CreateCard) error { +func (c *cardStore) create(ctx context.Context, card *core.Card, data io.ReadCloser) error { return c.db.Lock(func(execer db.Execer, binder db.Binder) error { - params, err := toSaveCardParams(card) + cardData, err := ioutil.ReadAll(data) + if err != nil { + return err + } + params, err := toSaveCardParams(card, cardData) if err != nil { return err } @@ -107,9 +111,13 @@ func (c *cardStore) create(ctx context.Context, card *core.CreateCard) error { }) } -func (c *cardStore) createPostgres(ctx context.Context, card *core.CreateCard) error { +func (c *cardStore) createPostgres(ctx context.Context, card *core.Card, data io.ReadCloser) error { return c.db.Lock(func(execer db.Execer, binder db.Binder) error { - params, err := toSaveCardParams(card) + cardData, err := ioutil.ReadAll(data) + if err != nil { + return err + } + params, err := toSaveCardParams(card, cardData) if err != nil { return err } diff --git a/store/card/card_test.go b/store/card/card_test.go index 5bb962cdd..80a81b46d 100644 --- a/store/card/card_test.go +++ b/store/card/card_test.go @@ -1,6 +1,7 @@ package card import ( + "bytes" "context" "database/sql" "io/ioutil" @@ -52,9 +53,11 @@ func testCardCreate(store *cardStore) func(t *testing.T) { Stage: 2, Step: 3, Schema: "https://myschema.com", - Data: []byte("{\"type\": \"AdaptiveCard\"}"), } - err := store.CreateCard(noContext, item) + buf := ioutil.NopCloser( + bytes.NewBuffer([]byte("{\"type\": \"AdaptiveCard\"}")), + ) + err := store.CreateCard(noContext, item, buf) if err != nil { t.Error(err) } diff --git a/store/card/scan.go b/store/card/scan.go index 9993eecdf..cb1880b70 100644 --- a/store/card/scan.go +++ b/store/card/scan.go @@ -27,14 +27,14 @@ func toParams(card *core.Card) (map[string]interface{}, error) { // helper function converts the card structure to a set // of named query parameters. -func toSaveCardParams(card *core.CreateCard) (map[string]interface{}, error) { +func toSaveCardParams(card *core.CreateCard, data []byte) (map[string]interface{}, error) { return map[string]interface{}{ "card_id": card.Id, "card_build": card.Build, "card_stage": card.Stage, "card_step": card.Step, "card_schema": card.Schema, - "card_data": card.Data, + "card_data": data, }, nil }