Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Users are unable to access internal Helix builds #8554

Closed
ChadNedzlek opened this issue Mar 4, 2022 · 15 comments
Closed

Users are unable to access internal Helix builds #8554

ChadNedzlek opened this issue Mar 4, 2022 · 15 comments
Labels
Needs Triage A new issue that needs to be associated with an epic.

Comments

@ChadNedzlek
Copy link
Member

We uninstalled Helix from the Microsoft GitHub org. Unfortunately, access to this org was what contolled access to helix.

Now no one can access things in helix unless we happen to have their credentials cached.

This is critical, and we need to do something. Unfortunateyl, installing it back there will give @adiaaida's projec tool as well as the Build Analysis tool a bunch of webhooks and access we don't want. This is a pickle.

/CC all these people @alexperovich @MattGal @ilyas1974 @Chrisboh @markwilkie @missymessa @mmitche

Maybe we can add the "read:orgs" scope to our OAuth request? Unfortunately, this hasn't changed in years, so I'm not 100% what the "cheapest" action we can take is.

@ChadNedzlek
Copy link
Member Author

It's possible adding read:org here https://github.com/dotnet/arcade-services/blob/main/src/Shared/Microsoft.DotNet.Web.Authentication/GitHub/GitHubAuthenticationExtensions.cs#L30, with a call to options.Scopes.Add("read:org") maybe? That's going to be a pain to get shipped to prod, since it's not even in the helix repo.

@ChadNedzlek
Copy link
Member Author

That might also not work... it's hard to test this... but I know the tokens we are getting now have no scopes, and if you ask for org membership, it only currently lists the orgs the app is installed on. It's also possible that oauth scope might not change anything, and we still need to get isntalled.

In that case, we need to split our app in twain... one app with almost no permissions, just for authenticating with Helix to be insalled in all the orgs we care about. Then another app that actually does the work that can be installed on a per repo basis.

@ChadNedzlek
Copy link
Member Author

Scratch the "scope" stuff, looks like that doesn't work with github apps, only OAuth ones. Looks like the hard path is the only path. Rats.

@akoeplinger
Copy link
Member

akoeplinger commented Mar 4, 2022

I'm a bit confused why it wouldn't be able to retrieve public members? Which endpoint are you using?

According to the public_members API docs it should work without the app being part of the org, as opposed to the members API which says says you get a 302 if the requester is not an organization member.

@MattGal
Copy link
Member

MattGal commented Mar 4, 2022

Remind me why we don't just put it back until this is sorted?

@markwilkie
Copy link
Member

This is my vote - I'll chime in on the email thread and ask Jeff to put things back for now.

@ChadNedzlek
Copy link
Member Author

@akoeplinger , unfortunately that doesn't play well with how ASP.NET does identity. We could hard code a bunch of checks inside I suppose, but right now, we just gather all the orgs/teams and make them claims on the principal. Then various pages/sites can auth based on those claims. So at the time we are asking what orgs you are in, we don't know what orgs matter. We could hard code a list, I suppose, but that's also a lot more web requests to make for each user, and we've already seen throttling in this are.

Also, it doesn't work: "message": "Resource protected by organization SAML enforcement. You must grant your OAuth token access to this organization.",

@ChadNedzlek
Copy link
Member Author

We've reinstalled the app to the "dotnet-features" repo in the MS org, which seems to have given us access to org membership again. That repo hasn't seen a commit in 8 years, so we are probably safe... until someone deletes it and everything is broken again, but we are unblocked for now.

@ilyas1974
Copy link
Contributor

@ChadNedzlek is there further action we should be taking here?

@ChadNedzlek
Copy link
Member Author

I don't think our current place is a sustainable one. We probably need to split up the helix app into 3 smaller apps.

  • One for the authentication of helix.dot.net that can be installed in the repositories... needs only org:read permissions
  • One for build analysis stuff that will trigger checks for PRs
  • One for internal tooling stuff, like the projects board @adiaaida

We got rid of the problem by doing something very hacky, but it's quite fragile and undocumented what we did.

@ilyas1974 ilyas1974 added Needs Triage A new issue that needs to be associated with an epic. and removed Critical Ops - First Responder labels Mar 18, 2022
@MattGal
Copy link
Member

MattGal commented Mar 21, 2022

@ChadNedzlek any reason we couldn't stay in this "fragile" state until we manage to make the "be a public member of the Microsoft GitHub org" part be mandatory, documented, and communicated to users? After that we wouldn't need to have any of the funky hacks here, just it needs to not be a surprise. I'm unaware of any downsides other than the requirement for a communication campaign to get everyone who uses Helix directly to update their profile.

@ChadNedzlek
Copy link
Member Author

The public part doesn't matter, I am a public member, and the API wasn't recognizing me as a MS member. If we aren't installed, we don't seem to get any MS membership at all.

@MattGal
Copy link
Member

MattGal commented Mar 21, 2022

The public part doesn't matter, I am a public member, and the API wasn't recognizing me as a MS member. If we aren't installed, we don't seem to get any MS membership at all.

I am pretty sure (and can do a demonstration ; you can too using darc update-subscription and toggling your public member-ness) that that's exactly what this code does. What am I missing? https://github.com/dotnet/arcade-services/blob/main/src/Maestro/Maestro.Web/Api/v2020_02_20/Controllers/SubscriptionsController.cs#L230-L244

@ChadNedzlek
Copy link
Member Author

That code appears to be doing something similar to how helix works. But based on my testing during the day where everything was broken, Organization.GetAllForUser only returns organizations where the application is installed. When I used the SAME token, when it wasn't installed, Microsoft was not in the list for me, and then after the workaround, Microsoft started appearing again. I don't think the public/private matters there for "GitHub Apps" (as opposed to OAuth apps, which have a whole other model)

@alexperovich
Copy link
Member

This issue is fixed. I opened #8708 to track a more long term shift in how we do this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Triage A new issue that needs to be associated with an epic.
Projects
None yet
Development

No branches or pull requests

6 participants