Github "Direct" User Repo Access #1391

danbrauer · 2024-11-20T21:02:18Z

Summary

This PR adds to the Github graph, adding repo access that is granted to all users who have a 'direct' affiliation to the repo.

Cartography currently does already map some direct user repo access, but only for collaborators with an 'outside' affiliation to the repo. This PR broadens that to include all collaborators, aka anybody with a 'direct' affiliation. This follows Github's naming for these concepts, as seen here.

In case it is unclear or for people newer to Github, note: this is focusing on access users are granted directly to a repo, as opposed to via a team. Access granted via team is outside the scope of this PR.

We think this is a valuable addition to the graph for a few reasons, including:

Our analysts want few-to-no users to be granted access directly to repos, on the thinking that managing access via teams can make access easier to automate (ie with ABAC/RBAC type logic) and to audit. Graphing direct-access, regardless of whether a user is within the org or outside it, will help highlight who to clean up.
Longer term, we eventually want to know, from the graph, all access a user has. This PR is a step in that direction. (In a future PR, we hope to add a user-team membership relationship. Since Cartography maps team-repo access rel, we could then have a user-team-repo graph, and that would complete the picture of user access in Github.)

Illustration of the intention

Screencaps

A REPO WITH OUTSIDE COLLABORATORS

BEFORE

AFTER

A REPO WITH NON-OUTSIDE COLLABORATORS

BEFORE
(no results, because these sorts of users are not graphed)

AFTER

GENERAL COUNTS TO GIVE A SENSE OF CONNECTIONS NOW THERE

BEFORE

AFTER

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.

N/A/ If you are implementing a new intel module:

Use the NodeSchema data model.

danbrauer · 2024-11-20T21:02:53Z

This is partial and not yet working. Tests and docs are not yet updated, and I haven't reviewed my own code. I am sharing it early because I wanted to ask a couple of questions, which I will share in slack soon.

danbrauer · 2024-11-21T23:29:29Z

This should be ready for review. I do still intend to do a review of my own for typos and the like but short of stuff like that, I think this is ready.

danbrauer · 2024-11-22T14:29:33Z

cartography/intel/github/repos.py

+@dataclass(frozen=False)
+class UserAffiliationAndRepoPermission:
+    """
+    Representation of a user's permission level and affiliation to a GitHub repo. See:
+    - Permission: https://docs.github.com/en/graphql/reference/enums#repositorypermission
+    - Affiliation: https://docs.github.com/en/graphql/reference/enums#collaboratoraffiliation
+    """
+    user: Dict
+    permission: str  # WRITE, MAINTAIN, ADMIN, etc
+    affiliation: str  # OUTSIDE, DIRECT


I was led to creating this from a similar thing done in teams.py. There, a namedtuple is used. Here, I used a dataclass. I could go back to using a namedtuple if that'd be preferred?

Yeah let's use a namedtuple instead so that it matches

achantavy

This is great, thank you!

Left a minor note about changing the dataclass to a namedtuple to be consistent, but otherwise this is great.

achantavy · 2024-11-23T05:26:47Z

cartography/intel/github/repos.py

+@dataclass(frozen=False)
+class UserAffiliationAndRepoPermission:
+    """
+    Representation of a user's permission level and affiliation to a GitHub repo. See:
+    - Permission: https://docs.github.com/en/graphql/reference/enums#repositorypermission
+    - Affiliation: https://docs.github.com/en/graphql/reference/enums#collaboratoraffiliation
+    """
+    user: Dict
+    permission: str  # WRITE, MAINTAIN, ADMIN, etc
+    affiliation: str  # OUTSIDE, DIRECT


Yeah let's use a namedtuple instead so that it matches

Signed-off-by: Daniel Brauer <[email protected]>

danbrauer · 2024-11-24T03:36:19Z

Updated to named tuple as request. If all looks good, please merge. Otherwise, I'll be back on Tue and happy to work through whatever there is.

achantavy · 2024-11-25T05:00:10Z

Thank you!

Including a note for future searchers: we deferred adding the data model here. See this thread for details: https://cloud-native.slack.com/archives/C080M2LRLDA/p1732137038521919

### Summary This PR adds to the Github graph, adding repo access that is granted to all users who have a 'direct' affiliation to the repo. Cartography currently [does](https://cartography-cncf.github.io/cartography/modules/github/schema.html#id6) already map some direct user repo access, but only for collaborators with an 'outside' affiliation to the repo. This PR broadens that to include all collaborators, aka anybody with a 'direct' affiliation. This follows Github's naming for these concepts, as seen [here](https://docs.github.com/en/graphql/reference/enums#collaboratoraffiliation). In case it is unclear or for people newer to Github, note: this is focusing on access users are granted directly to a repo, as opposed to via a team. Access granted via team is outside the scope of this PR. We think this is a valuable addition to the graph for a few reasons, including: 1. Our analysts want few-to-no users to be granted access directly to repos, on the thinking that managing access via teams can make access easier to automate (ie with ABAC/RBAC type logic) and to audit. Graphing direct-access, regardless of whether a user is within the org or outside it, will help highlight who to clean up. 2. Longer term, we eventually want to know, from the graph, _all_ access a user has. This PR is a step in that direction. (In a future PR, we hope to add a user-team membership relationship. Since Cartography maps team-repo access rel, we could then have a user-team-repo graph, and that would complete the picture of user access in Github.) #### Illustration of the intention ![Cartography AMPS User Direct Repo Access (3)](https://github.com/user-attachments/assets/83a28a9b-f4f9-40fe-bdc5-153aa5196070) #### Screencaps **A REPO WITH OUTSIDE COLLABORATORS** BEFORE ![CollabsBefore](https://github.com/user-attachments/assets/6806fe08-5e7c-4ced-a8c8-ed43a39566c6) AFTER ![CollabsAfter](https://github.com/user-attachments/assets/18e9f96d-eb8e-4cad-984a-7e7056615776) **A REPO WITH NON-OUTSIDE COLLABORATORS** BEFORE (no results, because these sorts of users are not graphed) ![Screenshot 2024-11-21 at 5 54 37 PM](https://github.com/user-attachments/assets/d5b371ba-f86e-4a97-a06f-9f62c2548e76) AFTER ![Screenshot 2024-11-21 at 5 54 25 PM](https://github.com/user-attachments/assets/04941eb1-10ff-4ff9-a95d-172295744978) **GENERAL COUNTS TO GIVE A SENSE OF CONNECTIONS NOW THERE** BEFORE ![UserRepoRelsBefore](https://github.com/user-attachments/assets/08d2b96f-41ca-44b6-923d-75247ef09812) AFTER ![UserRepoRelsAfter](https://github.com/user-attachments/assets/85e3bf51-6a60-4742-865a-454bfab1ef24) ### Related issues or links None ### Checklist Provide proof that this works (this makes reviews move faster). Please perform one or more of the following: - [X] Update/add unit or integration tests. - [X] 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: - [X] Update the [schema](https://github.com/lyft/cartography/tree/master/docs/root/modules) and [readme](https://github.com/lyft/cartography/blob/master/docs/schema/README.md). **N/A/** If you are implementing a new intel module: - [ ] Use the NodeSchema [data model](https://cartography-cncf.github.io/cartography/dev/writing-intel-modules.html#defining-a-node). --------- Signed-off-by: Daniel Brauer <[email protected]> Signed-off-by: chandanchowdhury <[email protected]>

cmm-lyft · 2024-12-11T16:18:47Z

cartography/intel/github/repos.py

-def transform(repos_json: List[Dict]) -> Dict:
+def transform(
+    repos_json: List[Dict], direct_collaborators: dict[str, List[UserAffiliationAndRepoPermission]],
+    outside_collaborators: dict[str, List[UserAffiliationAndRepoPermission]],


@achantavy this resulted in a breaking change as it is changing the signature of the function without providing a default value nor allowing to pass None as param.
Should we update this code? or this should be enforced for next versions?

Is the breaking change in the Lyft internal deployment of cartography? If so share the stack trace with me internally. I thought I accounted for the changes; sorry, I probably missed something.

danbrauer marked this pull request as draft November 20, 2024 21:02

danbrauer force-pushed the gh_user_repo_relationships branch 2 times, most recently from 170dc34 to f94f00f Compare November 21, 2024 22:11

danbrauer marked this pull request as ready for review November 21, 2024 23:28

danbrauer commented Nov 22, 2024

View reviewed changes

achantavy approved these changes Nov 23, 2024

View reviewed changes

danbrauer added 7 commits November 23, 2024 22:31

initial partial impl, no tests yet

85b8827

Signed-off-by: Daniel Brauer <[email protected]>

repo collaborator testing

45cbef1

Signed-off-by: Daniel Brauer <[email protected]>

repos collaborators docs updates

e562255

Signed-off-by: Daniel Brauer <[email protected]>

cleanup job

7f6ef0b

Signed-off-by: Daniel Brauer <[email protected]>

collab test data shows that outside collabs are also direct collabs

49cabad

Signed-off-by: Daniel Brauer <[email protected]>

comments tweaks

4d14d74

Signed-off-by: Daniel Brauer <[email protected]>

dataclass changed to namedtuple

7be764f

Signed-off-by: Daniel Brauer <[email protected]>

danbrauer force-pushed the gh_user_repo_relationships branch from bbc72fa to 7be764f Compare November 24, 2024 03:33

achantavy merged commit 774d67b into cartography-cncf:master Nov 25, 2024
7 checks passed

cmm-lyft reviewed Dec 11, 2024

View reviewed changes

danbrauer mentioned this pull request Dec 12, 2024

nonetype handling for repo collaborators #1405

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Github "Direct" User Repo Access #1391

Github "Direct" User Repo Access #1391

danbrauer commented Nov 20, 2024 •

edited

Loading

danbrauer commented Nov 20, 2024 •

edited

Loading

danbrauer commented Nov 21, 2024

danbrauer Nov 22, 2024

achantavy Nov 23, 2024

achantavy left a comment

achantavy Nov 23, 2024

danbrauer commented Nov 24, 2024 •

edited

Loading

achantavy commented Nov 25, 2024

cmm-lyft Dec 11, 2024

achantavy Dec 11, 2024

Github "Direct" User Repo Access #1391

Github "Direct" User Repo Access #1391

Conversation

danbrauer commented Nov 20, 2024 • edited Loading

Summary

Illustration of the intention

Screencaps

Related issues or links

Checklist

danbrauer commented Nov 20, 2024 • edited Loading

danbrauer commented Nov 21, 2024

danbrauer Nov 22, 2024

Choose a reason for hiding this comment

achantavy Nov 23, 2024

Choose a reason for hiding this comment

achantavy left a comment

Choose a reason for hiding this comment

achantavy Nov 23, 2024

Choose a reason for hiding this comment

danbrauer commented Nov 24, 2024 • edited Loading

achantavy commented Nov 25, 2024

cmm-lyft Dec 11, 2024

Choose a reason for hiding this comment

achantavy Dec 11, 2024

Choose a reason for hiding this comment

danbrauer commented Nov 20, 2024 •

edited

Loading

danbrauer commented Nov 20, 2024 •

edited

Loading

danbrauer commented Nov 24, 2024 •

edited

Loading