Skip to content

AAP-72278 Fix token visibility#990

Open
bhavenst wants to merge 1 commit intodevelfrom
AAP-72278
Open

AAP-72278 Fix token visibility#990
bhavenst wants to merge 1 commit intodevelfrom
AAP-72278

Conversation

@bhavenst
Copy link
Copy Markdown
Contributor

@bhavenst bhavenst commented Apr 29, 2026

Description

Fixes identified vulnerability. Any user could see all tokens regardless of ownership. Now superuser & admin can do that, org admins can see tokens from their org and randos can see only their own tokens.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Test update
  • Refactoring (no functional changes)
  • Development environment change
  • Configuration change

Self-Review Checklist

  • I have performed a self-review of my code
  • I have added relevant comments to complex code sections
  • I have updated documentation where needed
  • I have considered the security impact of these changes
  • I have thought about error handling and edge cases
  • I have tested the changes in my local environment

Testing Instructions

Prerequisites

Steps to Test

  1. Run GW w/ this DAB PR.
  2. Create a token as a non-superuser
  3. Log in as another non-superuser
  4. See that the first user's token is not visible on the tokens API endpoint (it was before).

Expected Results

Token visibility scoped to owners or users with elevated permissions.

Summary by CodeRabbit

  • New Features

    • Token visibility now filtered by user role and permissions. Superusers see all tokens; auditors have full read access; other users see only owned and authorized organization tokens.
  • Tests

    • Updated RBAC token endpoint tests to validate new access control behavior and per-role token visibility.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 29, 2026

Warning

Rate limit exceeded

@bhavenst has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 56 minutes and 13 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: 5c8942eb-51d6-4ff8-a455-c50021d8b096

📥 Commits

Reviewing files that changed from the base of the PR and between 6fb25c7 and 8a34bad.

📒 Files selected for processing (2)
  • ansible_base/oauth2_provider/views/token.py
  • test_app/tests/oauth2_provider/test_rbac.py
📝 Walkthrough

Walkthrough

OAuth2 token viewset adds customized token retrieval with role-based filtering. Superusers access all tokens; platform auditors access all tokens via safe methods; regular users see only owned tokens, plus tokens from applications in accessible organizations when RBAC is enabled. Tests update expected status codes and add comprehensive visibility assertions.

Changes

Cohort / File(s) Summary
Token Viewset Authorization
ansible_base/oauth2_provider/views/token.py
Added get_queryset() method to OAuth2TokenViewSet that implements role-based token filtering: superusers get full access, platform auditors get unrestricted access for safe methods, and regular users are filtered to owned tokens plus tokens from organizations they manage (when RBAC app is available).
RBAC Test Updates
test_app/tests/oauth2_provider/test_rbac.py
Refactored token read/change/delete test expectations to return 401/404 instead of 403 when access is denied. Added new token-list test verifying per-role token visibility, with superusers seeing all tokens and other roles seeing only owned or organization-associated tokens.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely summarizes the main change: fixing token visibility to scope access based on user roles and permissions.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch AAP-72278

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
Review rate limit: 0/1 reviews remaining, refill in 56 minutes and 13 seconds.

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

@github-actions
Copy link
Copy Markdown

DVCS PR Check Results:

PR appears valid (JIRA key(s) found)

@sonarqubecloud
Copy link
Copy Markdown

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