Paths

Table of Contentst

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)
	Thu, Nov 14, 2:35 AM

	Unknown Object (File)
	Sun, Nov 3, 12:27 PM

	Unknown Object (File)
	Fri, Nov 1, 1:38 PM

	Unknown Object (File)
	Sat, Oct 26, 12:59 AM

	Unknown Object (File)
	Fri, Oct 25, 7:11 AM

	Unknown Object (File)
	Wed, Oct 23, 9:07 AM

	Unknown Object (File)
	Mon, Oct 21, 1:02 AM

	Unknown Object (File)
	Sun, Oct 20, 7:09 AM

Subscribers

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

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

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

Path

Size

services/

blob/

src/

http/

10 lines

Diff 41441

services/blob/src/http/mod.rs

Loading...