Skip to content

Add integration tests for invalid_scope error handling - #4874

Merged
sahandilshan merged 1 commit into
thunder-id:mainfrom
sahandilshan:test/invalid-scope-integration
Aug 13, 2026
Merged

Add integration tests for invalid_scope error handling#4874
sahandilshan merged 1 commit into
thunder-id:mainfrom
sahandilshan:test/invalid-scope-integration

Conversation

@sahandilshan

@sahandilshan sahandilshan commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add 15 integration tests covering invalid_scope error handling at /oauth2/token and scope silent-drop behavior at /oauth2/authorize
  • Add RefreshAccessTokenRaw test helper that returns raw HTTP status + body (enabling assertion on error JSON for non-200 responses) with optional scope parameter

Test groups

Group Tests What it covers
A: Refresh token invalid_scope 6 (3 negative, 3 positive) Scope escalation beyond original grant returns error=invalid_scope (RFC 6749 §6); subset/same/omitted scope succeeds
B: Token exchange invalid_scope 3 (1 negative, 2 positive) Requesting scopes with a scopeless subject token returns error=invalid_scope; scoped subject with excess scopes silently filters
C: Client credentials silent drop 3 Scopes undefined on the resource server or unauthorized via RBAC are silently removed (HTTP 200, not error)
D: Authorize endpoint silent drop 3 Same silent-drop behavior verified through the full authorization code flow

Coverage verification

All target code paths confirmed hit via coverage-instrumented build:

  • refresh_token.go:454-459 (invalid_scope error return)
  • token_exchange.go:476-482 (scopeless subject escalation guard)
  • resourceindicators.go:241-244 (DownscopeToResourceServer filter)
  • client_credentials.go:177-184 (filterAuthorizedScopes)

Test plan

  • go vet ./oauth/token/... && go vet ./testutils/... passes
  • All 15 tests pass: make test_integration PACKAGE=./oauth/token/... RUN=TestInvalidScopeTestSuite
  • Coverage-instrumented run confirms target code paths are exercised

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Tests
    • Added OAuth integration coverage for invalid scopes across refresh-token, token-exchange, client-credentials, and authorization requests.
    • Verified that invalid scope requests return appropriate errors.
    • Added checks for successful token responses and detailed failure responses.
    • Added support for testing refresh-token requests with optional scope parameters and client authentication.

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 422bb903-a038-40f5-88a4-3266b762e2a5

📥 Commits

Reviewing files that changed from the base of the PR and between 5a86fa5 and 2590381.

📒 Files selected for processing (1)
  • tests/integration/oauth/token/invalid_scope_test.go

📝 Walkthrough

Walkthrough

Added an OAuth integration suite that provisions test resources and validates invalid-scope behavior for refresh tokens, token exchange, client credentials, and authorization flows. Added a raw refresh-token utility that returns status and response data for successful and failed requests.

Changes

OAuth invalid-scope validation

Layer / File(s) Summary
Provision OAuth test environment
tests/integration/oauth/token/invalid_scope_test.go
Defines OAuth test configuration, creates required resources, and removes them during teardown.
Implement token flow helpers
tests/integration/oauth/token/invalid_scope_test.go, tests/integration/testutils/oauth2_utils.go
Adds authorization, authentication, token-exchange, client-credentials, and raw refresh-token request helpers.
Validate scope behavior
tests/integration/oauth/token/invalid_scope_test.go
Tests scope rejection, narrowing, silent removal, and issued JWT scope claims across OAuth flows.

Estimated code review effort: 4 (Complex) | ~45 minutes

Mergeability Score: ⚪ Minimal · up to 25903

The PR adds integration coverage and a raw test helper without changing production behavior; no actionable merge-blocking risk remains, so it is merge-ready after normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant InvalidScopeTestSuite
  participant AuthorizationEndpoint
  participant TokenEndpoint
  InvalidScopeTestSuite->>AuthorizationEndpoint: Request authorization code or direct authentication
  AuthorizationEndpoint-->>InvalidScopeTestSuite: Return authorization response and code
  InvalidScopeTestSuite->>TokenEndpoint: Submit refresh, exchange, or client-credentials request
  TokenEndpoint-->>InvalidScopeTestSuite: Return token status, body, or decoded token
Loading

Possibly related PRs

Suggested reviewers: thamindudilshan, thiva-k

🚥 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
Title check ✅ Passed The title clearly and concisely describes the main change: integration tests for invalid_scope error handling.
Description check ✅ Passed The description explains the purpose, implementation approach, test coverage, and validation results, but omits several template headings and checklist items.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 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/integration/oauth/token/invalid_scope_test.go`:
- Around line 497-533: Update the successful refresh-token tests to assert the
exact granted scope set: in TestRefreshToken_SubsetScope_Succeeds require only
“read”, in TestRefreshToken_SameScopes_Succeeds require “openid read write”, and
in TestRefreshToken_NoScopeParam_GrantsOriginalScopes require the original
“openid read write” scopes. Replace the current partial or absent scope checks
with complete equality assertions.

In `@tests/integration/testutils/oauth2_utils.go`:
- Line 759: Update refreshAccessToken to call req.SetBasicAuth only when
clientID is non-empty. Preserve the existing request behavior while ensuring no
Authorization header is sent for an empty client ID.
🪄 Autofix

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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 59606867-5270-4ee3-90bb-2a5749b06fe8

📥 Commits

Reviewing files that changed from the base of the PR and between a8887d6 and 9700902.

📒 Files selected for processing (2)
  • tests/integration/oauth/token/invalid_scope_test.go
  • tests/integration/testutils/oauth2_utils.go

Comment thread tests/integration/oauth/token/invalid_scope_test.go
Comment thread tests/integration/testutils/oauth2_utils.go Outdated
@sahandilshan
sahandilshan force-pushed the test/invalid-scope-integration branch from 9700902 to 5a86fa5 Compare August 12, 2026 08:43

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/integration/oauth/token/invalid_scope_test.go`:
- Around line 587-591: Update all listed silent-drop assertions in
tests/integration/oauth/token/invalid_scope_test.go:587-591, 602-605, 623-626,
639-643, 652-656, and 665-669 to use ElementsMatch with the specified exact
scope sets; at 613-616, always extract scopeStr and assert
strings.Fields(scopeStr) is empty. If responses legitimately include extra
default scopes, retain membership assertions and document those expected extras
in a comment.
🪄 Autofix

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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 34e55d65-49eb-47ed-8a77-889c37bf5cb4

📥 Commits

Reviewing files that changed from the base of the PR and between 9700902 and 5a86fa5.

📒 Files selected for processing (2)
  • tests/integration/oauth/token/invalid_scope_test.go
  • tests/integration/testutils/oauth2_utils.go

Comment thread tests/integration/oauth/token/invalid_scope_test.go Outdated
@sahandilshan sahandilshan added the skip-changelog Skip generating changelog for a particular PR label Aug 13, 2026
Cover all invalid_scope code paths that previously had zero integration
test coverage: refresh token scope escalation (RFC 6749 §6) and token
exchange with scopeless subject tokens. Also add tests verifying the
silent scope-drop behavior for client_credentials and authorize
endpoints when scopes are undefined on the resource server or
unauthorized via RBAC.

Signed-off-by: Sahan Dilshan <sahandilshan222@gmail.com>
@sahandilshan
sahandilshan force-pushed the test/invalid-scope-integration branch from 5a86fa5 to 2590381 Compare August 13, 2026 10:05
@sahandilshan sahandilshan added the trigger-pr-builder Add when the PR is ready for CI; starts the PR Builder for this and all later pushes label Aug 13, 2026
@codecov

codecov Bot commented Aug 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@sahandilshan
sahandilshan added this pull request to the merge queue Aug 13, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 13, 2026
@sahandilshan
sahandilshan added this pull request to the merge queue Aug 13, 2026
Merged via the queue into thunder-id:main with commit 23b5fd0 Aug 13, 2026
47 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changelog Skip generating changelog for a particular PR trigger-pr-builder Add when the PR is ready for CI; starts the PR Builder for this and all later pushes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants