mirror of
https://github.com/hanzoai/base.git
synced 2026-10-21 03:41:50 +00:00
An aggregate reads no private email, and history no removed field
Red's review. The aggregate route reads columns directly, so it gated only hidden fields: grouping an auth collection by email, or counting its unique emails, answered addresses the records keep private, which the list masks and the filter resolver will not compare. core.EmailGated is now the one rule both ask: an auth collection's email, which a caller who is not a superuser reads only where emailVisibility is set. The aggregate route refuses it as it refuses a hidden field. TestAPrivateEmailIsNotAFigure is red's proof inverted: the group and the count are refused, and fail without the change. A record's history showed the changes of a field the collection had since removed. An org admin who removes a field means its values to be gone from view, so a caller who is not a superuser no longer reads them; _history keeps the row for a superuser. The composite uniqueness check is a read before the write, which PostgreSQL can race; that is now said where it is done and in LLM.md, with the fix (a unique expression index from the dialect).
This commit is contained in:
@@ -174,7 +174,8 @@ host states a program (`core.WithActor`), and nothing in a body sets one.
|
||||
`history` writes a `_history` row per create, update and delete (who, when,
|
||||
changed fields from and to), merging one writer's edits within ten minutes;
|
||||
`GET /v1/collections/{c}/records/{id}/history` answers under the record's view
|
||||
rule.
|
||||
rule, without the changes of a field hidden or removed since (a superuser reads
|
||||
the row whole).
|
||||
|
||||
**A where filter is the filter language, written as data.** `core.CompileWhere`
|
||||
turns `{"and"|"or"|"not"|field, op, value}` into Base's filter with bound
|
||||
@@ -182,7 +183,10 @@ parameters: per-type operators, relative dates in a time zone, `@me`,
|
||||
back-relations (`list_members_via_person.list` is "in list") and
|
||||
`inSegment`/`notInSegment` over the `segments` collection. `?where=` on the
|
||||
records list and `GET /v1/collections/{c}/aggregate?ops=&group=` (count, sums,
|
||||
averages, empties, uniques, grouped lanes) both run under the list rule.
|
||||
averages, empties, uniques, grouped lanes) both run under the list rule. An
|
||||
aggregate reads columns directly, so for a caller who is not a superuser it
|
||||
refuses a hidden field and an auth collection's email (`core.EmailGated`, the
|
||||
rule the filter resolver applies by ANDing `emailVisibility`).
|
||||
|
||||
**Grow never drops.** `core.GrowCollections` creates what is missing and adds
|
||||
fields, select values and indexes by name, keeping every field, rule, option and
|
||||
@@ -215,6 +219,7 @@ is `BASE_FRAME_ANCESTORS` (default `'self'`). It signs in with IAM PKCE
|
||||
|
||||
## Known problems
|
||||
|
||||
- A composite's unique primary part (an email, a phone, a URL) is held by a SELECT before the write (`checkUniquePart`), not by an index: the part is inside a JSON column and an expression index is spelled per engine. SQLite admits one writer, so it holds there; on a Base placed on PostgreSQL two writers of one address at once can both pass. Decided to keep until a placed Base takes concurrent writes of one key; the fix is a unique expression index from the dialect.
|
||||
- `migrations/1780500000_connect_messaging_calendar.go` gives `connectedAccount` encrypted token fields, so an OAuth token can sit in a record. Secrets belong in KMS; the collection is retired by another change, and nothing reads it.
|
||||
- `plugins/jsvm/internal/types/generated/types.d.ts` has not been regenerated for the composite field types: the generator names its aliases at random, so a run rewrites some 36,000 lines.
|
||||
|
||||
|
||||
@@ -214,7 +214,9 @@ func resolveAggregateField(app core.App, collection *core.Collection, path strin
|
||||
if field == nil {
|
||||
return nil, fmt.Errorf("%q has no field %q", collection.Name, segs[0])
|
||||
}
|
||||
if field.GetHidden() && !superuser {
|
||||
// a figure reads the column itself, so a value some records keep private
|
||||
// (an auth record's email) is out of its reach entirely
|
||||
if (field.GetHidden() || core.EmailGated(collection, field)) && !superuser {
|
||||
return nil, fmt.Errorf("%q is not filterable", segs[0])
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,81 @@
|
||||
package apis_test
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"net/url"
|
||||
"testing"
|
||||
|
||||
"github.com/hanzoai/base/core"
|
||||
"github.com/hanzoai/base/tests"
|
||||
"github.com/hanzoai/base/tools/types"
|
||||
)
|
||||
|
||||
// targets is an auth collection a member may list whose records keep their
|
||||
// emails private (emailVisibility off).
|
||||
func targets(t testing.TB, app *tests.TestApp) {
|
||||
t.Helper()
|
||||
|
||||
c := core.NewAuthCollection("targets")
|
||||
c.ListRule = types.Pointer("")
|
||||
if err := app.Save(c); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
for _, email := range []string{"[email protected]", "[email protected]", "[email protected]"} {
|
||||
r := core.NewRecord(c)
|
||||
r.Set("email", email)
|
||||
r.Set("emailVisibility", false)
|
||||
r.Set("password", "secret123456")
|
||||
if err := app.Save(r); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
var privateEmails = []string{"[email protected]", "[email protected]", "[email protected]"}
|
||||
|
||||
// An address a record keeps private reaches nobody but a superuser by any
|
||||
// route: not in a list, not through a filter, and not as a figure or a group.
|
||||
func TestAPrivateEmailIsNotAFigure(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
scenarios := []tests.ApiScenario{
|
||||
{
|
||||
Name: "the list masks it",
|
||||
Method: http.MethodGet,
|
||||
URL: "/v1/collections/targets/records",
|
||||
ExpectedStatus: 200,
|
||||
ExpectedContent: []string{`"totalItems":3`},
|
||||
NotExpectedContent: privateEmails,
|
||||
},
|
||||
{
|
||||
Name: "a filter finds none of it",
|
||||
Method: http.MethodGet,
|
||||
URL: "/v1/collections/targets/records?filter=" + url.QueryEscape(`email != ""`),
|
||||
ExpectedStatus: 200,
|
||||
ExpectedContent: []string{`"totalItems":0`},
|
||||
},
|
||||
{
|
||||
Name: "a group by it is refused",
|
||||
Method: http.MethodGet,
|
||||
URL: "/v1/collections/targets/aggregate?" + url.Values{"ops": {"count"}, "group": {"email"}}.Encode(),
|
||||
ExpectedStatus: 400,
|
||||
ExpectedContent: []string{`not filterable`},
|
||||
NotExpectedContent: privateEmails,
|
||||
},
|
||||
{
|
||||
Name: "a count of it is refused",
|
||||
Method: http.MethodGet,
|
||||
URL: "/v1/collections/targets/aggregate?" + url.Values{"ops": {"countUnique(email)"}}.Encode(),
|
||||
ExpectedStatus: 400,
|
||||
ExpectedContent: []string{`not filterable`},
|
||||
NotExpectedContent: []string{`"countUnique(email)":3`},
|
||||
},
|
||||
}
|
||||
|
||||
for _, s := range scenarios {
|
||||
s.Headers = map[string]string{"Authorization": aggregateMemberToken}
|
||||
s.BeforeTestFunc = func(t testing.TB, app *tests.TestApp, e *core.ServeEvent) { targets(t, app) }
|
||||
s.Test(t)
|
||||
}
|
||||
}
|
||||
@@ -110,8 +110,10 @@ func recordHistory(e *core.RequestEvent) error {
|
||||
})
|
||||
}
|
||||
|
||||
// visibleChanges drops the changes of fields the collection hides now, for a
|
||||
// caller who is not a superuser. A field hidden when it changed was never
|
||||
// visibleChanges drops the changes of fields the collection hides now, and of
|
||||
// fields it no longer has, for a caller who is not a superuser: an org admin who
|
||||
// removes a field means its values to be gone from view, and _history keeps the
|
||||
// row for a superuser's audit. A field hidden when it changed was never
|
||||
// recorded.
|
||||
func visibleChanges(collection *core.Collection, requestInfo *core.RequestInfo, raw string) json.RawMessage {
|
||||
if requestInfo.HasSuperuserAuth() || raw == "" {
|
||||
@@ -123,7 +125,7 @@ func visibleChanges(collection *core.Collection, requestInfo *core.RequestInfo,
|
||||
return json.RawMessage(`{}`)
|
||||
}
|
||||
for name := range changes {
|
||||
if f := collection.Fields.GetByName(name); f != nil && f.GetHidden() {
|
||||
if f := collection.Fields.GetByName(name); f == nil || f.GetHidden() {
|
||||
delete(changes, name)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -36,7 +36,7 @@ func TestRecordHistoryRoute(t *testing.T) {
|
||||
c := core.NewBaseCollection("deals")
|
||||
c.History = true
|
||||
c.ViewRule = viewRule
|
||||
c.Fields.Add(&core.TextField{Name: "title"}, &core.TextField{Name: "note"})
|
||||
c.Fields.Add(&core.TextField{Name: "title"}, &core.TextField{Name: "note"}, &core.TextField{Name: "stage"})
|
||||
if err := app.Save(c); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
@@ -54,12 +54,15 @@ func TestRecordHistoryRoute(t *testing.T) {
|
||||
r, _ = app.FindRecordById(c, id)
|
||||
r.Set("title", "Acme Inc")
|
||||
r.Set("note", "called")
|
||||
r.Set("stage", "negotiating")
|
||||
if err := app.SaveWithContext(bob, r); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
// the note is hidden after it changed: its change is no reader's now
|
||||
// the note is hidden after it changed, and the stage removed: neither
|
||||
// change is any reader's now
|
||||
c.Fields.GetByName("note").SetHidden(true)
|
||||
c.Fields.RemoveByName("stage")
|
||||
if err := app.Save(c); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
@@ -90,7 +93,7 @@ func TestRecordHistoryRoute(t *testing.T) {
|
||||
`"totalItems":2`, `"action":"update"`, `"action":"create"`, `"name":"Bob"`, `"source":"import"`,
|
||||
`"title":{"from":"Acme","to":"Acme Inc"}`, `"title":{"to":"Acme"}`,
|
||||
},
|
||||
excluded: []string{"call back", "called"},
|
||||
excluded: []string{"call back", "called", "negotiating", `"stage"`},
|
||||
order: []string{`"action":"update"`, `"action":"create"`},
|
||||
},
|
||||
{name: "a caller the view rule refuses", role: "member", rule: adminOnly, status: 404, content: []string{`"status":404`}},
|
||||
|
||||
@@ -78,6 +78,15 @@ var (
|
||||
ErrMustBeSystem = validation.NewError("validation_must_be_system", `The field must be marked as "System".`)
|
||||
)
|
||||
|
||||
// EmailGated reports whether field is an auth collection's email, which a
|
||||
// caller who is not a superuser reads only on the records that made it public
|
||||
// (emailVisibility). The filter resolver and the aggregate route both ask it, so
|
||||
// an address a record keeps private is out of reach of a comparison and of a
|
||||
// figure alike.
|
||||
func EmailGated(collection *Collection, field Field) bool {
|
||||
return collection != nil && collection.IsAuth() && field != nil && field.GetName() == FieldNameEmail
|
||||
}
|
||||
|
||||
// FieldFactoryFunc defines a simple function to construct a specific Field instance.
|
||||
type FieldFactoryFunc func() Field
|
||||
|
||||
|
||||
@@ -80,6 +80,10 @@ var errValueNotUnique = validation.NewError("validation_not_unique", "Value must
|
||||
//
|
||||
// It compares case-insensitively, since every such part is matched that way by
|
||||
// the people who type it.
|
||||
//
|
||||
// It is a read before the write, so it holds where the engine admits one writer
|
||||
// (SQLite). On PostgreSQL two writers of one value at once can both pass it; a
|
||||
// unique expression index from the dialect is what would close that.
|
||||
func checkUniquePart(ctx context.Context, app App, record *Record, field, part, value string) error {
|
||||
if value == "" {
|
||||
return nil
|
||||
|
||||
@@ -866,7 +866,7 @@ func (r *runner) finalizeActivePropsProcessing(collection *Collection, prop stri
|
||||
}
|
||||
|
||||
// allow querying only auth records with emails marked as public
|
||||
if field.GetName() == FieldNameEmail && !r.resolver.allowHiddenFields && collection.IsAuth() {
|
||||
if EmailGated(collection, field) && !r.resolver.allowHiddenFields {
|
||||
result.AfterBuild = func(expr dbx.Expression) dbx.Expression {
|
||||
return dbx.Enclose(dbx.And(expr, dbx.NewExp(fmt.Sprintf(
|
||||
"[[%s.%s]] = TRUE",
|
||||
|
||||
Reference in New Issue
Block a user