Skip to content

Commit

Permalink
satellite/console: don't delete expired project invitations
Browse files Browse the repository at this point in the history 
This change removes instances of project invitation deletion due to
expiration because we now want such invitations to be accessible beyond
their expiration date. In the future, project members will be able to
view and resend expired invitations within the Team page in the
satellite frontend.

References #5752

Change-Id: If24a9637945874d719b894a66c06f6e0e9805dfa
  • Loading branch information
@jewharton
jewharton committed Jun 21, 2023
1 parent b6026b9 commit d18f4f7
Show file tree
Hide file tree
Showing 7 changed files with 232 additions and 104 deletions.
8 changes: 8 additions & 0 deletions satellite/console/projectinvitations.go
View file
Expand Up @@ -22,6 +22,8 @@ type ProjectInvitations interface {
GetByProjectID(ctx context.Context, projectID uuid.UUID) ([]ProjectInvitation, error)
// GetByEmail returns all of the project member invitations for the specified email address.
GetByEmail(ctx context.Context, email string) ([]ProjectInvitation, error)
// Update updates the project member invitation specified by the given project ID and email address.
Update(ctx context.Context, projectID uuid.UUID, email string, request UpdateProjectInvitationRequest) (*ProjectInvitation, error)
// Delete removes a project member invitation from the database.
Delete(ctx context.Context, projectID uuid.UUID, email string) error
// DeleteBefore deletes project member invitations created prior to some time from the database.
Expand All @@ -35,3 +37,9 @@ type ProjectInvitation struct {
InviterID *uuid.UUID
CreatedAt time.Time
}

// UpdateProjectInvitationRequest contains all fields which may be updated by ProjectInvitations.Update.
type UpdateProjectInvitationRequest struct {
CreatedAt *time.Time
InviterID *uuid.UUID
}
64 changes: 18 additions & 46 deletions satellite/console/service.go
View file
Expand Up @@ -75,7 +75,7 @@ const (
projInviteInvalidErrMsg = "The invitation has expired or is invalid"
projInviteAlreadyMemberErrMsg = "You are already a member of the project"
projInviteResponseInvalidErrMsg = "Invalid project member invitation response"
projInviteExistsErrMsg = "User has already been invited"
projInviteActiveErrMsg = "The invitation for '%s' has not expired yet"
)

var (
Expand Down Expand Up @@ -143,8 +143,8 @@ var (
// or has expired.
ErrProjectInviteInvalid = errs.Class("invalid project invitation")

// ErrProjectInviteExists occurs when a user is invited to a project they've already been invited to.
ErrProjectInviteExists = errs.Class("user already invited to project")
// ErrProjectInviteActive occurs when trying to reinvite a user whose invitation hasn't expired yet.
ErrProjectInviteActive = errs.Class("project invitation active")
)

// Service is handling accounts related logic.
Expand Down Expand Up @@ -3470,26 +3470,10 @@ func (s *Service) GetUserProjectInvitations(ctx context.Context) (_ []ProjectInv
}

var active []ProjectInvitation
var deleteErrs []error
var expiredIDs []string
for _, invite := range invites {
if time.Now().After(invite.CreatedAt.Add(s.config.ProjectInvitationExpiration)) {
err := s.store.ProjectInvitations().Delete(ctx, invite.ProjectID, invite.Email)
if err != nil {
deleteErrs = append(deleteErrs, err)
expiredIDs = append(expiredIDs, invite.ProjectID.String())
}
continue
if !time.Now().After(invite.CreatedAt.Add(s.config.ProjectInvitationExpiration)) {
active = append(active, invite)
}
active = append(active, invite)
}

if len(deleteErrs) != 0 {
s.log.Warn("error deleting expired project invitations",
zap.Errors("errors", deleteErrs),
zap.String("email", user.Email),
zap.Strings("projectIDs", expiredIDs),
)
}

return active, nil
Expand Down Expand Up @@ -3580,6 +3564,7 @@ func (s *Service) RespondToProjectInvitation(ctx context.Context, projectID uuid
}

// InviteProjectMembers invites users by email to given project.
// If an invitation already exists and has expired, it will be replaced and the user will be sent a new email.
// Email addresses not belonging to a user are ignored.
// projectID here may be project.PublicID or project.ID.
func (s *Service) InviteProjectMembers(ctx context.Context, projectID uuid.UUID, emails []string) (invites []ProjectInvitation, err error) {
Expand Down Expand Up @@ -3611,24 +3596,13 @@ func (s *Service) InviteProjectMembers(ctx context.Context, projectID uuid.UUID,
if err != nil && !errs.Is(err, sql.ErrNoRows) {
return nil, Error.Wrap(err)
}
if invite != nil && time.Now().After(invite.CreatedAt.Add(s.config.ProjectInvitationExpiration)) {
// delete expired invite
err := s.store.ProjectInvitations().Delete(ctx, projectID, invitedUser.Email)
if err != nil {
s.log.Warn("error deleting project invitation",
zap.Error(err),
zap.String("email", invitedUser.Email),
zap.String("projectID", projectID.String()),
)
}
} else if invite != nil && !time.Now().After(invite.CreatedAt.Add(s.config.ProjectInvitationExpiration)) {
return nil, ErrProjectInviteExists.New(projInviteExistsErrMsg)
if invite != nil && !time.Now().After(invite.CreatedAt.Add(s.config.ProjectInvitationExpiration)) {
return nil, ErrProjectInviteActive.New(projInviteActiveErrMsg, invitedUser.Email)
}
users = append(users, invitedUser)
} else if !errs.Is(err, sql.ErrNoRows) {
return nil, Error.Wrap(err)
}

}

inviteTokens := make(map[string]string)
Expand All @@ -3641,11 +3615,17 @@ func (s *Service) InviteProjectMembers(ctx context.Context, projectID uuid.UUID,
InviterID: &user.ID,
})
if err != nil {
if dbx.IsConstraintError(err) {
// should not happen, but just in case.
return errs.New("%s is already invited", invited.Email)
if !dbx.IsConstraintError(err) {
return err
}
now := time.Now()
invite, err = tx.ProjectInvitations().Update(ctx, projectID, invited.Email, UpdateProjectInvitationRequest{
CreatedAt: &now,
InviterID: &user.ID,
})
if err != nil {
return err
}
return err
}
token, err := s.CreateInviteToken(ctx, isMember.project.PublicID, invited.Email, invite.CreatedAt.Add(s.config.ProjectInvitationExpiration))
if err != nil {
Expand Down Expand Up @@ -3707,14 +3687,6 @@ func (s *Service) GetInviteByToken(ctx context.Context, token string) (invite *P
return nil, ErrProjectInviteInvalid.New(projInviteInvalidErrMsg)
}
if time.Now().After(invite.CreatedAt.Add(s.config.ProjectInvitationExpiration)) {
err = s.store.ProjectInvitations().Delete(ctx, invite.ProjectID, invite.Email)
if err != nil {
s.log.Warn("error deleting expired project invitations",
zap.Error(err),
zap.String("email", email),
zap.String("projectID", project.ID.String()),
)
}
return nil, ErrProjectInviteInvalid.New(projInviteInvalidErrMsg)
}

Expand Down
71 changes: 30 additions & 41 deletions satellite/console/service_test.go
View file
Expand Up @@ -11,7 +11,6 @@ import (
"fmt"
"math/rand"
"sort"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -1975,31 +1974,23 @@ func TestProjectInvitations(t *testing.T) {
return project
}

setInviteDate := func(ctx context.Context, invite *console.ProjectInvitation, createdAt time.Time) *console.ProjectInvitation {
result, err := sat.DB.Testing().RawDB().ExecContext(ctx,
"UPDATE project_invitations SET created_at = $1 WHERE project_id = $2 AND email = $3",
createdAt, invite.ProjectID, strings.ToUpper(invite.Email),
)
require.NoError(t, err)

count, err := result.RowsAffected()
require.NoError(t, err)
require.EqualValues(t, 1, count)

invite, err = sat.DB.Console().ProjectInvitations().Get(ctx, invite.ProjectID, invite.Email)
require.NoError(t, err)
return invite
}

addInvite := func(t *testing.T, ctx context.Context, project *console.Project, email string, createdAt time.Time) *console.ProjectInvitation {
addInvite := func(t *testing.T, ctx context.Context, project *console.Project, email string) *console.ProjectInvitation {
invite, err := sat.DB.Console().ProjectInvitations().Insert(ctx, &console.ProjectInvitation{
ProjectID: project.ID,
Email: email,
InviterID: &project.OwnerID,
})
require.NoError(t, err)

return setInviteDate(ctx, invite, createdAt)
return invite
}

expireInvite := func(t *testing.T, ctx context.Context, invite *console.ProjectInvitation) {
createdAt := time.Now().Add(-sat.Config.Console.ProjectInvitationExpiration)
_, err := sat.DB.Console().ProjectInvitations().Update(ctx, invite.ProjectID, invite.Email, console.UpdateProjectInvitationRequest{
CreatedAt: &createdAt,
})
require.NoError(t, err)
}

t.Run("invite users", func(t *testing.T) {
Expand All @@ -2026,19 +2017,7 @@ func TestProjectInvitations(t *testing.T) {
invites, err = service.GetUserProjectInvitations(ctx3)
require.NoError(t, err)
require.Len(t, invites, 1)
invite := invites[0]

// inviting the same user again should fail if existing invite hasn't expired.
_, err = service.InviteProjectMembers(ctx, project.ID, []string{user3.Email})
require.Error(t, err)

// expire the invitation.
setInviteDate(ctx, &invite, time.Now().Add(-sat.Config.Console.ProjectInvitationExpiration))

// inviting the same user again should succeed because the existing invite has expired.
invites, err = service.InviteProjectMembers(ctx, project.ID, []string{user3.Email})
require.NoError(t, err)
require.Len(t, invites, 1)
user3Invite := invites[0]

// prevent unauthorized users from inviting others (user2 is not a member of the project yet).
_, err = service.InviteProjectMembers(ctx2, project.ID, []string{"other@mail.com"})
Expand All @@ -2047,9 +2026,18 @@ func TestProjectInvitations(t *testing.T) {

require.NoError(t, service.RespondToProjectInvitation(ctx2, project.ID, console.ProjectInvitationAccept))

// now that user2 is a member, they can invite others.
_, err = service.InviteProjectMembers(ctx2, project.ID, []string{"other@mail.com"})
// resending an active invitation should fail.
invites, err = service.InviteProjectMembers(ctx2, project.ID, []string{user3.Email})
require.True(t, console.ErrProjectInviteActive.Has(err))
require.Empty(t, invites)

// resending an expired invitation should succeed.
expireInvite(t, ctx, &user3Invite)
invites, err = service.InviteProjectMembers(ctx2, project.ID, []string{user3.Email})
require.NoError(t, err)
require.Len(t, invites, 1)
require.Equal(t, user2.ID, *invites[0].InviterID)
require.True(t, invites[0].CreatedAt.After(user3Invite.CreatedAt))

// inviting a project member should fail.
_, err = service.InviteProjectMembers(ctx, project.ID, []string{user2.Email})
Expand All @@ -2063,7 +2051,7 @@ func TestProjectInvitations(t *testing.T) {
require.NoError(t, err)
require.Empty(t, invites)

invite := addInvite(t, ctx, addProject(t, ctx), user.Email, time.Now())
invite := addInvite(t, ctx, addProject(t, ctx), user.Email)
invites, err = service.GetUserProjectInvitations(ctx)
require.NoError(t, err)
require.Len(t, invites, 1)
Expand All @@ -2072,7 +2060,7 @@ func TestProjectInvitations(t *testing.T) {
require.Equal(t, invite.InviterID, invites[0].InviterID)
require.WithinDuration(t, invite.CreatedAt, invites[0].CreatedAt, time.Second)

setInviteDate(ctx, invite, time.Now().Add(-sat.Config.Console.ProjectInvitationExpiration))
expireInvite(t, ctx, &invites[0])
invites, err = service.GetUserProjectInvitations(ctx)
require.NoError(t, err)
require.Empty(t, invites)
Expand Down Expand Up @@ -2118,7 +2106,7 @@ func TestProjectInvitations(t *testing.T) {
project, err := sat.AddProject(ctx, owner.ID, "Test Project")
require.NoError(t, err)

invite := addInvite(t, ctx, project, user.Email, time.Now())
invite := addInvite(t, ctx, project, user.Email)

someToken, err := service.CreateInviteToken(ctx, project.PublicID, "some@email.com", invite.CreatedAt)
require.NoError(t, err)
Expand All @@ -2136,7 +2124,7 @@ func TestProjectInvitations(t *testing.T) {
require.NotNil(t, inviteFromToken)
require.Equal(t, invite, inviteFromToken)

setInviteDate(ctx, invite, time.Now().Add(-sat.Config.Console.ProjectInvitationExpiration))
expireInvite(t, ctx, invite)
invites, err := service.GetUserProjectInvitations(ctx)
require.NoError(t, err)
require.Empty(t, invites)
Expand All @@ -2158,11 +2146,12 @@ func TestProjectInvitations(t *testing.T) {
user, ctx := getUserAndCtx(t)
proj := addProject(t, ctx)

addInvite(t, ctx, proj, user.Email, time.Now().Add(-sat.Config.Console.ProjectInvitationExpiration))
invite := addInvite(t, ctx, proj, user.Email)
expireInvite(t, ctx, invite)
err := service.RespondToProjectInvitation(ctx, proj.ID, console.ProjectInvitationAccept)
require.True(t, console.ErrProjectInviteInvalid.Has(err))

addInvite(t, ctx, proj, user.Email, time.Now())
addInvite(t, ctx, proj, user.Email)
require.NoError(t, service.RespondToProjectInvitation(ctx, proj.ID, console.ProjectInvitationAccept))

invites, err := service.GetUserProjectInvitations(ctx)
Expand All @@ -2186,7 +2175,7 @@ func TestProjectInvitations(t *testing.T) {
user, ctx := getUserAndCtx(t)
proj := addProject(t, ctx)

addInvite(t, ctx, proj, user.Email, time.Now())
addInvite(t, ctx, proj, user.Email)
require.NoError(t, service.RespondToProjectInvitation(ctx, proj.ID, console.ProjectInvitationDecline))

invites, err := service.GetUserProjectInvitations(ctx)
Expand Down
9 changes: 7 additions & 2 deletions satellite/satellitedb/dbx/project.dbx
View file
Expand Up @@ -164,9 +164,9 @@ model project_invitation (
// See satellitedb.normalizeEmail for details.
field email text
// inviter_id is the ID of the user who sent the invitation.
field inviter_id user.id setnull ( nullable )
field inviter_id user.id setnull ( nullable, updatable )
// created_at is the time that the invitation was created.
field created_at timestamp ( autoinsert )
field created_at timestamp ( autoinsert, updatable )
)

create project_invitation ( )
Expand All @@ -187,6 +187,11 @@ read all (
where project_invitation.project_id = ?
)

update project_invitation (
where project_invitation.project_id = ?
where project_invitation.email = ?
)

delete project_invitation (
where project_invitation.project_id = ?
where project_invitation.email = ?
Expand Down

0 comments on commit d18f4f7

Please sign in to comment.