Skip to content

Fix Authorization.__repr__ method#742

Merged
byewokko merged 3 commits intomasterfrom
fix/authorization-repr
Apr 2, 2026
Merged

Fix Authorization.__repr__ method#742
byewokko merged 3 commits intomasterfrom
fix/authorization-repr

Conversation

@byewokko
Copy link
Copy Markdown
Collaborator

@byewokko byewokko commented Apr 2, 2026

Issue

Printing expired Authorization object causes NotAuthenticatedError.

Solution

  • Simplify Authorization.__repr__.
  • Add unit tests for the__repr__ method.

Summary by CodeRabbit

  • Bug Fixes

    • Authorization display now omits raw timestamps and clearly indicates expired or superuser status for a cleaner, more readable representation.
  • Tests

    • Added unit tests covering representation across expired, tenant, global, and superuser authorization scenarios to ensure consistent output.

@byewokko byewokko self-assigned this Apr 2, 2026
@byewokko byewokko added the bug Something isn't working label Apr 2, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 2, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 35652c96-14ec-4180-97c4-f652699bed6b

📥 Commits

Reviewing files that changed from the base of the PR and between 76ebdb0 and 6c71a14.

📒 Files selected for processing (1)
  • asab/web/auth/authorization.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • asab/web/auth/authorization.py

📝 Walkthrough

Walkthrough

Updated Authorization.__repr__() to return shorter representations: it now shows [SUPERUSER] for valid superusers and falls back to [EXPIRED] if authentication fails; timestamp fields (iat/exp) were removed. Unit tests added to cover several authorization scenarios.

Changes

Cohort / File(s) Summary
Authorization Representation Logic
asab/web/auth/authorization.py
Changed Authorization.__repr__() to omit iat/exp timestamps and to conditionally return "<Authorization [SUPERUSER] cid=...>" for valid superusers or "<Authorization [EXPIRED] cid=...>" when a NotAuthenticatedError occurs.
Test Coverage
test/test_auth/test_authorization.py
Added tests asserting Authorization.__repr__() output across expired superuser, tenant-authorized, global/tenant with sub claim, and superuser scenarios; updated test fixtures to include "sub": "asab:test:testuser" where needed.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

Poem

🐰 A rabbit hops with glee,
My repr is short and spry,
[EXPIRED] or [SUPERUSER] I be,
cid shown, no clocks to cry,
Hooray — a tidier sky!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 8.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Fix Authorization.repr method' directly matches the main change in the changeset, which addresses issues with the Authorization.repr method by simplifying it and adding error handling.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/authorization-repr

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 and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@asab/web/auth/authorization.py`:
- Around line 64-70: In __repr__, avoid a TOCTOU by re-checking validity: call
is_valid() once, capture its result and, if valid, capture
has_superuser_access() into a local variable (so has_superuser_access() is not
invoked after the initial validity check) and then build the string using those
locals and self.CredentialsId; ensure any potential NotAuthenticatedError is not
raised during formatting by not calling has_superuser_access() unless the saved
is_valid result is True.
🪄 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

Run ID: 2043aa27-138b-4b18-b823-1b6de37d62b8

📥 Commits

Reviewing files that changed from the base of the PR and between f30d198 and 76ebdb0.

📒 Files selected for processing (2)
  • asab/web/auth/authorization.py
  • test/test_auth/test_authorization.py

@byewokko byewokko merged commit bed858f into master Apr 2, 2026
14 checks passed
@byewokko byewokko deleted the fix/authorization-repr branch April 2, 2026 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant