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

feat(github): OAuth for GitHub #2016

Merged
merged 16 commits into from
Aug 22, 2023
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions config/flipt.schema.cue
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,14 @@ import "strings"
service_account_token_path: string
cleanup?: #authentication.#authentication_cleanup
}

github?: {
enabled?: bool | *false
client_secret?: string
client_id?: string
redirect_address?: string
scopes?: [...string]
}
}

#authentication_cleanup: {
Expand All @@ -79,7 +87,7 @@ import "strings"
client_id?: string
client_secret?: string
redirect_address?: string
scopes?: [...string]
scopes?: [...string]
}
}

Expand Down Expand Up @@ -131,10 +139,10 @@ import "strings"
object?: {
type: "s3" | *""
s3?: {
region: string
bucket: string
prefix?: string
endpoint?: string
region: string
bucket: string
prefix?: string
endpoint?: string
poll_interval?: =~#duration | *"1m"
}
}
Expand Down
24 changes: 24 additions & 0 deletions config/flipt.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,30 @@
"required": [],
"title": "Kubernetes",
"additionalProperties": false
},
"github": {
"type": "object",
"properties": {
"enabled": {
"type": "boolean",
"default": false
},
"client_secret": {
"type": "string"
},
"client_id": {
"type": "string"
},
"redirect_address": {
"type": "string"
},
"scopes": {
"type": ["array", "null"],
"items": { "type": "string" }
}
},
"title": "Github",
"additionalProperties": false
}
},
"required": [],
Expand Down
4 changes: 3 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ require (
github.com/grpc-ecosystem/go-grpc-middleware v1.4.0
github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0
github.com/grpc-ecosystem/grpc-gateway/v2 v2.16.2
github.com/h2non/gock v1.2.0
github.com/hashicorp/cap v0.3.4
github.com/hashicorp/go-multierror v1.1.1
github.com/lib/pq v1.10.9
Expand Down Expand Up @@ -62,6 +63,7 @@ require (
go.uber.org/zap v1.25.0
golang.org/x/exp v0.0.0-20230510235704-dd950f8aeaea
golang.org/x/net v0.14.0
golang.org/x/oauth2 v0.10.0
golang.org/x/sync v0.3.0
google.golang.org/genproto/googleapis/api v0.0.0-20230726155614-23370e0ffb3e
google.golang.org/grpc v1.57.0
Expand Down Expand Up @@ -121,6 +123,7 @@ require (
github.com/google/uuid v1.3.0 // indirect
github.com/gorilla/securecookie v1.1.1 // indirect
github.com/grpc-ecosystem/grpc-gateway v1.16.0 // indirect
github.com/h2non/parth v0.0.0-20190131123155-b4df798d6542 // indirect
github.com/hashicorp/errwrap v1.1.0 // indirect
github.com/hashicorp/go-cleanhttp v0.5.2 // indirect
github.com/hashicorp/go-hclog v1.4.0 // indirect
Expand Down Expand Up @@ -179,7 +182,6 @@ require (
go.uber.org/multierr v1.11.0 // indirect
golang.org/x/crypto v0.12.0 // indirect
golang.org/x/mod v0.10.0 // indirect
golang.org/x/oauth2 v0.10.0 // indirect
golang.org/x/sys v0.11.0 // indirect
golang.org/x/text v0.12.0 // indirect
golang.org/x/time v0.3.0 // indirect
Expand Down
6 changes: 6 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,10 @@ github.com/grpc-ecosystem/grpc-gateway v1.16.0/go.mod h1:BDjrQk3hbvj6Nolgz8mAMFb
github.com/grpc-ecosystem/grpc-gateway/v2 v2.7.0/go.mod h1:hgWBS7lorOAVIJEQMi4ZsPv9hVvWI6+ch50m39Pf2Ks=
github.com/grpc-ecosystem/grpc-gateway/v2 v2.16.2 h1:dygLcbEBA+t/P7ck6a8AkXv6juQ4cK0RHBoh32jxhHM=
github.com/grpc-ecosystem/grpc-gateway/v2 v2.16.2/go.mod h1:Ap9RLCIJVtgQg1/BBgVEfypOAySvvlcpcVQkSzJCH4Y=
github.com/h2non/gock v1.2.0 h1:K6ol8rfrRkUOefooBC8elXoaNGYkpp7y2qcxGG6BzUE=
github.com/h2non/gock v1.2.0/go.mod h1:tNhoxHYW2W42cYkYb1WqzdbYIieALC99kpYr7rH/BQk=
github.com/h2non/parth v0.0.0-20190131123155-b4df798d6542 h1:2VTzZjLZBgl62/EtslCrtky5vbi9dd7HrQPQIx6wqiw=
github.com/h2non/parth v0.0.0-20190131123155-b4df798d6542/go.mod h1:Ow0tF8D4Kplbc8s8sSb3V2oUCygFHVp8gC3Dn6U4MNI=
github.com/hashicorp/cap v0.3.4 h1:RoqWYqr6LaDLuvnBCpod1sZtvuEhehIhu0GncmoHW40=
github.com/hashicorp/cap v0.3.4/go.mod h1:dHTmyMIVbzT981XxRoci5G//dfWmd/HhuNiCH6J5+IA=
github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4=
Expand Down Expand Up @@ -475,6 +479,8 @@ github.com/morikuni/aec v1.0.0/go.mod h1:BbKIizmSmc5MMPqRYbxO4ZU0S0+P200+tUnFx7P
github.com/mpvl/unique v0.0.0-20150818121801-cbe035fff7de h1:D5x39vF5KCwKQaw+OC9ZPiLVHXz3UFw2+psEX+gYcto=
github.com/mpvl/unique v0.0.0-20150818121801-cbe035fff7de/go.mod h1:kJun4WP5gFuHZgRjZUWWuH1DTxCtxbHDOIJsudS8jzY=
github.com/mrunalp/fileutils v0.5.0/go.mod h1:M1WthSahJixYnrXQl/DFQuteStB1weuxD2QJNHXfbSQ=
github.com/nbio/st v0.0.0-20140626010706-e9e8d9816f32 h1:W6apQkHrMkS0Muv8G/TipAy/FJl/rCYT0+EuS8+Z0z4=
github.com/nbio/st v0.0.0-20140626010706-e9e8d9816f32/go.mod h1:9wM+0iRr9ahx58uYLpLIr5fm8diHn0JbqRycJi6w0Ms=
github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno=
github.com/nxadm/tail v1.4.4/go.mod h1:kenIhsEOeOJmVchQTgglprH7qJGnHDVpk1VPCcaMI8A=
github.com/nxadm/tail v1.4.8 h1:nPr65rt6Y5JFSKQO7qToXr7pePgD6Gwiw05lkbyAQTE=
Expand Down
34 changes: 27 additions & 7 deletions internal/cmd/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
"go.flipt.io/flipt/internal/containers"
"go.flipt.io/flipt/internal/gateway"
"go.flipt.io/flipt/internal/server/auth"
"go.flipt.io/flipt/internal/server/auth/method"
authgithub "go.flipt.io/flipt/internal/server/auth/method/github"
authkubernetes "go.flipt.io/flipt/internal/server/auth/method/kubernetes"
authoidc "go.flipt.io/flipt/internal/server/auth/method/oidc"
authtoken "go.flipt.io/flipt/internal/server/auth/method/token"
Expand Down Expand Up @@ -119,6 +121,15 @@
logger.Debug("authentication method \"oidc\" server registered")
}

if authCfg.Methods.Github.Enabled {
githubServer := authgithub.NewServer(logger, store, authCfg)
register.Add(githubServer)

authOpts = append(authOpts, auth.WithServerSkipsAuthentication(githubServer))

logger.Debug("authentication method \"github\" registered")
}

Check warning on line 131 in internal/cmd/auth.go

View check run for this annotation

Codecov / codecov/patch

internal/cmd/auth.go#L124-L131

Added lines #L124 - L131 were not covered by tests

if authCfg.Methods.Kubernetes.Enabled {
kubernetesServer, err := authkubernetes.New(logger, store, authCfg)
if err != nil {
Expand Down Expand Up @@ -211,14 +222,23 @@
muxOpts = append(muxOpts, registerFunc(ctx, conn, rpcauth.RegisterAuthenticationMethodTokenServiceHandler))
}

if cfg.Methods.OIDC.Enabled {
oidcmiddleware := authoidc.NewHTTPMiddleware(cfg.Session)
muxOpts = append(muxOpts,
runtime.WithMetadata(authoidc.ForwardCookies),
runtime.WithForwardResponseOption(oidcmiddleware.ForwardResponseOption),
registerFunc(ctx, conn, rpcauth.RegisterAuthenticationMethodOIDCServiceHandler))
if cfg.SessionEnabled() && (cfg.Methods.OIDC.Enabled || cfg.Methods.Github.Enabled) {
muxOpts = append(muxOpts, runtime.WithMetadata(method.ForwardCookies))
}

Check warning on line 227 in internal/cmd/auth.go

View check run for this annotation

Codecov / codecov/patch

internal/cmd/auth.go#L225-L227

Added lines #L225 - L227 were not covered by tests

if cfg.Methods.OIDC.Enabled || cfg.Methods.Github.Enabled {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you might be able to boil this all down to the following.
SessionEnabled ensures at least one of these two methods are true (same as the .Enabled conditions here).
Also, SessionEnabled is only true, and is always true for OIDC or GitHub.

Suggested change
if cfg.SessionEnabled() && (cfg.Methods.OIDC.Enabled || cfg.Methods.Github.Enabled) {
muxOpts = append(muxOpts, runtime.WithMetadata(method.ForwardCookies))
}
if cfg.Methods.OIDC.Enabled || cfg.Methods.Github.Enabled {
if cfg.SessionEnabled() {
muxOpts = append(muxOpts, runtime.WithMetadata(method.ForwardCookies))

methodMiddleware := method.NewHTTPMiddleware(cfg.Session)
muxOpts = append(muxOpts, runtime.WithForwardResponseOption(methodMiddleware.ForwardResponseOption))

if cfg.Methods.OIDC.Enabled {
muxOpts = append(muxOpts, registerFunc(ctx, conn, rpcauth.RegisterAuthenticationMethodOIDCServiceHandler))
}

Check warning on line 235 in internal/cmd/auth.go

View check run for this annotation

Codecov / codecov/patch

internal/cmd/auth.go#L229-L235

Added lines #L229 - L235 were not covered by tests

if cfg.Methods.Github.Enabled {
muxOpts = append(muxOpts, registerFunc(ctx, conn, rpcauth.RegisterAuthenticationMethodGithubServiceHandler))
}

Check warning on line 239 in internal/cmd/auth.go

View check run for this annotation

Codecov / codecov/patch

internal/cmd/auth.go#L237-L239

Added lines #L237 - L239 were not covered by tests

middleware = append(middleware, oidcmiddleware.Handler)
middleware = append(middleware, methodMiddleware.Handler)

Check warning on line 241 in internal/cmd/auth.go

View check run for this annotation

Codecov / codecov/patch

internal/cmd/auth.go#L241

Added line #L241 was not covered by tests
}

if cfg.Methods.Kubernetes.Enabled {
Expand Down
39 changes: 39 additions & 0 deletions internal/config/authentication.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,15 @@
})
}

func (c *AuthenticationConfig) SessionEnabled() bool {
var sessionEnabled bool
for _, info := range c.Methods.AllMethods() {
sessionEnabled = sessionEnabled || (info.Enabled && info.SessionCompatible)
}

Check warning on line 121 in internal/config/authentication.go

View check run for this annotation

Codecov / codecov/patch

internal/config/authentication.go#L117-L121

Added lines #L117 - L121 were not covered by tests

return sessionEnabled

Check warning on line 123 in internal/config/authentication.go

View check run for this annotation

Codecov / codecov/patch

internal/config/authentication.go#L123

Added line #L123 was not covered by tests
}

func (c *AuthenticationConfig) validate() error {
var sessionEnabled bool
for _, info := range c.Methods.AllMethods() {
Expand Down Expand Up @@ -192,6 +201,7 @@
// method available for use within Flipt.
type AuthenticationMethods struct {
Token AuthenticationMethod[AuthenticationMethodTokenConfig] `json:"token,omitempty" mapstructure:"token"`
Github AuthenticationMethod[AuthenticationMethodGithubConfig] `json:"github,omitempty" mapstructure:"github"`
OIDC AuthenticationMethod[AuthenticationMethodOIDCConfig] `json:"oidc,omitempty" mapstructure:"oidc"`
Kubernetes AuthenticationMethod[AuthenticationMethodKubernetesConfig] `json:"kubernetes,omitempty" mapstructure:"kubernetes"`
}
Expand All @@ -200,6 +210,7 @@
func (a *AuthenticationMethods) AllMethods() []StaticAuthenticationMethodInfo {
return []StaticAuthenticationMethodInfo{
a.Token.info(),
a.Github.info(),
a.OIDC.info(),
a.Kubernetes.info(),
}
Expand Down Expand Up @@ -405,3 +416,31 @@
SessionCompatible: false,
}
}

// AuthenticationMethodGithubConfig contains configuration and information for completing an OAuth
// 2.0 flow with GitHub as a provider.
type AuthenticationMethodGithubConfig struct {
ClientSecret string `json:"clientSecret,omitempty" mapstructure:"client_secret"`
ClientId string `json:"clientId,omitempty" mapstructure:"client_id"`
RedirectAddress string `json:"redirectAddress,omitempty" mapstructure:"redirect_address"`
Scopes []string `json:"scopes,omitempty" mapstructure:"scopes"`
}

func (a AuthenticationMethodGithubConfig) setDefaults(defaults map[string]any) {}

// info describes properties of the authentication method "github".
func (a AuthenticationMethodGithubConfig) info() AuthenticationMethodInfo {
info := AuthenticationMethodInfo{
Method: auth.Method_METHOD_GITHUB,
SessionCompatible: true,
}

var metadata = make(map[string]any)

metadata["authorize_url"] = "/auth/v1/method/github/authorize"
metadata["callback_url"] = "/auth/v1/method/github/callback"

info.Metadata, _ = structpb.NewStruct(metadata)

return info
}
12 changes: 12 additions & 0 deletions internal/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -589,6 +589,18 @@ func TestLoad(t *testing.T) {
GracePeriod: 48 * time.Hour,
},
},
Github: AuthenticationMethod[AuthenticationMethodGithubConfig]{
Method: AuthenticationMethodGithubConfig{
ClientId: "abcdefg",
ClientSecret: "bcdefgh",
RedirectAddress: "http://auth.flipt.io",
},
Enabled: true,
Cleanup: &AuthenticationCleanupSchedule{
Interval: 2 * time.Hour,
GracePeriod: 48 * time.Hour,
},
},
},
}
return cfg
Expand Down
8 changes: 8 additions & 0 deletions internal/config/testdata/advanced.yml
Original file line number Diff line number Diff line change
Expand Up @@ -95,3 +95,11 @@ authentication:
cleanup:
interval: 2h
grace_period: 48h
github:
enabled: true
client_id: "abcdefg"
client_secret: "bcdefgh"
redirect_address: "http://auth.flipt.io"
cleanup:
interval: 2h
grace_period: 48h
Loading
Loading