Skip to content

fix(security): preserve authorization behavior over gRPC - #489

Merged
yordis merged 1 commit into
masterfrom
yordis/chore-retire-clientapi-security
Sep 13, 2026
Merged

yordis merged 1 commit into
masterfrom
yordis/chore-retire-clientapi-security

Conversation

@yordis

@yordis yordis commented Sep 12, 2026 •

Copy link
Copy Markdown
Member
  • Keeps stream authorization guarantees enforced while the unsupported client transport retires.
  • Preserves independently visible CI coverage for ACL inheritance, system streams, metadata, subscriptions, and the all-stream boundary.

@yordis
yordis requested a review from a team as a code owner September 12, 2026 23:21
@cursor

cursor Bot commented Sep 12, 2026 •

Copy link
Copy Markdown

PR Summary

Medium Risk
Authorization behavior is only validated through relocated integration tests; dropping ClientAPI security tests increases reliance on the new gRPC suite matching prior guarantees.

Overview
Replaces legacy TCP ClientAPI security integration tests with an equivalent gRPC Streams/Users suite, so authorization rules stay exercised as the old client transport goes away.

The large EventStore.Core.Tests.ClientAPI.Security tree (including AuthenticationTestBase and per-operation fixtures) is removed. Coverage moves under EventStore.Core.Tests.Services.Transport.Grpc.Security, built on GrpcSpecification with gRPC Append/Read/BatchAppend/Tombstone, Users API for setup, and StatusCode assertions for the same ACL scenarios (stream/system defaults, $all, metadata, subscriptions, delete ACLs, multi-role settings, default credentials).

CI sharding renames the matrix job from core-clientapi-security to core-grpc-security, wires scripts/test.sh to filter that namespace, and excludes it from the core-services shard so security runs as its own 20m job.

Reviewed by Cursor Bugbot for commit 96c7a31. Bugbot is set up for automated code reviews on this repo. Configure here.

@coderabbitai

coderabbitai Bot commented Sep 12, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 22 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 6e06a50c-3580-4f1f-b23c-546a80b1a6c5

📥 Commits

Reviewing files that changed from the base of the PR and between 7b0fbe3 and 96c7a31.

📒 Files selected for processing (38)
  • .github/workflows/build-container-ubuntu-lts.yml
  • scripts/test.sh
  • src/EventStore.Core.Tests/ClientAPI/Security/AuthenticationTestBase.cs
  • src/EventStore.Core.Tests/ClientAPI/Security/all_stream_with_no_acl_security.cs
  • src/EventStore.Core.Tests/ClientAPI/Security/authorized_default_credentials_security.cs
  • src/EventStore.Core.Tests/ClientAPI/Security/delete_stream_security.cs
  • src/EventStore.Core.Tests/ClientAPI/Security/multiple_role_security.cs
  • src/EventStore.Core.Tests/ClientAPI/Security/overriden_system_stream_security.cs
  • src/EventStore.Core.Tests/ClientAPI/Security/overriden_system_stream_security_for_all.cs
  • src/EventStore.Core.Tests/ClientAPI/Security/overriden_user_stream_security.cs
  • src/EventStore.Core.Tests/ClientAPI/Security/read_all_security.cs
  • src/EventStore.Core.Tests/ClientAPI/Security/read_stream_meta_security.cs
  • src/EventStore.Core.Tests/ClientAPI/Security/read_stream_security.cs
  • src/EventStore.Core.Tests/ClientAPI/Security/stream_security_inheritance.cs
  • src/EventStore.Core.Tests/ClientAPI/Security/subscribe_to_all_security.cs
  • src/EventStore.Core.Tests/ClientAPI/Security/subscribe_to_stream_security.cs
  • src/EventStore.Core.Tests/ClientAPI/Security/system_stream_security.cs
  • src/EventStore.Core.Tests/ClientAPI/Security/transactional_write_stream_security.cs
  • src/EventStore.Core.Tests/ClientAPI/Security/write_stream_meta_security.cs
  • src/EventStore.Core.Tests/ClientAPI/Security/write_stream_security.cs
  • src/EventStore.Core.Tests/Services/Transport/Grpc/Security/AuthenticationTestBase.cs
  • src/EventStore.Core.Tests/Services/Transport/Grpc/Security/all_stream_with_no_acl_security.cs
  • src/EventStore.Core.Tests/Services/Transport/Grpc/Security/authorized_default_credentials_security.cs
  • src/EventStore.Core.Tests/Services/Transport/Grpc/Security/delete_stream_security.cs
  • src/EventStore.Core.Tests/Services/Transport/Grpc/Security/multiple_role_security.cs
  • src/EventStore.Core.Tests/Services/Transport/Grpc/Security/overriden_system_stream_security.cs
  • src/EventStore.Core.Tests/Services/Transport/Grpc/Security/overriden_system_stream_security_for_all.cs
  • src/EventStore.Core.Tests/Services/Transport/Grpc/Security/overriden_user_stream_security.cs
  • src/EventStore.Core.Tests/Services/Transport/Grpc/Security/read_all_security.cs
  • src/EventStore.Core.Tests/Services/Transport/Grpc/Security/read_stream_meta_security.cs
  • src/EventStore.Core.Tests/Services/Transport/Grpc/Security/read_stream_security.cs
  • src/EventStore.Core.Tests/Services/Transport/Grpc/Security/stream_security_inheritance.cs
  • src/EventStore.Core.Tests/Services/Transport/Grpc/Security/subscribe_to_all_security.cs
  • src/EventStore.Core.Tests/Services/Transport/Grpc/Security/subscribe_to_stream_security.cs
  • src/EventStore.Core.Tests/Services/Transport/Grpc/Security/system_stream_security.cs
  • src/EventStore.Core.Tests/Services/Transport/Grpc/Security/transactional_write_stream_security.cs
  • src/EventStore.Core.Tests/Services/Transport/Grpc/Security/write_stream_meta_security.cs
  • src/EventStore.Core.Tests/Services/Transport/Grpc/Security/write_stream_security.cs

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@yordis
yordis added this pull request to stack #500 September 12, 2026 23:22
@yordis
yordis force-pushed the yordis/chore-retire-clientapi-security branch from a5d2e63 to 5044422 Compare September 13, 2026 00:15
@yordis
yordis force-pushed the yordis/chore-retire-clientapi-security branch from 5044422 to 5178aae Compare September 13, 2026 00:33
@yordis
yordis force-pushed the yordis/chore-retire-clientapi-security branch 2 times, most recently from 1ccea73 to c66fe60 Compare September 13, 2026 01:16
@yordis
yordis force-pushed the yordis/chore-retire-clientapi-security branch 2 times, most recently from 42cf126 to 868632d Compare September 13, 2026 01:46
@yordis
yordis force-pushed the yordis/chore-retire-clientapi-security branch from 868632d to 36de56c Compare September 13, 2026 02:04
@yordis
yordis force-pushed the yordis/chore-retire-clientapi-security branch 2 times, most recently from 0539aea to 6c0982f Compare September 13, 2026 02:59
@yordis
yordis force-pushed the yordis/chore-retire-clientapi-security branch from 6c0982f to 1f90916 Compare September 13, 2026 03:14
Base automatically changed from yordis/chore-retire-clientapi-persistent to master September 13, 2026 03:18
@yordis
yordis force-pushed the yordis/chore-retire-clientapi-security branch from 1f90916 to 53a2c48 Compare September 13, 2026 03:46
@yordis yordis changed the title chore(tests): retire redundant TCP security coverage fix(security): preserve authorization behavior over gRPC Sep 13, 2026
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
@yordis
yordis force-pushed the yordis/chore-retire-clientapi-security branch from 53a2c48 to 96c7a31 Compare September 13, 2026 04:23
@yordis
yordis merged commit 0ef196b into master Sep 13, 2026
32 of 47 checks passed
@yordis
yordis deleted the yordis/chore-retire-clientapi-security branch September 13, 2026 05:57
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.

1 participant