#194: improving auth user match checking#195
Merged
Merged
Conversation
…cause the failure
lsulak
requested review from
oto-macenauer-absa,
petr-pokorny-absa and
tmikula-dev
as code owners
July 10, 2026 15:28
Contributor
WalkthroughTopic POST authorization now accepts case differences between JWT subjects and configured usernames, while preserving existing response shapes and adding dispatch outcome logs. The repository ignore file was expanded with categorized development and infrastructure artifact patterns. ChangesTopic authorization and dispatch
Repository ignore policy
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant JWTDecoder
participant HandlerTopic
participant AccessConfig
participant Writers
JWTDecoder->>HandlerTopic: provide token subject
HandlerTopic->>AccessConfig: resolve configured username case-insensitively
AccessConfig-->>HandlerTopic: return configured username
HandlerTopic->>Writers: dispatch topic message
Writers-->>HandlerTopic: report write results
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
tmikula-dev
approved these changes
Jul 20, 2026
tmikula-dev
left a comment
tmikula-dev
left a comment
Collaborator
There was a problem hiding this comment.
Glad, that I learned the difference between .lower() and .casefold()
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tests/unit/handlers/test_handler_topic.py`:
- Around line 260-277: Update test_post_authorized_user_case_insensitive to use
the mocker fixture with mocker.patch.object for decode_jwt and each writer.write
instead of a patch context manager; rewrite assertions consistently as expected
== actual, including the success assertion, while preserving the test behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 880963d3-e614-4126-abf9-60a6beb0c8df
📒 Files selected for processing (3)
.gitignoresrc/handlers/handler_topic.pytests/unit/handlers/test_handler_topic.py
Comment on lines
+260
to
+277
| def test_post_authorized_user_case_insensitive(event_gate_module, make_event, valid_payload): | ||
| with patch.object(event_gate_module.handler_token, "decode_jwt", return_value={"sub": "testuser"}): | ||
| for writer in event_gate_module.handler_topic.writers.values(): | ||
| writer.write = MagicMock(return_value=None) | ||
|
|
||
| event = make_event( | ||
| "/topics/{topic_name}", | ||
| method="POST", | ||
| topic="public.cps.za.test", | ||
| body=valid_payload, | ||
| headers={"Authorization": "Bearer token"}, | ||
| ) | ||
| resp = event_gate_module.lambda_handler(event) | ||
| assert 202 == resp["statusCode"] | ||
| body = json.loads(resp["body"]) | ||
| assert body["success"] | ||
|
|
||
|
|
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Use mocker.patch.object and follow the assertion format.
As per coding guidelines, use the mocker fixture instead of unittest.mock.patch context managers, and write assertions strictly in the expected == actual format. Additionally, using mocker.patch.object on the writers ensures that the mocks are safely cleaned up after the test completes, preventing potential side effects on other tests.
♻️ Proposed refactor
-def test_post_authorized_user_case_insensitive(event_gate_module, make_event, valid_payload):
- with patch.object(event_gate_module.handler_token, "decode_jwt", return_value={"sub": "testuser"}):
- for writer in event_gate_module.handler_topic.writers.values():
- writer.write = MagicMock(return_value=None)
-
- event = make_event(
- "/topics/{topic_name}",
- method="POST",
- topic="public.cps.za.test",
- body=valid_payload,
- headers={"Authorization": "Bearer token"},
- )
- resp = event_gate_module.lambda_handler(event)
- assert 202 == resp["statusCode"]
- body = json.loads(resp["body"])
- assert body["success"]
+def test_post_authorized_user_case_insensitive(event_gate_module, make_event, valid_payload, mocker):
+ mocker.patch.object(event_gate_module.handler_token, "decode_jwt", return_value={"sub": "testuser"})
+ for writer in event_gate_module.handler_topic.writers.values():
+ mocker.patch.object(writer, "write", return_value=None)
+
+ event = make_event(
+ "/topics/{topic_name}",
+ method="POST",
+ topic="public.cps.za.test",
+ body=valid_payload,
+ headers={"Authorization": "Bearer token"},
+ )
+ resp = event_gate_module.lambda_handler(event)
+ assert 202 == resp["statusCode"]
+ body = json.loads(resp["body"])
+ assert True == body["success"]📝 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.
Suggested change
| def test_post_authorized_user_case_insensitive(event_gate_module, make_event, valid_payload): | |
| with patch.object(event_gate_module.handler_token, "decode_jwt", return_value={"sub": "testuser"}): | |
| for writer in event_gate_module.handler_topic.writers.values(): | |
| writer.write = MagicMock(return_value=None) | |
| event = make_event( | |
| "/topics/{topic_name}", | |
| method="POST", | |
| topic="public.cps.za.test", | |
| body=valid_payload, | |
| headers={"Authorization": "Bearer token"}, | |
| ) | |
| resp = event_gate_module.lambda_handler(event) | |
| assert 202 == resp["statusCode"] | |
| body = json.loads(resp["body"]) | |
| assert body["success"] | |
| def test_post_authorized_user_case_insensitive(event_gate_module, make_event, valid_payload, mocker): | |
| mocker.patch.object(event_gate_module.handler_token, "decode_jwt", return_value={"sub": "testuser"}) | |
| for writer in event_gate_module.handler_topic.writers.values(): | |
| mocker.patch.object(writer, "write", return_value=None) | |
| event = make_event( | |
| "/topics/{topic_name}", | |
| method="POST", | |
| topic="public.cps.za.test", | |
| body=valid_payload, | |
| headers={"Authorization": "Bearer token"}, | |
| ) | |
| resp = event_gate_module.lambda_handler(event) | |
| assert 202 == resp["statusCode"] | |
| body = json.loads(resp["body"]) | |
| assert True == body["success"] |
🤖 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 `@tests/unit/handlers/test_handler_topic.py` around lines 260 - 277, Update
test_post_authorized_user_case_insensitive to use the mocker fixture with
mocker.patch.object for decode_jwt and each writer.write instead of a patch
context manager; rewrite assertions consistently as expected == actual,
including the success assertion, while preserving the test behavior.
Source: Coding guidelines
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
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.
Release Notes
Closes: #194
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores