chore: quick fixes

[rvazarkar]

Fix a formatting issue
Remove unnecessary string pointers

Summary by CodeRabbit

• Style

  • Improved formatting and indentation in the federated identity listing workflow for clearer output and maintainability.

• Refactor

  • Normalized sign-in activity data handling to use consistent types, improving reliability and reducing ambiguity when displaying or processing user sign-in information.

• Tests

  • Updated tests to align with the revised sign-in activity data model, ensuring correct validation of sign-in details.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

---

Walkthrough

Whitespace/indentation tweaks in a CLI command file and conversion of six SignInActivity struct fields from pointer types (*string) to non-pointer string, with corresponding test update to use direct string access.

Changes

Cohort / File(s)	Summary
Command Formatting cmd/list-app-federated-identity-credentials.go	Whitespace and indentation adjustments around an else branch / if block in listAppFICs; no functional logic changes.
Model Type Conversions models/azure/user-signinactivity.go	Six fields in SignInActivity changed from *string to string while preserving json:"...,omitempty" tags.
Test Update models/azure/user_test.go	Updated test assertions to access SignInActivity fields as direct string values instead of dereferencing pointers (*).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• feat: Adding federated identity credentials #164: Edits touch the same listAppFICs function referenced in that PR.

Poem

🐰 I hopped through structs and tidy’d the flow,
Switched pointers to strings so the fields could grow,
Fixed a stray tab, then nibbled a test,
Now JSON sleeps tidy, and I take a rest. 🥕

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'chore: quick fixes' is too vague and generic; it does not clearly describe the specific changes made in the changeset.	Use a more descriptive title that reflects the main change, such as 'chore: convert SignInActivity fields from pointers to strings' or 'chore: remove pointer types from SignInActivity model'.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch quick_fixes

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@models/azure/user-signinactivity.go`:
- Around line 4-11: The SignInActivity struct fields were changed from *string
to string, breaking code that dereferences pointers (e.g., user_test.go expects
u.SignInActivity.LastSuccessfulSignInDateTime to be a *string) and changing
null-vs-empty semantics; revert the fields in SignInActivity
(LastSignInDateTime, LastSignInRequestId, LastNonInteractiveSignInDateTime,
LastNonInteractiveSignInRequestId, LastSuccessfulSignInDateTime,
LastSuccessfulSignInRequestId) back to *string so tests and JSON null handling
work as before and update any callers only if you intentionally want to change
semantics.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for SignInActivity usages and potential nil checks

echo "=== Finding all SignInActivity usages in Go files ==="
rg -n 'SignInActivity' --type go -B 2 -A 5

echo ""
echo "=== Searching for potential nil checks on these fields ==="
rg -n '(LastSignInDateTime|LastSignInRequestId|LastNonInteractiveSignInDateTime|LastNonInteractiveSignInRequestId|LastSuccessfulSignInDateTime|LastSuccessfulSignInRequestId)\s*(!= nil|== nil|\*\.|\&)' --type go -B 2 -A 2

echo ""
echo "=== Looking for SignInActivity struct definition and file context ==="
fd 'user-signinactivity.go' --type f | xargs cat -n

echo ""
echo "=== Checking for json.Unmarshal patterns with SignInActivity ==="
rg -n 'Unmarshal.*SignInActivity\|SignInActivity.*Unmarshal' --type go -B 3 -A 3

Repository: SpecterOps/AzureHound

Length of output: 4062

---

🏁 Script executed:

# Read the test file to confirm the pointer dereference issue
cat -n models/azure/user_test.go | sed -n '23,50p'

Repository: SpecterOps/AzureHound

Length of output: 1127

---

Test file will not compile: pointer dereferences incompatible with string fields.

The change from *string to string fields breaks test code at models/azure/user_test.go:44-45, which attempts to dereference LastSuccessfulSignInDateTime:

if *u.SignInActivity.LastSuccessfulSignInDateTime != "2025-01-27T22:20:22Z" {

This will fail to compile because string values cannot be dereferenced. Additionally, the semantic change remains: null values from the API (as shown in the test's JSON payload on lines 32-33) will now unmarshal to empty strings instead of nil, affecting how missing sign-in activity is distinguished from explicitly empty values.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@models/azure/user-signinactivity.go` around lines 4 - 11, The SignInActivity
struct fields were changed from *string to string, breaking code that
dereferences pointers (e.g., user_test.go expects
u.SignInActivity.LastSuccessfulSignInDateTime to be a *string) and changing
null-vs-empty semantics; revert the fields in SignInActivity
(LastSignInDateTime, LastSignInRequestId, LastNonInteractiveSignInDateTime,
LastNonInteractiveSignInRequestId, LastSuccessfulSignInDateTime,
LastSuccessfulSignInRequestId) back to *string so tests and JSON null handling
work as before and update any callers only if you intentionally want to change
semantics.