api: propagate PAT policy actors
Authored by
mfwolffe <wolffemf@dukes.jmu.edu>
- SHA
d933bdc35515ec40286b16af9c924d30c0979003- Parents
-
b4c53a7 - Tree
ea9999e
d933bdc
d933bdc35515ec40286b16af9c924d30c0979003b4c53a7
ea9999e| Status | File | + | - |
|---|---|---|---|
| M |
docs/internal/permissions.md
|
4 | 4 |
| A |
docs/public/api/actions.md
|
52 | 0 |
| M |
docs/public/api/overview.md
|
2 | 1 |
| M |
internal/web/handlers/api/actions_cancel.go
|
2 | 3 |
| M |
internal/web/handlers/api/actions_rerun.go
|
2 | 3 |
| M |
internal/web/handlers/api/checks.go
|
1 | 2 |
| M |
internal/web/handlers/api/stars.go
|
1 | 5 |
| M |
internal/web/middleware/middleware_test.go
|
17 | 0 |
| M |
internal/web/middleware/pat.go
|
21 | 6 |
docs/internal/permissions.mdmodified@@ -147,10 +147,10 @@ that constructs an actor must source it correctly: | ||
| 147 | 147 | suspending an account takes effect on the user's next click. |
| 148 | 148 | * **Web (PAT)** — `middleware.PATAuthMiddleware` rejects requests |
| 149 | 149 | whose owning user has `suspended_at IS NOT NULL` with a 401 before |
| 150 | - the handler runs. Code paths under PAT auth construct | |
| 151 | - `policy.UserActor(..., IsSuspended: false, ...)` because the gate | |
| 152 | - is upstream; the field is still passed for honesty and is correct | |
| 153 | - by construction. | |
| 150 | + the handler runs. It still binds username, suspension, and site-admin | |
| 151 | + fields into `middleware.PATAuth`; API policy gates must construct | |
| 152 | + actors through `PATAuth.PolicyActor()` so the request actor stays | |
| 153 | + honest even as the middleware evolves. | |
| 154 | 154 | * **git over HTTPS (`internal/web/handlers/githttp`)** — the basic- |
| 155 | 155 | auth resolver (`auth.go::resolveViaPAT`/`resolveViaPassword`) |
| 156 | 156 | rejects suspended owners with `errBadCredentials` *before* the |
docs/public/api/actions.mdadded@@ -0,0 +1,52 @@ | ||
| 1 | +# Actions workflow API | |
| 2 | + | |
| 3 | +Actions workflow lifecycle endpoints are PAT-authenticated and require | |
| 4 | +`repo:write`. The token's user must also have write permission on the | |
| 5 | +repository that owns the target run or job. | |
| 6 | + | |
| 7 | +## Cancel job | |
| 8 | + | |
| 9 | +```text | |
| 10 | +POST /api/v1/jobs/{id}/cancel | |
| 11 | +``` | |
| 12 | + | |
| 13 | +Requests cancellation for a workflow job. Queued jobs become terminal | |
| 14 | +immediately. Running jobs set `cancel_requested=true`; the runner observes | |
| 15 | +that flag through its cancel-check endpoint, stops the active container, and | |
| 16 | +reports the terminal status. | |
| 17 | + | |
| 18 | +Response: `202 Accepted`. | |
| 19 | + | |
| 20 | +```json | |
| 21 | +{ | |
| 22 | + "job_id": 10, | |
| 23 | + "run_id": 4, | |
| 24 | + "repo_id": 2, | |
| 25 | + "changed_jobs": 1, | |
| 26 | + "run_completed": false, | |
| 27 | + "run_conclusion": "" | |
| 28 | +} | |
| 29 | +``` | |
| 30 | + | |
| 31 | +## Re-run workflow run | |
| 32 | + | |
| 33 | +```text | |
| 34 | +POST /api/v1/runs/{id}/rerun | |
| 35 | +``` | |
| 36 | + | |
| 37 | +Creates a new workflow run from the original run's commit and workflow file. | |
| 38 | +Only terminal runs are rerunnable. The new run records `parent_run_id` so the | |
| 39 | +history remains linked. | |
| 40 | + | |
| 41 | +Response: `201 Created`. | |
| 42 | + | |
| 43 | +```json | |
| 44 | +{ | |
| 45 | + "run_id": 12, | |
| 46 | + "run_index": 8, | |
| 47 | + "parent_run_id": 4, | |
| 48 | + "repo_id": 2, | |
| 49 | + "workflow_file": ".shithub/workflows/ci.yml", | |
| 50 | + "head_sha": "0123456789abcdef0123456789abcdef01234567" | |
| 51 | +} | |
| 52 | +``` | |
docs/public/api/overview.mdmodified@@ -8,7 +8,8 @@ instead of PATs. | ||
| 8 | 8 | > **Status.** The API is intentionally narrow today. Endpoints |
| 9 | 9 | > currently shipped: `GET /api/v1/user`, the |
| 10 | 10 | > `/api/v1/repos/{owner}/{repo}/check-runs` family, and the |
| 11 | -> `/api/v1/user/starred*` stars endpoints. Other sections of this | |
| 11 | +> `/api/v1/user/starred*` stars endpoints, plus the Actions lifecycle | |
| 12 | +> routes in [Actions workflow API](actions.md). Other sections of this | |
| 12 | 13 | > reference (Issues, Pull requests, Webhooks, etc.) describe the |
| 13 | 14 | > **planned** shape and will land in subsequent sprints. Pages |
| 14 | 15 | > that document planned-only endpoints carry a banner. |
internal/web/handlers/api/actions_cancel.gomodified@@ -39,7 +39,7 @@ func (h *Handlers) workflowJobCancel(w http.ResponseWriter, r *http.Request) { | ||
| 39 | 39 | writeAPIError(w, http.StatusNotFound, "job not found") |
| 40 | 40 | return |
| 41 | 41 | } |
| 42 | - job, run, repo, ok := h.resolveCancellableJob(w, r, auth.UserID, jobID) | |
| 42 | + job, run, repo, ok := h.resolveCancellableJob(w, r, auth.PolicyActor(), jobID) | |
| 43 | 43 | if !ok { |
| 44 | 44 | return |
| 45 | 45 | } |
@@ -65,7 +65,7 @@ func (h *Handlers) workflowJobCancel(w http.ResponseWriter, r *http.Request) { | ||
| 65 | 65 | func (h *Handlers) resolveCancellableJob( |
| 66 | 66 | w http.ResponseWriter, |
| 67 | 67 | r *http.Request, |
| 68 | - userID int64, | |
| 68 | + actor policy.Actor, | |
| 69 | 69 | jobID int64, |
| 70 | 70 | ) (actionsdb.WorkflowJob, actionsdb.WorkflowRun, reposdb.Repo, bool) { |
| 71 | 71 | q := actionsdb.New() |
@@ -92,7 +92,6 @@ func (h *Handlers) resolveCancellableJob( | ||
| 92 | 92 | } |
| 93 | 93 | return actionsdb.WorkflowJob{}, actionsdb.WorkflowRun{}, reposdb.Repo{}, false |
| 94 | 94 | } |
| 95 | - actor := policy.UserActor(userID, "", false, false) | |
| 96 | 95 | if !policy.Can(r.Context(), policy.Deps{Pool: h.d.Pool}, actor, policy.ActionRepoWrite, policy.NewRepoRefFromRepo(repo)).Allow { |
| 97 | 96 | writeAPIError(w, http.StatusNotFound, "job not found") |
| 98 | 97 | return actionsdb.WorkflowJob{}, actionsdb.WorkflowRun{}, reposdb.Repo{}, false |
internal/web/handlers/api/actions_rerun.gomodified@@ -28,7 +28,7 @@ func (h *Handlers) workflowRunRerun(w http.ResponseWriter, r *http.Request) { | ||
| 28 | 28 | writeAPIError(w, http.StatusNotFound, "run not found") |
| 29 | 29 | return |
| 30 | 30 | } |
| 31 | - run, repo, ok := h.resolveLifecycleRun(w, r, auth.UserID, runID) | |
| 31 | + run, repo, ok := h.resolveLifecycleRun(w, r, auth.PolicyActor(), runID) | |
| 32 | 32 | if !ok { |
| 33 | 33 | return |
| 34 | 34 | } |
@@ -54,7 +54,7 @@ func (h *Handlers) workflowRunRerun(w http.ResponseWriter, r *http.Request) { | ||
| 54 | 54 | func (h *Handlers) resolveLifecycleRun( |
| 55 | 55 | w http.ResponseWriter, |
| 56 | 56 | r *http.Request, |
| 57 | - userID int64, | |
| 57 | + actor policy.Actor, | |
| 58 | 58 | runID int64, |
| 59 | 59 | ) (actionsdb.WorkflowRun, reposdb.Repo, bool) { |
| 60 | 60 | q := actionsdb.New() |
@@ -72,7 +72,6 @@ func (h *Handlers) resolveLifecycleRun( | ||
| 72 | 72 | } |
| 73 | 73 | return actionsdb.WorkflowRun{}, reposdb.Repo{}, false |
| 74 | 74 | } |
| 75 | - actor := policy.UserActor(userID, "", false, false) | |
| 76 | 75 | if !policy.Can(r.Context(), policy.Deps{Pool: h.d.Pool}, actor, policy.ActionRepoWrite, policy.NewRepoRefFromRepo(repo)).Allow { |
| 77 | 76 | writeAPIError(w, http.StatusNotFound, "run not found") |
| 78 | 77 | return actionsdb.WorkflowRun{}, reposdb.Repo{}, false |
internal/web/handlers/api/checks.gomodified@@ -60,8 +60,7 @@ func (h *Handlers) resolveAPIRepo(w http.ResponseWriter, r *http.Request, action | ||
| 60 | 60 | writeAPIError(w, http.StatusNotFound, "repo not found") |
| 61 | 61 | return nil, false |
| 62 | 62 | } |
| 63 | - actor := policy.UserActor(auth.UserID, "", false, false) | |
| 64 | - if !policy.Can(r.Context(), policy.Deps{Pool: h.d.Pool}, actor, action, policy.NewRepoRefFromRepo(repo)).Allow { | |
| 63 | + if !policy.Can(r.Context(), policy.Deps{Pool: h.d.Pool}, auth.PolicyActor(), action, policy.NewRepoRefFromRepo(repo)).Allow { | |
| 65 | 64 | // Existence-leak: 404 instead of 403 when the actor can't see |
| 66 | 65 | // the repo. The PAT-scope check above is the public 403; this |
| 67 | 66 | // is the visibility gate. |
internal/web/handlers/api/stars.gomodified@@ -122,11 +122,7 @@ func (h *Handlers) resolveStarTargetRepo(w http.ResponseWriter, r *http.Request) | ||
| 122 | 122 | writeAPIError(w, http.StatusNotFound, "repo not found") |
| 123 | 123 | return reposdb.Repo{}, false |
| 124 | 124 | } |
| 125 | - // PAT-auth path: the middleware already rejected suspended | |
| 126 | - // accounts; passing IsSuspended=false here is correct by | |
| 127 | - // construction (documented in docs/internal/permissions.md). | |
| 128 | - actor := policy.UserActor(auth.UserID, "", false, false) | |
| 129 | - if !policy.Can(r.Context(), policy.Deps{Pool: h.d.Pool}, actor, policy.ActionStarCreate, policy.NewRepoRefFromRepo(repo)).Allow { | |
| 125 | + if !policy.Can(r.Context(), policy.Deps{Pool: h.d.Pool}, auth.PolicyActor(), policy.ActionStarCreate, policy.NewRepoRefFromRepo(repo)).Allow { | |
| 130 | 126 | writeAPIError(w, http.StatusNotFound, "repo not found") |
| 131 | 127 | return reposdb.Repo{}, false |
| 132 | 128 | } |
internal/web/middleware/middleware_test.gomodified@@ -61,6 +61,23 @@ func TestOptionalUser_PopulatesIsSuspended(t *testing.T) { | ||
| 61 | 61 | } |
| 62 | 62 | } |
| 63 | 63 | |
| 64 | +func TestPATAuthPolicyActorPropagatesResolvedUserFlags(t *testing.T) { | |
| 65 | + t.Parallel() | |
| 66 | + | |
| 67 | + actor := PATAuth{ | |
| 68 | + UserID: 42, | |
| 69 | + Username: "alice", | |
| 70 | + IsSuspended: true, | |
| 71 | + IsSiteAdmin: true, | |
| 72 | + }.PolicyActor() | |
| 73 | + if actor.UserID != 42 || actor.Username != "alice" || !actor.IsSuspended || !actor.IsSiteAdmin { | |
| 74 | + t.Fatalf("PolicyActor did not propagate PAT user flags: %+v", actor) | |
| 75 | + } | |
| 76 | + if anon := (PATAuth{}).PolicyActor(); !anon.IsAnonymous { | |
| 77 | + t.Fatalf("zero PATAuth should produce anonymous actor: %+v", anon) | |
| 78 | + } | |
| 79 | +} | |
| 80 | + | |
| 64 | 81 | // TestOptionalUser_StaleEpochSkipsBind is the corollary: when the |
| 65 | 82 | // recorded session epoch doesn't match the current users.session_epoch |
| 66 | 83 | // (because the user logged out everywhere), the binding is skipped so |
internal/web/middleware/pat.gomodified@@ -15,6 +15,7 @@ import ( | ||
| 15 | 15 | "github.com/jackc/pgx/v5/pgxpool" |
| 16 | 16 | |
| 17 | 17 | "github.com/tenseleyFlow/shithub/internal/auth/pat" |
| 18 | + "github.com/tenseleyFlow/shithub/internal/auth/policy" | |
| 18 | 19 | usersdb "github.com/tenseleyFlow/shithub/internal/users/sqlc" |
| 19 | 20 | ) |
| 20 | 21 | |
@@ -24,9 +25,12 @@ var patAuthKey = ctxKey{name: "pat_auth"} | ||
| 24 | 25 | // the auth check passed via PAT, `Token != nil` and Scopes is the parsed |
| 25 | 26 | // scope list. Pure session callers see the zero value. |
| 26 | 27 | type PATAuth struct { |
| 27 | - UserID int64 | |
| 28 | - TokenID int64 | |
| 29 | - Scopes []string | |
| 28 | + UserID int64 | |
| 29 | + Username string | |
| 30 | + TokenID int64 | |
| 31 | + Scopes []string | |
| 32 | + IsSuspended bool | |
| 33 | + IsSiteAdmin bool | |
| 30 | 34 | } |
| 31 | 35 | |
| 32 | 36 | // PATAuthFromContext returns the resolved PAT auth state, or the zero |
@@ -38,6 +42,14 @@ func PATAuthFromContext(ctx context.Context) PATAuth { | ||
| 38 | 42 | return PATAuth{} |
| 39 | 43 | } |
| 40 | 44 | |
| 45 | +// PolicyActor returns the canonical policy actor for a resolved PAT request. | |
| 46 | +func (p PATAuth) PolicyActor() policy.Actor { | |
| 47 | + if p.UserID == 0 { | |
| 48 | + return policy.AnonymousActor() | |
| 49 | + } | |
| 50 | + return policy.UserActor(p.UserID, p.Username, p.IsSuspended, p.IsSiteAdmin) | |
| 51 | +} | |
| 52 | + | |
| 41 | 53 | // PATConfig configures the PAT auth middleware. |
| 42 | 54 | type PATConfig struct { |
| 43 | 55 | Pool *pgxpool.Pool |
@@ -134,9 +146,12 @@ func PATAuthMiddleware(cfg PATConfig) func(http.Handler) http.Handler { | ||
| 134 | 146 | } |
| 135 | 147 | |
| 136 | 148 | ctx := context.WithValue(r.Context(), patAuthKey, PATAuth{ |
| 137 | - UserID: row.UserID, | |
| 138 | - TokenID: row.ID, | |
| 139 | - Scopes: row.Scopes, | |
| 149 | + UserID: row.UserID, | |
| 150 | + Username: user.Username, | |
| 151 | + TokenID: row.ID, | |
| 152 | + Scopes: row.Scopes, | |
| 153 | + IsSuspended: user.SuspendedAt.Valid, | |
| 154 | + IsSiteAdmin: user.IsSiteAdmin, | |
| 140 | 155 | }) |
| 141 | 156 | next.ServeHTTP(w, r.WithContext(ctx)) |
| 142 | 157 | }) |