Skip to content

Commit 9c02f0c

Browse files
jrmccannonjrmccannon
authored
[PM-34883] - Add InjectOrganizationUserAttribute (#7536)
* Added InjectOrganizationUserAttribute and updated account-recovery put to use it. * Changes from code review
1 parent 53dc0c4 commit 9c02f0c

4 files changed

Lines changed: 302 additions & 45 deletions

File tree

‎src/Api/AdminConsole/Attributes/InjectOrganizationUserAttribute.cs‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
using Bit.Api.AdminConsole.Authorization;
2+
using Bit.Core.Exceptions;
3+
using Bit.Core.Repositories;
4+
using Microsoft.AspNetCore.Mvc;
5+
using Microsoft.AspNetCore.Mvc.ModelBinding;
6+
using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata;
7+
8+
namespace Bit.Api.AdminConsole.Attributes;
9+
10+
/// <summary>
11+
/// Binds a <see cref="Bit.Core.Entities.OrganizationUser"/> parameter by loading it from the database
12+
/// and validating that it belongs to the organization identified by the <c>orgId</c> or
13+
/// <c>organizationId</c> route parameter.
14+
/// </summary>
15+
/// <remarks>
16+
/// The organization user is resolved from the route parameter named by
17+
/// <see cref="OrganizationUserIdRouteParam"/> (default <c>"id"</c>). If the user is not found or
18+
/// does not belong to the organization, a <see cref="Bit.Core.Exceptions.NotFoundException"/> is thrown.
19+
/// </remarks>
20+
/// <example>
21+
/// <code><![CDATA[
22+
/// [HttpPut("{id}/recover-account")]
23+
/// [Authorize<ManageAccountRecoveryRequirement>]
24+
/// public async Task<IResult> PutRecoverAccount(Guid orgId, Guid id,
25+
/// [FromBody] OrganizationUserResetPasswordRequestModel model,
26+
/// [InjectOrganizationUser] OrganizationUser targetOrganizationUser)
27+
///
28+
/// [HttpPost("{organizationUserId}/accept")]
29+
/// public async Task<IResult> AcceptAsync(Guid organizationUserId,
30+
/// [InjectOrganizationUser("organizationUserId")] OrganizationUser organizationUser)
31+
/// ]]></code>
32+
/// </example>
33+
[AttributeUsage(AttributeTargets.Parameter)]
34+
public sealed class InjectOrganizationUserAttribute(string organizationUserIdRouteParam = "id")
35+
: ModelBinderAttribute(typeof(OrganizationUserModelBinder))
36+
{
37+
/// <summary>
38+
/// Name of the route parameter containing the organization user ID. Defaults to <c>"id"</c>.
39+
/// </summary>
40+
public string OrganizationUserIdRouteParam { get; } = organizationUserIdRouteParam;
41+
}
42+
43+
/// <summary>
44+
/// Custom model binder that loads an <see cref="Bit.Core.Entities.OrganizationUser"/> from the database,
45+
/// validates that it belongs to the organization identified by the route, and binds it to the parameter.
46+
/// </summary>
47+
/// <remarks>
48+
/// This binder is used via the <see cref="InjectOrganizationUserAttribute"/>.
49+
/// </remarks>
50+
public class OrganizationUserModelBinder : IModelBinder
51+
{
52+
public async Task BindModelAsync(ModelBindingContext bindingContext)
53+
{
54+
var defaultMetadata = bindingContext.ModelMetadata as DefaultModelMetadata;
55+
var attr = defaultMetadata?.Attributes.ParameterAttributes
56+
?.OfType<InjectOrganizationUserAttribute>()
57+
.FirstOrDefault()
58+
?? new InjectOrganizationUserAttribute();
59+
60+
Guid orgId;
61+
try
62+
{
63+
orgId = bindingContext.HttpContext.GetOrganizationId();
64+
}
65+
catch (InvalidOperationException)
66+
{
67+
throw new BadRequestException("Route parameter 'orgId' or 'organizationId' is missing or invalid.");
68+
}
69+
70+
var routeValues = bindingContext.ActionContext.RouteData.Values;
71+
if (!routeValues.TryGetValue(attr.OrganizationUserIdRouteParam, out var idValue)
72+
|| !Guid.TryParse(idValue?.ToString(), out var orgUserId))
73+
{
74+
throw new BadRequestException(
75+
$"Route parameter '{attr.OrganizationUserIdRouteParam}' is missing or invalid.");
76+
}
77+
78+
var repo = bindingContext.HttpContext.RequestServices
79+
.GetRequiredService<IOrganizationUserRepository>();
80+
81+
var organizationUser = await repo.GetByIdAsync(orgUserId);
82+
if (organizationUser is null || organizationUser.OrganizationId != orgId)
83+
{
84+
throw new NotFoundException();
85+
}
86+
87+
bindingContext.Result = ModelBindingResult.Success(organizationUser);
88+
}
89+
}

‎src/Api/AdminConsole/Controllers/OrganizationUsersController.cs‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// NOTE: This file is partially migrated to nullable reference types. Remove inline #nullable directives when addressing the FIXME.
33
#nullable disable
44

5+
using Bit.Api.AdminConsole.Attributes;
56
using Bit.Api.AdminConsole.Authorization;
67
using Bit.Api.AdminConsole.Authorization.Collections;
78
using Bit.Api.AdminConsole.Authorization.Requirements;
@@ -504,18 +505,15 @@ await _organizationService.UpdateUserResetPasswordEnrollmentAsync(
504505
/// </summary>
505506
[HttpPut("{id}/reset-password")]
506507
[Authorize<ManageAccountRecoveryRequirement>]
507-
public Task<IResult> PutResetPassword(Guid orgId, Guid id, [FromBody] OrganizationUserResetPasswordRequestModel model)
508-
=> PutRecoverAccount(orgId, id, model);
508+
public Task<IResult> PutResetPassword(Guid orgId, Guid id, [FromBody] OrganizationUserResetPasswordRequestModel model,
509+
[InjectOrganizationUser] OrganizationUser targetOrganizationUser)
510+
=> PutRecoverAccount(orgId, id, model, targetOrganizationUser);
509511

510512
[HttpPut("{id}/recover-account")]
511513
[Authorize<ManageAccountRecoveryRequirement>]
512-
public async Task<IResult> PutRecoverAccount(Guid orgId, Guid id, [FromBody] OrganizationUserResetPasswordRequestModel model)
514+
public async Task<IResult> PutRecoverAccount(Guid orgId, Guid id, [FromBody] OrganizationUserResetPasswordRequestModel model,
515+
[InjectOrganizationUser] OrganizationUser targetOrganizationUser)
513516
{
514-
var targetOrganizationUser = await _organizationUserRepository.GetByIdAsync(id);
515-
if (targetOrganizationUser == null || targetOrganizationUser.OrganizationId != orgId)
516-
{
517-
return TypedResults.NotFound();
518-
}
519517

520518
var authorizationResult = await _authorizationService.AuthorizeAsync(User, targetOrganizationUser, new RecoverAccountAuthorizationRequirement());
521519
if (!authorizationResult.Succeeded)

‎test/Api.Test/AdminConsole/Attributes/OrganizationUserModelBinderTests.cs‎

Lines changed: 202 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,202 @@
1+
using System.Reflection;
2+
using Bit.Api.AdminConsole.Attributes;
3+
using Bit.Core.Entities;
4+
using Bit.Core.Exceptions;
5+
using Bit.Core.Repositories;
6+
using Microsoft.AspNetCore.Http;
7+
using Microsoft.AspNetCore.Mvc;
8+
using Microsoft.AspNetCore.Mvc.ModelBinding;
9+
using Microsoft.AspNetCore.Routing;
10+
using Microsoft.Extensions.DependencyInjection;
11+
using NSubstitute;
12+
using Xunit;
13+
14+
namespace Bit.Api.Test.AdminConsole.Attributes;
15+
16+
public class OrganizationUserModelBinderTests
17+
{
18+
private readonly IOrganizationUserRepository _organizationUserRepository;
19+
private readonly OrganizationUser _organizationUser;
20+
private readonly Guid _orgId;
21+
private readonly Guid _orgUserId;
22+
23+
public OrganizationUserModelBinderTests()
24+
{
25+
_organizationUserRepository = Substitute.For<IOrganizationUserRepository>();
26+
_orgId = Guid.NewGuid();
27+
_orgUserId = Guid.NewGuid();
28+
_organizationUser = new OrganizationUser { Id = _orgUserId, OrganizationId = _orgId };
29+
}
30+
31+
[Fact]
32+
public async Task BindModelAsync_OrgUserExistsAndBelongsToOrg_BindsSuccessfully()
33+
{
34+
var binder = new OrganizationUserModelBinder();
35+
_organizationUserRepository.GetByIdAsync(_orgUserId).Returns(_organizationUser);
36+
37+
var context = CreateBindingContext();
38+
39+
await binder.BindModelAsync(context);
40+
41+
Assert.True(context.Result.IsModelSet);
42+
Assert.Equal(_organizationUser, context.Result.Model);
43+
}
44+
45+
[Fact]
46+
public async Task BindModelAsync_OrgUserNotFound_ThrowsNotFoundException()
47+
{
48+
var binder = new OrganizationUserModelBinder();
49+
_organizationUserRepository.GetByIdAsync(_orgUserId).Returns((OrganizationUser)null);
50+
51+
var context = CreateBindingContext();
52+
53+
await Assert.ThrowsAsync<NotFoundException>(() => binder.BindModelAsync(context));
54+
}
55+
56+
[Fact]
57+
public async Task BindModelAsync_OrgUserBelongsToDifferentOrg_ThrowsNotFoundException()
58+
{
59+
var binder = new OrganizationUserModelBinder();
60+
var wrongOrgUser = new OrganizationUser { Id = _orgUserId, OrganizationId = Guid.NewGuid() };
61+
_organizationUserRepository.GetByIdAsync(_orgUserId).Returns(wrongOrgUser);
62+
63+
var context = CreateBindingContext();
64+
65+
await Assert.ThrowsAsync<NotFoundException>(() => binder.BindModelAsync(context));
66+
}
67+
68+
[Fact]
69+
public async Task BindModelAsync_InvalidOrgId_ThrowsBadRequestException()
70+
{
71+
var binder = new OrganizationUserModelBinder();
72+
var context = CreateBindingContext(orgIdRouteValue: "not-a-guid");
73+
74+
var exception = await Assert.ThrowsAsync<BadRequestException>(() => binder.BindModelAsync(context));
75+
Assert.Equal("Route parameter 'orgId' or 'organizationId' is missing or invalid.", exception.Message);
76+
}
77+
78+
[Fact]
79+
public async Task BindModelAsync_MissingOrgId_ThrowsBadRequestException()
80+
{
81+
var binder = new OrganizationUserModelBinder();
82+
var context = CreateBindingContext(includeOrgId: false);
83+
84+
var exception = await Assert.ThrowsAsync<BadRequestException>(() => binder.BindModelAsync(context));
85+
Assert.Equal("Route parameter 'orgId' or 'organizationId' is missing or invalid.", exception.Message);
86+
}
87+
88+
[Fact]
89+
public async Task BindModelAsync_InvalidOrgUserId_ThrowsBadRequestException()
90+
{
91+
var binder = new OrganizationUserModelBinder();
92+
var context = CreateBindingContext(orgUserIdRouteValue: "not-a-guid");
93+
94+
var exception = await Assert.ThrowsAsync<BadRequestException>(() => binder.BindModelAsync(context));
95+
Assert.Equal("Route parameter 'id' is missing or invalid.", exception.Message);
96+
}
97+
98+
[Fact]
99+
public async Task BindModelAsync_MissingOrgUserId_ThrowsBadRequestException()
100+
{
101+
var binder = new OrganizationUserModelBinder();
102+
var context = CreateBindingContext(includeOrgUserId: false);
103+
104+
var exception = await Assert.ThrowsAsync<BadRequestException>(() => binder.BindModelAsync(context));
105+
Assert.Equal("Route parameter 'id' is missing or invalid.", exception.Message);
106+
}
107+
108+
[Fact]
109+
public async Task BindModelAsync_OrganizationIdRouteParam_ResolvesOrgId()
110+
{
111+
var binder = new OrganizationUserModelBinder();
112+
_organizationUserRepository.GetByIdAsync(_orgUserId).Returns(_organizationUser);
113+
114+
var context = CreateBindingContext(useOrganizationIdRoute: true);
115+
116+
await binder.BindModelAsync(context);
117+
118+
Assert.True(context.Result.IsModelSet);
119+
Assert.Equal(_organizationUser, context.Result.Model);
120+
}
121+
122+
[Fact]
123+
public async Task BindModelAsync_CustomRouteParamName_ReadsCorrectRouteValue()
124+
{
125+
var binder = new OrganizationUserModelBinder();
126+
_organizationUserRepository.GetByIdAsync(_orgUserId).Returns(_organizationUser);
127+
128+
var parameterInfo = typeof(OrganizationUserModelBinderTests)
129+
.GetMethod(nameof(DummyMethodWithCustomRouteParam), BindingFlags.NonPublic | BindingFlags.Static)!
130+
.GetParameters()[0];
131+
132+
var context = CreateBindingContext(
133+
orgUserIdRouteKey: "organizationUserId",
134+
parameterInfo: parameterInfo);
135+
136+
await binder.BindModelAsync(context);
137+
138+
Assert.True(context.Result.IsModelSet);
139+
Assert.Equal(_organizationUser, context.Result.Model);
140+
}
141+
142+
/// <summary>
143+
/// Dummy method used to produce a <see cref="ParameterInfo"/> carrying a custom
144+
/// <see cref="InjectOrganizationUserAttribute"/> for the custom route param test.
145+
/// </summary>
146+
private static void DummyMethodWithCustomRouteParam(
147+
[InjectOrganizationUser("organizationUserId")] OrganizationUser user)
148+
{ }
149+
150+
private DefaultModelBindingContext CreateBindingContext(
151+
string orgIdRouteValue = null,
152+
string orgUserIdRouteValue = null,
153+
string orgUserIdRouteKey = "id",
154+
bool includeOrgId = true,
155+
bool includeOrgUserId = true,
156+
bool useOrganizationIdRoute = false,
157+
ParameterInfo parameterInfo = null)
158+
{
159+
var httpContext = new DefaultHttpContext();
160+
var services = new ServiceCollection();
161+
services.AddScoped(_ => _organizationUserRepository);
162+
httpContext.RequestServices = services.BuildServiceProvider();
163+
164+
var routeData = new RouteData();
165+
if (includeOrgId)
166+
{
167+
var key = useOrganizationIdRoute ? "organizationId" : "orgId";
168+
routeData.Values[key] = orgIdRouteValue ?? _orgId.ToString();
169+
}
170+
if (includeOrgUserId)
171+
{
172+
routeData.Values[orgUserIdRouteKey] = orgUserIdRouteValue ?? _orgUserId.ToString();
173+
}
174+
175+
httpContext.Request.RouteValues = routeData.Values;
176+
177+
var actionContext = new ActionContext(
178+
httpContext,
179+
routeData,
180+
new Microsoft.AspNetCore.Mvc.Abstractions.ActionDescriptor(),
181+
new ModelStateDictionary());
182+
183+
var metadataProvider = new EmptyModelMetadataProvider();
184+
ModelMetadata metadata;
185+
186+
if (parameterInfo != null)
187+
{
188+
metadata = metadataProvider.GetMetadataForParameter(parameterInfo);
189+
}
190+
else
191+
{
192+
metadata = metadataProvider.GetMetadataForType(typeof(OrganizationUser));
193+
}
194+
195+
return new DefaultModelBindingContext
196+
{
197+
ActionContext = actionContext,
198+
ModelMetadata = metadata,
199+
ModelName = "organizationUser"
200+
};
201+
}
202+
}

0 commit comments

Comments
 (0)