-
Notifications
You must be signed in to change notification settings - Fork 45
docs: clarify OIDC principal format in authentication #1302
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for redpanda-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughThe PR updates the OIDC guidance in modules/manage/partials/authentication.adoc. It changes instructions to use the exact JWT sub claim value for superusers and ACL rules, removing prior examples that used equality checks and the user:OIDC: prefix. It clarifies that OIDC principals are derived from token claims via configured principal mapping and do not require a prefix. No code or exported entities are modified; this is a documentation-only change focused on principal mapping and representation. Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the authentication documentation to clarify how OIDC principals should be formatted in different contexts. The documentation was previously unclear about the format differences between OIDC and SASL authentication methods.
- Clarifies that OIDC principals use the sub claim value directly without prefixes
- Provides clearer examples for both superusers configuration and ACL rules
- Distinguishes OIDC principal formatting from SASL user formatting
dd3c9a5
to
363a18b
Compare
Update the authentication documentation to better explain how OIDC principals should be formatted in superusers configuration and ACL rules. The changes make it clear that the sub claim value is used directly without prefixes, unlike SASL users. Signed-off-by: Santiago Jimenez Giraldo <[email protected]>
363a18b
to
c1e986b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
modules/manage/partials/authentication.adoc (1)
69-69
: Use an admonition and scope the “User:” prefix note to ACLs; add ending periodConvert to a [NOTE] block for consistency and clarify that the “User:” prefix applies to SASL principals in ACLs only, while OIDC principals never use a prefix. Also add the missing period.
-Note: Unlike SASL users which require the User: prefix, OIDC principals use the value extracted from the token's claims (according to the configured principal mapping) without any prefix +[NOTE] +==== +In ACLs, SASL principals require the `User:` prefix (for example, `User:myuser`). OIDC principals do not use any prefix; use the value extracted from the token's claims according to the configured principal mapping (default `$.sub`). +====
- If your OIDC token's sub (subject) claim contains [email protected], then: | ||
* For superusers configuration: Use the exact sub claim value: [email protected] | ||
* For ACL rules: Use the exact sub claim value: [email protected] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Clarify equality and tighten wording (“contains” → “equals”), and be precise about where to use the value
“contains” is ambiguous for a single-valued claim. Recommend stating that sub equals the value, and aligning the bullets with “superusers list” and ACL usage.
- - If your OIDC token's sub (subject) claim contains [email protected], then:
-* For superusers configuration: Use the exact sub claim value: [email protected]
-* For ACL rules: Use the exact sub claim value: [email protected]
+ - If your OIDC token's sub (subject) claim is [email protected], then:
+* For the superusers list (cluster configuration): use the exact sub claim value: [email protected]
+* For ACL rules: use the exact sub claim value: [email protected]
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
- If your OIDC token's sub (subject) claim contains [email protected], then: | |
* For superusers configuration: Use the exact sub claim value: [email protected] | |
* For ACL rules: Use the exact sub claim value: [email protected] | |
- If your OIDC token's sub (subject) claim is [email protected], then: | |
* For the superusers list (cluster configuration): use the exact sub claim value: [email protected] | |
* For ACL rules: use the exact sub claim value: [email protected] |
🤖 Prompt for AI Agents
In modules/manage/partials/authentication.adoc around lines 66 to 68, the
wording is ambiguous — replace "contains" with "equals" and clarify the bullets
to state where to use the exact sub claim value: specify "For superusers list:
use the exact sub claim value ([email protected])" and "For ACL rules: use the
exact sub claim value ([email protected])" so both bullets consistently and
precisely indicate equality and the intended configuration target.
- If your OIDC token's sub (subject) claim contains [email protected], then: | ||
* For superusers configuration: Use the exact sub claim value: [email protected] | ||
* For ACL rules: Use the exact sub claim value: [email protected] | ||
Note: Unlike SASL users which require the User: prefix, OIDC principals use the value extracted from the token's claims (according to the configured principal mapping) without any prefix |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- If your OIDC token's sub (subject) claim contains [email protected], then: | |
* For superusers configuration: Use the exact sub claim value: [email protected] | |
* For ACL rules: Use the exact sub claim value: [email protected] | |
Note: Unlike SASL users which require the User: prefix, OIDC principals use the value extracted from the token's claims (according to the configured principal mapping) without any prefix | |
If your OIDC token's sub (subject) claim is [email protected], use that exact sub claim value for superusers configuration and ACL rules. | |
Unlike SASL users which require the `User:` prefix, OIDC principals use the value extracted from the token's claims (according to the configured principal mapping) without any prefix |
- If your token's `sub` claim is `[email protected]`, then: | ||
* In the `superusers` list, specify the principal as `[email protected]` | ||
* In ACL rules, specify the principal as `user:OIDC:[email protected]` | ||
- If your OIDC token's sub (subject) claim contains [email protected], then: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure "contains" is clearer than "is", to me, contains implies that there could be other characters preceding or following it.
- If your OIDC token's sub (subject) claim contains [email protected], then: | ||
* For superusers configuration: Use the exact sub claim value: [email protected] | ||
* For ACL rules: Use the exact sub claim value: [email protected] | ||
Note: Unlike SASL users which require the User: prefix, OIDC principals use the value extracted from the token's claims (according to the configured principal mapping) without any prefix |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really know where this prefix comes in, or is displayed, or where the user specifies it.
I think it's pretty much in :8081/security/acls
, and what's returned from rpk security acl list
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you maybe help me here @r-vasquez , both are valid in my experience (User:, ), I am not sure what's preferable or which one should be in the documentation ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My 2c with this doc change is that the document talks about configuring principals, and then we mention that we can use sub claims, I think we should be more explicit, something like:
If your OIDC token's sub (subject) claim is [email protected], use that exact sub claim value as your principal for superusers configuration and ACL rules.
Unlike SASL users which require the `User:` principal type prefix, OIDC principals use the value extracted from the token's claims (according to the configured principal mapping) without any prefix
I think it's pretty much in :8081/security/acls, and what's returned from rpk security acl list?
Yes, also when creating, we have an example in this same page below
Grant the new user describe privileges for a topic called myfirsttopic:
rpk security acl create --allow-principal User:myuser \
Which is actually not needed, but it doesn't hurt. 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Grant the new user describe privileges for a topic called myfirsttopic:
rpk security acl create --allow-principal User:myuser \
_Which is actually not needed, but it doesn't hurt. 😅_
It's not needed, and shouldn't be in the docs.
Oh, I do not like this UX:
Without prefix:
rpk security acl create --allow-principal myuser ...
rpk security acl list
PRINCIPAL HOST RESOURCE-TYPE RESOURCE-NAME RESOURCE-PATTERN-TYPE OPERATION PERMISSION ERROR
User:myuser * TOPIC model- PREFIXED ALL ALLOW
With prefix
rpk security acl create --allow-principal User:myuser ...
rpk security acl list
PRINCIPAL HOST RESOURCE-TYPE RESOURCE-NAME RESOURCE-PATTERN-TYPE OPERATION PERMISSION ERROR
User:myuser * TOPIC model- PREFIXED ALL ALLOW
Principal that contains the User:
prefix:
rpk security acl create --allow-principal User:User:myuser ...
rpk security acl list
PRINCIPAL HOST RESOURCE-TYPE RESOURCE-NAME RESOURCE-PATTERN-TYPE OPERATION PERMISSION ERROR
User:User:myuser * TOPIC model- PREFIXED ALL ALLOW
The prefix isn't needed because to get a role, you ask for a role:
rpk security acl create --allow-role myrole ...
rpk security acl list
PRINCIPAL HOST RESOURCE-TYPE RESOURCE-NAME RESOURCE-PATTERN-TYPE OPERATION PERMISSION ERROR
RedpandaRole:myrole * TOPIC model- PREFIXED ALL ALLOW
Description
Update the authentication documentation to better explain how OIDC principals should be formatted in superusers configuration and ACL rules. The changes make it clear that the sub claim value is used directly without prefixes, unlike SASL users.
Resolves https://redpandadata.atlassian.net/browse/
Review deadline:
Page previews
Checks