[server] Add authorization to databaseExists and tableExists RPC call…#3078
[server] Add authorization to databaseExists and tableExists RPC call…#3078vaibhavk1992 wants to merge 1 commit intoapache:mainfrom
Conversation
apache#2007) Currently, the databaseExists and tableExists RPC methods do not enforce authorization, allowing any authenticated user to check if databases or tables exist, which poses a security risk. This commit adds authorization checks to both methods: - databaseExists: Checks DESCRIBE permission on the database - tableExists: Checks DESCRIBE permission on the table For security, both methods return false for unauthorized access, preventing information disclosure. Unauthorized users cannot distinguish between "resource doesn't exist" and "no permission", which prevents reconnaissance attacks to map database structure. Changes: - Modified RpcServiceBase.databaseExists() to catch authorization exceptions - Modified RpcServiceBase.tableExists() to catch authorization exceptions - Added testDatabaseExistsAuthorization() integration test - Added testTableExistsAuthorization() integration test Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
There was a problem hiding this comment.
Pull request overview
Adds authorization enforcement to “existence check” RPCs to prevent unauthorized users from probing database/table names via databaseExists and tableExists.
Changes:
- Add DESCRIBE authorization checks to
RpcServiceBase.databaseExists()andRpcServiceBase.tableExists(), returningfalsewhen unauthorized to avoid information disclosure. - Add integration tests validating the new “unauthorized => false” behavior for both APIs.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| fluss-server/src/main/java/org/apache/fluss/server/RpcServiceBase.java | Enforces DESCRIBE authorization for database/table existence checks and hides existence when unauthorized. |
| fluss-client/src/test/java/org/apache/fluss/client/security/acl/FlussAuthorizationITCase.java | Adds IT coverage for unauthorized vs authorized outcomes of databaseExists/tableExists. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (exists) { | ||
| try { | ||
| authorizeTable(OperationType.DESCRIBE, tablePath); | ||
| // If we get here, user is authorized | ||
| } catch (Exception e) { | ||
| // Not authorized - hide table existence | ||
| LOG.debug( | ||
| "User {} not authorized to access table '{}', returning false", | ||
| currentSession().getPrincipal(), | ||
| tablePath); | ||
| exists = false; | ||
| } |
There was a problem hiding this comment.
Same issue as databaseExists: catching Exception will hide unexpected errors and authorizer.authorize(...) will emit WARN logs for normal unauthorized probes. Prefer a non-throwing authorization check (authorizer.isAuthorized(currentSession(), DESCRIBE, Resource.table(tablePath))) and only treat explicit authorization denial as exists=false.
| if (exists) { | |
| try { | |
| authorizeTable(OperationType.DESCRIBE, tablePath); | |
| // If we get here, user is authorized | |
| } catch (Exception e) { | |
| // Not authorized - hide table existence | |
| LOG.debug( | |
| "User {} not authorized to access table '{}', returning false", | |
| currentSession().getPrincipal(), | |
| tablePath); | |
| exists = false; | |
| } | |
| if (exists | |
| && !authorizer.isAuthorized( | |
| currentSession(), OperationType.DESCRIBE, Resource.table(tablePath))) { | |
| LOG.debug( | |
| "User {} not authorized to access table '{}', returning false", | |
| currentSession().getPrincipal(), | |
| tablePath); | |
| exists = false; |
| // For security, return false for both non-existent and unauthorized databases | ||
| if (exists) { | ||
| try { | ||
| authorizeDatabase(OperationType.DESCRIBE, databaseName); |
There was a problem hiding this comment.
I suggest to check the authorization first (which is more efficient than check metadata first). Additionally, please use authorizer.isAuthorized instead of wrapping authorize() in a try-catch block for cleaner and more performant code.
Reasons for improvement:
- Performance Optimization: Checking authorization before fetching metadata avoids unnecessary I/O or computation costs if the user lacks permissions. This fail-fast approach reduces resource waste.
- Using
isAuthorizedprovides a explicit boolean check, which is more readable and idiomatic than using exception handling (try-catch) for control flow. Avoiding exception try-catch is also more efficent in CPU-wise.
| if (exists) { | ||
| try { | ||
| authorizeTable(OperationType.DESCRIBE, tablePath); | ||
| // If we get here, user is authorized | ||
| } catch (Exception e) { | ||
| // Not authorized - hide table existence | ||
| LOG.debug( | ||
| "User {} not authorized to access table '{}', returning false", | ||
| currentSession().getPrincipal(), | ||
| tablePath); | ||
| exists = false; | ||
| } |
Currently, the databaseExists and tableExists RPC methods do not enforce authorization, allowing any authenticated user to check if databases or tables exist, which poses a security risk.
This commit adds authorization checks to both methods:
For security, both methods return false for unauthorized access, preventing information disclosure. Unauthorized users cannot distinguish between "resource doesn't exist" and "no permission", which prevents reconnaissance attacks to map database structure.
Changes: