Skip to content

Commit 43d6ea1

Browse files
enjenj
committed
jwt: refactor CEL eval to drop unstructured and map[string]any
This prepares us to add support for distributed claims support in CEL expressions. Signed-off-by: Monis Khan <[email protected]>
1 parent 2cc53d7 commit 43d6ea1

File tree

3 files changed

+86
-93
lines changed

3 files changed

+86
-93
lines changed

‎staging/src/k8s.io/apiserver/pkg/authentication/cel/interface.go‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,7 @@ import (
2222

2323
celgo "github.com/google/cel-go/cel"
2424
"github.com/google/cel-go/common/types/ref"
25-
26-
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
25+
"github.com/google/cel-go/common/types/traits"
2726
)
2827

2928
// ExpressionAccessor is an interface that provides access to a CEL expression.
@@ -55,17 +54,17 @@ type Compiler interface {
5554
type ClaimsMapper interface {
5655
// EvalClaimMapping evaluates the given claim mapping expression and returns a EvaluationResult.
5756
// This is used for username, groups and uid claim mapping that contains a single expression.
58-
EvalClaimMapping(ctx context.Context, claims *unstructured.Unstructured) (EvaluationResult, error)
57+
EvalClaimMapping(ctx context.Context, claims traits.Mapper) (EvaluationResult, error)
5958
// EvalClaimMappings evaluates the given expressions and returns a list of EvaluationResult.
6059
// This is used for extra claim mapping and claim validation that contains a list of expressions.
61-
EvalClaimMappings(ctx context.Context, claims *unstructured.Unstructured) ([]EvaluationResult, error)
60+
EvalClaimMappings(ctx context.Context, claims traits.Mapper) ([]EvaluationResult, error)
6261
}
6362

6463
// UserMapper provides a CEL expression mapper configured with the user CEL variable.
6564
type UserMapper interface {
6665
// EvalUser evaluates the given user expressions and returns a list of EvaluationResult.
6766
// This is used for user validation that contains a list of expressions.
68-
EvalUser(ctx context.Context, userInfo *unstructured.Unstructured) ([]EvaluationResult, error)
67+
EvalUser(ctx context.Context, userInfo traits.Mapper) ([]EvaluationResult, error)
6968
}
7069

7170
var _ ExpressionAccessor = &ClaimMappingExpression{}

‎staging/src/k8s.io/apiserver/pkg/authentication/cel/mapper.go‎

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@ import (
2020
"context"
2121
"fmt"
2222

23-
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
23+
"github.com/google/cel-go/common/types/traits"
24+
"github.com/google/cel-go/interpreter"
2425
)
2526

2627
var _ ClaimsMapper = &mapper{}
@@ -57,8 +58,8 @@ func NewUserMapper(compilationResults []CompilationResult) UserMapper {
5758
}
5859

5960
// EvalClaimMapping evaluates the given claim mapping expression and returns a EvaluationResult.
60-
func (m *mapper) EvalClaimMapping(ctx context.Context, claims *unstructured.Unstructured) (EvaluationResult, error) {
61-
results, err := m.eval(ctx, map[string]interface{}{claimsVarName: claims.Object})
61+
func (m *mapper) EvalClaimMapping(ctx context.Context, claims traits.Mapper) (EvaluationResult, error) {
62+
results, err := m.eval(ctx, &varNameActivation{name: claimsVarName, value: claims})
6263
if err != nil {
6364
return EvaluationResult{}, err
6465
}
@@ -69,16 +70,16 @@ func (m *mapper) EvalClaimMapping(ctx context.Context, claims *unstructured.Unst
6970
}
7071

7172
// EvalClaimMappings evaluates the given expressions and returns a list of EvaluationResult.
72-
func (m *mapper) EvalClaimMappings(ctx context.Context, claims *unstructured.Unstructured) ([]EvaluationResult, error) {
73-
return m.eval(ctx, map[string]interface{}{claimsVarName: claims.Object})
73+
func (m *mapper) EvalClaimMappings(ctx context.Context, claims traits.Mapper) ([]EvaluationResult, error) {
74+
return m.eval(ctx, &varNameActivation{name: claimsVarName, value: claims})
7475
}
7576

7677
// EvalUser evaluates the given user expressions and returns a list of EvaluationResult.
77-
func (m *mapper) EvalUser(ctx context.Context, userInfo *unstructured.Unstructured) ([]EvaluationResult, error) {
78-
return m.eval(ctx, map[string]interface{}{userVarName: userInfo.Object})
78+
func (m *mapper) EvalUser(ctx context.Context, userInfo traits.Mapper) ([]EvaluationResult, error) {
79+
return m.eval(ctx, &varNameActivation{name: userVarName, value: userInfo})
7980
}
8081

81-
func (m *mapper) eval(ctx context.Context, input map[string]interface{}) ([]EvaluationResult, error) {
82+
func (m *mapper) eval(ctx context.Context, input *varNameActivation) ([]EvaluationResult, error) {
8283
evaluations := make([]EvaluationResult, len(m.compilationResults))
8384

8485
for i, compilationResult := range m.compilationResults {
@@ -95,3 +96,19 @@ func (m *mapper) eval(ctx context.Context, input map[string]interface{}) ([]Eval
9596

9697
return evaluations, nil
9798
}
99+
100+
var _ interpreter.Activation = &varNameActivation{}
101+
102+
type varNameActivation struct {
103+
name string
104+
value traits.Mapper
105+
}
106+
107+
func (v *varNameActivation) ResolveName(name string) (any, bool) {
108+
if v.name != name {
109+
return nil, false
110+
}
111+
return v.value, true
112+
}
113+
114+
func (v *varNameActivation) Parent() interpreter.Activation { return nil }

‎staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc.go‎

Lines changed: 57 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -36,19 +36,16 @@ import (
3636
"io/ioutil"
3737
"net/http"
3838
"net/url"
39-
"reflect"
4039
"strings"
4140
"sync"
4241
"sync/atomic"
4342
"time"
4443

4544
"github.com/coreos/go-oidc"
4645
celgo "github.com/google/cel-go/cel"
46+
"github.com/google/cel-go/common/types"
4747
"github.com/google/cel-go/common/types/ref"
4848

49-
authenticationv1 "k8s.io/api/authentication/v1"
50-
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
51-
"k8s.io/apimachinery/pkg/runtime"
5249
"k8s.io/apimachinery/pkg/util/net"
5350
"k8s.io/apimachinery/pkg/util/sets"
5451
"k8s.io/apimachinery/pkg/util/wait"
@@ -58,6 +55,7 @@ import (
5855
authenticationcel "k8s.io/apiserver/pkg/authentication/cel"
5956
authenticationtokenjwt "k8s.io/apiserver/pkg/authentication/token/jwt"
6057
"k8s.io/apiserver/pkg/authentication/user"
58+
"k8s.io/apiserver/pkg/cel/lazy"
6159
certutil "k8s.io/client-go/util/cert"
6260
"k8s.io/klog/v2"
6361
)
@@ -708,36 +706,31 @@ func (a *jwtAuthenticator) AuthenticateToken(ctx context.Context, token string)
708706
}
709707
}
710708

711-
var claimsUnstructured *unstructured.Unstructured
712-
// Convert the claims to unstructured so that we can evaluate the CEL expressions
709+
var claimsValue *lazy.MapValue
710+
// Convert the claims to traits.Mapper so that we can evaluate the CEL expressions
713711
// against the claims. This is done once here so that we don't have to convert
714-
// the claims to unstructured multiple times in the CEL mapper for each mapping.
712+
// the claims to traits.Mapper multiple times in the CEL mapper for each mapping.
715713
// Only perform this conversion if any of the mapping or validation rules contain
716-
// CEL expressions.
717-
// TODO(aramase): In the future when we look into making distributed claims work,
718-
// we should see if we can skip this function and use a dynamic type resolver for
719-
// both json.RawMessage and the distributed claim fetching.
714+
// CEL expressions. The traits.Mapper is lazily evaluated against the expressions.
720715
if a.celMapper.Username != nil || a.celMapper.Groups != nil || a.celMapper.UID != nil || a.celMapper.Extra != nil || a.celMapper.ClaimValidationRules != nil {
721-
if claimsUnstructured, err = convertObjectToUnstructured(&c); err != nil {
722-
return nil, false, fmt.Errorf("oidc: could not convert claims to unstructured: %w", err)
723-
}
716+
claimsValue = newClaimsValue(c)
724717
}
725718

726719
var username string
727-
if username, err = a.getUsername(ctx, c, claimsUnstructured); err != nil {
720+
if username, err = a.getUsername(ctx, c, claimsValue); err != nil {
728721
return nil, false, err
729722
}
730723

731724
info := &user.DefaultInfo{Name: username}
732-
if info.Groups, err = a.getGroups(ctx, c, claimsUnstructured); err != nil {
725+
if info.Groups, err = a.getGroups(ctx, c, claimsValue); err != nil {
733726
return nil, false, err
734727
}
735728

736-
if info.UID, err = a.getUID(ctx, c, claimsUnstructured); err != nil {
729+
if info.UID, err = a.getUID(ctx, c, claimsValue); err != nil {
737730
return nil, false, err
738731
}
739732

740-
extra, err := a.getExtra(ctx, c, claimsUnstructured)
733+
extra, err := a.getExtra(ctx, c, claimsValue)
741734
if err != nil {
742735
return nil, false, err
743736
}
@@ -762,7 +755,7 @@ func (a *jwtAuthenticator) AuthenticateToken(ctx context.Context, token string)
762755
}
763756

764757
if a.celMapper.ClaimValidationRules != nil {
765-
evalResult, err := a.celMapper.ClaimValidationRules.EvalClaimMappings(ctx, claimsUnstructured)
758+
evalResult, err := a.celMapper.ClaimValidationRules.EvalClaimMappings(ctx, claimsValue)
766759
if err != nil {
767760
return nil, false, fmt.Errorf("oidc: error evaluating claim validation expression: %w", err)
768761
}
@@ -778,15 +771,13 @@ func (a *jwtAuthenticator) AuthenticateToken(ctx context.Context, token string)
778771
}
779772

780773
if a.celMapper.UserValidationRules != nil {
781-
// Convert the user info to unstructured so that we can evaluate the CEL expressions
774+
// Convert the user info to traits.Mapper so that we can evaluate the CEL expressions
782775
// against the user info. This is done once here so that we don't have to convert
783-
// the user info to unstructured multiple times in the CEL mapper for each mapping.
784-
userInfoUnstructured, err := convertUserInfoToUnstructured(info)
785-
if err != nil {
786-
return nil, false, fmt.Errorf("oidc: could not convert user info to unstructured: %w", err)
787-
}
776+
// the user info to traits.Mapper multiple times in the CEL mapper for each mapping.
777+
// The traits.Mapper is lazily evaluated against the expressions.
778+
userInfoVal := newUserInfoValue(info)
788779

789-
evalResult, err := a.celMapper.UserValidationRules.EvalUser(ctx, userInfoUnstructured)
780+
evalResult, err := a.celMapper.UserValidationRules.EvalUser(ctx, userInfoVal)
790781
if err != nil {
791782
return nil, false, fmt.Errorf("oidc: error evaluating user info validation rule: %w", err)
792783
}
@@ -812,9 +803,9 @@ func (a *jwtAuthenticator) HealthCheck() error {
812803
return nil
813804
}
814805

815-
func (a *jwtAuthenticator) getUsername(ctx context.Context, c claims, claimsUnstructured *unstructured.Unstructured) (string, error) {
806+
func (a *jwtAuthenticator) getUsername(ctx context.Context, c claims, claimsValue *lazy.MapValue) (string, error) {
816807
if a.celMapper.Username != nil {
817-
evalResult, err := a.celMapper.Username.EvalClaimMapping(ctx, claimsUnstructured)
808+
evalResult, err := a.celMapper.Username.EvalClaimMapping(ctx, claimsValue)
818809
if err != nil {
819810
return "", fmt.Errorf("oidc: error evaluating username claim expression: %w", err)
820811
}
@@ -860,7 +851,7 @@ func (a *jwtAuthenticator) getUsername(ctx context.Context, c claims, claimsUnst
860851
return username, nil
861852
}
862853

863-
func (a *jwtAuthenticator) getGroups(ctx context.Context, c claims, claimsUnstructured *unstructured.Unstructured) ([]string, error) {
854+
func (a *jwtAuthenticator) getGroups(ctx context.Context, c claims, claimsValue *lazy.MapValue) ([]string, error) {
864855
groupsClaim := a.jwtAuthenticator.ClaimMappings.Groups.Claim
865856
if len(groupsClaim) > 0 {
866857
if _, ok := c[groupsClaim]; ok {
@@ -888,7 +879,7 @@ func (a *jwtAuthenticator) getGroups(ctx context.Context, c claims, claimsUnstru
888879
return nil, nil
889880
}
890881

891-
evalResult, err := a.celMapper.Groups.EvalClaimMapping(ctx, claimsUnstructured)
882+
evalResult, err := a.celMapper.Groups.EvalClaimMapping(ctx, claimsValue)
892883
if err != nil {
893884
return nil, fmt.Errorf("oidc: error evaluating group claim expression: %w", err)
894885
}
@@ -900,7 +891,7 @@ func (a *jwtAuthenticator) getGroups(ctx context.Context, c claims, claimsUnstru
900891
return groups, nil
901892
}
902893

903-
func (a *jwtAuthenticator) getUID(ctx context.Context, c claims, claimsUnstructured *unstructured.Unstructured) (string, error) {
894+
func (a *jwtAuthenticator) getUID(ctx context.Context, c claims, claimsValue *lazy.MapValue) (string, error) {
904895
uidClaim := a.jwtAuthenticator.ClaimMappings.UID.Claim
905896
if len(uidClaim) > 0 {
906897
var uid string
@@ -914,7 +905,7 @@ func (a *jwtAuthenticator) getUID(ctx context.Context, c claims, claimsUnstructu
914905
return "", nil
915906
}
916907

917-
evalResult, err := a.celMapper.UID.EvalClaimMapping(ctx, claimsUnstructured)
908+
evalResult, err := a.celMapper.UID.EvalClaimMapping(ctx, claimsValue)
918909
if err != nil {
919910
return "", fmt.Errorf("oidc: error evaluating uid claim expression: %w", err)
920911
}
@@ -925,7 +916,7 @@ func (a *jwtAuthenticator) getUID(ctx context.Context, c claims, claimsUnstructu
925916
return evalResult.EvalResult.Value().(string), nil
926917
}
927918

928-
func (a *jwtAuthenticator) getExtra(ctx context.Context, c claims, claimsUnstructured *unstructured.Unstructured) (map[string][]string, error) {
919+
func (a *jwtAuthenticator) getExtra(ctx context.Context, c claims, claimsValue *lazy.MapValue) (map[string][]string, error) {
929920
extra := make(map[string][]string)
930921

931922
if credentialID := getCredentialID(c); len(credentialID) > 0 {
@@ -936,7 +927,7 @@ func (a *jwtAuthenticator) getExtra(ctx context.Context, c claims, claimsUnstruc
936927
return extra, nil
937928
}
938929

939-
evalResult, err := a.celMapper.Extra.EvalClaimMappings(ctx, claimsUnstructured)
930+
evalResult, err := a.celMapper.Extra.EvalClaimMappings(ctx, claimsValue)
940931
if err != nil {
941932
return nil, err
942933
}
@@ -1023,7 +1014,7 @@ func (c claims) unmarshalClaim(name string, v interface{}) error {
10231014
if !ok {
10241015
return fmt.Errorf("claim not present")
10251016
}
1026-
return json.Unmarshal([]byte(val), v)
1017+
return json.Unmarshal(val, v)
10271018
}
10281019

10291020
func (c claims) hasClaim(name string) bool {
@@ -1033,6 +1024,25 @@ func (c claims) hasClaim(name string) bool {
10331024
return true
10341025
}
10351026

1027+
func newClaimsValue(c claims) *lazy.MapValue {
1028+
lazyMap := lazy.NewMapValue(types.NewObjectType("kubernetes.claims"))
1029+
for name, msg := range c { // TODO add distributed claims support
1030+
lazyMap.Append(name, func(_ *lazy.MapValue) ref.Val {
1031+
data, err := msg.MarshalJSON()
1032+
if err != nil {
1033+
return types.WrapErr(err) // impossible since RawMessage never errors
1034+
}
1035+
1036+
var value any // TODO how do we do multiple levels of lazy decoding?
1037+
if err := json.Unmarshal(data, &value); err != nil {
1038+
return types.NewErr("claim %q failed to unmarshal: %w", name, err)
1039+
}
1040+
return types.DefaultTypeAdapter.NativeToValue(value)
1041+
})
1042+
}
1043+
return lazyMap
1044+
}
1045+
10361046
// convertCELValueToStringList converts the CEL value to a string list.
10371047
// The CEL value needs to be either a string or a list of strings.
10381048
// "", [] are treated as not being present and will return nil.
@@ -1113,50 +1123,17 @@ func checkValidationRulesEvaluation(results []authenticationcel.EvaluationResult
11131123
return nil
11141124
}
11151125

1116-
func convertObjectToUnstructured(obj interface{}) (*unstructured.Unstructured, error) {
1117-
if obj == nil || reflect.ValueOf(obj).IsNil() {
1118-
return &unstructured.Unstructured{Object: nil}, nil
1119-
}
1120-
ret, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj)
1121-
if err != nil {
1122-
return nil, err
1123-
}
1124-
return &unstructured.Unstructured{Object: ret}, nil
1125-
}
1126-
1127-
func convertUserInfoToUnstructured(info user.Info) (*unstructured.Unstructured, error) {
1128-
userInfo := &authenticationv1.UserInfo{
1129-
Extra: make(map[string]authenticationv1.ExtraValue),
1130-
Groups: info.GetGroups(),
1131-
UID: info.GetUID(),
1132-
Username: info.GetName(),
1133-
}
1134-
// Convert the extra information in the user object
1135-
for key, val := range info.GetExtra() {
1136-
userInfo.Extra[key] = authenticationv1.ExtraValue(val)
1137-
}
1138-
1139-
// Convert the user info to unstructured so that we can evaluate the CEL expressions
1140-
// against the user info. This is done once here so that we don't have to convert
1141-
// the user info to unstructured multiple times in the CEL mapper for each mapping.
1142-
userInfoUnstructured, err := convertObjectToUnstructured(userInfo)
1143-
if err != nil {
1144-
return nil, err
1145-
}
1146-
1147-
// check if the user info contains the required fields. If not, set them to empty values.
1148-
// This is done because the CEL expressions expect these fields to be present.
1149-
if userInfoUnstructured.Object["username"] == nil {
1150-
userInfoUnstructured.Object["username"] = ""
1151-
}
1152-
if userInfoUnstructured.Object["uid"] == nil {
1153-
userInfoUnstructured.Object["uid"] = ""
1154-
}
1155-
if userInfoUnstructured.Object["groups"] == nil {
1156-
userInfoUnstructured.Object["groups"] = []string{}
1157-
}
1158-
if userInfoUnstructured.Object["extra"] == nil {
1159-
userInfoUnstructured.Object["extra"] = map[string]authenticationv1.ExtraValue{}
1126+
func newUserInfoValue(info user.Info) *lazy.MapValue {
1127+
lazyMap := lazy.NewMapValue(types.NewObjectType("kubernetes.UserInfo"))
1128+
field := func(name string, get func() any) {
1129+
lazyMap.Append(name, func(_ *lazy.MapValue) ref.Val {
1130+
value := get()
1131+
return types.DefaultTypeAdapter.NativeToValue(value)
1132+
})
11601133
}
1161-
return userInfoUnstructured, nil
1134+
field("username", func() any { return info.GetName() })
1135+
field("uid", func() any { return info.GetUID() })
1136+
field("groups", func() any { return info.GetGroups() })
1137+
field("extra", func() any { return info.GetExtra() })
1138+
return lazyMap
11621139
}

0 commit comments

Comments
 (0)