feat: add support for Federated identity credentials: BED-7398#2453
Open
feat: add support for Federated identity credentials: BED-7398#2453
Conversation
# Conflicts: # cmd/api/src/config/config.go # cmd/api/src/config/default.go
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR introduces support for Azure Federated Identity Credentials as a new node type and adds the AZAuthenticatesTo relationship. Changes span database schema migrations, Go service converters, schema definitions, UI help components, icon mappings, test fixtures, and an API schema update. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~28 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
coderabbitai
bot
added
enhancement
coderabbitai
bot
added
api
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
packages/javascript/bh-shared-ui/src/components/HelpTexts/AZAuthenticatesTo/References.tsx (1)
22-27: Use descriptive link text instead of rendering the full URL.The current raw URL is hard to scan and brittle for future text updates. Prefer readable anchor text while keeping the same
href.♻️ Proposed refactor
<a target='_blank' rel='noopener noreferrer' href='https://medium.com/@zahmed333/understanding-federated-identity-credentials-simplifying-secure-access-6b67fa79475b'> - https://medium.com/@zahmed333/understanding-federated-identity-credentials-simplifying-secure-access-6b67fa79475b + Understanding Federated Identity Credentials </a>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/javascript/bh-shared-ui/src/components/HelpTexts/AZAuthenticatesTo/References.tsx` around lines 22 - 27, Replace the raw URL anchor in the References component with descriptive link text: locate the <a> element in References.tsx that currently renders the full Medium URL and change its children from the long URL string to a concise, accessible label (e.g., "Understanding Federated Identity Credentials (Medium)") while keeping the href, target, and rel attributes unchanged; ensure the anchor text is meaningful and preserves the original href for correct navigation.packages/javascript/bh-shared-ui/src/components/HelpTexts/AZAuthenticatesTo/AZAuthenticatesTo.tsx (1)
22-27: Lock the aggregator object shape withas const.This helps preserve literal keys and prevents accidental mutation of the mapping.
♻️ Proposed refactor
const AZAuthenticatesTo = { general: General, abuse: Abuse, opsec: Opsec, references: References, -}; +} as const;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/javascript/bh-shared-ui/src/components/HelpTexts/AZAuthenticatesTo/AZAuthenticatesTo.tsx` around lines 22 - 27, The AZAuthenticatesTo object currently allows its shape and keys to be widened or mutated; update the declaration of AZAuthenticatesTo to use a const assertion (append "as const") so TypeScript preserves the literal keys and makes the mapping readonly, e.g., apply the assertion to the AZAuthenticatesTo initializer so the object shape is locked and cannot be altered at compile time; ensure any usages expecting mutable behavior are adjusted to accept the readonly/literal type if needed.packages/javascript/bh-shared-ui/src/components/HelpTexts/AZAuthenticatesTo/General.tsx (1)
21-29: Consider semantic paragraph splitting instead of<br />.This reads as two separate thoughts; two
<p>blocks are a bit cleaner for structure and accessibility.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/javascript/bh-shared-ui/src/components/HelpTexts/AZAuthenticatesTo/General.tsx` around lines 21 - 29, The paragraph in the JSX using <p className='edge-accordion-body2'> contains two distinct thoughts separated by a <br />; split this into two semantic <p> elements (both using the same className if styling should remain) instead of the <br /> to improve structure and accessibility—update the JSX in General.tsx where the paragraph is rendered to replace the single <p> with two <p className='edge-accordion-body2'> blocks containing the respective sentences.cmd/api/src/services/graphify/azure_convertors.go (1)
210-231: Typo in variable name: "federatedIdentifyCredential" should be "federatedIdentityCredential".The variable name has a typo on line 220 (
Identifyinstead ofIdentity), which affects readability and consistency with the type name.✏️ Suggested fix
for _, rawFIC := range data.FICs { var ( - federatedIdentifyCredential models.FICData + federatedIdentityCredential models.FICData ) - if err := json.Unmarshal(rawFIC.FIC, &federatedIdentifyCredential); err != nil { + if err := json.Unmarshal(rawFIC.FIC, &federatedIdentityCredential); err != nil { slog.Error(fmt.Sprintf(SerialError, "app federated identity credential data", err)) } else { - node, rel := ein.ConvertAppFederatedIdentityCredential(federatedIdentifyCredential, rawFIC.AppId) + node, rel := ein.ConvertAppFederatedIdentityCredential(federatedIdentityCredential, rawFIC.AppId) converted.NodeProps = append(converted.NodeProps, node) converted.RelProps = append(converted.RelProps, rel) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/api/src/services/graphify/azure_convertors.go` around lines 210 - 231, In convertAzureAppFIC, rename the misspelled local variable federatedIdentifyCredential to federatedIdentityCredential for consistency with the models.FICData type and readability; update all uses of that identifier (the json.Unmarshal target and the argument passed to ein.ConvertAppFederatedIdentityCredential) so the variable name matches exactly and compile cleanly, leaving the surrounding error handling, rawFIC, convert call, and appending to converted.NodeProps/converted.RelProps unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/go/ein/azure.go`:
- Around line 241-270: The relationship endpoints in
ConvertAppFederatedIdentityCredential are reversed: NewIngestibleRelationship
currently sets the FIC as source and the App as target, but the App should be
the source for an azure.Owns relation. Fix by swapping the two
IngestibleEndpoint arguments in the NewIngestibleRelationship call so the App
endpoint (Kind: azure.App, Value: strings.ToUpper(appID)) is the first/source
endpoint and the FIC endpoint (Kind: azure.FederatedIdentityCredential, Value:
strings.ToUpper(federatedIdentityCredential.ID)) is the second/target endpoint,
keeping RelType: azure.Owns (consistent with ConvertAzureOwnerToRel).
---
Nitpick comments:
In `@cmd/api/src/services/graphify/azure_convertors.go`:
- Around line 210-231: In convertAzureAppFIC, rename the misspelled local
variable federatedIdentifyCredential to federatedIdentityCredential for
consistency with the models.FICData type and readability; update all uses of
that identifier (the json.Unmarshal target and the argument passed to
ein.ConvertAppFederatedIdentityCredential) so the variable name matches exactly
and compile cleanly, leaving the surrounding error handling, rawFIC, convert
call, and appending to converted.NodeProps/converted.RelProps unchanged.
In
`@packages/javascript/bh-shared-ui/src/components/HelpTexts/AZAuthenticatesTo/AZAuthenticatesTo.tsx`:
- Around line 22-27: The AZAuthenticatesTo object currently allows its shape and
keys to be widened or mutated; update the declaration of AZAuthenticatesTo to
use a const assertion (append "as const") so TypeScript preserves the literal
keys and makes the mapping readonly, e.g., apply the assertion to the
AZAuthenticatesTo initializer so the object shape is locked and cannot be
altered at compile time; ensure any usages expecting mutable behavior are
adjusted to accept the readonly/literal type if needed.
In
`@packages/javascript/bh-shared-ui/src/components/HelpTexts/AZAuthenticatesTo/General.tsx`:
- Around line 21-29: The paragraph in the JSX using <p
className='edge-accordion-body2'> contains two distinct thoughts separated by a
<br />; split this into two semantic <p> elements (both using the same className
if styling should remain) instead of the <br /> to improve structure and
accessibility—update the JSX in General.tsx where the paragraph is rendered to
replace the single <p> with two <p className='edge-accordion-body2'> blocks
containing the respective sentences.
In
`@packages/javascript/bh-shared-ui/src/components/HelpTexts/AZAuthenticatesTo/References.tsx`:
- Around line 22-27: Replace the raw URL anchor in the References component with
descriptive link text: locate the <a> element in References.tsx that currently
renders the full Medium URL and change its children from the long URL string to
a concise, accessible label (e.g., "Understanding Federated Identity Credentials
(Medium)") while keeping the href, target, and rel attributes unchanged; ensure
the anchor text is meaningful and preserves the original href for correct
navigation.
ℹ️ Review info
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (20)
cmd/api/src/api/v2/azure.gocmd/api/src/database/migration/extensions/ad_graph_schema.sqlcmd/api/src/database/migration/extensions/az_graph_schema.sqlcmd/api/src/services/graphify/azure_convertors.gogo.modpackages/cue/bh/azure/azure.cuepackages/go/ein/azure.gopackages/go/graphschema/azure/azure.gopackages/go/graphschema/common/common.gopackages/go/schemagen/generator/sql.gopackages/javascript/bh-shared-ui/src/components/HelpTexts/AZAuthenticatesTo/AZAuthenticatesTo.tsxpackages/javascript/bh-shared-ui/src/components/HelpTexts/AZAuthenticatesTo/Abuse.tsxpackages/javascript/bh-shared-ui/src/components/HelpTexts/AZAuthenticatesTo/General.tsxpackages/javascript/bh-shared-ui/src/components/HelpTexts/AZAuthenticatesTo/Opsec.tsxpackages/javascript/bh-shared-ui/src/components/HelpTexts/AZAuthenticatesTo/References.tsxpackages/javascript/bh-shared-ui/src/components/HelpTexts/index.tsxpackages/javascript/bh-shared-ui/src/graphSchema.tspackages/javascript/bh-shared-ui/src/utils/icons.tspackages/javascript/bh-shared-ui/src/views/UserProfile/UserProfile.test.tsxpackages/javascript/bh-shared-ui/src/views/Users/UserActionsMenu.test.tsx
# Conflicts: # packages/go/graphschema/azure/azure.go # packages/go/graphschema/common/common.go
test: add test for new ein function
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/go/ein/azure.go (1)
241-270: Hoist and name local initializations more explicitly in this converter.At Line 242 and Line 254, consider using a single
var (...)block with richer names to align with repo Go style.As per coding guidelines, "Group variable initializations in a `var ( ... )` block and hoist them to the top of the function when possible" and "Prefer rich variable names, for example: `databaseInterface` instead of `di` or `dbi`".Proposed refactor
func ConvertAppFederatedIdentityCredential(federatedIdentityCredential models.FICData, appID string) (IngestibleNode, IngestibleRelationship) { - node := IngestibleNode{ + var ( + federatedIdentityCredentialNode = IngestibleNode{ ObjectID: strings.ToUpper(federatedIdentityCredential.ID), PropertyMap: map[string]any{ common.Description.String(): federatedIdentityCredential.Description, common.Name.String(): federatedIdentityCredential.Name, azure.Issuer.String(): federatedIdentityCredential.Issuer, azure.Audiences.String(): federatedIdentityCredential.Audiences, azure.Subject.String(): federatedIdentityCredential.Subject, }, Labels: []graph.Kind{azure.FederatedIdentityCredential, azure.Entity}, - } + } - rel := NewIngestibleRelationship( + authenticatesToRelationship = NewIngestibleRelationship( IngestibleEndpoint{ Value: strings.ToUpper(federatedIdentityCredential.ID), Kind: azure.FederatedIdentityCredential, }, IngestibleEndpoint{ Kind: azure.App, Value: strings.ToUpper(appID), }, IngestibleRel{ RelProps: map[string]any{}, RelType: azure.AZAuthenticatesTo, }, - ) + ) + ) - return node, rel + return federatedIdentityCredentialNode, authenticatesToRelationship }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/go/ein/azure.go` around lines 241 - 270, The ConvertAppFederatedIdentityCredential function currently declares local composites inline (node and rel); hoist these initializations into a var (...) block at the top of the function and give them clearer names (e.g., federatedIdentityNode for the IngestibleNode and authenticatesRel for the IngestibleRelationship) to follow repo style; build federatedIdentityNode using fields from federatedIdentityCredential and appID, then construct authenticatesRel via NewIngestibleRelationship and return those variables instead of the inline literals.packages/go/ein/azure_test.go (1)
61-80: Add assertions for base-kind labeling on the generated FIC node.At Line 73 onward, this test validates relationship direction/type, but it should also assert that the node keeps both
azure.FederatedIdentityCredentialandazure.Entitylabels to guard query compatibility.Based on learnings, "In BloodHound, AD nodes are represented by a base kind ... and Azure nodes by az.Entity ... code that filters or queries graph nodes via KindIn uses the base kind to capture all inherited subkinds."Proposed test additions
node, rel := ein.ConvertAppFederatedIdentityCredential(testData, appID) require.NotNil(t, node) require.NotNil(t, rel) + require.Contains(t, node.Labels, azure.FederatedIdentityCredential) + require.Contains(t, node.Labels, azure.Entity) require.Equal(t, rel.RelType, azure.AZAuthenticatesTo) require.Equal(t, rel.Source.Value, strings.ToUpper(testData.ID)) require.Equal(t, rel.Target.Value, strings.ToUpper(appID))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/go/ein/azure_test.go` around lines 61 - 80, The test Test_ConvertAppFederatedIdentityCredential should also assert that the created node includes both the specific and base-kind labels so queries that rely on the base kind work; update the test after calling ein.ConvertAppFederatedIdentityCredential to check that the returned node has labels for azure.FederatedIdentityCredential and azure.Entity (i.e., ensure the node's label set contains both identifiers) in addition to the existing relationship assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/go/ein/azure_test.go`:
- Around line 61-80: The test Test_ConvertAppFederatedIdentityCredential should
also assert that the created node includes both the specific and base-kind
labels so queries that rely on the base kind work; update the test after calling
ein.ConvertAppFederatedIdentityCredential to check that the returned node has
labels for azure.FederatedIdentityCredential and azure.Entity (i.e., ensure the
node's label set contains both identifiers) in addition to the existing
relationship assertions.
In `@packages/go/ein/azure.go`:
- Around line 241-270: The ConvertAppFederatedIdentityCredential function
currently declares local composites inline (node and rel); hoist these
initializations into a var (...) block at the top of the function and give them
clearer names (e.g., federatedIdentityNode for the IngestibleNode and
authenticatesRel for the IngestibleRelationship) to follow repo style; build
federatedIdentityNode using fields from federatedIdentityCredential and appID,
then construct authenticatesRel via NewIngestibleRelationship and return those
variables instead of the inline literals.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: c5b5c7ed-a65f-4551-806a-8bb287373f0a
📒 Files selected for processing (2)
packages/go/ein/azure.gopackages/go/ein/azure_test.go
# Conflicts: # go.mod # go.sum
rvazarkar
changed the title
wes-mil
approved these changes
Mar 11, 2026
Contributor
wes-mil
left a comment
wes-mil
left a comment
There was a problem hiding this comment.
I don't fully understand the changes, but I also don't see anything obviously wrong and tests appear to cover
mykeelium
approved these changes
Mar 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds support for Federated Identity Credentials and the AZAuthenticatesTo edge from research
Motivation and Context
https://specterops.atlassian.net/browse/BED-7398
How Has This Been Tested?
Local ingest testing using SpecterDev data, unit testing for ingest
Manual UI testing
Screenshots (optional):
Types of changes
Checklist:
Summary by CodeRabbit
Release Notes
New Features
Documentation