ING-1368: unknown permission errors#322
Merged
Merged
Conversation
3cbd186 to
e8035cd
Compare
There was a problem hiding this comment.
Pull request overview
This PR enhances test coverage by adding permission validation tests across all services and refactors user creation to improve test performance. The key changes include:
- Added comprehensive permission error handling tests for users with no permissions and read-only access across KV, Query, Search, and Management services
- Refactored user creation from per-test to suite-level initialization, creating reusable test users (read-only, no-permissions, etc.)
- Added error handling for authentication failures in search, query admin, collection admin, and bucket admin services
- Fixed error reporting issues in subdoc operations and GetAllReplicas
Reviewed changes
Copilot reviewed 24 out of 25 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| testutils/dinocluster.go | Refactored user management functions to be package-level and accept password parameters |
| testutils/cbcluster.go | Removed unused ReadUser/ReadPass fields from test cluster config |
| go.mod | Updated gocbcorex dependency version |
| gateway/test/suite_test.go | Added suite-level initialization for test users (read-only, no-permissions) and client certificates |
| gateway/test/search_test.go | Added permission validation tests for search operations |
| gateway/test/query_test.go | Added permission tests for query operations with different credential types |
| gateway/test/query_mgmt_test.go | Restructured tests and added permission validation for index management operations |
| gateway/test/mtls_test.go | Refactored mTLS tests to use pre-created users and added client cert configuration tests |
| gateway/test/dapi_proxy_test.go | Added permission tests for proxy endpoints |
| gateway/test/dapi_mtls_test.go | Refactored to use pre-created certificates instead of creating users per test |
| gateway/test/dapi_crud_test.go | Added permission validation to common error test cases |
| gateway/test/crud_test.go | Added permission tests for KV operations with read-only credentials |
| gateway/test/collection_mgmt_test.go | Restructured collection management tests and added permission validation |
| gateway/test/bucket_mgmt_test.go | Added no-permissions test case to common error handling |
| gateway/dataimpl/server_v1/searchserver.go | Added authentication failure error handling for search queries |
| gateway/dataimpl/server_v1/queryadminserver.go | Added authentication failure handling across all query admin operations |
| gateway/dataimpl/server_v1/kvserver.go | Fixed error handling in GetAllReplicas for access errors |
| gateway/dataimpl/server_v1/errorhandler.go | Added new error status methods for scope/collection access denied and cert auth disabled |
| gateway/dataimpl/server_v1/collectionadminserver.go | Added access denied error handling for collection management operations |
| gateway/dataimpl/server_v1/bucketadminserver.go | Added access denied error handling for bucket listing |
| gateway/dataimpl/server_v1/authhandler.go | Added handling for cert auth disabled error |
| gateway/dapiimpl/server_v1/dataapi_subdoc.go | Fixed access error to return write access error instead of read access error |
| gateway/auth/cbauthauthenticator.go | Added ErrCertAuthDisabled error and handling |
| .github/actions/install-cbdinocluster/action.yml | Updated cbdinocluster version from v0.0.96 to v0.0.98 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
20be415 to
119e9e9
Compare
ING-1368: Initialize users and certs in test setup ING-1368: Add auth tests for fts ING-1368: Add no permission case to bucket mgmt tests ING-1368: Refactor and add permissions tests for collection mgmt ING-1368: Add permissions tests to crud ops tests ING-1368: Add permissions tests for data api crud ops ING-1368: Add permission tests for Data API proxy ING-1368: Add credential tests for query management ING-1368: Add cred tests for Query ING-1368: Add accessor funcs for test credentials ING-1368: Handle srv version dependent query behaviour
119e9e9 to
26903a3
Compare
brett19
approved these changes
Dec 8, 2025
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.
This PR adds cases where the user has insufficient permissions to all of our testing and the appropriate error handling where needed in order to make the tests pass. It also changed the way in which users are created. Previously each test would create it's own user as and when they are needed, which slowed the testing down. Now we create a set of users on starting the tests and re-use these among all of the tests.
Finally some refactoring was done to some of the tests: