diff --git a/Content.Server/Access/Systems/IdCardConsoleSystem.cs b/Content.Server/Access/Systems/IdCardConsoleSystem.cs index d5f1947d87..7c772830ba 100644 --- a/Content.Server/Access/Systems/IdCardConsoleSystem.cs +++ b/Content.Server/Access/Systems/IdCardConsoleSystem.cs @@ -1,3 +1,4 @@ +using System.Diagnostics.CodeAnalysis; using System.Linq; using Content.Server.Chat.Systems; using Content.Server.Containers; @@ -84,7 +85,7 @@ public sealed class IdCardConsoleSystem : SharedIdCardConsoleSystem { newState = new IdCardConsoleBoundUserInterfaceState( component.PrivilegedIdSlot.HasItem, - PrivilegedIdIsAuthorized(uid, component), + PrivilegedIdIsAuthorized(uid, component, out _), false, null, null, @@ -109,7 +110,7 @@ public sealed class IdCardConsoleSystem : SharedIdCardConsoleSystem newState = new IdCardConsoleBoundUserInterfaceState( component.PrivilegedIdSlot.HasItem, - PrivilegedIdIsAuthorized(uid, component), + PrivilegedIdIsAuthorized(uid, component, out _), true, targetIdComponent.FullName, targetIdComponent.LocalizedJobTitle, @@ -138,13 +139,13 @@ public sealed class IdCardConsoleSystem : SharedIdCardConsoleSystem if (!Resolve(uid, ref component)) return; - if (component.TargetIdSlot.Item is not { Valid: true } targetId || !PrivilegedIdIsAuthorized(uid, component)) + if (component.TargetIdSlot.Item is not { Valid: true } targetId || !PrivilegedIdIsAuthorized(uid, component, out var privilegedId)) return; _idCard.TryChangeFullName(targetId, newFullName, player: player); _idCard.TryChangeJobTitle(targetId, newJobTitle, player: player); - if (_prototype.TryIndex(newJobProto, out var job) + if (_prototype.Resolve(newJobProto, out var job) && _prototype.Resolve(job.Icon, out var jobIcon)) { _idCard.TryChangeJobIcon(targetId, jobIcon, player: player); @@ -166,10 +167,7 @@ public sealed class IdCardConsoleSystem : SharedIdCardConsoleSystem return; } - var oldTags = _access.TryGetTags(targetId) ?? new List>(); - oldTags = oldTags.ToList(); - - var privilegedId = component.PrivilegedIdSlot.Item; + var oldTags = _access.TryGetTags(targetId)?.ToList() ?? new List>(); if (oldTags.SequenceEqual(newAccessList)) return; @@ -177,8 +175,7 @@ public sealed class IdCardConsoleSystem : SharedIdCardConsoleSystem // 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(); + var privilegedPerms = _accessReader.FindAccessTags(privilegedId.Value); if (!difference.IsSubsetOf(privilegedPerms)) { _sawmill.Warning($"User {ToPrettyString(uid)} tried to modify permissions they could not give/take!"); @@ -191,26 +188,24 @@ public sealed class IdCardConsoleSystem : SharedIdCardConsoleSystem /*TODO: ECS SharedIdCardConsoleComponent and then log on card ejection, together with the save. This current implementation is pretty shit as it logs 27 entries (27 lines) if someone decides to give themselves AA*/ - _adminLogger.Add(LogType.Action, LogImpact.Medium, - $"{ToPrettyString(player):player} has modified {ToPrettyString(targetId):entity} with the following accesses: [{string.Join(", ", addedTags.Union(removedTags))}] [{string.Join(", ", newAccessList)}]"); + _adminLogger.Add(LogType.Action, + $"{player} has modified {targetId} with the following accesses: [{string.Join(", ", addedTags.Union(removedTags))}] [{string.Join(", ", newAccessList)}]"); } /// /// 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) + private bool PrivilegedIdIsAuthorized(EntityUid uid, IdCardConsoleComponent component, [NotNullWhen(true)] out EntityUid? id) { - if (!Resolve(uid, ref component)) - return true; + id = null; + if (component.PrivilegedIdSlot.Item == null) + return false; + id = component.PrivilegedIdSlot.Item; if (!TryComp(uid, out var reader)) return true; - var privilegedId = component.PrivilegedIdSlot.Item; - return privilegedId != null && _accessReader.IsAllowed(privilegedId.Value, uid, reader); + return _accessReader.IsAllowed(id.Value, uid, reader); } private void UpdateStationRecord(EntityUid uid, EntityUid targetId, string newFullName, ProtoId newJobTitle, JobPrototype? newJobProto)