coder / coder/internal

rbac: dbauthz fails closed when fetching related resources

Offen
#262 3 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen
help wanted tech-debt
Vorherrschende Sprache
Keine Sprachdaten
Sterne
3
Forks
0
PR-Merge-Kennzahlen
Keine gemergten PRs in 30 T.

Beschreibung

**Problem:**

Given:
- You have resource A related to resource B
- You authorize actions on resource A by checking relevant permissions on resource B
- Resource A exists, but for some reason references to a nonexistant resource B

When:
- You ask if you can do X on A

Then:
- You get `sql.ErrNoRows` relating to the fetch of resource B, which is _completely unintuitive_

This was encountered when writing a test case for provisioner jobs using `dbgen.ProvisionerJob`, but I could imagine a situation where this could arise in the "real world".

Example to reproduce:

```
var defOrgID uuid.UUID
if orig.OrganizationID == uuid.Nil {
defOrg, _ := db.GetDefaultOrganization(genCtx)
defOrgID = defOrg.ID
}

jobID := takeFirst(orig.ID, uuid.New())
// Always set some tags to prevent Acquire from grabbing jobs it should not.
tags := takeFirstMap(orig.Tags, database.StringMap{"user": "", "scope": "organization"})
if orig.Tags == nil && !orig.StartedAt.Time.IsZero() {
// Make sure when we acquire the job, we only get this one.
tags[jobID.String()] = "true"
}

job, err := db.InsertProvisionerJob(genCtx, database.InsertProvisionerJobParams{
ID: jobID,
CreatedAt: takeFirst(orig.CreatedAt, dbtime.Now()),
UpdatedAt: takeFirst(orig.UpdatedAt, dbtime.Now()),
OrganizationID: takeFirst(orig.OrganizationID, defOrgID, uuid.New()),
InitiatorID: takeFirst(orig.InitiatorID, uuid.New()),
Provisioner: takeFirst(orig.Provisioner, database.ProvisionerTypeEcho),
StorageMethod: takeFirst(orig.StorageMethod, database.ProvisionerStorageMethodFile),
FileID: takeFirst(orig.FileID, uuid.New()),
Type: takeFirst(orig.Type, database.ProvisionerJobTypeWorkspaceBuild),
Input: takeFirstSlice(orig.Input, []byte("{}")),
Tags: tags,
TraceMetadata: pqtype.NullRawMessage{},
})
_ = job
require.NoError(t, err, "insert job")
job, err = db.GetProvisionerJobByID(genCtx, jobID)
require.NoError(t, err, "get job: %s", jobID.String())

```

**Suggested solution:**

Fail open if the related resource is not present instead.

cc @mafredri

Beitragsleitfaden

Für dieses Repository ist kein Beitragsleitfaden indexiert

Rechercherichtung

Start with the dbgen.ProvisionerJob reproduction and trace the GetProvisionerJobByID path, including where the related resource B is fetched and sql.ErrNoRows escapes. Check the existing RBAC or dbauthz tests for missing related resources; done means the missing resource produces the intended fail-open authorization result instead of exposing the database error.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Bewertung

Tech-Stack
go, sql
Bereich
authorization, backend
Issue-Typ
Bug
Schwierigkeit
4/5
Geschätzter Aufwand
3-5 Tage
Aktivitätsstatus
Veraltet
Klarheit
Größtenteils klar
Anfängerfreundlichkeit
45/100

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.