Skip to content

Commit 44e91ac

Browse files
onutconutc
committed
fix(owner): make spritz ownership id-only
1 parent ce9e181 commit 44e91ac

6 files changed

Lines changed: 134 additions & 24 deletions

File tree

‎api/main.go‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -293,9 +293,6 @@ func (s *server) createSpritz(c echo.Context) error {
293293
return writeError(c, http.StatusBadRequest, "spec.owner.id is required")
294294
}
295295
}
296-
if owner.Email == "" {
297-
owner.Email = principal.Email
298-
}
299296
if s.auth.enabled() && !principal.IsAdmin && owner.ID != principal.ID {
300297
return writeError(c, http.StatusForbidden, "owner mismatch")
301298
}

‎api/main_create_owner_test.go‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
package main
2+
3+
import (
4+
"bytes"
5+
"encoding/json"
6+
"net/http"
7+
"net/http/httptest"
8+
"testing"
9+
10+
"github.com/labstack/echo/v4"
11+
"k8s.io/apimachinery/pkg/runtime"
12+
"sigs.k8s.io/controller-runtime/pkg/client/fake"
13+
14+
spritzv1 "spritz.sh/operator/api/v1"
15+
)
16+
17+
func newTestSpritzScheme(t *testing.T) *runtime.Scheme {
18+
t.Helper()
19+
scheme := runtime.NewScheme()
20+
if err := spritzv1.AddToScheme(scheme); err != nil {
21+
t.Fatalf("failed to register spritz scheme: %v", err)
22+
}
23+
return scheme
24+
}
25+
26+
func newCreateSpritzTestServer(t *testing.T) *server {
27+
t.Helper()
28+
scheme := newTestSpritzScheme(t)
29+
return &server{
30+
client: fake.NewClientBuilder().WithScheme(scheme).Build(),
31+
scheme: scheme,
32+
namespace: "spritz-test",
33+
auth: authConfig{
34+
mode: authModeHeader,
35+
headerID: "X-Spritz-User-Id",
36+
headerEmail: "X-Spritz-User-Email",
37+
},
38+
internalAuth: internalAuthConfig{enabled: false},
39+
userConfigPolicy: userConfigPolicy{},
40+
}
41+
}
42+
43+
func TestCreateSpritzOwnerUsesIDAndOmitsEmail(t *testing.T) {
44+
s := newCreateSpritzTestServer(t)
45+
e := echo.New()
46+
secured := e.Group("", s.authMiddleware())
47+
secured.POST("/api/spritzes", s.createSpritz)
48+
49+
body := []byte(`{"name":"tidal-ember","spec":{"image":"example.com/spritz:latest"}}`)
50+
req := httptest.NewRequest(http.MethodPost, "/api/spritzes", bytes.NewReader(body))
51+
req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON)
52+
req.Header.Set("X-Spritz-User-Id", "bcf6c03e-51a1-4f05-97d8-d616405b42a2")
53+
req.Header.Set("X-Spritz-User-Email", "bcf6c03e-51a1-4f05-97d8-d616405b42a2")
54+
rec := httptest.NewRecorder()
55+
56+
e.ServeHTTP(rec, req)
57+
58+
if rec.Code != http.StatusCreated {
59+
t.Fatalf("expected status 201, got %d: %s", rec.Code, rec.Body.String())
60+
}
61+
62+
var payload map[string]any
63+
if err := json.Unmarshal(rec.Body.Bytes(), &payload); err != nil {
64+
t.Fatalf("failed to decode response json: %v", err)
65+
}
66+
data, ok := payload["data"].(map[string]any)
67+
if !ok {
68+
t.Fatalf("expected data object in response, got %#v", payload["data"])
69+
}
70+
spec, ok := data["spec"].(map[string]any)
71+
if !ok {
72+
t.Fatalf("expected spec object in response, got %#v", data["spec"])
73+
}
74+
owner, ok := spec["owner"].(map[string]any)
75+
if !ok {
76+
t.Fatalf("expected owner object in response, got %#v", spec["owner"])
77+
}
78+
if owner["id"] != "bcf6c03e-51a1-4f05-97d8-d616405b42a2" {
79+
t.Fatalf("expected owner.id to be principal id, got %#v", owner["id"])
80+
}
81+
if _, exists := owner["email"]; exists {
82+
t.Fatalf("expected owner.email to be omitted from response, got %#v", owner["email"])
83+
}
84+
}
85+
86+
func TestCreateSpritzRejectsOwnerIDMismatchForNonAdmin(t *testing.T) {
87+
s := newCreateSpritzTestServer(t)
88+
e := echo.New()
89+
secured := e.Group("", s.authMiddleware())
90+
secured.POST("/api/spritzes", s.createSpritz)
91+
92+
body := []byte(`{"name":"tidal-ember","spec":{"image":"example.com/spritz:latest","owner":{"id":"someone-else"}}}`)
93+
req := httptest.NewRequest(http.MethodPost, "/api/spritzes", bytes.NewReader(body))
94+
req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON)
95+
req.Header.Set("X-Spritz-User-Id", "current-user")
96+
rec := httptest.NewRecorder()
97+
98+
e.ServeHTTP(rec, req)
99+
100+
if rec.Code != http.StatusForbidden {
101+
t.Fatalf("expected status 403, got %d: %s", rec.Code, rec.Body.String())
102+
}
103+
}

‎crd/generated/spritz.sh_spritzes.yaml‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -268,9 +268,6 @@ spec:
268268
owner:
269269
description: SpritzOwner identifies the creator of a spritz.
270270
properties:
271-
email:
272-
format: email
273-
type: string
274271
id:
275272
minLength: 1
276273
type: string

‎crd/spritz.sh_spritzes.yaml‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -268,9 +268,6 @@ spec:
268268
owner:
269269
description: SpritzOwner identifies the creator of a spritz.
270270
properties:
271-
email:
272-
format: email
273-
type: string
274271
id:
275272
minLength: 1
276273
type: string

‎helm/spritz/crds/spritz.sh_spritzes.yaml‎

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,6 @@ spec:
5656
type: object
5757
spec:
5858
description: SpritzSpec defines the desired state of Spritz.
59-
x-kubernetes-validations:
60-
- message: spec.repo and spec.repos are mutually exclusive
61-
rule: '!(has(self.repo) && has(self.repos) && size(self.repos) > 0)'
6259
properties:
6360
annotations:
6461
additionalProperties:
@@ -271,9 +268,6 @@ spec:
271268
owner:
272269
description: SpritzOwner identifies the creator of a spritz.
273270
properties:
274-
email:
275-
format: email
276-
type: string
277271
id:
278272
minLength: 1
279273
type: string
@@ -356,19 +350,17 @@ spec:
356350
- url
357351
type: object
358352
repos:
359-
description: Repos holds multiple repositories to clone. When set,
360-
repo is ignored.
361353
items:
362-
description: SpritzRepo describes the repository to clone inside the
363-
workload.
354+
description: SpritzRepo describes the repository to clone inside
355+
the workload.
364356
properties:
365357
auth:
366358
description: SpritzRepoAuth describes how to authenticate git
367359
clone operations.
368360
properties:
369361
netrcKey:
370-
description: NetrcKey points to a Secret key containing a
371-
full .netrc file.
362+
description: NetrcKey points to a Secret key containing
363+
a full .netrc file.
372364
type: string
373365
passwordKey:
374366
description: PasswordKey points to a Secret key containing
@@ -461,6 +453,29 @@ spec:
461453
More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
462454
type: object
463455
type: object
456+
sharedMounts:
457+
description: SharedMounts configures per-spritz shared directories.
458+
items:
459+
properties:
460+
mode:
461+
type: string
462+
mountPath:
463+
type: string
464+
name:
465+
type: string
466+
pollSeconds:
467+
type: integer
468+
publishSeconds:
469+
type: integer
470+
scope:
471+
type: string
472+
syncMode:
473+
type: string
474+
required:
475+
- mountPath
476+
- name
477+
type: object
478+
type: array
464479
ssh:
465480
description: SpritzSSH configures SSH access behavior.
466481
properties:
@@ -500,6 +515,9 @@ spec:
500515
- image
501516
- owner
502517
type: object
518+
x-kubernetes-validations:
519+
- message: spec.repo and spec.repos are mutually exclusive
520+
rule: '!(has(self.repo) && has(self.repos) && size(self.repos) > 0)'
503521
status:
504522
description: SpritzStatus defines the observed state of Spritz.
505523
properties:

‎operator/api/v1/spritz_types.go‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,7 @@ type SpritzRepoAuth struct {
6161
type SpritzOwner struct {
6262
// +kubebuilder:validation:MinLength=1
6363
ID string `json:"id"`
64-
// +kubebuilder:validation:Format=email
65-
Email string `json:"email,omitempty"`
66-
Team string `json:"team,omitempty"`
64+
Team string `json:"team,omitempty"`
6765
}
6866

6967
// SpritzFeatures toggles optional capabilities.

0 commit comments

Comments
 (0)