You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
@@ -49,7 +52,7 @@ go clean --testcache && TEST_DBS="sqlite,postgres" go test -p 1 -v -run TestSign
49
52
2.**Schema changes must update ALL 13+ database providers**
50
53
3.**Run `make generate-graphql`** after editing `schema.graphqls`
51
54
4.**Security**: parameterized queries only, `crypto/rand` for tokens, `crypto/subtle` for comparisons, never log secrets
52
-
5.**Tests**: integration tests with real DBs, table-driven subtests, testify assertions
55
+
5.**Tests**: integration tests use SQLite via `getTestConfig()` (no `runForEachDB`); storage tests cover all DBs via `TEST_DBS`; testify assertions
53
56
6.**NEVER commit to main** — always work on a feature branch (`feat/`, `fix/`, `security/`, `chore/`), push to the branch, and create a merge request. Main must stay deployable.
# Full module test run. Storage provider tests honour TEST_DBS (defaults to all).
7
+
# Integration tests and memory_store/db tests always use SQLite.
8
+
# Redis memory_store tests run only when TEST_ENABLE_REDIS=1.
9
+
GO_TEST_ALL := go test -p 1 -v ./...
10
+
6
11
.PHONY: all bootstrap build build-app build-dashboard build-local-image build-push-image trivy-scan
7
12
8
13
all: build build-app build-dashboard
@@ -39,51 +44,50 @@ clean:
39
44
dev:
40
45
go run main.go --database-type=sqlite --database-url=test.db --jwt-type=HS256 --jwt-secret=test --admin-secret=admin --client-id=123456 --client-secret=secret
41
46
42
-
test: test-cleanup test-docker-up
43
-
go clean --testcache && TEST_DBS="postgres" go test -p 1 -v ./...
44
-
$(MAKE) test-cleanup
47
+
test:
48
+
go clean --testcache && TEST_DBS="sqlite"$(GO_TEST_ALL)
// Default RPS cap per IP; raised from 10 to reduce false positives on busy UIs.
58
-
defaultRateLimitRPS=float64(30)
58
+
defaultRateLimitRPS=30
59
59
defaultRateLimitBurst=20
60
60
)
61
61
@@ -161,7 +161,7 @@ func init() {
161
161
f.BoolVar(&rootArgs.config.DisableAdminHeaderAuth, "disable-admin-header-auth", false, "Disable admin authentication via X-Authorizer-Admin-Secret header")
162
162
163
163
// Rate limiting flags
164
-
f.Float64Var(&rootArgs.config.RateLimitRPS, "rate-limit-rps", defaultRateLimitRPS, "Maximum requests per second per IP for rate limiting")
164
+
f.IntVar(&rootArgs.config.RateLimitRPS, "rate-limit-rps", defaultRateLimitRPS, "Maximum requests per second per IP for rate limiting")
165
165
f.IntVar(&rootArgs.config.RateLimitBurst, "rate-limit-burst", defaultRateLimitBurst, "Maximum burst size per IP for rate limiting")
166
166
f.BoolVar(&rootArgs.config.RateLimitFailClosed, "rate-limit-fail-closed", false, "On rate-limit backend errors, reject with 503 instead of allowing the request")
# Optional fields and NULL semantics across storage providers
2
+
3
+
This document explains how **nullable** fields (especially `*int64` timestamps such as `email_verified_at`, `phone_number_verified_at`, `revoked_timestamp`) are **stored**, **updated**, and why we **do not** add broad `json`/`bson`**`omitempty`** tags to those struct fields.
4
+
5
+
## Goal
6
+
7
+
Application code uses Go **nil pointers** to mean “unset / not verified / not revoked.” Updates must **clear** a previously set value when the pointer is **nil**, matching SQL **NULL** semantics.
|**SQL (GORM)**|`Save(&user)`| Written as **SQL NULL**. |
14
+
|**Cassandra**| JSON → map → `UPDATE`| Nil map values become **`= null`** in CQL. |
15
+
|**MongoDB**|`UpdateOne` with `$set` and the `User` struct | Driver marshals nil pointers as **BSON Null** when the field is **not**`omitempty`, so the field is cleared in the document. |
16
+
|**Couchbase**|`Upsert` full document |`encoding/json` encodes nil pointers as JSON **`null`** unless the field uses `json:",omitempty"`, in which case the key is **omitted** and old values can persist. |
17
+
|**ArangoDB**|`UpdateDocument` with struct | Encoding follows JSON-style rules; nil pointers become **`null`** when not omitted by tags. |
18
+
|**DynamoDB**|`UpdateItem` with **SET** from marshalled attributes | Nil pointers are **omitted from SET** (see `internal/storage/db/dynamodb/marshal.go`). Attributes are **not** removed automatically, so **explicit REMOVE** is required to clear a previously stored attribute. Implemented for users in `internal/storage/db/dynamodb/user.go` (`updateByHashKeyWithRemoves`, `userDynamoRemoveAttrsIfNil`). Reads may normalize `0` → unset via `normalizeUserOptionalPtrs`. |
19
+
20
+
## Why not use `omitempty` on `json` / `bson` for nullable auth fields?
21
+
22
+
For **document** databases, **`omitempty`** means: *if this pointer is nil, do not include this key in the encoded payload.*
23
+
24
+
During an **update**, omitting a key usually means **“do not change this field”**, not **“set to null.”** That reproduces the DynamoDB-class bug: the old value remains.
25
+
26
+
Therefore, for fields where **nil must clear** stored state, keep **`json` / `bson` tags without `omitempty`** (as in `internal/storage/schemas/user.go`) unless every call site is proven to do a **full document replace** and you have verified the driver behaviour end-to-end.
27
+
28
+
MongoDB’s own guidance aligns with this: `omitempty` skips marshaling empty values, which is wrong when you need to persist **null** to clear a field in `$set`.
29
+
30
+
## DynamoDB specifics
31
+
32
+
-**PutItem**: Omitting nil pointers keeps items small; optional attributes may be absent (same idea as “omitempty” on write, but implemented in custom `marshalStruct` by skipping nil pointers).
33
+
-**UpdateItem**: Only **SET** attributes present in the marshalled map. Clearing requires **`REMOVE`** for the corresponding attribute names when the Go field is nil.
34
+
- Do **not** rely on adding `dynamo:",omitempty"` alone to “fix” updates: the custom marshaller already skips nil pointers; the gap was **REMOVE** on update, not tag-based omission.
35
+
36
+
## Related code
37
+
38
+
- Schema: `internal/storage/schemas/user.go`
39
+
- DynamoDB user update + REMOVE list: `internal/storage/db/dynamodb/user.go`
-**`internal/memory_store/db`**: Runs one subtest per entry in `TEST_DBS` (URLs aligned with `internal/integration_tests/test_helper.go` — keep `test_config_test.go` in sync when adding backends).
46
+
-**`internal/memory_store` (Redis / in-memory)**: Not driven by `TEST_DBS`. In-memory tests always run; **Redis** subtests run only when **`TEST_ENABLE_REDIS=1`** (or `true`) and Redis is reachable (e.g. `localhost:6380` in `provider_test.go`). See `redisMemoryStoreTestsEnabled` in `provider_test.go`.
0 commit comments