Differential D12414

[comm-lib] Verify CSAT in HTTP middleware
ClosedPublic
Actions

Authored by bartek on Jun 13 2024, 1:04 AM.

Tags

None

Referenced Files

	F2831585: D12414.diff
	Sat, Sep 28, 12:28 AM

	Unknown Object (File)
	Thu, Sep 26, 4:30 AM

	Unknown Object (File)
	Thu, Sep 26, 1:19 AM

	Unknown Object (File)
	Tue, Sep 3, 1:19 PM

	Unknown Object (File)
	Tue, Sep 3, 4:56 AM

	Unknown Object (File)
	Tue, Sep 3, 2:25 AM

	Unknown Object (File)
	Tue, Sep 3, 2:25 AM

	Unknown Object (File)
	Tue, Sep 3, 2:25 AM

View All 50 Files

Subscribers

Details

Reviewers

varun
will
kamil

Commits

rCOMM17e30f45d3b8: [comm-lib] Verify CSAT in HTTP middleware

Summary

Updated the HTTP middleware function to verify authorization credential, unless verification is disabled.

Depends on D12412, D12413

Test Plan

At this point, test plan from D12412 also requires credentials to be valid (token present in Identity).

Diff Detail

Repository

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

bartek created this revision.Jun 13 2024, 1:04 AM

bartek held this revision as a draft.

Herald added subscribers: tomek, ashoat. · View Herald TranscriptJun 13 2024, 1:05 AM

bartek added a child revision: D12415: [blob] Hide endpoints behind auth middleware.Jun 13 2024, 1:07 AM

bartek mentioned this in D12416: [reports] Require auth for viewing reports.Jun 13 2024, 1:10 AM

bartek mentioned this in D9283: [services-lib] Call token verification in HTTP auth middleware.Jun 13 2024, 1:10 AM

Harbormaster failed remote builds in B29641: Diff 41264!Jun 13 2024, 1:27 AM

Disable verification in commtest

Harbormaster completed remote builds in B29646: Diff 41269.Jun 13 2024, 1:50 AM

bartek published this revision for review.Jun 13 2024, 1:53 AM

bartek added inline comments.

shared/comm-lib/src/auth/types.rs
44–46 ↗	(On Diff #41269)	This is used only in low level logs (debug/trace). If this is still too dangerous, I'll remove the user_id entirely

ashoat added inline comments.Jun 13 2024, 8:21 AM

shared/comm-lib/src/auth/types.rs
44–46 ↗	(On Diff #41269)	Will these logs appear in production?

bartek added inline comments.Jun 13 2024, 9:58 PM

shared/comm-lib/src/auth/types.rs
44–46 ↗	(On Diff #41269)	No, we need to explicitly lower log level to debug (we do it only on staging) or even lower to trace (I do it only locally in dev env)

ashoat added inline comments.Jun 14 2024, 2:28 PM

shared/comm-lib/src/auth/types.rs
44–46 ↗	(On Diff #41269)	Thanks for explaining!

varun accepted this revision.Jun 17 2024, 8:59 PM

This revision is now accepted and ready to land.Jun 17 2024, 8:59 PM

Closed by commit rCOMM17e30f45d3b8: [comm-lib] Verify CSAT in HTTP 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: rCOMM17e30f45d3b8: [comm-lib] Verify CSAT in HTTP middleware.

bartek mentioned this in rCOMMc459cea60ff1: [reports] Require auth for viewing reports.

Revision Contents
Changeset List

Path

Size

shared/

comm-lib/

src/

auth/

13 lines

http/

24 lines

Diff 41264

shared/comm-lib/src/auth/types.rs

Loading...

shared/comm-lib/src/http/auth.rs

Loading...