feat(identity): Contracts and GetUserPermissions gRPC (Phase 2)#92
Merged
Conversation
Co-authored-by: Phuong Nguyen <phuongnse@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (3)
📝 WalkthroughWalkthroughThis PR implements a gRPC endpoint for querying user permissions in the Identity module. It adds a Protobuf contract and Contracts project, implements a CQRS query and handler that aggregates role permissions, exposes it via an IdentityGrpcService with validation and error mapping, wires the endpoint into the API host, adds tests, and updates progress/playbook docs. ChangesIdentity gRPC GetUserPermissions
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/epics/E02-identity-access/README.md`:
- Line 69: Replace the plain “Avro lifecycle events pending” note with an
explicit deferred-followup callout using the required format; update the
Contracts line referencing Axis.Identity.Contracts / IdentityService.proto
(`GetUserPermissions`) to read something like “Deferred (PR #<N> follow-up):
Avro lifecycle events” (replace <N> with the actual PR number) so the entry
documents the follow-up as a Deferred (PR `#N` follow-up): callout rather than
plain “pending”.
In `@docs/epics/README.md`:
- Line 45: Update the Phase 2 line to replace the generic “follow” note with an
explicit deferred callout using the required pattern: change the trailing text
after “Identity `Axis.Identity.Contracts` + `GetUserPermissions` gRPC pilot;” to
include “Deferred (PR `#N` follow-up): remaining modules + Kafka
`OrganizationVerified`” (fill in the actual PR number) so the line reads using
the exact phrase "Deferred (PR `#N` follow-up): ..." instead of "follow" and
ensure `OrganizationVerified` and module wording are preserved.
In `@src/Modules/Identity/Axis.Identity.Contracts/Protos/identity_service.proto`:
- Line 5: The proto package declaration in identity_service.proto (package
axis.identity.v1) does not match its directory layout; move
identity_service.proto into Protos/axis/identity/v1/identity_service.proto so
the directory path aligns with the package, and then update the contracts
project include/import path (the contracts project file that references the
Protos folder) to point to the new Protos/axis/identity/v1 location so Buf
PACKAGE_DIRECTORY_MATCH and CI pass.
In
`@src/Modules/Identity/Axis.Identity.Infrastructure/Extensions/IdentityInfrastructureExtensions.cs`:
- Around line 56-59: The Identity gRPC service is mapped without auth or
rate-limiting and the GetUserPermissions method has no guards; modify
MapIdentityGrpc to capture the endpoint convention builder (e.g., var builder =
app.MapGrpcService<IdentityGrpcService>();) and chain
builder.RequireAuthorization() and builder.RequireRateLimiting("auth");
additionally add endpoint/method-level protection by decorating
IdentityGrpcService.GetUserPermissions with the [Authorize] attribute (and/or
[EnableRateLimiting("auth")] if using attribute-based rate limiting in your
project) so the permissions RPC cannot be invoked anonymously.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fa3e91ea-65a7-478f-9247-9b871a0d9138
📒 Files selected for processing (15)
Axis.slnDirectory.Packages.propsdocs/PROGRESS.mddocs/epics/E02-identity-access/README.mddocs/epics/README.mdsrc/Axis.Api/Program.cssrc/Axis.Api/appsettings.jsonsrc/Modules/Identity/Axis.Identity.Application/Queries/GetUserPermissions/GetUserPermissionsHandler.cssrc/Modules/Identity/Axis.Identity.Application/Queries/GetUserPermissions/GetUserPermissionsQuery.cssrc/Modules/Identity/Axis.Identity.Contracts/Axis.Identity.Contracts.csprojsrc/Modules/Identity/Axis.Identity.Contracts/Protos/identity_service.protosrc/Modules/Identity/Axis.Identity.Infrastructure/Axis.Identity.Infrastructure.csprojsrc/Modules/Identity/Axis.Identity.Infrastructure/Extensions/IdentityInfrastructureExtensions.cssrc/Modules/Identity/Axis.Identity.Infrastructure/Grpc/IdentityGrpcService.cstests/Modules/Identity/Axis.Identity.Application.Tests/Queries/GetUserPermissionsHandlerTests.cs
|
|
||
| option csharp_namespace = "Axis.Identity.Contracts.Grpc"; | ||
|
|
||
| package axis.identity.v1; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Confirm declared package
rg -n '^\s*package\s+' src/Modules/Identity/Axis.Identity.Contracts/Protos/identity_service.proto
# Re-run lint if buf exists in your environment
if command -v buf >/dev/null 2>&1; then
buf lint
else
echo "buf CLI not installed locally; run 'buf lint' in CI/dev container to verify."
fiRepository: phuong-labs/axis
Length of output: 347
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the Identity.Contracts csproj
csproj="$(fd -e csproj -a --hidden . src/Modules/Identity -t f | head -n 50 | rg -n "Axis\.Identity\.Contracts|Identity\.Contracts" -o || true)"
# If the above heuristic fails, just list all csproj under that module
if [ -z "${csproj}" ]; then
echo "No matching csproj found by name heuristic; listing csproj under src/Modules/Identity:"
fd -e csproj -a --hidden src/Modules/Identity -t f
exit 0
fi
echo "Candidate csproj: $csproj"
# Search within the csproj for the Protobuf Include entry referencing identity_service.proto
rg -n --hidden -S 'Protobuf\s+Include=.*identity_service\.proto' src/Modules/Identity -g '*.csproj' || true
# Also show any Protobuf entries in that contracts project file
rg -n --hidden -S '<Protobuf\s+Include=' src/Modules/Identity -g '*.csproj' || trueRepository: phuong-labs/axis
Length of output: 442
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect existing .proto files under Axis.Identity.Contracts/Protos to see how package->directory mapping is done elsewhere
fd -e proto src/Modules/Identity/Axis.Identity.Contracts/Protos -t f | sort > /tmp/protos_identity.txt
echo "Found protos:"
cat /tmp/protos_identity.txt
echo
echo "Package declarations and containing directories:"
while IFS= read -r p; do
dir="$(dirname "$p")"
pkg="$(rg -n '^\s*package\s+' "$p" | head -n1 | sed -E 's/^[0-9]+:\s*package\s*//; s/;\s*$//' || true)"
echo "- $p"
echo " dir: $dir"
echo " package: $pkg"
done < /tmp/protos_identity.txtRepository: phuong-labs/axis
Length of output: 570
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -e proto -t f src/Modules/Identity/Axis.Identity.Contracts/Protos --full-path | sort > /tmp/protos_identity.txt
echo "Found protos:"
cat /tmp/protos_identity.txt
echo
echo "Package declarations and containing directories:"
while IFS= read -r p; do
dir="$(dirname "$p")"
pkg="$(rg -n '^\s*package\s+' "$p" | head -n1 | sed -E 's/^[0-9]+:\s*package\s*//; s/;\s*$//' || true)"
echo "- $p"
echo " dir: $dir"
echo " package: ${pkg:-<none>}"
done < /tmp/protos_identity.txtRepository: phuong-labs/axis
Length of output: 363
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find all proto files under any Axis.*.Contracts/Protos directory
fd -e proto -t f --full-path 'src/Modules/*/Axis.*.Contracts/Protos' > /tmp/all_contract_protos.txt
echo "Proto count:"
wc -l /tmp/all_contract_protos.txt
echo
echo "Sample of proto files (first 30):"
head -n 30 /tmp/all_contract_protos.txt
echo
echo "Checking package declarations vs directory suffix for each proto:"
python3 - <<'PY'
import re, pathlib, sys
paths=[p.strip() for p in open('/tmp/all_contract_protos.txt') if p.strip()]
for p in paths:
text=pathlib.Path(p).read_text(encoding='utf-8', errors='ignore')
m=re.search(r'^\s*package\s+([^;]+);', text, re.M)
pkg=m.group(1).strip() if m else None
rel=pathlib.Path(p).relative_to(pathlib.Path('.').resolve())
# best-effort: print path relative to Protos/ root
parts=list(rel.parts)
# find the index of 'Protos'
if 'Protos' in parts:
i=parts.index('Protos')
after=parts[i+1:-1] # dir parts after Protos
else:
after=parts[:-1]
after_dir='/'.join(after)
print(f"- {rel.as_posix()}")
print(f" package: {pkg}")
print(f" dir_after_Protos: {after_dir}")
PYRepository: phuong-labs/axis
Length of output: 206
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Enumerate all proto files under src/Modules and keep only those in Axis.*.Contracts/Protos
fd -e proto -t f --full-path src/Modules | rg 'Axis\..*\.Contracts/Protos/' > /tmp/all_contract_protos.txt
echo "Proto count:"
wc -l /tmp/all_contract_protos.txt
echo
echo "Sample of proto files (first 30):"
head -n 30 /tmp/all_contract_protos.txt
echo
echo "Checking package declarations vs directory suffix for each proto:"
python3 - <<'PY'
import re, pathlib
paths=[p.strip() for p in open('/tmp/all_contract_protos.txt') if p.strip()]
if not paths:
print("No proto files found matching Axis.*.Contracts/Protos under src/Modules.")
raise SystemExit(0)
for p in paths:
pth=pathlib.Path(p)
text=pth.read_text(encoding='utf-8', errors='ignore')
m=re.search(r'^\s*package\s+([^;]+);', text, re.M)
pkg=m.group(1).strip() if m else None
parts=pth.parts
# locate 'Protos' in path
if 'Protos' in parts:
i=parts.index('Protos')
# directory after Protos, before filename
after='/'.join(parts[i+1:-1])
else:
after=''
print(f"- {pth.as_posix()}")
print(f" package: {pkg}")
print(f" dir_after_Protos: {after}")
PYRepository: phuong-labs/axis
Length of output: 407
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate buf config files
fd -e yaml -e yml -t f --full-path 'buf' . > /tmp/buf_yaml_files.txt || true
# More targeted: common buf filenames
fd --full-path 'buf.yaml' . > /tmp/buf_yaml_files2.txt || true
fd --full-path 'buf.yml' . > /tmp/buf_yaml_files3.txt || true
fd --full-path 'buf.gen.yaml' . > /tmp/buf_yaml_files4.txt || true
fd --full-path 'buf.gen.yml' . > /tmp/buf_yaml_files5.txt || true
cat /tmp/buf_yaml_files.txt /tmp/buf_yaml_files2.txt /tmp/buf_yaml_files3.txt /tmp/buf_yaml_files4.txt /tmp/buf_yaml_files5.txt 2>/dev/null | sort -u > /tmp/buf_configs.txt
echo "Buf config files found:"
cat /tmp/buf_configs.txt
echo
echo "---- buf.yaml (or first buf config) contents: ----"
first="$(head -n1 /tmp/buf_configs.txt || true)"
if [ -n "$first" ]; then
echo "Showing: $first"
sed -n '1,200p' "$first"
fi
echo
echo "---- Lint rules (PACKAGE_DIRECTORY_MATCH) in buf configs ----"
rg -n 'PACKAGE_DIRECTORY_MATCH|lint:|breaking:|modules:' -S /tmp/buf_configs.txt . || trueRepository: phuong-labs/axis
Length of output: 852
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search by common buf config filenames (case-insensitive just in case)
fd --full-path -t f -i 'buf.yaml' . > /tmp/buf_files_by_name.txt || true
fd --full-path -t f -i 'buf.yml' . >> /tmp/buf_files_by_name.txt || true
fd --full-path -t f -i 'buf.gen.yaml' . >> /tmp/buf_files_by_name.txt || true
fd --full-path -t f -i 'buf.gen.yml' . >> /tmp/buf_files_by_name.txt || true
fd --full-path -t f -i 'buf.*.yaml' . >> /tmp/buf_files_by_name.txt || true
# De-dup
sort -u /tmp/buf_files_by_name.txt > /tmp/buf_files.txt
echo "Buf config files found by filename:"
cat /tmp/buf_files.txt || true
echo
echo "---- Showing first 2 buf config files (if any) ----"
count=0
while IFS= read -r f; do
[ -z "$f" ] && continue
echo "### $f"
sed -n '1,200p' "$f"
echo
count=$((count+1))
[ $count -ge 2 ] && break
done < /tmp/buf_files.txtRepository: phuong-labs/axis
Length of output: 149
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for buf-related configs, including ignored/hidden files
rm -f /tmp/buf_files.txt
touch /tmp/buf_files.txt
fd --full-path --hidden --no-ignore -t f -i 'buf.work.yaml' . >> /tmp/buf_files.txt 2>/dev/null || true
fd --full-path --hidden --no-ignore -t f -i 'buf.yaml' . >> /tmp/buf_files.txt 2>/dev/null || true
fd --full-path --hidden --no-ignore -t f -i 'buf.yml' . >> /tmp/buf_files.txt 2>/dev/null || true
fd --full-path --hidden --no-ignore -t f -i 'buf.gen.yaml' . >> /tmp/buf_files.txt 2>/dev/null || true
fd --full-path --hidden --no-ignore -t f -i 'buf.gen.yml' . >> /tmp/buf_files.txt 2>/dev/null || true
fd --full-path --hidden --no-ignore -t f -i 'buf.*.yaml' . >> /tmp/buf_files.txt 2>/dev/null || true
fd --full-path --hidden --no-ignore -t f -i 'buf.lock' . >> /tmp/buf_files.txt 2>/dev/null || true
# De-dup
sort -u /tmp/buf_files.txt > /tmp/buf_files_uniq.txt
echo "Buf-related files found:"
cat /tmp/buf_files_uniq.txt || true
echo
echo "---- Head of first 5 (if any) ----"
n=0
while IFS= read -r f; do
[ -z "$f" ] && continue
echo "### $f"
sed -n '1,120p' "$f" || true
echo
n=$((n+1))
[ $n -ge 5 ] && break
done < /tmp/buf_files_uniq.txtRepository: phuong-labs/axis
Length of output: 121
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Broad search for any files containing "version:" or "modules:" used by buf configs,
# in reasonably small set (by filename patterns first).
rm -f /tmp/buf_candidate_files.txt
touch /tmp/buf_candidate_files.txt
# Common buf config filenames
for pat in 'buf.yaml' 'buf.yml' 'buf.work.yaml' 'buf.work.yml' 'buf.gen.yaml' 'buf.gen.yml' '*.buf.*.yaml' '*.buf.*.yml' '.buf*'; do
fd --full-path --hidden --no-ignore -t f -i "$pat" . >> /tmp/buf_candidate_files.txt 2>/dev/null || true
done
# Also search filenames that literally contain "buf"
fd --full-path --hidden --no-ignore -t f -i '*buf*' . >> /tmp/buf_candidate_files.txt 2>/dev/null || true
# De-dup and filter to YAML-ish/texty small files
sort -u /tmp/buf_candidate_files.txt | rg -i '\.(ya?ml)$|buf' > /tmp/buf_candidate_files_filtered.txt || true
echo "Buf candidate config files (by name):"
head -n 50 /tmp/buf_candidate_files_filtered.txt || true
echo
echo "Search for 'lint:' / 'modules:' / 'version:' in those candidates:"
while IFS= read -r f; do
[ -z "$f" ] && continue
if [ -f "$f" ]; then
if rg -n '(^|\s)(lint:|modules:|version:|breaking:)' "$f" >/dev/null 2>&1; then
echo "MATCH: $f"
rg -n '(^|\s)(lint:|modules:|version:|breaking:)' "$f" | head -n 20
echo "----"
fi
fi
done < /tmp/buf_candidate_files_filtered.txtRepository: phuong-labs/axis
Length of output: 204
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Hard search for any buf config/work files by name (case-insensitive)
find . -maxdepth 8 -type f \( -iname 'buf.yaml' -o -iname 'buf.yml' -o -iname 'buf.work.yaml' -o -iname 'buf.work.yml' -o -iname 'buf.gen.yaml' -o -iname 'buf.gen.yml' \) 2>/dev/null | sort > /tmp/buf_named_files.txt || true
echo "Named buf config/work files:"
cat /tmp/buf_named_files.txt || true
echo
# 2) Broader: any file with 'buf' in the filename and yaml-ish extension
find . -maxdepth 10 -type f -iname '*buf*.y*ml' 2>/dev/null | sort > /tmp/buf_yaml_files.txt || true
echo "Any '*buf*.y*ml' files:"
head -n 50 /tmp/buf_yaml_files.txt || true
echo
# 3) Content search for common buf config keys to catch unconventional filenames
rg -n --hidden --no-ignore '^\s*(version:|lint:|breaking:|modules:|build:|roots:|deps:)\s' -S . > /tmp/buf_config_key_hits.txt || true
echo "First hits for common buf config keys:"
head -n 50 /tmp/buf_config_key_hits.txt || trueRepository: phuong-labs/axis
Length of output: 360
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Full search (no maxdepth) for any buf-related config/work files
find . -type f \( -iname 'buf.yaml' -o -iname 'buf.yml' -o -iname 'buf.work.yaml' -o -iname 'buf.work.yml' -o -iname 'buf.gen.yaml' -o -iname 'buf.gen.yml' \) -print | sort > /tmp/buf_named_files_full.txt || true
echo "Named buf config/work files:"
cat /tmp/buf_named_files_full.txt || true
echo
echo "Any files with 'buf' in name and yaml/yml extension:"
find . -type f -iname '*buf*.ya?ml' -print | sort | head -n 50 || trueRepository: phuong-labs/axis
Length of output: 143
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="src/Modules/Identity/Axis.Identity.Contracts/Protos/identity_service.proto"
echo "---- $file (first 200 lines) ----"
sed -n '1,200p' "$file" | cat -n
echo
echo "---- service + rpc lines ----"
rg -n '^\s*service\s+|^\s*rpc\s+' "$file" || true
echo
echo "---- request/response message definitions (best-effort) ----"
rg -n '^\s*(message|option)\s+' "$file" || trueRepository: phuong-labs/axis
Length of output: 1141
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Run buf lint with debug verbosity and filter for root/module/roots info
# (avoid huge output by grepping key lines only)
if command -v buf >/dev/null 2>&1; then
# some buf versions use --debug, others --verbose; try debug first
(buf lint --debug 2>&1 || true) | rg -n 'root|roots|module|workdir|buf\.yaml|buf\.yml|Protos|PACKAGE_DIRECTORY_MATCH' || true
else
echo "buf CLI not installed locally."
fiRepository: phuong-labs/axis
Length of output: 1343
Align proto package with Buf PACKAGE_DIRECTORY_MATCH
identity_service.proto declares package axis.identity.v1; (line 5) but the file lives under .../Protos/, so Buf PACKAGE_DIRECTORY_MATCH expects it under axis/identity/v1 relative to repo root and fails lint/CI. Move the proto to Protos/axis/identity/v1/identity_service.proto and update the contracts project include path.
🔧 Suggested update in the contracts project file
- <Protobuf Include="Protos\identity_service.proto" GrpcServices="Both" />
+ <Protobuf Include="Protos\axis\identity\v1\identity_service.proto" GrpcServices="Both" />🧰 Tools
🪛 Buf (1.69.0)
[error] 5-5: Files with package "axis.identity.v1" must be within a directory "axis/identity/v1" relative to root but were in directory "src/Modules/Identity/Axis.Identity.Contracts/Protos".
(PACKAGE_DIRECTORY_MATCH)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Modules/Identity/Axis.Identity.Contracts/Protos/identity_service.proto`
at line 5, The proto package declaration in identity_service.proto (package
axis.identity.v1) does not match its directory layout; move
identity_service.proto into Protos/axis/identity/v1/identity_service.proto so
the directory path aligns with the package, and then update the contracts
project include/import path (the contracts project file that references the
Protos folder) to point to the new Protos/axis/identity/v1 location so Buf
PACKAGE_DIRECTORY_MATCH and CI pass.
Co-authored-by: Phuong Nguyen <phuongnse@users.noreply.github.com>
Co-authored-by: Phuong Nguyen <phuongnse@users.noreply.github.com>
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Addresses CodeRabbit review on Phase 2 Identity gRPC pilot: proto path aligned with Buf
PACKAGE_DIRECTORY_MATCH, gRPC endpoint requires JWT +authrate limiting, docs useDeferred (PR #93 follow-up):callouts.Linked spec
Requirements
identity_service.proto→Protos/axis/identity/v1/(packageaxis.identity.v1)MapGrpcService<IdentityGrpcService>()→.RequireAuthorization()+.RequireRateLimiting("auth")Summary by CodeRabbit
New Features
Tests
Documentation