chore: set raw claims if they exist in authz metadata (#3125)

* chore: go mod tidy Signed-off-by: Mark Phelps <[email protected]> * chore: set raw claims if they exist in authz metadata Signed-off-by: Mark Phelps <[email protected]> * chore: fix authn oidc server test Signed-off-by: Mark Phelps <[email protected]> * chore: skip authz on auth public server Signed-off-by: Mark Phelps <[email protected]> * chore: log for debugging Signed-off-by: Mark Phelps <[email protected]> --------- Signed-off-by: Mark Phelps <[email protected]>
flipt-io · May 28, 2024 · b15e83e · b15e83e
1 parent fbd21f2
commit b15e83e
Show file tree

Hide file tree

Showing 14 changed files with 59 additions and 206 deletions.
diff --git a/go.mod b/go.mod
@@ -50,7 +50,6 @@ require (
 	github.com/hashicorp/golang-lru/v2 v2.0.7
 	github.com/iancoleman/strcase v0.3.0
 	github.com/jackc/pgx/v5 v5.5.5
-	github.com/jmespath/go-jmespath v0.4.0
 	github.com/libsql/libsql-client-go v0.0.0-20230917132930-48c310b27e7b
 	github.com/magefile/mage v1.15.0
 	github.com/mattn/go-sqlite3 v1.14.22
@@ -205,6 +204,7 @@ require (
 	github.com/jackc/pgservicefile v0.0.0-20221227161230-091c0ba34f0a // indirect
 	github.com/jackc/puddle/v2 v2.2.1 // indirect
 	github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 // indirect
+	github.com/jmespath/go-jmespath v0.4.0 // indirect
 	github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 // indirect
 	github.com/kevinburke/ssh_config v1.2.0 // indirect
 	github.com/klauspost/compress v1.17.8 // indirect

diff --git a/go.work.sum b/go.work.sum
@@ -481,6 +481,7 @@ github.com/gabriel-vasile/mimetype v1.4.1/go.mod h1:05Vi0w3Y9c/lNvJOdmIwvrrAhX3r
 github.com/garyburd/redigo v0.0.0-20150301180006-535138d7bcd7/go.mod h1:NR3MbYisc3/PwhQ00EMzDiPmrwpPxAn5GI05/YaO1SY=
 github.com/getsentry/raven-go v0.2.0/go.mod h1:KungGk8q33+aIAZUIVWZDr2OfAEBsO49PX4NzFV5kcQ=
 github.com/ghodss/yaml v0.0.0-20150909031657-73d445a93680/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04=
+github.com/ghodss/yaml v1.0.0 h1:wQHKEahhL6wmXdzwWG11gIVCkOv05bNOh+Rxn0yngAk=
 github.com/go-ini/ini v1.25.4/go.mod h1:ByCAeIL28uOIIG0E3PJtZPDL8WnHpFKFOtgjp+3Ies8=
 github.com/go-kit/log v0.2.0/go.mod h1:NwTd00d/i8cPZ3xOwwiv2PO5MOcx78fFErGNcVmBjv0=
 github.com/go-kit/log v0.2.1/go.mod h1:NwTd00d/i8cPZ3xOwwiv2PO5MOcx78fFErGNcVmBjv0=
@@ -550,6 +551,7 @@ github.com/google/go-containerregistry v0.5.1/go.mod h1:Ct15B4yir3PLOP5jsy0GNeYV
 github.com/google/go-github/v39 v39.2.0/go.mod h1:C1s8C5aCC9L+JXIYpJM5GYytdX52vC1bLvHEF1IhBrE=
 github.com/google/go-pkcs11 v0.2.1-0.20230907215043-c6f79328ddf9/go.mod h1:6eQoGcuNJpa7jnd5pMGdkSaQpNDYvPlXWMcjXXThLlY=
 github.com/google/gofuzz v1.1.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
+github.com/google/martian v2.1.0+incompatible h1:/CP5g8u/VJHijgedC/Legn3BAbAaWPgecwXBIDzw5no=
 github.com/google/martian/v3 v3.1.0/go.mod h1:y5Zk1BBys9G+gd6Jrk0W3cC1+ELVxBWuIGO+w/tUAp0=
 github.com/google/pprof v0.0.0-20201023163331-3e6fc7fc9c4c/go.mod h1:kpwsk12EmLew5upagYY7GY0pfYCcupk39gWOCRROcvE=
 github.com/google/pprof v0.0.0-20201203190320-1bf35d6f28c2/go.mod h1:kpwsk12EmLew5upagYY7GY0pfYCcupk39gWOCRROcvE=

diff --git a/internal/cmd/grpc.go b/internal/cmd/grpc.go
@@ -386,7 +386,7 @@ func NewGRPCServer(
 
 		interceptors = append(interceptors, authzmiddlewaregrpc.AuthorizationRequiredInterceptor(logger, policyEngine, authzOpts...))
 
-		logger.Debug("authorization required")
+		logger.Info("authorization middleware enabled")
 	}
 
 	// audit sinks configuration

diff --git a/internal/config/authentication.go b/internal/config/authentication.go
@@ -469,13 +469,12 @@ func (a AuthenticationMethodOIDCConfig) validate() error {
 
 // AuthenticationOIDCProvider configures provider credentials
 type AuthenticationMethodOIDCProvider struct {
-	IssuerURL         string   `json:"issuerURL,omitempty" mapstructure:"issuer_url" yaml:"issuer_url,omitempty"`
-	ClientID          string   `json:"-,omitempty" 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"`
-	UsePKCE           bool     `json:"usePKCE,omitempty" mapstructure:"use_pkce" yaml:"use_pkce,omitempty"`
-	RoleAttributePath string   `json:"roleAttributePath,omitempty" mapstructure:"role_attribute_path" yaml:"role_attribute_path,omitempty"`
+	IssuerURL       string   `json:"issuerURL,omitempty" mapstructure:"issuer_url" yaml:"issuer_url,omitempty"`
+	ClientID        string   `json:"-,omitempty" 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"`
+	UsePKCE         bool     `json:"usePKCE,omitempty" mapstructure:"use_pkce" yaml:"use_pkce,omitempty"`
 }
 
 func (a AuthenticationMethodOIDCProvider) validate() error {

diff --git a/internal/server/authn/method/metadata.go b/internal/server/authn/method/metadata.go
@@ -1,7 +1,7 @@
 package method
 
 const (
-	StorageMetadataRole  = "io.flipt.auth.role"
-	StorageMetadataEmail = "io.flipt.auth.email"
-	StorageMetadataName  = "io.flipt.auth.name"
+	StorageMetadataClaims = "io.flipt.auth.claims"
+	StorageMetadataEmail  = "io.flipt.auth.email"
+	StorageMetadataName   = "io.flipt.auth.name"
 )
diff --git a/internal/server/authn/method/oidc/server.go b/internal/server/authn/method/oidc/server.go
@@ -2,6 +2,7 @@ package oidc
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"strings"
 	"time"
@@ -145,23 +146,19 @@ func (s *Server) Callback(ctx context.Context, req *auth.CallbackRequest) (_ *au
 		storageMetadataOIDCProvider: req.Provider,
 	}
 
-	cfg, err := s.configFor(req.Provider)
-	if err != nil {
+	rawClaims := make(map[string]interface{})
+	if err := responseToken.IDToken().Claims(&rawClaims); err != nil {
 		return nil, err
 	}
 
-	if cfg.RoleAttributePath != "" {
-		rawClaims := make(map[string]interface{})
-		if err := responseToken.IDToken().Claims(&rawClaims); err != nil {
-			return nil, err
-		}
-
-		role, err := method.SearchJSONForAttr[string](cfg.RoleAttributePath, rawClaims)
-		if err != nil {
-			return nil, err
-		}
+	// marshal raw claims to JSON
+	rawClaimsJSON, err := json.Marshal(rawClaims)
+	if err != nil {
+		return nil, err
+	}
 
-		metadata[method.StorageMetadataRole] = role
+	if rawClaimsJSON != nil {
+		metadata[method.StorageMetadataClaims] = string(rawClaimsJSON)
 	}
 
 	// Extract custom claims

diff --git a/internal/server/authn/method/oidc/server_test.go b/internal/server/authn/method/oidc/server_test.go
@@ -109,11 +109,10 @@ func Test_Server_ImplicitFlow(t *testing.T) {
 					Method: config.AuthenticationMethodOIDCConfig{
 						Providers: map[string]config.AuthenticationMethodOIDCProvider{
 							"google": {
-								IssuerURL:         tp.Addr(),
-								ClientID:          id,
-								ClientSecret:      secret,
-								RedirectAddress:   clientAddress,
-								RoleAttributePath: "contains(roles[*], 'admin') && 'admin' || contains(roles[*], 'editor') && 'editor' || 'viewer'",
+								IssuerURL:       tp.Addr(),
+								ClientID:        id,
+								ClientSecret:    secret,
+								RedirectAddress: clientAddress,
 							},
 						},
 					},
@@ -215,12 +214,11 @@ func Test_Server_PKCE(t *testing.T) {
 					Method: config.AuthenticationMethodOIDCConfig{
 						Providers: map[string]config.AuthenticationMethodOIDCProvider{
 							"google": {
-								IssuerURL:         tp.Addr(),
-								ClientID:          id,
-								ClientSecret:      secret,
-								RedirectAddress:   clientAddress,
-								UsePKCE:           true,
-								RoleAttributePath: "contains(roles[*], 'admin') && 'admin' || contains(roles[*], 'editor') && 'editor' || 'viewer'",
+								IssuerURL:       tp.Addr(),
+								ClientID:        id,
+								ClientSecret:    secret,
+								RedirectAddress: clientAddress,
+								UsePKCE:         true,
 							},
 						},
 					},
@@ -346,15 +344,19 @@ func testOIDCFlow(t *testing.T, ctx context.Context, tpAddr, clientAddress strin
 
 		assert.Empty(t, response.ClientToken) // middleware moves it to cookie
 		assert.Equal(t, auth.Method_METHOD_OIDC, response.Authentication.Method)
-		assert.Equal(t, map[string]string{
+
+		for k, v := range map[string]string{
 			"io.flipt.auth.oidc.provider": "google",
 			"io.flipt.auth.oidc.email":    "[email protected]",
 			"io.flipt.auth.email":         "[email protected]",
 			"io.flipt.auth.oidc.name":     "Mark Phelps",
 			"io.flipt.auth.name":          "Mark Phelps",
 			"io.flipt.auth.oidc.sub":      "mark",
-			"io.flipt.auth.role":          "admin",
-		}, response.Authentication.Metadata)
+		} {
+			assert.Equal(t, v, response.Authentication.Metadata[k])
+		}
+
+		assert.NotEmpty(t, response.Authentication.Metadata["io.flipt.auth.claims"])
 
 		// ensure expiry is set
 		assert.NotNil(t, response.Authentication.ExpiresAt)

diff --git a/internal/server/authn/method/util.go b/internal/server/authn/method/util.go
@@ -3,10 +3,6 @@ package method
 import (
 	"context"
 
-	"encoding/json"
-	"fmt"
-
-	"github.com/jmespath/go-jmespath"
 	"go.flipt.io/flipt/errors"
 	"google.golang.org/grpc/metadata"
 )
@@ -30,46 +26,3 @@ func CallbackValidateState(ctx context.Context, state string) error {
 
 	return nil
 }
-
-func SearchJSONForAttr[T ~string](attributePath string, data any) (T, error) {
-	v, err := searchJSONForAttr(attributePath, data)
-	if err != nil {
-		return "", err
-	}
-	return v.(T), nil
-}
-
-func searchJSONForAttr(attributePath string, data any) (any, error) {
-	if attributePath == "" {
-		return "", fmt.Errorf("attribute path: %q", attributePath)
-	}
-
-	if data == nil {
-		return "", fmt.Errorf("empty json, attribute path: %q", attributePath)
-	}
-
-	// Copy the data to a new variable
-	var jsonData = data
-
-	// If the data is a byte slice, try to unmarshal it into a JSON object
-	if dataBytes, ok := data.([]byte); ok {
-		// If the byte slice is empty, return an error
-		if len(dataBytes) == 0 {
-			return "", fmt.Errorf("empty json, attribute path: %q", attributePath)
-		}
-
-		// Try to unmarshal the byte slice
-		if err := json.Unmarshal(dataBytes, &jsonData); err != nil {
-			return "", fmt.Errorf("%v: %w", "failed to unmarshal user info JSON response", err)
-		}
-	}
-
-	// Search for the attribute in the JSON object
-	value, err := jmespath.Search(attributePath, jsonData)
-	if err != nil {
-		return "", fmt.Errorf("failed to search user info JSON response with provided path: %q: %w", attributePath, err)
-	}
-
-	// Return the value and nil error
-	return value, nil
-}
diff --git a/internal/server/authn/public/server.go b/internal/server/authn/public/server.go
@@ -49,3 +49,7 @@ 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
+}
diff --git a/internal/server/authz/engine.go b/internal/server/authz/engine.go
@@ -109,6 +109,8 @@ func (e *Engine) IsAllowed(ctx context.Context, input map[string]interface{}) (b
 	e.mu.RLock()
 	defer e.mu.RUnlock()
 
+	e.logger.Debug("evaluating policy", zap.Any("input", input))
+
 	results, err := e.query.Eval(ctx, rego.EvalInput(input))
 	if err != nil {
 		return false, err

diff --git a/internal/server/authz/middleware/grpc/middleware.go b/internal/server/authz/middleware/grpc/middleware.go
@@ -73,6 +73,20 @@ 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,

diff --git a/internal/server/authz/policies/default.json b/internal/server/authz/policies/default.json
diff --git a/internal/server/authz/policies/policy.schema.json b/internal/server/authz/policies/policy.schema.json
diff --git a/internal/server/authz/policies/schema_test.go b/internal/server/authz/policies/schema_test.go