Skip to content

Commit e99ed21

Browse files
rmunnhahn-kev
andauthored
Org managers have full access to projects owned by their org (#919)
* Org managers have full access to projects in their org Org managers can view, edit, and sync any projects owned by the org they manage, even if they themselves are not explicitly listed as a member of that project. This is similar to how site admins have full access to anything, except restricted to projects owned by the org(s) in question. * Await PermissionService calls that are now async Some PermissionService calls now have to be async because they query the database. We try to cache the results of those queries when possible to save time, but the queries still need to be async. Async calls can't be used in the Where clause of a LINQ expression, so CanSyncProject had to be split into sync and async versions; the sync version doesn't query whether a user is an org manager/admin, but the only place where it's called is in a method that has already checked org managership earlier. * Allow org admins to edit project data in frontend * Fix LexAuthUserTests * Set up to test cloning confidential projects First step is to teach integration fixture how to create confidential projects. * IntegrationFixture can now create projects owned by orgs Now we're ready to test Send/Receive of confidential projects by an org manager who's not explicitly listed as a project member. * Test S/R of confidential project by org manager * Cache project ID to org IDs list for 1 hour This will greatly reduce the number of lookups that need to be made when an organization manager are trying to manage a project that he's not an explicit member of. * Also cache project confidentiality for 1 hour --------- Co-authored-by: Kevin Hahn <kevin_hahn@sil.org>
1 parent 62e5eec commit e99ed21

File tree

15 files changed

+145
-42
lines changed

15 files changed

+145
-42
lines changed

backend/LexBoxApi/Auth/JwtTicketDataFormat.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ public static void FixUpArrayClaims(JwtSecurityToken token)
9393
{
9494
if (!token.Payload.TryGetValue(claimName, out var value))
9595
{
96-
return; // no claim to fix up, so nothing to do
96+
continue; // no claim to fix up, so nothing to do
9797
}
9898

9999
// if there's only 1 object it will be a stored in the payload as just an object and not an array.

backend/LexBoxApi/Controllers/IntegrationController.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public class IntegrationController(
3535
[ProducesResponseType(StatusCodes.Status302Found)]
3636
public async Task<ActionResult> OpenWithFlex(Guid projectId)
3737
{
38-
if (!permissionService.CanSyncProject(projectId)) return Unauthorized();
38+
if (!await permissionService.CanSyncProjectAsync(projectId)) return Unauthorized();
3939
var project = await lexBoxDbContext.Projects.FirstOrDefaultAsync(p => p.Id == projectId);
4040
if (project is null) return NotFound();
4141
var repoId = await hgService.GetRepositoryIdentifier(project);

backend/LexBoxApi/GraphQL/OrgMutations.cs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using LexBoxApi.Auth;
22
using LexBoxApi.Auth.Attributes;
33
using LexBoxApi.Models.Org;
4+
using LexBoxApi.Services;
45
using LexCore.Entities;
56
using LexCore.Exceptions;
67
using LexCore.ServiceInterfaces;
@@ -61,6 +62,7 @@ public async Task<Organization> DeleteOrg(Guid orgId,
6162
public async Task<IQueryable<Organization>> AddProjectToOrg(
6263
LexBoxDbContext dbContext,
6364
IPermissionService permissionService,
65+
[Service] ProjectService projectService,
6466
Guid orgId,
6567
Guid projectId)
6668
{
@@ -71,7 +73,7 @@ public async Task<IQueryable<Organization>> AddProjectToOrg(
7173
.Include(p => p.Organizations)
7274
.SingleOrDefaultAsync();
7375
NotFoundException.ThrowIfNull(project);
74-
permissionService.AssertCanManageProject(projectId);
76+
await permissionService.AssertCanManageProject(projectId);
7577

7678
if (project.Organizations.Exists(o => o.Id == orgId))
7779
{
@@ -81,6 +83,7 @@ public async Task<IQueryable<Organization>> AddProjectToOrg(
8183
project.Organizations.Add(org);
8284
project.UpdateUpdatedDate();
8385
org.UpdateUpdatedDate();
86+
projectService.InvalidateProjectOrgIdsCache(projectId);
8487
await dbContext.SaveChangesAsync();
8588
return dbContext.Orgs.Where(o => o.Id == orgId);
8689
}
@@ -93,6 +96,7 @@ public async Task<IQueryable<Organization>> AddProjectToOrg(
9396
public async Task<IQueryable<Organization>> RemoveProjectFromOrg(
9497
LexBoxDbContext dbContext,
9598
IPermissionService permissionService,
99+
[Service] ProjectService projectService,
96100
Guid orgId,
97101
Guid projectId)
98102
{
@@ -103,13 +107,14 @@ public async Task<IQueryable<Organization>> RemoveProjectFromOrg(
103107
.Include(p => p.Organizations)
104108
.SingleOrDefaultAsync();
105109
NotFoundException.ThrowIfNull(project);
106-
permissionService.AssertCanManageProject(projectId);
110+
await permissionService.AssertCanManageProject(projectId);
107111
var foundOrg = project.Organizations.FirstOrDefault(o => o.Id == orgId);
108112
if (foundOrg is not null)
109113
{
110114
project.Organizations.Remove(foundOrg);
111115
project.UpdateUpdatedDate();
112116
org.UpdateUpdatedDate();
117+
projectService.InvalidateProjectOrgIdsCache(projectId);
113118
await dbContext.SaveChangesAsync();
114119
}
115120
// If org did not own project, return with no error

backend/LexBoxApi/GraphQL/ProjectMutations.cs

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ public async Task<IQueryable<Project>> AddProjectMember(IPermissionService permi
7575
LexBoxDbContext dbContext,
7676
[Service] IEmailService emailService)
7777
{
78-
permissionService.AssertCanManageProject(input.ProjectId);
78+
await permissionService.AssertCanManageProject(input.ProjectId);
7979
var project = await dbContext.Projects.FindAsync(input.ProjectId);
8080
NotFoundException.ThrowIfNull(project);
8181
var user = await dbContext.Users.Include(u => u.Projects).FindByEmailOrUsername(input.UsernameOrEmail);
@@ -231,7 +231,7 @@ public async Task<IQueryable<ProjectUsers>> ChangeProjectMemberRole(
231231
IPermissionService permissionService,
232232
LexBoxDbContext dbContext)
233233
{
234-
permissionService.AssertCanManageProjectMemberRole(input.ProjectId, input.UserId);
234+
await permissionService.AssertCanManageProjectMemberRole(input.ProjectId, input.UserId);
235235
var projectUser =
236236
await dbContext.ProjectUsers
237237
.Include(r => r.Project)
@@ -259,7 +259,7 @@ public async Task<IQueryable<Project>> ChangeProjectName(ChangeProjectNameInput
259259
IPermissionService permissionService,
260260
LexBoxDbContext dbContext)
261261
{
262-
permissionService.AssertCanManageProject(input.ProjectId);
262+
await permissionService.AssertCanManageProject(input.ProjectId);
263263
if (input.Name.IsNullOrEmpty()) throw new RequiredException("Project name cannot be empty");
264264

265265
var project = await dbContext.Projects.FindAsync(input.ProjectId);
@@ -280,7 +280,7 @@ public async Task<IQueryable<Project>> ChangeProjectDescription(ChangeProjectDes
280280
IPermissionService permissionService,
281281
LexBoxDbContext dbContext)
282282
{
283-
permissionService.AssertCanManageProject(input.ProjectId);
283+
await permissionService.AssertCanManageProject(input.ProjectId);
284284
var project = await dbContext.Projects.FindAsync(input.ProjectId);
285285
NotFoundException.ThrowIfNull(project);
286286

@@ -297,14 +297,16 @@ public async Task<IQueryable<Project>> ChangeProjectDescription(ChangeProjectDes
297297
[UseProjection]
298298
public async Task<IQueryable<Project>> SetProjectConfidentiality(SetProjectConfidentialityInput input,
299299
IPermissionService permissionService,
300+
[Service] ProjectService projectService,
300301
LexBoxDbContext dbContext)
301302
{
302-
permissionService.AssertCanManageProject(input.ProjectId);
303+
await permissionService.AssertCanManageProject(input.ProjectId);
303304
var project = await dbContext.Projects.FindAsync(input.ProjectId);
304305
NotFoundException.ThrowIfNull(project);
305306

306307
project.IsConfidential = input.IsConfidential;
307308
project.UpdateUpdatedDate();
309+
projectService.InvalidateProjectConfidentialityCache(input.ProjectId);
308310
await dbContext.SaveChangesAsync();
309311
return dbContext.Projects.Where(p => p.Id == input.ProjectId);
310312
}
@@ -340,7 +342,7 @@ public async Task<IQueryable<Project>> RemoveProjectMember(RemoveProjectMemberIn
340342
IPermissionService permissionService,
341343
LexBoxDbContext dbContext)
342344
{
343-
permissionService.AssertCanManageProject(input.ProjectId);
345+
await permissionService.AssertCanManageProject(input.ProjectId);
344346
await dbContext.ProjectUsers.Where(pu => pu.ProjectId == input.ProjectId && pu.UserId == input.UserId)
345347
.ExecuteDeleteAsync();
346348
// Not doing .Include() above because we don't want the project or user removed, just the many-to-many table row
@@ -379,10 +381,11 @@ public async Task<DraftProject> DeleteDraftProject(
379381
public async Task<IQueryable<Project>> SoftDeleteProject(
380382
Guid projectId,
381383
IPermissionService permissionService,
384+
[Service] ProjectService projectService,
382385
LexBoxDbContext dbContext,
383386
IHgService hgService)
384387
{
385-
permissionService.AssertCanManageProject(projectId);
388+
await permissionService.AssertCanManageProject(projectId);
386389

387390
var project = await dbContext.Projects.Include(p => p.Users).FirstOrDefaultAsync(p => p.Id == projectId);
388391
if (project is null)
@@ -402,6 +405,7 @@ public async Task<IQueryable<Project>> SoftDeleteProject(
402405
using var transaction = await dbContext.Database.BeginTransactionAsync();
403406
await dbContext.SaveChangesAsync();
404407
await hgService.SoftDeleteRepo(projectCode, timestamp);
408+
projectService.InvalidateProjectConfidentialityCache(projectId);
405409
await transaction.CommitAsync();
406410

407411
return dbContext.Projects.Where(p => p.Id == projectId);

backend/LexBoxApi/Services/PermissionService.cs

Lines changed: 34 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,23 +2,33 @@
22
using LexCore.Auth;
33
using LexCore.Entities;
44
using LexCore.ServiceInterfaces;
5-
using LexData;
65

76
namespace LexBoxApi.Services;
87

98
public class PermissionService(
109
LoggedInContext loggedInContext,
11-
LexBoxDbContext dbContext,
1210
ProjectService projectService)
1311
: IPermissionService
1412
{
1513
private LexAuthUser? User => loggedInContext.MaybeUser;
1614

15+
private async ValueTask<bool> ManagesOrgThatOwnsProject(Guid projectId)
16+
{
17+
if (User is not null && User.Orgs.Any(o => o.Role == OrgRole.Admin))
18+
{
19+
// Org admins can view, edit, and sync all projects, even confidential ones
20+
var managedOrgIds = User.Orgs.Where(o => o.Role == OrgRole.Admin).Select(o => o.OrgId).ToHashSet();
21+
var projectOrgIds = await projectService.LookupProjectOrgIds(projectId);
22+
if (projectOrgIds.Any(oId => managedOrgIds.Contains(oId))) return true;
23+
}
24+
return false;
25+
}
26+
1727
public async ValueTask<bool> CanSyncProject(string projectCode)
1828
{
1929
if (User is null) return false;
2030
if (User.Role == UserRole.admin) return true;
21-
return CanSyncProject(await projectService.LookupProjectId(projectCode));
31+
return await CanSyncProjectAsync(await projectService.LookupProjectId(projectCode));
2232
}
2333

2434
public bool CanSyncProject(Guid projectId)
@@ -29,24 +39,32 @@ public bool CanSyncProject(Guid projectId)
2939
return User.Projects.Any(p => p.ProjectId == projectId);
3040
}
3141

42+
public async ValueTask<bool> CanSyncProjectAsync(Guid projectId)
43+
{
44+
if (CanSyncProject(projectId)) return true;
45+
// Org managers can sync any project owned by their org(s)
46+
return await ManagesOrgThatOwnsProject(projectId);
47+
}
48+
3249
public async ValueTask AssertCanSyncProject(string projectCode)
3350
{
3451
if (!await CanSyncProject(projectCode)) throw new UnauthorizedAccessException();
3552
}
3653

37-
public void AssertCanSyncProject(Guid projectId)
54+
public async ValueTask AssertCanSyncProject(Guid projectId)
3855
{
39-
if (!CanSyncProject(projectId)) throw new UnauthorizedAccessException();
56+
if (!await CanSyncProjectAsync(projectId)) throw new UnauthorizedAccessException();
4057
}
4158

4259
public async ValueTask<bool> CanViewProject(Guid projectId)
4360
{
4461
if (User is not null && User.Role == UserRole.admin) return true;
4562
if (User is not null && User.Projects.Any(p => p.ProjectId == projectId)) return true;
46-
var project = await dbContext.Projects.FindAsync(projectId);
47-
if (project is null) return false;
48-
if (project.IsConfidential is null) return false; // Private by default
49-
return project.IsConfidential == false; // Explicitly set to public
63+
// Org admins can view all projects, even confidential ones
64+
if (await ManagesOrgThatOwnsProject(projectId)) return true;
65+
var isConfidential = await projectService.LookupProjectConfidentiality(projectId);
66+
if (isConfidential is null) return false; // Private by default
67+
return isConfidential == false; // Explicitly set to public
5068
}
5169

5270
public async ValueTask AssertCanViewProject(Guid projectId)
@@ -65,22 +83,23 @@ public async ValueTask AssertCanViewProject(string projectCode)
6583
if (!await CanViewProject(projectCode)) throw new UnauthorizedAccessException();
6684
}
6785

68-
public bool CanManageProject(Guid projectId)
86+
public async ValueTask<bool> CanManageProject(Guid projectId)
6987
{
7088
if (User is null) return false;
7189
if (User.Role == UserRole.admin) return true;
72-
return User.Projects.Any(p => p.ProjectId == projectId && p.Role == ProjectRole.Manager);
90+
if (User.Projects.Any(p => p.ProjectId == projectId && p.Role == ProjectRole.Manager)) return true;
91+
return await ManagesOrgThatOwnsProject(projectId);
7392
}
7493

75-
public void AssertCanManageProject(Guid projectId)
94+
public async ValueTask AssertCanManageProject(Guid projectId)
7695
{
77-
if (!CanManageProject(projectId)) throw new UnauthorizedAccessException();
96+
if (!await CanManageProject(projectId)) throw new UnauthorizedAccessException();
7897
}
7998

80-
public void AssertCanManageProjectMemberRole(Guid projectId, Guid userId)
99+
public async ValueTask AssertCanManageProjectMemberRole(Guid projectId, Guid userId)
81100
{
82101
if (User is null) throw new UnauthorizedAccessException();
83-
AssertCanManageProject(projectId);
102+
await AssertCanManageProject(projectId);
84103
if (User.Role != UserRole.admin && userId == User.Id)
85104
throw new UnauthorizedAccessException("Not allowed to change own project role.");
86105
}

backend/LexBoxApi/Services/ProjectService.cs

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ public async Task<Guid> CreateProject(CreateProjectInput input)
5252
}
5353
await dbContext.SaveChangesAsync();
5454
await hgService.InitRepo(input.Code);
55+
InvalidateProjectOrgIdsCache(projectId);
56+
InvalidateProjectConfidentialityCache(projectId);
5557
await transaction.CommitAsync();
5658
return projectId;
5759
}
@@ -133,6 +135,39 @@ public async Task FinishReset(string code, Stream? zipFile = null)
133135
await dbContext.SaveChangesAsync();
134136
}
135137

138+
public async ValueTask<Guid[]> LookupProjectOrgIds(Guid projectId)
139+
{
140+
var cacheKey = $"ProjectOrgsForId:{projectId}";
141+
if (memoryCache.TryGetValue(cacheKey, out Guid[]? orgIds)) return orgIds ?? [];
142+
orgIds = await dbContext.Projects
143+
.Where(p => p.Id == projectId)
144+
.Select(p => p.Organizations.Select(o => o.Id).ToArray())
145+
.FirstOrDefaultAsync();
146+
memoryCache.Set(cacheKey, orgIds, TimeSpan.FromHours(1));
147+
return orgIds ?? [];
148+
}
149+
150+
public void InvalidateProjectOrgIdsCache(Guid projectId)
151+
{
152+
try { memoryCache.Remove($"ProjectOrgsForId:{projectId}"); }
153+
catch (Exception) { } // Never allow this to throw
154+
}
155+
156+
public async ValueTask<bool?> LookupProjectConfidentiality(Guid projectId)
157+
{
158+
var cacheKey = $"ProjectConfidentiality:{projectId}";
159+
if (memoryCache.TryGetValue(cacheKey, out bool? confidential)) return confidential;
160+
var project = await dbContext.Projects.FindAsync(projectId);
161+
memoryCache.Set(cacheKey, project?.IsConfidential, TimeSpan.FromHours(1));
162+
return project?.IsConfidential;
163+
}
164+
165+
public void InvalidateProjectConfidentialityCache(Guid projectId)
166+
{
167+
try { memoryCache.Remove($"ProjectConfidentiality:{projectId}"); }
168+
catch (Exception) { } // Never allow this to throw
169+
}
170+
136171
public async Task UpdateProjectMetadataForCode(string projectCode)
137172
{
138173
var project = await dbContext.Projects

backend/LexCore/Entities/Organization.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using System.ComponentModel.DataAnnotations.Schema;
22
using System.Linq.Expressions;
3+
using System.Text.Json.Serialization;
34
using EntityFrameworkCore.Projectables;
45

56
namespace LexCore.Entities;
@@ -30,6 +31,7 @@ public class OrgMember : EntityBase
3031
public Organization? Organization { get; set; }
3132
}
3233

34+
[JsonConverter(typeof(JsonStringEnumConverter))]
3335
public enum OrgRole
3436
{
3537
Unknown = 0,

backend/LexCore/ServiceInterfaces/IPermissionService.cs

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,24 @@ namespace LexCore.ServiceInterfaces;
55
public interface IPermissionService
66
{
77
ValueTask<bool> CanSyncProject(string projectCode);
8+
/// <summary>
9+
/// Does NOT check permissions for org managers, because that requires async DB access.
10+
/// Use CanSyncProject(projectCode) or CanSyncProjectAsync(projectId) if org manager permissions also need to be checked.
11+
/// </summary>
812
bool CanSyncProject(Guid projectId);
13+
/// <summary>
14+
/// Does all the checks from CanSyncProject, plus allows org managers to access the org as well.
15+
/// </summary>
16+
ValueTask<bool> CanSyncProjectAsync(Guid projectId);
917
ValueTask AssertCanSyncProject(string projectCode);
10-
void AssertCanSyncProject(Guid projectId);
18+
ValueTask AssertCanSyncProject(Guid projectId);
1119
ValueTask<bool> CanViewProject(Guid projectId);
1220
ValueTask AssertCanViewProject(Guid projectId);
1321
ValueTask<bool> CanViewProject(string projectCode);
1422
ValueTask AssertCanViewProject(string projectCode);
15-
bool CanManageProject(Guid projectId);
16-
void AssertCanManageProject(Guid projectId);
17-
void AssertCanManageProjectMemberRole(Guid projectId, Guid userId);
23+
ValueTask<bool> CanManageProject(Guid projectId);
24+
ValueTask AssertCanManageProject(Guid projectId);
25+
ValueTask AssertCanManageProjectMemberRole(Guid projectId, Guid userId);
1826
void AssertIsAdmin();
1927
void AssertCanDeleteAccount(Guid userId);
2028
bool HasProjectCreatePermission();

backend/Testing/Fixtures/IntegrationFixture.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,9 @@ private static void InitTemplateRepo()
5555
}
5656
}
5757

58-
public ProjectConfig InitLocalFlexProjectWithRepo(HgProtocol? protocol = null, [CallerMemberName] string projectName = "")
58+
public ProjectConfig InitLocalFlexProjectWithRepo(HgProtocol? protocol = null, bool isConfidential = false, Guid? owningOrgId = null, [CallerMemberName] string projectName = "")
5959
{
60-
var projectConfig = Utils.GetNewProjectConfig(protocol, projectName);
60+
var projectConfig = Utils.GetNewProjectConfig(protocol, isConfidential, owningOrgId, projectName);
6161
InitLocalFlexProjectWithRepo(projectConfig);
6262
return projectConfig;
6363
}

backend/Testing/LexCore/LexAuthUserTests.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ static LexAuthUserTests()
3636
Name = "test",
3737
UpdatedDate = DateTimeOffset.Now.ToUnixTimeSeconds(),
3838
Locale = "en",
39+
Orgs = [ new AuthUserOrg(OrgRole.Admin, LexData.SeedingData.TestOrgId) ],
3940
Projects = new[]
4041
{
4142
new AuthUserProject(ProjectRole.Manager, new Guid("42f566c0-a4d2-48b5-a1e1-59c82289ff99"))
@@ -183,7 +184,8 @@ public void CanParseFromKnownGoodJwt()
183184
var newUser = LexAuthUser.FromClaimsPrincipal(principal);
184185
newUser.ShouldNotBeNull();
185186
newUser.UpdatedDate.ShouldBe(0);
186-
//old jwt doesn't have updated date, we're ok with that so we correct the value to make the equivalence work
187+
//old jwt doesn't have updated date or orgs, we're ok with that so we correct the values to make the equivalence work
188+
newUser.Orgs = [ new AuthUserOrg(OrgRole.Admin, LexData.SeedingData.TestOrgId) ];
187189
newUser.UpdatedDate = _user.UpdatedDate;
188190
newUser.ShouldBeEquivalentTo(_user);
189191
}

0 commit comments

Comments
 (0)