From 5cb1d70a3b55e1b39d2697d8b19294f92d59d406 Mon Sep 17 00:00:00 2001 From: Moony Date: Fri, 5 May 2023 08:56:54 -0500 Subject: [PATCH] ID Console can no longer grant access the privileged ID doesn't have. (read: AA nerf) (#14699) Co-authored-by: moonheart08 Co-authored-by: metalgearsloth --- .../Access/UI/IdCardConsoleWindow.xaml.cs | 5 ++- .../Access/Systems/IdCardConsoleSystem.cs | 39 +++++++++++++------ .../Components/IdCardConsoleComponent.cs | 3 ++ .../Systems/SharedIdCardConsoleSystem.cs | 3 ++ .../Machines/Computers/computers.yml | 2 - .../Roles/Jobs/Command/head_of_personnel.yml | 13 +++++++ 6 files changed, 50 insertions(+), 15 deletions(-) diff --git a/Content.Client/Access/UI/IdCardConsoleWindow.xaml.cs b/Content.Client/Access/UI/IdCardConsoleWindow.xaml.cs index 9bc410db0f..ffd949a643 100644 --- a/Content.Client/Access/UI/IdCardConsoleWindow.xaml.cs +++ b/Content.Client/Access/UI/IdCardConsoleWindow.xaml.cs @@ -116,7 +116,7 @@ namespace Content.Client.Access.UI // this is a sussy way to do this foreach (var access in job.Access) { - if (_accessButtons.TryGetValue(access, out var button)) + if (_accessButtons.TryGetValue(access, out var button) && !button.Disabled) { button.Pressed = true; } @@ -131,7 +131,7 @@ namespace Content.Client.Access.UI foreach (var access in groupPrototype.Tags) { - if (_accessButtons.TryGetValue(access, out var button)) + if (_accessButtons.TryGetValue(access, out var button) && !button.Disabled) { button.Pressed = true; } @@ -187,6 +187,7 @@ namespace Content.Client.Access.UI if (interfaceEnabled) { button.Pressed = state.TargetIdAccessList?.Contains(accessName) ?? false; + button.Disabled = (!state.AllowedModifyAccessList?.Contains(accessName)) ?? true; } } diff --git a/Content.Server/Access/Systems/IdCardConsoleSystem.cs b/Content.Server/Access/Systems/IdCardConsoleSystem.cs index 4318987233..e33bb946bf 100644 --- a/Content.Server/Access/Systems/IdCardConsoleSystem.cs +++ b/Content.Server/Access/Systems/IdCardConsoleSystem.cs @@ -54,16 +54,18 @@ public sealed class IdCardConsoleSystem : SharedIdCardConsoleSystem if (!component.Initialized) return; + var privilegedIdName = string.Empty; + string[]? possibleAccess = null; + if (component.PrivilegedIdSlot.Item is { Valid: true } item) + { + privilegedIdName = EntityManager.GetComponent(item).EntityName; + possibleAccess = _accessReader.FindAccessTags(item).ToArray(); + } + IdCardConsoleBoundUserInterfaceState newState; // this could be prettier if (component.TargetIdSlot.Item is not { Valid: true } targetId) { - var privilegedIdName = string.Empty; - if (component.PrivilegedIdSlot.Item is { Valid: true } item) - { - privilegedIdName = EntityManager.GetComponent(item).EntityName; - } - newState = new IdCardConsoleBoundUserInterfaceState( component.PrivilegedIdSlot.HasItem, PrivilegedIdIsAuthorized(uid, component), @@ -71,6 +73,7 @@ public sealed class IdCardConsoleSystem : SharedIdCardConsoleSystem null, null, null, + possibleAccess, string.Empty, privilegedIdName, string.Empty); @@ -79,9 +82,6 @@ public sealed class IdCardConsoleSystem : SharedIdCardConsoleSystem { var targetIdComponent = EntityManager.GetComponent(targetId); var targetAccessComponent = EntityManager.GetComponent(targetId); - var name = string.Empty; - if (component.PrivilegedIdSlot.Item is { Valid: true } item) - name = EntityManager.GetComponent(item).EntityName; var jobProto = string.Empty; if (_station.GetOwningStation(uid) is { } station @@ -99,8 +99,9 @@ public sealed class IdCardConsoleSystem : SharedIdCardConsoleSystem targetIdComponent.FullName, targetIdComponent.JobTitle, targetAccessComponent.Tags.ToArray(), + possibleAccess, jobProto, - name, + privilegedIdName, EntityManager.GetComponent(targetId).EntityName); } @@ -130,16 +131,29 @@ public sealed class IdCardConsoleSystem : SharedIdCardConsoleSystem if (!newAccessList.TrueForAll(x => component.AccessLevels.Contains(x))) { - Logger.Warning("Tried to write unknown access tag."); + _sawmill.Warning($"User {ToPrettyString(uid)} tried to write unknown access tag."); return; } var oldTags = _access.TryGetTags(targetId) ?? new List(); oldTags = oldTags.ToList(); + var privilegedId = component.PrivilegedIdSlot.Item; + if (oldTags.SequenceEqual(newAccessList)) return; + // I hate that C# doesn't have an option for this and don't desire to write this out the hard way. + // var difference = newAccessList.Difference(oldTags); + var difference = (newAccessList.Union(oldTags)).Except(newAccessList.Intersect(oldTags)).ToHashSet(); + // NULL SAFETY: PrivilegedIdIsAuthorized checked this earlier. + var privilegedPerms = _accessReader.FindAccessTags(privilegedId!.Value).ToHashSet(); + if (!difference.IsSubsetOf(privilegedPerms)) + { + _sawmill.Warning($"User {ToPrettyString(uid)} tried to modify permissions they could not give/take!"); + return; + } + var addedTags = newAccessList.Except(oldTags).Select(tag => "+" + tag).ToList(); var removedTags = oldTags.Except(newAccessList).Select(tag => "-" + tag).ToList(); _access.TrySetTags(targetId, newAccessList); @@ -155,6 +169,9 @@ public sealed class IdCardConsoleSystem : SharedIdCardConsoleSystem /// /// Returns true if there is an ID in and said ID satisfies the requirements of . /// + /// + /// Other code relies on the fact this returns false if privileged Id is null. Don't break that invariant. + /// private bool PrivilegedIdIsAuthorized(EntityUid uid, IdCardConsoleComponent? component = null) { if (!Resolve(uid, ref component)) diff --git a/Content.Shared/Access/Components/IdCardConsoleComponent.cs b/Content.Shared/Access/Components/IdCardConsoleComponent.cs index 87c5f48bf9..53001867c5 100644 --- a/Content.Shared/Access/Components/IdCardConsoleComponent.cs +++ b/Content.Shared/Access/Components/IdCardConsoleComponent.cs @@ -85,6 +85,7 @@ public sealed class IdCardConsoleComponent : Component public readonly string? TargetIdFullName; public readonly string? TargetIdJobTitle; public readonly string[]? TargetIdAccessList; + public readonly string[]? AllowedModifyAccessList; public readonly string TargetIdJobPrototype; public IdCardConsoleBoundUserInterfaceState(bool isPrivilegedIdPresent, @@ -93,6 +94,7 @@ public sealed class IdCardConsoleComponent : Component string? targetIdFullName, string? targetIdJobTitle, string[]? targetIdAccessList, + string[]? allowedModifyAccessList, string targetIdJobPrototype, string privilegedIdName, string targetIdName) @@ -103,6 +105,7 @@ public sealed class IdCardConsoleComponent : Component TargetIdFullName = targetIdFullName; TargetIdJobTitle = targetIdJobTitle; TargetIdAccessList = targetIdAccessList; + AllowedModifyAccessList = allowedModifyAccessList; TargetIdJobPrototype = targetIdJobPrototype; PrivilegedIdName = privilegedIdName; TargetIdName = targetIdName; diff --git a/Content.Shared/Access/Systems/SharedIdCardConsoleSystem.cs b/Content.Shared/Access/Systems/SharedIdCardConsoleSystem.cs index 8661b47ccf..9abb91de47 100644 --- a/Content.Shared/Access/Systems/SharedIdCardConsoleSystem.cs +++ b/Content.Shared/Access/Systems/SharedIdCardConsoleSystem.cs @@ -10,12 +10,15 @@ namespace Content.Shared.Access.Systems public abstract class SharedIdCardConsoleSystem : EntitySystem { [Dependency] private readonly ItemSlotsSystem _itemSlotsSystem = default!; + [Dependency] private readonly ILogManager _log = default!; public const string Sawmill = "idconsole"; + protected ISawmill _sawmill = default!; public override void Initialize() { base.Initialize(); + _sawmill = _log.GetSawmill(Sawmill); SubscribeLocalEvent(OnComponentInit); SubscribeLocalEvent(OnComponentRemove); diff --git a/Resources/Prototypes/Entities/Structures/Machines/Computers/computers.yml b/Resources/Prototypes/Entities/Structures/Machines/Computers/computers.yml index 0fd8d2c811..33eced8fca 100644 --- a/Resources/Prototypes/Entities/Structures/Machines/Computers/computers.yml +++ b/Resources/Prototypes/Entities/Structures/Machines/Computers/computers.yml @@ -409,8 +409,6 @@ name: ID card computer description: Terminal for programming Nanotrasen employee ID cards to access parts of the station. components: - - type: AccessReader - access: [["HeadOfPersonnel"]] - type: IdCardConsole privilegedIdSlot: name: id-card-console-privileged-id diff --git a/Resources/Prototypes/Roles/Jobs/Command/head_of_personnel.yml b/Resources/Prototypes/Roles/Jobs/Command/head_of_personnel.yml index a8ba120570..b05f20c463 100644 --- a/Resources/Prototypes/Roles/Jobs/Command/head_of_personnel.yml +++ b/Resources/Prototypes/Roles/Jobs/Command/head_of_personnel.yml @@ -34,6 +34,19 @@ - Hydroponics - External # I mean they'll give themselves the rest of the access levels *anyways*. + # As of 15/03/23 they can't do that so here's MOST of the rest of the access levels. + # Head level access that isn't their own was deliberately left out, get AA from the captain instead. + - Chemistry + - Engineering + - Quartermaster + - Research + - Salvage + - Security + - Brig + - Cargo + - Atmospherics + - Cargo + - Medical - type: startingGear id: HoPGear