From 712010d013454ab905921caf7a50eb2ec2328066 Mon Sep 17 00:00:00 2001 From: Kelvin Wang Date: Mon, 1 Apr 2019 12:16:37 -0400 Subject: [PATCH] if(influxdb): updated auth shall return --- auth.go | 2 +- authorizer/auth.go | 6 +++--- authorizer/auth_test.go | 6 +++--- bolt/authorization.go | 22 ++++++++++++--------- cmd/influx/authorization.go | 16 ++++++++-------- http/auth_service.go | 22 +++++++++++++-------- inmem/auth_service.go | 8 ++++---- kv/auth.go | 34 ++++++++++++++------------------- mock/auth_service.go | 8 +++++--- prometheus/auth_service.go | 2 +- prometheus/auth_service_test.go | 4 ++-- testing/auth.go | 5 ++++- zap/auth_service.go | 2 +- 13 files changed, 73 insertions(+), 64 deletions(-) diff --git a/auth.go b/auth.go index 3354a59c6d..e41fdb3ded 100644 --- a/auth.go +++ b/auth.go @@ -104,7 +104,7 @@ type AuthorizationService interface { CreateAuthorization(ctx context.Context, a *Authorization) error // UpdateAuthorization updates the status and description if available. - UpdateAuthorization(ctx context.Context, id ID, udp *AuthorizationUpdate) error + UpdateAuthorization(ctx context.Context, id ID, udp *AuthorizationUpdate) (*Authorization, error) // Removes a authorization by token. DeleteAuthorization(ctx context.Context, id ID) error diff --git a/authorizer/auth.go b/authorizer/auth.go index f842e2d9fd..a20d33a173 100644 --- a/authorizer/auth.go +++ b/authorizer/auth.go @@ -144,14 +144,14 @@ func VerifyPermissions(ctx context.Context, ps []influxdb.Permission) error { } // UpdateAuthorization checks to see if the authorizer on context has write access to the authorization provided. -func (s *AuthorizationService) UpdateAuthorization(ctx context.Context, id influxdb.ID, upd *influxdb.AuthorizationUpdate) error { +func (s *AuthorizationService) UpdateAuthorization(ctx context.Context, id influxdb.ID, upd *influxdb.AuthorizationUpdate) (*influxdb.Authorization, error) { a, err := s.s.FindAuthorizationByID(ctx, id) if err != nil { - return err + return nil, err } if err := authorizeWriteAuthorization(ctx, a.UserID); err != nil { - return err + return nil, err } return s.s.UpdateAuthorization(ctx, id, upd) diff --git a/authorizer/auth_test.go b/authorizer/auth_test.go index 8d6c007e32..faa4516b59 100644 --- a/authorizer/auth_test.go +++ b/authorizer/auth_test.go @@ -244,8 +244,8 @@ func TestAuthorizationService_WriteAuthorization(t *testing.T) { m.DeleteAuthorizationFn = func(ctx context.Context, id influxdb.ID) error { return nil } - m.UpdateAuthorizationFn = func(ctx context.Context, id influxdb.ID, upd *influxdb.AuthorizationUpdate) error { - return nil + m.UpdateAuthorizationFn = func(ctx context.Context, id influxdb.ID, upd *influxdb.AuthorizationUpdate) (*influxdb.Authorization, error) { + return nil, nil } s := authorizer.NewAuthorizationService(m) @@ -258,7 +258,7 @@ func TestAuthorizationService_WriteAuthorization(t *testing.T) { }) t.Run("update authorization", func(t *testing.T) { - err := s.UpdateAuthorization(ctx, 10, &influxdb.AuthorizationUpdate{Status: influxdb.Active.Ptr()}) + _, err := s.UpdateAuthorization(ctx, 10, &influxdb.AuthorizationUpdate{Status: influxdb.Active.Ptr()}) influxdbtesting.ErrorsEqual(t, err, tt.wants.err) }) diff --git a/bolt/authorization.go b/bolt/authorization.go index 938ef02192..e46f2179ed 100644 --- a/bolt/authorization.go +++ b/bolt/authorization.go @@ -383,9 +383,12 @@ func (c *Client) deleteAuthorization(ctx context.Context, tx *bolt.Tx, id platfo } // UpdateAuthorization updates the status and description if available. -func (c *Client) UpdateAuthorization(ctx context.Context, id platform.ID, upd *platform.AuthorizationUpdate) error { - return c.db.Update(func(tx *bolt.Tx) error { - if pe := c.updateAuthorization(ctx, tx, id, upd); pe != nil { +func (c *Client) UpdateAuthorization(ctx context.Context, id platform.ID, upd *platform.AuthorizationUpdate) (*platform.Authorization, error) { + var a *platform.Authorization + err := c.db.Update(func(tx *bolt.Tx) error { + var pe *platform.Error + a, pe = c.updateAuthorization(ctx, tx, id, upd) + if pe != nil { return &platform.Error{ Err: pe, Op: platform.OpUpdateAuthorization, @@ -393,12 +396,13 @@ func (c *Client) UpdateAuthorization(ctx context.Context, id platform.ID, upd *p } return nil }) + return a, err } -func (c *Client) updateAuthorization(ctx context.Context, tx *bolt.Tx, id platform.ID, upd *platform.AuthorizationUpdate) *platform.Error { +func (c *Client) updateAuthorization(ctx context.Context, tx *bolt.Tx, id platform.ID, upd *platform.AuthorizationUpdate) (*platform.Authorization, *platform.Error) { a, pe := c.findAuthorizationByID(ctx, tx, id) if pe != nil { - return pe + return nil, pe } if upd.Status != nil { @@ -410,22 +414,22 @@ func (c *Client) updateAuthorization(ctx context.Context, tx *bolt.Tx, id platfo b, err := encodeAuthorization(a) if err != nil { - return &platform.Error{ + return nil, &platform.Error{ Err: err, } } encodedID, err := id.Encode() if err != nil { - return &platform.Error{ + return nil, &platform.Error{ Err: err, } } if err = tx.Bucket(authorizationBucket).Put(encodedID, b); err != nil { - return &platform.Error{ + return nil, &platform.Error{ Err: err, } } - return nil + return a, nil } diff --git a/cmd/influx/authorization.go b/cmd/influx/authorization.go index 76463cc93a..897609e030 100644 --- a/cmd/influx/authorization.go +++ b/cmd/influx/authorization.go @@ -456,14 +456,14 @@ func authorizationActiveF(cmd *cobra.Command, args []string) error { } ctx := context.TODO() - a, err := s.FindAuthorizationByID(ctx, id) - if err != nil { + if _, err := s.FindAuthorizationByID(ctx, id); err != nil { return err } - if err := s.UpdateAuthorization(context.Background(), id, &platform.AuthorizationUpdate{ + a, err := s.UpdateAuthorization(context.Background(), id, &platform.AuthorizationUpdate{ Status: platform.Active.Ptr(), - }); err != nil { + }) + if err != nil { return err } @@ -527,14 +527,14 @@ func authorizationInactiveF(cmd *cobra.Command, args []string) error { } ctx := context.TODO() - a, err := s.FindAuthorizationByID(ctx, id) - if err != nil { + if _, err = s.FindAuthorizationByID(ctx, id); err != nil { return err } - if err := s.UpdateAuthorization(context.Background(), id, &platform.AuthorizationUpdate{ + a, err := s.UpdateAuthorization(context.Background(), id, &platform.AuthorizationUpdate{ Status: platform.Inactive.Ptr(), - }); err != nil { + }) + if err != nil { return err } diff --git a/http/auth_service.go b/http/auth_service.go index b84875b23c..fb9251255b 100644 --- a/http/auth_service.go +++ b/http/auth_service.go @@ -476,7 +476,8 @@ func (h *AuthorizationHandler) handleUpdateAuthorization(w http.ResponseWriter, return } - if err := h.AuthorizationService.UpdateAuthorization(ctx, a.ID, req.AuthorizationUpdate); err != nil { + a, err = h.AuthorizationService.UpdateAuthorization(ctx, a.ID, req.AuthorizationUpdate) + if err != nil { EncodeError(ctx, err, w) return } @@ -743,20 +744,20 @@ func (s *AuthorizationService) CreateAuthorization(ctx context.Context, a *platf } // UpdateAuthorization updates the status and description if available. -func (s *AuthorizationService) UpdateAuthorization(ctx context.Context, id platform.ID, upd *platform.AuthorizationUpdate) error { +func (s *AuthorizationService) UpdateAuthorization(ctx context.Context, id platform.ID, upd *platform.AuthorizationUpdate) (*platform.Authorization, error) { u, err := newURL(s.Addr, authorizationIDPath(id)) if err != nil { - return err + return nil, err } b, err := json.Marshal(upd) if err != nil { - return err + return nil, err } req, err := http.NewRequest("PATCH", u.String(), bytes.NewReader(b)) if err != nil { - return err + return nil, err } req.Header.Set("Content-Type", "application/json") @@ -766,15 +767,20 @@ func (s *AuthorizationService) UpdateAuthorization(ctx context.Context, id platf resp, err := hc.Do(req) if err != nil { - return err + return nil, err } defer resp.Body.Close() if err := CheckError(resp); err != nil { - return err + return nil, err } - return nil + var res authResponse + if err := json.NewDecoder(resp.Body).Decode(&res); err != nil { + return nil, err + } + + return res.toPlatform(), nil } // DeleteAuthorization removes a authorization by id. diff --git a/inmem/auth_service.go b/inmem/auth_service.go index 381a513319..8deec1c982 100644 --- a/inmem/auth_service.go +++ b/inmem/auth_service.go @@ -193,11 +193,11 @@ func (s *Service) DeleteAuthorization(ctx context.Context, id platform.ID) error } // UpdateAuthorization updates the status and description if available. -func (s *Service) UpdateAuthorization(ctx context.Context, id platform.ID, upd *platform.AuthorizationUpdate) error { +func (s *Service) UpdateAuthorization(ctx context.Context, id platform.ID, upd *platform.AuthorizationUpdate) (*platform.Authorization, error) { op := OpPrefix + platform.OpUpdateAuthorization a, err := s.FindAuthorizationByID(ctx, id) if err != nil { - return &platform.Error{ + return nil, &platform.Error{ Err: err, Op: op, } @@ -208,7 +208,7 @@ func (s *Service) UpdateAuthorization(ctx context.Context, id platform.ID, upd * switch status { case platform.Active, platform.Inactive: default: - return &platform.Error{ + return nil, &platform.Error{ Code: platform.EInvalid, Msg: "unknown authorization status", Op: op, @@ -221,5 +221,5 @@ func (s *Service) UpdateAuthorization(ctx context.Context, id platform.ID, upd * a.Description = *upd.Description } - return s.PutAuthorization(ctx, a) + return a, s.PutAuthorization(ctx, a) } diff --git a/kv/auth.go b/kv/auth.go index 24ed313601..350f2628ec 100644 --- a/kv/auth.go +++ b/kv/auth.go @@ -412,27 +412,21 @@ func (s *Service) deleteAuthorization(ctx context.Context, tx Tx, id influxdb.ID return nil } -// SetAuthorizationStatus updates the status of the authorization. Useful -// for setting an authorization to inactive or active. -func (s *Service) SetAuthorizationStatus(ctx context.Context, id influxdb.ID, status influxdb.Status) error { - return s.kv.Update(ctx, func(tx Tx) error { - return s.updateAuthorization(ctx, tx, id, &influxdb.AuthorizationUpdate{ - Status: &status, - }) - }) -} - // UpdateAuthorization updates the status and description if available. -func (s *Service) UpdateAuthorization(ctx context.Context, id influxdb.ID, upd *influxdb.AuthorizationUpdate) error { - return s.kv.Update(ctx, func(tx Tx) error { - return s.updateAuthorization(ctx, tx, id, upd) +func (s *Service) UpdateAuthorization(ctx context.Context, id influxdb.ID, upd *influxdb.AuthorizationUpdate) (*influxdb.Authorization, error) { + var a *influxdb.Authorization + var err error + err = s.kv.Update(ctx, func(tx Tx) error { + a, err = s.updateAuthorization(ctx, tx, id, upd) + return err }) + return a, err } -func (s *Service) updateAuthorization(ctx context.Context, tx Tx, id influxdb.ID, upd *influxdb.AuthorizationUpdate) error { +func (s *Service) updateAuthorization(ctx context.Context, tx Tx, id influxdb.ID, upd *influxdb.AuthorizationUpdate) (*influxdb.Authorization, error) { a, err := s.findAuthorizationByID(ctx, tx, id) if err != nil { - return err + return nil, err } if upd.Status != nil { @@ -444,29 +438,29 @@ func (s *Service) updateAuthorization(ctx context.Context, tx Tx, id influxdb.ID v, err := encodeAuthorization(a) if err != nil { - return &influxdb.Error{ + return nil, &influxdb.Error{ Err: err, } } encodedID, err := id.Encode() if err != nil { - return &influxdb.Error{ + return nil, &influxdb.Error{ Err: err, } } b, err := tx.Bucket(authBucket) if err != nil { - return err + return nil, err } if err = b.Put(encodedID, v); err != nil { - return &influxdb.Error{ + return nil, &influxdb.Error{ Err: err, } } - return nil + return a, nil } func authIndexBucket(tx Tx) (Bucket, error) { diff --git a/mock/auth_service.go b/mock/auth_service.go index d0b4562078..aaffa66623 100644 --- a/mock/auth_service.go +++ b/mock/auth_service.go @@ -21,7 +21,7 @@ type AuthorizationService struct { FindAuthorizationsFn func(context.Context, platform.AuthorizationFilter, ...platform.FindOptions) ([]*platform.Authorization, int, error) CreateAuthorizationFn func(context.Context, *platform.Authorization) error DeleteAuthorizationFn func(context.Context, platform.ID) error - UpdateAuthorizationFn func(context.Context, platform.ID, *platform.AuthorizationUpdate) error + UpdateAuthorizationFn func(context.Context, platform.ID, *platform.AuthorizationUpdate) (*platform.Authorization, error) } // NewAuthorizationService returns a mock AuthorizationService where its methods will return @@ -35,7 +35,9 @@ func NewAuthorizationService() *AuthorizationService { }, CreateAuthorizationFn: func(context.Context, *platform.Authorization) error { return nil }, DeleteAuthorizationFn: func(context.Context, platform.ID) error { return nil }, - UpdateAuthorizationFn: func(context.Context, platform.ID, *platform.AuthorizationUpdate) error { return nil }, + UpdateAuthorizationFn: func(context.Context, platform.ID, *platform.AuthorizationUpdate) (*platform.Authorization, error) { + return nil, nil + }, } } @@ -64,6 +66,6 @@ func (s *AuthorizationService) DeleteAuthorization(ctx context.Context, id platf } // UpdateAuthorization updates the status and description if available. -func (s *AuthorizationService) UpdateAuthorization(ctx context.Context, id platform.ID, upd *platform.AuthorizationUpdate) error { +func (s *AuthorizationService) UpdateAuthorization(ctx context.Context, id platform.ID, upd *platform.AuthorizationUpdate) (*platform.Authorization, error) { return s.UpdateAuthorizationFn(ctx, id, upd) } diff --git a/prometheus/auth_service.go b/prometheus/auth_service.go index c45ae92529..025f3bafd5 100644 --- a/prometheus/auth_service.go +++ b/prometheus/auth_service.go @@ -110,7 +110,7 @@ func (s *AuthorizationService) DeleteAuthorization(ctx context.Context, id platf } // UpdateAuthorization updates the status and description. -func (s *AuthorizationService) UpdateAuthorization(ctx context.Context, id platform.ID, upd *platform.AuthorizationUpdate) (err error) { +func (s *AuthorizationService) UpdateAuthorization(ctx context.Context, id platform.ID, upd *platform.AuthorizationUpdate) (a *platform.Authorization, err error) { defer func(start time.Time) { labels := prometheus.Labels{ "method": "setAuthorizationStatus", diff --git a/prometheus/auth_service_test.go b/prometheus/auth_service_test.go index 59daa4198a..b1ef540782 100644 --- a/prometheus/auth_service_test.go +++ b/prometheus/auth_service_test.go @@ -38,8 +38,8 @@ func (a *authzSvc) DeleteAuthorization(context.Context, platform.ID) error { return a.Err } -func (a *authzSvc) UpdateAuthorization(context.Context, platform.ID, *platform.AuthorizationUpdate) error { - return a.Err +func (a *authzSvc) UpdateAuthorization(context.Context, platform.ID, *platform.AuthorizationUpdate) (*platform.Authorization, error) { + return nil, a.Err } func TestAuthorizationService_Metrics(t *testing.T) { diff --git a/testing/auth.go b/testing/auth.go index 42275839e9..0547074e34 100644 --- a/testing/auth.go +++ b/testing/auth.go @@ -682,7 +682,7 @@ func UpdateAuthorization( defer done() ctx := context.Background() - err := s.UpdateAuthorization(ctx, tt.args.id, tt.args.upd) + updatedAuth, err := s.UpdateAuthorization(ctx, tt.args.id, tt.args.upd) diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) if tt.wants.err == nil { @@ -693,6 +693,9 @@ func UpdateAuthorization( if diff := cmp.Diff(authorization, tt.wants.authorization, authorizationCmpOptions...); diff != "" { t.Errorf("authorization is different -got/+want\ndiff %s", diff) } + if diff := cmp.Diff(authorization, updatedAuth, authorizationCmpOptions...); diff != "" { + t.Errorf("authorization is different -got/+want\ndiff %s", diff) + } } }) } diff --git a/zap/auth_service.go b/zap/auth_service.go index e651d5e7d2..4b39475fd2 100644 --- a/zap/auth_service.go +++ b/zap/auth_service.go @@ -71,7 +71,7 @@ func (s *AuthorizationService) DeleteAuthorization(ctx context.Context, id platf } // UpdateAuthorization updates an authorization's status, description and logs any errors. -func (s *AuthorizationService) UpdateAuthorization(ctx context.Context, id platform.ID, upd *platform.AuthorizationUpdate) (err error) { +func (s *AuthorizationService) UpdateAuthorization(ctx context.Context, id platform.ID, upd *platform.AuthorizationUpdate) (a *platform.Authorization, err error) { defer func() { if err != nil { s.Logger.Info("error updating authorization", zap.Error(err))