Skip to content

Commit f7492bd

Browse files
matrivmergify[bot]
matriv
authored and
mergify[bot]
committed
Extract AL privileges check to methods
Simple refactoring to avoid repetition of the same caller code.
1 parent e5fbb34 commit f7492bd

File tree

9 files changed

+74
-202
lines changed

9 files changed

+74
-202
lines changed

‎server/src/main/java/io/crate/auth/AccessControlImpl.java

Lines changed: 31 additions & 141 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,16 @@ private void visitRelation(AnalyzedRelation relation, Role user, Permission perm
278278
relation.accept(relationVisitor, new RelationContext(user, permission));
279279
}
280280

281+
private void hasALPrivileges(Role user) {
282+
Privileges.ensureUserHasPrivilege(
283+
relationVisitor.roles,
284+
user,
285+
Permission.AL,
286+
Securable.CLUSTER,
287+
null
288+
);
289+
}
290+
281291
@Override
282292
protected Void visitAnalyzedStatement(AnalyzedStatement analyzedStatement, Role user) {
283293
throwRequiresSuperUserPermission(user.name());
@@ -306,7 +316,7 @@ public Void visitClose(AnalyzedClose close, Role user) {
306316
@Override
307317
public Void visitSwapTable(AnalyzedSwapTable swapTable, Role user) {
308318
Roles roles = relationVisitor.roles;
309-
if (!roles.hasPrivilege(user, Permission.AL, Securable.CLUSTER, null)) {
319+
if (!roles.hasALPrivileges(user)) {
310320
if (!roles.hasPrivilege(user, Permission.DDL, Securable.TABLE, swapTable.target().ident().fqn())
311321
|| !roles.hasPrivilege(user, Permission.DDL, Securable.TABLE, swapTable.source().ident().fqn())
312322
) {
@@ -318,25 +328,13 @@ public Void visitSwapTable(AnalyzedSwapTable swapTable, Role user) {
318328

319329
@Override
320330
public Void visitGCDanglingArtifacts(AnalyzedGCDanglingArtifacts gcDanglingArtifacts, Role user) {
321-
Privileges.ensureUserHasPrivilege(
322-
relationVisitor.roles,
323-
user,
324-
Permission.AL,
325-
Securable.CLUSTER,
326-
null
327-
);
331+
hasALPrivileges(user);
328332
return null;
329333
}
330334

331335
@Override
332336
public Void visitRerouteRetryFailedStatement(AnalyzedRerouteRetryFailed rerouteRetryFailed, Role user) {
333-
Privileges.ensureUserHasPrivilege(
334-
relationVisitor.roles,
335-
user,
336-
Permission.AL,
337-
Securable.CLUSTER,
338-
null
339-
);
337+
hasALPrivileges(user);
340338
return null;
341339
}
342340

@@ -346,13 +344,7 @@ public Void visitAnalyzedAlterRole(AnalyzedAlterRole analysis, Role user) {
346344
if (analysis.roleName().equals(user.name())) {
347345
return null;
348346
}
349-
Privileges.ensureUserHasPrivilege(
350-
relationVisitor.roles,
351-
user,
352-
Permission.AL,
353-
Securable.CLUSTER,
354-
null
355-
);
347+
hasALPrivileges(user);
356348
return null;
357349
}
358350

@@ -543,13 +535,7 @@ public Void visitAnalyzedAlterTableRenameTable(AnalyzedAlterTableRenameTable ana
543535
@Override
544536
public Void visitSetStatement(AnalyzedSetStatement analysis, Role user) {
545537
if (analysis.scope().equals(SetStatement.Scope.GLOBAL)) {
546-
Privileges.ensureUserHasPrivilege(
547-
relationVisitor.roles,
548-
user,
549-
Permission.AL,
550-
Securable.CLUSTER,
551-
null
552-
);
538+
hasALPrivileges(user);
553539
}
554540
return null;
555541
}
@@ -672,13 +658,7 @@ public Void visitRestoreSnapshotAnalyzedStatement(AnalyzedRestoreSnapshot analys
672658

673659
@Override
674660
public Void visitResetAnalyzedStatement(AnalyzedResetStatement resetAnalyzedStatement, Role user) {
675-
Privileges.ensureUserHasPrivilege(
676-
relationVisitor.roles,
677-
user,
678-
Permission.AL,
679-
Securable.CLUSTER,
680-
null
681-
);
661+
hasALPrivileges(user);
682662
return null;
683663
}
684664

@@ -712,37 +692,19 @@ public Void visitCreateViewStmt(CreateViewStmt createViewStmt, Role user) {
712692

713693
@Override
714694
protected Void visitAnalyzedCreateRole(AnalyzedCreateRole createRole, Role user) {
715-
Privileges.ensureUserHasPrivilege(
716-
relationVisitor.roles,
717-
user,
718-
Permission.AL,
719-
Securable.CLUSTER,
720-
null
721-
);
695+
hasALPrivileges(user);
722696
return null;
723697
}
724698

725699
@Override
726700
protected Void visitDropRole(AnalyzedDropRole dropRole, Role user) {
727-
Privileges.ensureUserHasPrivilege(
728-
relationVisitor.roles,
729-
user,
730-
Permission.AL,
731-
Securable.CLUSTER,
732-
null
733-
);
701+
hasALPrivileges(user);
734702
return null;
735703
}
736704

737705
@Override
738706
public Void visitPrivilegesStatement(AnalyzedPrivileges changePrivileges, Role user) {
739-
Privileges.ensureUserHasPrivilege(
740-
relationVisitor.roles,
741-
user,
742-
Permission.AL,
743-
Securable.CLUSTER,
744-
null
745-
);
707+
hasALPrivileges(user);
746708
for (Privilege privilege : changePrivileges.privileges()) {
747709
if (privilege.policy() == Policy.GRANT) {
748710
Privileges.ensureUserHasPrivilege(
@@ -815,13 +777,7 @@ public Void visitOptimizeTableStatement(AnalyzedOptimizeTable optimizeTable, Rol
815777

816778
@Override
817779
public Void visitCreatePublication(AnalyzedCreatePublication createPublication, Role user) {
818-
Privileges.ensureUserHasPrivilege(
819-
relationVisitor.roles,
820-
user,
821-
Permission.AL,
822-
Securable.CLUSTER,
823-
null
824-
);
780+
hasALPrivileges(user);
825781
// All tables cannot be checked on publication creation - they are checked before actual replication starts
826782
// and a table gets published only if publication owner has DQL, DML and DDL privileges on that table.
827783
for (RelationName relationName: createPublication.tables()) {
@@ -840,25 +796,13 @@ public Void visitCreatePublication(AnalyzedCreatePublication createPublication,
840796

841797
@Override
842798
public Void visitDropPublication(AnalyzedDropPublication dropPublication, Role user) {
843-
Privileges.ensureUserHasPrivilege(
844-
relationVisitor.roles,
845-
user,
846-
Permission.AL,
847-
Securable.CLUSTER,
848-
null
849-
);
799+
hasALPrivileges(user);
850800
return null;
851801
}
852802

853803
@Override
854804
public Void visitAlterPublication(AnalyzedAlterPublication alterPublication, Role user) {
855-
Privileges.ensureUserHasPrivilege(
856-
relationVisitor.roles,
857-
user,
858-
Permission.AL,
859-
Securable.CLUSTER,
860-
null
861-
);
805+
hasALPrivileges(user);
862806
for (RelationName relationName: alterPublication.tables()) {
863807
for (Permission permission : READ_WRITE_DEFINE) {
864808
Privileges.ensureUserHasPrivilege(
@@ -875,109 +819,55 @@ public Void visitAlterPublication(AnalyzedAlterPublication alterPublication, Rol
875819

876820
@Override
877821
public Void visitCreateSubscription(AnalyzedCreateSubscription createSubscription, Role user) {
878-
Privileges.ensureUserHasPrivilege(
879-
relationVisitor.roles,
880-
user,
881-
Permission.AL,
882-
Securable.CLUSTER,
883-
null
884-
);
822+
hasALPrivileges(user);
885823
return null;
886824
}
887825

888826
@Override
889827
public Void visitDropSubscription(AnalyzedDropSubscription dropSubscription, Role user) {
890-
Privileges.ensureUserHasPrivilege(
891-
relationVisitor.roles,
892-
user,
893-
Permission.AL,
894-
Securable.CLUSTER,
895-
null
896-
);
828+
hasALPrivileges(user);
897829
return null;
898830
}
899831

900832
@Override
901833
public Void visitAlterSubscription(AnalyzedAlterSubscription alterSubscription, Role user) {
902-
Privileges.ensureUserHasPrivilege(
903-
relationVisitor.roles,
904-
user,
905-
Permission.AL,
906-
Securable.CLUSTER,
907-
null
908-
);
834+
hasALPrivileges(user);
909835
return null;
910836
}
911837

912838
@Override
913839
public Void visitAnalyze(AnalyzedAnalyze analyzedAnalyze, Role user) {
914-
Privileges.ensureUserHasPrivilege(
915-
relationVisitor.roles,
916-
user,
917-
Permission.AL,
918-
Securable.CLUSTER,
919-
null
920-
);
840+
hasALPrivileges(user);
921841
return null;
922842
}
923843

924844
@Override
925845
public Void visitCreateServer(AnalyzedCreateServer createServer, Role user) {
926-
Privileges.ensureUserHasPrivilege(
927-
relationVisitor.roles,
928-
user,
929-
Permission.AL,
930-
Securable.CLUSTER,
931-
null
932-
);
846+
hasALPrivileges(user);
933847
return null;
934848
}
935849

936850
@Override
937851
public Void visitAlterServer(AnalyzedAlterServer analyzedAlterServer, Role user) {
938-
Privileges.ensureUserHasPrivilege(
939-
relationVisitor.roles,
940-
user,
941-
Permission.AL,
942-
Securable.CLUSTER,
943-
null
944-
);
852+
hasALPrivileges(user);
945853
return null;
946854
}
947855

948856
@Override
949857
public Void visitDropServer(AnalyzedDropServer dropServer, Role user) {
950-
Privileges.ensureUserHasPrivilege(
951-
relationVisitor.roles,
952-
user,
953-
Permission.AL,
954-
Securable.CLUSTER,
955-
null
956-
);
858+
hasALPrivileges(user);
957859
return null;
958860
}
959861

960862
@Override
961863
public Void visitCreateUserMapping(AnalyzedCreateUserMapping createUserMapping, Role user) {
962-
Privileges.ensureUserHasPrivilege(
963-
relationVisitor.roles,
964-
user,
965-
Permission.AL,
966-
Securable.CLUSTER,
967-
null
968-
);
864+
hasALPrivileges(user);
969865
return null;
970866
}
971867

972868
@Override
973869
public Void visitDropUserMapping(AnalyzedDropUserMapping dropUserMapping, Role user) {
974-
Privileges.ensureUserHasPrivilege(
975-
relationVisitor.roles,
976-
user,
977-
Permission.AL,
978-
Securable.CLUSTER,
979-
null
980-
);
870+
hasALPrivileges(user);
981871
return null;
982872
}
983873

‎server/src/main/java/io/crate/execution/engine/collect/sources/InformationSchemaIterables.java

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,6 @@
7979
import io.crate.metadata.table.TableInfo;
8080
import io.crate.metadata.view.ViewInfo;
8181
import io.crate.protocols.postgres.types.PGTypes;
82-
import io.crate.role.Permission;
8382
import io.crate.role.Role;
8483
import io.crate.role.Roles;
8584
import io.crate.role.Securable;
@@ -393,7 +392,7 @@ public Iterable<ForeignTable.Option> foreignTableOptions() {
393392

394393

395394
public Iterable<String> enabledRoles(Role role, Roles roles) {
396-
boolean isAdmin = roles.hasPrivilege(role, Permission.AL, Securable.CLUSTER, null);
395+
boolean isAdmin = roles.hasALPrivileges(role);
397396
return () -> Iterators.concat(
398397
List.of(role.name()).iterator(),
399398
applicableRoles(role, roles, isAdmin)
@@ -404,12 +403,12 @@ public Iterable<String> enabledRoles(Role role, Roles roles) {
404403
}
405404

406405
public Iterable<ApplicableRole> administrableRoleAuthorizations(Role role, Roles roles) {
407-
boolean isAdmin = roles.hasPrivilege(role, Permission.AL, Securable.CLUSTER, null);
406+
boolean isAdmin = roles.hasALPrivileges(role);
408407
return () -> applicableRoles(role, roles, isAdmin).filter(ApplicableRole::isGrantable).iterator();
409408
}
410409

411410
public Iterable<ApplicableRole> applicableRoles(Role role, Roles roles) {
412-
boolean isAdmin = roles.hasPrivilege(role, Permission.AL, Securable.CLUSTER, null);
411+
boolean isAdmin = roles.hasALPrivileges(role);
413412
return () -> applicableRoles(role, roles, isAdmin).iterator();
414413
}
415414

@@ -427,7 +426,7 @@ private Stream<ApplicableRole> applicableRoles(Role role, Roles roles, boolean i
427426
}
428427

429428
public Iterable<RoleTableGrant> roleTableGrants(Role role, Roles roles) {
430-
boolean isAdmin = roles.hasPrivilege(role, Permission.AL, Securable.CLUSTER, null);
429+
boolean isAdmin = roles.hasALPrivileges(role);
431430
return () -> roleTableGrants(role, role, roles, isAdmin).distinct().iterator();
432431
}
433432

0 commit comments

Comments
 (0)