Customizable client authorization #84
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| package v1alpha1 | ||
|
|
||
| import ( | ||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| ) | ||
|
|
||
| // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object | ||
|
|
||
| // AuthorizationConfiguration provides versioned configuration for authorization. | ||
| type AuthorizationConfiguration struct { | ||
| metav1.TypeMeta | ||
| Type string `json:"type"` | ||
| CEL *CELConfiguration `json:"cel,omitempty"` | ||
| } | ||
|
|
||
| type CELConfiguration struct { | ||
| Expression string `json:"expression"` | ||
| } | ||
|
|
||
| func init() { | ||
| SchemeBuilder.Register(&AuthorizationConfiguration{}) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,128 @@ | ||
| package authorization | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
|
|
||
| celgo "github.com/google/cel-go/cel" | ||
| jumpstarterdevv1alpha1 "github.com/jumpstarter-dev/jumpstarter-controller/api/v1alpha1" | ||
| "k8s.io/apimachinery/pkg/runtime" | ||
| "k8s.io/apiserver/pkg/authorization/authorizer" | ||
| "k8s.io/apiserver/pkg/authorization/cel" | ||
| "k8s.io/apiserver/pkg/cel/environment" | ||
| "sigs.k8s.io/controller-runtime/pkg/client" | ||
| ) | ||
|
|
||
| type CELAuthorizer struct { | ||
| reader client.Reader | ||
| prefix string | ||
| program celgo.Program | ||
| } | ||
| type Expression struct { | ||
| Expression string | ||
| } | ||
|
|
||
| func (v *Expression) GetExpression() string { | ||
| return v.Expression | ||
| } | ||
|
|
||
| func (v *Expression) ReturnTypes() []*celgo.Type { | ||
| return []*celgo.Type{celgo.BoolType} | ||
| } | ||
|
|
||
| func NewCELAuthorizer(reader client.Reader, prefix string, expression string) (authorizer.Authorizer, error) { | ||
| env, err := environment.MustBaseEnvSet( | ||
| environment.DefaultCompatibilityVersion(), | ||
| false, | ||
| ).Extend(environment.VersionedOptions{ | ||
| IntroducedVersion: environment.DefaultCompatibilityVersion(), | ||
| EnvOptions: []celgo.EnvOption{ | ||
| celgo.Variable("kind", celgo.StringType), | ||
| celgo.Variable("self", celgo.DynType), | ||
| celgo.Variable("user", celgo.DynType), | ||
| }, | ||
| }) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| compiler := cel.NewCompiler(env) | ||
|
|
||
| compiled, err := compiler.CompileCELExpression(&Expression{ | ||
| Expression: expression, | ||
| }) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| return &CELAuthorizer{ | ||
| reader: reader, | ||
| prefix: prefix, | ||
| program: compiled.Program, | ||
| }, nil | ||
| } | ||
|
|
||
| func (b *CELAuthorizer) Authorize( | ||
| ctx context.Context, | ||
| attributes authorizer.Attributes, | ||
| ) (authorizer.Decision, string, error) { | ||
| var self map[string]interface{} | ||
| var err error | ||
|
|
||
| switch attributes.GetResource() { | ||
| case "Exporter": | ||
| var e jumpstarterdevv1alpha1.Exporter | ||
| if err := b.reader.Get(ctx, client.ObjectKey{ | ||
| Namespace: attributes.GetNamespace(), | ||
| Name: attributes.GetName(), | ||
| }, &e); err != nil { | ||
| return authorizer.DecisionDeny, "failed to get exporter", err | ||
| } | ||
| self, err = runtime.DefaultUnstructuredConverter.ToUnstructured(&e) | ||
| if err != nil { | ||
| return authorizer.DecisionDeny, "failed to serialize exporter", err | ||
| } | ||
| self["spec"].(map[string]any)["username"] = e.Username(b.prefix) | ||
| case "Client": | ||
| var c jumpstarterdevv1alpha1.Client | ||
| if err := b.reader.Get(ctx, client.ObjectKey{ | ||
| Namespace: attributes.GetNamespace(), | ||
| Name: attributes.GetName(), | ||
| }, &c); err != nil { | ||
| return authorizer.DecisionDeny, "failed to get client", err | ||
| } | ||
| self, err = runtime.DefaultUnstructuredConverter.ToUnstructured(&c) | ||
| if err != nil { | ||
| return authorizer.DecisionDeny, "failed to serialize client", err | ||
| } | ||
| self["spec"].(map[string]any)["username"] = c.Username(b.prefix) | ||
| default: | ||
| return authorizer.DecisionDeny, "invalid object kind", nil | ||
| } | ||
|
|
||
| user := attributes.GetUser() | ||
| value, _, err := b.program.Eval(map[string]any{ | ||
| "kind": attributes.GetResource(), | ||
| "self": self, | ||
| "user": map[string]any{ | ||
| "username": user.GetName(), | ||
| "uid": user.GetUID(), | ||
| "groups": user.GetGroups(), | ||
| "extra": user.GetExtra(), | ||
| }, | ||
| }) | ||
| if err != nil { | ||
| return authorizer.DecisionDeny, "failed to evaluate expression", err | ||
| } | ||
|
|
||
| result, ok := value.Value().(bool) | ||
| if !ok { | ||
| return authorizer.DecisionDeny, "failed to evaluate expression", fmt.Errorf("result type mismatch") | ||
| } | ||
|
|
||
| if result { | ||
| return authorizer.DecisionAllow, "", nil | ||
| } else { | ||
| return authorizer.DecisionDeny, "permission denied", nil | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| package authorization | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
|
|
||
| jumpstarterdevv1alpha1 "github.com/jumpstarter-dev/jumpstarter-controller/api/v1alpha1" | ||
| "k8s.io/apimachinery/pkg/runtime" | ||
| "k8s.io/apimachinery/pkg/runtime/serializer" | ||
| "k8s.io/apiserver/pkg/authorization/authorizer" | ||
| "sigs.k8s.io/controller-runtime/pkg/client" | ||
| ) | ||
|
|
||
| func LoadAuthorizationConfiguration( | ||
| ctx context.Context, | ||
| scheme *runtime.Scheme, | ||
| configuration []byte, | ||
| reader client.Reader, | ||
| prefix string, | ||
| ) (authorizer.Authorizer, error) { | ||
| var authorizationConfiguration jumpstarterdevv1alpha1.AuthorizationConfiguration | ||
| if err := runtime.DecodeInto( | ||
| serializer.NewCodecFactory(scheme, serializer.EnableStrict). | ||
| UniversalDecoder(jumpstarterdevv1alpha1.GroupVersion), | ||
| configuration, | ||
| &authorizationConfiguration, | ||
| ); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| switch authorizationConfiguration.Type { | ||
| case "Basic": | ||
| return NewBasicAuthorizer(reader, prefix), nil | ||
| case "CEL": | ||
| if authorizationConfiguration.CEL == nil { | ||
| return nil, fmt.Errorf("CEL authorizer configuration missing") | ||
| } | ||
| return NewCELAuthorizer(reader, prefix, authorizationConfiguration.CEL.Expression) | ||
| default: | ||
| return nil, fmt.Errorf("unsupported authorizer type: %s", authorizationConfiguration.Type) | ||
| } | ||
| } | ||
|
Comment on lines
+31
to
+42
There was a problem hiding this comment. 💡 Verification agent 🧩 Analysis chainVerify CEL expression validity during initialization. For the CEL authorizer type, it would be beneficial to validate the expression syntax during initialization to fail fast on invalid expressions. 🏁 Script executed: #!/bin/bash
# Description: Search for CEL expression validation in the codebase.
# Test: Look for CEL expression validation in NewCELAuthorizer.
ast-grep --pattern $'func NewCELAuthorizer($_, $_, $expr string) ($_, error) {
$$$
}'
# Test: Look for CEL expression validation in cel.go.
rg -A 5 'NewCELAuthorizer'Length of output: 966 CEL expression validation: Additional check required during initialization The current implementation calls
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Authorize()method handles resource fetching and CEL evaluation logically.ExporterorClientis correct, but consider gracefully handling the case wherespecdoesn't exist before using.(map[string]any), to avoid potential runtime panics.DecisionAlloworDecisionDenyis consistent with standard authorizer patterns.Apply a safer approach for type assertions on
self["spec"]to prevent panics, for example:Also applies to: 72-85, 87-99, 100-101, 103-116, 118-127