Add ADCS node accordion items #1656
Conversation
WalkthroughThis update introduces several new API endpoints and supporting logic for querying PKI hierarchies, certificate template relationships, trusted CAs, and ADCS escalations across multiple Active Directory-related entity types. The changes span backend API route registration, handler logic, traversal delegates, OpenAPI specifications, JavaScript client methods, UI endpoint mappings, and test coverage, all in an additive and non-breaking manner. Changes
Sequence Diagram(s)sequenceDiagram
participant Client (UI/JS)
participant API Server
participant GraphDB
Client->>API Server: GET /api/v2/{entity}/{id}/{relationship}
API Server->>GraphDB: Query relationship delegate (e.g., PKI hierarchy, escalations)
GraphDB-->>API Server: Return related nodes/paths
API Server-->>Client: Return results (list/graph/count)
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (1)
packages/go/openapi/doc/openapi.json (1)
9887-9940: Repeat the404omission – please add it once for all new endpointsSame concern as above for
aiacas/{object_id}/pki-hierarchy. Rather than fixing piecemeal, consider adding the missing404block to every newly-introduced relationship path in this PR to stay consistent.
🧹 Nitpick comments (3)
packages/go/openapi/doc/openapi.json (3)
10196-10252: Missing 404 response & copy-paste drift
/enterprisecas/{object_id}/pki-hierarchyinherits the same missing-404 problem.
While editing, watch out for lingering copy-paste artefacts (e.g., incorrect tag arrays or descriptions) – do a quick grep to ensure each summary/description matches the path.
10409-10462: Consistency: add 404 & verify tag spelling
NT Auth StoresvsNTAuth Storesappears elsewhere in the spec. Ensure tag names are identical to avoid duplicated sections in generated docs, and add the 404 response block.
10565-10618: Pagination parameters OK, but missingPrefer: pathdocAll new relationship endpoints support graph & path results via the
Preferheader, yet the description only mentions “list, graph, or count”. Add “path” to the description and examples so consumers are aware of the option.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
cmd/api/src/api/registration/v2.go(2 hunks)cmd/api/src/api/v2/ad_entity.go(6 hunks)cmd/api/src/api/v2/ad_related_entity.go(1 hunks)cmd/api/src/api/v2/ad_related_entity_test.go(1 hunks)packages/go/analysis/ad/queries.go(1 hunks)packages/go/openapi/doc/openapi.json(7 hunks)packages/go/openapi/src/openapi.yaml(2 hunks)packages/go/openapi/src/paths/aiacas.aiaca.id.pki-hierarchy.yaml(1 hunks)packages/go/openapi/src/paths/certtemplate.certtemplates.id.published-to-cas.yaml(1 hunks)packages/go/openapi/src/paths/domains.domains.id.adcs-escalations.yaml(1 hunks)packages/go/openapi/src/paths/enterprisecas.enterprisecas.id.pki-hierarchy.yaml(1 hunks)packages/go/openapi/src/paths/enterprisecas.enterprisecas.id.published-templates.yaml(1 hunks)packages/go/openapi/src/paths/ntauthstores.ntauthstores.id.trusted-cas.yaml(1 hunks)packages/go/openapi/src/paths/rootcas.rootcas.id.pki-hierarchy.yaml(1 hunks)packages/javascript/bh-shared-ui/src/utils/content.ts(8 hunks)packages/javascript/js-client-library/src/client.ts(6 hunks)
🧰 Additional context used
🧠 Learnings (3)
cmd/api/src/api/v2/ad_related_entity_test.go (2)
Learnt from: superlinkx
PR: SpecterOps/BloodHound#1503
File: cmd/api/src/services/job/jobs_test.go:19-143
Timestamp: 2025-05-27T16:58:33.295Z
Learning: Tests in cmd/api/src/services/job/jobs_test.go have been found to be flaky in the past and are due for rewrite. They should be skipped with t.Skip() until they can be properly rewritten.
Learnt from: LawsonWillard
PR: SpecterOps/BloodHound#1595
File: cmd/api/src/api/v2/saved_queries_test.go:2594-2594
Timestamp: 2025-06-17T22:37:36.389Z
Learning: In Go table-driven tests, there's a distinction between main test function parallelism and subtest parallelism. Main test functions can safely use t.Parallel() for performance benefits, but individual subtests within table-driven tests may need to run sequentially to avoid race conditions with mocks, deferred functions, or shared resources.
cmd/api/src/api/v2/ad_entity.go (1)
Learnt from: JonasBK
PR: SpecterOps/BloodHound#1434
File: packages/go/analysis/ad/gpos.go:102-126
Timestamp: 2025-06-18T08:27:18.317Z
Learning: In Active Directory's containment hierarchy, each user/computer has exactly one direct parent container, forming a tree structure. When processing GPO edges in packages/go/analysis/ad/gpos.go, the fetchDirectChildUsersAndComputers function only returns direct children, ensuring each user/computer is processed exactly once by its immediate parent container, eliminating the need for deduplication logic.
packages/go/analysis/ad/queries.go (2)
Learnt from: elikmiller
PR: SpecterOps/BloodHound#1563
File: packages/go/graphschema/azure/azure.go:24-24
Timestamp: 2025-06-06T23:12:14.181Z
Learning: In BloodHound, files in packages/go/graphschema/*/`*.go` are generated from CUE schemas. When `just prepare-for-codereview` is run, it triggers code generation that may automatically add import aliases or other formatting changes. These changes are legitimate outputs of the generation process, not manual edits that would be overwritten.
Learnt from: JonasBK
PR: SpecterOps/BloodHound#1434
File: packages/go/analysis/ad/gpos.go:102-126
Timestamp: 2025-06-18T08:27:18.317Z
Learning: In Active Directory's containment hierarchy, each user/computer has exactly one direct parent container, forming a tree structure. When processing GPO edges in packages/go/analysis/ad/gpos.go, the fetchDirectChildUsersAndComputers function only returns direct children, ensuring each user/computer is processed exactly once by its immediate parent container, eliminating the need for deduplication logic.
🧬 Code Graph Analysis (3)
cmd/api/src/api/registration/v2.go (1)
cmd/api/src/api/constant.go (1)
URIPathVariableObjectID(64-64)
cmd/api/src/api/v2/ad_related_entity.go (3)
cmd/api/src/api/v2/model.go (1)
Resources(102-115)cmd/api/src/utils/test/request.go (1)
Request(135-151)packages/go/analysis/ad/queries.go (12)
CreateRootCAPKIHierarchyPathDelegate(1852-1860)CreateRootCAPKIHierarchyListDelegate(1862-1874)CreateCAPKIHierarchyPathDelegate(1876-1899)CreateCAPKIHierarchyListDelegate(1901-1933)CreatePublishedTemplatesPathDelegate(1935-1943)CreatePublishedTemplatesListDelegate(1945-1957)CreatePublishedToCAsPathDelegate(1959-1967)CreatePublishedToCAsListDelegate(1969-1981)CreateTrustedCAsPathDelegate(1983-1991)CreateTrustedCAsListDelegate(1993-2005)CreateADCSEscalationsPathDelegate(2007-2015)CreateADCSEscalationsListDelegate(2017-2029)
packages/javascript/js-client-library/src/client.ts (1)
packages/javascript/js-client-library/src/requests.ts (1)
RequestOptions(26-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build BloodHound Container Image / Build and Package Container
- GitHub Check: build-ui
- GitHub Check: run-tests
- GitHub Check: run-analysis
🔇 Additional comments (49)
packages/go/openapi/src/paths/ntauthstores.ntauthstores.id.trusted-cas.yaml (1)
1-46: Excellent OpenAPI specification structure!The endpoint definition follows established patterns with proper parameter references, comprehensive response codes, and clear documentation. The use of shared schemas and consistent tagging aligns well with the broader PKI functionality enhancement.
packages/go/openapi/src/paths/rootcas.rootcas.id.pki-hierarchy.yaml (1)
1-46: Consistent and well-tailored OpenAPI specification!The Root CA PKI hierarchy endpoint specification maintains excellent consistency with other PKI endpoints while appropriately customizing the operation ID and description for the Root CA context.
packages/go/openapi/src/paths/certtemplate.certtemplates.id.published-to-cas.yaml (1)
1-46: Clear and well-structured endpoint specification!The Certificate Template published-to-CAs endpoint specification clearly describes the relationship being queried and maintains excellent consistency with the PKI endpoint patterns established across the API.
packages/go/openapi/src/paths/aiacas.aiaca.id.pki-hierarchy.yaml (1)
1-46: Excellent pattern consistency for PKI hierarchy endpoints!The AIA CA PKI hierarchy specification maintains perfect consistency with other PKI hierarchy endpoints while being appropriately customized for AIA CA entities. This uniformity enhances API usability and maintainability.
cmd/api/src/api/registration/v2.go (6)
262-262: Well-integrated ADCS escalations endpoint!The domain ADCS escalations route follows established patterns with proper permission requirements and consistent naming conventions.
275-275: Consistent PKI hierarchy implementation for AIA CAs!The AIA CA PKI hierarchy route registration maintains consistency with the established API patterns and properly requires GraphDBRead permissions.
280-280: Properly implemented Root CA PKI hierarchy endpoint!The route registration follows established conventions with appropriate handler method naming and security requirements.
285-286: Excellent Enterprise CA endpoint implementations!Both the PKI hierarchy and published templates endpoints for Enterprise CAs are well-structured with consistent naming patterns and proper permission requirements.
291-291: Well-implemented NT Auth Store trusted CAs endpoint!The route registration maintains consistency with other relationship endpoints and properly requires GraphDBRead permissions for this read operation.
296-296: Consistent Certificate Template relationship endpoint!The published-to-CAs route registration follows established patterns and appropriately integrates with the PKI functionality being added.
packages/go/openapi/src/paths/enterprisecas.enterprisecas.id.pki-hierarchy.yaml (1)
1-46: LGTM! Well-structured OpenAPI specification.The OpenAPI specification follows established patterns and correctly defines the Enterprise CA PKI hierarchy endpoint with appropriate parameters, responses, and documentation.
packages/go/openapi/src/paths/domains.domains.id.adcs-escalations.yaml (1)
1-46: LGTM! Consistent OpenAPI specification.The specification follows the same well-established pattern as other entity endpoints, correctly defining the Domain ADCS escalations endpoint with proper parameters and responses.
packages/go/openapi/src/paths/enterprisecas.enterprisecas.id.published-templates.yaml (1)
1-46: LGTM! Consistent and well-documented endpoint.The OpenAPI specification correctly defines the Enterprise CA published templates endpoint following established patterns with appropriate parameters and responses.
cmd/api/src/api/v2/ad_related_entity.go (1)
220-242: LGTM! Consistent handler implementation.The new handler methods follow the established pattern perfectly, delegating to the common
handleAdRelatedEntityQueryfunction with appropriate path and list delegates. The implementation is clean and consistent with existing code.cmd/api/src/api/v2/ad_related_entity_test.go (1)
495-541: LGTM! Comprehensive test coverage with consistent patterns.The new test functions follow the established testing pattern perfectly, providing appropriate coverage for all the new handler methods. The consistency with existing tests ensures maintainability and reliability.
cmd/api/src/api/v2/ad_entity.go (7)
150-150: LGTM! Domain ADCS escalations query delegate added correctly.The addition of the ADCS escalations delegate follows the established pattern and aligns with the PR objective of exposing ADCS escalation paths for Domain entities.
173-174: LGTM! AIACA entity count queries enhanced with controllers and PKI hierarchy.The additions follow the existing code pattern and provide the necessary query delegates for the new PKI hierarchy functionality.
184-185: LGTM! RootCA entity enhanced with controllers and PKI hierarchy queries.Consistent with other CA entity types and follows established patterns.
195-197: LGTM! EnterpriseCA entity properly extended with comprehensive query delegates.The addition of controllers, PKI hierarchy, and published templates delegates provides complete coverage for EnterpriseCA relationships as outlined in the PR objectives.
207-207: LGTM! NTAuthStore trusted CAs delegate added correctly.The addition enables querying of trusted CA relationships for NT Auth Stores as specified in the PR requirements.
216-217: LGTM! CertTemplate entity enhanced with controllers and published-to-CAs queries.The additions complete the CertTemplate relationship querying capabilities and follow established code patterns.
150-217: All new delegate functions are present and correctly implementedAll referenced ListDelegate functions—CreateADCSEscalationsListDelegate, CreateCAPKIHierarchyListDelegate, CreateRootCAPKIHierarchyListDelegate, CreatePublishedTemplatesListDelegate, CreateTrustedCAsListDelegate, and CreatePublishedToCAsListDelegate—are defined in packages/go/analysis/ad/queries.go. They follow the standard
(tx, node, skip, limit) (NodeSet, error)signature and propagate errors from the underlyingops.AcyclicTraverse*calls as expected.packages/go/openapi/src/openapi.yaml (7)
469-470: LGTM! Domain ADCS escalations endpoint added correctly.The endpoint path
/api/v2/domains/{object_id}/adcs-escalationsproperly corresponds to the "adcs-escalations" count query delegate added in the Go code.
491-492: LGTM! AIACA PKI hierarchy endpoint added correctly.The endpoint follows established naming conventions and aligns with the PKI hierarchy delegate in the Go code.
499-500: LGTM! RootCA PKI hierarchy endpoint added correctly.Consistent with the AIACA endpoint pattern and matches the corresponding Go delegate.
505-510: LGTM! EnterpriseCA endpoints properly organized and comprehensive.The controllers, PKI hierarchy, and published templates endpoints provide complete coverage for EnterpriseCA relationships as specified in the PR objectives.
517-518: LGTM! NTAuthStore trusted CAs endpoint added correctly.The endpoint naming is consistent and matches the trusted-cas delegate in the Go code.
525-526: LGTM! CertTemplate published-to-CAs endpoint added correctly.The endpoint completes the CertTemplate relationship API coverage and follows established patterns.
469-526: Verified: All referenced OpenAPI path specification files exist and define GET operationsAll of the newly referenced files under
packages/go/openapi/src/paths/were found and include aget:operation. No further changes are required here.packages/javascript/bh-shared-ui/src/utils/content.ts (12)
702-706: LGTM! PKI Hierarchy section properly added for AIACA entities.The new section follows the established pattern and naming conventions consistently.
714-718: LGTM! Published To CAs section properly added for CertTemplate entities.The implementation is consistent with existing patterns and provides appropriate PKI visibility.
846-850: LGTM! ADCS Escalations section properly added for Domain entities.This addition provides valuable security insights into ADCS escalation vectors, which aligns well with the PR objectives.
858-867: LGTM! PKI sections properly added for EnterpriseCA entities.Both "PKI Hierarchy" and "Published Templates" sections are well-implemented and provide comprehensive PKI visibility for Enterprise Certificate Authorities.
961-965: LGTM! Trusted CAs section properly added for NTAuthStore entities.This addition enhances PKI trust relationship visibility, which is crucial for security analysis.
995-999: LGTM! PKI Hierarchy section properly added for RootCA entities.This completes the PKI hierarchy coverage across all relevant certificate authority types.
1635-1636: LGTM! AIACA PKI hierarchy endpoint properly mapped.The endpoint mapping correctly uses the API client method with proper signal handling.
1641-1644: LGTM! CertTemplate published to CAs endpoint properly mapped.The implementation follows the established pattern with correct API method and signal handling.
1711-1714: LGTM! Domain ADCS escalations endpoint properly mapped.This endpoint mapping correctly implements the ADCS escalation analysis functionality.
1719-1726: LGTM! EnterpriseCA endpoint mappings properly implemented.Both PKI hierarchy and published templates endpoints are correctly mapped with appropriate API client methods and signal handling.
1761-1764: LGTM! NTAuthStore trusted CAs endpoint properly mapped.The implementation correctly uses the API client method with proper signal handling.
1775-1776: LGTM! RootCA PKI hierarchy endpoint properly mapped.The endpoint mapping completes the PKI hierarchy functionality with correct implementation.
packages/javascript/js-client-library/src/client.ts (7)
1546-1559: LGTM! Method correctly implements ADCS escalations endpoint.The implementation follows the established patterns for domain sub-resource endpoints and properly handles pagination and filtering parameters.
2095-2108: LGTM! PKI hierarchy endpoint for AIACA correctly implemented.The method follows the established pattern for AIACA sub-resource endpoints with proper parameter handling.
2138-2151: LGTM! PKI hierarchy endpoint for RootCA correctly implemented.The method maintains consistency with other RootCA endpoints and follows the established patterns.
2187-2206: LGTM! PKI hierarchy endpoint for EnterpriseCA correctly implemented.The method completes the consistent PKI hierarchy pattern across all CA entity types.
2208-2227: LGTM! Published templates endpoint for EnterpriseCA correctly implemented.The method properly enables retrieval of published certificate templates, supporting the ADCS functionality requirements.
2263-2276: LGTM! Trusted CAs endpoint for NTAuthStore correctly implemented.The method enables retrieval of trusted CA relationships, supporting the required ADCS trust relationship functionality.
2312-2331: LGTM! Published CAs endpoint for Certificate Template correctly implemented.The method provides the reverse relationship for published templates, enabling comprehensive certificate template publication tracking.
packages/go/analysis/ad/queries.go (1)
2007-2029: ADCS escalations delegates follow standard Create…Delegate signatures and all escalation kinds are definedI’ve verified that CreateADCSEscalationsPathDelegate/ListDelegate use the same
(tx graph.Transaction, node *graph.Node) (graph.PathSet, error)and(tx graph.Transaction, node *graph.Node, skip, limit int) (graph.NodeSet, error)patterns as the other Create*Delegate functions in queries.go. All ADCS escalation relationship kinds (GoldenCert, ADCSESC1, ADCSESC3, ADCSESC4, ADCSESC6a, ADCSESC6b, ADCSESC9a, ADCSESC9b, ADCSESC10a, ADCSESC10b) are present in the schema’s InboundRelationshipKinds/OutboundRelationshipKinds. No further changes required.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
packages/go/openapi/src/paths/aiacas.aiaca.id.pki-hierarchy.yaml (1)
24-27: Tag set is drifting from existing conventionPrevious endpoints group CA-related APIs under the single tag
"AIA CAs".
Adding"Community"and"Enterprise"here creates a three-tag combination not used elsewhere and clutters the rendered docs. Consider using component-levelx-specterops-plan/x-specterops-license(as done in other parts of the spec) instead of public tags.packages/go/openapi/src/paths/domains.domains.id.adcs-escalations.yaml (1)
24-27: Consistent taggingSame remark as in the AIA-CA spec: consider consolidating tags to avoid bloating the tag list in Swagger UI.
packages/go/openapi/src/paths/enterprisecas.enterprisecas.id.pki-hierarchy.yaml (1)
24-27: Redundant tagsSame tagging consistency issue as noted for the other files.
packages/go/openapi/src/paths/ntauthstores.ntauthstores.id.trusted-cas.yaml (1)
24-27: Tag proliferationSee earlier comments on
"Community"/"Enterprise"tags.packages/go/openapi/src/paths/certtemplate.certtemplates.id.published-to-cas.yaml (1)
24-27: Tag bloatConsider reducing public tags; replicate existing approach used elsewhere in the spec.
packages/go/openapi/doc/openapi.json (2)
10409-10426: OperationId casing inconsistency (TrustedCasvsTrustedCAs)
GetNtAuthStoreEntityTrustedCasdrops the capital “A” compared to the path segment/trusted-cas. Stick to one convention to avoid confusion and potential code-gen issues:- "operationId": "GetNtAuthStoreEntityTrustedCas", + "operationId": "GetNtAuthStoreEntityTrustedCAs",Same applies to any handler or client code generated from the spec.
10199-10231: Consider DRY-ing the repeated related-entity parameter setsEvery new related-entity endpoint copies the same four query parameters (
skip,limit,type,sort-by). You can collapse these into a single reusable “standardRelatedEntityParams” array incomponents/parametersand$refit here. This keeps the spec shorter and makes later updates (e.g., addingfilter) one-shot.Also applies to: 10409-10441, 10565-10597
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
cmd/api/src/api/registration/v2.go(2 hunks)cmd/api/src/api/v2/ad_entity.go(6 hunks)cmd/api/src/api/v2/ad_related_entity.go(1 hunks)cmd/api/src/api/v2/ad_related_entity_test.go(1 hunks)packages/go/analysis/ad/queries.go(1 hunks)packages/go/openapi/doc/openapi.json(7 hunks)packages/go/openapi/src/openapi.yaml(2 hunks)packages/go/openapi/src/paths/aiacas.aiaca.id.pki-hierarchy.yaml(1 hunks)packages/go/openapi/src/paths/certtemplate.certtemplates.id.published-to-cas.yaml(1 hunks)packages/go/openapi/src/paths/domains.domains.id.adcs-escalations.yaml(1 hunks)packages/go/openapi/src/paths/enterprisecas.enterprisecas.id.pki-hierarchy.yaml(1 hunks)packages/go/openapi/src/paths/enterprisecas.enterprisecas.id.published-templates.yaml(1 hunks)packages/go/openapi/src/paths/ntauthstores.ntauthstores.id.trusted-cas.yaml(1 hunks)packages/go/openapi/src/paths/rootcas.rootcas.id.pki-hierarchy.yaml(1 hunks)packages/javascript/bh-shared-ui/src/utils/content.ts(8 hunks)packages/javascript/js-client-library/src/client.ts(6 hunks)
🧰 Additional context used
🧠 Learnings (3)
cmd/api/src/api/v2/ad_entity.go (1)
Learnt from: JonasBK
PR: SpecterOps/BloodHound#1434
File: packages/go/analysis/ad/gpos.go:102-126
Timestamp: 2025-06-18T08:27:18.317Z
Learning: In Active Directory's containment hierarchy, each user/computer has exactly one direct parent container, forming a tree structure. When processing GPO edges in packages/go/analysis/ad/gpos.go, the fetchDirectChildUsersAndComputers function only returns direct children, ensuring each user/computer is processed exactly once by its immediate parent container, eliminating the need for deduplication logic.
cmd/api/src/api/v2/ad_related_entity_test.go (3)
Learnt from: superlinkx
PR: SpecterOps/BloodHound#1503
File: cmd/api/src/services/job/jobs_test.go:19-143
Timestamp: 2025-05-27T16:58:33.295Z
Learning: Tests in cmd/api/src/services/job/jobs_test.go have been found to be flaky in the past and are due for rewrite. They should be skipped with t.Skip() until they can be properly rewritten.
Learnt from: mvlipka
PR: SpecterOps/BloodHound#1615
File: cmd/api/src/test/integration/harnesses/AZPIMRolesHarness.json:190-194
Timestamp: 2025-06-26T20:50:03.695Z
Learning: The JSON files in cmd/api/src/test/integration/harnesses/ directory (like AZPIMRolesHarness.json) are used as reference documents for writing Go test harnesses, not as actual data files that get parsed by the application code. Minor formatting issues like trailing spaces in these reference files don't affect functionality.
Learnt from: LawsonWillard
PR: SpecterOps/BloodHound#1595
File: cmd/api/src/api/v2/saved_queries_test.go:2594-2594
Timestamp: 2025-06-17T22:37:36.389Z
Learning: In Go table-driven tests, there's a distinction between main test function parallelism and subtest parallelism. Main test functions can safely use t.Parallel() for performance benefits, but individual subtests within table-driven tests may need to run sequentially to avoid race conditions with mocks, deferred functions, or shared resources.
packages/go/analysis/ad/queries.go (2)
Learnt from: elikmiller
PR: SpecterOps/BloodHound#1563
File: packages/go/graphschema/azure/azure.go:24-24
Timestamp: 2025-06-06T23:12:14.181Z
Learning: In BloodHound, files in packages/go/graphschema/*/`*.go` are generated from CUE schemas. When `just prepare-for-codereview` is run, it triggers code generation that may automatically add import aliases or other formatting changes. These changes are legitimate outputs of the generation process, not manual edits that would be overwritten.
Learnt from: JonasBK
PR: SpecterOps/BloodHound#1434
File: packages/go/analysis/ad/gpos.go:102-126
Timestamp: 2025-06-18T08:27:18.317Z
Learning: In Active Directory's containment hierarchy, each user/computer has exactly one direct parent container, forming a tree structure. When processing GPO edges in packages/go/analysis/ad/gpos.go, the fetchDirectChildUsersAndComputers function only returns direct children, ensuring each user/computer is processed exactly once by its immediate parent container, eliminating the need for deduplication logic.
🧬 Code Graph Analysis (2)
cmd/api/src/api/registration/v2.go (1)
cmd/api/src/api/constant.go (1)
URIPathVariableObjectID(64-64)
packages/javascript/js-client-library/src/client.ts (1)
packages/javascript/js-client-library/src/requests.ts (1)
RequestOptions(26-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build BloodHound Container Image / Build and Package Container
- GitHub Check: run-tests
🔇 Additional comments (39)
packages/go/openapi/src/paths/aiacas.aiaca.id.pki-hierarchy.yaml (1)
17-20: Path parameter reference may not match template{id}The file references
parameters/path.object-id.yaml, which (in existing specs) usually defines a path parameter namedobject-id.
However, the endpoint’s template (derived from the filenameaiacas/{id}/pki-hierarchy) uses{id}.
If the referenced parameter schema still exposes a parameter calledobject-id, the OpenAPI document will be invalid (Unresolved reference / mismatched name).
Please verify that the referenced parameter file defines exactlyname: id, or switch the$refto the correct parameter file (e.g.path.id.yaml).packages/go/openapi/src/paths/domains.domains.id.adcs-escalations.yaml (1)
17-20: Potential path-parameter mismatch identical to previous file
path.object-id.yamlis referenced while the route template appears to be/domains/{id}/adcs-escalations. Re-check that the parameter file exportsid, notobject-id, otherwise code-gen and validation will break.packages/go/openapi/src/paths/enterprisecas.enterprisecas.id.pki-hierarchy.yaml (1)
17-20: Check the$reffor the path parameterEnsure
path.object-id.yamlmatches{id}in the path template; otherwise generator output for clients/servers will be incorrect.packages/go/openapi/src/paths/ntauthstores.ntauthstores.id.trusted-cas.yaml (1)
17-20: Verify parameter name
path.object-id.yamlvs{id}– please confirm they align.packages/go/openapi/src/paths/certtemplate.certtemplates.id.published-to-cas.yaml (1)
17-20: Mismatch between parameter reference and route templateSame concern: confirm that the referenced parameter file defines
id.packages/go/openapi/src/paths/rootcas.rootcas.id.pki-hierarchy.yaml (1)
1-46: LGTM! Well-structured OpenAPI specification.The OpenAPI specification correctly follows established patterns and conventions. The endpoint design is consistent with other entity endpoints, using appropriate parameter references, response schemas, and tags.
packages/go/openapi/src/paths/enterprisecas.enterprisecas.id.published-templates.yaml (1)
1-46: LGTM! Consistent OpenAPI specification.The specification follows the same well-established pattern as other entity endpoints, maintaining consistency across the API. All parameter references and response schemas are correctly defined.
cmd/api/src/api/registration/v2.go (2)
262-262: LGTM! Consistent route registration pattern.The new ADCS escalations endpoint follows the established pattern with proper URL formatting, handler method, and permission requirements.
275-275: LGTM! Well-structured API route additions.All new PKI-related routes follow consistent patterns:
- Proper URL path formatting using
fmt.Sprintfandapi.URIPathVariableObjectID- Appropriate handler method assignments
- Correct
GraphDBReadpermission requirements- Consistent naming conventions for endpoints
The additive nature of these changes maintains backward compatibility.
Also applies to: 280-280, 285-286, 291-291, 296-296
cmd/api/src/api/v2/ad_entity.go (1)
150-150: LGTM! Consistent count query addition.The new ADCS escalations count query follows the established pattern and integrates well with the existing Domain entity count queries.
cmd/api/src/api/v2/ad_related_entity_test.go (1)
495-541: LGTM! Excellent test coverage with consistent patterns.All new test functions follow the established testing patterns perfectly:
- Proper mock setup and cleanup
- Consistent use of shared
setupCases()for test scenarios- Standard
apitest.NewHarnessusage for test execution- Appropriate test naming conventions
The tests provide comprehensive coverage for all new ADCS-related API handlers.
cmd/api/src/api/v2/ad_related_entity.go (1)
220-242: LGTM! New ADCS handlers follow established patterns.The six new handler methods are well-implemented and consistent with the existing codebase:
- All follow the same pattern of calling
handleAdRelatedEntityQuery- Query names match function names exactly
- Delegate functions are properly namespaced w 8000 ith
adAnalysis- Each provides both path and list delegates for polymorphic responses
- Naming conventions align with existing handlers
The implementation correctly extends the API surface for ADCS-related entities while maintaining code consistency.
packages/go/openapi/src/openapi.yaml (1)
469-470: LGTM! OpenAPI spec updates are well-structured.The new endpoint definitions are properly integrated:
- All follow the established URL pattern:
/api/v2/{entity_type}/{object_id}/{relationship}- References to external YAML files use consistent
./paths/directory structure- Naming conventions align with the API handler methods
- Proper placement within the existing endpoint hierarchy
- Entity types (domains, aiacas, rootcas, enterprisecas, ntauthstores, certtemplates) are logically organized
The OpenAPI specification correctly documents the new ADCS-related endpoints while maintaining consistency with the existing API structure.
Also applies to: 491-492, 499-500, 505-510, 517-518, 525-526
packages/javascript/js-client-library/src/client.ts (6)
1546-1559: LGTM! ADCS escalations endpoint follows established patterns.The
getDomainADCSEscalationsV2method correctly implements the standard pattern for domain-related entity queries, with proper parameter types and consistent endpoint structure.
2095-2108: LGTM! PKI hierarchy endpoint implementation is consistent.The
getAIACAPKIHierarchyV2method follows the established pattern for AIACA entity queries with proper parameter handling and endpoint structure.
2138-2151: LGTM! RootCA PKI hierarchy follows consistent implementation.The
getRootCAPKIHierarchyV2method maintains consistency with other RootCA methods and follows the established parameter and endpoint patterns.
2187-2227: LGTM! EnterpriseCA methods follow established patterns.Both
getEnterpriseCAPKIHierarchyV2andgetEnterpriseCAPublishedTemplatesV2methods are well-implemented with consistent parameter formatting and proper endpoint structures. The multiline parameter formatting enhances readability for methods with multiple parameters.
2263-2276: LGTM! NTAuthStore trusted CAs endpoint is properly implemented.The
getNTAuthStoreTrustedCAsV2method correctly follows the established pattern for NTAuthStore entity queries with consistent parameter handling.
2312-2331: LGTM! CertTemplate published CAs endpoint follows conventions.The
getCertTemplatePublishedToCAsV2method maintains consistency with other CertTemplate methods and properly implements the standard parameter and endpoint patterns.packages/javascript/bh-shared-ui/src/utils/content.ts (7)
702-706: LGTM! PKI Hierarchy section properly implemented.The new accordion section follows the established pattern and naming conventions correctly.
714-718: LGTM! Published To CAs section properly implemented.The new accordion section follows the established pattern and naming conventions correctly.
846-850: LGTM! ADCS Escalations section properly implemented.The new accordion section follows the established pattern and naming conventions correctly.
858-867: LGTM! EnterpriseCA sections properly implemented.Both PKI Hierarchy and Published Templates sections follow the established pattern and naming conventions correctly.
961-965: LGTM! Trusted CAs section properly implemented.The new accordion section follows the established pattern and naming conventions correctly.
995-999: LGTM! PKI Hierarchy section properly implemented.The new accordion section follows the established pattern and naming conventions correctly.
1635-1636: LGTM! All endpoint mappings consistently implemented.The new API endpoint mappings follow the established patterns correctly:
- Consistent parameter handling and API client method calls
- Proper request cancellation support with controller.signal
- Uniform response handling with .then((res) => res.data)
- Method names follow the established V2 naming convention
Also applies to: 1641-1644, 1711-1714, 1719-1722, 1723-1726, 1761-1764, 1775-1776
packages/go/openapi/doc/openapi.json (1)
9413-9466: Missingsecurityrequirement blockAll other protected v2 endpoints explicitly declare the
securityrequirement (e.g.,[{ "ApiKeyAuth": [] }]). The new Domain-ADCS escalation path omits it, meaning tooling may treat it as public. Add the standard security stanza to stay consistent:"responses": { ... } + , + "security": [ + { "ApiKeyAuth": [] } + ](Adjust to the scheme actually used in the rest of the spec.)
packages/go/analysis/ad/queries.go (12)
1852-1860: LGTM! Well-implemented Root CA PKI hierarchy path delegate.The function correctly implements a path delegate for traversing Root CA PKI hierarchies using appropriate relationship types and inbound direction.
1862-1874: LGTM! Properly implemented list delegate with correct pagination and exclusion logic.The function correctly implements pagination and excludes the root node from results, following established patterns in the codebase.
1876-1899: LGTM! Well-designed bidirectional CA PKI hierarchy traversal.The function properly handles both inbound and outbound traversals with appropriate relationship types for each direction, and correctly combines the results.
1901-1933: LGTM! Correctly implements bidirectional list traversal with proper pagination.The function mirrors the path delegate logic appropriately, handling pagination and root node exclusion for both traversal directions.
1935-1943: LGTM! Clean implementation for published templates traversal.The function correctly uses inbound direction and the appropriate PublishedTo relationship for finding certificate templates published to CAs.
1945-1957: LGTM! Properly implemented list delegate with correct pagination.The function follows the established pattern and correctly implements pagination while excluding the root node from results.
1959-1967: LGTM! Correct implementation for published-to-CAs traversal.The function properly uses outbound direction to find CAs that a certificate template is published to, complementing the published templates delegate.
1969-1981: LGTM! Correctly implements list delegate with proper pagination.The function appropriately mirrors the path delegate logic while adding pagination support and root node exclusion.
1983-1991: LGTM! Clean implementation for trusted CAs traversal.The function correctly uses inbound direction and the TrustedForNTAuth relationship to find CAs trusted by NT Auth Stores.
1993-2005: LGTM! Properly implemented list delegate with correct pagination.The function follows the established pattern and correctly implements pagination while excluding the root node from results.
2007-2015: LGTM! Comprehensive ADCS escalations traversal implementation.The function correctly covers all major ADCS escalation relationship types and uses appropriate inbound direction to find potential attackers.
2017-2029: LGTM! Comprehensive ADCS escalations list delegate with proper pagination.The function correctly implements the list delegate pattern while covering all major ADCS escalation relationship types with proper pagination and root node exclusion.
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/go/openapi/src/paths/enterprisecas.enterprisecas.id.controllers.yaml (1)
34-45: Missing404 Not Foundresponse for non-existent CAAll other entity-related endpoints (see sibling file) return
404when the{id}does not correspond to an existing object. Omitting it here creates inconsistent error semantics and forces callers to fall back to500/400heuristics.Add a
404section identical to the pattern used elsewhere:$ref: './../responses/related-entity-query-results.yaml' + 404: + $ref: './../responses/not-found.yaml'
🧹 Nitpick comments (1)
packages/go/openapi/src/paths/enterprisecas.enterprisecas.id.yaml (1)
29-33:hydrate-countsparameter may need stronger semantics
hydrate-countsis currently a boolean toggle, but the description (“Get info and counts”) implies the counts are always present. To avoid ambiguity for SDK generators and documentation:- description: Get info and counts for this Enterprise CA node. + description: Get basic Enterprise CA info. + If the optional `hydrate-counts=true` query parameter is supplied, the + response additionally includes related-object counts.Also ensure
entity-info-query-results.yamlsupports an optionalcountssection.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/go/openapi/doc/openapi.json(6 hunks)packages/go/openapi/src/paths/domains.domains.id.adcs-escalations.yaml(1 hunks)packages/go/openapi/src/paths/enterprisecas.enterprisecas.id.controllers.yaml(1 hunks)packages/go/openapi/src/paths/enterprisecas.enterprisecas.id.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/go/openapi/src/paths/domains.domains.id.adcs-escalations.yaml
- packages/go/openapi/doc/openapi.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build BloodHound Container Image / Build and Package Container
- GitHub Check: run-analysis
- GitHub Check: build-ui
- GitHub Check: run-tests
🔇 Additional comments (1)
packages/go/openapi/src/paths/enterprisecas.enterprisecas.id.yaml (1)
21-24: Clarify breaking-change impact of new operation and removed pagination parametersRenaming the operation (
GetEnterpriseCaEntity*Controllers*→GetEnterpriseCaEntity) and dropping theskip/limit/type/sort-byquery params fundamentally changes the contract. Any client previously expecting paginated related-entity results will now receive a single-entity payload and could break at runtime.Please confirm that:
- All first-party consumers have been updated.
- External API users were notified or the change is gated behind a new major API version.
If backward compatibility is required, consider keeping the old path/operation or introducing a
v3namespace.
Description
This PR adds new API endpoints and UI accordion sections for ADCS (Active Directory Certificate Services) entities to provide better visibility into PKI infrastructure and certificate-related attack paths.
Motivation and Context
Resolves BED-6145
New accordion items:
These changes enhance BloodHound's ADCS analysis capabilities by exposing critical PKI relationships that are essential for understanding certificate-based attack paths. The accordion UI provides security analysts with an intuitive way to explore PKI hierarchies, certificate template publications, and potential ADCS escalation vectors directly from entity detail pages.
How Has This Been Tested?
Locally with this dataset:
20250611101235_BloodHound.zip
Screenshots (optional):
Types of changes
Checklist:
Summary by CodeRabbit
New Features
Tests