Support multiple users with same login name but different forges (#5612)

Co-authored-by: 6543 <6543@obermui.de>
Co-authored-by: Robert Kaussow <mail@thegeeklab.de>

server/api/login.go

@@ -160,13 +160,29 @@ func HandleAuth(c *gin.Context) {
 		}
 	}
 
+	var user *model.User
+
 	// get the user from the database
-	user, err := _store.GetUserRemoteID(userFromForge.ForgeRemoteID, userFromForge.Login)
+	user, err = _store.GetUserByRemoteID(forgeID, userFromForge.ForgeRemoteID)
 	if err != nil && !errors.Is(err, types.RecordNotExist) {
 		log.Error().Err(err).Msgf("cannot get user %s", userFromForge.Login)
 		c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+"/login?error=internal_error")
 		return
 	}
+	// update user login (in case forge supports renaming)
+	if user != nil {
+		user.Login = userFromForge.Login
+	}
+
+	// re-try with login name
+	if user == nil || errors.Is(err, types.RecordNotExist) {
+		user, err = _store.GetUserByLogin(forgeID, userFromForge.Login)
+		if err != nil && !errors.Is(err, types.RecordNotExist) {
+			log.Error().Err(err).Msgf("cannot get user %s", userFromForge.Login)
+			c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+"/login?error=internal_error")
+			return
+		}
+	}
 
 	if user == nil || errors.Is(err, types.RecordNotExist) {
 		// if self-registration is disabled we should return a not authorized error

server/api/login_test.go

@@ -158,7 +158,8 @@ func TestHandleAuth(t *testing.T) {
 
 		_manager.On("ForgeByID", int64(1)).Return(_forge, nil)
 		_forge.On("Login", mock.Anything, mock.Anything).Return(user, "", nil)
-		_store.On("GetUserRemoteID", user.ForgeRemoteID, user.Login).Return(nil, types.RecordNotExist)
+		_store.On("GetUserByRemoteID", user.ForgeID, user.ForgeRemoteID).Return(nil, types.RecordNotExist)
+		_store.On("GetUserByLogin", user.ForgeID, user.Login).Return(nil, types.RecordNotExist)
 		_store.On("CreateUser", mock.Anything).Return(nil)
 		_store.On("OrgFindByName", user.Login, user.ForgeID).Return(nil, nil)
 		_store.On("OrgCreate", mock.Anything).Return(nil)
@@ -192,7 +193,7 @@ func TestHandleAuth(t *testing.T) {
 
 		_manager.On("ForgeByID", int64(1)).Return(_forge, nil)
 		_forge.On("Login", mock.Anything, mock.Anything).Return(user, "", nil)
-		_store.On("GetUserRemoteID", user.ForgeRemoteID, user.Login).Return(user, nil)
+		_store.On("GetUserByRemoteID", user.ForgeID, user.ForgeRemoteID).Return(user, nil)
 		_store.On("OrgGet", org.ID).Return(org, nil)
 		_store.On("UpdateUser", mock.Anything).Return(nil)
 		_forge.On("Repos", mock.Anything, mock.Anything, mock.Anything).Return(nil, nil)
@@ -224,7 +225,8 @@ func TestHandleAuth(t *testing.T) {
 
 		_manager.On("ForgeByID", int64(1)).Return(_forge, nil)
 		_forge.On("Login", mock.Anything, mock.Anything).Return(user, "", nil)
-		_store.On("GetUserRemoteID", user.ForgeRemoteID, user.Login).Return(nil, types.RecordNotExist)
+		_store.On("GetUserByRemoteID", user.ForgeID, user.ForgeRemoteID).Return(nil, types.RecordNotExist)
+		_store.On("GetUserByLogin", user.ForgeID, user.Login).Return(nil, types.RecordNotExist)
 
 		api.HandleAuth(c)
 
@@ -285,7 +287,7 @@ func TestHandleAuth(t *testing.T) {
 
 		_manager.On("ForgeByID", int64(1)).Return(_forge, nil)
 		_forge.On("Login", mock.Anything, mock.Anything).Return(user, "", nil)
-		_store.On("GetUserRemoteID", user.ForgeRemoteID, user.Login).Return(user, nil)
+		_store.On("GetUserByRemoteID", user.ForgeID, user.ForgeRemoteID).Return(user, nil)
 		_store.On("OrgFindByName", user.Login, user.ForgeID).Return(nil, types.RecordNotExist)
 		_store.On("OrgCreate", mock.Anything).Return(nil)
 		_store.On("UpdateUser", mock.Anything).Return(nil)
@@ -319,7 +321,7 @@ func TestHandleAuth(t *testing.T) {
 
 		_manager.On("ForgeByID", int64(1)).Return(_forge, nil)
 		_forge.On("Login", mock.Anything, mock.Anything).Return(user, "", nil)
-		_store.On("GetUserRemoteID", user.ForgeRemoteID, user.Login).Return(user, nil)
+		_store.On("GetUserByRemoteID", user.ForgeID, user.ForgeRemoteID).Return(user, nil)
 		_store.On("OrgFindByName", user.Login, user.ForgeID).Return(org, nil)
 		_store.On("OrgUpdate", mock.Anything).Return(nil)
 		_store.On("UpdateUser", mock.Anything).Return(nil)
@@ -353,7 +355,7 @@ func TestHandleAuth(t *testing.T) {
 
 		_manager.On("ForgeByID", int64(1)).Return(_forge, nil)
 		_forge.On("Login", mock.Anything, mock.Anything).Return(user, "", nil)
-		_store.On("GetUserRemoteID", user.ForgeRemoteID, user.Login).Return(user, nil)
+		_store.On("GetUserByRemoteID", user.ForgeID, user.ForgeRemoteID).Return(user, nil)
 		_store.On("OrgGet", user.OrgID).Return(org, nil)
 		_store.On("OrgUpdate", mock.Anything).Return(nil)
 		_store.On("UpdateUser", mock.Anything).Return(nil)

server/api/users.go

@@ -16,7 +16,10 @@ package api
 
 import (
 	"encoding/base32"
+	"errors"
+	"fmt"
 	"net/http"
+	"strconv"
 
 	"github.com/gin-gonic/gin"
 	"github.com/google/tink/go/subtle/random"
@@ -24,8 +27,11 @@ import (
 	"go.woodpecker-ci.org/woodpecker/v3/server/model"
 	"go.woodpecker-ci.org/woodpecker/v3/server/router/middleware/session"
 	"go.woodpecker-ci.org/woodpecker/v3/server/store"
+	"go.woodpecker-ci.org/woodpecker/v3/server/store/types"
 )
 
+const defaultForgeID = 1
+
 // GetUsers
 //
 //	@Summary		List users
@@ -56,8 +62,23 @@ func GetUsers(c *gin.Context) {
 //	@Tags			Users
 //	@Param			Authorization	header	string	true	"Insert your personal access token"	default(Bearer <personal access token>)
 //	@Param			login			path	string	true	"the user's login name"
+//	@Param			forge_id		query	string	true	"specify forge (else default will be used)"
+//	@Param			forge_remote_id	query	string	false	"specify user id at forge (else fallback to login)"
 func GetUser(c *gin.Context) {
-	user, err := store.FromContext(c).GetUserLogin(c.Param("login"))
+	forgeID, err := strconv.ParseInt(c.DefaultQuery("forge_id", fmt.Sprint(defaultForgeID)), 10, 64)
+	if err != nil {
+		c.AbortWithStatus(http.StatusBadRequest)
+		return
+	}
+	forgeRemoteID := model.ForgeRemoteID(c.Query("forge_remote_id"))
+
+	var user *model.User
+
+	if forgeRemoteID.IsValid() {
+		user, err = store.FromContext(c).GetUserByRemoteID(forgeID, forgeRemoteID)
+	} else {
+		user, err = store.FromContext(c).GetUserByLogin(forgeID, c.Param("login"))
+	}
 	if err != nil {
 		handleDBError(c, err)
 		return
@@ -87,14 +108,26 @@ func PatchUser(c *gin.Context) {
 		return
 	}
 
-	user, err := _store.GetUserLogin(c.Param("login"))
-	if err != nil {
+	if in.ForgeID < defaultForgeID {
+		in.ForgeID = defaultForgeID
+	}
+
+	user, err := store.FromContext(c).GetUserByRemoteID(in.ForgeID, in.ForgeRemoteID)
+	if err != nil && !errors.Is(err, types.RecordNotExist) {
 		handleDBError(c, err)
 		return
 	}
 
-	// TODO: allow to change login (currently used as primary key)
+	if user == nil {
+		user, err = _store.GetUserByLogin(in.ForgeID, c.Param("login"))
+		if err != nil {
+			handleDBError(c, err)
+			return
+		}
+	}
+
 	// TODO: disallow to change login, email, avatar if the user is using oauth
+	user.Login = in.Login
 	user.Email = in.Email
 	user.Avatar = in.Avatar
 	user.Admin = in.Admin
@@ -132,7 +165,7 @@ func PostUser(c *gin.Context) {
 		Hash: base32.StdEncoding.EncodeToString(
 			random.GetRandomBytes(32),
 		),
-		ForgeID:       1,                        // TODO: replace with forge id when multiple forges are supported
+		ForgeID:       in.ForgeID,
 		ForgeRemoteID: model.ForgeRemoteID("0"), // TODO: search for the user in the forge and get the remote id
 	}
 	if err = user.Validate(); err != nil {
@@ -156,10 +189,25 @@ func PostUser(c *gin.Context) {
 //	@Tags			Users
 //	@Param			Authorization	header	string	true	"Insert your personal access token"	default(Bearer <personal access token>)
 //	@Param			login			path	string	true	"the user's login name"
+//	@Param			forge_id		query	string	true	"specify forge (else default will be used)"
+//	@Param			forge_remote_id	query	string	false	"specify user id at forge (else fallback to login)"
 func DeleteUser(c *gin.Context) {
 	_store := store.FromContext(c)
 
-	user, err := _store.GetUserLogin(c.Param("login"))
+	forgeID, err := strconv.ParseInt(c.DefaultQuery("forge_id", fmt.Sprint(defaultForgeID)), 10, 64)
+	if err != nil {
+		c.AbortWithStatus(http.StatusBadRequest)
+		return
+	}
+	forgeRemoteID := model.ForgeRemoteID(c.Query("forge_remote_id"))
+
+	var user *model.User
+
+	if forgeRemoteID.IsValid() {
+		user, err = store.FromContext(c).GetUserByRemoteID(forgeID, forgeRemoteID)
+	} else {
+		user, err = store.FromContext(c).GetUserByLogin(forgeID, c.Param("login"))
+	}
 	if err != nil {
 		handleDBError(c, err)
 		return

server/model/repo.go

@@ -45,9 +45,9 @@ func (mode ApprovalMode) Valid() bool {
 type Repo struct {
 	ID      int64 `json:"id,omitempty"                    xorm:"pk autoincr 'id'"`
 	UserID  int64 `json:"-"                               xorm:"INDEX 'user_id'"`
-	ForgeID int64 `json:"forge_id,omitempty"              xorm:"forge_id"`
+	ForgeID int64 `json:"forge_id,omitempty"              xorm:"UNIQUE(forge) forge_id"`
 	// ForgeRemoteID is the unique identifier for the repository on the forge.
-	ForgeRemoteID                ForgeRemoteID        `json:"forge_remote_id"                 xorm:"forge_remote_id"`
+	ForgeRemoteID                ForgeRemoteID        `json:"forge_remote_id"                 xorm:"UNIQUE(forge) forge_remote_id"`
 	OrgID                        int64                `json:"org_id"                          xorm:"INDEX 'org_id'"`
 	Owner                        string               `json:"owner"                           xorm:"UNIQUE(name) 'owner'"`
 	Name                         string               `json:"name"                            xorm:"UNIQUE(name) 'name'"`

server/model/user.go

@@ -34,9 +34,9 @@ type User struct {
 	// required: true
 	ID int64 `json:"id" xorm:"pk autoincr 'id'"`
 
-	ForgeID int64 `json:"forge_id,omitempty" xorm:"forge_id"`
+	ForgeID int64 `json:"forge_id,omitempty" xorm:"forge_id UNIQUE(forge)"`
 
-	ForgeRemoteID ForgeRemoteID `json:"-" xorm:"forge_remote_id"`
+	ForgeRemoteID ForgeRemoteID `json:"forge_remote_id" xorm:"forge_remote_id UNIQUE(forge)"`
 
 	// Login is the username for this user.
 	//

server/store/datastore/org_test.go

@@ -61,9 +61,9 @@ func TestOrgCRUD(t *testing.T) {
 	someUser := &model.Org{Name: "some_other_u", IsUser: true}
 	assert.NoError(t, store.OrgCreate(someUser))
 	assert.NoError(t, store.OrgCreate(&model.Org{Name: "some_other_org"}))
-	assert.NoError(t, store.CreateRepo(&model.Repo{UserID: 1, Owner: "some_other_u", Name: "abc", FullName: "some_other_u/abc", OrgID: someUser.ID}))
-	assert.NoError(t, store.CreateRepo(&model.Repo{UserID: 1, Owner: "some_other_u", Name: "xyz", FullName: "some_other_u/xyz", OrgID: someUser.ID}))
-	assert.NoError(t, store.CreateRepo(&model.Repo{UserID: 1, Owner: "renamedorg", Name: "567", FullName: "renamedorg/567", OrgID: orgOne.ID}))
+	assert.NoError(t, store.CreateRepo(&model.Repo{ForgeRemoteID: "a", UserID: 1, Owner: "some_other_u", Name: "abc", FullName: "some_other_u/abc", OrgID: someUser.ID}))
+	assert.NoError(t, store.CreateRepo(&model.Repo{ForgeRemoteID: "b", UserID: 1, Owner: "some_other_u", Name: "xyz", FullName: "some_other_u/xyz", OrgID: someUser.ID}))
+	assert.NoError(t, store.CreateRepo(&model.Repo{ForgeRemoteID: "c", UserID: 1, Owner: "renamedorg", Name: "567", FullName: "renamedorg/567", OrgID: orgOne.ID}))
 	assert.Error(t, store.OrgCreate(&model.Org{Name: ""}), "expect to fail if name is empty")
 
 	// get all repos for a specific org

server/store/datastore/pipeline_test.go

@@ -206,8 +206,8 @@ func TestPipelineIncrement(t *testing.T) {
 	store, closer := newTestStore(t, new(model.Pipeline), new(model.Repo))
 	defer closer()
 
-	assert.NoError(t, store.CreateRepo(&model.Repo{ID: 1, Owner: "1", Name: "1", FullName: "1/1"}))
-	assert.NoError(t, store.CreateRepo(&model.Repo{ID: 2, Owner: "2", Name: "2", FullName: "2/2"}))
+	assert.NoError(t, store.CreateRepo(&model.Repo{ID: 1, Owner: "1", Name: "1", FullName: "1/1", ForgeRemoteID: "1"}))
+	assert.NoError(t, store.CreateRepo(&model.Repo{ID: 2, Owner: "2", Name: "2", FullName: "2/2", ForgeRemoteID: "2"}))
 
 	pipelineA := &model.Pipeline{RepoID: 1}
 	if !assert.NoError(t, store.CreatePipeline(pipelineA)) {

server/store/datastore/repo_test.go

@@ -254,22 +254,25 @@ func TestRepoCount(t *testing.T) {
 	defer closer()
 
 	repo1 := &model.Repo{
-		Owner:    "bradrydzewski",
-		Name:     "test",
-		FullName: "bradrydzewski/test",
-		IsActive: true,
+		ForgeRemoteID: "A",
+		Owner:         "bradrydzewski",
+		Name:          "test",
+		FullName:      "bradrydzewski/test",
+		IsActive:      true,
 	}
 	repo2 := &model.Repo{
-		Owner:    "test",
-		Name:     "test",
-		FullName: "test/test",
-		IsActive: true,
+		ForgeRemoteID: "B",
+		Owner:         "test",
+		Name:          "test",
+		FullName:      "test/test",
+		IsActive:      true,
 	}
 	repo3 := &model.Repo{
-		Owner:    "test",
-		Name:     "test-ui",
-		FullName: "test/test-ui",
-		IsActive: false,
+		ForgeRemoteID: "C",
+		Owner:         "test",
+		Name:          "test-ui",
+		FullName:      "test/test-ui",
+		IsActive:      false,
 	}
 	assert.NoError(t, store.CreateRepo(repo1))
 	assert.NoError(t, store.CreateRepo(repo2))
@@ -297,10 +300,12 @@ func TestRepoCrud(t *testing.T) {
 	defer closer()
 
 	repo := model.Repo{
-		UserID:   1,
-		FullName: "bradrydzewski/test",
-		Owner:    "bradrydzewski",
-		Name:     "test",
+		ForgeID:       1,
+		ForgeRemoteID: "bradrydzewskitest",
+		UserID:        1,
+		FullName:      "bradrydzewski/test",
+		Owner:         "bradrydzewski",
+		Name:          "test",
 	}
 	assert.NoError(t, store.CreateRepo(&repo))
 	pipeline := model.Pipeline{
@@ -313,10 +318,12 @@ func TestRepoCrud(t *testing.T) {
 
 	// create unrelated
 	repoUnrelated := model.Repo{
-		UserID:   2,
-		FullName: "x/x",
-		Owner:    "x",
-		Name:     "x",
+		ForgeRemoteID: "xx",
+		ForgeID:       1,
+		UserID:        2,
+		FullName:      "x/x",
+		Owner:         "x",
+		Name:          "x",
 	}
 	assert.NoError(t, store.CreateRepo(&repoUnrelated))
 	pipelineUnrelated := model.Pipeline{
@@ -350,6 +357,7 @@ func TestRepoRedirection(t *testing.T) {
 
 	repo := model.Repo{
 		UserID:        1,
+		ForgeID:       1,
 		ForgeRemoteID: "1",
 		FullName:      "bradrydzewski/test",
 		Owner:         "bradrydzewski",
@@ -378,10 +386,11 @@ func TestRepoRedirection(t *testing.T) {
 
 	// test getting repo without forge ID (use name fallback)
 	repo = model.Repo{
-		UserID:   1,
-		FullName: "bradrydzewski/test-no-forge-id",
-		Owner:    "bradrydzewski",
-		Name:     "test-no-forge-id",
+		UserID:        1,
+		ForgeRemoteID: "bradrydzewski/test-no-forge-id",
+		FullName:      "bradrydzewski/test-no-forge-id",
+		Owner:         "bradrydzewski",
+		Name:          "test-no-forge-id",
 	}
 	assert.NoError(t, store.CreateRepo(&repo))
 

server/store/datastore/user.go

@@ -18,8 +18,6 @@ import (
 	"errors"
 	"fmt"
 
-	"xorm.io/xorm"
-
 	"go.woodpecker-ci.org/woodpecker/v3/server/model"
 	"go.woodpecker-ci.org/woodpecker/v3/server/store/types"
 )
@@ -29,23 +27,16 @@ func (s storage) GetUser(id int64) (*model.User, error) {
 	return user, wrapGet(s.engine.ID(id).Get(user))
 }
 
-func (s storage) GetUserRemoteID(remoteID model.ForgeRemoteID, login string) (*model.User, error) {
+func (s storage) GetUserByRemoteID(forgeID int64, userRemoteID model.ForgeRemoteID) (*model.User, error) {
 	sess := s.engine.NewSession()
 	user := new(model.User)
-	err := wrapGet(sess.Where("forge_remote_id = ?", remoteID).Get(user))
-	if err != nil {
-		return s.getUserLogin(sess, login)
-	}
-	return user, err
+	return user, wrapGet(sess.Where("forge_id = ? AND forge_remote_id = ?", forgeID, userRemoteID).Get(user))
 }
 
-func (s storage) GetUserLogin(login string) (*model.User, error) {
-	return s.getUserLogin(s.engine.NewSession(), login)
-}
-
-func (s storage) getUserLogin(sess *xorm.Session, login string) (*model.User, error) {
+func (s storage) GetUserByLogin(forgeID int64, login string) (*model.User, error) {
+	sess := s.engine.NewSession()
 	user := new(model.User)
-	return user, wrapGet(sess.Where("login=?", login).Get(user))
+	return user, wrapGet(sess.Where("forge_id = ? AND login=?", forgeID, login).Get(user))
 }
 
 func (s storage) GetUserList(p *model.ListOptions) ([]*model.User, error) {

server/store/datastore/user_test.go

@@ -32,11 +32,12 @@ func TestUsers(t *testing.T) {
 	assert.Zero(t, count)
 
 	user := model.User{
-		Login:        "joe",
-		AccessToken:  "f0b461ca586c27872b43a0685cbc2847",
-		RefreshToken: "976f22a5eef7caacb7e678d6c52f49b1",
-		Email:        "foo@bar.com",
-		Avatar:       "b9015b0857e16ac4d94a0ffd9a0b79c8",
+		Login:         "joe",
+		ForgeRemoteID: "joe",
+		AccessToken:   "f0b461ca586c27872b43a0685cbc2847",
+		RefreshToken:  "976f22a5eef7caacb7e678d6c52f49b1",
+		Email:         "foo@bar.com",
+		Avatar:        "b9015b0857e16ac4d94a0ffd9a0b79c8",
 	}
 	err = store.CreateUser(&user)
 	assert.NoError(t, err)
@@ -54,25 +55,27 @@ func TestUsers(t *testing.T) {
 	assert.Equal(t, user.Email, getUser.Email)
 	assert.Equal(t, user.Avatar, getUser.Avatar)
 
-	getUser, err = store.GetUserLogin(user.Login)
+	getUser, err = store.GetUserByLogin(user.ForgeID, user.Login)
 	assert.NoError(t, err)
 	assert.Equal(t, user.ID, getUser.ID)
 	assert.Equal(t, user.Login, getUser.Login)
 
 	// check unique login
 	user2 := model.User{
-		Login:       "Joe",
-		Email:       "foo2@bar.com",
-		AccessToken: "ab20g0ddaf012c744e136da16aa21ad9",
+		Login:         "Joe",
+		ForgeRemoteID: "joe",
+		Email:         "foo2@bar.com",
+		AccessToken:   "ab20g0ddaf012c744e136da16aa21ad9",
 	}
 	err2 = store.CreateUser(&user2)
 	assert.Error(t, err2)
 
 	user2 = model.User{
-		Login:       "jane",
-		Email:       "foo@bar.com",
-		AccessToken: "ab20g0ddaf012c744e136da16aa21ad9",
-		Hash:        "A",
+		Login:         "jane",
+		ForgeRemoteID: "jane",
+		Email:         "foo@bar.com",
+		AccessToken:   "ab20g0ddaf012c744e136da16aa21ad9",
+		Hash:          "A",
 	}
 	assert.NoError(t, store.CreateUser(&user2))
 	users, err := store.GetUserList(&model.ListOptions{Page: 1, PerPage: 50})
@@ -112,9 +115,10 @@ func TestCreateUserWithExistingOrg(t *testing.T) {
 
 	// Create a new user with the same name as the existing organization
 	newUser := &model.User{
-		Login:   "existingOrg",
-		Hash:    "A",
-		ForgeID: 1,
+		Login:         "existingOrg",
+		ForgeRemoteID: "A",
+		Hash:          "A",
+		ForgeID:       1,
 	}
 	err = store.CreateUser(newUser)
 	assert.NoError(t, err)
@@ -124,9 +128,10 @@ func TestCreateUserWithExistingOrg(t *testing.T) {
 	assert.Equal(t, "existingOrg", updatedOrg.Name)
 
 	newUser2 := &model.User{
-		Login:   "new-user",
-		ForgeID: 1,
-		Hash:    "B",
+		Login:         "new-user",
+		ForgeRemoteID: "B",
+		ForgeID:       1,
+		Hash:          "B",
 	}
 	err = store.CreateUser(newUser2)
 	assert.NoError(t, err)

server/store/mocks/mock_Store.go

@@ -2712,6 +2712,142 @@ func (_c *MockStore_GetUser_Call) RunAndReturn(run func(n int64) (*model.User, e
 	return _c
 }
 
+// GetUserByLogin provides a mock function for the type MockStore
+func (_mock *MockStore) GetUserByLogin(n int64, s string) (*model.User, error) {
+	ret := _mock.Called(n, s)
+
+	if len(ret) == 0 {
+		panic("no return value specified for GetUserByLogin")
+	}
+
+	var r0 *model.User
+	var r1 error
+	if returnFunc, ok := ret.Get(0).(func(int64, string) (*model.User, error)); ok {
+		return returnFunc(n, s)
+	}
+	if returnFunc, ok := ret.Get(0).(func(int64, string) *model.User); ok {
+		r0 = returnFunc(n, s)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(*model.User)
+		}
+	}
+	if returnFunc, ok := ret.Get(1).(func(int64, string) error); ok {
+		r1 = returnFunc(n, s)
+	} else {
+		r1 = ret.Error(1)
+	}
+	return r0, r1
+}
+
+// MockStore_GetUserByLogin_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'GetUserByLogin'
+type MockStore_GetUserByLogin_Call struct {
+	*mock.Call
+}
+
+// GetUserByLogin is a helper method to define mock.On call
+//   - n int64
+//   - s string
+func (_e *MockStore_Expecter) GetUserByLogin(n interface{}, s interface{}) *MockStore_GetUserByLogin_Call {
+	return &MockStore_GetUserByLogin_Call{Call: _e.mock.On("GetUserByLogin", n, s)}
+}
+
+func (_c *MockStore_GetUserByLogin_Call) Run(run func(n int64, s string)) *MockStore_GetUserByLogin_Call {
+	_c.Call.Run(func(args mock.Arguments) {
+		var arg0 int64
+		if args[0] != nil {
+			arg0 = args[0].(int64)
+		}
+		var arg1 string
+		if args[1] != nil {
+			arg1 = args[1].(string)
+		}
+		run(
+			arg0,
+			arg1,
+		)
+	})
+	return _c
+}
+
+func (_c *MockStore_GetUserByLogin_Call) Return(user *model.User, err error) *MockStore_GetUserByLogin_Call {
+	_c.Call.Return(user, err)
+	return _c
+}
+
+func (_c *MockStore_GetUserByLogin_Call) RunAndReturn(run func(n int64, s string) (*model.User, error)) *MockStore_GetUserByLogin_Call {
+	_c.Call.Return(run)
+	return _c
+}
+
+// GetUserByRemoteID provides a mock function for the type MockStore
+func (_mock *MockStore) GetUserByRemoteID(n int64, forgeRemoteID model.ForgeRemoteID) (*model.User, error) {
+	ret := _mock.Called(n, forgeRemoteID)
+
+	if len(ret) == 0 {
+		panic("no return value specified for GetUserByRemoteID")
+	}
+
+	var r0 *model.User
+	var r1 error
+	if returnFunc, ok := ret.Get(0).(func(int64, model.ForgeRemoteID) (*model.User, error)); ok {
+		return returnFunc(n, forgeRemoteID)
+	}
+	if returnFunc, ok := ret.Get(0).(func(int64, model.ForgeRemoteID) *model.User); ok {
+		r0 = returnFunc(n, forgeRemoteID)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(*model.User)
+		}
+	}
+	if returnFunc, ok := ret.Get(1).(func(int64, model.ForgeRemoteID) error); ok {
+		r1 = returnFunc(n, forgeRemoteID)
+	} else {
+		r1 = ret.Error(1)
+	}
+	return r0, r1
+}
+
+// MockStore_GetUserByRemoteID_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'GetUserByRemoteID'
+type MockStore_GetUserByRemoteID_Call struct {
+	*mock.Call
+}
+
+// GetUserByRemoteID is a helper method to define mock.On call
+//   - n int64
+//   - forgeRemoteID model.ForgeRemoteID
+func (_e *MockStore_Expecter) GetUserByRemoteID(n interface{}, forgeRemoteID interface{}) *MockStore_GetUserByRemoteID_Call {
+	return &MockStore_GetUserByRemoteID_Call{Call: _e.mock.On("GetUserByRemoteID", n, forgeRemoteID)}
+}
+
+func (_c *MockStore_GetUserByRemoteID_Call) Run(run func(n int64, forgeRemoteID model.ForgeRemoteID)) *MockStore_GetUserByRemoteID_Call {
+	_c.Call.Run(func(args mock.Arguments) {
+		var arg0 int64
+		if args[0] != nil {
+			arg0 = args[0].(int64)
+		}
+		var arg1 model.ForgeRemoteID
+		if args[1] != nil {
+			arg1 = args[1].(model.ForgeRemoteID)
+		}
+		run(
+			arg0,
+			arg1,
+		)
+	})
+	return _c
+}
+
+func (_c *MockStore_GetUserByRemoteID_Call) Return(user *model.User, err error) *MockStore_GetUserByRemoteID_Call {
+	_c.Call.Return(user, err)
+	return _c
+}
+
+func (_c *MockStore_GetUserByRemoteID_Call) RunAndReturn(run func(n int64, forgeRemoteID model.ForgeRemoteID) (*model.User, error)) *MockStore_GetUserByRemoteID_Call {
+	_c.Call.Return(run)
+	return _c
+}
+
 // GetUserCount provides a mock function for the type MockStore
 func (_mock *MockStore) GetUserCount() (int64, error) {
 	ret := _mock.Called()
@@ -2827,136 +2963,6 @@ func (_c *MockStore_GetUserList_Call) RunAndReturn(run func(p *model.ListOptions
 	return _c
 }
 
-// GetUserLogin provides a mock function for the type MockStore
-func (_mock *MockStore) GetUserLogin(s string) (*model.User, error) {
-	ret := _mock.Called(s)
-
-	if len(ret) == 0 {
-		panic("no return value specified for GetUserLogin")
-	}
-
-	var r0 *model.User
-	var r1 error
-	if returnFunc, ok := ret.Get(0).(func(string) (*model.User, error)); ok {
-		return returnFunc(s)
-	}
-	if returnFunc, ok := ret.Get(0).(func(string) *model.User); ok {
-		r0 = returnFunc(s)
-	} else {
-		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(*model.User)
-		}
-	}
-	if returnFunc, ok := ret.Get(1).(func(string) error); ok {
-		r1 = returnFunc(s)
-	} else {
-		r1 = ret.Error(1)
-	}
-	return r0, r1
-}
-
-// MockStore_GetUserLogin_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'GetUserLogin'
-type MockStore_GetUserLogin_Call struct {
-	*mock.Call
-}
-
-// GetUserLogin is a helper method to define mock.On call
-//   - s string
-func (_e *MockStore_Expecter) GetUserLogin(s interface{}) *MockStore_GetUserLogin_Call {
-	return &MockStore_GetUserLogin_Call{Call: _e.mock.On("GetUserLogin", s)}
-}
-
-func (_c *MockStore_GetUserLogin_Call) Run(run func(s string)) *MockStore_GetUserLogin_Call {
-	_c.Call.Run(func(args mock.Arguments) {
-		var arg0 string
-		if args[0] != nil {
-			arg0 = args[0].(string)
-		}
-		run(
-			arg0,
-		)
-	})
-	return _c
-}
-
-func (_c *MockStore_GetUserLogin_Call) Return(user *model.User, err error) *MockStore_GetUserLogin_Call {
-	_c.Call.Return(user, err)
-	return _c
-}
-
-func (_c *MockStore_GetUserLogin_Call) RunAndReturn(run func(s string) (*model.User, error)) *MockStore_GetUserLogin_Call {
-	_c.Call.Return(run)
-	return _c
-}
-
-// GetUserRemoteID provides a mock function for the type MockStore
-func (_mock *MockStore) GetUserRemoteID(forgeRemoteID model.ForgeRemoteID, s string) (*model.User, error) {
-	ret := _mock.Called(forgeRemoteID, s)
-
-	if len(ret) == 0 {
-		panic("no return value specified for GetUserRemoteID")
-	}
-
-	var r0 *model.User
-	var r1 error
-	if returnFunc, ok := ret.Get(0).(func(model.ForgeRemoteID, string) (*model.User, error)); ok {
-		return returnFunc(forgeRemoteID, s)
-	}
-	if returnFunc, ok := ret.Get(0).(func(model.ForgeRemoteID, string) *model.User); ok {
-		r0 = returnFunc(forgeRemoteID, s)
-	} else {
-		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(*model.User)
-		}
-	}
-	if returnFunc, ok := ret.Get(1).(func(model.ForgeRemoteID, string) error); ok {
-		r1 = returnFunc(forgeRemoteID, s)
-	} else {
-		r1 = ret.Error(1)
-	}
-	return r0, r1
-}
-
-// MockStore_GetUserRemoteID_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'GetUserRemoteID'
-type MockStore_GetUserRemoteID_Call struct {
-	*mock.Call
-}
-
-// GetUserRemoteID is a helper method to define mock.On call
-//   - forgeRemoteID model.ForgeRemoteID
-//   - s string
-func (_e *MockStore_Expecter) GetUserRemoteID(forgeRemoteID interface{}, s interface{}) *MockStore_GetUserRemoteID_Call {
-	return &MockStore_GetUserRemoteID_Call{Call: _e.mock.On("GetUserRemoteID", forgeRemoteID, s)}
-}
-
-func (_c *MockStore_GetUserRemoteID_Call) Run(run func(forgeRemoteID model.ForgeRemoteID, s string)) *MockStore_GetUserRemoteID_Call {
-	_c.Call.Run(func(args mock.Arguments) {
-		var arg0 model.ForgeRemoteID
-		if args[0] != nil {
-			arg0 = args[0].(model.ForgeRemoteID)
-		}
-		var arg1 string
-		if args[1] != nil {
-			arg1 = args[1].(string)
-		}
-		run(
-			arg0,
-			arg1,
-		)
-	})
-	return _c
-}
-
-func (_c *MockStore_GetUserRemoteID_Call) Return(user *model.User, err error) *MockStore_GetUserRemoteID_Call {
-	_c.Call.Return(user, err)
-	return _c
-}
-
-func (_c *MockStore_GetUserRemoteID_Call) RunAndReturn(run func(forgeRemoteID model.ForgeRemoteID, s string) (*model.User, error)) *MockStore_GetUserRemoteID_Call {
-	_c.Call.Return(run)
-	return _c
-}
-
 // GlobalRegistryFind provides a mock function for the type MockStore
 func (_mock *MockStore) GlobalRegistryFind(s string) (*model.Registry, error) {
 	ret := _mock.Called(s)

server/store/store.go

@@ -26,10 +26,10 @@ type Store interface {
 	// Users
 	// GetUser gets a user by unique ID.
 	GetUser(int64) (*model.User, error)
-	// GetUserRemoteID gets a user by remote ID with fallback to login name.
-	GetUserRemoteID(model.ForgeRemoteID, string) (*model.User, error)
-	// GetUserLogin gets a user by unique Login name.
-	GetUserLogin(string) (*model.User, error)
+	// GetUserByRemoteID gets a user by remote ID.
+	GetUserByRemoteID(int64, model.ForgeRemoteID) (*model.User, error)
+	// GetUserByLogin gets a user by its login name.
+	GetUserByLogin(int64, string) (*model.User, error)
 	// GetUserList gets a list of all users in the system.
 	GetUserList(p *model.ListOptions) ([]*model.User, error)
 	// GetUserCount gets a count of all users in the system.