←back to thread

312 points campuscodi | 4 comments | | HN request time: 0.634s | source
Show context
asmor ◴[] No.43375068[source]
GitHub's SAML implementation is useless. The idea is that you can bring your own account into an enterprise, and that sort of works on the site itself, but it does not prevent apps where you log in with GitHub from reading your organization membership once you have authorized an app on the organization level (and if you didn't, it hides the membership from oauth tokens, so it has this capability!).

A SAML session is only required if said app fetches data via a token obtained from that user - and in my glance around, this was almost never the case - SAST tools almost always use app instance tokens and are happy to show anyone with a GitHub account in your organization your code. Tailscale fixed this when I pointed it out, Sonarcloud told me to please don't tell anyone and GitHub took a few weeks to say this is totally expected behavior - when no vendor I told did, and their docs contradicted them.

I swear, reporting security bugs is a thankless endeavor, even if you just randomly stumble over them. I couldn't imagine doing this as a job.

replies(6): >>43375206 #>>43375506 #>>43375716 #>>43375938 #>>43377351 #>>43377358 #
peterldowns ◴[] No.43375938[source]
To be fair to the vendors, Github makes it extremely difficult to do the right thing here. I built a repo/commit/pr-analysis tool (https://dev.log.xyz) and it took a lot of effort to make it so that "iff you can see it in Github you can see it in Devlog." The entire experience was beyond frustrating.

Github also makes their OAuth permissions picker extremely confusing. When I "login with Github" I am never sure exactly what I'm sharing, from which organizations I'm a member of.

replies(2): >>43377681 #>>43377682 #
1. wutwutwat ◴[] No.43377682[source]
> Github makes it extremely difficult to do the right thing here ... it took a lot of effort to make it so that "iff you can see it in Github you can see it in Devlog." The entire experience was beyond frustrating.

Do they? You don't have to mess with syncing teams, memberships, or assignment to repos if you don't want to. You can make one api call:

> The authenticated user has explicit permission to access repositories they own, repositories where they are a collaborator, and repositories that they can access through an organization membership.

https://docs.github.com/en/rest/repos/repos?apiVersion=2022-...

replies(2): >>43377720 #>>43381082 #
2. asmor ◴[] No.43377720[source]
I should've tested this endpoint. GitHub's SAML implementation is done by a different team, always lags behind in quality and does some pretty unclean patching of the data - i.e. the notification filtering is done in the templating engine, so if all your notifications are SAML gated you get the header, no "all caught up" below it and (this is live from my account) "1-0 of 113".

So I'd give it about a 50:50 chance of working.

Edit: I just realized it eats your non-gated notifications too, if they're further down than position 25, and the "Next" button just leads to the same page with "?query=". Yay, another ticket about how glued on GitHub Enterprise Cloud is. The last one (GitHub eats API calls to accept invites to SAML organizations, deletes the invite, and sends a 200, writes success to the audit log... but ends up being a no-op) only has been 2 months or so ago. Thanks Microsoft.

replies(1): >>43420646 #
3. peterldowns ◴[] No.43381082[source]
Yes, they do make it difficult. Keeping an updated set of {(user, repo, capabilities)} is necessary for correctly implementing "iff you can see it in Github you can see it in Devlog". Polling this REST endpoint user-by-user doesn't really work — both architecturally (are you going to poll this endpoint? how frequently?) and pragmatically (the REST API is both much slower and much less stable than the GraphQL API, and pagination works poorly.)

I ended up using the GraphQL API and making a query like this:

    query($cursor0: String, $cursor1: String) {
      search(query:"org:peterldowns", type: REPOSITORY, first: 100, after: $cursor0) {
       pageInfo {
        hasNextPage
        endCursor
       }
       repositories: nodes {
        ... on Repository {
         id
         name
         collaborators(first: 100, after: $cursor1) {
          pageInfo {
           hasNextPage
           endCursor
          }
          edges {
           permission
           node {
            login
            id
           }
          }
         }
        }
       }
      }
     }

Removing permissions that are no longer present in this result set is left as an exercise for the reader.

I stopped working on the product so I never implemented the event stream consumer that would let me listen for "this user was removed as a collaborator" or "this team no longer has admin access to that repo". The entire permissioning model for Github is extremely complex and learning about all of its intricacies was half the battle.

4. asmor ◴[] No.43420646[source]
And the results are in.

"Our engineering team has indicated they will not be able to fully dive in and implement any resolutions in the near-term."

Considering they didn't in the past 7 years, I have little hope for the next 7.