Skip to content

Conversation

mgoworko
Copy link
Collaborator

@mgoworko mgoworko commented Sep 20, 2025

🚀Added separate ROR KBN authentication and authorization rules

Summary by CodeRabbit

  • New Features

    • Split combined KBN rule into separate authentication and authorization rules that compose; JWT handling centralized for KBN flows.
  • Refactor

    • Rule ordering, decoder selection, variable usage and local-user resolution updated to recognize the new rule types; group-matching logic unified and improved; impersonation-warning handling expanded.
  • Tests

    • Added extensive unit and integration tests covering decoding, YAML configs, JWT signatures, authn/authz flows and groups logic.

@mgoworko mgoworko marked this pull request as ready for review September 21, 2025 13:19

This comment was marked as outdated.

coderabbitai[bot]

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

@mgoworko mgoworko requested a review from coutoPL September 21, 2025 14:50
coderabbitai[bot]

This comment was marked as resolved.

Copy link
Collaborator

@coutoPL coutoPL left a comment

Choose a reason for hiding this comment

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

LGTM

coderabbitai[bot]

This comment was marked as outdated.

coderabbitai[bot]

This comment was marked as outdated.

Copy link
Collaborator Author

@mgoworko mgoworko left a comment

Choose a reason for hiding this comment

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

Description of the changes after e2e tests:

@mgoworko mgoworko requested a review from coutoPL October 6, 2025 16:24
coderabbitai[bot]

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as outdated.

val rule = new RorKbnAuthorizationRule(settings)
Right(RuleDefinition.create[RorKbnAuthorizationRule](rule))
case (Some(_), None) =>
Left(RulesLevelCreationError(Message(s"Cannot create ${RorKbnAuthorizationRule.Name.name.show} - missing groups settings")))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe "missing groups logic"? And maybe it'd be good to add this link to the doc too?

https://github.com/beshu-tech/readonlyrest-docs/blob/master/details/authorization-rules-details.md#checking-groups-logic

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added this link in both places. Is it correct, that this section is not published on docs.readonlyrest.com ? (At least I couldn't find it)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably not. During adding the new rules descriptions, could you please check why?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

On docs.readonlyrest.com the authorization-rules-details.md document is linked as redirection to Github repo blob, not a document exposed on the website. I'm not sure if it is correct, but the links are in this PR are done the same way, so it is consistent.

@mgoworko mgoworko requested a review from coutoPL October 10, 2025 18:45
coderabbitai[bot]

This comment was marked as outdated.

Copy link
Collaborator

@coutoPL coutoPL left a comment

Choose a reason for hiding this comment

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

LGTM

@mgoworko mgoworko merged commit 6dbd71d into sscarduzio:develop Oct 11, 2025
22 checks passed
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.

2 participants