diff --git a/models/forgejo_migrations/v16c_add_team_invite_invited_id.go b/models/forgejo_migrations/v16c_add_team_invite_invited_id.go new file mode 100644 index 0000000000..5a1663eedf --- /dev/null +++ b/models/forgejo_migrations/v16c_add_team_invite_invited_id.go @@ -0,0 +1,26 @@ +// Copyright 2026 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package forgejo_migrations + +import ( + "forgejo.org/modules/optional" + + "code.forgejo.org/xorm/xorm" +) + +func init() { + registerMigration(&Migration{ + Description: "add invited_id to team_invite", + Upgrade: addTeamInviteInvitedID, + }) +} + +func addTeamInviteInvitedID(x *xorm.Engine) error { + // the invited_id is set None if we are inviting someone who does not have an account yet + type TeamInvite struct { + InvitedID optional.Option[int64] `xorm:"index REFERENCES(user, id)"` + } + _, err := x.SyncWithOptions(xorm.SyncOptions{IgnoreDropIndices: true}, new(TeamInvite)) + return err +} diff --git a/models/organization/team_invite.go b/models/organization/team_invite.go index 3fdabcb433..9b9a059f6c 100644 --- a/models/organization/team_invite.go +++ b/models/organization/team_invite.go @@ -9,6 +9,7 @@ import ( "forgejo.org/models/db" user_model "forgejo.org/models/user" + "forgejo.org/modules/optional" "forgejo.org/modules/timeutil" "forgejo.org/modules/util" @@ -16,8 +17,9 @@ import ( ) type ErrTeamInviteAlreadyExist struct { - TeamID int64 - Email string + TeamID int64 + Email string + InvitedUserID int64 } func IsErrTeamInviteAlreadyExist(err error) bool { @@ -26,7 +28,7 @@ func IsErrTeamInviteAlreadyExist(err error) bool { } func (err ErrTeamInviteAlreadyExist) Error() string { - return fmt.Sprintf("team invite already exists [team_id: %d, email: %s]", err.TeamID, err.Email) + return fmt.Sprintf("team invite already exists [team_id: %d, email: %s, invited_user_id: %d]", err.TeamID, err.Email, err.InvitedUserID) } func (err ErrTeamInviteAlreadyExist) Unwrap() error { @@ -50,38 +52,42 @@ func (err ErrTeamInviteNotFound) Unwrap() error { return util.ErrNotExist } -// ErrUserEmailAlreadyAdded represents a "user by email already added to team" error. -type ErrUserEmailAlreadyAdded struct { - Email string +// ErrInvitedUserAlreadyAdded indicates that a user is already part of a team and can not be invited again. +type ErrInvitedUserAlreadyAdded struct { + Email string + InvitedUserID optional.Option[int64] } // IsErrUserEmailAlreadyAdded checks if an error is a ErrUserEmailAlreadyAdded. func IsErrUserEmailAlreadyAdded(err error) bool { - _, ok := err.(ErrUserEmailAlreadyAdded) + _, ok := err.(ErrInvitedUserAlreadyAdded) return ok } -func (err ErrUserEmailAlreadyAdded) Error() string { - return fmt.Sprintf("user with email already added [email: %s]", err.Email) +func (err ErrInvitedUserAlreadyAdded) Error() string { + return fmt.Sprintf("user with email already added [email: %s, invited_user_id: %d]", err.Email, err.InvitedUserID) } -func (err ErrUserEmailAlreadyAdded) Unwrap() error { +func (err ErrInvitedUserAlreadyAdded) Unwrap() error { return util.ErrAlreadyExist } // TeamInvite represents an invite to a team type TeamInvite struct { - ID int64 `xorm:"pk autoincr"` - Token string `xorm:"UNIQUE(token) INDEX NOT NULL DEFAULT ''"` - InviterID int64 `xorm:"NOT NULL DEFAULT 0"` - OrgID int64 `xorm:"INDEX NOT NULL DEFAULT 0"` - TeamID int64 `xorm:"UNIQUE(team_mail) INDEX NOT NULL DEFAULT 0"` - Email string `xorm:"UNIQUE(team_mail) NOT NULL DEFAULT ''"` - CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` - UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` + ID int64 `xorm:"pk autoincr"` + Token string `xorm:"UNIQUE(token) INDEX NOT NULL DEFAULT ''"` + InviterID int64 `xorm:"NOT NULL DEFAULT 0"` + OrgID int64 `xorm:"INDEX NOT NULL DEFAULT 0"` + TeamID int64 `xorm:"UNIQUE(team_mail) INDEX NOT NULL DEFAULT 0"` + Email string `xorm:"UNIQUE(team_mail) NOT NULL DEFAULT ''"` + InvitedID optional.Option[int64] `xorm:"index REFERENCES(user, id)"` + InvitedUser *user_model.User `xorm:"-"` + CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` + UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` } -func CreateTeamInvite(ctx context.Context, doer *user_model.User, team *Team, email string) (*TeamInvite, error) { +// CreateTeamInviteByEmail creates a TeamInvite for someone who does not have an account yet. +func CreateTeamInviteByEmail(ctx context.Context, doer *user_model.User, team *Team, email string) (*TeamInvite, error) { has, err := db.GetEngine(ctx).Exist(&TeamInvite{ TeamID: team.ID, Email: email, @@ -111,7 +117,7 @@ func CreateTeamInvite(ctx context.Context, doer *user_model.User, team *Team, em } if exist { - return nil, ErrUserEmailAlreadyAdded{ + return nil, ErrInvitedUserAlreadyAdded{ Email: email, } } @@ -129,6 +135,56 @@ func CreateTeamInvite(ctx context.Context, doer *user_model.User, team *Team, em return invite, db.Insert(ctx, invite) } +// CreateTeamInviteForUser creates a TeamInvite for someone who already has an account on the instance. +func CreateTeamInviteForUser(ctx context.Context, doer, invited *user_model.User, team *Team) (*TeamInvite, error) { + has, err := db.GetEngine(ctx).Exist(&TeamInvite{ + TeamID: team.ID, + InvitedID: optional.Some(invited.ID), + }) + if err != nil { + return nil, err + } + if has { + return nil, ErrTeamInviteAlreadyExist{ + TeamID: team.ID, + Email: invited.Email, + } + } + + // check if the user is already a team member + exist, err := db.GetEngine(ctx). + Where(builder.Eq{ + "org_id": team.OrgID, + "team_id": team.ID, + "uid": invited.ID, + }). + Table("team_user"). + Exist() + if err != nil { + return nil, err + } + + if exist { + return nil, ErrInvitedUserAlreadyAdded{ + InvitedUserID: optional.Some(invited.ID), + } + } + + token := util.CryptoRandomString(util.RandomStringMedium) + + invite := &TeamInvite{ + Token: token, + InviterID: doer.ID, + OrgID: team.OrgID, + TeamID: team.ID, + Email: invited.Email, + InvitedID: optional.Some(invited.ID), + InvitedUser: invited, + } + + return invite, db.Insert(ctx, invite) +} + func RemoveInviteByID(ctx context.Context, inviteID, teamID int64) error { _, err := db.DeleteByBean(ctx, &TeamInvite{ ID: inviteID, @@ -156,3 +212,17 @@ func GetInviteByToken(ctx context.Context, token string) (*TeamInvite, error) { } return invite, nil } + +func (i *TeamInvite) LoadInvitedUser(ctx context.Context) error { + if i.InvitedUser == nil { + hasInvitedUser, userID := i.InvitedID.Get() + if hasInvitedUser { + user, err := user_model.GetUserByID(ctx, userID) + if err != nil { + return err + } + i.InvitedUser = user + } + } + return nil +} diff --git a/models/organization/team_invite_test.go b/models/organization/team_invite_test.go index 8d55864237..c725ba7646 100644 --- a/models/organization/team_invite_test.go +++ b/models/organization/team_invite_test.go @@ -24,19 +24,70 @@ func TestTeamInvite(t *testing.T) { user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) // user 2 already added to team 2, should result in error - _, err := organization.CreateTeamInvite(db.DefaultContext, user2, team, user2.Email) + _, err := organization.CreateTeamInviteByEmail(db.DefaultContext, user2, team, user2.Email) require.Error(t, err) }) - t.Run("CreateAndRemove", func(t *testing.T) { + t.Run("UserExistsInTeam", func(t *testing.T) { + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + // user 4 already added to team 2, should result in error + _, err := organization.CreateTeamInviteForUser(db.DefaultContext, user2, user4, team) + require.Error(t, err) + }) + + t.Run("CreateAndRemoveByUser", func(t *testing.T) { + user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + user5 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5}) + + invite, err := organization.CreateTeamInviteForUser(db.DefaultContext, user1, user5, team) + assert.NotNil(t, invite) + require.NoError(t, err) + + // Shouldn't allow duplicate invite by email + _, err = organization.CreateTeamInviteByEmail(db.DefaultContext, user1, team, user5.Email) + require.Error(t, err) + // Shouldn't allow duplicate invite by user + _, err = organization.CreateTeamInviteForUser(db.DefaultContext, user1, user5, team) + require.Error(t, err) + + // should remove invite + require.NoError(t, organization.RemoveInviteByID(db.DefaultContext, invite.ID, invite.TeamID)) + + // invite should not exist + _, err = organization.GetInviteByToken(db.DefaultContext, invite.Token) + require.Error(t, err) + }) + + t.Run("CreateByEmailAndRemove", func(t *testing.T) { user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) - invite, err := organization.CreateTeamInvite(db.DefaultContext, user1, team, "org3@example.com") + invite, err := organization.CreateTeamInviteByEmail(db.DefaultContext, user1, team, "org3@example.com") assert.NotNil(t, invite) require.NoError(t, err) // Shouldn't allow duplicate invite - _, err = organization.CreateTeamInvite(db.DefaultContext, user1, team, "org3@example.com") + _, err = organization.CreateTeamInviteByEmail(db.DefaultContext, user1, team, "org3@example.com") + require.Error(t, err) + + // should remove invite + require.NoError(t, organization.RemoveInviteByID(db.DefaultContext, invite.ID, invite.TeamID)) + + // invite should not exist + _, err = organization.GetInviteByToken(db.DefaultContext, invite.Token) + require.Error(t, err) + }) + + t.Run("CreateByUserAndRemove", func(t *testing.T) { + user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + + invite, err := organization.CreateTeamInviteByEmail(db.DefaultContext, user1, team, "org3@example.com") + assert.NotNil(t, invite) + require.NoError(t, err) + + // Shouldn't allow duplicate invite + _, err = organization.CreateTeamInviteByEmail(db.DefaultContext, user1, team, "org3@example.com") require.Error(t, err) // should remove invite diff --git a/modules/setting/service.go b/modules/setting/service.go index 7e43ebc5d4..eced82f186 100644 --- a/modules/setting/service.go +++ b/modules/setting/service.go @@ -82,6 +82,7 @@ var Service = struct { NoReplyAddress string UserLocationMapURL string EnableUserHeatmap bool + AddMembersByInvitations bool AutoWatchNewRepos bool AutoWatchOnChanges bool DefaultOrgMemberVisible bool @@ -245,6 +246,7 @@ func loadServiceFrom(rootCfg ConfigProvider) { Service.NoReplyAddress = sec.Key("NO_REPLY_ADDRESS").MustString("noreply." + Domain) Service.UserLocationMapURL = sec.Key("USER_LOCATION_MAP_URL").MustString("https://www.openstreetmap.org/search?query=") Service.EnableUserHeatmap = sec.Key("ENABLE_USER_HEATMAP").MustBool(true) + Service.AddMembersByInvitations = sec.Key("ADD_MEMBERS_BY_INVITATIONS").MustBool(false) Service.AutoWatchNewRepos = sec.Key("AUTO_WATCH_NEW_REPOS").MustBool(true) Service.AutoWatchOnChanges = sec.Key("AUTO_WATCH_ON_CHANGES").MustBool(false) modes := sec.Key("ALLOWED_USER_VISIBILITY_MODES").Strings(",") diff --git a/options/locale_next/locale_en-US.json b/options/locale_next/locale_en-US.json index 3eb680af6d..f45e3d5e14 100644 --- a/options/locale_next/locale_en-US.json +++ b/options/locale_next/locale_en-US.json @@ -908,6 +908,8 @@ "teams.add_all_repos.modal.header": "Add all repositories", "teams.remove_all_repos.modal.header": "Remove all repositories", "members.add_member": "Add member", + "members.invite_member": "Invite member", + "members.invite_team_member": "Invite to team", "members.user": "User", "members.user_already_member": "This user is already a member of the organization.", "members.no_team_selected": "Organization members must belong to at least one team.", diff --git a/routers/api/v1/org/team.go b/routers/api/v1/org/team.go index bd76e4d37e..732166f6c1 100644 --- a/routers/api/v1/org/team.go +++ b/routers/api/v1/org/team.go @@ -498,8 +498,8 @@ func AddTeamMember(ctx *context.APIContext) { if ctx.Written() { return } - if err := models.AddTeamMember(ctx, ctx.Org.Team, u.ID); err != nil { - ctx.Error(http.StatusInternalServerError, "AddMember", err) + if err := org_service.InviteOrAddTeamMember(ctx, ctx.Doer, u, ctx.Org.Team); err != nil { + ctx.Error(http.StatusInternalServerError, "InviteOrAddTeamMember", err) return } ctx.Status(http.StatusNoContent) diff --git a/routers/web/org/members.go b/routers/web/org/members.go index 8a8c5122de..437e15d05e 100644 --- a/routers/web/org/members.go +++ b/routers/web/org/members.go @@ -16,6 +16,7 @@ import ( "forgejo.org/modules/setting" shared_user "forgejo.org/routers/web/shared/user" "forgejo.org/services/context" + org_service "forgejo.org/services/org" ) const ( @@ -77,6 +78,7 @@ func Members(ctx *context.Context) { ctx.Data["MembersIsPublicMember"] = membersIsPublic ctx.Data["MembersIsUserOrgOwner"] = organization.IsUserOrgOwner(ctx, members, org.ID) ctx.Data["MembersTwoFaStatus"] = members.GetTwoFaStatus(ctx) + ctx.Data["AddMembersByInvitations"] = setting.Service.AddMembersByInvitations ctx.HTML(http.StatusOK, tplMembers) } @@ -154,9 +156,9 @@ func MembersAction(ctx *context.Context) { for _, team := range teams { addToTeam := ctx.FormBool(fmt.Sprintf("team_%d", team.ID)) if addToTeam { - err = models.AddTeamMember(ctx, team, u.ID) + err = org_service.InviteOrAddTeamMember(ctx, ctx.Doer, u, team) if err != nil { - ctx.ServerError("AddTeamMember", err) + ctx.ServerError("InviteOrAddTeamMember", err) return } addedToTeam = true diff --git a/routers/web/org/teams.go b/routers/web/org/teams.go index a4cc647cdb..839646c1c4 100644 --- a/routers/web/org/teams.go +++ b/routers/web/org/teams.go @@ -133,7 +133,7 @@ func TeamsAction(ctx *context.Context) { if err != nil { if user_model.IsErrUserNotExist(err) { if setting.MailService != nil && validation.ValidateEmail(uname) == nil { - if err := org_service.CreateTeamInvite(ctx, ctx.Doer, ctx.Org.Team, uname); err != nil { + if err := org_service.CreateTeamInviteByEmail(ctx, ctx.Doer, ctx.Org.Team, uname); err != nil { if org_model.IsErrTeamInviteAlreadyExist(err) { ctx.Flash.Error(ctx.Tr("form.duplicate_invite_to_team")) } else if org_model.IsErrUserEmailAlreadyAdded(err) { @@ -162,7 +162,7 @@ func TeamsAction(ctx *context.Context) { if ctx.Org.Team.IsMember(ctx, u.ID) { ctx.Flash.Error(ctx.Tr("org.teams.add_duplicate_users")) } else { - err = models.AddTeamMember(ctx, ctx.Org.Team, u.ID) + err = org_service.InviteOrAddTeamMember(ctx, ctx.Doer, u, ctx.Org.Team) } page = "team" @@ -371,6 +371,7 @@ func TeamMembers(ctx *context.Context) { ctx.Data["Title"] = ctx.Org.Team.Name ctx.Data["PageIsOrgTeams"] = true ctx.Data["PageIsOrgTeamMembers"] = true + ctx.Data["AddMembersByInvitations"] = setting.Service.AddMembersByInvitations if err := shared_user.LoadHeaderCount(ctx); err != nil { ctx.ServerError("LoadHeaderCount", err) @@ -396,6 +397,12 @@ func TeamMembers(ctx *context.Context) { ctx.ServerError("GetInvitesByTeamID", err) return } + for _, invite := range invites { + if invite.LoadInvitedUser(ctx) != nil { + ctx.ServerError("LoadInvitedUser", err) + return + } + } ctx.Data["Invites"] = invites ctx.Data["IsEmailInviteEnabled"] = setting.MailService != nil @@ -583,6 +590,12 @@ func TeamInvite(ctx *context.Context) { return } + linkedToUser, invitedUserID := invite.InvitedID.Get() + if linkedToUser && invitedUserID != ctx.Doer.ID { + ctx.NotFound("ErrTeamInviteNotFound", nil) + return + } + ctx.Data["Title"] = ctx.Tr("org.teams.invite_team_member", team.Name) ctx.Data["Invite"] = invite ctx.Data["Organization"] = org @@ -604,6 +617,12 @@ func TeamInvitePost(ctx *context.Context) { return } + linkedToUser, invitedUserID := invite.InvitedID.Get() + if linkedToUser && invitedUserID != ctx.Doer.ID { + ctx.NotFound("ErrTeamInviteNotFound", nil) + return + } + if err := models.AddTeamMember(ctx, team, ctx.Doer.ID); err != nil { ctx.ServerError("AddTeamMember", err) return diff --git a/services/org/team_invite.go b/services/org/team_invite.go index 9c5da25522..48775bc69f 100644 --- a/services/org/team_invite.go +++ b/services/org/team_invite.go @@ -6,17 +6,37 @@ package org import ( "context" + "forgejo.org/models" org_model "forgejo.org/models/organization" user_model "forgejo.org/models/user" + "forgejo.org/modules/setting" "forgejo.org/services/mailer" ) -// CreateTeamInvite make a persistent invite in db and mail it -func CreateTeamInvite(ctx context.Context, inviter *user_model.User, team *org_model.Team, uname string) error { - invite, err := org_model.CreateTeamInvite(ctx, inviter, team, uname) +// CreateTeamInviteByEmail makes a persistent invite in db for someone without an account and mails it to them. +func CreateTeamInviteByEmail(ctx context.Context, inviter *user_model.User, team *org_model.Team, uname string) error { + invite, err := org_model.CreateTeamInviteByEmail(ctx, inviter, team, uname) if err != nil { return err } return mailer.MailTeamInvite(ctx, inviter, team, invite) } + +// CreateTeamInviteByUser makes a persistent invite in db for someone with an account already and mails it. +func CreateTeamInviteByUser(ctx context.Context, inviter, invited *user_model.User, team *org_model.Team) error { + invite, err := org_model.CreateTeamInviteForUser(ctx, inviter, invited, team) + if err != nil { + return err + } + // TODO: instead of only sending an email, also create an in-app notification + return mailer.MailTeamInvite(ctx, inviter, team, invite) +} + +// InviteOrAddTeamMember invites the user to the team if all team changes should go through invites, or adds them directly otherwise. +func InviteOrAddTeamMember(ctx context.Context, inviter, invited *user_model.User, team *org_model.Team) error { + if setting.Service.AddMembersByInvitations && inviter.ID != invited.ID { + return CreateTeamInviteByUser(ctx, inviter, invited, team) + } + return models.AddTeamMember(ctx, team, invited.ID) +} diff --git a/services/org/team_invite_test.go b/services/org/team_invite_test.go new file mode 100644 index 0000000000..dbb3656e98 --- /dev/null +++ b/services/org/team_invite_test.go @@ -0,0 +1,73 @@ +// Copyright 2026 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package org + +import ( + "testing" + + "forgejo.org/models/db" + "forgejo.org/models/organization" + "forgejo.org/models/unittest" + user_model "forgejo.org/models/user" + "forgejo.org/modules/optional" + "forgejo.org/modules/setting" + "forgejo.org/modules/test" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_InviteTeamMember(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + defer test.MockVariableValue(&setting.Service.AddMembersByInvitations, true)() + + team := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: 2}) + invited := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5}) + inviter := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + + require.NoError(t, InviteOrAddTeamMember(db.DefaultContext, inviter, invited, team)) + + unittest.AssertExistsAndLoadBean(t, &organization.TeamInvite{TeamID: team.ID, InviterID: inviter.ID, InvitedID: optional.Some(invited.ID)}) + isMember, err := organization.IsTeamMember(db.DefaultContext, team.OrgID, team.ID, invited.ID) + require.NoError(t, err) + assert.False(t, isMember) +} + +func Test_AddTeamMember(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + defer test.MockVariableValue(&setting.Service.AddMembersByInvitations, false)() + + team := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: 2}) + invited := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5}) + inviter := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + + require.NoError(t, InviteOrAddTeamMember(db.DefaultContext, inviter, invited, team)) + + unittest.AssertNotExistsBean(t, &organization.TeamInvite{TeamID: team.ID, InviterID: inviter.ID, InvitedID: optional.Some(invited.ID)}) + isMember, err := organization.IsTeamMember(db.DefaultContext, team.OrgID, team.ID, invited.ID) + require.NoError(t, err) + assert.True(t, isMember) +} + +func Test_SelfInvite(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + defer test.MockVariableValue(&setting.Service.AddMembersByInvitations, true)() + + team := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: 2}) + inviter := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + + isMember, err := organization.IsTeamMember(db.DefaultContext, team.OrgID, team.ID, inviter.ID) + require.NoError(t, err) + assert.False(t, isMember) + + // the inviter invites themselves to a team (they are owner of the org) + require.NoError(t, InviteOrAddTeamMember(db.DefaultContext, inviter, inviter, team)) + + // even though `ADD_MEMBERS_BY_INVITATIONS` is on, we don't generate an invite to the inviter, + // they are added to the team directly + unittest.AssertNotExistsBean(t, &organization.TeamInvite{TeamID: team.ID, InviterID: inviter.ID, InvitedID: optional.Some(inviter.ID)}) + isMember, err = organization.IsTeamMember(db.DefaultContext, team.OrgID, team.ID, inviter.ID) + require.NoError(t, err) + assert.True(t, isMember) +} diff --git a/services/user/delete.go b/services/user/delete.go index bcd465ce9d..192e4c8793 100644 --- a/services/user/delete.go +++ b/services/user/delete.go @@ -88,6 +88,8 @@ func deleteUser(ctx context.Context, u *user_model.User, purge bool) (err error) &user_model.UserOpenID{UID: u.ID}, &issues_model.Reaction{UserID: u.ID}, &organization.TeamUser{UID: u.ID}, + &organization.TeamInvite{InviterID: u.ID}, + &organization.TeamInvite{InvitedID: optional.Some(u.ID)}, &issues_model.Stopwatch{UserID: u.ID}, &user_model.Setting{UserID: u.ID}, &user_model.UserBadge{UserID: u.ID}, diff --git a/templates/org/member/add_org_member.tmpl b/templates/org/member/add_org_member.tmpl index e2b8f1b572..c95fafc356 100644 --- a/templates/org/member/add_org_member.tmpl +++ b/templates/org/member/add_org_member.tmpl @@ -11,7 +11,13 @@
-
{{ctx.Locale.Tr "members.add_member"}}
+
+ {{if $.AddMembersByInvitations}} + {{ctx.Locale.Tr "members.invite_member"}} + {{else}} + {{ctx.Locale.Tr "members.add_member"}} + {{end}} +
@@ -49,7 +55,13 @@ {{svg "octicon-x"}} {{ctx.Locale.Tr "cancel"}} - +
diff --git a/templates/org/member/members.tmpl b/templates/org/member/members.tmpl index 08bb8b9894..35d7338fdd 100644 --- a/templates/org/member/members.tmpl +++ b/templates/org/member/members.tmpl @@ -7,7 +7,12 @@ {{if $.IsOrganizationOwner}}
diff --git a/templates/org/team/members.tmpl b/templates/org/team/members.tmpl index e027db68da..8401ae960c 100644 --- a/templates/org/team/members.tmpl +++ b/templates/org/team/members.tmpl @@ -16,7 +16,13 @@ - + {{end}} @@ -57,9 +63,20 @@
{{range .Invites}}
-
- {{.Email}} -
+ {{if .InvitedUser}} +
+ {{ctx.AvatarUtils.Avatar .InvitedUser 32}} +
+
+
+ {{template "shared/user/name" .InvitedUser}} +
+
+ {{else}} +
+ {{.Email}} +
+ {{end}}
diff --git a/tests/integration/api_team_test.go b/tests/integration/api_team_test.go index 5055aae35d..987818d308 100644 --- a/tests/integration/api_team_test.go +++ b/tests/integration/api_team_test.go @@ -17,7 +17,10 @@ import ( "forgejo.org/models/unit" "forgejo.org/models/unittest" user_model "forgejo.org/models/user" + "forgejo.org/modules/optional" + "forgejo.org/modules/setting" api "forgejo.org/modules/structs" + "forgejo.org/modules/test" "forgejo.org/services/convert" "forgejo.org/tests" @@ -484,3 +487,39 @@ func TestAPIGetTeamRepoAccessTokenResources(t *testing.T) { }) }) } + +func TestAPIAddMemberDirectly(t *testing.T) { + defer tests.PrepareTestEnv(t)() + defer test.MockVariableValue(&setting.Service.AddMembersByInvitations, false)() + token := getUserToken(t, "user1", auth_model.AccessTokenScopeWriteOrganization) + + team := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: 2}) + user5 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5}) + + req := NewRequestf(t, "PUT", "/api/v1/teams/%d/members/%s", team.ID, user5.Name). + AddTokenAuth(token) + MakeRequest(t, req, http.StatusNoContent) + + isMember, err := organization.IsTeamMember(db.DefaultContext, team.OrgID, team.ID, user5.ID) + require.NoError(t, err) + assert.True(t, isMember) +} + +func TestAPIAddMemberGeneratesInvite(t *testing.T) { + defer tests.PrepareTestEnv(t)() + defer test.MockVariableValue(&setting.Service.AddMembersByInvitations, true)() + + token := getUserToken(t, "user1", auth_model.AccessTokenScopeWriteOrganization) + + team := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: 2}) + user5 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5}) + + req := NewRequestf(t, "PUT", "/api/v1/teams/%d/members/%s", team.ID, user5.Name). + AddTokenAuth(token) + MakeRequest(t, req, http.StatusNoContent) + + isMember, err := organization.IsTeamMember(db.DefaultContext, team.OrgID, team.ID, user5.ID) + require.NoError(t, err) + assert.False(t, isMember) + unittest.AssertExistsAndLoadBean(t, &organization.TeamInvite{TeamID: team.ID, InviterID: 1, InvitedID: optional.Some(user5.ID)}) +} diff --git a/tests/integration/fixtures/TestDisplayInvites/team_invite.yml b/tests/integration/fixtures/TestDisplayInvites/team_invite.yml new file mode 100644 index 0000000000..eadabebb64 --- /dev/null +++ b/tests/integration/fixtures/TestDisplayInvites/team_invite.yml @@ -0,0 +1,21 @@ +- + id: 1 + token: "T3jOj8VZi-vK3LPGvav3EM" + inviter_id: 1 + org_id: 3 + team_id: 2 + email: "user31@example.com" + invited_id: 31 + created_unix: 1778862588 + updated_unix: 1778862588 + +- + id: 2 + token: "D89db371f-uDtb824cBl9t" + inviter_id: 1 + org_id: 3 + team_id: 2 + email: "external_user@example.com" + invited_id: null + created_unix: 1778862590 + updated_unix: 1778862590 diff --git a/tests/integration/org_members_test.go b/tests/integration/org_members_test.go index b0cdb2f3f0..b068042256 100644 --- a/tests/integration/org_members_test.go +++ b/tests/integration/org_members_test.go @@ -13,6 +13,9 @@ import ( "forgejo.org/models/organization" "forgejo.org/models/unittest" user_model "forgejo.org/models/user" + "forgejo.org/modules/optional" + "forgejo.org/modules/setting" + "forgejo.org/modules/test" "forgejo.org/tests" "github.com/stretchr/testify/assert" @@ -56,7 +59,18 @@ func TestOrgMembersPage(t *testing.T) { assert.Less(t, 2, doc.Find(".members .list .link-action").Length()) assert.Less(t, 2, doc.Find(".members .list .delete-button").Length()) /* Adding new members is possible */ - doc.AssertElement(t, "#add-org-member-button", true) + assert.Equal(t, "Add member", strings.TrimSpace(doc.Find("#add-org-member-button").Text())) + }) + + t.Run("Owner PoV with ADD_MEMBERS_BY_INVITATIONS setting on", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + defer test.MockVariableValue(&setting.Service.AddMembersByInvitations, true)() + + session := loginUser(t, "user2") // user2 owns org3 + doc := NewHTMLParser(t, session.MakeRequest(t, NewRequest(t, "GET", testPage), http.StatusOK).Body) + + /* Inviting new members is possible */ + assert.Equal(t, "Invite member", strings.TrimSpace(doc.Find("#add-org-member-button").Text())) }) } @@ -221,3 +235,42 @@ func TestOrgAddExistingMemberFails(t *testing.T) { require.NoError(t, err) assert.False(t, isTeamMember) } + +func TestOrgAddMemberGeneratesAnInvite(t *testing.T) { + defer tests.PrepareTestEnv(t)() + defer test.MockVariableValue(&setting.Service.AddMembersByInvitations, true)() + + org := unittest.AssertExistsAndLoadBean(t, &organization.Organization{ID: 3}) + team1 := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: 1}) + team2 := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: 2}) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5}) + + isMember, err := organization.IsTeamMember(db.DefaultContext, team1.OrgID, team1.ID, user.ID) + require.NoError(t, err) + assert.False(t, isMember) + isMember, err = organization.IsTeamMember(db.DefaultContext, team2.OrgID, team2.ID, user.ID) + require.NoError(t, err) + assert.False(t, isMember) + + session := loginUser(t, "user2") + + teamURL := fmt.Sprintf("/org/%s/members", org.Name) + req := NewRequestWithValues(t, "POST", teamURL+"/action/add", map[string]string{ + "uid": "2", + "uname": user.LoginName, + "team_1": "on", + "team_2": "on", + }) + resp := session.MakeRequest(t, req, http.StatusSeeOther) + assert.Equal(t, teamURL, resp.Header().Get("Location")) + + user = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5}) + isMember, err = organization.IsTeamMember(db.DefaultContext, team1.OrgID, team1.ID, user.ID) + require.NoError(t, err) + assert.False(t, isMember) + isMember, err = organization.IsTeamMember(db.DefaultContext, team2.OrgID, team2.ID, user.ID) + require.NoError(t, err) + assert.False(t, isMember) + unittest.AssertExistsAndLoadBean(t, &organization.TeamInvite{TeamID: team1.ID, InviterID: 2, InvitedID: optional.Some(user.ID)}) + unittest.AssertExistsAndLoadBean(t, &organization.TeamInvite{TeamID: team2.ID, InviterID: 2, InvitedID: optional.Some(user.ID)}) +} diff --git a/tests/integration/org_team_invite_test.go b/tests/integration/org_team_invite_test.go index b36d1acb71..27d1b1414b 100644 --- a/tests/integration/org_team_invite_test.go +++ b/tests/integration/org_team_invite_test.go @@ -360,3 +360,89 @@ func TestOrgTeamEmailInviteRedirectsExistingUserWithLogin(t *testing.T) { require.NoError(t, err) assert.True(t, isMember) } + +// Test that a user can accept a team invite linked to their existing account +func TestOrgTeamEmailInviteExistingUser(t *testing.T) { + if setting.MailService == nil { + t.Skip() + return + } + + defer tests.PrepareTestEnv(t)() + + team := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: 2}) + inviter := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5}) + + isMember, err := organization.IsTeamMember(db.DefaultContext, team.OrgID, team.ID, user.ID) + require.NoError(t, err) + assert.False(t, isMember) + + // create the invite + invite, err := organization.CreateTeamInviteForUser(db.DefaultContext, inviter, user, team) + require.NoError(t, err) + + // log in the invited user + session := loginUser(t, "user5") + + // view the invite + inviteURL := fmt.Sprintf("/org/invite/%s", invite.Token) + req := NewRequest(t, "GET", inviteURL) + session.MakeRequest(t, req, http.StatusOK) + + // accept the invite + req = NewRequest(t, "POST", inviteURL) + resp := session.MakeRequest(t, req, http.StatusSeeOther) + req = NewRequest(t, "GET", test.RedirectURL(resp)) + session.MakeRequest(t, req, http.StatusOK) + + isMember, err = organization.IsTeamMember(db.DefaultContext, team.OrgID, team.ID, user.ID) + require.NoError(t, err) + assert.True(t, isMember) +} + +// Test that a user cannot accept an invite if it was meant for another user +func TestOrgTeamEmailInviteCannotBeAcceptedByOtherUser(t *testing.T) { + if setting.MailService == nil { + t.Skip() + return + } + + defer tests.PrepareTestEnv(t)() + + team := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: 2}) + inviter := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + invited := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5}) + attacker := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 11}) + + isMember, err := organization.IsTeamMember(db.DefaultContext, team.OrgID, team.ID, invited.ID) + require.NoError(t, err) + assert.False(t, isMember) + isMember, err = organization.IsTeamMember(db.DefaultContext, team.OrgID, team.ID, attacker.ID) + require.NoError(t, err) + assert.False(t, isMember) + + // create the invite for the invited user + invite, err := organization.CreateTeamInviteForUser(db.DefaultContext, inviter, invited, team) + require.NoError(t, err) + + // log in as the attacker + session := loginUser(t, attacker.Name) + + // viewing the invite doesn't work + inviteURL := fmt.Sprintf("/org/invite/%s", invite.Token) + req := NewRequest(t, "GET", inviteURL) + session.MakeRequest(t, req, http.StatusNotFound) + + // accepting the invite doesn't either + req = NewRequest(t, "POST", inviteURL) + session.MakeRequest(t, req, http.StatusNotFound) + + // neither the invited user nor the attacker are part of the team + isMember, err = organization.IsTeamMember(db.DefaultContext, team.OrgID, team.ID, invited.ID) + require.NoError(t, err) + assert.False(t, isMember) + isMember, err = organization.IsTeamMember(db.DefaultContext, team.OrgID, team.ID, attacker.ID) + require.NoError(t, err) + assert.False(t, isMember) +} diff --git a/tests/integration/org_team_test.go b/tests/integration/org_team_test.go index e4bdfb801a..3e6ecb5060 100644 --- a/tests/integration/org_team_test.go +++ b/tests/integration/org_team_test.go @@ -72,3 +72,52 @@ func TestPaginatedRepos(t *testing.T) { doc := NewHTMLParser(t, body) assert.Contains(t, strings.TrimSpace(doc.Find("a.item.navigation:contains('Next')").AttrOr("href", "")), fmt.Sprintf("%s?page=2", teamURL)) } + +func TestDisplayInvites(t *testing.T) { + defer unittest.OverrideFixtures("tests/integration/fixtures/TestDisplayInvites")() + defer tests.PrepareTestEnv(t)() + + org := unittest.AssertExistsAndLoadBean(t, &organization.Organization{ID: 3}) + team := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: 2}) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + + session := loginUser(t, user.Name) + + teamURL := fmt.Sprintf("/org/%s/teams/%s", org.Name, team.LowerName) + body := session.MakeRequest(t, NewRequest(t, "GET", teamURL), http.StatusOK).Body + doc := NewHTMLParser(t, body) + + // the two invited users are shown + assert.Equal(t, "/user31", doc.Find("a:contains('user31')").AttrOr("href", "")) + assert.Equal(t, 1, doc.Find("div.flex-item-main:contains('external_user@example.com')").Length()) +} + +func TestAddMembersByInvitations(t *testing.T) { + defer tests.PrepareTestEnv(t)() + defer test.MockVariableValue(&setting.Service.AddMembersByInvitations, true)() + + org := unittest.AssertExistsAndLoadBean(t, &organization.Organization{ID: 3}) + team := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: 2}) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + + session := loginUser(t, user.Name) + + teamURL := fmt.Sprintf("/org/%s/teams/%s", org.Name, team.LowerName) + body := session.MakeRequest(t, NewRequest(t, "GET", teamURL), http.StatusOK).Body + + // the button to add a team member says "invite" + doc := NewHTMLParser(t, body) + doc.AssertElement(t, "button.primary:contains('Invite to team')", true) + + // invite user "user31" to the team + req := NewRequestWithValues(t, "POST", fmt.Sprintf("%s/action/add", teamURL), map[string]string{ + "uname": "user31", + }) + resp := session.MakeRequest(t, req, http.StatusSeeOther) + assert.Equal(t, teamURL, resp.Header().Get("Location")) + + // the invited user is listed on the team page + body = session.MakeRequest(t, NewRequest(t, "GET", teamURL), http.StatusOK).Body + doc = NewHTMLParser(t, body) + assert.Equal(t, "/user31", doc.Find("a:contains('user31')").AttrOr("href", "")) +}