Skip to content

Commit a56c5a9

Browse files
authored
support signing for upload endpoints (#2376)
1 parent 865d5a2 commit a56c5a9

File tree

8 files changed

+323
-16
lines changed

8 files changed

+323
-16
lines changed

taco/internal/api/routes.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -271,8 +271,10 @@ func RegisterRoutes(e *echo.Echo, deps Dependencies) {
271271
// Upload endpoints exempt from auth middleware (Terraform doesn't send auth headers)
272272
// Security: These validate lock ownership and have RBAC checks in handlers
273273
// Upload URLs can only be obtained from authenticated CreateStateVersion calls
274-
e.PUT("/tfe/api/v2/state-versions/:id/upload", tfeHandler.UploadStateVersion)
275-
e.PUT("/tfe/api/v2/state-versions/:id/json-upload", tfeHandler.UploadJSONStateOutputs)
274+
tfeSignedUrlsGroup := e.Group("/tfe/api/v2")
275+
tfeSignedUrlsGroup.Use(middleware.VerifySignedURL)
276+
tfeSignedUrlsGroup.PUT("/state-versions/:id/upload", tfeHandler.UploadStateVersion)
277+
tfeSignedUrlsGroup.PUT("/state-versions/:id/json-upload", tfeHandler.UploadJSONStateOutputs)
276278

277279
// Keep discovery endpoints unprotected (needed for terraform login)
278280
e.GET("/.well-known/terraform.json", tfeHandler.GetWellKnownJson)

taco/internal/auth/signed_url.go

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
package auth
2+
3+
import (
4+
"crypto/hmac"
5+
"crypto/sha256"
6+
"encoding/base64"
7+
"fmt"
8+
"net/url"
9+
"strconv"
10+
"time"
11+
12+
"github.com/diggerhq/digger/opentaco/internal/config"
13+
)
14+
15+
16+
// SignURL creates a signed URL valid until expiry
17+
func SignURL(baseURL, path string, expiry time.Time) (string, error) {
18+
secret, err := config.GetConfig().GetSecretKey()
19+
if err != nil {
20+
return "", fmt.Errorf("signing secret key error: %w", err)
21+
}
22+
u, _ := url.Parse(baseURL)
23+
u.Path = path
24+
q := u.Query()
25+
q.Set("exp", fmt.Sprint(expiry.Unix()))
26+
27+
// Compute HMAC
28+
mac := hmac.New(sha256.New, []byte(secret))
29+
mac.Write([]byte(path + q.Get("exp")))
30+
sig := base64.URLEncoding.EncodeToString(mac.Sum(nil))
31+
q.Set("sig", sig)
32+
u.RawQuery = q.Encode()
33+
34+
return u.String(), nil
35+
}
36+
37+
38+
func VerifySignedUrl(signedUrl string) error {
39+
secret, err := config.GetConfig().GetSecretKey()
40+
if err != nil {
41+
return fmt.Errorf("signing secret key error: %w", err)
42+
}
43+
44+
u, _ := url.Parse(signedUrl)
45+
path := u.Path
46+
query := u.Query()
47+
exp := query.Get("exp")
48+
sig := query.Get("sig")
49+
50+
// Parse expiry timestamp
51+
unix, err := strconv.ParseInt(exp, 10, 64)
52+
if err != nil {
53+
return fmt.Errorf("signing expired url error: %w", err)
54+
}
55+
if time.Now().Unix() > unix {
56+
return fmt.Errorf("the signed url is expired")
57+
}
58+
59+
// Compute expected signature
60+
mac := hmac.New(sha256.New, []byte(secret))
61+
mac.Write([]byte(path + exp))
62+
expectedSig := base64.URLEncoding.EncodeToString(mac.Sum(nil))
63+
64+
if !hmac.Equal([]byte(sig), []byte(expectedSig)) {
65+
return fmt.Errorf("the signed url is invalid")
66+
}
67+
return nil
68+
}
Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
package auth
2+
3+
import (
4+
"errors"
5+
"net/url"
6+
"testing"
7+
"time"
8+
9+
"github.com/diggerhq/digger/opentaco/internal/config"
10+
)
11+
12+
// helper: set mock config for each test
13+
func withMockConfig(t *testing.T, secret string, err error, fn func()) {
14+
orig := config.GetConfig() // capture current provider
15+
16+
config.SetConfig(&config.MockConfig{
17+
Secret: secret,
18+
Err: err,
19+
})
20+
21+
t.Cleanup(func() {
22+
config.SetConfig(orig)
23+
})
24+
25+
fn()
26+
}
27+
28+
func TestSignAndVerify_Success(t *testing.T) {
29+
withMockConfig(t, "test-secret", nil, func() {
30+
exp := time.Now().Add(1 * time.Hour)
31+
32+
signed, err := SignURL("https://example.com", "/files/123", exp)
33+
if err != nil {
34+
t.Fatalf("SignURL() unexpected error: %v", err)
35+
}
36+
37+
if err := VerifySignedUrl(signed); err != nil {
38+
t.Fatalf("VerifySignedUrl() unexpected error: %v", err)
39+
}
40+
})
41+
}
42+
43+
func TestVerifySignedUrl_Expired(t *testing.T) {
44+
withMockConfig(t, "test-secret", nil, func() {
45+
expired := time.Now().Add(-2 * time.Minute)
46+
47+
signed, err := SignURL("https://example.com", "/files/123", expired)
48+
if err != nil {
49+
t.Fatalf("SignURL() unexpected error: %v", err)
50+
}
51+
52+
if err := VerifySignedUrl(signed); err == nil {
53+
t.Fatalf("expected error for expired URL, got nil")
54+
}
55+
})
56+
}
57+
58+
func TestVerifySignedUrl_TamperedPath(t *testing.T) {
59+
withMockConfig(t, "test-secret", nil, func() {
60+
exp := time.Now().Add(1 * time.Hour)
61+
62+
signed, err := SignURL("https://example.com", "/files/123", exp)
63+
if err != nil {
64+
t.Fatalf("SignURL() unexpected error: %v", err)
65+
}
66+
67+
// parse and change the path AFTER signing
68+
u, _ := url.Parse(signed)
69+
u.Path = "/files/999" // attacker changes resource
70+
tampered := u.String()
71+
72+
if err := VerifySignedUrl(tampered); err == nil {
73+
t.Fatalf("expected invalid signature error for tampered path, got nil")
74+
}
75+
})
76+
}
77+
78+
func TestVerifySignedUrl_TamperedSignature(t *testing.T) {
79+
withMockConfig(t, "test-secret", nil, func() {
80+
exp := time.Now().Add(1 * time.Hour)
81+
82+
signed, err := SignURL("https://example.com", "/files/123", exp)
83+
if err != nil {
84+
t.Fatalf("SignURL() unexpected error: %v", err)
85+
}
86+
87+
u, _ := url.Parse(signed)
88+
q := u.Query()
89+
q.Set("sig", "definitely-wrong-signature")
90+
u.RawQuery = q.Encode()
91+
tampered := u.String()
92+
93+
if err := VerifySignedUrl(tampered); err == nil {
94+
t.Fatalf("expected invalid signature error for tampered signature, got nil")
95+
}
96+
})
97+
}
98+
99+
func TestVerifySignedUrl_BadExpiryFormat(t *testing.T) {
100+
withMockConfig(t, "test-secret", nil, func() {
101+
exp := time.Now().Add(1 * time.Hour)
102+
103+
signed, err := SignURL("https://example.com", "/files/123", exp)
104+
if err != nil {
105+
t.Fatalf("SignURL() unexpected error: %v", err)
106+
}
107+
108+
u, _ := url.Parse(signed)
109+
q := u.Query()
110+
q.Set("exp", "not-a-timestamp") // break exp
111+
u.RawQuery = q.Encode()
112+
bad := u.String()
113+
114+
if err := VerifySignedUrl(bad); err == nil {
115+
t.Fatalf("expected error for bad exp format, got nil")
116+
}
117+
})
118+
}
119+
120+
func TestSignURL_SecretError(t *testing.T) {
121+
withMockConfig(t, "", errors.New("nope"), func() {
122+
_, err := SignURL("https://example.com", "/files/123", time.Now().Add(time.Hour))
123+
if err == nil {
124+
t.Fatalf("expected error because secret retrieval failed in SignURL, got nil")
125+
}
126+
})
127+
}
128+
129+
func TestVerifySignedUrl_SecretError(t *testing.T) {
130+
withMockConfig(t, "", errors.New("nope"), func() {
131+
// doesn't matter if URL shape is valid, config will fail first
132+
testURL := "https://example.com/files/123?exp=123&sig=abc"
133+
134+
if err := VerifySignedUrl(testURL); err == nil {
135+
t.Fatalf("expected error because secret retrieval failed in VerifySignedUrl, got nil")
136+
}
137+
})
138+
}
139+

taco/internal/config/config.go

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
package config
2+
3+
import (
4+
"errors"
5+
"os"
6+
)
7+
8+
// ConfigProvider is what the rest of the code depends on.
9+
// Both Config and MockConfig will satisfy this.
10+
type ConfigProvider interface {
11+
GetSecretKey() (string, error)
12+
}
13+
14+
// Config is the real implementation that reads from env.
15+
type Config struct{}
16+
17+
func (c *Config) GetSecretKey() (string, error) {
18+
secret := os.Getenv("OPENTACO_SECRET_KEY")
19+
if secret == "" {
20+
return "", errors.New("OPENTACO_SECRET_KEY environment variable not set")
21+
}
22+
return secret, nil
23+
}
24+
25+
// MockConfig is a test double you can inject in tests.
26+
type MockConfig struct {
27+
Secret string
28+
Err error
29+
}
30+
31+
func (m *MockConfig) GetSecretKey() (string, error) {
32+
if m.Err != nil {
33+
return "", m.Err
34+
}
35+
return m.Secret, nil
36+
}
37+
38+
// active is the "current" config provider used by prod code.
39+
// default is the real env-based config.
40+
var active ConfigProvider = &Config{}
41+
42+
// GetConfig returns whichever provider is currently active.
43+
func GetConfig() ConfigProvider {
44+
return active
45+
}
46+
47+
// SetConfig lets tests (or special code) replace the active provider.
48+
// You call this in tests to inject MockConfig.
49+
// DO NOT call this in normal runtime code unless you really mean to override.
50+
func SetConfig(p ConfigProvider) {
51+
active = p
52+
}

taco/internal/middleware/auth.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,8 @@ func OpaqueOnlyVerifier(apiTokenMgr *auth.APITokenManager) AccessTokenVerifier {
198198
if apiTokenMgr == nil {
199199
return echo.NewHTTPError(http.StatusInternalServerError, "API token manager not configured")
200200
}
201-
201+
202+
202203
// Default to "default" org (no context available in this verifier)
203204
if _, err := apiTokenMgr.Verify(context.Background(), "default", token); err != nil {
204205
return echo.ErrUnauthorized
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
package middleware
2+
3+
import (
4+
"log/slog"
5+
"net/http"
6+
7+
"github.com/diggerhq/digger/opentaco/internal/auth"
8+
"github.com/labstack/echo/v4"
9+
)
10+
11+
12+
// VerifySignedURL middleware
13+
func VerifySignedURL(next echo.HandlerFunc) echo.HandlerFunc {
14+
return func(c echo.Context) error {
15+
err := auth.VerifySignedUrl(c.Request().URL.String())
16+
if err != nil {
17+
slog.Error("could not verify signature", "error", err.Error())
18+
return c.NoContent(http.StatusUnauthorized)
19+
}
20+
return next(c)
21+
}
22+
}

taco/internal/tfe/workspaces.go

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@ import (
55
"encoding/base64"
66
"encoding/json"
77
"fmt"
8+
9+
"github.com/diggerhq/digger/opentaco/internal/auth"
810
"github.com/diggerhq/digger/opentaco/internal/domain/tfe"
11+
912
"io"
1013
"net/http"
1114
"strings"
@@ -679,9 +682,20 @@ func (h *TfeHandler) CreateStateVersion(c echo.Context) error {
679682

680683
// Build URLs
681684
baseURL := getBaseURL(c)
682-
uploadURL := fmt.Sprintf("%s/tfe/api/v2/state-versions/%s/upload", baseURL, stateVersionID)
685+
686+
signedUploadUrl, err := auth.SignURL(baseURL, fmt.Sprintf("/tfe/api/v2/state-versions/%s/upload", stateVersionID), time.Now().Add(2*time.Minute))
687+
if err != nil {
688+
fmt.Printf("CreateStateVersion: ERROR - Failed to sign URL: %v\n", err)
689+
return c.JSON(500, map[string]string{"error": "Failed to sign URL"})
690+
}
691+
692+
signedJsonUploadUrl, err := auth.SignURL(baseURL, fmt.Sprintf("/tfe/api/v2/state-versions/%s/json-upload", stateVersionID), time.Now().Add(2*time.Minute))
693+
if err != nil {
694+
fmt.Printf("CreateStateVersion: ERROR - Failed to sign URL: %v\n", err)
695+
return c.JSON(500, map[string]string{"error": "Failed to sign URL"})
696+
}
697+
683698
downloadURL := fmt.Sprintf("%s/tfe/api/v2/state-versions/%s/download", baseURL, stateVersionID)
684-
jsonUploadURL := fmt.Sprintf("%s/tfe/api/v2/state-versions/%s/json-upload", baseURL, stateVersionID)
685699

686700
// Derive serial and lineage from existing state (if any)
687701
serial := 0
@@ -699,7 +713,6 @@ func (h *TfeHandler) CreateStateVersion(c echo.Context) error {
699713
}
700714

701715
fmt.Printf("CreateStateVersion: baseURL='%s'\n", baseURL)
702-
fmt.Printf("CreateStateVersion: uploadURL='%s'\n", uploadURL)
703716

704717
// Build the response
705718
response := map[string]interface{}{
@@ -710,10 +723,10 @@ func (h *TfeHandler) CreateStateVersion(c echo.Context) error {
710723
"created-at": time.Now().UTC().Format(time.RFC3339),
711724
"updated-at": time.Now().UTC().Format(time.RFC3339),
712725
"size": 0,
713-
"upload-url": uploadURL,
714-
"hosted-state-upload-url": uploadURL,
726+
"upload-url": signedUploadUrl,
727+
"hosted-state-upload-url": signedUploadUrl,
715728
"hosted-state-download-url": downloadURL,
716-
"hosted-json-state-upload-url": jsonUploadURL,
729+
"hosted-json-state-upload-url": signedJsonUploadUrl,
717730
"serial": serial,
718731
"lineage": lineage,
719732
},

0 commit comments

Comments
 (0)