Skip to content

branch-4.1: [improvement](log) Log the user name of HTTP requests for auditing #66745 - #67319

Merged
yiguolei merged 1 commit into
branch-4.1from
auto-pick-66745-branch-4.1
Sep 7, 2026
Merged

branch-4.1: [improvement](log) Log the user name of HTTP requests for auditing #66745#67319
yiguolei merged 1 commit into
branch-4.1from
auto-pick-66745-branch-4.1

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

Cherry-picked from #66745

@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@hello-stephen

Copy link
Copy Markdown
Contributor

run buildall

…66745)
### What problem does this PR solve?
Problem Summary:
`HttpRequest::debug_string()` masks the whole `Authorization` header, so
the BE request log shows that a request carried credentials but not
whose, leaving operations such as `/api/update_config` without an audit
trail. Keep the user name of HTTP Basic credentials and mask only the
password, rendering `<user>:***MASKED***`. Every other sensitive header
(`token`, `auth-token`, `auth_code`, `proxy-authorization`), every
non-Basic scheme, and any credential that cannot be decoded stays masked
in full. What is emitted is a rendering, not the header value: the real
one is base64 encoded.
One more path wrote credentials to the logs in clear text and is fixed
as well: FE's `BaseController` logged the raw `Authorization` header,
that is `base64(user:password)`, at INFO level when parsing failed. This
is a parse failure rather than an authentication failure, so it runs
before the password is verified and the header often carries valid
credentials. Only whether the header was absent or malformed is logged
now.
Note: an earlier revision of this PR also rewrote the `permission
verification failed` log in `HttpHandlerWithAuth`, which streamed
`TCheckAuthRequest` and let the thrift-generated `printTo()` dump
`passwd`. #66618 has since landed a fix for that same leak, so this PR
was rebased onto it and no longer touches that file.
### Release note
None
### Check List (For Author)
- Test
- [x] Unit Test
- [x] Manual test (add detailed scripts or steps below)
`HttpRequestTest` covers the rendering of the `Authorization` header:
the user name is kept for Basic credentials (including a password
containing colons and a case insensitive scheme), while a malformed,
undecodable or non-Basic credential and every other sensitive header
stay masked in full.
Also verified on a single FE + BE cluster that no credential appears in
`fe.log`, `be.INFO` or `be.WARNING`.
- Behavior changed:
- [x] No.
- Does this need documentation?
- [x] No.
@yiguolei
yiguoleiforce-pushed the auto-pick-66745-branch-4.1 branch from 9d4e63a to 23bdf9cCompareSeptember 6, 2026 07:57
@yiguolei

Copy link
Copy Markdown
Contributor

run buildall

@hello-stephen

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 100.00% (21/21) 🎉

Increment coverage report
Complete coverage report

CategoryCoverage
Function Coverage74.42% (31374/42157)
Line Coverage58.63% (348947/595143)
Region Coverage55.42% (291428/525869)
Branch Coverage56.20% (131099/233266)

@github-actions

Copy link
Copy Markdown
ContributorAuthor

PR approved by anyone and no changes requested.

@github-actions

Copy link
Copy Markdown
ContributorAuthor

PR approved by at least one committer and no changes requested.

@github-actionsgithub-actionsBot added the approved Indicates a PR has been approved by one committer. label Sep 7, 2026
@yiguolei
yiguolei merged commit e0cfca1 into branch-4.1Sep 7, 2026
31 of 35 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approvedIndicates a PR has been approved by one committer.reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@hello-stephen@yiguolei@liaoxin01