Skip to content

add support for env credentials or loading creds from json file #127

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: 2.X
Choose a base branch
from
Open

add support for env credentials or loading creds from json file #127

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
07183cf
add support for env credentials or loading creds from env
tmolbergen Dec 15, 2024
168ed1b
add some docs
tmolbergen Dec 15, 2024
2b18dea
Merge branch 'SpecterOps:2.X' into 2.X
tmolbergen Jan 16, 2025
eb52074
Merge remote-tracking branch 'specterops/2.X' into 2.X
tmolbergen Nov 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,12 @@ The listing below details the CLI arguments SharpHound supports. Additional deta

--ldappassword Password for LDAP

--ldapenvusername Username for LDAP stored in a defined environment variable (argument takes the environment variable as input)

--ldapenvpassword Password for LDAP stored in a defined environment variable (argument takes the environment variable as input)

--ldapcredentialsjsonpath Path argument for a json file containing {"Username": "xxxx", "Password": "xxxx"} to be used instead of --ldapusername and --ldappassword

--domaincontroller Override domain controller to pull LDAP from. This option can result in data loss

--ldapport (Default: 0) Override port for LDAP
Expand Down
6 changes: 6 additions & 0 deletions src/JsonExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@

namespace Sharphound
{

public class Credentials
{
public string Username { get; set; }
public string Password { get; set; }
}
public class CacheContractResolver : DefaultContractResolver
{
private static readonly CacheContractResolver Instance = new();
Expand Down
9 changes: 9 additions & 0 deletions src/Options.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,15 @@ public class Options
[Option(HelpText = "Password for LDAP", Default = null)]
public string LDAPPassword { get; set; }

[Option(HelpText = "Path to Json file containing username/password", Default = null)]
public string LDAPCredentialJsonPath { get; set; }

[Option(HelpText = "Enviroment variable name for LDAP Username", Default = null)]
public string LDAPEnvUsername { get; set; }

[Option(HelpText = "Enviroment variable name for LDAP Password", Default = null)]
public string LDAPEnvPassword { get; set; }
Comment on lines +84 to +88
Copy link

@coderabbitai coderabbitai bot Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix typo: "Enviroment" → "Environment".

The help text contains a spelling error that should be corrected for consistency and professionalism.

Apply this diff:

-        [Option(HelpText = "Enviroment variable name for LDAP Username", Default = null)]
+        [Option(HelpText = "Environment variable name for LDAP Username", Default = null)]
         public string LDAPEnvUsername { get; set; }

-        [Option(HelpText = "Enviroment variable name for LDAP Password", Default = null)]
+        [Option(HelpText = "Environment variable name for LDAP Password", Default = null)]
         public string LDAPEnvPassword { get; set; }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
[Option(HelpText = "Enviroment variable name for LDAP Username", Default = null)]
public string LDAPEnvUsername { get; set; }
[Option(HelpText = "Enviroment variable name for LDAP Password", Default = null)]
public string LDAPEnvPassword { get; set; }
[Option(HelpText = "Environment variable name for LDAP Username", Default = null)]
public string LDAPEnvUsername { get; set; }
[Option(HelpText = "Environment variable name for LDAP Password", Default = null)]
public string LDAPEnvPassword { get; set; }
🤖 Prompt for AI Agents 
In src/Options.cs around lines 84 to 88, the HelpText strings for
LDAPEnvUsername and LDAPEnvPassword contain a typo "Enviroment"; update both
HelpText values to read "Environment variable name for LDAP Username" and
"Environment variable name for LDAP Password" respectively to correct the
spelling.


[Option(HelpText = "Do the session enumeration with local admin credentials instead of domain credentials", Default = false)]
public bool DoLocalAdminSessionEnum { get; set; }

Expand Down
26 changes: 22 additions & 4 deletions src/Sharphound.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,28 @@ await options.WithParsedAsync(async options =>

if (options.DomainController != null) ldapOptions.Server = options.DomainController;

if (options.LDAPUsername != null)
{
if (options.LDAPPassword == null)
{
if (options.LDAPCredentialJsonPath != null) {
string json = File.ReadAllText(options.LDAPCredentialJsonPath);
Credentials ldapCreds = JsonConvert.DeserializeObject<Credentials>(json);
ldapOptions.Username = ldapCreds.Username;
ldapOptions.Password = ldapCreds.Password;
}
Comment on lines +103 to +108
Copy link

@coderabbitai coderabbitai bot Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Add missing using statements and error handling for JSON credential loading.

The code references File.ReadAllText and JsonConvert.DeserializeObject but the required using statements (System.IO and Newtonsoft.Json) are not present in the file. Additionally, there is no error handling for file operations or JSON deserialization failures.

Apply this diff to add the required using statements at the top of the file:

 using System;
 using System.DirectoryServices.Protocols;
+using System.IO;
 using System.Threading;
 using System.Threading.Tasks;
 using CommandLine;
 using Microsoft.Extensions.Logging;
 using Microsoft.Win32;
+using Newtonsoft.Json;
 using Sharphound.Client;
 using SharpHoundCommonLib;
 using SharpHoundCommonLib.Enums;

Then apply this diff to add error handling and validation:

                     if (options.LDAPCredentialJsonPath != null) {
-                        string json = File.ReadAllText(options.LDAPCredentialJsonPath);
-                        Credentials ldapCreds = JsonConvert.DeserializeObject<Credentials>(json);
-                        ldapOptions.Username = ldapCreds.Username;
-                        ldapOptions.Password = ldapCreds.Password;
+                        try {
+                            string json = File.ReadAllText(options.LDAPCredentialJsonPath);
+                            Credentials ldapCreds = JsonConvert.DeserializeObject<Credentials>(json);
+                            
+                            if (ldapCreds == null || string.IsNullOrEmpty(ldapCreds.Username) || string.IsNullOrEmpty(ldapCreds.Password)) {
+                                logger.LogError("Invalid credentials in JSON file. Both Username and Password must be provided.");
+                                return;
+                            }
+                            
+                            ldapOptions.Username = ldapCreds.Username;
+                            ldapOptions.Password = ldapCreds.Password;
+                        } catch (FileNotFoundException) {
+                            logger.LogError("LDAP credentials JSON file not found at path: {Path}", options.LDAPCredentialJsonPath);
+                            return;
+                        } catch (JsonException ex) {
+                            logger.LogError("Failed to parse LDAP credentials JSON file: {Message}", ex.Message);
+                            return;
+                        } catch (Exception ex) {
+                            logger.LogError("Error reading LDAP credentials from JSON file: {Message}", ex.Message);
+                            return;
+                        }
                     }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In src/Sharphound.cs around lines 103 to 108, the code uses File.ReadAllText and
JsonConvert.DeserializeObject but lacks the required using directives and any
error handling; add using System.IO; and using Newtonsoft.Json; at the top of
the file, then wrap the file-read and deserialization block in a try-catch that
catches IO and JSON exceptions (e.g., IOException, JsonException, and a general
Exception fallback), validate that the deserialized Credentials object and its
Username/Password are not null or empty, and on error either log a clear message
and rethrow or throw a new descriptive exception so callers can handle the
failure instead of letting unhandled exceptions propagate silently.


if (options.LDAPEnvUsername != null) {
if (options.LDAPEnvPassword == null) {
logger.LogError("You must specify LDAPEnvPassword if using the LDAPEnvUsername options");
return;
}

string EnvUsername = Environment.GetEnvironmentVariable(options.LDAPEnvUsername);
string EnvPassword = Environment.GetEnvironmentVariable(options.LDAPEnvPassword);

ldapOptions.Username = EnvUsername;
ldapOptions.Password = EnvPassword;
}
Comment on lines +110 to +121
Copy link

@coderabbitai coderabbitai bot Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Add validation for environment variable credential values.

The code retrieves credentials from environment variables but doesn't validate that the variables exist or contain non-empty values. If an environment variable doesn't exist, Environment.GetEnvironmentVariable returns null, which would set invalid credentials silently.

Apply this diff:

                     if (options.LDAPEnvUsername != null) {
                         if (options.LDAPEnvPassword == null) {
                             logger.LogError("You must specify LDAPEnvPassword if using the LDAPEnvUsername options");
                             return;
                         }

                         string EnvUsername = Environment.GetEnvironmentVariable(options.LDAPEnvUsername);
                         string EnvPassword = Environment.GetEnvironmentVariable(options.LDAPEnvPassword);
+                        
+                        if (string.IsNullOrEmpty(EnvUsername)) {
+                            logger.LogError("Environment variable {VarName} not found or empty", options.LDAPEnvUsername);
+                            return;
+                        }
+                        
+                        if (string.IsNullOrEmpty(EnvPassword)) {
+                            logger.LogError("Environment variable {VarName} not found or empty", options.LDAPEnvPassword);
+                            return;
+                        }

                         ldapOptions.Username = EnvUsername;
                         ldapOptions.Password = EnvPassword;
                     }
🤖 Prompt for AI Agents 
In src/Sharphound.cs around lines 110 to 121, the code reads LDAP credentials
from environment variables but does not validate that the retrieved values are
present or non-empty; update the block to check
Environment.GetEnvironmentVariable(...) results with string.IsNullOrWhiteSpace
for both username and password, and if either is null/empty log a clear error
referencing the offending options (options.LDAPEnvUsername or
options.LDAPEnvPassword) and return early instead of assigning invalid
credentials to ldapOptions.Username/Password.


if (options.LDAPUsername != null) {
if (options.LDAPPassword == null) {
logger.LogError("You must specify LDAPPassword if using the LDAPUsername options");
return;
}
Expand Down