feat(auth/github): add organization membership check to GitHub (flipt…

…-io#2508)

* feat(auth/github): add organization membership check to GitHub
authentication method

fixes flipt-io#2065

* rename configuration option

* cleanup

* cleanup

* improve code test coverage

* Update internal/config/authentication.go

Co-authored-by: Mark Phelps <209477+markphelps@users.noreply.github.com>

* address PR review feedback

* cleanup

* feat: support specific auth method validation (#4)

---------

Co-authored-by: Mark Phelps <209477+markphelps@users.noreply.github.com>

config/flipt.schema.cue

@@ -73,6 +73,7 @@ import "strings"
 				client_id?:        string
 				redirect_address?: string
 				scopes?: [...string]
+				allowed_organizations?: [...] | string
 			}
 		}
 

config/flipt.schema.json

@@ -197,6 +197,10 @@
                 "scopes": {
                   "type": ["array", "null"],
                   "items": { "type": "string" }
+                },
+                "allowed_organizations": {
+                  "type": ["array", "null"],
+                  "additionalProperties": false
                 }
               },
               "title": "Github",

internal/config/authentication.go

@@ -3,6 +3,7 @@ package config
 import (
 	"fmt"
 	"net/url"
+	"slices"
 	"strings"
 	"testing"
 	"time"
@@ -133,6 +134,7 @@ func (c *AuthenticationConfig) SessionEnabled() bool {
 
 func (c *AuthenticationConfig) validate() error {
 	var sessionEnabled bool
+
 	for _, info := range c.Methods.AllMethods() {
 		sessionEnabled = sessionEnabled || (info.Enabled && info.SessionCompatible)
 		if info.Cleanup == nil {
@@ -169,6 +171,12 @@ func (c *AuthenticationConfig) validate() error {
 		c.Session.Domain = host
 	}
 
+	for _, info := range c.Methods.AllMethods() {
+		if err := info.validate(); err != nil {
+			return err
+		}
+	}
+
 	return nil
 }
 
@@ -245,6 +253,8 @@ type StaticAuthenticationMethodInfo struct {
 
 	// used for bootstrapping defaults
 	setDefaults func(map[string]any)
+	// used for auth method specific validation
+	validate func() error
 
 	// used for testing purposes to ensure all methods
 	// are appropriately cleaned up via the background process.
@@ -284,6 +294,7 @@ func (a AuthenticationMethodInfo) Name() string {
 type AuthenticationMethodInfoProvider interface {
 	setDefaults(map[string]any)
 	info() AuthenticationMethodInfo
+	validate() error
 }
 
 // AuthenticationMethod is a container for authentication methods.
@@ -309,6 +320,7 @@ func (a *AuthenticationMethod[C]) info() StaticAuthenticationMethodInfo {
 		Cleanup:                  a.Cleanup,
 
 		setDefaults: a.setDefaults,
+		validate:    a.validate,
 		setEnabled: func() {
 			a.Enabled = true
 		},
@@ -318,6 +330,14 @@ func (a *AuthenticationMethod[C]) info() StaticAuthenticationMethodInfo {
 	}
 }
 
+func (a *AuthenticationMethod[C]) validate() error {
+	if !a.Enabled {
+		return nil
+	}
+
+	return a.Method.validate()
+}
+
 // AuthenticationMethodTokenConfig contains fields used to configure the authentication
 // method "token".
 // This authentication method supports the ability to create static tokens via the
@@ -336,6 +356,8 @@ func (a AuthenticationMethodTokenConfig) info() AuthenticationMethodInfo {
 	}
 }
 
+func (a AuthenticationMethodTokenConfig) validate() error { return nil }
+
 // AuthenticationMethodTokenBootstrapConfig contains fields used to configure the
 // bootstrap process for the authentication method "token".
 type AuthenticationMethodTokenBootstrapConfig struct {
@@ -380,6 +402,8 @@ func (a AuthenticationMethodOIDCConfig) info() AuthenticationMethodInfo {
 	return info
 }
 
+func (a AuthenticationMethodOIDCConfig) validate() error { return nil }
+
 // AuthenticationOIDCProvider configures provider credentials
 type AuthenticationMethodOIDCProvider struct {
 	IssuerURL       string   `json:"issuerURL,omitempty" mapstructure:"issuer_url" yaml:"issuer_url,omitempty"`
@@ -426,13 +450,16 @@ func (a AuthenticationMethodKubernetesConfig) info() AuthenticationMethodInfo {
 	}
 }
 
+func (a AuthenticationMethodKubernetesConfig) validate() error { return nil }
+
 // AuthenticationMethodGithubConfig contains configuration and information for completing an OAuth
 // 2.0 flow with GitHub as a provider.
 type AuthenticationMethodGithubConfig struct {
-	ClientId        string   `json:"-" mapstructure:"client_id" yaml:"-"`
-	ClientSecret    string   `json:"-" mapstructure:"client_secret" yaml:"-"`
-	RedirectAddress string   `json:"redirectAddress,omitempty" mapstructure:"redirect_address" yaml:"redirect_address,omitempty"`
-	Scopes          []string `json:"scopes,omitempty" mapstructure:"scopes" yaml:"scopes,omitempty"`
+	ClientId             string   `json:"-" mapstructure:"client_id" yaml:"-"`
+	ClientSecret         string   `json:"-" mapstructure:"client_secret" yaml:"-"`
+	RedirectAddress      string   `json:"redirectAddress,omitempty" mapstructure:"redirect_address" yaml:"redirect_address,omitempty"`
+	Scopes               []string `json:"scopes,omitempty" mapstructure:"scopes" yaml:"scopes,omitempty"`
+	AllowedOrganizations []string `json:"allowedOrganizations,omitempty" mapstructure:"allowed_organizations" yaml:"allowed_organizations,omitempty"`
 }
 
 func (a AuthenticationMethodGithubConfig) setDefaults(defaults map[string]any) {}
@@ -453,3 +480,12 @@ func (a AuthenticationMethodGithubConfig) info() AuthenticationMethodInfo {
 
 	return info
 }
+
+func (a AuthenticationMethodGithubConfig) validate() error {
+	// ensure scopes contain read:org if allowed organizations is not empty
+	if len(a.AllowedOrganizations) > 0 && !slices.Contains(a.Scopes, "read:org") {
+		return fmt.Errorf("scopes must contain read:org when allowed_organizations is not empty")
+	}
+
+	return nil
+}

internal/config/config_test.go

@@ -445,6 +445,11 @@ func TestLoad(t *testing.T) {
 				return cfg
 			},
 		},
+		{
+			name:    "authentication github requires read:org scope when allowing orgs",
+			path:    "./testdata/authentication/github_no_org_scope.yml",
+			wantErr: errors.New("scopes must contain read:org when allowed_organizations is not empty"),
+		},
 		{
 			name: "advanced",
 			path: "./testdata/advanced.yml",

internal/config/testdata/authentication/github_no_org_scope.yml

@@ -0,0 +1,12 @@
+authentication:
+  required: true
+  session:
+    domain: "http://localhost:8080"
+    secure: false
+  methods:
+    github:
+      enabled: true
+      scopes:
+        - "user:email"
+      allowed_organizations:
+        - "github.com/flipt-io"

internal/server/auth/method/github/server.go

@@ -5,12 +5,14 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
+	"slices"
 	"strings"
 	"time"
 
 	"go.flipt.io/flipt/errors"
 	"go.flipt.io/flipt/internal/config"
 	"go.flipt.io/flipt/internal/server/auth/method"
+	authmiddlewaregrpc "go.flipt.io/flipt/internal/server/auth/middleware/grpc"
 	storageauth "go.flipt.io/flipt/internal/storage/auth"
 	"go.flipt.io/flipt/rpc/flipt/auth"
 	"go.uber.org/zap"
@@ -20,8 +22,12 @@ import (
 	"google.golang.org/protobuf/types/known/timestamppb"
 )
 
+type endpoint string
+
 const (
-	githubUserAPI = "https://api.github.com/user"
+	githubAPI                        = "https://api.github.com"
+	githubUser              endpoint = "/user"
+	githubUserOrganizations endpoint = "/user/orgs"
 )
 
 // OAuth2Client is our abstraction of communication with an OAuth2 Provider.
@@ -112,31 +118,6 @@ func (s *Server) Callback(ctx context.Context, r *auth.CallbackRequest) (*auth.C
 		return nil, errors.New("invalid token")
 	}
 
-	c := &http.Client{
-		Timeout: 5 * time.Second,
-	}
-
-	userReq, err := http.NewRequestWithContext(ctx, "GET", githubUserAPI, nil)
-	if err != nil {
-		return nil, err
-	}
-
-	userReq.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token.AccessToken))
-	userReq.Header.Set("Accept", "application/vnd.github+json")
-
-	userResp, err := c.Do(userReq)
-	if err != nil {
-		return nil, err
-	}
-
-	defer func() {
-		userResp.Body.Close()
-	}()
-
-	if userResp.StatusCode != http.StatusOK {
-		return nil, fmt.Errorf("github user info response status: %q", userResp.Status)
-	}
-
 	var githubUserResponse struct {
 		Name      string `json:"name,omitempty"`
 		Email     string `json:"email,omitempty"`
@@ -145,7 +126,7 @@ func (s *Server) Callback(ctx context.Context, r *auth.CallbackRequest) (*auth.C
 		ID        uint64 `json:"id,omitempty"`
 	}
 
-	if err := json.NewDecoder(userResp.Body).Decode(&githubUserResponse); err != nil {
+	if err = api(ctx, token, githubUser, &githubUserResponse); err != nil {
 		return nil, err
 	}
 
@@ -171,6 +152,20 @@ func (s *Server) Callback(ctx context.Context, r *auth.CallbackRequest) (*auth.C
 		metadata[storageMetadataGitHubPreferredUsername] = githubUserResponse.Login
 	}
 
+	if len(s.config.Methods.Github.Method.AllowedOrganizations) != 0 {
+		var githubUserOrgsResponse []githubSimpleOrganization
+		if err = api(ctx, token, githubUserOrganizations, &githubUserOrgsResponse); err != nil {
+			return nil, err
+		}
+		if !slices.ContainsFunc(s.config.Methods.Github.Method.AllowedOrganizations, func(org string) bool {
+			return slices.ContainsFunc(githubUserOrgsResponse, func(githubOrg githubSimpleOrganization) bool {
+				return githubOrg.Login == org
+			})
+		}) {
+			return nil, authmiddlewaregrpc.ErrUnauthenticated
+		}
+	}
+
 	clientToken, a, err := s.store.CreateAuthentication(ctx, &storageauth.CreateAuthenticationRequest{
 		Method:    auth.Method_METHOD_GITHUB,
 		ExpiresAt: timestamppb.New(time.Now().UTC().Add(s.config.Session.TokenLifetime)),
@@ -185,3 +180,36 @@ func (s *Server) Callback(ctx context.Context, r *auth.CallbackRequest) (*auth.C
 		Authentication: a,
 	}, nil
 }
+
+type githubSimpleOrganization struct {
+	Login string
+}
+
+// api calls Github API, decodes and stores successful response in the value pointed to by v.
+func api(ctx context.Context, token *oauth2.Token, endpoint endpoint, v any) error {
+	c := &http.Client{
+		Timeout: 5 * time.Second,
+	}
+
+	userReq, err := http.NewRequestWithContext(ctx, "GET", string(githubAPI+endpoint), nil)
+	if err != nil {
+		return err
+	}
+
+	userReq.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token.AccessToken))
+	userReq.Header.Set("Accept", "application/vnd.github+json")
+
+	resp, err := c.Do(userReq)
+	if err != nil {
+		return err
+	}
+
+	defer func() {
+		resp.Body.Close()
+	}()
+
+	if resp.StatusCode != http.StatusOK {
+		return fmt.Errorf("github %s info response status: %q", endpoint, resp.Status)
+	}
+	return json.NewDecoder(resp.Body).Decode(v)
+}