fix(oauth2): return 412 instead of 500 on duplicate api scope group name

[smarcet]

Summary

PUT /admin/api/v1/api-scope-groups/{id} (and POST) returned 500 when the name was already used by another group. ApiScopeGroupService threw OAuth2\Exceptions\InvalidApiScopeGroup, which ApiScopeGroupController does not catch (it only maps ValidationException -> 412 and EntityNotFoundException -> 404), so it fell into the generic 500 handler.

The service now throws models\exceptions\ValidationException for both name-collision checks (update and create), so the API answers 412 with the message.

The "group id does not exist" case still throws InvalidApiScopeGroup; left untouched to keep this fix scoped.

Tests

tests/unit/ApiScopeGroupServiceTest.php: update and create with an already used name expect ValidationException. The update case failed before the fix with InvalidApiScopeGroup.

docker exec idp-app sh -c 'cd /var/www && vendor/bin/phpunit tests/unit/ApiScopeGroupServiceTest.php'

Summary by CodeRabbit

• Bug Fixes
  • Creating or updating a scope group with a name already in use now returns a validation error that identifies the duplicate name, instead of a specialized scope group error. This gives the same validation feedback for duplicate names in both cases.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: ba269690-f56d-46d4-9bf2-bb0ffdcc15d1

📥 Commits

Reviewing files that changed from the base of the PR and between 3aa9925 and 5ea2de7.

📒 Files selected for processing (2)

• app/Services/OAuth2/ApiScopeGroupService.php
• tests/unit/ApiScopeGroupServiceTest.php

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

Duplicate group-name checks in ApiScopeGroupService now raise ValidationException during create and update. Unit tests cover both cases and verify that the exception contains the duplicate name.

Changes

Scope group validation

Layer / File(s)	Summary
Duplicate-name exceptions and tests app/Services/OAuth2/ApiScopeGroupService.php, tests/unit/ApiScopeGroupServiceTest.php	The create and update duplicate-name checks now raise ValidationException. Tests verify each case and check that the exception contains the duplicate name.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 5ea2d

Duplicate-name requests receive the intended 412 response, with the separate invalid-ID behavior unchanged. No actionable merge risk is established.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 5ea2d

This is a narrow error-contract correction within existing administrator endpoints. Authorization and mutation ordering remain unchanged. The response now confirms a duplicate name to authorized administrators; external consumer compatibility and deployed database guarantees were not verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The evidenced exposure change is confined to duplicate-name failure responses within the existing server-administrator scope-group API. The inspected transition does not grant additional mutation authority or create an unauthenticated entrypoint.

Trust Boundaries and Controls

• inferred — An authorized caller can now confirm that a submitted name already exists through the validation response. This is not retained as a material disclosure concern: the same administrator authorization group already exposes scope-group list and retrieval operations, and the PR leaves that privilege boundary intact.

Resilience and Maintainability Implications

• observed — Name uniqueness still relies on the same application-level check before mutation. The inspected entity mapping does not establish a database unique constraint; deployed schema enforcement and process-termination recovery remain unverified. These limitations predate the PR and do not establish an introduced concurrency or cleanup defect.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files.	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 clearly and concisely describes the main change: duplicate API scope group names now return HTTP 412 instead of HTTP 500.
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.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-164/

This page is automatically updated on each push to this PR.