Paths

Table of Contentst

Differential D10932

[reports] Always create AuthorizationCredential directly
ClosedPublic
Actions

Authored by bartek on Feb 2 2024, 2:05 AM.

Tags

None

Referenced Files

	F3358001: D10932.id36770.diff
	Sun, Nov 24, 2:51 AM

	F3357907: D10932.id36580.diff
	Sun, Nov 24, 2:27 AM

	F3357772: D10932.id36769.diff
	Sun, Nov 24, 1:29 AM

	F3357497: D10932.diff
	Sat, Nov 23, 11:54 PM

	Unknown Object (File)
	Fri, Nov 8, 10:03 PM

	Unknown Object (File)
	Fri, Nov 8, 7:14 PM

	Unknown Object (File)
	Fri, Nov 8, 7:14 PM

	Unknown Object (File)
	Fri, Nov 8, 7:14 PM

View All 72 Files

Subscribers

Details

Reviewers

Commits

rCOMM38451e4cef66: [reports] Always create AuthorizationCredential directly

Summary

Noticed this while testing D10931.
For authenticated endpoints this doesnt matter because they're always behind auth middleware.
But for optionally-authenticated endpoints, the request extensions isn't set by the middleware so it's always None.
Using AuthorizationCredential::from_request() also does the extensions check if possible so it's better to call it instead.

Test Plan

Test plan for D10931

Diff Detail

Repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

bartek created this revision.Feb 2 2024, 2:05 AM

bartek held this revision as a draft.

Herald added subscribers: tomek, ashoat. · View Herald TranscriptFeb 2 2024, 2:05 AM

bartek published this revision for review.Feb 2 2024, 2:18 AM

bartek added inline comments.

services/reports/src/service.rs
238 ↗	(On Diff #36580)	Ah, I should also update the comment. This should be `Some` for all endpoints as long as the `Authorization` header is provided.

Harbormaster completed remote builds in B26462: Diff 36580.Feb 2 2024, 2:24 AM

bartek added a child revision: D10931: [lib][native] Pass CSAT to Reports endpoint.Feb 2 2024, 2:34 AM

michal accepted this revision.Feb 2 2024, 9:18 AM

This revision is now accepted and ready to land.Feb 2 2024, 9:18 AM

Update comment

This revision was landed with ongoing or failed builds.Feb 7 2024, 1:35 AM

Closed by commit rCOMM38451e4cef66: [reports] Always create AuthorizationCredential directly (authored by bartek). · Explain Why

This revision was automatically updated to reflect the committed changes.

bartek added a commit: rCOMM38451e4cef66: [reports] Always create AuthorizationCredential directly.

Harbormaster completed remote builds in B26612: Diff 36769.Feb 7 2024, 1:51 AM

Revision Contents
Changeset List

Path

Size

services/

reports/

src/

11 lines

shared/

comm-lib/

src/

auth/

2 lines

Diff 36770

services/reports/src/service.rs

Loading...

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

Loading...