docs: add a tech note on API authn/authz #96427
Conversation
|
This change is |
knz
force-pushed
the
20230202-api-authn-tech-note
branch
12 times, most recently
from
c778230 to
f501c46
Compare
96451: server: only forward the SQL identity in gRPC metadata r=andreimatei a=knz Requested by `@andreimatei` . Informs #96427. Informs #45018. Prior to this patch, we were forwarding any and all gRPC metdata during a RPC fanout. This was creating doubt and confusion, about how much data is really important/useful to forward. Analysis suggests we only care about the SQL user identity resulting from HTTP authentication. So this patch limits the forwarding to just that information. This specialization makes the forwarding logic easier to understand. This patch additionally renames functions as follows: | Old name | New name | |---------------------------------|---------------------------------------| | `userFromContext` | `userFromIncomingRPCContext` | | `getSQLUsername` | `userFromHTTPAuthInfoContext` | | `apiToOutgoingGatewayCtx` | `forwardHTTPAuthInfoToRPCCalls` | | `forwardAuthenticationMetadata` | `translateHTTPAuthInfoToGRPCMetadata` | | `propagateGatewayMetadata` | `forwardSQLIdentityThroughRPCCalls` | Release note: None Epic: None Co-authored-by: Raphael 'kena' Poss <[email protected]>
knz
force-pushed
the
20230202-api-authn-tech-note
branch
from
f501c46 to
b7f2729
Compare
There was a problem hiding this comment.
good note
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @dhartunian and @knz)
docs/tech-notes/api_authentication.md line 47 at r1 (raw file):
### Unauthenticated pure HTTP handler HTTP -> direct HTTP handler
could you explain how come authenticationV2Mux is not in the call chain here? Where's the distinction being made between handlers that want auth and those who don't?
There was a problem hiding this comment.
Reviewable status:
There was a problem hiding this comment.
Reviewable status:
docs/tech-notes/api_authentication.md line 47 at r1 (raw file):
Previously, andreimatei (Andrei Matei) wrote…
could you explain how come
authenticationV2Muxis not in the call chain here? Where's the distinction being made between handlers that want auth and those who don't?
it's just health and vars. We opted them out, because they don't leak state out of the cluster.
|
@dhartunian Andrei's question raises a good point: the prometheus vars endpoint is currently exposed without authentication (for reasons that can be pulled back from the git history). What do you think will happen if we start exposing more "intelligent" labels for metrics? I remember your project to annotate metrics using statement fingerprints. Maybe tenant labels as well. Do we want this data to be scrapable without authentication? This might need followup issues to investigate further. |
Release note: None
knz
force-pushed
the
20230202-api-authn-tech-note
branch
from
b7f2729 to
c9521be
Compare
|
bors r=andreimatei,dhartunian |
|
Build succeeded: |
Text of the tech note here: https://github.com/knz/cockroach/blob/20230202-api-authn-tech-note/docs/tech-notes/api_authentication.md
Release note: None
Epic: None