feat: add suport for custom triger authentication#1
Conversation
WalkthroughThis pull request adds optional gRPC authentication support to CustomTrigger. Two new fields, AuthToken (a SecretKeySelector) and AuthHeader (a string defaulting to "authorization"), are introduced in the Go type and propagated through generated OpenAPI, JSON schema, swagger, protobuf, and deepcopy artifacts. The custom-trigger implementation adds a bearerToken type implementing grpc.PerRPCCredentials, wired into dial-option construction to send the token as gRPC metadata, with accompanying unit tests. Documentation and an example manifest are updated to describe and demonstrate the new fields. Changes
Sequence Diagram(s)sequenceDiagram
participant NewCustomTrigger
participant SecretVolume
participant bearerToken
participant grpc.Dial
NewCustomTrigger->>SecretVolume: read AuthToken secret
SecretVolume-->>NewCustomTrigger: token value
NewCustomTrigger->>bearerToken: construct with token, AuthHeader
NewCustomTrigger->>grpc.Dial: WithPerRPCCredentials(bearerToken)
grpc.Dial->>bearerToken: GetRequestMetadata()
bearerToken-->>grpc.Dial: {authHeader: "Bearer <token>"}
Compact metadata:
Related issues: None specified Suggested labels: enhancement, api-change, documentation Suggested reviewers: None specified Poem:
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
pkg/apis/events/v1alpha1/sensor_types.go (1)
669-673: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winNothing enforces
SecurewhenAuthTokenis set.Per the downstream
bearerToken.GetRequestMetadataimplementation, the token is sent as"Bearer " + tokengRPC metadata on every request regardless of whetherSecureis true. Docs recommend enablingsecurebut nothing validates or warns if a user setsAuthTokenwithoutSecure: true, risking the token being sent in plaintext over the wire.Consider adding a webhook/CRD validation rule (or at minimum a runtime warning) requiring
Secure: truewhenAuthTokenis set.examples/sensors/custom-trigger.yaml (1)
16-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExample doesn't demonstrate the recommended
secure: truepairing.The comment says to "Enable
secureto protect the token in transit," but the example doesn't setsecure: truealongsideauthToken. Since this file is a reference example, showing the secure configuration would reinforce the documented best practice.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b7c5e204-693f-4065-8623-861c017f6e64
⛔ Files ignored due to path filters (1)
pkg/apis/events/v1alpha1/generated.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (11)
api/jsonschema/schema.jsonapi/openapi-spec/swagger.jsondocs/APIs.mddocs/sensors/triggers/build-your-own-trigger.mdexamples/sensors/custom-trigger.yamlpkg/apis/events/openapi/openapi_generated.gopkg/apis/events/v1alpha1/generated.protopkg/apis/events/v1alpha1/sensor_types.gopkg/apis/events/v1alpha1/zz_generated.deepcopy.gopkg/sensors/triggers/custom-trigger/custom-trigger.gopkg/sensors/triggers/custom-trigger/custom-trigger_test.go
| if ct.AuthToken != nil { | ||
| token, err := sharedutil.GetSecretFromVolume(ct.AuthToken) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to retrieve the auth token, %w", err) | ||
| } | ||
| if !ct.Secure { | ||
| logger.Warn("sending the auth token over an insecure connection, consider enabling 'secure' to protect the credentials in transit") | ||
| } | ||
| opt = append(opt, grpc.WithPerRPCCredentials(bearerToken{header: ct.AuthHeader, token: token, requireTransportSecurity: ct.Secure})) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
grpc-go PerRPCCredentials RequireTransportSecurity insecure transport bearer token
💡 Result:
In gRPC-Go, when you use PerRPCCredentials (such as a bearer token), the library enforces transport security by default to prevent the insecure transmission of credentials [1][2]. ### The Mechanism The credentials.PerRPCCredentials interface includes a method RequireTransportSecurity() bool [3][4]. * When this method returns true (which is the default for standard implementations like OAuth2/JWT) [2], the gRPC client checks the transport credentials configured for the connection [5]. * If you are using an insecure transport (e.g., via credentials/insecure), the gRPC client will fail to establish the connection or the RPC, as it detects a mismatch between the security requirements of your per-RPC credentials and the underlying transport security [5]. ### How to Resolve the Conflict You have two primary options depending on your security requirements: 1. Enable Transport Security (Recommended): If you must send bearer tokens, the best practice is to configure the transport with TLS [1]. go import "google.golang.org/grpc/credentials" // Example: Using TLS transport creds := credentials.NewClientTLSFromCert(nil, "") conn, err := grpc.NewClient(serverAddr, grpc.WithTransportCredentials(creds), grpc.WithPerRPCCredentials(myTokenAuth), ) 2. Disable Transport Security (Not Recommended for Production): If you are working in a controlled, trusted environment (e.g., local development or service mesh) and absolutely need to disable transport security while using credentials that require it, you must implement a custom PerRPCCredentials type where RequireTransportSecurity() returns false. go type InsecureTokenAuth struct { Token string } func (t InsecureTokenAuth) GetRequestMetadata(ctx context.Context, uri ...string) (map[string]string, error) { return map[string]string{"authorization": "Bearer " + t.Token}, nil } func (t InsecureTokenAuth) RequireTransportSecurity() bool { return false // Overriding to false allows use over insecure transport } Warning: Setting RequireTransportSecurity() to false allows your credentials to be sent in plaintext. Ensure your network is secure. ### Summary The grpc: the credentials require transport level security error occurs because PerRPCCredentials is explicitly signaling that it should not be used over an insecure connection [5]. Overriding RequireTransportSecurity() to return false is the mechanism provided to bypass this safety check when necessary [6].
Citations:
- 1: https://github.com/grpc/grpc-go/blob/master/Documentation/grpc-auth-support.md
- 2: https://github.com/grpc/grpc-go/blob/master/credentials/oauth/oauth.go
- 3: https://pkg.go.dev/google.golang.org/grpc/credentials
- 4: https://github.com/grpc/grpc-go/blob/master/credentials/credentials.go
- 5: https://github.com/grpc/grpc-go/blob/v1.79.3/clientconn.go
- 6: https://groups.google.com/g/grpc-io/c/sN30bEPNr6o
🏁 Script executed:
# Find the bearerToken implementation
cd pkg/sensors/triggers/custom-trigger && grep -n "type bearerToken" custom-trigger.goRepository: pipekit/argo-events
Length of output: 186
🏁 Script executed:
# Look at lines 38-60 where bearerToken is defined (based on scratchpad context)
cd pkg/sensors/triggers/custom-trigger && sed -n '38,80p' custom-trigger.goRepository: pipekit/argo-events
Length of output: 1433
🏁 Script executed:
# Check if RequireTransportSecurity is implemented
cd pkg/sensors/triggers/custom-trigger && grep -A 5 "RequireTransportSecurity" custom-trigger.goRepository: pipekit/argo-events
Length of output: 352
Fail fast instead of sending bearer tokens over plaintext.
When AuthToken is configured and ct.Secure is false, this sets RequireTransportSecurity to false and still attaches Bearer <token> metadata. A warning does not prevent credential exposure in transit; reject this configuration instead.
Proposed fix
if ct.AuthToken != nil {
+ if !ct.Secure {
+ return nil, fmt.Errorf("invalid config, authToken requires secure connection")
+ }
token, err := sharedutil.GetSecretFromVolume(ct.AuthToken)
if err != nil {
return nil, fmt.Errorf("failed to retrieve the auth token, %w", err)
}
- if !ct.Secure {
- logger.Warn("sending the auth token over an insecure connection, consider enabling 'secure' to protect the credentials in transit")
- }
- opt = append(opt, grpc.WithPerRPCCredentials(bearerToken{header: ct.AuthHeader, token: token, requireTransportSecurity: ct.Secure}))
+ opt = append(opt, grpc.WithPerRPCCredentials(bearerToken{header: ct.AuthHeader, token: token, requireTransportSecurity: true}))
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ct.AuthToken != nil { | |
| token, err := sharedutil.GetSecretFromVolume(ct.AuthToken) | |
| if err != nil { | |
| return nil, fmt.Errorf("failed to retrieve the auth token, %w", err) | |
| } | |
| if !ct.Secure { | |
| logger.Warn("sending the auth token over an insecure connection, consider enabling 'secure' to protect the credentials in transit") | |
| } | |
| opt = append(opt, grpc.WithPerRPCCredentials(bearerToken{header: ct.AuthHeader, token: token, requireTransportSecurity: ct.Secure})) | |
| if ct.AuthToken != nil { | |
| if !ct.Secure { | |
| return nil, fmt.Errorf("invalid config, authToken requires secure connection") | |
| } | |
| token, err := sharedutil.GetSecretFromVolume(ct.AuthToken) | |
| if err != nil { | |
| return nil, fmt.Errorf("failed to retrieve the auth token, %w", err) | |
| } | |
| opt = append(opt, grpc.WithPerRPCCredentials(bearerToken{header: ct.AuthHeader, token: token, requireTransportSecurity: true})) | |
| } |
1 participant
Checklist: