Managing GitHub Access - Githubissues

shiragoodman commented 1 year ago

Problem Statement

VFS team members rely on Platform crew (through the Platform Orientation process) to add them to the vets.gov-write Github team so that they can get access to the common Github repositories used across VA.gov teams. The vets.gov-write github team was a carry over from the Vets.gov contract. It predates VSP, which has since been re-contracted as VA.gov Platform. It was decided that VSP, and then VA.gov Platform, would utilize vets.gov-write as the Platform github team. It was initially designed to support the read/write repo access that Platform teams need.

vets.gov-write gh team grants access to the following repositories within the VA Github Organization:

number	repo name	permissions
1	betamocks	read
2	breakers	read
3	content-build	write
4	covid19-chatbot	write
5	devops	write
6	gibct-data-service	write
7	performance-dashboard	write
8	platform-console-api	maintain
9	platform-pst-console-ui-configuration	write
10	react-jsonschema-form	write
11	va-adr	write
12	va.gov-cms-devops	read
13	va.gov-research-repository	write
14	va.gov-support-slackbot	write
15	va.gov-team	write
16	va.gov-team-sensitive	write
17	va.gov-vfs-teams	read
18	vagov-content	write
19	veteran-facing-services-tools	write
20	vets-api	write
21	vets-api-mockdata	write
22	vets-contrib	write
23	vets-json-schema	write
24	vets-website	write
25	vets.gov	write
26	vets.gov-team	read
27	vsp-infra-demo	read

There are several problems with this current process, including the following:

It puts the responsibility of managing repo access on Platform.
- The choice to include "Add VFS team member to vets.gov-write" was made by Mike Chelen in tickets https://github.com/department-of-veterans-affairs/va.gov-team/issues/35154 and https://github.com/department-of-veterans-affairs/va.gov-team/issues/39321 as a short-term solution to merge getting repo access with the Platform Orientation gh ticket.
It assumes that all VFS teams need the same level of access.
It assumes that all roles within VFS teams need the same level of access.
VFS teams and team members could be given access to repos that should be strictly permissible to Platform crew only.

Follow your problem description up with a "How might we... ___" statement re-framing that challenge as an opportunity. Don't hint too much at what the solution might be, you should have enough of a focal point here to guide your ideas, but plenty of freedom to think laterally and innovatively as you experiment and prototype later.

Hypothesis or Bet

How will this initiative impact the quality of VFS or Platform teams' work? How will this initiative be easy for VFS or Platform teams? Or how will it be easier than what they did before?

We will know we're done when... ("Definition of Done")

What requirements does this project need to meet for you to finish this initiative?

Known Blockers/Dependencies

List any blockers or dependencies for this work to be completed

Projected Launch Date

When do you expect to be completed rolling out this initiative*

OCTO Objective and OKR:

Objective: Our digital experiences are the best way to access VA health care and benefits. OKR All new products have a faster transaction time than those they replaced.

Launch Checklist

Guidance (delete before posting)

This checklist is intended to be used to help answer, "is my Platform initiative ready for launch?". All of the items in this checklist should be completed, with artifacts linked---or have a brief explanation of why they've been skipped---before launching a given Platforminitiative. All links or explanations can be provided in Required Artifacts sections. The items that can be skipped are marked as such.

Keep in mind the distinction between Product and Initiative --- each Product needs specific supporting documentation, but Initiatives to improve existing Products should reuse existing documentation for that Product. VSP Product Terminology for details.

Is this service / tool / feature...

... tested?

[ ] Usability test (TODO: link) has been performed, to validate that new changes enable users to do what was intended and that these changes don't worsen quality elsewhere. If usability test isn't relevant for this change, document the reason for skipping it.
- [ ] ... and issues discovered in usability testing have been addressed.
- Note on skipping: metrics that show the impact of before/after can be a substitute for usability testing.
[ ] End-to-end manual QA or UAT is complete, to validate there are no high-severity issues before launching
[ ] (if applicable) New functionality has thorough, automated tests running in CI/CD

... documented?

[ ] New documentation is written pursuant to our documentation style guide
[ ] Product is included in the List of VSP Products
- List the existing product that this initiative fits within, or add a new product to this list.
[ ] Internal-facing: there's a Product Outline
[ ] External-facing: a User Guide on Platform Website exists for this product/feature tool
[ ] (if applicable) Post to #vsp-service-design for external communication about this change (e.g. customer-facing meetings)

... measurable

[ ] (if applicable) This change has clearly-defined success metrics, with instrumentation of those analytics where possible, or a reason documented for skipping it.
- For help, see: Analytics team
[ ] This change has an accompanying Platform Release Plan.

When you're ready to launch...

[ ] Configure your communication to teams about your work using the Change Communication Guide
[ ] Conduct a go/no-go when you're almost ready to launch.

Required Artifacts

Documentation

PRODUCT_NAME: directory name used for your product documentation
Product Outline: link to Product Outline
User Guide: link to User Guide

Testing

Usability test: link to GitHub issue, or provide reason for skipping
Manual QA: link to GitHub issue or documented results
Automated tests: link to tests, or "N/A"

Measurement

Success metrics: link to where success metrics are measured, or where they're defined (Product Outline is OK), or provide reason for skipping
Release plan: link to Release Plan ticket

TODOs

[ ] Convert this issue to an epic
[ ] Add your team's label to this epic
[ ] Add this initiative to the Digital Experience Product Board using these instructions

shiragoodman commented 1 year ago

@BillChapmanUSDS @humancompanion-usds FYI this came up in the Governance roadmap planning. We need to discuss the level of access we are giving VFS and Platform team members.

andreahewitt-odd commented 8 months ago

I want to call out a couple things:

The write access also controls folks ability to edit and move tickets in Zenhub -- we need to discover what would happen if we changed the permissions for the repos in regards to that.
Anything that is mission critical to VA.gov has checks before code is pushed -- so a designer can make updates to the read me with no oversight but not to the actual code without automated and manual checks. I think before we make any changes, we'd want to do discovery about the the journey for code to go from a contributor to production for all the repos. Some have internal team review on top of Platform reviews.
Is this a security vulnerability that a security engineer/PO has defined or validate?

shiragoodman commented 8 months ago

@andreahewitt-odd please see my responses:

The write access also controls folks ability to edit and move tickets in Zenhub -- we need to discover what would happen if we changed the permissions for the repos in regards to that.

Interesting thought. I had assumed Zenhub permissions were separate?

Anything that is mission critical to VA.gov has checks before code is pushed -- so a designer can make updates to the read me with no oversight but not to the actual code without automated and manual checks. I think before we make any changes, we'd want to do discovery about the the journey for code to go from a contributor to production for all the repos. Some have internal team review on top of Platform reviews.

Fair points. I think it also would be valuable to evaluate the repos we manage through the vets.gov-write gh team. Many look like they're no longer in use.

Is this a security vulnerability that a security engineer/PO has defined or validate?

Chelen was previously involved. I have informed Matt Dingee too. If you're asking if a security SME or OCTO PO has actually declared this a problem we need to solve? No. Just little old me raising what looks like it could be a security concern.

andreahewitt-odd commented 7 months ago

@shiragoodman Thanks for the responses! Zenhub invitation is separate but write access is what drives Zenhub permissions too and I'm not sure what repos VFS teams use for Zenhub. I would definitely suggest more discovery around this because if we do it the wrong way, we could slow down VFS teams.

I would chat with our security team to help determine if it is a security vulnerability -- that will also help you all determine priority. Ken would be a good resource!

little-oddball commented 7 months ago

@andreahewitt-odd - one security argument would be "blanket" access to so many repos and folks should have access to the least amount possible. I believe one challenge here is with the evolution of the permissions, etc. untangling is gonna be a tedious climb.

patrickvinograd commented 7 months ago

Some other random comments:

There is a vets.gov-read team that has approximately the same set of repos listed but with read-only access. It's basically unused, only a few OCTO people are in it, but it could be an option for more limited access.
A key use case I'd like to advocate for simplifying is "running VA.gov stack locally" - while vets-api and vets-website are public repos, all the onboarding instructions instruct you to clone vets-api-mockdata in order to do anything useful with local development, and that repo is private, hence needing to go through platform orientation and get added to vets.gov-write to even clone it. Could we streamline the path to getting read access without doing all of platform orientation? Or could vets-api-mockdata be made read-accessible to all-va (i.e. org members?)
Unfortunately a GH team can only have one parent. I know some teams have done a good job segmenting their membership into "full team" vs. "developers" or even "frontend developers" vs. "backend developers" with a parent-child relationship between teams. This means only the parent team can be added to one of these meta-teams like vets.gov-write. It would be nice if you could add the full team for read access and issue management, and just add the developers to with code repo commit access, but I don't see a way to do that with nicely nested teams :/

patrickvinograd commented 7 months ago

(I don't need to be involved in any decision making just providing some extra input as I've thought about the best way to structure this and helped teams figure out why they do/don't have access to things)

kenmayo commented 7 months ago

There's an argument to be made that this is a vulnerability, since we're not really following the principles of least privilege, but the counter to that is 1) Is Github capable of handling a more complex privilege structure? and 2) Does the Platform team want to manage a more complex privilege structure?

If the answers to both questions are yes, then it's just a matter of deciding how we want to break up privileges and manage them. If either is no, then we accept the risk and move on.

shiragoodman commented 7 months ago

Thanks for your input, @kenmayo . I can't answer your 1st question, but I can make an attempt to answer the 2nd.

For some context, we haven't always done things this way... If you look at the tickets linked in the lower part of the Problem Statement (#35154 and #39321), you can get an idea of when things changed. When I first joined Platform team in 2020, the vets.gov-write team was used for individual Platform team members only. However, VFS teams were still managed under the Teams tab... ie VFS teams were children of the parent team vets.gov-write. This was nice because:

Platform crew wasn't responsible for adding individuals, only teams
VFS teams were accountable and responsible for managing their team members in Github
Possibly less support requests due to access issues since it's the responsibility of the VFS team to manage

I suppose that could be considered a more complex privilege structure. It's somewhat inline with what former Platform crew chief, Mike Chelen, had proposed in this comment.

@andreahewitt-odd I'll let you and Platform leadership make the call here. We're transitioning the tasks associated with granting vets.gov-write access over to @alyssagallion's team, Platform Support, so it seemed like a good time to raise these concerns, again. It sort of feels like we've been kicking the can down the road for a while now with this one, and it would be nice to have some closure.