Skip to content

Commit 66730e6

Browse files
feat: Refactor file transport handling across controllers and views
- Updated authorization policies to remove legacy FTP claims and replace them with file transport claims. - Refactored GameServersController, CredentialsController, and MapRotationsController to use new file transport properties. - Removed deprecated FTP handling from various models and extensions. - Added new extension methods for setting file transport properties in CreateGameServerDto and EditGameServerDto. - Updated tests to reflect changes in file transport logic and ensure proper authorization handling.
1 parent 343bbfb commit 66730e6

25 files changed

Lines changed: 135 additions & 344 deletions

.github/copilot-instructions.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ ASP.NET Core 9 web application (`src/XtremeIdiots.Portal.Web/`) providing the Xt
4747

4848
## Authorization Model
4949

50-
The portal uses a `{Domain}.{Action}` policy naming convention (e.g., `AdminActions.Create`, `GameServers.Credentials.Ftp.Read`). Policies are defined as constants in the `AuthPolicies` class (`Auth/Constants/AuthPolicies.cs`) and registered via `PolicyExtensions.AddXtremeIdiotsPolicies()`. Each policy maps to a marker requirement class and a corresponding authorization handler.
50+
The portal uses a `{Domain}.{Action}` policy naming convention (e.g., `AdminActions.Create`, `GameServers.Credentials.FileTransport.Read`). Policies are defined as constants in the `AuthPolicies` class (`Auth/Constants/AuthPolicies.cs`) and registered via `PolicyExtensions.AddXtremeIdiotsPolicies()`. Each policy maps to a marker requirement class and a corresponding authorization handler.
5151

5252
### Role Hierarchy
5353

docs/authorization-model.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ This document explains **how** the portal's authorization model works. For the e
44

55
## Policy Naming Convention
66

7-
All policies follow a `{Domain}.{Action}` naming pattern (e.g., `AdminActions.Create`, `GameServers.Credentials.Ftp.Read`). Policy name constants are defined in `AuthPolicies.cs` and registered via `PolicyExtensions.AddXtremeIdiotsPolicies()`. Each policy maps to a marker requirement class and a corresponding authorization handler.
7+
All policies follow a `{Domain}.{Action}` naming pattern (e.g., `AdminActions.Create`, `GameServers.Credentials.FileTransport.Read`). Policy name constants are defined in `AuthPolicies.cs` and registered via `PolicyExtensions.AddXtremeIdiotsPolicies()`. Each policy maps to a marker requirement class and a corresponding authorization handler.
88

99
## Role Hierarchy
1010

@@ -36,7 +36,7 @@ Most policies are scoped to a `GameType`. A HeadAdmin for COD4 can only exercise
3636

3737
### Server Scoped
3838

39-
Some additional permissions are scoped to a specific game server (identified by GUID). For example, a user can be granted `GameServers.Credentials.Ftp.Read` for a single server without access to FTP credentials for other servers in the same game type. Server-scoped checks typically appear as `(GameType, Guid)` resource tuples.
39+
Some additional permissions are scoped to a specific game server (identified by GUID). For example, a user can be granted `GameServers.Credentials.FileTransport.Read` for a single server without access to file transport credentials for other servers in the same game type. Server-scoped checks typically appear as `(GameType, Guid)` resource tuples.
4040

4141
### Ownership-Based
4242

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
using System.Security.Claims;
2+
using Microsoft.AspNetCore.Authorization;
3+
using XtremeIdiots.Portal.Repository.Abstractions.Constants.V1;
4+
using XtremeIdiots.Portal.Web.Auth.Constants;
5+
using XtremeIdiots.Portal.Web.Auth.Handlers;
6+
using XtremeIdiots.Portal.Web.Auth.Requirements;
7+
8+
namespace XtremeIdiots.Portal.Web.Tests.Auth.Handlers;
9+
10+
public class GameServersAuthHandlerTests
11+
{
12+
[Fact]
13+
public async Task HandleAsync_FileTransportRead_DoesNotSucceedForLegacyFtpClaim()
14+
{
15+
var serverId = Guid.NewGuid();
16+
var requirement = new GameServersCredentialsFileTransportRead();
17+
var user = CreateUser(new Claim("GameServers.Credentials.Ftp.Read", serverId.ToString()));
18+
var context = new AuthorizationHandlerContext([requirement], user, (GameType.CallOfDuty4, serverId));
19+
20+
var sut = new GameServersAuthHandler();
21+
await sut.HandleAsync(context);
22+
23+
Assert.False(context.HasSucceeded);
24+
}
25+
26+
[Fact]
27+
public async Task HandleAsync_FileTransportRead_SucceedsForFileTransportClaim()
28+
{
29+
var serverId = Guid.NewGuid();
30+
var requirement = new GameServersCredentialsFileTransportRead();
31+
var user = CreateUser(new Claim(AuthPolicies.GameServers_Credentials_FileTransport_Read, serverId.ToString()));
32+
var context = new AuthorizationHandlerContext([requirement], user, (GameType.CallOfDuty4, serverId));
33+
34+
var sut = new GameServersAuthHandler();
35+
await sut.HandleAsync(context);
36+
37+
Assert.True(context.HasSucceeded);
38+
}
39+
40+
private static ClaimsPrincipal CreateUser(params Claim[] claims)
41+
{
42+
var identity = new ClaimsIdentity(claims, authenticationType: "TestAuthType");
43+
return new ClaimsPrincipal(identity);
44+
}
45+
}

src/XtremeIdiots.Portal.Web.Tests/Controllers/GameServersControllerTests.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,6 @@ public async Task Edit_WhenUserCannotEditFileTransport_PreservesExistingFileTran
286286
var redirect = Assert.IsType<RedirectToActionResult>(result);
287287
Assert.Equal("Index", redirect.ActionName);
288288
Assert.NotNull(capturedUpdate);
289-
Assert.True(capturedUpdate.FtpEnabled);
290289
Assert.True(capturedUpdate.FileTransportEnabled);
291290
Assert.Equal(XtremeIdiots.Portal.Repository.Abstractions.Constants.V1.FileTransportType.Ftp, capturedUpdate.FileTransportType);
292291
}
@@ -354,7 +353,7 @@ public async Task Edit_WhenUserSelectsSftp_PersistsTransportEnabledAndType()
354353
Assert.NotNull(capturedUpdate);
355354
Assert.True(capturedUpdate.FileTransportEnabled);
356355
Assert.Equal(XtremeIdiots.Portal.Repository.Abstractions.Constants.V1.FileTransportType.Sftp, capturedUpdate.FileTransportType);
357-
Assert.False(capturedUpdate.FtpEnabled);
356+
Assert.Null(capturedUpdate.FtpEnabled);
358357
}
359358

360359
private static GameServerDto CreateGameServerDto(bool ftpEnabled, bool fileTransportEnabled, string fileTransportType)

src/XtremeIdiots.Portal.Web.Tests/Extensions/FileTransportCompatibilityExtensionsTests.cs

Lines changed: 0 additions & 107 deletions
This file was deleted.

src/XtremeIdiots.Portal.Web.Tests/Extensions/GameServerDtoExtensionsTests.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public void ToViewModel_MapsAllProperties()
4444
// Arrange
4545
var dto = CreateGameServerDto(agentEnabled: true,
4646
ftpEnabled: true, rconEnabled: true, banFileSyncEnabled: true, serverListEnabled: true,
47-
serverListPosition: 5);
47+
serverListPosition: 5, fileTransportEnabled: true, fileTransportType: "Ftp");
4848

4949
// Act
5050
var viewModel = dto.ToViewModel();
@@ -80,7 +80,7 @@ public void ToViewModel_MapsAgentEnabled(bool agentEnabled)
8080
}
8181

8282
[Fact]
83-
public void ToViewModel_WhenLegacyFtpEnabledWithoutFileTransportType_InfersFtp()
83+
public void ToViewModel_WhenTransportTypeIsMissing_UsesUnknown()
8484
{
8585
// Arrange
8686
var dto = CreateGameServerDto(ftpEnabled: true);
@@ -89,7 +89,7 @@ public void ToViewModel_WhenLegacyFtpEnabledWithoutFileTransportType_InfersFtp()
8989
var viewModel = dto.ToViewModel();
9090

9191
// Assert
92-
Assert.Equal(FileTransportType.Ftp, viewModel.FileTransportType);
92+
Assert.Equal(FileTransportType.Unknown, viewModel.FileTransportType);
9393
}
9494

9595
[Fact]
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
using XtremeIdiots.Portal.Repository.Abstractions.Constants.V1;
2+
using XtremeIdiots.Portal.Repository.Abstractions.Models.V1.GameServers;
3+
using XtremeIdiots.Portal.Web.Extensions;
4+
5+
namespace XtremeIdiots.Portal.Web.Tests.Extensions;
6+
7+
public class GameServerFileTransportExtensionsTests
8+
{
9+
[Fact]
10+
public void SetFileTransportProperties_WhenEnabledAndUnknown_DefaultsToFtp()
11+
{
12+
var dto = new CreateGameServerDto("Server", GameType.CallOfDuty4, "host", 28960);
13+
14+
dto.SetFileTransportProperties(true, FileTransportType.Unknown);
15+
16+
Assert.True(dto.FileTransportEnabled);
17+
Assert.Equal(FileTransportType.Ftp, dto.FileTransportType);
18+
}
19+
20+
[Fact]
21+
public void SetFileTransportProperties_WhenDisabled_UsesUnknown()
22+
{
23+
var dto = new EditGameServerDto(Guid.NewGuid());
24+
25+
dto.SetFileTransportProperties(false, FileTransportType.Sftp);
26+
27+
Assert.False(dto.FileTransportEnabled);
28+
Assert.Equal(FileTransportType.Unknown, dto.FileTransportType);
29+
}
30+
}

src/XtremeIdiots.Portal.Web/Auth/Constants/AuthPolicies.cs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,6 @@ public static class AuthPolicies
2525
// Game Servers — Credentials
2626
public const string GameServers_Credentials_FileTransport_Read = "GameServers.Credentials.FileTransport.Read";
2727
public const string GameServers_Credentials_FileTransport_Write = "GameServers.Credentials.FileTransport.Write";
28-
public const string GameServers_Credentials_Ftp_Read = "GameServers.Credentials.Ftp.Read";
29-
public const string GameServers_Credentials_Ftp_Write = "GameServers.Credentials.Ftp.Write";
3028
public const string GameServers_Credentials_Rcon_Read = "GameServers.Credentials.Rcon.Read";
3129
public const string GameServers_Credentials_Rcon_Write = "GameServers.Credentials.Rcon.Write";
3230

src/XtremeIdiots.Portal.Web/Auth/Handlers/BaseAuthorizationHelper.cs

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,7 @@ public static class ClaimGroups
4040
UserProfileClaimType.HeadAdmin,
4141
UserProfileClaimType.GameAdmin,
4242
AuthPolicies.GameServers_Credentials_FileTransport_Read,
43-
AdditionalPermission.GameServers_Credentials_Rcon_Read,
44-
AdditionalPermission.GameServers_Credentials_Ftp_Read
43+
AdditionalPermission.GameServers_Credentials_Rcon_Read
4544
];
4645

4746
public readonly static string[] GameServerAccessLevels =
@@ -359,29 +358,15 @@ public static void CheckRconCredentialsAccess(AuthorizationHandlerContext contex
359358
context.Succeed(requirement);
360359
}
361360

362-
/// <summary>
363-
/// Checks if the user has FTP credentials access for a specific game server
364-
/// </summary>
365-
/// <param name="context">The authorization context</param>
366-
/// <param name="requirement">The authorization requirement to succeed if access is granted</param>
367-
/// <param name="gameServerId">The game server ID to check permissions for</param>
368-
public static void CheckFtpCredentialsAccess(AuthorizationHandlerContext context, IAuthorizationRequirement requirement, Guid gameServerId)
369-
{
370-
if (context.User.HasClaim(AdditionalPermission.GameServers_Credentials_Ftp_Read, gameServerId.ToString()))
371-
context.Succeed(requirement);
372-
}
373-
374361
/// <summary>
375362
/// Checks if the user has file transport credentials access for a specific game server.
376-
/// Accepts both the new transport-neutral claim and the legacy FTP claim during migration.
377363
/// </summary>
378364
/// <param name="context">The authorization context</param>
379365
/// <param name="requirement">The authorization requirement to succeed if access is granted</param>
380366
/// <param name="gameServerId">The game server ID to check permissions for</param>
381367
public static void CheckFileTransportCredentialsAccess(AuthorizationHandlerContext context, IAuthorizationRequirement requirement, Guid gameServerId)
382368
{
383-
if (context.User.HasClaim(AuthPolicies.GameServers_Credentials_FileTransport_Read, gameServerId.ToString()) ||
384-
context.User.HasClaim(AdditionalPermission.GameServers_Credentials_Ftp_Read, gameServerId.ToString()))
369+
if (context.User.HasClaim(AuthPolicies.GameServers_Credentials_FileTransport_Read, gameServerId.ToString()))
385370
{
386371
context.Succeed(requirement);
387372
}

src/XtremeIdiots.Portal.Web/Auth/Handlers/GameServersAuthHandler.cs

Lines changed: 0 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,6 @@ public Task HandleAsync(AuthorizationHandlerContext context)
3535
case GameServersCredentialsFileTransportWrite:
3636
HandleCredentialsFileTransportWrite(context, requirement);
3737
break;
38-
case GameServersCredentialsFtpRead:
39-
HandleCredentialsFtpRead(context, requirement);
40-
break;
41-
case GameServersCredentialsFtpWrite:
42-
HandleCredentialsFtpWrite(context, requirement);
43-
break;
4438
case GameServersCredentialsRconRead:
4539
HandleCredentialsRconRead(context, requirement);
4640
break;
@@ -146,42 +140,12 @@ private static void HandleCredentialsFileTransportRead(AuthorizationHandlerConte
146140
}
147141

148142
BaseAuthorizationHelper.CheckDirectPermissionGrant(context, requirement, "GameServers.Credentials.FileTransport.Read");
149-
BaseAuthorizationHelper.CheckDirectPermissionGrant(context, requirement, "GameServers.Credentials.Ftp.Read");
150143
}
151144

152145
private static void HandleCredentialsFileTransportWrite(AuthorizationHandlerContext context, IAuthorizationRequirement requirement)
153146
{
154147
BaseAuthorizationHelper.CheckSeniorOrHeadAdminAccessWithResource(context, requirement);
155148
BaseAuthorizationHelper.CheckDirectPermissionGrant(context, requirement, "GameServers.Credentials.FileTransport.Write");
156-
BaseAuthorizationHelper.CheckDirectPermissionGrant(context, requirement, "GameServers.Credentials.Ftp.Write");
157-
}
158-
159-
private static void HandleCredentialsFtpRead(AuthorizationHandlerContext context, IAuthorizationRequirement requirement)
160-
{
161-
BaseAuthorizationHelper.CheckSeniorAdminAccess(context, requirement);
162-
163-
if (context.Resource is Tuple<GameType, Guid> refTuple)
164-
{
165-
BaseAuthorizationHelper.CheckHeadAdminAccess(context, requirement, refTuple.Item1);
166-
if (!context.HasSucceeded)
167-
BaseAuthorizationHelper.CheckFileTransportCredentialsAccess(context, requirement, refTuple.Item2);
168-
}
169-
else if (context.Resource is (GameType gameType, Guid gameServerId))
170-
{
171-
BaseAuthorizationHelper.CheckHeadAdminAccess(context, requirement, gameType);
172-
if (!context.HasSucceeded)
173-
BaseAuthorizationHelper.CheckFileTransportCredentialsAccess(context, requirement, gameServerId);
174-
}
175-
176-
BaseAuthorizationHelper.CheckDirectPermissionGrant(context, requirement, "GameServers.Credentials.Ftp.Read");
177-
BaseAuthorizationHelper.CheckDirectPermissionGrant(context, requirement, "GameServers.Credentials.FileTransport.Read");
178-
}
179-
180-
private static void HandleCredentialsFtpWrite(AuthorizationHandlerContext context, IAuthorizationRequirement requirement)
181-
{
182-
BaseAuthorizationHelper.CheckSeniorOrHeadAdminAccessWithResource(context, requirement);
183-
BaseAuthorizationHelper.CheckDirectPermissionGrant(context, requirement, "GameServers.Credentials.Ftp.Write");
184-
BaseAuthorizationHelper.CheckDirectPermissionGrant(context, requirement, "GameServers.Credentials.FileTransport.Write");
185149
}
186150

187151
private static void HandleCredentialsRconRead(AuthorizationHandlerContext context, IAuthorizationRequirement requirement)

0 commit comments

Comments
 (0)