Skip to content

[security] Support impersonation via SASL/PLAIN authorization id - #3857

Open
beryllw wants to merge 5 commits into
apache:mainfrom
beryllw:sasl-plain-impersonation
Open

[security] Support impersonation via SASL/PLAIN authorization id#3857
beryllw wants to merge 5 commits into
apache:mainfrom
beryllw:sasl-plain-impersonation

Conversation

@beryllw

@beryllw beryllw commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Purpose

Linked issue: close #3837

Brief change log

Tests

API and Format

Documentation

@beryllw
beryllw requested a review from litiliu August 6, 2026 05:59
@beryllw

beryllw commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Thanks a lot for the review, @litiliu!
@loserwang1024 could you take a look when you get a chance?

@litiliu litiliu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One compatibility concern with #3047: FlussProtocolPlugin materializes security.sasl.plain.credentials into security.sasl.plain.jaas.config by parsing and regenerating only user_* options. This PR introduces impersonate_* as a non-credential JAAS option, so it will be silently dropped whenever the credentials map is configured or dynamically updated.

Is impersonation intentionally unsupported together with security.sasl.plain.credentials? If not, the JAAS regeneration should preserve impersonate_*, with tests covering both startup and dynamic credentials updates.

…ns when materializing SASL/PLAIN credentials
@beryllw

beryllw commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

I'll open a follow-up PR to support dynamically altering the impersonation grants themselves.

@beryllw
beryllw requested a review from litiliu August 7, 2026 01:58
* Register a new prefix here when this handler starts consuming a new option; a unit test
* asserts the plugin's extraction covers every prefix listed here.
*/
public static final List<String> KNOWN_OPTION_PREFIXES =

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be simpler and more compatible to preserve all options from the original PlainLoginModule entry instead of maintaining a prefix allowlist? Since security.sasl.plain.credentials is syntactic sugar, materializing it should ideally overlay the corresponding user_* options without dropping unrelated JAAS options. We could parse the single entry through JaasConfig, copy AppConfigurationEntry#getOptions(), overlay the credentials, and serialize it safely. This would also avoid exposing and synchronizing KNOWN_OPTION_PREFIXES.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support impersonation via SASL/PLAIN authorization id

2 participants