Page MenuHomePhabricator
Differential D12415

[blob] Hide endpoints behind auth middleware
ClosedPublic
Actions

Authored by bartek on Jun 13 2024, 1:07 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Dec 21, 2:55 PM
Unknown Object (File)
Sat, Dec 21, 2:55 PM
Unknown Object (File)
Sat, Dec 21, 2:55 PM
Unknown Object (File)
Sat, Dec 21, 2:55 PM
Unknown Object (File)
Sat, Dec 21, 2:54 PM
Unknown Object (File)
Wed, Nov 27, 11:39 AM
Unknown Object (File)
Wed, Nov 27, 11:04 AM
Unknown Object (File)
Nov 25 2024, 6:57 AM
View All Files
Subscribers
ashoat
tomek

Details

Reviewers
varun
will
kamil
Commits
rCOMM496f450b731f: [blob] Hide endpoints behind auth middleware
Summary

Wrapped all /blob/** endpoints with auth middleware.

By the way, added /health which was missing, but it's handy for AWS ECS to do health checks (until now it treated 404 as okay)

Depends on D12414

Test Plan

Test plan for D12414 for all blob endpoints. /health works without auth

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

bartek created this revision.Jun 13 2024, 1:07 AM
bartek held this revision as a draft.
Herald added subscribers: tomek, ashoat. · View Herald TranscriptJun 13 2024, 1:07 AM
bartek added a child revision: D12416: [reports] Require auth for viewing reports.Jun 13 2024, 1:10 AM
Harbormaster failed remote builds in B29642: Diff 41265!Jun 13 2024, 1:29 AM
bartek updated this revision to Diff 41270.Jun 13 2024, 1:55 AM

Rebase

Harbormaster completed remote builds in B29647: Diff 41270.Jun 13 2024, 2:11 AM
bartek published this revision for review.Jun 13 2024, 2:11 AM
ashoat retitled this revision from [blob] Hide edpoints behind auth middleware to [blob] Hide endpoints behind auth middleware.Jun 13 2024, 8:23 AM
varun accepted this revision.Jun 17 2024, 9:00 PM
varun added inline comments.
services/blob/src/http/mod.rs
42 ↗(On Diff #41270)

do we need this clone?

This revision is now accepted and ready to land.Jun 17 2024, 9:00 PM
bartek added inline comments.Jun 17 2024, 11:58 PM
services/blob/src/http/mod.rs
42 ↗(On Diff #41270)

yes, because HttpServer::new(move || { moves it into the closure, and the closure can be called multiple times, once for each HTTP listener thread.
But probably moving the get_comm_authentication_middleware() call into the closure could avoid the clone() call. Under the hood, they're equivalent, though.

Closed by commit rCOMM496f450b731f: [blob] Hide endpoints behind auth middleware (authored by bartek). · Explain WhyJun 18 2024, 2:19 AM
This revision was automatically updated to reflect the committed changes.
bartek added a commit: rCOMM496f450b731f: [blob] Hide endpoints behind auth middleware.

Revision Contents
Changeset List

PathSize
services/
blob/
src/
http/
10 lines

Diff 41441

View Options

services/blob/src/http/mod.rs

Loading...