diff --git a/.dockerignore b/.dockerignore index b064cec..29ea359 100644 --- a/.dockerignore +++ b/.dockerignore @@ -5,3 +5,4 @@ codevault.exe *.log config*.yaml* docker-compose.yaml +/data diff --git a/README.md b/README.md index 63f7b56..adec632 100644 --- a/README.md +++ b/README.md @@ -58,7 +58,7 @@ server: database: path: ./codevault.db rate_limit: - max_failures: 10 # failed getValue lookups per IP before 404-ing everything + max_failures: 10 # failed getValue lookups per IP before 404/401-ing get/post requests window: 1m0s # sliding window for max_failures observability: logging: @@ -83,7 +83,7 @@ raw spec from `/api/openapi.json`. - `POST /api/v1/getValue` `{"code": "..."}` → `200 {"value": "..."}` / `404` (not found or rate-limited) / `400` (bad body). - `POST /api/v1/setValue` `{"code": "...", "value": "..."}` (Basic Auth) → - `204` / `400` (bad body) / `401` (bad credentials). + `204` / `400` (bad body) / `401` (bad credentials or rate-limited) / '403' (owned by other user). ## Metrics diff --git a/docker-compose.yaml.example b/docker-compose.yaml.example index b2033d6..a75dd95 100644 --- a/docker-compose.yaml.example +++ b/docker-compose.yaml.example @@ -11,7 +11,7 @@ services: # server.trusted_proxies must list the nginx container/network's actual IP or CIDR # (gin needs IPs/CIDRs, not hostnames) - e.g. this compose network's subnet. - ./config.yaml:/app/config.yaml:ro - - codevault-data:/app/data + - ./data:/app/data logging: driver: json-file options: @@ -25,6 +25,3 @@ services: reservations: cpus: "0.25" memory: 64M - -volumes: - codevault-data: diff --git a/internal/api/generated.go b/internal/api/generated.go index 113a239..de1af23 100644 --- a/internal/api/generated.go +++ b/internal/api/generated.go @@ -189,6 +189,14 @@ func (response SetValue401Response) VisitSetValueResponse(w http.ResponseWriter) return nil } +type SetValue403Response struct { +} + +func (response SetValue403Response) VisitSetValueResponse(w http.ResponseWriter) error { + w.WriteHeader(403) + return nil +} + // StrictServerInterface represents all server handlers. type StrictServerInterface interface { @@ -280,14 +288,14 @@ func (sh *strictHandler) SetValue(ctx *gin.Context) { // Base64 encoded, gzipped, json marshaled Swagger object var swaggerSpec = []string{ - "H4sIAAAAAAAC/7STwW7bMAyGX8XgdjRiZ+3Jt26HIYcBwwL0EuSgSEzCwhVVkQ4QFH73QZKTLcuKbgV6", - "siH95E9+pJ7B8mNgj14FumcQu8dHk3+/ot6bfsAf+DSgaDoKkQNGJcwCyw7TV48BoQPRSH4H41hDxKeB", - "IjroVkW1rk8q3jygVRjr3/JLYC94bXBI1687FNnfLJZvbKH+V+scX79YwViDoB0i6XGZwBbTjRGyd4Pu", - "z8BTTD6Fc4q9aoAxZSC/5VwJaZ9uvrDDezP0Wt19XyRzjELsoYP5rJ21qXoO6E0g6OBm1s5uoIZgdJ/N", - "GxOoOcyb3YQmM+HCJpExSuwXDrrzfKC0jKKf2R0LNa/oc4gJoSebg5oHSVWcdij9fYy4hQ4+NL+WrJk2", - "rPlzvcZLthoHzAdlOXLtn9r2Heyn7cv+DsVGClp4ZkG15cG7RPW22F9qFv5genLVhKjaJEZZfHstLgk9", - "6ylpNj2NRF4dyfJ9R7J8y0hebFOUU9j/g5tfi7+RCPldxbGiKc5GdOiVTC8XDw261cUTW63HdeL8MwAA", - "//98/iCR6gQAAA==", + "H4sIAAAAAAAC/7RTTY/TMBD9K9bAMWpSulxyWzigSiAhKu2l6sGNp61XqcdrjwvVKv8djZ0WSlm+xJ5i", + "Oe/NG7838wgd7T05dByhfYTY7XCv8/Ed8p3uE37Ch4SR5coH8hjYYgZ0ZFC+e/3lPbot76B93VTAR4/Q", + "QuRg3RaGoYKAD8kGNNAuC2l1RtH6HjuGofpOLnpyEa/1DvJbDr9WKLCfSSz+z4uqP+0kl6uebGioIGKX", + "guXjQmwvPax1tN1tEu0xDuHkWziX2DF7GKSCdRvKnVju5c9bMninU8/q9uNcxDFESw5amE6aSSPdk0en", + "vYUWZpNmMoMKvOZdFq+1t/VhWm9Hp7JFVKwSozRbcnMD7TkuKE/GyG/IHIuJjtFliva+t10m1fdRujhN", + "mJxeBtxACy/qbyNYj/NX/zh8w6W3HBLmizIrufdXTfMM8uMwZn2DsQvWc/EzA9SGkjPi6k2Rv8TM3UH3", + "1qjRIrUWjzL45hpcCjriU9Eseook/jaSxfNGsviXSJ58ZmQS2t8bN70Gf7AxWrdVFJQdeV1Ag46t7mOh", + "za5psiqKPjs0an1U2hHvMKgUMVzsJrTLi61croaVRPM1AAD//whGvnU7BQAA", } // GetSwagger returns the content of the embedded swagger specification file diff --git a/internal/api/handlers.go b/internal/api/handlers.go index d364d3b..b169cd3 100644 --- a/internal/api/handlers.go +++ b/internal/api/handlers.go @@ -6,8 +6,10 @@ import ( "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/metric" + "go.uber.org/zap" "github.com/kanootoko/codevault/internal/db" + "github.com/kanootoko/codevault/internal/observability/logging" "github.com/kanootoko/codevault/internal/observability/metrics" ) @@ -22,7 +24,7 @@ func NewHandler(secrets *db.SecretsRepository, m metrics.ValueMetrics) *StrictHa } func (h *StrictHandler) GetValue(ctx context.Context, request GetValueRequestObject) (GetValueResponseObject, error) { - if request.Body == nil || request.Body.Code == "" { + if request.Body == nil || request.Body.Code == "" || len(request.Body.Code) > 50 { return GetValue400Response{}, nil } @@ -35,21 +37,36 @@ func (h *StrictHandler) GetValue(ctx context.Context, request GetValueRequestObj return nil, err } + logger := logging.LoggerMustFromContext(ctx) + h.Metrics.GetSuccessTotal.Add(ctx, 1, metric.WithAttributes(attribute.String("code", request.Body.Code))) + ip, _ := IPFromContext(ctx) + logger.Info("get value succeeded", zap.String("code", request.Body.Code), zap.String("ip", ip)) return GetValue200JSONResponse{Value: value}, nil } func (h *StrictHandler) SetValue(ctx context.Context, request SetValueRequestObject) (SetValueResponseObject, error) { - if request.Body == nil || request.Body.Code == "" || request.Body.Value == "" || len(request.Body.Value) > 10*1024*1024 { + if request.Body == nil || request.Body.Code == "" || len(request.Body.Code) > 50 || + request.Body.Value == "" || len(request.Body.Value) > 10*1024*1024 { h.Metrics.SetFailedTotal.Add(ctx, 1, metric.WithAttributes(attribute.String("reason", "bad_request"))) return SetValue400Response{}, nil } - if err := h.Secrets.Set(ctx, request.Body.Code, request.Body.Value); err != nil { + login, _ := LoginFromContext(ctx) + ip, _ := IPFromContext(ctx) + + logger := logging.LoggerMustFromContext(ctx) + + if err := h.Secrets.Set(ctx, request.Body.Code, request.Body.Value, login); err != nil { + if errors.Is(err, db.ErrForbidden) { + h.Metrics.SetFailedTotal.Add(ctx, 1, metric.WithAttributes(attribute.String("reason", "forbidden"))) + logger.Warn("set value forbidden", zap.String("code", request.Body.Code), zap.String("ip", ip), zap.String("user", login)) + return SetValue403Response{}, nil + } return nil, err } - login, _ := LoginFromContext(ctx) h.Metrics.SetSuccessTotal.Add(ctx, 1, metric.WithAttributes(attribute.String("user", login))) + logger.Info("set value succeeded", zap.String("code", request.Body.Code), zap.String("ip", ip), zap.String("user", login)) return SetValue204Response{}, nil } diff --git a/internal/api/middleware.go b/internal/api/middleware.go index cfa2301..ec5a72a 100644 --- a/internal/api/middleware.go +++ b/internal/api/middleware.go @@ -8,8 +8,10 @@ import ( "github.com/gin-gonic/gin" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/metric" + "go.uber.org/zap" "github.com/kanootoko/codevault/internal/db" + "github.com/kanootoko/codevault/internal/observability/logging" "github.com/kanootoko/codevault/internal/observability/metrics" "github.com/kanootoko/codevault/internal/ratelimit" ) @@ -17,6 +19,7 @@ import ( type contextKey string const loginContextKey contextKey = "login" +const ipContextKey contextKey = "ip" // LoginFromContext returns the Basic Auth login stashed by BasicAuthMiddleware, // if any. Strict-server handlers only see context.Context, not *gin.Context, so @@ -26,12 +29,33 @@ func LoginFromContext(ctx context.Context) (string, bool) { return login, ok } +// IPFromContext returns the client IP stashed by InstrumentMiddleware, if any. +// Strict-server handlers only see context.Context, not *gin.Context, so this is +// how they recover it. +func IPFromContext(ctx context.Context) (string, bool) { + ip, ok := ctx.Value(ipContextKey).(string) + return ip, ok +} + // BasicAuthMiddleware checks the request's HTTP Basic Auth credentials against -// the DB-backed user store. -func BasicAuthMiddleware(users *db.UsersRepository, vm metrics.ValueMetrics) gin.HandlerFunc { +// the DB-backed user store. It shares the same per-IP failure limiter as +// RateLimitMiddleware, so repeated bad credentials get throttled the same way +// repeated failed lookups do, instead of the caller being able to hammer the +// user store indefinitely. +func BasicAuthMiddleware(users *db.UsersRepository, limiter *ratelimit.Limiter, vm metrics.ValueMetrics) gin.HandlerFunc { return func(c *gin.Context) { + ip := c.ClientIP() + if !limiter.Allowed(ip) { + limiter.RecordFailure(ip) + vm.SetFailedTotal.Add(c.Request.Context(), 1, metric.WithAttributes(attribute.String("reason", "rate_limited"))) + c.Header("WWW-Authenticate", `Basic realm="codevault"`) + c.AbortWithStatus(http.StatusUnauthorized) + return + } + login, password, ok := c.Request.BasicAuth() if !ok || users.Verify(c.Request.Context(), login, password) != nil { + limiter.RecordFailure(ip) vm.SetFailedTotal.Add(c.Request.Context(), 1, metric.WithAttributes(attribute.String("reason", "unauthorized"))) c.Header("WWW-Authenticate", `Basic realm="codevault"`) c.AbortWithStatus(http.StatusUnauthorized) @@ -64,9 +88,13 @@ func RateLimitMiddleware(limiter *ratelimit.Limiter, vm metrics.ValueMetrics) gi } } -// InstrumentMiddleware records request count and duration metrics per route. -func InstrumentMiddleware(route string, m metrics.HTTPMetrics) gin.HandlerFunc { +// InstrumentMiddleware records request count and duration metrics per route. Also injects logger to context. +func InstrumentMiddleware(route string, m metrics.HTTPMetrics, logger *zap.Logger) gin.HandlerFunc { return func(c *gin.Context) { + ctx := context.WithValue(c.Request.Context(), ipContextKey, c.ClientIP()) + ctx = logging.CtxWithLogger(ctx, logger) + c.Request = c.Request.WithContext(ctx) + start := time.Now() c.Next() duration := time.Since(start).Seconds() diff --git a/internal/api/openapi.yaml b/internal/api/openapi.yaml index 7424983..aa0b394 100644 --- a/internal/api/openapi.yaml +++ b/internal/api/openapi.yaml @@ -41,6 +41,8 @@ paths: description: Invalid request body '401': description: Missing or invalid credentials + '403': + description: Code owned by another user components: securitySchemes: basicAuth: @@ -54,6 +56,7 @@ components: properties: code: type: string + maxLength: 50 GetValueResponse: type: object required: @@ -69,5 +72,6 @@ components: properties: code: type: string + maxLength: 50 value: type: string diff --git a/internal/api/swagger.go b/internal/api/swagger.go index 94c29a5..fd6e131 100644 --- a/internal/api/swagger.go +++ b/internal/api/swagger.go @@ -29,6 +29,9 @@ func RegisterSwaggerHandlers(router *gin.Engine) error { router.GET("/api", func(c *gin.Context) { c.Redirect(http.StatusFound, "/api/docs/index.html") }) + router.GET("/docs", func(c *gin.Context) { + c.Redirect(http.StatusFound, "/api/docs/index.html") + }) router.GET("/api/docs/*any", ginSwagger.WrapHandler(swaggerFiles.Handler, ginSwagger.URL("/api/openapi.json"))) return nil diff --git a/internal/db/schema.sql b/internal/db/schema.sql index 5b47e00..743a17d 100644 --- a/internal/db/schema.sql +++ b/internal/db/schema.sql @@ -7,6 +7,7 @@ CREATE TABLE IF NOT EXISTS users ( CREATE TABLE IF NOT EXISTS secrets ( code TEXT PRIMARY KEY, value TEXT NOT NULL, + user TEXT REFERENCES users(login) NOT NULL, created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ); diff --git a/internal/db/secrets.go b/internal/db/secrets.go index db8cfcc..15b875f 100644 --- a/internal/db/secrets.go +++ b/internal/db/secrets.go @@ -8,6 +8,7 @@ import ( ) var ErrNotFound = errors.New("secret not found") +var ErrForbidden = errors.New("code owned by another user") type SecretsRepository struct { db *sql.DB @@ -29,14 +30,35 @@ func (r *SecretsRepository) Get(ctx context.Context, code string) (string, error return value, nil } -func (r *SecretsRepository) Set(ctx context.Context, code, value string) error { - _, err := r.db.ExecContext(ctx, ` - INSERT INTO secrets (code, value, updated_at) - VALUES (?, ?, CURRENT_TIMESTAMP) - ON CONFLICT(code) DO UPDATE SET value = excluded.value, updated_at = CURRENT_TIMESTAMP - `, code, value) +// Set stores value under code, owned by user. If code already exists and is +// owned by a different user, it returns ErrForbidden and leaves the stored +// value untouched. +func (r *SecretsRepository) Set(ctx context.Context, code, value, user string) error { + tx, err := r.db.BeginTx(ctx, nil) if err != nil { + return fmt.Errorf("could not begin transaction: %w", err) + } + defer func() { _ = tx.Rollback() }() + + var owner string + err = tx.QueryRowContext(ctx, "SELECT user FROM secrets WHERE code = ?", code).Scan(&owner) + if err != nil && !errors.Is(err, sql.ErrNoRows) { + return fmt.Errorf("could not check secret owner: %w", err) + } + if !errors.Is(err, sql.ErrNoRows) && owner != user { + return ErrForbidden + } + + if _, err := tx.ExecContext(ctx, ` + INSERT INTO secrets (code, value, user, created_at, updated_at) + VALUES (?, ?, ?, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP) + ON CONFLICT(code) DO UPDATE SET value = excluded.value, updated_at = CURRENT_TIMESTAMP + `, code, value, user); err != nil { return fmt.Errorf("could not set secret: %w", err) } + + if err := tx.Commit(); err != nil { + return fmt.Errorf("could not commit transaction: %w", err) + } return nil } diff --git a/internal/db/secrets_test.go b/internal/db/secrets_test.go new file mode 100644 index 0000000..7dae350 --- /dev/null +++ b/internal/db/secrets_test.go @@ -0,0 +1,53 @@ +package db_test + +import ( + "context" + "database/sql" + "errors" + "path/filepath" + "testing" + + "github.com/kanootoko/codevault/internal/db" +) + +func openTestDB(t *testing.T) *sql.DB { + t.Helper() + sqlDB, err := db.Open(filepath.Join(t.TempDir(), "test.db")) + if err != nil { + t.Fatalf("could not open test db: %v", err) + } + t.Cleanup(func() { _ = sqlDB.Close() }) + return sqlDB +} + +func TestSecretsRepository_SetOwnership(t *testing.T) { + ctx := context.Background() + secrets := db.NewSecretsRepository(openTestDB(t)) + + if err := secrets.Set(ctx, "code1", "value1", "alice"); err != nil { + t.Fatalf("alice's initial set failed: %v", err) + } + + if err := secrets.Set(ctx, "code1", "value2", "alice"); err != nil { + t.Fatalf("alice's overwrite failed: %v", err) + } + value, err := secrets.Get(ctx, "code1") + if err != nil { + t.Fatalf("get failed: %v", err) + } + if value != "value2" { + t.Fatalf("expected value2, got %q", value) + } + + err = secrets.Set(ctx, "code1", "value3", "bob") + if !errors.Is(err, db.ErrForbidden) { + t.Fatalf("expected ErrForbidden for bob, got %v", err) + } + value, err = secrets.Get(ctx, "code1") + if err != nil { + t.Fatalf("get failed: %v", err) + } + if value != "value2" { + t.Fatalf("expected value to remain value2 after forbidden write, got %q", value) + } +} diff --git a/internal/httpserver/server.go b/internal/httpserver/server.go index de9c9d1..4f553b2 100644 --- a/internal/httpserver/server.go +++ b/internal/httpserver/server.go @@ -30,6 +30,7 @@ func New( limiter *ratelimit.Limiter, m metrics.Metrics, trustedProxies []string, + logger *zap.Logger, ) (*gin.Engine, error) { router := gin.New() // Strict-server handlers only see context.Context (via *gin.Context, which @@ -54,13 +55,13 @@ func New( wrapper := api.ServerInterfaceWrapper{Handler: strictHandler} router.POST("/api/v1/getValue", - api.InstrumentMiddleware("getValue", m.HTTP), + api.InstrumentMiddleware("getValue", m.HTTP, logger), api.RateLimitMiddleware(limiter, m.Value), wrapper.GetValue, ) router.POST("/api/v1/setValue", - api.InstrumentMiddleware("setValue", m.HTTP), - api.BasicAuthMiddleware(users, m.Value), + api.InstrumentMiddleware("setValue", m.HTTP, logger), + api.BasicAuthMiddleware(users, limiter, m.Value), wrapper.SetValue, ) diff --git a/internal/runcmd/run.go b/internal/runcmd/run.go index 03bec73..a043b86 100644 --- a/internal/runcmd/run.go +++ b/internal/runcmd/run.go @@ -41,7 +41,7 @@ func RunServer(ctx context.Context, appConfig config.Config) error { limiter := ratelimit.NewLimiter(appConfig.RateLimit.MaxFailures, appConfig.RateLimit.Window) - router, err := httpserver.New(secrets, users, limiter, appMetrics, appConfig.Server.TrustedProxies) + router, err := httpserver.New(secrets, users, limiter, appMetrics, appConfig.Server.TrustedProxies, logger) if err != nil { return fmt.Errorf("error on building http router: %w", err) } diff --git a/internal/version.go b/internal/version.go index 0d55bb3..54d335d 100644 --- a/internal/version.go +++ b/internal/version.go @@ -1,4 +1,4 @@ package internal -const Version = "0.1.0" +const Version = "0.2.0" const PackageName = "codevault" diff --git a/internal/webui/static/get.html b/internal/webui/static/get.html index 8521f14..5bd4940 100644 --- a/internal/webui/static/get.html +++ b/internal/webui/static/get.html @@ -4,7 +4,7 @@