Skip to content

Commit fd21a23

Browse files
ECOPROJECT-4721 | fix: Validate JWT source_id in agent handlers
Signed-off-by: Aviel Segev <asegev@redhat.com>
1 parent d7ba2e1 commit fd21a23

6 files changed

Lines changed: 213 additions & 44 deletions

File tree

internal/api_server/agentserver/server.go

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -72,15 +72,9 @@ func (s *AgentServer) Run(ctx context.Context) error {
7272
metricMiddleware.Handler,
7373
middleware.RequestID,
7474
log.ConditionalLogger(s.cfg.Service.LogLevel, zap.L(), "router_agent"),
75+
auth.NewAgentAuthenticator(s.cfg.Service.Auth.AgentAuthenticationEnabled, s.store).Authenticator,
7576
)
7677

77-
zap.S().Infow("agent authentication", "enabled", s.cfg.Service.Auth.AgentAuthenticationEnabled)
78-
if s.cfg.Service.Auth.AgentAuthenticationEnabled {
79-
router.Use(
80-
auth.NewAgentAuthenticator(s.store).Authenticator,
81-
)
82-
}
83-
8478
router.Use(
8579
middleware.Recoverer,
8680
oapimiddleware.OapiRequestValidatorWithOptions(swagger, &oapiOpts),

internal/auth/agent_authenticator.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,17 @@ type AgentAuthenticator struct {
7676
store store.Store
7777
}
7878

79-
func NewAgentAuthenticator(store store.Store) *AgentAuthenticator {
79+
func NewAgentAuthenticator(enabled bool, store store.Store) Authenticator {
80+
if enabled {
81+
zap.S().Named("auth").Info("agent authentication enabled")
82+
return newProductionAgentAuthenticator(store)
83+
}
84+
85+
zap.S().Named("auth").Info("agent authentication disabled, using none authenticator")
86+
return NewNoneAgentAuthenticator()
87+
}
88+
89+
func newProductionAgentAuthenticator(store store.Store) *AgentAuthenticator {
8090
return &AgentAuthenticator{store: store}
8191
}
8292

internal/auth/agent_authenticator_test.go

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
package auth_test
22

33
import (
4+
"bytes"
45
"crypto/rand"
56
"crypto/rsa"
67
"crypto/x509"
8+
"encoding/json"
79
"encoding/pem"
810
"fmt"
911
"net/http"
@@ -60,7 +62,7 @@ var _ = Describe("agent authentication", Ordered, func() {
6062

6163
token := generateAgentToken("kid", "my_source", "GothamCity", privateKey)
6264

63-
agentAuthenticator := auth.NewAgentAuthenticator(s)
65+
agentAuthenticator := auth.NewAgentAuthenticator(true, s).(*auth.AgentAuthenticator)
6466

6567
agentJwt, err := agentAuthenticator.Authenticate(token)
6668
Expect(err).To(BeNil())
@@ -87,7 +89,7 @@ var _ = Describe("agent authentication", Ordered, func() {
8789

8890
token := generateAgentToken("missing-key-kid", "my_source", "GothamCity", privateKey)
8991

90-
agentAuthenticator := auth.NewAgentAuthenticator(s)
92+
agentAuthenticator := auth.NewAgentAuthenticator(true, s).(*auth.AgentAuthenticator)
9193

9294
_, err = agentAuthenticator.Authenticate(token)
9395
Expect(err).ToNot(BeNil())
@@ -112,7 +114,7 @@ var _ = Describe("agent authentication", Ordered, func() {
112114

113115
token := generateAgentToken("kid", "my_source", "GothamCity", signingKey)
114116

115-
agentAuthenticator := auth.NewAgentAuthenticator(s)
117+
agentAuthenticator := auth.NewAgentAuthenticator(true, s).(*auth.AgentAuthenticator)
116118

117119
_, err = agentAuthenticator.Authenticate(token)
118120
Expect(err).ToNot(BeNil())
@@ -140,7 +142,7 @@ var _ = Describe("agent authentication", Ordered, func() {
140142

141143
token := generateAgentToken("1234_kid", "my_source", "org_id", privateKey)
142144

143-
agentAuthenticator := auth.NewAgentAuthenticator(s)
145+
agentAuthenticator := auth.NewAgentAuthenticator(true, s)
144146
h := &handler{}
145147
ts := httptest.NewServer(agentAuthenticator.Authenticator(h))
146148
defer ts.Close()
@@ -159,6 +161,36 @@ var _ = Describe("agent authentication", Ordered, func() {
159161
})
160162
})
161163

164+
Context("none authenticator middleware", func() {
165+
It("successfully extracts sourceId from request body", func() {
166+
innerHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
167+
// Verify AgentJWT is in context
168+
agentJWT := auth.MustHaveAgent(r.Context())
169+
Expect(agentJWT.SourceID).To(Equal("test-source-123"))
170+
171+
// Verify body is still readable for downstream
172+
var body map[string]interface{}
173+
err := json.NewDecoder(r.Body).Decode(&body)
174+
Expect(err).To(BeNil(), "Body should be readable by downstream handler")
175+
Expect(body["sourceId"]).To(Equal("test-source-123"))
176+
177+
w.WriteHeader(200)
178+
})
179+
180+
noneAuthenticator := auth.NewNoneAgentAuthenticator()
181+
ts := httptest.NewServer(noneAuthenticator.Authenticator(innerHandler))
182+
defer ts.Close()
183+
184+
body := `{"sourceId":"test-source-123"}`
185+
req, err := http.NewRequest(http.MethodPost, ts.URL, bytes.NewBufferString(body))
186+
Expect(err).To(BeNil())
187+
188+
resp, rerr := http.DefaultClient.Do(req)
189+
Expect(rerr).To(BeNil())
190+
Expect(resp.StatusCode).To(Equal(200))
191+
})
192+
})
193+
162194
})
163195

164196
func generateAgentToken(kid, sourceID, orgID string, signingKey *rsa.PrivateKey) string {
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
package auth
2+
3+
import (
4+
"bytes"
5+
"encoding/json"
6+
"fmt"
7+
"io"
8+
"net/http"
9+
"time"
10+
)
11+
12+
type NoneAgentAuthenticator struct{}
13+
14+
func NewNoneAgentAuthenticator() *NoneAgentAuthenticator {
15+
return &NoneAgentAuthenticator{}
16+
}
17+
18+
func (n *NoneAgentAuthenticator) Authenticator(next http.Handler) http.Handler {
19+
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
20+
bodyBytes, err := io.ReadAll(r.Body)
21+
if err != nil {
22+
http.Error(w, fmt.Sprintf("failed to read request body: %v", err), http.StatusBadRequest)
23+
return
24+
}
25+
_ = r.Body.Close()
26+
27+
var req struct {
28+
SourceID string `json:"sourceId"`
29+
}
30+
31+
if err := json.Unmarshal(bodyBytes, &req); err != nil {
32+
http.Error(w, fmt.Sprintf("missing source id from request body: %v", err), http.StatusBadRequest)
33+
return
34+
}
35+
36+
r.Body = io.NopCloser(bytes.NewReader(bodyBytes))
37+
38+
agentJWT := AgentJWT{
39+
ExpireAt: time.Now().Add(defaultExpirationPeriod * time.Hour),
40+
IssueAt: time.Now(),
41+
Issuer: "none",
42+
OrgID: "internal",
43+
SourceID: req.SourceID,
44+
}
45+
ctx := NewTokenContext(r.Context(), agentJWT)
46+
next.ServeHTTP(w, r.WithContext(ctx))
47+
})
48+
}

internal/handlers/v1alpha1/agent.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77

88
v1alpha1 "github.com/kubev2v/migration-planner/api/v1alpha1/agent"
99
agentServer "github.com/kubev2v/migration-planner/internal/api/server/agent"
10+
"github.com/kubev2v/migration-planner/internal/auth"
1011
apiMappers "github.com/kubev2v/migration-planner/internal/handlers/v1alpha1/mappers"
1112
"github.com/kubev2v/migration-planner/internal/handlers/validator"
1213
"github.com/kubev2v/migration-planner/internal/service"
@@ -31,6 +32,13 @@ func (h *AgentHandler) UpdateSourceInventory(ctx context.Context, request agentS
3132
return agentServer.UpdateSourceInventory400JSONResponse{Message: "empty body"}, nil
3233
}
3334

35+
agentJWT := auth.MustHaveAgent(ctx)
36+
if agentJWT.SourceID != request.Id.String() {
37+
return agentServer.UpdateSourceInventory403JSONResponse{
38+
Message: fmt.Sprintf("agent is not authorized to update source %s", request.Id),
39+
}, nil
40+
}
41+
3442
data, err := json.Marshal(request.Body.Inventory)
3543
if err != nil {
3644
return agentServer.UpdateSourceInventory500JSONResponse{Message: err.Error()}, nil
@@ -74,6 +82,13 @@ func (h *AgentHandler) UpdateAgentStatus(ctx context.Context, request agentServe
7482
return agentServer.UpdateAgentStatus400JSONResponse{Message: err.Error()}, nil
7583
}
7684

85+
agentJWT := auth.MustHaveAgent(ctx)
86+
if agentJWT.SourceID != request.Body.SourceId.String() {
87+
return agentServer.UpdateAgentStatus403JSONResponse{
88+
Message: fmt.Sprintf("agent is not authorized to update source %s", request.Body.SourceId),
89+
}, nil
90+
}
91+
7792
_, created, err := h.srv.UpdateAgentStatus(ctx, mappers.AgentUpdateForm{
7893
ID: request.Id,
7994
SourceID: request.Body.SourceId,

0 commit comments

Comments
 (0)