darkkirb/harness-drone
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

re-assign repository ownership when user deleted or deactivated

Browse source
This commit is contained in:
Brad Rydzewski 2020-06-08 14:32:28 -04:00
parent fcaea8f4ea
commit 840f58dc7f
13 changed files with 396 additions and 67 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
cmd/drone-server/inject_service.go
View file

@ -36,6 +36,7 @@ import (
"github.com/drone/drone/service/status"
"github.com/drone/drone/service/syncer"
"github.com/drone/drone/service/token"
"github.com/drone/drone/service/transfer"
"github.com/drone/drone/service/user"
"github.com/drone/drone/session"
"github.com/drone/drone/trigger"
@ -56,6 +57,7 @@ var serviceSet = wire.NewSet(
parser.New,
pubsub.New,
token.Renewer,
transfer.New,
trigger.New,
user.New,

4
cmd/drone-server/wire_gen.go
View file

@ -18,6 +18,7 @@ import (
"github.com/drone/drone/service/license"
"github.com/drone/drone/service/linker"
"github.com/drone/drone/service/token"
"github.com/drone/drone/service/transfer"
"github.com/drone/drone/service/user"
"github.com/drone/drone/store/cron"
"github.com/drone/drone/store/perm"
@ -90,8 +91,9 @@ func InitializeApplication(config2 config.Config) (application, error) {
}
batcher := provideBatchStore(db, config2)
syncer := provideSyncer(repositoryService, repositoryStore, userStore, batcher, config2)
transferer := transfer.New(repositoryStore, permStore)
userService := user.New(client, renewer)
server := api.New(buildStore, commitService, cronStore, corePubsub, globalSecretStore, hookService, logStore, coreLicense, licenseService, organizationService, permStore, repositoryStore, repositoryService, scheduler, secretStore, stageStore, stepStore, statusService, session, logStream, syncer, system, triggerer, userStore, userService, webhookSender)
server := api.New(buildStore, commitService, cronStore, corePubsub, globalSecretStore, hookService, logStore, coreLicense, licenseService, organizationService, permStore, repositoryStore, repositoryService, scheduler, secretStore, stageStore, stepStore, statusService, session, logStream, syncer, system, transferer, triggerer, userStore, userService, webhookSender)
admissionService := provideAdmissionPlugin(client, organizationService, userService, config2)
hookParser := parser.New(client)
coreLinker := linker.New(client)

23
core/transfer.go Normal file
View file

@ -0,0 +1,23 @@
// Copyright 2019 Drone IO, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package core
import "context"
// Transferer handles transfering repository ownership from one
// user to another user account.
type Transferer interface {
Transfer(ctx context.Context, user *User) error
}

2
go.sum
View file

@ -295,6 +295,7 @@ github.com/peterbourgon/diskv v2.0.1+incompatible h1:UBdAOUP5p4RWqPBg048CAvpKN+v
github.com/peterbourgon/diskv v2.0.1+incompatible/go.mod h1:uqqh8zWWbv1HBMNONnaR/tNboyR3/BZd58JJSHlUSCU=
github.com/pkg/errors v0.8.0 h1:WdK/asTD0HN+q6hsWO3/vpuAkAr+tw6aNJNDFFf0+qw=
github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/prometheus/client_golang v0.8.0 h1:1921Yw9Gc3iSc4VQh3PIoOqgPCZS7G/4xQNVUp8Mda8=
github.com/prometheus/client_golang v0.8.0/go.mod h1:7SWBe2y4D6OKWSNQJUaRYU/AaXPKyh/dDVn+NZz0KFw=
@ -319,6 +320,7 @@ github.com/spf13/pflag v1.0.3/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnIn
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q=
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
github.com/unrolled/secure v0.0.0-20181022170031-4b6b7cf51606 h1:dU9yXzNi9rl6Mou7+3npdfPyeFPb2+7BHs3zL47bhPY=
github.com/unrolled/secure v0.0.0-20181022170031-4b6b7cf51606/go.mod h1:mnPT77IAdsi/kV7+Es7y+pXALeV3h7G6dQF6mNYjcLA=

7
handler/api/api.go
View file

@ -82,6 +82,7 @@ func New(
stream core.LogStream,
syncer core.Syncer,
system *core.System,
transferer core.Transferer,
triggerer core.Triggerer,
users core.UserStore,
userz core.UserService,
@ -110,6 +111,7 @@ func New(
Stream: stream,
Syncer: syncer,
System: system,
Transferer: transferer,
Triggerer: triggerer,
Users: users,
Userz: userz,
@ -141,6 +143,7 @@ type Server struct {
Stream core.LogStream
Syncer core.Syncer
System *core.System
Transferer core.Transferer
Triggerer core.Triggerer
Users core.UserStore
Userz core.UserService
@ -318,8 +321,8 @@ func (s Server) Handler() http.Handler {
r.Get("/", users.HandleList(s.Users))
r.Post("/", users.HandleCreate(s.Users, s.Userz, s.Webhook))
r.Get("/{user}", users.HandleFind(s.Users))
r.Patch("/{user}", users.HandleUpdate(s.Users))
r.Delete("/{user}", users.HandleDelete(s.Users, s.Webhook))
r.Patch("/{user}", users.HandleUpdate(s.Users, s.Transferer))
r.Delete("/{user}", users.HandleDelete(s.Users, s.Transferer, s.Webhook))
r.Get("/{user}/repos", users.HandleRepoList(s.Users, s.Repos))
})

8
handler/api/users/delete.go
View file

@ -15,6 +15,7 @@
package users
import (
"context"
"net/http"
"github.com/drone/drone/core"
@ -28,6 +29,7 @@ import (
// to delete the named user account from the system.
func HandleDelete(
users core.UserStore,
transferer core.Transferer,
sender core.WebhookSender,
) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
@ -40,6 +42,12 @@ func HandleDelete(
return
}
err = transferer.Transfer(context.Background(), user)
if err != nil {
logger.FromRequest(r).WithError(err).
Warnln("api: cannot transfer repository ownership")
}
err = users.Delete(r.Context(), user)
if err != nil {
render.InternalError(w, err)

12
handler/api/users/delete_test.go
View file

@ -25,6 +25,9 @@ func TestUserDelete(t *testing.T) {
users.EXPECT().FindLogin(gomock.Any(), mockUser.Login).Return(mockUser, nil)
users.EXPECT().Delete(gomock.Any(), mockUser).Return(nil)
transferer := mock.NewMockTransferer(controller)
transferer.EXPECT().Transfer(gomock.Any(), mockUser).Return(nil)
webhook := mock.NewMockWebhookSender(controller)
webhook.EXPECT().Send(gomock.Any(), gomock.Any()).Return(nil)
@ -37,7 +40,7 @@ func TestUserDelete(t *testing.T) {
context.WithValue(context.Background(), chi.RouteCtxKey, c),
)
HandleDelete(users, webhook)(w, r)
HandleDelete(users, transferer, webhook)(w, r)
if got, want := w.Body.Len(), 0; want != got {
t.Errorf("Want response body size %d, got %d", want, got)
}
@ -64,7 +67,7 @@ func TestUserDelete_NotFound(t *testing.T) {
context.WithValue(context.Background(), chi.RouteCtxKey, c),
)
HandleDelete(users, webhook)(w, r)
HandleDelete(users, nil, webhook)(w, r)
if got, want := w.Code, 404; want != got {
t.Errorf("Want response code %d, got %d", want, got)
}
@ -78,6 +81,9 @@ func TestUserDelete_InternalError(t *testing.T) {
users.EXPECT().FindLogin(gomock.Any(), mockUser.Login).Return(mockUser, nil)
users.EXPECT().Delete(gomock.Any(), mockUser).Return(sql.ErrConnDone)
transferer := mock.NewMockTransferer(controller)
transferer.EXPECT().Transfer(gomock.Any(), mockUser).Return(nil)
webhook := mock.NewMockWebhookSender(controller)
c := new(chi.Context)
@ -89,7 +95,7 @@ func TestUserDelete_InternalError(t *testing.T) {
context.WithValue(context.Background(), chi.RouteCtxKey, c),
)
HandleDelete(users, webhook)(w, r)
HandleDelete(users, transferer, webhook)(w, r)
if got, want := w.Code, http.StatusInternalServerError; want != got {
t.Errorf("Want response code %d, got %d", want, got)
}

13
handler/api/users/update.go
View file

@ -15,6 +15,7 @@
package users
import (
"context"
"encoding/json"
"net/http"
@ -32,7 +33,7 @@ type userInput struct {
// HandleUpdate returns an http.HandlerFunc that processes an http.Request
// to update a user account.
func HandleUpdate(users core.UserStore) http.HandlerFunc {
func HandleUpdate(users core.UserStore, transferer core.Transferer) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
login := chi.URLParam(r, "user")
@ -73,5 +74,15 @@ func HandleUpdate(users core.UserStore) http.HandlerFunc {
} else {
render.JSON(w, user, 200)
}
if user.Active {
return
}
err = transferer.Transfer(context.Background(), user)
if err != nil {
logger.FromRequest(r).WithError(err).
Warnln("api: cannot transfer repository ownership")
}
}
}

13
handler/api/users/update_test.go
View file

@ -13,9 +13,9 @@ import (
"net/http/httptest"
"testing"
"github.com/drone/drone/core"
"github.com/drone/drone/handler/api/errors"
"github.com/drone/drone/mock"
"github.com/drone/drone/core"
"github.com/go-chi/chi"
"github.com/golang/mock/gomock"
@ -39,6 +39,9 @@ func TestUpdate(t *testing.T) {
users.EXPECT().FindLogin(gomock.Any(), user.Login).Return(user, nil)
users.EXPECT().Update(gomock.Any(), user)
transferer := mock.NewMockTransferer(controller)
transferer.EXPECT().Transfer(gomock.Any(), user).Return(nil)
c := new(chi.Context)
c.URLParams.Add("user", "octocat")
@ -50,7 +53,7 @@ func TestUpdate(t *testing.T) {
context.WithValue(context.Background(), chi.RouteCtxKey, c),
)
HandleUpdate(users)(w, r)
HandleUpdate(users, transferer)(w, r)
if got, want := w.Code, 200; want != got {
t.Errorf("Want response code %d, got %d", want, got)
}
@ -82,7 +85,7 @@ func TestUpdate_BadRequest(t *testing.T) {
context.WithValue(context.Background(), chi.RouteCtxKey, c),
)
HandleUpdate(users)(w, r)
HandleUpdate(users, nil)(w, r)
if got, want := w.Code, 400; want != got {
t.Errorf("Want response code %d, got %d", want, got)
}
@ -112,7 +115,7 @@ func TestUpdate_NotFound(t *testing.T) {
context.WithValue(context.Background(), chi.RouteCtxKey, c),
)
HandleUpdate(users)(w, r)
HandleUpdate(users, nil)(w, r)
if got, want := w.Code, 404; want != got {
t.Errorf("Want response code %d, got %d", want, got)
}
@ -152,7 +155,7 @@ func TestUpdate_UpdateFailed(t *testing.T) {
context.WithValue(context.Background(), chi.RouteCtxKey, c),
)
HandleUpdate(users)(w, r)
HandleUpdate(users, nil)(w, r)
if got, want := w.Code, http.StatusInternalServerError; want != got {
t.Errorf("Want response code %d, got %d", want, got)
}

2
mock/mock.go
View file

@ -6,4 +6,4 @@
package mock
//go:generate mockgen -package=mock -destination=mock_gen.go github.com/drone/drone/core Pubsub,Canceler,ConvertService,ValidateService,NetrcService,Renewer,HookParser,UserService,RepositoryService,CommitService,StatusService,HookService,FileService,Batcher,BuildStore,CronStore,LogStore,PermStore,SecretStore,GlobalSecretStore,StageStore,StepStore,RepositoryStore,UserStore,Scheduler,Session,OrganizationService,SecretService,RegistryService,ConfigService,Triggerer,Syncer,LogStream,WebhookSender,LicenseService
//go:generate mockgen -package=mock -destination=mock_gen.go github.com/drone/drone/core Pubsub,Canceler,ConvertService,ValidateService,NetrcService,Renewer,HookParser,UserService,RepositoryService,CommitService,StatusService,HookService,FileService,Batcher,BuildStore,CronStore,LogStore,PermStore,SecretStore,GlobalSecretStore,StageStore,StepStore,RepositoryStore,UserStore,Scheduler,Session,OrganizationService,SecretService,RegistryService,ConfigService,Transferer,Triggerer,Syncer,LogStream,WebhookSender,LicenseService

39
mock/mock_gen.go
View file

@ -1,5 +1,5 @@
// Code generated by MockGen. DO NOT EDIT.
// Source: github.com/drone/drone/core (interfaces: Pubsub,Canceler,ConvertService,ValidateService,NetrcService,Renewer,HookParser,UserService,RepositoryService,CommitService,StatusService,HookService,FileService,Batcher,BuildStore,CronStore,LogStore,PermStore,SecretStore,GlobalSecretStore,StageStore,StepStore,RepositoryStore,UserStore,Scheduler,Session,OrganizationService,SecretService,RegistryService,ConfigService,Triggerer,Syncer,LogStream,WebhookSender,LicenseService)
// Source: github.com/drone/drone/core (interfaces: Pubsub,Canceler,ConvertService,ValidateService,NetrcService,Renewer,HookParser,UserService,RepositoryService,CommitService,StatusService,HookService,FileService,Batcher,BuildStore,CronStore,LogStore,PermStore,SecretStore,GlobalSecretStore,StageStore,StepStore,RepositoryStore,UserStore,Scheduler,Session,OrganizationService,SecretService,RegistryService,ConfigService,Transferer,Triggerer,Syncer,LogStream,WebhookSender,LicenseService)
// Package mock is a generated GoMock package.
package mock
@ -2442,6 +2442,43 @@ func (mr *MockConfigServiceMockRecorder) Find(arg0, arg1 interface{}) *gomock.Ca
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Find", reflect.TypeOf((*MockConfigService)(nil).Find), arg0, arg1)
}
// MockTransferer is a mock of Transferer interface
type MockTransferer struct {
ctrl *gomock.Controller
recorder *MockTransfererMockRecorder
}
// MockTransfererMockRecorder is the mock recorder for MockTransferer
type MockTransfererMockRecorder struct {
mock *MockTransferer
}
// NewMockTransferer creates a new mock instance
func NewMockTransferer(ctrl *gomock.Controller) *MockTransferer {
mock := &MockTransferer{ctrl: ctrl}
mock.recorder = &MockTransfererMockRecorder{mock}
return mock
}
// EXPECT returns an object that allows the caller to indicate expected use
func (m *MockTransferer) EXPECT() *MockTransfererMockRecorder {
return m.recorder
}
// Transfer mocks base method
func (m *MockTransferer) Transfer(arg0 context.Context, arg1 *core.User) error {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "Transfer", arg0, arg1)
ret0, _ := ret[0].(error)
return ret0
}
// Transfer indicates an expected call of Transfer
func (mr *MockTransfererMockRecorder) Transfer(arg0, arg1 interface{}) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Transfer", reflect.TypeOf((*MockTransferer)(nil).Transfer), arg0, arg1)
}
// MockTriggerer is a mock of Triggerer interface
type MockTriggerer struct {
ctrl *gomock.Controller

113
service/transfer/transfer.go Normal file
View file

@ -0,0 +1,113 @@
// Copyright 2020 Drone IO, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package transfer
import (
"context"
"runtime/debug"
"github.com/drone/drone/core"
"github.com/hashicorp/go-multierror"
"github.com/sirupsen/logrus"
)
// Transferer handles transfering repository ownership from one
// user to another user account.
type Transferer struct {
Repos core.RepositoryStore
Perms core.PermStore
}
// New returns a new repository transfer service.
func New(repos core.RepositoryStore, perms core.PermStore) core.Transferer {
return &Transferer{
Repos: repos,
Perms: perms,
}
}
// Transfer transfers all repositories owned by the specified user
// to an alternate account with sufficient admin permissions.
func (t *Transferer) Transfer(ctx context.Context, user *core.User) error {
defer func() {
// taking the paranoid approach to recover from
// a panic that should absolutely never happen.
if r := recover(); r != nil {
logrus.Errorf("transferer: unexpected panic: %s", r)
debug.PrintStack()
}
}()
repos, err := t.Repos.List(ctx, user.ID)
if err != nil {
return err
}
var result error
for _, repo := range repos {
// only transfer repository ownership if the deactivated
// user owns the repository.
if repo.UserID != user.ID {
continue
}
members, err := t.Perms.List(ctx, repo.UID)
if err != nil {
result = multierror.Append(result, err)
continue
}
var admin int64
for _, member := range members {
// only transfer the repository to an admin user
// that is not equal to the deactivated user.
if repo.UserID == member.UserID {
continue
}
if member.Admin {
admin = member.UserID
break
}
}
if admin == 0 {
logrus.
WithField("repo.id", repo.ID).
WithField("repo.namespace", repo.Namespace).
WithField("repo.name", repo.Name).
Traceln("repository disabled")
} else {
logrus.
WithField("repo.id", repo.ID).
WithField("repo.namespace", repo.Namespace).
WithField("repo.name", repo.Name).
WithField("old.user.id", repo.UserID).
WithField("new.user.id", admin).
Traceln("repository owner re-assigned")
}
// if no alternate user was found the repository id
// is reset to the zero value, indicating the repository
// has no owner.
repo.UserID = admin
err = t.Repos.Update(ctx, repo)
if err != nil {
result = multierror.Append(result, err)
}
}
return result
}

119
service/transfer/transfer_test.go Normal file
View file

@ -0,0 +1,119 @@
// Copyright 2020 Drone.IO Inc. All rights reserved.
// Use of this source code is governed by the Drone Non-Commercial License
// that can be found in the LICENSE file.
package transfer
import (
"context"
"testing"
"github.com/drone/drone/core"
"github.com/drone/drone/mock"
"github.com/golang/mock/gomock"
)
var nocontext = context.Background()
func TestTransfer(t *testing.T) {
controller := gomock.NewController(t)
defer controller.Finish()
mockRepo := &core.Repository{
ID: 1,
UserID: 2,
UID: "123",
}
mockRepos := []*core.Repository{
mockRepo,
}
mockCollabs := []*core.Collaborator{
{
UserID: 1, // do not match non-admin
Admin: false,
},
{
UserID: 2, // do not match existing owner
Admin: true,
},
{
UserID: 3,
Admin: true,
},
}
mockUser := &core.User{
ID: 2,
}
checkRepo := func(ctx context.Context, updated *core.Repository) error {
if updated.UserID != 3 {
t.Errorf("Expect repository owner id assigned to user id 3")
}
return nil
}
repos := mock.NewMockRepositoryStore(controller)
repos.EXPECT().List(gomock.Any(), mockUser.ID).Return(mockRepos, nil).Times(1)
repos.EXPECT().Update(gomock.Any(), mockRepo).Do(checkRepo).Times(1)
perms := mock.NewMockPermStore(controller)
perms.EXPECT().List(gomock.Any(), mockRepo.UID).Return(mockCollabs, nil).Times(1)
r := New(
repos,
perms,
)
err := r.Transfer(nocontext, mockUser)
if err != nil {
t.Error(err)
}
}
func TestTransfer_NoOwner(t *testing.T) {
controller := gomock.NewController(t)
defer controller.Finish()
mockRepo := &core.Repository{
ID: 1,
UserID: 2,
UID: "123",
}
mockRepos := []*core.Repository{
mockRepo,
}
mockCollabs := []*core.Collaborator{
{
UserID: 2, // same user id
Admin: true,
},
}
mockUser := &core.User{
ID: 2,
}
checkRepo := func(ctx context.Context, updated *core.Repository) error {
if updated.UserID != 0 {
t.Errorf("Expect repository owner id reset to zero value")
}
return nil
}
repos := mock.NewMockRepositoryStore(controller)
repos.EXPECT().List(gomock.Any(), mockUser.ID).Return(mockRepos, nil).Times(1)
repos.EXPECT().Update(gomock.Any(), mockRepo).Do(checkRepo).Times(1)
perms := mock.NewMockPermStore(controller)
perms.EXPECT().List(gomock.Any(), mockRepo.UID).Return(mockCollabs, nil).Times(1)
r := New(
repos,
perms,
)
err := r.Transfer(nocontext, mockUser)
if err != nil {
t.Error(err)
}
}