Skip to content

Reuse principal role prefix constant in token validation - #5300

Open
flyrain wants to merge 1 commit into
apache:mainfrom
flyrain:yf/reuse-principal-role-prefix
Open

Reuse principal role prefix constant in token validation#5300
flyrain wants to merge 1 commit into
apache:mainfrom
flyrain:yf/reuse-principal-role-prefix

Conversation

@flyrain

@flyrain flyrain commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Checklist

  • 🛡️ Don't disclose security issues! (contact security@apache.org)
  • 🔗 Clearly explained why the changes are needed, or linked related issues: Fixes #
  • 🧪 Added/updated tests with good coverage, or manually tested (and explained how)
  • 💡 Added comments for complex logic
  • 🧾 Updated CHANGELOG.md (if needed)
  • 📚 Updated documentation in site/content/in-dev/unreleased (if needed)

Copilot AI lite review requested due to automatic review settings August 16, 2026 06:07
@github-project-automation github-project-automation Bot moved this to PRs In Progress in Basic Kanban Board Aug 16, 2026

Copilot AI 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.

Pull request overview

This PR reduces duplication in OAuth token request scope validation by reusing the shared PRINCIPAL_ROLE prefix constant from DefaultAuthenticator, keeping the prefix definition centralized within the runtime service auth layer.

Changes:

  • Replaced the local POLARIS_ROLE_PREFIX constant with DefaultAuthenticator.PRINCIPAL_ROLE_PREFIX during scope validation.
  • Removed the now-redundant prefix constant from TokenRequestValidator.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

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.

3 participants