Add team membership check to GitHub authentication method

[thepabloaguilar]

Problem

I want to deploy Flipt with GitHub auth enabled but I want to define that only people in specific teams can access it.
Example: Only people in my-organization and in my-flag-team can access the service!

Ideal Solution

Extend the current GitHub authentication method to support GitHub Org teams filter! With what I know so far we can continue relaying in read:org scope which theorically can also read the org team membership:

Read-only access to organization membership, organization projects, and team membership.

If the user don't pass allowed_teams (which maybe can have a better name if needed) the behavior will be same as now but if that config is provided we should match the user team!

My suggestion for allowed_teams values is to adopt the pattern ORG:TEAM, so if the user provide two orgs that has two teams with the same name they can choose which team should be considered to each org:

authentication:
  methods:
    github:
      enabled: true
      scopes:
        - read:org
      allowed_organizations:
        - my-org
        - my-other-org
      allowed_teams:
        - my-org:my-team

Search

• I searched for other open and closed issues before opening this

Additional Context

Refs #2065

[thepabloaguilar]

If y'all think it's a good idea I can get it done!

[markphelps]

@thepabloaguilar I like it alot! Great idea.

Im guessing in your example yaml above you meant allowed_teams for the last section?

One question, would you imagine they would have to have the org in aallowed_organizations as well? For example:

authentication:
  methods:
    github:
      enabled: true
      scopes:
        - read:org
      allowed_organizations:
        - my-org
        - my-other-org
      allowed_teams:
        - foo-org:my-team

^ this would be invalid because foo-org is not in allowed_organizations?

[thepabloaguilar]

Im guessing in your example yaml above you meant allowed_teams for the last section?

Yeah, I've forgot to change it 😆

About your question about that case I'd say it's an invalid example, a great description of the allowed_teams would be:

Allowed teams to access Flipt inside your allowed organizations

Or we can change allowed_organizations to accept and treat different depending the item shape, like, the users could declare:

allowed_organizations:
  - my-org // Filter only by the org
  - my-other-org:my-team // Filter by te org and team

Or even changing the config shape to something like this (but we'll be doing a possible breaking change):

allowed_organizations:
  - name: my-org
  - name: my-other-org
    teams:
      - my-team

But idk if it's a great idea

[markphelps]

Im guessing in your example yaml above you meant allowed_teams for the last section?

Yeah, I've forgot to change it 😆

About your question about that case I'd say it's an invalid example, a great description of the allowed_teams would be:

Allowed teams to access Flipt inside your allowed organizations

Or we can change allowed_organizations to accept and treat different depending the item shape, like, the users could declare:

allowed_organizations:
  - my-org // Filter only by the org
  - my-other-org:my-team // Filter by te org and team

Or even changing the config shape to something like this (but we'll be doing a possible breaking change):

allowed_organizations:
  - name: my-org
  - name: my-other-org
    teams:
      - my-team

But idk if it's a great idea

yeah I think for simplicity and to maintain backward compat that your first example config is the best, having separate keys for allowed_organizations and allowed_teams. And I agree that it would be an error if the org prefix in allowed_teams wasn't in the allowed_organizations list also

[thepabloaguilar]

Great then, I'll try to implement it!

[thepabloaguilar]

We have a "problem", GitHub API only allows us to get all the user teams without filtering by organizations! What I'll to is set the per_page number to be 50 which I consider a "safe" value, it'll be hard an user being in a 50 different teams across organizations!

Ref: List teams for the authenticated user

[markphelps]

Yeah thats unfortunate that we have to get team membership that way, but I think its ok. As you said I doubt there are many users who are a member of that many teams.

I looked at how Grafana does it as they also allow specifying orgs and team membership enforcement for GitHub OAuth. They do the same (albeit their max is 100 which I think would also be fine) https://github.com/grafana/grafana/blob/c0933fa6bb4038c519466d12bd2e61693e32a4e1/pkg/login/social/connectors/github_oauth.go#L207

Another option (maybe, haven't validated it) is using GitHub's GraphQL API. I very much dislike GraphQL personally, but maybe it makes sense here?

ChatGPT said we could do it with the following query:

query IsMemberOfTeam($organizationLogin: String!, $teamSlug: String!) {
  organization(login: $organizationLogin) {
    team(slug: $teamSlug) {
      members {
        edges {
          node {
            login
          }
        }
      }
    }
  }
}

vars:

{
  "organizationLogin": "my-org",
  "teamSlug": "my-team"
}

But I haven't validated it.

Either way I think either solution would work for our needs:

1. getting the N team memberships for a given user (either 50 or 100) and then looping through to see if we have a match
2. or trying to go the more 'direct' approach via GraphQL (if it works)

wdty?

[thepabloaguilar]

Moving to graphql requires more changes in the server side while maintaining two different http clients (rest & graphql)!

I vote to continue calling the REST API because of simplicity (already works, don't need to add extra libs) and should be better making one call returning N teams instead calling the GraphQL query N times (where N is the number of allowed_teams)!

Now, about the structure we can also consider this:

allowed_teams:
  - org: foo-org
    teams:
      - my-team

[markphelps]

Moving to graphql requires more changes in the server side while maintaining two different http clients (rest & graphql)!

I vote to continue calling the REST API because of simplicity (already works, don't need to add extra libs) and should be better making one call returning N teams instead calling the GraphQL query N times (where N is the number of allowed_teams)!

Now, about the structure we can also consider this:

allowed_teams:
  - organization: foo-org
    teams:
      - my-team

can we spell out organization so it matches the above (allowed_organizations)? otherwise lgtm!

[thepabloaguilar]

can we spell out organization so it matches the above (allowed_organizations)?

Sure!

I already implemented it, just need to write the server tests!! The final structure was:

allowed_teams:
  foo-org:
    - my-team

Also, I was thinking on changing from allowed_teams to allowed_teams_by_organization to make it more explicit! WDYT?

[markphelps]

can we spell out organization so it matches the above (allowed_organizations)?

Sure!

I already implemented it, just need to write the server tests!! The final structure was:

allowed_teams:
  foo-org:
    - my-team

Also, I was thinking on changing from allowed_teams to allowed_teams_by_organization to make it more explicit! WDYT?

My only thoughts re this are that for the environment variables configuration it would be quite long:

FLIPT_AUTHENTICATION_METHODS_GITHUB_ALLOWED_TEAMS_BY_ORGANIZATION

We do have #2844 which would help alleviate this, but I think allowed_teams is good enough, wdyt?

[thepabloaguilar]

Yeah, we could continue with allowed_teams! If it's well documented there isn't a problem 😄

[markphelps]

Hey @thepabloaguilar , just checking to see if I can help with anything, tests/etc?

[thepabloaguilar]

Thanks for asking @markphelps, I didn't have time to finish 😆

I guess will be able to open a PR today!