Refactored validation within GeneralCommands and tackles various null…

…able reference warnings via appropriate suppression or logic improvement
eclipserporg · Nov 13, 2023 · a00a2cd · a00a2cd
1 parent 6ba630a
commit a00a2cd
Show file tree

Hide file tree

Showing 10 changed files with 64 additions and 79 deletions.
diff --git a/DiscordBot.App/Commands/GeneralCommands.cs b/DiscordBot.App/Commands/GeneralCommands.cs
@@ -36,9 +36,9 @@ public async Task HelpVerifyCommand(InteractionContext ctx)
     [SlashCommand("pingserver", "ping server")]
     public async Task PingServerCommand(InteractionContext ctx)
     {
-        if (ctx.Channel != _discordService.CommandsChannel)
+        if (!IsValidInteractionContext(ctx, out string errorOutput))
         {
-            await ctx.CreateResponseAsync("Invalid channel.");
+            await ctx.CreateResponseAsync(errorOutput);
             return;
         }
 
@@ -56,26 +56,14 @@ public async Task PingServerCommand(InteractionContext ctx)
     [SlashCommand("remove-read-only", "remove read only from user")]
     public async Task RemoveReadOnlyCommand(InteractionContext ctx, [Option("target", "target member to remove read only")] DiscordUser targetMember)
     {
-        if (ctx.Channel != _discordService.CommandsChannel)
-        {
-            await ctx.CreateResponseAsync("Invalid channel.");
-            return;
-        }
-        Log.Information("RemoveReadOnlyCommand");
-
-        var senderMember = ctx.Member;
 
-        if (targetMember == null || senderMember == null)
+        if (!IsValidInteractionContext(ctx, out string errorOutput, targetMember))
         {
-            await ctx.CreateResponseAsync("```SUCH MEMBERS DO NOT EXIST```");
+            await ctx.CreateResponseAsync(errorOutput);
             return;
         }
 
-        if (targetMember.Id == senderMember.Id)
-        {
-            await ctx.CreateResponseAsync("```YOU CAN'T DO THIS ACTION ON YOURSELF```");
-            return;
-        }
+        Log.Information("RemoveReadOnlyCommand");
 
         var member = await _discordService.Guild.GetMemberAsync(targetMember.Id);
 
@@ -101,23 +89,13 @@ public async Task RemoveReadOnlyCommand(InteractionContext ctx, [Option("target"
     [SlashCommand("read-only", "put read only on user")]
     public async Task ReadOnlyCommand(InteractionContext ctx, [Option("target", "target member")] DiscordUser targetMember, [Option("reason", "reason for this action")] string reason)
     {
-        if (ctx.Channel != _discordService.CommandsChannel)
-            return;
-        Log.Information("ReadOnlyCommand");
-
-        var senderMember = ctx.Member;
-
-        if (targetMember == null || senderMember == null)
+        if (!IsValidInteractionContext(ctx, out string errorOutput, targetMember))
         {
-            await ctx.CreateResponseAsync("```SUCH MEMBERS DO NOT EXIST```");
+            await ctx.CreateResponseAsync(errorOutput);
             return;
         }
 
-        if (targetMember.Id == senderMember.Id)
-        {
-            await ctx.CreateResponseAsync("```YOU CAN'T DO THIS ACTION ON YOURSELF```");
-            return;
-        }
+        Log.Information("ReadOnlyCommand");
 
         var member = await _discordService.Guild.GetMemberAsync(targetMember.Id);
         if (member.Roles.Any(x => x.Id == _discordService.ReadOnlyRole.Id))
@@ -139,11 +117,12 @@ public async Task RemoveReadOnlyCommand(InteractionContext ctx, [Option("target"
     [SlashCommand("apps", "get active applications")]
     public async Task AppsCommand(InteractionContext ctx)
     {
-        if (ctx.Channel != _discordService.CommandsChannel)
+        if (!IsValidInteractionContext(ctx, out string errorOutput))
         {
-            await ctx.CreateResponseAsync("Invalid channel.");
+            await ctx.CreateResponseAsync(errorOutput);
             return;
         }
+
         Log.Information("AppsCommand");
 
         var waitingIssuers = await _serverDiscordApi.GetApps();
@@ -153,25 +132,28 @@ public async Task AppsCommand(InteractionContext ctx)
     [SlashCommand("ann", "post announcement in server")]
     public async Task AnnCommand(InteractionContext ctx, [Option("announcement", "announcement to post on the server")] string text)
     {
-        Log.Information("AnnCommand");
-        if (ctx.Channel != _discordService.CommandsChannel)
+
+        if (!IsValidInteractionContext(ctx, out string errorOutput))
         {
-            await ctx.CreateResponseAsync("Invalid channel.");
+            await ctx.CreateResponseAsync(errorOutput);
             return;
         }
 
+        Log.Information("AnnCommand");
+
         await _serverDiscordApi.PostAnnouncement(text);
         await ctx.CreateResponseAsync("Announcement sent!");
     }
 
     [SlashCommand("save", "save server state")]
     public async Task SaveCommand(InteractionContext ctx)
     {
-        if (ctx.Channel != _discordService.CommandsChannel)
+        if (!IsValidInteractionContext(ctx, out string errorOutput))
         {
-            await ctx.CreateResponseAsync("Invalid channel.");
+            await ctx.CreateResponseAsync(errorOutput);
             return;
         }
+
         Log.Information("SaveCommand");
 
         if (await _serverDiscordApi.PostSave())
@@ -187,28 +169,16 @@ public async Task SaveCommand(InteractionContext ctx)
     [SlashCommand("kick", "kick user")]
     public async Task KickCommand(InteractionContext ctx, [Option("target", "target member")] DiscordUser targetMember, [Option("reason", "reason for this action")] string reason)
     {
-        if (ctx.Channel != _discordService.CommandsChannel)
-        {
-            await ctx.CreateResponseAsync("Invalid channel.");
-            return;
-        }
-        Log.Information("KickCommand");
-
-        var senderMember = ctx.Member;
 
-        if (targetMember == null || senderMember == null)
+        if (!IsValidInteractionContext(ctx, out string errorOutput, targetMember))
         {
-            await ctx.CreateResponseAsync("```SUCH MEMBERS DO NOT EXIST```");
+            await ctx.CreateResponseAsync(errorOutput);
             return;
         }
 
-        if (targetMember.Id == senderMember.Id)
-        {
-            await ctx.CreateResponseAsync("```YOU CAN'T DO THIS ACTION ON YOURSELF```");
-            return;
-        }
+        Log.Information("KickCommand");
 
-        if(await _serverDiscordApi.PostKick(nameof(_discordService.CommandsChannel), ctx.User.Id, ctx.User.Username, targetMember.Id, targetMember.Username, reason))
+        if (await _serverDiscordApi.PostKick(nameof(_discordService.CommandsChannel), ctx.User.Id, ctx.User.Username, targetMember.Id, targetMember.Username, reason))
         {
             var member = await _discordService.Guild.GetMemberAsync(targetMember.Id);
             await member.RemoveAsync(reason);
@@ -222,34 +192,47 @@ public async Task SaveCommand(InteractionContext ctx)
     [SlashCommand("ban", "ban user")]
     public async Task BanCommand(InteractionContext ctx, [Option("target", "target member")] DiscordUser targetMember, [Option("reason", "reason for this action")] string reason)
     {
-        if (ctx.Channel != _discordService.CommandsChannel)
+        if (!IsValidInteractionContext(ctx, out string errorOutput, targetMember))
         {
-            await ctx.CreateResponseAsync("Invalid channel.");
+            await ctx.CreateResponseAsync(errorOutput);
             return;
         }
+
         Log.Information("BanCommand");
 
+        if(await _serverDiscordApi.PostBan(nameof(_discordService.CommandsChannel), ctx.User.Id, ctx.User.Username, targetMember.Id, targetMember.Username, reason))
+        {
+            await _discordService.Guild.BanMemberAsync(targetMember.Id, 0, reason);
+            await ctx.CreateResponseAsync("Banned user");
+            return;
+        }
+
+        await ctx.CreateResponseAsync("Failed to ban user");
+    }
+
+    private bool IsValidInteractionContext(InteractionContext ctx, out string errorOutput, DiscordUser? targetMember = null)
+    {
+        errorOutput = "";
         var senderMember = ctx.Member;
 
-        if (targetMember == null || senderMember == null)
+        if (ctx.Channel != _discordService.CommandsChannel)
         {
-            await ctx.CreateResponseAsync("```SUCH MEMBERS DO NOT EXIST```");
-            return;
+            errorOutput = "Invalid channel.";
+            return false;
         }
 
-        if (targetMember.Id == senderMember.Id)
+        if (targetMember == null || senderMember == null)
         {
-            await ctx.CreateResponseAsync("```YOU CAN'T DO THIS ACTION ON YOURSELF```");
-            return;
+            errorOutput = "```SUCH MEMBERS DO NOT EXIST```";
+            return false;
         }
 
-        if(await _serverDiscordApi.PostBan(nameof(_discordService.CommandsChannel), ctx.User.Id, ctx.User.Username, targetMember.Id, targetMember.Username, reason))
+        if (targetMember.Id == senderMember.Id)
         {
-            await _discordService.Guild.BanMemberAsync(targetMember.Id, 0, reason);
-            await ctx.CreateResponseAsync("Banned user");
-            return;
+            errorOutput = "```YOU CAN'T DO THIS ACTION ON YOURSELF```";
+            return false;
         }
 
-        await ctx.CreateResponseAsync("Failed to ban user");
+        return true;
     }
 }
diff --git a/DiscordBot.App/Middlewares/BasicAuthenticationHandler.cs b/DiscordBot.App/Middlewares/BasicAuthenticationHandler.cs
@@ -26,7 +26,7 @@ public BasicAuthenticationHandler(
 
     protected override async Task<AuthenticateResult> HandleAuthenticateAsync()
     {
-        string authHeader = Request.Headers["Authorization"];
+        string? authHeader = Request.Headers["Authorization"];
         if (authHeader != null && authHeader.StartsWith("Basic"))
         {
 
@@ -43,9 +43,9 @@ protected override async Task<AuthenticateResult> HandleAuthenticateAsync()
                 var principal = new ClaimsPrincipal(identity);
                 var ticket = new AuthenticationTicket(principal, Scheme.Name);
 
-                return AuthenticateResult.Success(ticket);
+                return await Task.FromResult(AuthenticateResult.Success(ticket));
             }
         }
-        return AuthenticateResult.Fail("Failed to authenticate");
+        return await Task.FromResult(AuthenticateResult.Fail("Failed to authenticate"));
     }
 }
diff --git a/DiscordBot.App/Program.cs b/DiscordBot.App/Program.cs
@@ -28,7 +28,7 @@
 builder.Services.AddSingleton<LoginService>();
 builder.Services.AddSingleton<RunnerService>();
 builder.Services.AddSingleton<GuildJoinService>();
-builder.Services.AddHostedService(provider => provider.GetService<RunnerService>());
+builder.Services.AddHostedService(provider => provider.GetRequiredService<RunnerService>());
 
 builder.Services.AddAuthentication("BasicAuthentication")
         .AddScheme<AuthenticationSchemeOptions, BasicAuthenticationHandler>("BasicAuthentication", null);
@@ -44,15 +44,15 @@
     app.UseSwaggerUI();
 }
 
-await app.Services.GetService<DiscordService>().Start();
+await app.Services.GetRequiredService<DiscordService>().Start();
 
 app.UseHttpsRedirection();
 app.UseAuthentication();
 app.UseAuthorization();
 
 app.Use(async (context, next) =>
 {
-    if (!context.Connection.RemoteIpAddress.Equals(context.Connection.LocalIpAddress))
+    if (context.Connection.RemoteIpAddress == null || !context.Connection.RemoteIpAddress.Equals(context.Connection.LocalIpAddress))
     {
         context.Response.StatusCode = 403;
         return;

diff --git a/DiscordBot.App/Services/DiscordService.cs b/DiscordBot.App/Services/DiscordService.cs
@@ -10,6 +10,7 @@
 
 namespace DiscordBot.Services;
 
+#nullable disable
 public class DiscordService
 {
     public DiscordClient Client { get; private set; }

diff --git a/DiscordBot.App/Settings/DiscordSettings.cs b/DiscordBot.App/Settings/DiscordSettings.cs
@@ -1,5 +1,6 @@
 namespace DiscordBot.Settings;
 
+#nullable disable
 public class Roles
 {
     public ulong Member { get; set; }

diff --git a/DiscordBot.App/Settings/ServerApiSettings.cs b/DiscordBot.App/Settings/ServerApiSettings.cs
@@ -1,5 +1,5 @@
 namespace DiscordBot.Settings;
-
+#nullable disable
 public class ServerApiSettings
 {
     public string Url { get; set; }

diff --git a/DiscordBot.Data/Models/Credentials.cs b/DiscordBot.Data/Models/Credentials.cs
@@ -1,5 +1,5 @@
 namespace DiscordBot.Models;
-
+#nullable disable
 public class Credentials
 {
     public string Username { get; set; }

diff --git a/DiscordBot.Data/Models/DiscordUserDto.cs b/DiscordBot.Data/Models/DiscordUserDto.cs
@@ -1,5 +1,5 @@
 namespace DiscordBot.Models;
-
+#nullable disable
 public class DiscordUserDto
 {
     public string Id { get; set; }

diff --git a/DiscordBot.Data/Models/ResponsePairDto.cs b/DiscordBot.Data/Models/ResponsePairDto.cs
@@ -1,5 +1,5 @@
 namespace DiscordBot.Models;
-
+#nullable disable
 public class ResponsePairDto
 {
     public ResponsePairDto()

diff --git a/DiscordBot.FakeServer/Middlewares/BasicAuthenticationHandler.cs b/DiscordBot.FakeServer/Middlewares/BasicAuthenticationHandler.cs
@@ -26,7 +26,7 @@ public BasicAuthenticationHandler(
 
     protected override async Task<AuthenticateResult> HandleAuthenticateAsync()
     {
-        string authHeader = Request.Headers["Authorization"];
+        string? authHeader = Request.Headers["Authorization"];
         if (authHeader != null && authHeader.StartsWith("Basic", StringComparison.InvariantCultureIgnoreCase))
         {
 
@@ -43,9 +43,9 @@ protected override async Task<AuthenticateResult> HandleAuthenticateAsync()
                 var principal = new ClaimsPrincipal(identity);
                 var ticket = new AuthenticationTicket(principal, Scheme.Name);
 
-                return AuthenticateResult.Success(ticket);
+                return await Task.FromResult(AuthenticateResult.Success(ticket));
             }
         }
-        return AuthenticateResult.Fail("Failed to authenticate");
+        return await Task.FromResult(AuthenticateResult.Fail("Failed to authenticate"));
     }
 }