From 7eaf4f88fecbf78452e735b968b96e5e922a693e Mon Sep 17 00:00:00 2001 From: Robin Munn Date: Tue, 26 Nov 2024 13:02:39 +0700 Subject: [PATCH] User typeahead enabled for non-admin project managers (#1237) Add a new usersICanSee GQL query, which looks for: * All users in one of my orgs * All users in one of the projects I manage * All users in one of the non-confidential projects I'm a member of Note that projects with `isConfidential = null` are treated as public (non-confidential) by this query. The "Add Project Member" typeahead is now updated to use that query, which allows project managers who aren't site admins to use it to find users whose email address they don't know. E.g. if Test Manager has Test Editor as part of project A, and he also manages project B, then when he clicks on "Add Members" in project B, he can type Test Editor's name and select him to add, without knowing his email address. --------- Co-authored-by: Tim Haasdyk --- backend/LexBoxApi/GraphQL/LexQueries.cs | 9 + .../Services/DevGqlSchemaWriterService.cs | 1 + backend/LexBoxApi/Services/UserService.cs | 18 +- .../ApiTests/UsersICanSeeQueryTests.cs | 98 +++++++ .../Fixtures/TempProjectWithoutRepo.cs | 39 +++ .../LexCore/Services/UserServiceTest.cs | 262 ++++++++++++++++++ backend/Testing/Testing.csproj | 1 + frontend/schema.graphql | 16 +- frontend/src/lib/forms/UserTypeahead.svelte | 17 +- frontend/src/lib/gql/typeahead-queries.ts | 18 +- .../org/[org_id]/AddOrgMemberModal.svelte | 5 +- .../project/[project_code]/+page.svelte | 2 +- .../[project_code]/AddProjectMember.svelte | 7 +- 13 files changed, 468 insertions(+), 25 deletions(-) create mode 100644 backend/Testing/ApiTests/UsersICanSeeQueryTests.cs create mode 100644 backend/Testing/Fixtures/TempProjectWithoutRepo.cs create mode 100644 backend/Testing/LexCore/Services/UserServiceTest.cs diff --git a/backend/LexBoxApi/GraphQL/LexQueries.cs b/backend/LexBoxApi/GraphQL/LexQueries.cs index f2326d2ef..72d947436 100644 --- a/backend/LexBoxApi/GraphQL/LexQueries.cs +++ b/backend/LexBoxApi/GraphQL/LexQueries.cs @@ -187,6 +187,15 @@ public IQueryable UsersInMyOrg(LexBoxDbContext context, LoggedInContext lo return context.Users.Where(u => u.Organizations.Any(orgMember => myOrgIds.Contains(orgMember.OrgId))); } + [UseOffsetPaging] + [UseProjection] + [UseFiltering] + [UseSorting] + public IQueryable UsersICanSee(UserService userService, LoggedInContext loggedInContext) + { + return userService.UserQueryForTypeahead(loggedInContext.User); + } + [UseProjection] [GraphQLType] public async Task OrgById(LexBoxDbContext dbContext, diff --git a/backend/LexBoxApi/Services/DevGqlSchemaWriterService.cs b/backend/LexBoxApi/Services/DevGqlSchemaWriterService.cs index a40a93c3f..c0b77997e 100644 --- a/backend/LexBoxApi/Services/DevGqlSchemaWriterService.cs +++ b/backend/LexBoxApi/Services/DevGqlSchemaWriterService.cs @@ -33,6 +33,7 @@ public static async Task GenerateGqlSchema(string[] args) .AddScoped() .AddScoped() .AddScoped() + .AddScoped() .AddScoped() .AddLexGraphQL(builder.Environment, true); var host = builder.Build(); diff --git a/backend/LexBoxApi/Services/UserService.cs b/backend/LexBoxApi/Services/UserService.cs index 277eba5a1..dc3f3a29f 100644 --- a/backend/LexBoxApi/Services/UserService.cs +++ b/backend/LexBoxApi/Services/UserService.cs @@ -1,13 +1,14 @@ using System.Net.Mail; -using LexBoxApi.Auth; using LexBoxApi.Services.Email; +using LexCore.Auth; +using LexCore.Entities; using LexCore.Exceptions; using LexData; using Microsoft.EntityFrameworkCore; namespace LexBoxApi.Services; -public class UserService(LexBoxDbContext dbContext, IEmailService emailService, LexAuthService lexAuthService) +public class UserService(LexBoxDbContext dbContext, IEmailService emailService) { public async Task ForgotPassword(string email) { @@ -83,4 +84,17 @@ public static (string name, string? email, string? username) ExtractNameAndAddre } return (name, email, username); } + + public IQueryable UserQueryForTypeahead(LexAuthUser user) + { + var myOrgIds = user.Orgs.Select(o => o.OrgId).ToList(); + var myProjectIds = user.Projects.Select(p => p.ProjectId).ToList(); + var myManagedProjectIds = user.Projects.Where(p => p.Role == ProjectRole.Manager).Select(p => p.ProjectId).ToList(); + return dbContext.Users.Where(u => + u.Id == user.Id || + u.Organizations.Any(orgMember => myOrgIds.Contains(orgMember.OrgId)) || + u.Projects.Any(projMember => + myManagedProjectIds.Contains(projMember.ProjectId) || + (projMember.Project != null && projMember.Project.IsConfidential != true && myProjectIds.Contains(projMember.ProjectId)))); + } } diff --git a/backend/Testing/ApiTests/UsersICanSeeQueryTests.cs b/backend/Testing/ApiTests/UsersICanSeeQueryTests.cs new file mode 100644 index 000000000..5c23843fa --- /dev/null +++ b/backend/Testing/ApiTests/UsersICanSeeQueryTests.cs @@ -0,0 +1,98 @@ +using System.Text.Json.Nodes; +using Shouldly; +using Testing.Services; + +namespace Testing.ApiTests; + +[Trait("Category", "Integration")] +public class UsersICanSeeQueryTests : ApiTestBase +{ + private async Task QueryUsersICanSee(bool expectGqlError = false) + { + var json = await ExecuteGql( + $$""" + query { + usersICanSee(take: 10) { + totalCount + items { + id + name + } + } + } + """, + expectGqlError, expectSuccessCode: false); + return json; + } + + private async Task AddUserToProject(Guid projectId, string username) + { + await ExecuteGql( + $$""" + mutation { + addProjectMember(input: { + projectId: "{{projectId}}", + usernameOrEmail: "{{username}}", + role: EDITOR, + canInvite: false + }) { + project { + id + } + errors { + __typename + ... on Error { + message + } + } + } + } + """); + } + + private JsonArray GetUsers(JsonObject json) + { + var users = json["data"]!["usersICanSee"]!["items"]!.AsArray(); + users.ShouldNotBeNull(); + return users; + } + + private void MustHaveUser(JsonArray users, string userName) + { + users.ShouldNotBeNull().ShouldNotBeEmpty(); + users.ShouldContain(node => node!["name"]!.GetValue() == userName, + "user list " + users.ToJsonString()); + } + + private void MustNotHaveUser(JsonArray users, string userName) + { + users.ShouldNotBeNull().ShouldNotBeEmpty(); + users.ShouldNotContain(node => node!["name"]!.GetValue() == userName, + "user list " + users.ToJsonString()); + } + + [Fact] + public async Task ManagerCanSeeProjectMembersOfAllProjects() + { + await LoginAs("manager"); + await using var project = await this.RegisterProjectInLexBox(Utils.GetNewProjectConfig(isConfidential: true)); + //refresh jwt + await LoginAs("manager"); + await AddUserToProject(project.Id, "qa@test.com"); + var json = GetUsers(await QueryUsersICanSee()); + MustHaveUser(json, "Qa Admin"); + } + + [Fact] + public async Task MemberCanSeeNotProjectMembersOfConfidentialProjects() + { + await LoginAs("manager"); + await using var project = await this.RegisterProjectInLexBox(Utils.GetNewProjectConfig(isConfidential: true)); + //refresh jwt + await LoginAs("manager"); + await AddUserToProject(project.Id, "qa@test.com"); + await LoginAs("editor"); + var json = GetUsers(await QueryUsersICanSee()); + MustNotHaveUser(json, "Qa Admin"); + } +} diff --git a/backend/Testing/Fixtures/TempProjectWithoutRepo.cs b/backend/Testing/Fixtures/TempProjectWithoutRepo.cs new file mode 100644 index 000000000..81c5aa702 --- /dev/null +++ b/backend/Testing/Fixtures/TempProjectWithoutRepo.cs @@ -0,0 +1,39 @@ +using LexCore.Entities; +using LexData; +using Testing.Services; + +namespace Testing.Fixtures; + +public class TempProjectWithoutRepo(LexBoxDbContext dbContext, Project project) : IAsyncDisposable +{ + public Project Project => project; + public static async Task Create(LexBoxDbContext dbContext, bool isConfidential = false, Guid? managerId = null) + { + var config = Utils.GetNewProjectConfig(isConfidential: isConfidential); + var project = new Project + { + Name = config.Name, + Code = config.Code, + IsConfidential = config.IsConfidential, + LastCommit = null, + Organizations = [], + Users = [], + RetentionPolicy = RetentionPolicy.Test, + Type = ProjectType.FLEx, + Id = config.Id, + }; + if (managerId is Guid id) + { + project.Users.Add(new ProjectUsers { ProjectId = project.Id, UserId = id, Role = ProjectRole.Manager }); + } + dbContext.Add(project); + await dbContext.SaveChangesAsync(); + return new TempProjectWithoutRepo(dbContext, project); + } + + public async ValueTask DisposeAsync() + { + dbContext.Remove(project); + await dbContext.SaveChangesAsync(); + } +} diff --git a/backend/Testing/LexCore/Services/UserServiceTest.cs b/backend/Testing/LexCore/Services/UserServiceTest.cs new file mode 100644 index 000000000..16028cd9c --- /dev/null +++ b/backend/Testing/LexCore/Services/UserServiceTest.cs @@ -0,0 +1,262 @@ +using LexBoxApi.Services; +using LexBoxApi.Services.Email; +using LexCore.Auth; +using LexCore.Entities; +using LexData; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; +using Moq; +using Testing.Fixtures; +using FluentAssertions; + +namespace Testing.LexCore.Services; + +[Collection(nameof(TestingServicesFixture))] +public class UserServiceTest : IAsyncLifetime +{ + private readonly UserService _userService; + + private readonly LexBoxDbContext _lexBoxDbContext; + private List ManagedProjects { get; } = []; + private List ManagedUsers { get; } = []; + private List ManagedOrgs { get; } = []; + + // Users created for this test + private User? Robin { get; set; } + private User? John { get; set; } + private User? Alan { get; set; } + private User? Marian { get; set; } + private User? Bishop { get; set; } + private User? Tuck { get; set; } + private User? Sheriff { get; set; } + private User? Guy { get; set; } + // Projects created for this test + private Project? Sherwood { get; set; } + private Project? Nottingham { get; set; } + // Orgs created for this test + private Organization? Outlaws { get; set; } + private Organization? LawEnforcement { get; set; } + private Organization? Church { get; set; } + + public UserServiceTest(TestingServicesFixture testing) + { + var serviceProvider = testing.ConfigureServices(s => + { + s.AddScoped(_ => Mock.Of()); + s.AddScoped(); + }); + _userService = serviceProvider.GetRequiredService(); + _lexBoxDbContext = serviceProvider.GetRequiredService(); + } + + public Task InitializeAsync() + { + Robin = CreateUser("Robin Hood"); + John = CreateUser("Little John"); + Alan = CreateUser("Alan a Dale"); + Marian = CreateUser("Maid Marian"); + Bishop = CreateUser("Bishop of Hereford"); + Tuck = CreateUser("Friar Tuck"); + Sheriff = CreateUser("Sheriff of Nottingham"); + Guy = CreateUser("Guy of Gisbourne"); + + Nottingham = CreateProject([Sheriff.Id], [Marian.Id, Tuck.Id]); + Sherwood = CreateConfidentialProject([Robin.Id, Marian.Id], [John.Id, Alan.Id, Tuck.Id]); + + Outlaws = CreateOrg([Robin.Id], [John.Id]); // Alan a Dale should *NOT* be in this org + LawEnforcement = CreateOrg([Sheriff.Id], [Guy.Id]); + Church = CreateOrg([Bishop.Id], [Tuck.Id]); + + return _lexBoxDbContext.SaveChangesAsync(); + } + + public Task DisposeAsync() + { + foreach (var project in ManagedProjects) + { + _lexBoxDbContext.Remove(project); + } + foreach (var user in ManagedUsers) + { + _lexBoxDbContext.Remove(user); + } + foreach (var org in ManagedOrgs) + { + _lexBoxDbContext.Remove(org); + } + return _lexBoxDbContext.SaveChangesAsync(); + } + + public void UserListShouldBe(IEnumerable actual, IEnumerable expected) + { + var actualNames = actual.Select(u => u.Name); + var expectedNames = expected.Select(u => u?.Name ?? ""); + actualNames.Should().BeEquivalentTo(expectedNames, options => options.WithoutStrictOrdering()); + } + + [Fact] + public async Task ManagerCanSeeAllUsersEvenInConfidentialProjects() + { + // Robin Hood is in Outlaws org (admin) and Sherwood project (private, manager) + var authUser = new LexAuthUser(Robin!); + var users = await _userService.UserQueryForTypeahead(authUser).ToArrayAsync(); + // John, who is in both the Outlaws org (user) and Sherwood project (member) is not duplicated + UserListShouldBe(users, [Robin, Marian, John, Alan, Tuck]); + } + + [Fact] + public async Task NonManagerCanNotSeeUsersInConfidentialProjects() + { + // Little John is in Outlaws org (user) and Sherwood project (private, member) + var authUser = new LexAuthUser(John!); + var users = await _userService.UserQueryForTypeahead(authUser).ToArrayAsync(); + // John can see Robin because he shares an org, but not Marian even though she's a manager of the Sherwood project + UserListShouldBe(users, [Robin, John]); + } + + [Fact] + public async Task ManagerOfOneProjectAndMemberOfAnotherPublicProjectCanSeeUsersInBoth() + { + // Maid Marian is in no orgs and two projects: Sherwood (private, manager) and Nottingham (public, member) + var authUser = new LexAuthUser(Marian!); + var users = await _userService.UserQueryForTypeahead(authUser).ToArrayAsync(); + // Marian can see everyone in both projects; Tuck is not duplicated despite being in both projects + UserListShouldBe(users, [Robin, Marian, John, Alan, Tuck, Sheriff]); + } + + [Fact] + public async Task ManagerOfOneProjectAndMemberOfAnotherConfidentialProjectCanNotSeeUsersInConfidentialProject() + { + // Sheriff of Nottingham is in LawEnforcement org (admin) and Nottingham project (pulbic, manager) + try + { + // Sheriff tries to sneak into Sherwood... + await AddUserToProject(Sherwood!, Sheriff!); + // ... but can still only see the users in Nottingham and LawEnforcement + var authUser = new LexAuthUser(Sheriff!); + var users = await _userService.UserQueryForTypeahead(authUser).ToArrayAsync(); + UserListShouldBe(users, [Sheriff, Guy, Marian, Tuck]); + } + finally + { + await RemoveUserFromProject(Sherwood!, Sheriff!); + } + } + + [Fact] + public async Task OrgAdminsInNoProjectsCanSeeOnlyTheirOrg() + { + // Bishop of Hereford is in Church org (admin) but no projects + var authUser = new LexAuthUser(Bishop!); + var users = await _userService.UserQueryForTypeahead(authUser).ToArrayAsync(); + // Bishop can only see members of Church org + UserListShouldBe(users, [Bishop, Tuck]); + } + + [Fact] + public async Task OrgMembersInNoProjectsCanSeeOnlyTheirOrg() + { + // Guy of Gisborne is in LawEnforcement org (user) but no projects + var authUser = new LexAuthUser(Guy!); + var users = await _userService.UserQueryForTypeahead(authUser).ToArrayAsync(); + // Guy can only see members of LawEnforcement org + UserListShouldBe(users, [Sheriff, Guy]); + } + + [Fact] + public async Task OrgAndProjectMembersCanSeeFellowOrgMembersAndFellowPublicProjectMembersButNotFellowPrivateProjectMembers() + { + // Friar Tuck is in Church org (user) and two projects: Nottingham (public, member) and Sherwood (private, member) + var authUser = new LexAuthUser(Tuck!); + var users = await _userService.UserQueryForTypeahead(authUser).ToArrayAsync(); + // Tuck can see everyone in Church and Nottingham, but nobody in Sherwood because it's private — though he can see Marian because he shares a public project with her + UserListShouldBe(users, [Bishop, Tuck, Sheriff, Marian]); + } + + [Fact] + public async Task MemberOfOnePrivateProjectButNoOrgsCanOnlySeeHimself() + { + // Alan a Dale is in Sherwood project (private, member) but no orgs + var authUser = new LexAuthUser(Alan!); + var users = await _userService.UserQueryForTypeahead(authUser).ToArrayAsync(); + // Alan can see himself in the Sherwood project, but nobody else because it's private + UserListShouldBe(users, [Alan]); + } + + private User CreateUser(string name) + { + var email = name.ToLowerInvariant().Replace(' ', '_') + "@example.com"; + var user = new User + { + Name = name, + Email = email, + CanCreateProjects = true, + EmailVerified = true, + IsAdmin = name.Contains("Admin"), + PasswordHash = "", + Salt = "" + }; + _lexBoxDbContext.Add(user); + ManagedUsers.Add(user); + return user; // Caller must call SaveChanges after all users and projects are added + } + + private Project CreateProject(IEnumerable managers, IEnumerable members, bool isConfidential = false) + { + var config = Testing.Services.Utils.GetNewProjectConfig(); + var project = new Project + { + Name = config.Name, + Code = config.Code, + IsConfidential = isConfidential, + LastCommit = null, + Organizations = [], + Users = [], + RetentionPolicy = RetentionPolicy.Test, + Type = ProjectType.FLEx, + Id = config.Id, + }; + project.Users.AddRange(managers.Select(userId => new ProjectUsers { UserId = userId, Role = ProjectRole.Manager })); + project.Users.AddRange(members.Select(userId => new ProjectUsers { UserId = userId, Role = ProjectRole.Editor })); + _lexBoxDbContext.Add(project); + ManagedProjects.Add(project); + return project; // Caller must call SaveChanges after all users and projects are added + } + + private Project CreateConfidentialProject(IEnumerable managers, IEnumerable members) + { + return CreateProject(managers, members, true); + } + + private async Task AddUserToProject(Project project, User user, ProjectRole role = ProjectRole.Editor) + { + var pu = project.Users.FirstOrDefault(pu => pu.UserId == user.Id); + if (pu is null) project.Users.Add(new ProjectUsers { UserId = user.Id, Role = role }); + else pu.Role = role; + await _lexBoxDbContext.SaveChangesAsync(); + } + + private async Task RemoveUserFromProject(Project project, User user) + { + var pu = project.Users.FirstOrDefault(pu => pu.UserId == user.Id); + if (pu is not null) project.Users.Remove(pu); + await _lexBoxDbContext.SaveChangesAsync(); + } + + private Organization CreateOrg(IEnumerable managers, IEnumerable members) + { + var id = Guid.NewGuid(); + var shortId = id.ToString().Split("-")[0]; + var org = new Organization + { + Name = shortId, + Members = [], + Projects = [], + }; + org.Members.AddRange(managers.Select(userId => new OrgMember { UserId = userId, Role = OrgRole.Admin })); + org.Members.AddRange(members.Select(userId => new OrgMember { UserId = userId, Role = OrgRole.User })); + _lexBoxDbContext.Add(org); + ManagedOrgs.Add(org); + return org; + } +} diff --git a/backend/Testing/Testing.csproj b/backend/Testing/Testing.csproj index 3ccb8771e..0c2ffc87a 100644 --- a/backend/Testing/Testing.csproj +++ b/backend/Testing/Testing.csproj @@ -27,6 +27,7 @@ + diff --git a/frontend/schema.graphql b/frontend/schema.graphql index 9e136c3a6..9a315d8c3 100644 --- a/frontend/schema.graphql +++ b/frontend/schema.graphql @@ -192,6 +192,10 @@ type InvalidEmailError implements Error { address: String! } +type InvalidOperationError implements Error { + message: String! +} + type IsAdminResponse { value: Boolean! } @@ -441,6 +445,7 @@ type Query { orgs(where: OrganizationFilterInput @cost(weight: "10") orderBy: [OrganizationSortInput!] @cost(weight: "10")): [Organization!]! @cost(weight: "10") myOrgs(where: OrganizationFilterInput @cost(weight: "10") orderBy: [OrganizationSortInput!] @cost(weight: "10")): [Organization!]! @cost(weight: "10") usersInMyOrg(skip: Int take: Int where: UserFilterInput @cost(weight: "10") orderBy: [UserSortInput!] @cost(weight: "10")): UsersInMyOrgCollectionSegment @listSize(assumedSize: 1000, slicingArguments: [ "take" ], sizedFields: [ "items" ]) @cost(weight: "10") + usersICanSee(skip: Int take: Int where: UserFilterInput @cost(weight: "10") orderBy: [UserSortInput!] @cost(weight: "10")): UsersICanSeeCollectionSegment @listSize(assumedSize: 1000, slicingArguments: [ "take" ], sizedFields: [ "items" ]) @cost(weight: "10") orgById(orgId: UUID!): OrgById @cost(weight: "10") users(skip: Int take: Int where: UserFilterInput @cost(weight: "10") orderBy: [UserSortInput!] @cost(weight: "10")): UsersCollectionSegment @authorize(policy: "AdminRequiredPolicy") @listSize(assumedSize: 1000, slicingArguments: [ "take" ], sizedFields: [ "items" ]) @cost(weight: "10") me: MeDto @cost(weight: "10") @@ -562,6 +567,15 @@ type UsersCollectionSegment { totalCount: Int! @cost(weight: "10") } +"A segment of a collection." +type UsersICanSeeCollectionSegment { + "Information to aid in pagination." + pageInfo: CollectionSegmentInfo! + "A flattened list of the items." + items: [User!] + totalCount: Int! @cost(weight: "10") +} + "A segment of a collection." type UsersInMyOrgCollectionSegment { "Information to aid in pagination." @@ -615,7 +629,7 @@ union LeaveProjectError = NotFoundError | LastMemberCantLeaveError union RemoveProjectFromOrgError = DbError | NotFoundError -union SendNewVerificationEmailByAdminError = NotFoundError | DbError | UniqueValueError +union SendNewVerificationEmailByAdminError = NotFoundError | DbError | InvalidOperationError union SetOrgMemberRoleError = DbError | NotFoundError | OrgMemberInvitedByEmail diff --git a/frontend/src/lib/forms/UserTypeahead.svelte b/frontend/src/lib/forms/UserTypeahead.svelte index 2f300197d..4f721f17e 100644 --- a/frontend/src/lib/forms/UserTypeahead.svelte +++ b/frontend/src/lib/forms/UserTypeahead.svelte @@ -1,6 +1,6 @@