feat: EntraUser sign on to AWS Identity Center (#1779) #1787

achantavy · 2025-08-06T23:54:56Z

Summary

Describe your changes.

Checklist

Provide proof that this works (this makes reviews move faster). Please perform one or more of the following:

Update/add unit or integration tests.
Include a screenshot showing what the graph looked like before and after your changes.
Include console log trace showing what happened before and after your changes.

If you are changing a node or relationship:

Update the schema and readme.

If you are implementing a new intel module:

Use the NodeSchema data model.

Signed-off-by: Alex Chantavy <[email protected]>

achantavy · 2025-08-07T00:00:03Z

tests/unit/cartography/intel/entra/test_applications.py

@@ -45,119 +32,3 @@ def test_transform_app_role_assignments():
    assert assignment3["principal_display_name"] == "Finance Team"
    assert assignment3["resource_display_name"] == "Finance Tracker"
    assert assignment3["principal_type"] == "Group"
-
-
-@patch("cartography.intel.entra.applications.load")


removing these brittle and not very useful unit tests

cubic-dev-ai

cubic analysis

2 issues found across 9 files • Review in cubic

_{React with 👍 or 👎 to teach cubic. You can also tag @cubic-dev-ai to give feedback, ask questions, or re-run the review.}

cubic-dev-ai · 2025-08-07T00:01:05Z

tests/data/entra/applications.py

+        "display_name": "Finance Tracker Service Principal",
+        "reply_urls": [
+            "https://finance.example.com/callback",
+            "https://d-90663f7df1.awsapps.com/start",


AWS Identity Center instance id in this reply_urls entry is inconsistent with the corresponding login_url and aws_identity_center_instance_id values, which can cause equality assertions in tests to fail (Based on your team's feedback about ensuring mock data is internally consistent).

Prompt for AI agents

Address the following comment on tests/data/entra/applications.py at line 191: <comment>AWS Identity Center instance id in this reply_urls entry is inconsistent with the corresponding login_url and aws_identity_center_instance_id values, which can cause equality assertions in tests to fail (Based on your team's feedback about ensuring mock data is internally consistent).</comment> <file context> @@ -153,3 +154,57 @@ "application_app_id": "ffffffff-eeee-dddd-cccc-bbbbbbbbbbbb", }, ] + +MOCK_SERVICE_PRINCIPALS = [ + ServicePrincipal( + id="sp-11111111-1111-1111-1111-111111111111", + app_id="aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee", + display_name="Finance Tracker Service Principal", </file context>

Suggested change

"https://d-90663f7df1.awsapps.com/start",

"https://d-1234567890.awsapps.com/start",

✅ Addressed in 4c856f9

cubic-dev-ai · 2025-08-07T00:01:05Z

cartography/models/entra/entra_user_to_aws_sso.py

+    target_node_label: str = "AWSSSOUser"
+    target_node_matcher: TargetNodeMatcher = make_target_node_matcher(
+        {
+            "user_name": PropertyRef("aws_user_name"),


Target node matcher relies solely on user_name, which is not guaranteed to be unique across Identity Stores; include identity_store_id in the matcher to avoid incorrect links (Based on your team's feedback about ensuring relationship matchers use stable, unique keys).

Prompt for AI agents

Address the following comment on cartography/models/entra/entra_user_to_aws_sso.py at line 27: <comment>Target node matcher relies solely on user_name, which is not guaranteed to be unique across Identity Stores; include identity_store_id in the matcher to avoid incorrect links (Based on your team's feedback about ensuring relationship matchers use stable, unique keys).</comment> <file context> @@ -0,0 +1,40 @@ +from dataclasses import dataclass + +from cartography.models.core.common import PropertyRef +from cartography.models.core.relationships import CartographyRelProperties +from cartography.models.core.relationships import CartographyRelSchema +from cartography.models.core.relationships import LinkDirection +from cartography.models.core.relationships import make_source_node_matcher +from cartography.models.core.relationships import make_target_node_matcher +from cartography.models.core.relationships import SourceNodeMatcher </file context>

✅ Addressed in 4c856f9

achantavy · 2025-08-07T00:01:52Z

cartography/intel/entra/applications.py

@@ -235,10 +245,48 @@ def transform_app_role_assignments(
    return result


+def transform_service_principals(
+    service_principals: list[ServicePrincipal],


@shyammukund this is what I was talking about at lunch: use list[SpecificType] wherever possible (like here with the Entra SDK because it's available to us. This way we can do things like spn.login_url instead of getattr(spn, 'login_url', None)

jychp

LGTM but:

I’m a bit uncomfortable having AWS-specific logic inside an Entra module.

It blurs the module boundaries and could become hard to maintain over time due to cross-dependencies. Especially since we don’t enforce any module execution order.

For example: if Entra runs before AWS, the behavior won’t be what users expect. This could be interpreted as a bug unless the user has deep knowledge of the implementation (i.e., actually read the code to understand the dependency).

I like the idea of:

standalone modules
cross-cutting modules (like analysis or ontology)

IMHO, we should eventually move this logic into a dedicated module (e.g., sso-federation, or another name I’m terrible at naming).

Note: since this is the only case for now, I’m fine merging it but we should open an issue right away to track this and clean it up later.

jychp · 2025-08-07T13:06:02Z

cartography/intel/entra/applications.py

 from typing import Any
-from typing import Dict


jychp · 2025-08-07T13:08:43Z

cartography/intel/entra/applications.py

@@ -313,14 +377,100 @@ def cleanup_app_role_assignments(
    ).run(neo4j_session)


+@timeit
+def cleanup_service_principals(


nitpicking: I think all the cleanup logic can be in a single function as they have the same purpose and same parameters

jychp · 2025-08-07T13:13:50Z

cartography/intel/entra/applications.py

-        neo4j_session, transformed_assignments, update_tag, tenant_id
-    )
+    # Attach sign-on relationships
+    sync_entra_to_aws_identity_center(neo4j_session, update_tag, tenant_id)


I don't really like that multiple sync_* functions have different behaviors:

sync_entra_applications: performs cleanup and orchestrates other syncs

sync_entra_apps_roles_spns and sync_entra_to_aws_identity_center: don't perform cleanup

For consistency, we should either:

move the cleanup logic into each sub-function

or (my preferred option) make the sub-functions private (_sync_*) to highlight the difference in responsibility

did the first option

Signed-off-by: Alex Chantavy <[email protected]>

achantavy · 2025-09-14T17:18:56Z

I've got this working on my local setup, going to test it e2e soon and then merge when that works

Signed-off-by: Alex Chantavy <[email protected]>

achantavy · 2025-09-14T21:36:29Z

Looks good, going to merge this in.

It's also possible to link to additional federated apps via the entra service principal's login url. The code can be extended there through the federation folder

achantavy · 2025-09-14T21:37:45Z

I’m a bit uncomfortable having AWS-specific logic inside an Entra module.

Created a federation/ folder to help keep these cross-cuts more organized

Signed-off-by: Alex Chantavy <[email protected]>

achantavy added 5 commits August 6, 2025 07:35

types

ef13077

Signed-off-by: Alex Chantavy <[email protected]>

sign on toazure to sign on to aws!

21b118c

Signed-off-by: Alex Chantavy <[email protected]>

docs

94f09d7

Signed-off-by: Alex Chantavy <[email protected]>

docs, tests, remove brittle unit tests

131804a

Signed-off-by: Alex Chantavy <[email protected]>

Merge branch 'master' into signintoazuretoaws

f2a6ea3

achantavy linked an issue Aug 6, 2025 that may be closed by this pull request

Sign in with Entra ID to AWS #1779

Closed

achantavy commented Aug 7, 2025

View reviewed changes

cubic-dev-ai bot reviewed Aug 7, 2025

View reviewed changes

achantavy commented Aug 7, 2025

View reviewed changes

achantavy changed the title ~~feat: EntraUser sign on to AWS Identity Center (closes #1779)~~ feat: EntraUser sign on to AWS Identity Center (#1779) Aug 7, 2025

jychp approved these changes Aug 7, 2025

View reviewed changes

achantavy added 5 commits September 14, 2025 02:05

merge

96251b8

Signed-off-by: Alex Chantavy <[email protected]>

make it make sense

817e079

Signed-off-by: Alex Chantavy <[email protected]>

fix tests

623d8f4

Signed-off-by: Alex Chantavy <[email protected]>

match uv.lock with master

e3a10cc

Signed-off-by: Alex Chantavy <[email protected]>

Merge branch 'master' into signintoazuretoaws

ea9d114

update readme

43ace00

Signed-off-by: Alex Chantavy <[email protected]>

cubic feedback

4c856f9

Signed-off-by: Alex Chantavy <[email protected]>

achantavy enabled auto-merge (squash) September 14, 2025 21:50

achantavy merged commit dd7fef9 into master Sep 14, 2025
5 of 8 checks passed

achantavy deleted the signintoazuretoaws branch September 14, 2025 21:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: EntraUser sign on to AWS Identity Center (#1779) #1787

feat: EntraUser sign on to AWS Identity Center (#1779) #1787

Uh oh!

achantavy commented Aug 6, 2025

Uh oh!

achantavy Aug 7, 2025

Uh oh!

cubic-dev-ai bot left a comment

Uh oh!

cubic-dev-ai bot Aug 7, 2025 •

edited

Loading

Uh oh!

cubic-dev-ai bot Aug 7, 2025 •

edited

Loading

Uh oh!

achantavy Aug 7, 2025

Uh oh!

jychp left a comment

Uh oh!

jychp Aug 7, 2025

Uh oh!

jychp Aug 7, 2025

Uh oh!

achantavy Sep 14, 2025

Uh oh!

jychp Aug 7, 2025

Uh oh!

achantavy Sep 14, 2025

Uh oh!

achantavy commented Sep 14, 2025

Uh oh!

achantavy commented Sep 14, 2025

Uh oh!

achantavy commented Sep 14, 2025

Uh oh!

Uh oh!

Uh oh!

	"https://d-90663f7df1.awsapps.com/start",
	"https://d-1234567890.awsapps.com/start",

feat: EntraUser sign on to AWS Identity Center (#1779) #1787

feat: EntraUser sign on to AWS Identity Center (#1779) #1787

Uh oh!

Conversation

achantavy commented Aug 6, 2025

Summary

Related issues or links

Checklist

Uh oh!

achantavy Aug 7, 2025

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

cubic analysis

Uh oh!

cubic-dev-ai bot Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

achantavy Aug 7, 2025

Choose a reason for hiding this comment

Uh oh!

jychp left a comment

Choose a reason for hiding this comment

Uh oh!

jychp Aug 7, 2025

Choose a reason for hiding this comment

Uh oh!

jychp Aug 7, 2025

Choose a reason for hiding this comment

Uh oh!

achantavy Sep 14, 2025

Choose a reason for hiding this comment

Uh oh!

jychp Aug 7, 2025

Choose a reason for hiding this comment

Uh oh!

achantavy Sep 14, 2025

Choose a reason for hiding this comment

Uh oh!

achantavy commented Sep 14, 2025

Uh oh!

achantavy commented Sep 14, 2025

Uh oh!

achantavy commented Sep 14, 2025

Uh oh!

Uh oh!

Uh oh!

cubic-dev-ai bot Aug 7, 2025 •

edited

Loading

cubic-dev-ai bot Aug 7, 2025 •

edited

Loading