Add org permissions fetcher #53

mic67mel · 2021-02-10T13:11:00Z

[ x ] Tick to sign-off your agreement to the Developer Certificate of Origin (DCO) 1.1

What

Add a fetcher that will gather forks list for each repo of a Github organization.

Why

This fetcher is needed to control access to source code.

How

fetch_org_permissions.py
README.md

Test

N/A

Context

New feature: GitHub organization, repo collaborators, forks and teams fetcher #52

mstancamp · 2021-02-10T15:54:30Z

arboretum/permissions/fetchers/github/fetch_repo_forks.py

@@ -0,0 +1,61 @@
+# -*- mode:python; coding:utf-8 -*-
+# Copyright (c) 2020 IBM Corp. All rights reserved.


Copyright should be 2021.

alfinkel

Pausing... I think #53 (comment) needs resolution before we proceed.

alfinkel · 2021-02-15T18:00:54Z

CHANGES.md

+# [0.10.0](https://github.com/ComplianceAsCode/auditree-arboretum/releases/tag/v0.10.0)
+
+- [ADDED] Added Github org forks 


Rebase and change this to 0.11.0.

Add punctuation.

Suggested change

# [0.10.0](https://github.com/ComplianceAsCode/auditree-arboretum/releases/tag/v0.10.0)

- [ADDED] Added Github org forks

# [0.11.0](https://github.com/ComplianceAsCode/auditree-arboretum/releases/tag/v0.11.0)

- [ADDED] GitHub org forks fetcher added to `Permissions`.

alfinkel · 2021-02-15T18:01:07Z

arboretum/__init__.py

@@ -14,4 +14,4 @@
 # limitations under the License.
 """Arboretum - Checking your compliance & security posture, continuously."""

-__version__ = '0.9.0'
+__version__ = '0.10.0'


alfinkel · 2021-02-15T18:04:45Z

arboretum/permissions/README.md

@@ -95,6 +95,78 @@ day.
  from arboretum.permissions.fetchers.github.fetch_org_collaborators import GithubOrgCollaboratorsFetcher
  ```

+### Organization Integrity (Repository Forks)
+
+* Class: [GithubRepoForksFetcher][gh-org-fetcher]


This isn't the gh-org-fetcher. You should provide a proper link and reference it here correctly.

Also, it seems like it would make sense to rename the class as GithubOrgForksFetcher? Suggest renaming the class and changing the link ref for gh-org-fetcher to gh-org-collaborators-fetcher and adding a gh-org-forks-fetcher link reference to button it all up.

alfinkel · 2021-02-15T18:07:47Z

arboretum/permissions/README.md

+
+* Class: [GithubRepoForksFetcher][gh-org-fetcher]
+* Purpose: Writes the repository forks in Github organizations to the evidence locker. This fetcher class is only meant for use with Github or Github Enterprise organizations.
+* Behavior: For each Github organization specified, an evidence file  is stored in


Suggested change

* Behavior: For each Github organization specified, an evidence file is stored in

* Behavior: For each Github organization specified, an evidence file is stored in

alfinkel · 2021-02-15T18:17:02Z

arboretum/permissions/README.md

+  * `org.permissions.org_integrity.orgs`
+     * Required
+     * List of dictionaries each containing Github organization retrieval configuration.
+        * `url`
+           * Required
+           * String in the form of `"https://github.com/my-org"` or `"https://github.<company>.com/my-org"`.
+           * Use to define the Github organization url to use.
+        * `repos`
+           * Optional
+           * List of strings in the form of `["my-repo", "my-other-repo"]`.
+           * Defaults to all repositories in the organization.
+           * Use if looking to filter collaborator evidence to a subset of repositories in the organization otherwise do not include.


By doing this you're tying the collaborators fetcher and the forks fetcher together. If someone chooses to run both of these fetchers in their environment then they're forced to run them for the exact same set of orgs and repos. Is that the desired behavior?

alfinkel

Just some cosmetic refactoring needed. Feel free to squash commits down to one as part of the next review cycle. I think we're almost GTG here.

alfinkel · 2021-02-25T12:36:37Z

arboretum/permissions/README.md

@@ -16,14 +16,12 @@ how to include the fetchers and checks from this library in your downstream proj

 ## Fetchers

-### Organization Integrity (Repository Collaborators)
+### Organization Integrity (Repository Collaborators and Forks)


Suggested change

### Organization Integrity (Repository Collaborators and Forks)

### Organization Integrity Permissions

alfinkel · 2021-02-25T12:44:21Z

arboretum/permissions/README.md

+* Behavior: For each Github organization specified an evidence file per collaborator type (affiliation) and an evidence file with the list of repository forks are stored in
+the locker. The default is to retrieve all collaborators by affiliation and all forks from all repositories in each specified Github organization.  TTL is set to 1 day.


Suggested change

* Behavior: For each Github organization specified an evidence file per collaborator type (affiliation) and an evidence file with the list of repository forks are stored in

the locker. The default is to retrieve all collaborators by affiliation and all forks from all repositories in each specified Github organization. TTL is set to 1 day.

* Behavior: For each Github organization specified, Github collaborator and Github fork evidence files per collaborator type (affiliation) are stored in

the locker containing details for the specified repositories in the organization. The default is to

retrieve all collaborators and all forks by affiliation from all repositories in each specified Github organization. TTL is set to 1 day.

alfinkel · 2021-02-25T12:44:50Z

arboretum/permissions/README.md

@@ -36,7 +34,7 @@ day.
           * Optional
           * List of strings in the form of `["my-repo", "my-other-repo"]`.
           * Defaults to all repositories in the organization.
-           * Use if looking to filter collaborator evidence to a subset of repositories in the organization otherwise do not include.
+           * Use if looking to filter collaborator and fork evidence to a subset of repositories in the organization otherwise do not include.


Suggested change

* Use if looking to filter collaborator and fork evidence to a subset of repositories in the organization otherwise do not include.

* Use if looking to filter permissions evidence to a subset of repositories in the organization otherwise do not include.

alfinkel · 2021-02-25T12:46:26Z

arboretum/permissions/README.md

-[repository-permissions]: https://docs.github.com/en/free-pro-team@latest/github/setting-up-and-managing-organizations-and-teams/repository-permission-levels-for-an-organization
-[org-collaborators-check]: https://github.com/ComplianceAsCode/auditree-arboretum/blob/main/arboretum/permissions/checks/test_org_collaborators.py
+[fetch-org-permissions]: https://github.com/ComplianceAsCode/auditree-arboretum/blob/main/arboretum/permissions/fetchers/github/fetch_org_permissions.py
+[repository-permissions]: https://docs.github.com/en/free-pro-team@latest/github/setting-up-and-managing-organizations-and-teams/repository-permission-levels-for-an-organization[org-collaborators-check]: https://github.com/ComplianceAsCode/auditree-arboretum/blob/main/arboretum/permissions/checks/test_org_collaborators.py


repository-permissions doesn't seem right here. I think you're breaking the link.

alfinkel · 2021-02-25T12:50:21Z

arboretum/permissions/fetchers/github/fetch_org_permissions.py

+
+
+class GithubOrgPermissionsFetcher(collabs.GithubOrgCollaboratorsFetcher):
+    """Fetch Github repository colaborators and forks."""


Suggested change

"""Fetch Github repository colaborators and forks."""

"""Fetch Github repository permissions evidence."""

BTW you should turn auto spell check on in your IDE or editor (collaborators is misspelled). It would make the review go smoother.

alfinkel · 2021-02-25T12:53:11Z

arboretum/permissions/fetchers/github/fetch_org_permissions.py

+    @classmethod
+    def setUpClass(cls):
+        """Initialize the fetcher object with configuration settings."""
+        super(GithubOrgPermissionsFetcher, cls).setUpClass()
+        cls.gh_pool = {}
+        return cls


Remove this. You're just replicating the parent class method and running it twice.

alfinkel · 2021-02-25T12:57:52Z

arboretum/permissions/fetchers/github/fetch_org_permissions.py

+from compliance.evidence import DAY, RawEvidence, raw_evidence
+from compliance.utils.data_parse import get_sha256_hash
+from compliance.utils.services.github import Github


Since you've imported the entire fetch_orgs_collaborators module as collabs you don't really need to reimport these. Instead you should be able to just reference them in the code via collabs like collabs.DAY, collabs.Github, etc...

alfinkel · 2021-02-25T13:03:22Z

arboretum/permissions/fetchers/github/fetch_org_permissions.py

+        """Fetch Github repository forks."""
+        for config in self.config.get('org.permissions.org_integrity.orgs'):
+            host, org = config['url'].rsplit('/', 1)
+


Can you explain the purpose of having this empty line? If not then I suggest removing it.

alfinkel · 2021-02-25T13:05:09Z

arboretum/permissions/fetchers/github/fetch_org_permissions.py

+            json_file = f'gh_forks_{url_hash}.json'
+            path = ['permissions', json_file]


Suggested change

json_file = f'gh_forks_{url_hash}.json'

path = ['permissions', json_file]

path = ['permissions', f'gh_forks_{url_hash}.json']

alfinkel · 2021-02-25T13:10:18Z

arboretum/permissions/fetchers/github/fetch_org_permissions.py

+                        forks_url = f'repos/{org}/{repo}/forks'
+                        forks[repo] = self.gh_pool[host].paginate_api(
+                            forks_url
+                        )


Suggested change

forks_url = f'repos/{org}/{repo}/forks'

forks[repo] = self.gh_pool[host].paginate_api(

forks_url

)

forks[repo] = self.gh_pool[host].paginate_api(

f'repos/{org}/{repo}/forks'

)

alfinkel

Can you add a deprecated decorator with a reason (similar to the entry in the CHANGES.md) to the org integrity collabs fetcher class? The deprecated module is already being brought into arboretum through the framework.

ref: https://github.com/tantale/deprecated#usage

see here for framework example.

I think once you do that we should be good to go.

alfinkel · 2021-03-01T17:07:35Z

CHANGES.md

@@ -1,3 +1,7 @@
+# [0.11.0](https://github.com/ComplianceAsCode/auditree-arboretum/releases/tag/v0.11.0)
+
+- [ADDED] Organization repository permissions fetcher added to `permissions`.


Can you add more detail to your [ADDED] entry and add a [DEPRECATED] entry for the org repo collabs fetcher and state that its functionality is now part of the permissions fetcher? Something like:

[ADDED] Github org integrity fetcher added to `permissions`. [ADDED] Github org integrity forks fetcher functionality added Github org integrity fetcher. [DEPRECATED] Github org integrity collaborators fetcher functionality moved to Github org integrity fetcher.

alfinkel · 2021-03-01T17:12:38Z

arboretum/permissions/fetchers/github/fetch_org_permissions.py

+            host, org = config['url'].rsplit('/', 1)
+            url_hash = collabs.get_sha256_hash([config['url']], 10)
+            path = ['permissions', f'gh_forks_{url_hash}.json']
+            description = (f'Fork of the {org} GH org')


This needs a bit of clarification and the parentheses aren't needed. Suggest:

Suggested change

description = (f'Fork of the {org} GH org')

description = f'Forks of repos in the {org} GH org'

alfinkel

If not for my dumb suggestions in the last review this would be good to go. Make the few tweaks I mentioned below and we can merge. Thanks!

alfinkel · 2021-03-02T13:40:38Z

CHANGES.md

+
+- [ADDED] Github org integrity fetcher added to `permissions`.
+- [ADDED] Github org integrity forks fetcher functionality added Github org integrity fetcher.
+- [DEPRECATED] Github org integrity collaborators fetcher functionality moved to Github org integrity permissions fetcher.


Suggested change

- [DEPRECATED] Github org integrity collaborators fetcher functionality moved to Github org integrity permissions fetcher.

- [CHANGED] Github org integrity collaborators fetcher functionality added to Github org integrity permissions fetcher.

alfinkel · 2021-03-02T13:42:05Z

arboretum/permissions/fetchers/github/fetch_org_collaborators.py

+from deprecated import deprecated

+
+@deprecated(
+    reason=(
+        'Github org integrity collaborators fetcher functionality'
+        'moved to Github org integrity permissions fetcher'
+    )
+)


Let's remove all of this. We shouldn't deprecate the fetcher since it is the parent class for the new org integrity fetcher. Sorry for causing the unnecessary thrash here.

alfinkel

+1

mic67mel requested review from alfinkel, cletomartin and drsm79 as code owners February 10, 2021 13:11

mic67mel changed the title ~~Forks fetcher~~ Add forks fetcher Feb 10, 2021

mstancamp reviewed Feb 10, 2021

View reviewed changes

alfinkel requested changes Feb 15, 2021

View reviewed changes

mic67mel changed the title ~~Add forks fetcher~~ Add org permissions fetcher Feb 17, 2021

mic67mel force-pushed the add-ghe-forks-fetcher branch from 9869996 to 08f2c87 Compare February 17, 2021 18:39

alfinkel requested changes Feb 25, 2021

View reviewed changes

mic67mel force-pushed the add-ghe-forks-fetcher branch 2 times, most recently from 02ded94 to 95c7b01 Compare March 1, 2021 16:44

alfinkel requested changes Mar 1, 2021

View reviewed changes

mic67mel force-pushed the add-ghe-forks-fetcher branch from 6b1719a to 6658634 Compare March 2, 2021 12:09

alfinkel requested changes Mar 2, 2021

View reviewed changes

mic67mel force-pushed the add-ghe-forks-fetcher branch from ce75557 to c8d9105 Compare March 2, 2021 14:35

Add permissions fetcher

e2d0319

mic67mel force-pushed the add-ghe-forks-fetcher branch from a67f914 to e2d0319 Compare March 2, 2021 15:03

alfinkel approved these changes Mar 2, 2021

View reviewed changes

alfinkel merged commit 5d63dd0 into ComplianceAsCode:main Mar 2, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add org permissions fetcher #53

Add org permissions fetcher #53

mic67mel commented Feb 10, 2021 •

edited

Loading

mstancamp Feb 10, 2021

mic67mel Feb 10, 2021

alfinkel left a comment •

edited

Loading

alfinkel Feb 15, 2021

alfinkel Feb 15, 2021

alfinkel Feb 15, 2021

alfinkel Feb 15, 2021

alfinkel Feb 15, 2021

alfinkel left a comment

alfinkel Feb 25, 2021

alfinkel Feb 25, 2021

alfinkel Feb 25, 2021

alfinkel Feb 25, 2021

alfinkel Feb 25, 2021

alfinkel Feb 25, 2021

alfinkel Feb 25, 2021

alfinkel Feb 25, 2021

alfinkel Feb 25, 2021

alfinkel Feb 25, 2021

alfinkel left a comment

alfinkel Mar 1, 2021

alfinkel Mar 1, 2021

alfinkel left a comment

alfinkel Mar 2, 2021

alfinkel Mar 2, 2021

alfinkel left a comment

		@@ -0,0 +1,61 @@
		# -- mode:python; coding:utf-8 --
		# Copyright (c) 2020 IBM Corp. All rights reserved.

		# [0.10.0](https://github.com/ComplianceAsCode/auditree-arboretum/releases/tag/v0.10.0)

		- [ADDED] Added Github org forks

	* Behavior: For each Github organization specified, an evidence file is stored in
	* Behavior: For each Github organization specified, an evidence file is stored in

	### Organization Integrity (Repository Collaborators and Forks)
	### Organization Integrity Permissions

		* Behavior: For each Github organization specified an evidence file per collaborator type (affiliation) and an evidence file with the list of repository forks are stored in
		the locker. The default is to retrieve all collaborators by affiliation and all forks from all repositories in each specified Github organization. TTL is set to 1 day.

-* Behavior: For each Github organization specified an evidence file per collaborator type (affiliation) and an evidence file with the list of repository forks are stored in
-the locker. The default is to retrieve all collaborators by affiliation and all forks from all repositories in each specified Github organization.  TTL is set to 1 day.
+* Behavior: For each Github organization specified, Github collaborator and Github fork evidence files per collaborator type (affiliation) are stored in
+the locker containing details for the specified repositories in the organization. The default is to
+retrieve all collaborators and all forks by affiliation from all repositories in each specified Github organization.  TTL is set to 1 day.

	* Use if looking to filter collaborator and fork evidence to a subset of repositories in the organization otherwise do not include.
	* Use if looking to filter permissions evidence to a subset of repositories in the organization otherwise do not include.



		class GithubOrgPermissionsFetcher(collabs.GithubOrgCollaboratorsFetcher):
		"""Fetch Github repository colaborators and forks."""

	"""Fetch Github repository colaborators and forks."""
	"""Fetch Github repository permissions evidence."""

		json_file = f'gh_forks_{url_hash}.json'
		path = ['permissions', json_file]

	json_file = f'gh_forks_{url_hash}.json'
	path = ['permissions', json_file]
	path = ['permissions', f'gh_forks_{url_hash}.json']

		@@ -1,3 +1,7 @@
		# [0.11.0](https://github.com/ComplianceAsCode/auditree-arboretum/releases/tag/v0.11.0)

		- [ADDED] Organization repository permissions fetcher added to `permissions`.

	description = (f'Fork of the {org} GH org')
	description = f'Forks of repos in the {org} GH org'

	- [DEPRECATED] Github org integrity collaborators fetcher functionality moved to Github org integrity permissions fetcher.
	- [CHANGED] Github org integrity collaborators fetcher functionality added to Github org integrity permissions fetcher.

Add org permissions fetcher #53

Add org permissions fetcher #53

Conversation

mic67mel commented Feb 10, 2021 • edited Loading

What

Why

How

Test

Context

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alfinkel left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alfinkel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alfinkel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alfinkel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alfinkel left a comment

Choose a reason for hiding this comment

mic67mel commented Feb 10, 2021 •

edited

Loading

alfinkel left a comment •

edited

Loading