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

fix: Authz fixes #3132

Merged
merged 9 commits into from
May 29, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
3 changes: 3 additions & 0 deletions config/flipt.schema.cue
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,9 @@ import "strings"
}

#experimental: {
authorization?: {
enabled?: bool | *false
}
cloud?: {
enabled?: bool | *false
}
Expand Down
32 changes: 31 additions & 1 deletion config/flipt.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@
},
"ui": {
"$ref": "#/definitions/ui"
},
"experimental": {
"$ref": "#/definitions/experimental"
}
},

Expand Down Expand Up @@ -348,7 +351,7 @@
"type": "object",
"additionalProperties": false,
"properties": {
"path": {"type": "string"}
"path": { "type": "string" }
}
},
"poll_duration": {
Expand Down Expand Up @@ -1324,6 +1327,33 @@
}
},
"title": "Analytics"
},
"experimental": {
"type": "object",
"additionalProperties": false,
"properties": {
"cloud": {
"type": "object",
"additionalProperties": false,
"properties": {
"enabled": {
"type": "boolean",
"default": false
}
}
},
"authorization": {
"type": "object",
"additionalProperties": false,
"properties": {
"enabled": {
"type": "boolean",
"default": false
}
}
}
},
"title": "Experimental"
}
}
}
1 change: 1 addition & 0 deletions internal/config/authorization.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

var (
_ defaulter = (*AuthorizationConfig)(nil)
_ validator = (*AuthorizationConfig)(nil)
)

// AuthorizationConfig configures Flipts authorization mechanisms
Expand Down
2 changes: 1 addition & 1 deletion internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ type Config struct {
Version string `json:"version,omitempty" mapstructure:"version,omitempty" yaml:"version,omitempty"`
Audit AuditConfig `json:"audit,omitempty" mapstructure:"audit" yaml:"audit,omitempty"`
Authentication AuthenticationConfig `json:"authentication,omitempty" mapstructure:"authentication" yaml:"authentication,omitempty"`
Authorization AuthorizationConfig `json:"authorization,omitempty" mapstructure:"authorization" yaml:"authorization,omitempty"`
Authorization AuthorizationConfig `json:"authorization,omitempty" mapstructure:"authorization" yaml:"authorization,omitempty" experiment:"authorization"`
Cache CacheConfig `json:"cache,omitempty" mapstructure:"cache" yaml:"cache,omitempty"`
Cloud CloudConfig `json:"cloud,omitempty" mapstructure:"cloud" yaml:"cloud,omitempty" experiment:"cloud"`
Cors CorsConfig `json:"cors,omitempty" mapstructure:"cors" yaml:"cors,omitempty"`
Expand Down
3 changes: 3 additions & 0 deletions internal/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -725,6 +725,8 @@ func TestLoad(t *testing.T) {
},
},
}

cfg.Experimental.Authorization.Enabled = true
return cfg
},
},
Expand Down Expand Up @@ -916,6 +918,7 @@ func TestLoad(t *testing.T) {
},
}

cfg.Experimental.Authorization.Enabled = true
return cfg
},
},
Expand Down
3 changes: 2 additions & 1 deletion internal/config/experimental.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ import (
// ExperimentalConfig allows for experimental features to be enabled
// and disabled.
type ExperimentalConfig struct {
Cloud ExperimentalFlag `json:"cloud,omitempty" mapstructure:"cloud" yaml:"cloud,omitempty"`
Cloud ExperimentalFlag `json:"cloud,omitempty" mapstructure:"cloud" yaml:"cloud,omitempty"`
Authorization ExperimentalFlag `json:"authorization,omitempty" mapstructure:"authorization" yaml:"authorization,omitempty"`
}

func (c *ExperimentalConfig) deprecations(v *viper.Viper) []deprecated {
Expand Down
4 changes: 4 additions & 0 deletions internal/config/testdata/advanced.yml
Original file line number Diff line number Diff line change
Expand Up @@ -118,3 +118,7 @@ authorization:
backend: local
local:
path: "/path/to/policy/data.json"

experimental:
authorization:
enabled: true
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,7 @@ authorization:
local:
path: "/path/to/policy.rego"
poll_duration: 30s

experimental:
authorization:
enabled: true
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,10 @@ authentication:
bootstrap:
token: "s3cr3t!"
expiration: 24h

authorization:
required: true

experimental:
authorization:
enabled: true
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,10 @@ authentication:
methods:
token:
enabled: false

authorization:
required: true

experimental:
authorization:
enabled: true
4 changes: 0 additions & 4 deletions internal/server/authn/method/github/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,6 @@ func (s *Server) SkipsAuthentication(ctx context.Context) bool {
return true
}

func (s *Server) SkipsAuthorization(ctx context.Context) bool {
return true
}

func callbackURL(host string) string {
// strip trailing slash from host
host = strings.TrimSuffix(host, "/")
Expand Down
4 changes: 0 additions & 4 deletions internal/server/authn/method/kubernetes/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,6 @@ func (s *Server) SkipsAuthentication(ctx context.Context) bool {
return true
}

func (s *Server) SkipsAuthorization(ctx context.Context) bool {
return true
}

// VerifyServiceAccount takes a service account token, configured by a kubernetes environment,
// validates it's authenticity and (if valid) creates a Flipt client token and returns it.
// The returned client token is valid for the lifetime of the service account JWT.
Expand Down
4 changes: 0 additions & 4 deletions internal/server/authn/method/oidc/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,6 @@ func (s *Server) SkipsAuthentication(ctx context.Context) bool {
return true
}

func (s *Server) SkipsAuthorization(ctx context.Context) bool {
return true
}

// AuthorizeURL constructs and returns a URL directed at the requested OIDC provider
// based on our internal oauth2 client configuration.
// The operation is configured to return a URL which ultimately redirects to the
Expand Down
16 changes: 8 additions & 8 deletions internal/server/authn/middleware/grpc/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,14 +79,14 @@ type InterceptorOptions struct {
skippedServers []any
}

func skipped(ctx context.Context, server any, o InterceptorOptions) bool {
if skipSrv, ok := server.(SkipsAuthenticationServer); ok && skipSrv.SkipsAuthentication(ctx) {
func skipped(ctx context.Context, info *grpc.UnaryServerInfo, o InterceptorOptions) bool {
if skipSrv, ok := info.Server.(SkipsAuthenticationServer); ok && skipSrv.SkipsAuthentication(ctx) {
return true
}

// TODO: refactor to remove this check
for _, s := range o.skippedServers {
if s == server {
if s == info.Server {
return true
}
}
Expand Down Expand Up @@ -122,7 +122,7 @@ func AuthenticationRequiredInterceptor(logger *zap.Logger, o ...containers.Optio

return func(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) {
// skip auth for any preconfigured servers
if skipped(ctx, info.Server, opts) {
if skipped(ctx, info, opts) {
logger.Debug("skipping authentication for server", zap.String("method", info.FullMethod))
return handler(ctx, req)
}
Expand Down Expand Up @@ -157,7 +157,7 @@ func JWTAuthenticationInterceptor(logger *zap.Logger, validator jwt.Validator, e

return func(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) {
// skip auth for any preconfigured servers
if skipped(ctx, info.Server, opts) {
if skipped(ctx, info, opts) {
logger.Debug("skipping authentication for server", zap.String("method", info.FullMethod))
return handler(ctx, req)
}
Expand Down Expand Up @@ -246,7 +246,7 @@ func ClientTokenAuthenticationInterceptor(logger *zap.Logger, authenticator Clie

return func(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) {
// skip auth for any preconfigured servers
if skipped(ctx, info.Server, opts) {
if skipped(ctx, info, opts) {
logger.Debug("skipping authentication for server", zap.String("method", info.FullMethod))
return handler(ctx, req)
}
Expand Down Expand Up @@ -305,7 +305,7 @@ func EmailMatchingInterceptor(logger *zap.Logger, rgxs []*regexp.Regexp, o ...co

return func(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) {
// skip auth for any preconfigured servers
if skipped(ctx, info.Server, opts) {
if skipped(ctx, info, opts) {
logger.Debug("skipping authentication for server", zap.String("method", info.FullMethod))
return handler(ctx, req)
}
Expand Down Expand Up @@ -349,7 +349,7 @@ func NamespaceMatchingInterceptor(logger *zap.Logger, o ...containers.Option[Int

return func(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) {
// skip auth for any preconfigured servers
if skipped(ctx, info.Server, opts) {
if skipped(ctx, info, opts) {
logger.Debug("skipping authentication for server", zap.String("method", info.FullMethod))
return handler(ctx, req)
}
Expand Down
4 changes: 0 additions & 4 deletions internal/server/authn/public/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,3 @@ func (s *Server) RegisterGRPC(server *grpc.Server) {
func (s *Server) SkipsAuthentication(ctx context.Context) bool {
return true
}

func (s *Server) SkipsAuthorization(ctx context.Context) bool {
return true
}
2 changes: 1 addition & 1 deletion internal/server/authz/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ func (e *Engine) updatePolicy(ctx context.Context) error {
e.policyHash = hash

r := rego.New(
rego.Query("data.authz.v1.allow"),
rego.Query("data.flipt.authz.v1.allow"),
rego.Module("policy.rego", string(policy)),
rego.Store(e.store),
)
Expand Down
2 changes: 1 addition & 1 deletion internal/server/authz/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ func (d dataSource) Get(context.Context, []byte) (data map[string]any, _ []byte,
}

var (
testRBACPolicy = policySource(`package authz.v1
testRBACPolicy = policySource(`package flipt.authz.v1

import data
import rego.v1
Expand Down
40 changes: 22 additions & 18 deletions internal/server/authz/middleware/grpc/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,32 @@ type InterceptorOptions struct {
skippedServers []any
}

func skipped(ctx context.Context, server any, o InterceptorOptions) bool {
if skipSrv, ok := server.(SkipsAuthorizationServer); ok && skipSrv.SkipsAuthorization(ctx) {
var (
// methods which should always skip authorization
skippedMethods = map[string]any{
"/flipt.auth.AuthenticationService/GetAuthenticationSelf": struct{}{},
"/flipt.auth.AuthenticationService/ExpireAuthenticationSelf": struct{}{},
}
)

func skipped(ctx context.Context, info *grpc.UnaryServerInfo, o InterceptorOptions) bool {
// if we skip authentication then we must skip authorization
if skipSrv, ok := info.Server.(authmiddlewaregrpc.SkipsAuthenticationServer); ok && skipSrv.SkipsAuthentication(ctx) {
return true
}

if skipSrv, ok := info.Server.(SkipsAuthorizationServer); ok && skipSrv.SkipsAuthorization(ctx) {
return true
}
markphelps marked this conversation as resolved.
Show resolved Hide resolved

// skip authz for any preconfigured methods
if _, ok := skippedMethods[info.FullMethod]; ok {
return true
}

// TODO: refactor to remove this check
for _, s := range o.skippedServers {
if s == server {
if s == info.Server {
return true
}
}
Expand All @@ -56,7 +74,7 @@ func AuthorizationRequiredInterceptor(logger *zap.Logger, policyVerifier authz.V

return func(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) {
// skip authz for any preconfigured servers
if skipped(ctx, info.Server, opts) {
if skipped(ctx, info, opts) {
logger.Debug("skipping authorization for server", zap.String("method", info.FullMethod))
return handler(ctx, req)
}
Expand All @@ -73,20 +91,6 @@ func AuthorizationRequiredInterceptor(logger *zap.Logger, policyVerifier authz.V
return ctx, errUnauthorized
}

// unmarshal auth.metadata["io.flipt.auth.claims"] if present to make writing policies easier
// if auth.Metadata != nil {
// if claims, ok := auth.Metadata["io.flipt.auth.claims"]; ok {
// var claimsMap map[string]interface{}

// if err := json.Unmarshal([]byte(claims), &claimsMap); err != nil {
// logger.Error("unauthorized", zap.String("reason", "failed to unmarshal claims"))
// return ctx, errUnauthorized
// }

// auth.Metadata["io.flipt.auth.claims"] = claimsMap
// }
// }

allowed, err := policyVerifier.IsAllowed(ctx, map[string]interface{}{
"request": requester.Request(),
"authentication": auth,
Expand Down
3 changes: 2 additions & 1 deletion internal/telemetry/telemetry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ func TestShutdown(t *testing.T) {
}

var experimental = map[string]any{
"cloud": map[string]any{},
"cloud": map[string]any{},
"authorization": map[string]any{},
}

func TestPing(t *testing.T) {
Expand Down
12 changes: 12 additions & 0 deletions rpc/flipt/auth/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,15 @@ import (
func (req *CreateTokenRequest) Request() flipt.Request {
return flipt.NewRequest(flipt.ResourceAuthentication, flipt.ActionCreate, flipt.WithSubject(flipt.SubjectToken))
}

func (req *ListAuthenticationsRequest) Request() flipt.Request {
return flipt.NewRequest(flipt.ResourceAuthentication, flipt.ActionRead)
}

func (req *GetAuthenticationRequest) Request() flipt.Request {
return flipt.NewRequest(flipt.ResourceAuthentication, flipt.ActionRead)
}

func (req *DeleteAuthenticationRequest) Request() flipt.Request {
return flipt.NewRequest(flipt.ResourceAuthentication, flipt.ActionDelete)
}
Loading