Remove CVar.SECURE and replace auth CVars with env variables.

Robust.Client/ClientIoC.cs

@@ -82,6 +82,7 @@ namespace Robust.Client
             IoCManager.Register<IFontManager, FontManager>();
             IoCManager.Register<IFontManagerInternal, FontManager>();
             IoCManager.Register<IMidiManager, MidiManager>();
+            IoCManager.Register<IAuthManager, AuthManager>();
             switch (mode)
             {
                 case GameController.DisplayMode.Headless:

Robust.Client/Console/Commands/LauncherAuthCommand.cs

@@ -1,13 +1,13 @@
 #if !FULL_RELEASE
+using System;
 using System.IO;
 using System.Linq;
 using System.Text.Json;
 using System.Text.Json.Serialization;
 using Robust.Client.Utility;
-using Robust.Shared;
 using Robust.Shared.Console;
-using Robust.Shared.Interfaces.Configuration;
 using Robust.Shared.IoC;
+using Robust.Shared.Network;
 
 namespace Robust.Client.Console.Commands
 {
@@ -39,9 +39,9 @@ namespace Robust.Client.Console.Commands
             var token = login.Token.Token;
             var userId = login.UserId;
 
-            var cfg = IoCManager.Resolve<IConfigurationManagerInternal>();
-            cfg.SetSecureCVar(CVars.AuthUserId, userId);
-            cfg.SetSecureCVar(CVars.AuthToken, token);
+            var cfg = IoCManager.Resolve<IAuthManager>();
+            cfg.Token = token;
+            cfg.UserId = new NetUserId(Guid.Parse(userId));
         }
 
         private sealed class LauncherConfig

Robust.Client/GameController.cs

@@ -31,6 +31,7 @@ using Robust.Shared.Interfaces.Serialization;
 using Robust.Shared.Interfaces.Timers;
 using Robust.Shared.IoC;
 using Robust.Shared.Log;
+using Robust.Shared.Network;
 using Robust.Shared.Prototypes;
 using Robust.Shared.Serialization;
 using Robust.Shared.Timing;
@@ -67,6 +68,7 @@ namespace Robust.Client
         [Dependency] private readonly IComponentManager _componentManager = default!;
         [Dependency] private readonly IPlayerManager _playerManager = default!;
         [Dependency] private readonly IRobustMappedStringSerializer _stringSerializer = default!;
+        [Dependency] private readonly IAuthManager _authManager = default!;
 
         private CommandLineArgs? _commandLineArgs;
         private bool _disableAssemblyLoadContext;
@@ -186,6 +188,8 @@ namespace Robust.Client
                 _client.PlayerNameOverride = _commandLineArgs.Username;
             }
 
+            _authManager.LoadFromEnv();
+
             _clyde.Ready();
 
             if ((_commandLineArgs?.Connect == true || _commandLineArgs?.Launcher == true)

Robust.Server/ServerIoC.cs

@@ -73,6 +73,7 @@ namespace Robust.Server
             IoCManager.Register<IWatchdogApi, WatchdogApi>();
             IoCManager.Register<IScriptHost, ScriptHost>();
             IoCManager.Register<IMetricsManager, MetricsManager>();
+            IoCManager.Register<IAuthManager, AuthManager>();
         }
     }
 }

Robust.Shared/CVars.cs

@@ -2,6 +2,7 @@ using System;
 using System.Runtime.InteropServices;
 using Robust.Shared.Configuration;
 using Robust.Shared.Log;
+using Robust.Shared.Network;
 
 namespace Robust.Shared
 {
@@ -218,17 +219,9 @@ namespace Robust.Shared
         public static readonly CVarDef<bool> AuthAllowLocal =
             CVarDef.Create("auth.allowlocal", true, CVar.SERVERONLY);
 
-        public static readonly CVarDef<string> AuthServerPubKey =
-            CVarDef.Create("auth.serverpubkey", "", CVar.SECURE | CVar.CLIENTONLY);
-
-        public static readonly CVarDef<string> AuthToken =
-            CVarDef.Create("auth.token", "", CVar.SECURE | CVar.CLIENTONLY);
-
-        public static readonly CVarDef<string> AuthUserId =
-            CVarDef.Create("auth.userid", "", CVar.SECURE | CVar.CLIENTONLY);
-
+        // Only respected on server, client goes through IAuthManager for security.
         public static readonly CVarDef<string> AuthServer =
-            CVarDef.Create("auth.server", "https://central.spacestation14.io/auth/", CVar.SECURE);
+            CVarDef.Create("auth.server", AuthManager.DefaultAuthServer, CVar.SERVERONLY);
 
         /*
          * DISPLAY

Robust.Shared/Configuration/CVar.cs

@@ -57,11 +57,6 @@ namespace Robust.Shared.Configuration
         /// <remarks>
         ///     This is intended to aid shared code.
         /// </remarks>
-        CLIENTONLY = 128,
-
-        /// <summary>
-        /// This var has to kept secure and may not be accessed by content.
-        /// </summary>
-        SECURE = 256
+        CLIENTONLY = 128
     }
 }

Robust.Shared/Configuration/ConfigurationManager.cs

@@ -82,14 +82,6 @@ namespace Robust.Shared.Configuration
                 var tomlValue = TypeConvert(obj);
                 if (_configVars.TryGetValue(tablePath, out var cfgVar))
                 {
-                    if ((cfgVar.Flags & CVar.SECURE) != 0)
-                    {
-                        // DO NOT read SECURE CVars.
-                        // client config is in a location content can access via the user data API.
-                        // Basically all secure CVars are something
-                        // the launcher should be passing in via env vars anyways.
-                        return;
-                    }
                     // overwrite the value with the saved one
                     cfgVar.Value = tomlValue;
                     cfgVar.ValueChanged?.Invoke(cfgVar.Value);
@@ -298,15 +290,13 @@ namespace Robust.Shared.Configuration
         /// <inheritdoc />
         public bool IsCVarRegistered(string name)
         {
-            return _configVars.TryGetValue(name, out var cVar) && cVar.Registered && (cVar.Flags & CVar.SECURE) == 0;
+            return _configVars.TryGetValue(name, out var cVar) && cVar.Registered;
         }
 
         /// <inheritdoc />
         public IEnumerable<string> GetRegisteredCVars()
         {
-            return _configVars
-                .Where(p => (p.Value.Flags & CVar.SECURE) == 0)
-                .Select(p => p.Key);
+            return _configVars.Select(p => p.Key);
         }
 
         /// <inheritdoc />
@@ -315,10 +305,10 @@ namespace Robust.Shared.Configuration
             SetCVarInternal(name, value);
         }
 
-        private void SetCVarInternal(string name, object value, bool allowSecure = false)
+        private void SetCVarInternal(string name, object value)
         {
             //TODO: Make flags work, required non-derpy net system.
-            if (_configVars.TryGetValue(name, out var cVar) && cVar.Registered && (allowSecure || (cVar.Flags & CVar.SECURE) == 0))
+            if (_configVars.TryGetValue(name, out var cVar) && cVar.Registered)
             {
                 if (!Equals(cVar.OverrideValueParsed ?? cVar.Value, value))
                 {
@@ -341,15 +331,6 @@ namespace Robust.Shared.Configuration
 
         /// <inheritdoc />
         public T GetCVar<T>(string name)
-        {
-            if (_configVars.TryGetValue(name, out var cVar) && cVar.Registered && (cVar.Flags & CVar.SECURE) == 0)
-                //TODO: Make flags work, required non-derpy net system.
-                return (T)(cVar.OverrideValueParsed ?? cVar.Value ?? cVar.DefaultValue)!;
-
-            throw new InvalidConfigurationException($"Trying to get unregistered variable '{name}'");
-        }
-
-        public T GetSecureCVar<T>(string name)
         {
             if (_configVars.TryGetValue(name, out var cVar) && cVar.Registered)
                 //TODO: Make flags work, required non-derpy net system.
@@ -358,18 +339,6 @@ namespace Robust.Shared.Configuration
             throw new InvalidConfigurationException($"Trying to get unregistered variable '{name}'");
         }
 
-        public void SetSecureCVar(string name, object value)
-        {
-            SetCVarInternal(name, value, allowSecure: true);
-        }
-
-
-        public void SetSecureCVar<T>(CVarDef<T> def, T value) where T : notnull
-        {
-            SetSecureCVar(def.Name, value);
-        }
-
-
         public T GetCVar<T>(CVarDef<T> def) where T : notnull
         {
             return GetCVar<T>(def.Name);
@@ -377,7 +346,7 @@ namespace Robust.Shared.Configuration
 
         public Type GetCVarType(string name)
         {
-            if (!_configVars.TryGetValue(name, out var cVar) || !cVar.Registered || (cVar.Flags & CVar.SECURE) != 0)
+            if (!_configVars.TryGetValue(name, out var cVar) || !cVar.Registered)
             {
                 throw new InvalidConfigurationException($"Trying to get type of unregistered variable '{name}'");
             }

Robust.Shared/Interfaces/Configuration/IConfigurationManagerInternal.cs

@@ -9,11 +9,6 @@ namespace Robust.Shared.Interfaces.Configuration
         void OverrideConVars(IEnumerable<(string key, string value)> cVars);
         void LoadCVarsFromAssembly(Assembly assembly);
 
-        T GetSecureCVar<T>(string name);
-
-        void SetSecureCVar(string name, object value);
-        void SetSecureCVar<T>(CVarDef<T> def, T value) where T : notnull;
-
         void Initialize(bool isServer);
 
         /// <summary>

Robust.Shared/Network/AuthManager.cs

@@ -0,0 +1,59 @@
+using System;
+using System.Diagnostics.CodeAnalysis;
+
+namespace Robust.Shared.Network
+{
+    // Basically turbo-lightweight IConfigurationManager for the purposes of auth var loading from env.
+
+    /// <summary>
+    ///     Stores client authentication parameters.
+    /// </summary>
+    internal interface IAuthManager
+    {
+        NetUserId? UserId { get; set; }
+        string? Server { get; set; }
+        string? Token { get; set; }
+        string? PubKey { get; set; }
+
+        void LoadFromEnv();
+    }
+
+    internal sealed class AuthManager : IAuthManager
+    {
+        public const string DefaultAuthServer = "https://central.spacestation14.io/auth/";
+
+        public NetUserId? UserId { get; set; }
+        public string? Server { get; set; } = DefaultAuthServer;
+        public string? Token { get; set; }
+        public string? PubKey { get; set; }
+
+        public void LoadFromEnv()
+        {
+            if (TryGetVar("ROBUST_AUTH_SERVER", out var server))
+            {
+                Server = server;
+            }
+
+            if (TryGetVar("ROBUST_AUTH_USERID", out var userId))
+            {
+                UserId = new NetUserId(Guid.Parse(userId));
+            }
+
+            if (TryGetVar("ROBUST_AUTH_PUBKEY", out var pubKey))
+            {
+                PubKey = pubKey;
+            }
+
+            if (TryGetVar("ROBUST_AUTH_TOKEN", out var token))
+            {
+                Token = token;
+            }
+
+            static bool TryGetVar(string var, [NotNullWhen(true)] out string? val)
+            {
+                val = Environment.GetEnvironmentVariable(var);
+                return val != null;
+            }
+        }
+    }
+}

Robust.Shared/Network/NetManager.ClientConnect.cs

@@ -122,10 +122,10 @@ namespace Robust.Shared.Network
         private async Task CCDoHandshake(NetPeerData peer, NetConnection connection, string userNameRequest,
             CancellationToken cancel)
         {
-            var authToken = _config.GetSecureCVar<string>("auth.token");
-            var pubKey = _config.GetSecureCVar<string>("auth.serverpubkey");
-            var authServer = _config.GetSecureCVar<string>("auth.server");
-            var userIdStr = _config.GetSecureCVar<string>("auth.userid");
+            var authToken = _authManager.Token;
+            var pubKey = _authManager.PubKey;
+            var authServer = _authManager.Server;
+            var userId = _authManager.UserId;
 
             var hasPubKey = !string.IsNullOrEmpty(pubKey);
             var authenticate = !string.IsNullOrEmpty(authToken);
@@ -160,7 +160,7 @@ namespace Robust.Shared.Network
                 if (hasPubKey)
                 {
                     // public key provided by launcher.
-                    keyBytes = Convert.FromBase64String(pubKey);
+                    keyBytes = Convert.FromBase64String(pubKey!);
                 }
                 else
                 {
@@ -190,7 +190,7 @@ namespace Robust.Shared.Network
                 {
                     SharedSecret = encryptedSecret,
                     VerifyToken = encryptedVerifyToken,
-                    UserId = new Guid(userIdStr)
+                    UserId = userId!.Value.UserId
                 };
 
                 var outEncRespMsg = peer.Peer.CreateMessage();

Robust.Shared/Network/NetManager.ServerAuth.cs

@@ -51,8 +51,7 @@ namespace Robust.Shared.Network
                 var isLocal = IPAddress.IsLoopback(ip) && _config.GetCVar(CVars.AuthAllowLocal);
                 var canAuth = msgLogin.CanAuth;
                 var needPk = msgLogin.NeedPubKey;
-                var authServer = _config.GetSecureCVar<string>("auth.server");
-
+                var authServer = _config.GetCVar(CVars.AuthServer);
 
                 if (Auth == AuthMode.Required && !isLocal)
                 {

Robust.Shared/Network/NetManager.cs

@@ -111,6 +111,7 @@ namespace Robust.Shared.Network
         private readonly Dictionary<Type, long> _bandwidthUsage = new();
 
         [Dependency] private readonly IConfigurationManagerInternal _config = default!;
+        [Dependency] private readonly IAuthManager _authManager = default!;
 
         /// <summary>
         ///     Holds lookup table for NetMessage.Id -> NetMessage.Type

Robust.UnitTesting/Shared/Configuration/ConfigurationManagerTest.cs

@@ -1,23 +0,0 @@
-using NUnit.Framework;
-using Robust.Shared.Configuration;
-
-namespace Robust.UnitTesting.Shared.Configuration
-{
-    internal sealed class ConfigurationManagerTest
-    {
-        [Test]
-        public void TestSecureCVar()
-        {
-            var cfg = new ConfigurationManager();
-
-            cfg.RegisterCVar("auth.token", "honk", CVar.SECURE);
-
-            Assert.That(() => cfg.GetCVar<string>("auth.token"), Throws.TypeOf<InvalidConfigurationException>());
-            Assert.That(() => cfg.GetCVarType("auth.token"), Throws.TypeOf<InvalidConfigurationException>());
-            Assert.That(() => cfg.SetCVar("auth.token", "foo"), Throws.TypeOf<InvalidConfigurationException>());
-            Assert.That(cfg.GetSecureCVar<string>("auth.token"), Is.EqualTo("honk"));
-            Assert.That(cfg.IsCVarRegistered("auth.token"), Is.False);
-            Assert.That(cfg.GetRegisteredCVars(), Does.Not.Contain("auth.token"));
-        }
-    }
-}