check remote system when creating user

This commit is contained in:
Brad Rydzewski 2019-09-28 14:27:37 -07:00
parent 6255396612
commit dce18f9afd
8 changed files with 130 additions and 14 deletions

View file

@ -19,7 +19,6 @@ import (
"github.com/drone/drone/service/org"
"github.com/drone/drone/service/token"
"github.com/drone/drone/service/user"
"github.com/drone/drone/store/batch"
"github.com/drone/drone/store/cron"
"github.com/drone/drone/store/perm"
"github.com/drone/drone/store/secret"
@ -86,11 +85,11 @@ func InitializeApplication(config2 config.Config) (application, error) {
if err != nil {
return application{}, err
}
batcher := batch.New(db)
batcher := provideBatchStore(db, config2)
syncer := provideSyncer(repositoryService, repositoryStore, userStore, batcher, config2)
server := api.New(buildStore, commitService, cronStore, corePubsub, globalSecretStore, hookService, logStore, coreLicense, licenseService, permStore, repositoryStore, repositoryService, scheduler, secretStore, stageStore, stepStore, statusService, session, logStream, syncer, system, triggerer, userStore, webhookSender)
userService := user.New(client, renewer)
server := api.New(buildStore, commitService, cronStore, corePubsub, globalSecretStore, hookService, logStore, coreLicense, licenseService, permStore, repositoryStore, repositoryService, scheduler, secretStore, stageStore, stepStore, statusService, session, logStream, syncer, system, triggerer, userStore, userService, webhookSender)
organizationService := orgs.New(client, renewer)
userService := user.New(client)
admissionService := provideAdmissionPlugin(client, organizationService, userService, config2)
hookParser := parser.New(client)
coreLinker := linker.New(client)

View file

@ -79,6 +79,9 @@ type (
UserService interface {
// Find returns the authenticated user.
Find(ctx context.Context, access, refresh string) (*User, error)
// FindLogin returns a user by username.
FindLogin(ctx context.Context, user *User, login string) (*User, error)
}
)

View file

@ -79,6 +79,7 @@ func New(
system *core.System,
triggerer core.Triggerer,
users core.UserStore,
userz core.UserService,
webhook core.WebhookSender,
) Server {
return Server{
@ -105,6 +106,7 @@ func New(
System: system,
Triggerer: triggerer,
Users: users,
Userz: userz,
Webhook: webhook,
}
}
@ -134,6 +136,7 @@ type Server struct {
System *core.System
Triggerer core.Triggerer
Users core.UserStore
Userz core.UserService
Webhook core.WebhookSender
}
@ -285,7 +288,7 @@ func (s Server) Handler() http.Handler {
r.Route("/users", func(r chi.Router) {
r.Use(acl.AuthorizeAdmin)
r.Get("/", users.HandleList(s.Users))
r.Post("/", users.HandleCreate(s.Users, s.Webhook))
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))

View file

@ -32,7 +32,7 @@ type userWithToken struct {
// HandleCreate returns an http.HandlerFunc that processes an http.Request
// to create the named user account in the system.
func HandleCreate(users core.UserStore, sender core.WebhookSender) http.HandlerFunc {
func HandleCreate(users core.UserStore, service core.UserService, sender core.WebhookSender) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
in := new(core.User)
err := json.NewDecoder(r.Body).Decode(in)
@ -64,6 +64,21 @@ func HandleCreate(users core.UserStore, sender core.WebhookSender) http.HandlerF
return
}
// if the user is not a machine account, we lookup
// the user in the remote system. We can then augment
// the user input with the remote system data.
if !user.Machine {
remote, err := service.FindLogin(r.Context(), nil, user.Login)
if err == nil {
if user.Login != remote.Login {
user.Login = remote.Login
}
if user.Email == "" {
user.Email = remote.Email
}
}
}
err = users.Create(r.Context(), user)
if err == core.ErrUserLimit {
render.ErrorCode(w, err, 402)

View file

@ -38,12 +38,68 @@ func TestCreate(t *testing.T) {
webhook := mock.NewMockWebhookSender(controller)
webhook.EXPECT().Send(gomock.Any(), gomock.Any()).Return(nil)
service := mock.NewMockUserService(controller)
service.EXPECT().FindLogin(gomock.Any(), gomock.Any(), "octocat").Return(nil, errors.New("not found"))
in := new(bytes.Buffer)
json.NewEncoder(in).Encode(&core.User{Login: "octocat"})
w := httptest.NewRecorder()
r := httptest.NewRequest("POST", "/", in)
HandleCreate(users, webhook)(w, r)
HandleCreate(users, service, webhook)(w, r)
if got, want := w.Code, 200; want != got {
t.Errorf("Want response code %d, got %d", want, got)
}
out := new(core.User)
json.NewDecoder(w.Body).Decode(out)
if got, want := out.Login, "octocat"; got != want {
t.Errorf("Want user login %s, got %s", want, got)
}
if got, want := out.Active, true; got != want {
t.Errorf("Want user active %v, got %v", want, got)
}
if got := out.Created; got == 0 {
t.Errorf("Want user created set to current unix timestamp, got %v", got)
}
if got := out.Updated; got == 0 {
t.Errorf("Want user updated set to current unix timestamp, got %v", got)
}
}
func TestCreate_CorrectName(t *testing.T) {
controller := gomock.NewController(t)
defer controller.Finish()
users := mock.NewMockUserStore(controller)
users.EXPECT().Create(gomock.Any(), gomock.Any()).Do(func(_ context.Context, in *core.User) error {
if got, want := in.Login, "octocat"; got != want {
t.Errorf("Want user login %s, got %s", want, got)
}
if got, want := in.Email, "octocat@github.com"; got != want {
t.Errorf("Want user email %s, got %s", want, got)
}
if in.Hash == "" {
t.Errorf("Expect user secert generated")
}
return nil
})
webhook := mock.NewMockWebhookSender(controller)
webhook.EXPECT().Send(gomock.Any(), gomock.Any()).Return(nil)
service := mock.NewMockUserService(controller)
service.EXPECT().FindLogin(gomock.Any(), gomock.Any(), "Octocat").Return(&core.User{
Login: "octocat",
Email: "octocat@github.com",
}, nil)
in := new(bytes.Buffer)
json.NewEncoder(in).Encode(&core.User{Login: "Octocat"})
w := httptest.NewRecorder()
r := httptest.NewRequest("POST", "/", in)
HandleCreate(users, service, webhook)(w, r)
if got, want := w.Code, 200; want != got {
t.Errorf("Want response code %d, got %d", want, got)
}
@ -72,7 +128,7 @@ func TestCreate_BadRequest(t *testing.T) {
w := httptest.NewRecorder()
r := httptest.NewRequest("POST", "/", in)
HandleCreate(nil, nil)(w, r)
HandleCreate(nil, nil, nil)(w, r)
if got, want := w.Code, http.StatusBadRequest; want != got {
t.Errorf("Want response code %d, got %d", want, got)
}
@ -93,12 +149,15 @@ func TestCreateError(t *testing.T) {
webhook := mock.NewMockWebhookSender(controller)
service := mock.NewMockUserService(controller)
service.EXPECT().FindLogin(gomock.Any(), gomock.Any(), "octocat").Return(nil, errors.New("not found"))
in := new(bytes.Buffer)
json.NewEncoder(in).Encode(&core.User{Login: "octocat"})
w := httptest.NewRecorder()
r := httptest.NewRequest("POST", "/", in)
HandleCreate(users, webhook)(w, r)
HandleCreate(users, service, webhook)(w, r)
if got, want := w.Code, http.StatusInternalServerError; want != got {
t.Errorf("Want response code %d, got %d", want, got)
}

View file

@ -240,6 +240,21 @@ func (mr *MockUserServiceMockRecorder) Find(arg0, arg1, arg2 interface{}) *gomoc
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Find", reflect.TypeOf((*MockUserService)(nil).Find), arg0, arg1, arg2)
}
// FindLogin mocks base method
func (m *MockUserService) FindLogin(arg0 context.Context, arg1 *core.User, arg2 string) (*core.User, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "FindLogin", arg0, arg1, arg2)
ret0, _ := ret[0].(*core.User)
ret1, _ := ret[1].(error)
return ret0, ret1
}
// FindLogin indicates an expected call of FindLogin
func (mr *MockUserServiceMockRecorder) FindLogin(arg0, arg1, arg2 interface{}) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "FindLogin", reflect.TypeOf((*MockUserService)(nil).FindLogin), arg0, arg1, arg2)
}
// MockRepositoryService is a mock of RepositoryService interface
type MockRepositoryService struct {
ctrl *gomock.Controller

View file

@ -23,12 +23,13 @@ import (
type service struct {
client *scm.Client
renew core.Renewer
}
// New returns a new User service that provides access to
// user data from the source code management system.
func New(client *scm.Client) core.UserService {
return &service{client: client}
func New(client *scm.Client, renew core.Renewer) core.UserService {
return &service{client: client, renew: renew}
}
func (s *service) Find(ctx context.Context, access, refresh string) (*core.User, error) {
@ -40,6 +41,27 @@ func (s *service) Find(ctx context.Context, access, refresh string) (*core.User,
if err != nil {
return nil, err
}
return convert(src), nil
}
func (s *service) FindLogin(ctx context.Context, user *core.User, login string) (*core.User, error) {
err := s.renew.Renew(ctx, user, false)
if err != nil {
return nil, err
}
ctx = context.WithValue(ctx, scm.TokenKey{}, &scm.Token{
Token: user.Token,
Refresh: user.Refresh,
})
src, _, err := s.client.Users.FindLogin(ctx, login)
if err != nil {
return nil, err
}
return convert(src), nil
}
func convert(src *scm.User) *core.User {
dst := &core.User{
Login: src.Login,
Email: src.Email,
@ -51,5 +73,5 @@ func (s *service) Find(ctx context.Context, access, refresh string) (*core.User,
if !src.Updated.IsZero() {
dst.Updated = src.Updated.Unix()
}
return dst, nil
return dst
}

View file

@ -59,7 +59,7 @@ func TestFind(t *testing.T) {
Created: now.Unix(),
Updated: now.Unix(),
}
got, err := New(client).Find(noContext, "755bb80e5b", "e08f3fa43e")
got, err := New(client, nil).Find(noContext, "755bb80e5b", "e08f3fa43e")
if err != nil {
t.Error(err)
}
@ -79,7 +79,7 @@ func TestFind_Error(t *testing.T) {
client := new(scm.Client)
client.Users = mockUsers
got, err := New(client).Find(noContext, "755bb80e5b", "e08f3fa43e")
got, err := New(client, nil).Find(noContext, "755bb80e5b", "e08f3fa43e")
if err == nil {
t.Errorf("Expect error finding user")
}