{@code actorMayMutate} is true for built-in ADMIN and DBA (wired from + * {@code AccessControlService.currentUserMayMutateSql()}). It gates Editor/MCP + * DDL/DML; chat stays {@link MutationMode#READ_ONLY_ONLY}. The same flag is + * consulted by data-access policy during query execution so mutators are not + * blocked by schema redaction meant for read-only roles. + */ public record QueryExecutionContext( QueryExecutionOrigin origin, MutationMode mutationMode, String actorUsername, - boolean actorIsAdmin, + boolean actorMayMutate, boolean mutationConfirmed ) { @@ -27,12 +36,12 @@ public static QueryExecutionContext chat() { ); } - public static QueryExecutionContext editor(String actorUsername, boolean actorIsAdmin, boolean mutationConfirmed) { + public static QueryExecutionContext editor(String actorUsername, boolean actorMayMutate, boolean mutationConfirmed) { return new QueryExecutionContext( QueryExecutionOrigin.EDITOR, - actorIsAdmin ? MutationMode.MAY_MUTATE : MutationMode.READ_ONLY_ONLY, + actorMayMutate ? MutationMode.MAY_MUTATE : MutationMode.READ_ONLY_ONLY, actorUsername, - actorIsAdmin, + actorMayMutate, mutationConfirmed ); } @@ -51,25 +60,25 @@ public static QueryExecutionContext mcp(String actorUsername) { return mcp(actorUsername, false); } - public static QueryExecutionContext mcp(String actorUsername, boolean actorIsAdmin) { - return mcp(actorUsername, actorIsAdmin, false); + public static QueryExecutionContext mcp(String actorUsername, boolean actorMayMutate) { + return mcp(actorUsername, actorMayMutate, false); } /** - * MCP / coding-agent SQL. Developers stay read-only. Admins may run + * MCP / coding-agent SQL. Developers stay read-only. Admins and DBAs may run * non-destructive DDL/DML after the same confirmation gate as the Editor. * DROP and TRUNCATE stay blocked in {@link QueryExecutionPolicyService}. */ public static QueryExecutionContext mcp( String actorUsername, - boolean actorIsAdmin, + boolean actorMayMutate, boolean mutationConfirmed ) { return new QueryExecutionContext( QueryExecutionOrigin.MCP, - actorIsAdmin ? MutationMode.MAY_MUTATE : MutationMode.READ_ONLY_ONLY, + actorMayMutate ? MutationMode.MAY_MUTATE : MutationMode.READ_ONLY_ONLY, actorUsername, - actorIsAdmin, + actorMayMutate, mutationConfirmed ); } @@ -77,13 +86,13 @@ public static QueryExecutionContext mcp( public static QueryExecutionContext forSqlSurface( boolean mcpBearer, String actorUsername, - boolean actorIsAdmin, + boolean actorMayMutate, boolean mutationConfirmed ) { if (mcpBearer) { - return mcp(actorUsername, actorIsAdmin, mutationConfirmed); + return mcp(actorUsername, actorMayMutate, mutationConfirmed); } - return editor(actorUsername, actorIsAdmin, mutationConfirmed); + return editor(actorUsername, actorMayMutate, mutationConfirmed); } public static QueryExecutionContext scheduled() { @@ -100,12 +109,16 @@ public static QueryExecutionContext api(String actorUsername) { return api(actorUsername, false); } - public static QueryExecutionContext api(String actorUsername, boolean actorIsAdmin) { + /** + * API / dashboard SQL is always read-only. The {@code actorMayMutate} flag here + * only influences data-access policy bypass (admins), not mutation mode. + */ + public static QueryExecutionContext api(String actorUsername, boolean actorMayMutate) { return new QueryExecutionContext( QueryExecutionOrigin.API, MutationMode.READ_ONLY_ONLY, actorUsername, - actorIsAdmin, + actorMayMutate, false ); } diff --git a/backend/src/main/java/com/dbaagent/service/QueryExecutionPolicyException.java b/backend/src/main/java/com/dbaagent/service/QueryExecutionPolicyException.java index 7af6835..4a93c22 100644 --- a/backend/src/main/java/com/dbaagent/service/QueryExecutionPolicyException.java +++ b/backend/src/main/java/com/dbaagent/service/QueryExecutionPolicyException.java @@ -51,7 +51,7 @@ public static QueryExecutionPolicyException editorMutationForbidden(String query return new QueryExecutionPolicyException( EDITOR_MUTATION_FORBIDDEN, HttpStatus.FORBIDDEN, - "Only admins can execute DDL or DML from the SQL Editor. This Editor run was blocked before any database changes were attempted.", + "Only admins or DBAs can execute DDL or DML from the SQL Editor. This Editor run was blocked before any database changes were attempted.", false, queryType, List.of() diff --git a/backend/src/main/java/com/dbaagent/service/QueryExecutionPolicyService.java b/backend/src/main/java/com/dbaagent/service/QueryExecutionPolicyService.java index a783969..a3c3ab0 100644 --- a/backend/src/main/java/com/dbaagent/service/QueryExecutionPolicyService.java +++ b/backend/src/main/java/com/dbaagent/service/QueryExecutionPolicyService.java @@ -156,7 +156,7 @@ public PolicyDecision enforce( } StatementClassification mutation = classifications.getFirst(); - if (!effectiveContext.actorIsAdmin()) { + if (!effectiveContext.actorMayMutate()) { throw QueryExecutionPolicyException.editorMutationForbidden(mutation.queryType()); } @@ -164,7 +164,7 @@ public PolicyDecision enforce( && isDropOrTruncateStatement(mutation.queryType(), statements.getFirst())) { throw QueryExecutionPolicyException.unsafeMutation( "DROP and TRUNCATE are blocked on MCP and coding-agent loops. " - + "CREATE, ALTER, and DML still require admin privileges plus confirmation.", + + "CREATE, ALTER, and DML still require admin or DBA privileges plus confirmation.", mutation.queryType() ); } @@ -316,7 +316,7 @@ private StatementClassification classifyStatement(String statement, QueryExecuti // provider's `isReadOnlyQuery` strips only comments, still sees the leading quote, // and answers false. That combination used to fall through as mutating=true, and a // user pasting a SELECT with the double quotes it carried in source code was told - // "Only admins can execute DDL or DML" — a permissions error for a syntax problem. + // "Only admins or DBAs can execute DDL or DML" — a permissions error for a syntax problem. // // It stays blocked: the parser rejected it, so nothing here can vouch for it being // read-only, and this is deliberately reported the same way to admins rather than diff --git a/backend/src/main/java/com/dbaagent/service/UserDataAccessPolicyService.java b/backend/src/main/java/com/dbaagent/service/UserDataAccessPolicyService.java index db5db37..47f6679 100644 --- a/backend/src/main/java/com/dbaagent/service/UserDataAccessPolicyService.java +++ b/backend/src/main/java/com/dbaagent/service/UserDataAccessPolicyService.java @@ -129,7 +129,7 @@ public QueryGuardDecision enforcePreExecution( ConnectionChatAccessPolicyService.EffectivePolicy policy = policyService.resolveEffectivePolicy( connectionId, executionContext.actorUsername(), - executionContext.actorIsAdmin() + executionContext.actorMayMutate() ); if (!policy.protectsAnything()) { return QueryGuardDecision.allow(policy); @@ -339,7 +339,7 @@ public QueryResult redactResult( ConnectionChatAccessPolicyService.EffectivePolicy policy = policyService.resolveEffectivePolicy( connectionId, executionContext.actorUsername(), - executionContext.actorIsAdmin() + executionContext.actorMayMutate() ); if (!policy.protectsAnything() || !policy.redactMode()) { return result; diff --git a/backend/src/main/java/com/dbaagent/service/security/AccessControlService.java b/backend/src/main/java/com/dbaagent/service/security/AccessControlService.java index fc6bcbb..026ea04 100644 --- a/backend/src/main/java/com/dbaagent/service/security/AccessControlService.java +++ b/backend/src/main/java/com/dbaagent/service/security/AccessControlService.java @@ -5,6 +5,7 @@ import com.dbaagent.model.ChatFeedback; import com.dbaagent.model.EffectiveConnectionAccess; import com.dbaagent.model.Permission; +import com.dbaagent.model.Role; import com.dbaagent.repository.AnalysisHistoryRepository; import com.dbaagent.repository.ChatFeedbackRepository; import com.dbaagent.repository.ChatRepository; @@ -297,6 +298,38 @@ public boolean isCurrentUserAdmin() { .anyMatch(authority -> "ROLE_ADMIN".equals(authority.getAuthority())); } + /** + * Whether the current principal may run confirmed DDL/DML on SQL surfaces + * (Editor / MCP). Built-in ADMIN and DBA only — not custom roles, and not + * DEVELOPER / DATA_ENGINEER. Distinct from {@link #isCurrentUserAdmin()}: + * DBA must not receive MANAGE_USERS or other admin-only product controls. + */ + public boolean currentUserMayMutateSql() { + if (ImpersonationContext.isActive()) { + return ImpersonationContext.current() + .map(state -> { + if (state.target() == null) { + return false; + } + Role role = state.target().getRoleEnum(); + return role == Role.ADMIN || role == Role.DBA; + }) + .orElse(false); + } + if (!authEnabled) { + return true; + } + Authentication authentication = currentAuthentication(); + if (authentication == null || !authentication.isAuthenticated()) { + return false; + } + return authentication.getAuthorities().stream() + .anyMatch(authority -> { + String value = authority.getAuthority(); + return "ROLE_ADMIN".equals(value) || "ROLE_DBA".equals(value); + }); + } + private Authentication currentAuthentication() { return SecurityContextHolder.getContext().getAuthentication(); } diff --git a/backend/src/test/java/com/dbaagent/controller/ExplainControllerPolicyTest.java b/backend/src/test/java/com/dbaagent/controller/ExplainControllerPolicyTest.java index 4188dc3..57a623c 100644 --- a/backend/src/test/java/com/dbaagent/controller/ExplainControllerPolicyTest.java +++ b/backend/src/test/java/com/dbaagent/controller/ExplainControllerPolicyTest.java @@ -140,7 +140,7 @@ void useAnalyzeTrue_mcpBearer_usesMcpExecutionContext() { when(httpRequest.getHeader(HttpHeaders.AUTHORIZATION)) .thenReturn("Bearer dsql_mcp_public.secret"); when(accessControlService.getCurrentUsername()).thenReturn("admin"); - when(accessControlService.isCurrentUserAdmin()).thenReturn(true); + when(accessControlService.currentUserMayMutateSql()).thenReturn(true); when(explainPlanService.analyzeQuery(eq("conn-1"), anyString(), eq(true))) .thenReturn(new ExplainPlanAnalysis()); @@ -154,7 +154,7 @@ void useAnalyzeTrue_mcpBearer_usesMcpExecutionContext() { assertThat(captor.getValue().origin()).isEqualTo(QueryExecutionOrigin.MCP); assertThat(captor.getValue().mutationMode()) .isEqualTo(QueryExecutionContext.MutationMode.MAY_MUTATE); - assertThat(captor.getValue().actorIsAdmin()).isTrue(); + assertThat(captor.getValue().actorMayMutate()).isTrue(); } @Test diff --git a/backend/src/test/java/com/dbaagent/service/QueryExecutionContextTest.java b/backend/src/test/java/com/dbaagent/service/QueryExecutionContextTest.java index d471825..6e3f3bc 100644 --- a/backend/src/test/java/com/dbaagent/service/QueryExecutionContextTest.java +++ b/backend/src/test/java/com/dbaagent/service/QueryExecutionContextTest.java @@ -13,20 +13,29 @@ void mcpFactoryProducesReadOnlyContextWithMcpOrigin() { assertThat(ctx.origin()).isEqualTo(QueryExecutionOrigin.MCP); assertThat(ctx.mutationMode()).isEqualTo(QueryExecutionContext.MutationMode.READ_ONLY_ONLY); assertThat(ctx.actorUsername()).isEqualTo("user-1"); - assertThat(ctx.actorIsAdmin()).isFalse(); + assertThat(ctx.actorMayMutate()).isFalse(); assertThat(ctx.mutationConfirmed()).isFalse(); } @Test - void mcpFactoryHonoursAdminFlagFromSecurityContext() { + void mcpFactoryHonoursMayMutateFlagFromSecurityContext() { QueryExecutionContext ctx = QueryExecutionContext.mcp("admin", true); assertThat(ctx.origin()).isEqualTo(QueryExecutionOrigin.MCP); assertThat(ctx.actorUsername()).isEqualTo("admin"); - assertThat(ctx.actorIsAdmin()).isTrue(); + assertThat(ctx.actorMayMutate()).isTrue(); assertThat(ctx.mutationMode()).isEqualTo(QueryExecutionContext.MutationMode.MAY_MUTATE); assertThat(ctx.mutationConfirmed()).isFalse(); } + @Test + void mcpDbaMayMutateWithConfirmation() { + QueryExecutionContext ctx = QueryExecutionContext.mcp("dba", true, true); + assertThat(ctx.origin()).isEqualTo(QueryExecutionOrigin.MCP); + assertThat(ctx.mutationMode()).isEqualTo(QueryExecutionContext.MutationMode.MAY_MUTATE); + assertThat(ctx.actorMayMutate()).isTrue(); + assertThat(ctx.mutationConfirmed()).isTrue(); + } + @Test void mcpAdminConfirmedFactoryPassesConfirmationFlag() { QueryExecutionContext ctx = QueryExecutionContext.mcp("admin", true, true); @@ -36,11 +45,19 @@ void mcpAdminConfirmedFactoryPassesConfirmationFlag() { } @Test - void mcpNonAdminRemainsReadOnlyEvenWhenConfirmed() { + void mcpNonMutatorRemainsReadOnlyEvenWhenConfirmed() { QueryExecutionContext ctx = QueryExecutionContext.mcp("dev", false, true); assertThat(ctx.origin()).isEqualTo(QueryExecutionOrigin.MCP); assertThat(ctx.mutationMode()).isEqualTo(QueryExecutionContext.MutationMode.READ_ONLY_ONLY); - assertThat(ctx.actorIsAdmin()).isFalse(); + assertThat(ctx.actorMayMutate()).isFalse(); + } + + @Test + void editorDbaGetsMayMutateMode() { + QueryExecutionContext ctx = QueryExecutionContext.editor("dba", true, false); + assertThat(ctx.origin()).isEqualTo(QueryExecutionOrigin.EDITOR); + assertThat(ctx.mutationMode()).isEqualTo(QueryExecutionContext.MutationMode.MAY_MUTATE); + assertThat(ctx.actorMayMutate()).isTrue(); } @Test @@ -60,7 +77,7 @@ void scheduledFactoryProducesMayMutateInternalActor() { assertThat(ctx.origin()).isEqualTo(QueryExecutionOrigin.SCHEDULED); assertThat(ctx.mutationMode()).isEqualTo(QueryExecutionContext.MutationMode.MAY_MUTATE); assertThat(ctx.actorUsername()).isNull(); - assertThat(ctx.actorIsAdmin()).isTrue(); + assertThat(ctx.actorMayMutate()).isTrue(); assertThat(ctx.mutationConfirmed()).isTrue(); } diff --git a/backend/src/test/java/com/dbaagent/service/QueryExecutionPolicyServiceTest.java b/backend/src/test/java/com/dbaagent/service/QueryExecutionPolicyServiceTest.java index 6634989..a766569 100644 --- a/backend/src/test/java/com/dbaagent/service/QueryExecutionPolicyServiceTest.java +++ b/backend/src/test/java/com/dbaagent/service/QueryExecutionPolicyServiceTest.java @@ -97,6 +97,48 @@ void editorMutation_adminRequiresConfirmation() { assertThat(exception.getWarnings()).isNotEmpty(); } + @Test + void editorMutation_dbaRequiresConfirmation() { + QueryExecutionPolicyException exception = assertThrows( + QueryExecutionPolicyException.class, + () -> service.enforce( + new QueryRequest("UPDATE customers SET property_status = 'ACTIVE' WHERE customer_id = 9", null, null), + QueryExecutionContext.editor("dba", true, false), + "mysql" + ) + ); + + assertThat(exception.getErrorCode()).isEqualTo(QueryExecutionPolicyException.EDITOR_MUTATION_CONFIRMATION_REQUIRED); + assertThat(exception.isRequiresConfirmation()).isTrue(); + } + + @Test + void editorMutation_dbaConfirmedIsAllowed() { + QueryExecutionPolicyService.PolicyDecision decision = service.enforce( + new QueryRequest("UPDATE customers SET property_status = 'ACTIVE' WHERE customer_id = 9", null, null), + QueryExecutionContext.editor("dba", true, true), + "mysql" + ); + + assertThat(decision.mutating()).isTrue(); + assertThat(decision.primaryQueryType()).isEqualTo("UPDATE"); + } + + @Test + void editorMutation_developerStillForbidden() { + QueryExecutionPolicyException exception = assertThrows( + QueryExecutionPolicyException.class, + () -> service.enforce( + new QueryRequest("DELETE FROM bookings WHERE id = 1", null, null), + QueryExecutionContext.editor("developer", false, true), + "mysql" + ) + ); + + assertThat(exception.getErrorCode()).isEqualTo(QueryExecutionPolicyException.EDITOR_MUTATION_FORBIDDEN); + assertThat(exception.getMessage()).contains("admins or DBAs"); + } + @Test void editorConfirmedDeleteWithoutWhere_isBlocked() { QueryExecutionPolicyException exception = assertThrows( @@ -619,7 +661,7 @@ void mcpAdminExplainDrop_isBlockedEvenWhenConfirmed() { // ── A malformed statement is a syntax error, not a permissions problem ────────── // // Reported from the field: a user pasted a SELECT that still carried the double - // quotes it had in source code and was told "Only admins can execute DDL or DML from + // quotes it had in source code and was told "Only admins or DBAs can execute DDL or DML from // the SQL Editor", which reads as a permissions problem and sent them looking for a // role fix. The statement is neither DDL nor DML — it is not valid SQL at all. // @@ -644,7 +686,7 @@ void selectPastedWithItsSurroundingQuotes_isReportedAsASyntaxErrorNotAPermission ); assertThat(exception.getErrorCode()).isEqualTo(QueryExecutionPolicyException.STATEMENT_NOT_PARSEABLE); - assertThat(exception.getMessage()).doesNotContain("Only admins"); + assertThat(exception.getMessage()).doesNotContain("Only admins or DBAs"); assertThat(exception.getMessage()).contains("could not parse"); } diff --git a/backend/src/test/java/com/dbaagent/service/security/AccessControlServiceTest.java b/backend/src/test/java/com/dbaagent/service/security/AccessControlServiceTest.java index 2c07f30..5551446 100644 --- a/backend/src/test/java/com/dbaagent/service/security/AccessControlServiceTest.java +++ b/backend/src/test/java/com/dbaagent/service/security/AccessControlServiceTest.java @@ -295,6 +295,70 @@ void assignedChatEditorUserCanUseChatEditor() { assertEquals(403, ex.getStatusCode().value()); } + + @Test + void dbaMayMutateSqlButIsNotAdmin() { + SecurityContextHolder.getContext().setAuthentication( + new UsernamePasswordAuthenticationToken( + "dba-user", + null, + List.of(new SimpleGrantedAuthority("ROLE_DBA")) + ) + ); + + assertTrue(accessControlService.currentUserMayMutateSql()); + assertFalse(accessControlService.isCurrentUserAdmin()); + } + + @Test + void adminMayMutateSql() { + SecurityContextHolder.getContext().setAuthentication( + new UsernamePasswordAuthenticationToken( + "admin", + null, + List.of(new SimpleGrantedAuthority("ROLE_ADMIN")) + ) + ); + + assertTrue(accessControlService.currentUserMayMutateSql()); + assertTrue(accessControlService.isCurrentUserAdmin()); + } + + @Test + void developerMayNotMutateSql() { + SecurityContextHolder.getContext().setAuthentication( + new UsernamePasswordAuthenticationToken( + "dev", + null, + List.of(new SimpleGrantedAuthority("ROLE_DEVELOPER")) + ) + ); + + assertFalse(accessControlService.currentUserMayMutateSql()); + assertFalse(accessControlService.isCurrentUserAdmin()); + } + + @Test + void impersonatingDbaAllowsMutateSql() { + com.dbaagent.model.User impersonator = new com.dbaagent.model.User(); + impersonator.setId(1L); + impersonator.setUsername("admin"); + impersonator.setRole("ADMIN"); + com.dbaagent.model.User target = new com.dbaagent.model.User(); + target.setId(2L); + target.setUsername("dba-target"); + target.setRole("DBA"); + com.dbaagent.security.ImpersonationContext.enter( + new com.dbaagent.security.ImpersonationContext.State(impersonator, target) + ); + SecurityContextHolder.getContext().setAuthentication( + new UsernamePasswordAuthenticationToken("dba-target", null, List.of()) + ); + + assertTrue(accessControlService.currentUserMayMutateSql()); + assertFalse(accessControlService.isCurrentUserAdmin()); + } + private ConnectionAccessService.ResolvedConnectionAccess resolved( String connectionId, EffectiveConnectionAccess effectiveAccess, diff --git a/src/components/tabs/Core/SqlRunnerTab.js b/src/components/tabs/Core/SqlRunnerTab.js index 2777d5b..03b7e03 100644 --- a/src/components/tabs/Core/SqlRunnerTab.js +++ b/src/components/tabs/Core/SqlRunnerTab.js @@ -226,7 +226,7 @@ function looksLikeSingleLineDashComment(sql) { } export default function SqlRunnerTab({ connectionId }) { - const { isAdmin, username } = useAuth(); + const { isAdmin, mayMutateSql, username } = useAuth(); const { data: connectionsData } = useConnections(); const currentConnection = (Array.isArray(connectionsData) ? connectionsData : []).find( (c) => c?.id === connectionId @@ -2372,13 +2372,13 @@ export default function SqlRunnerTab({ connectionId }) {