From 23796c56f93b187353c271484b345b54627cfd5b Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Wed, 7 Sep 2022 12:25:12 +0200 Subject: [PATCH] cleanup+categorization of TODOs --- gradle.properties | 3 ++- sql/24-hs-domain.sql | 3 --- sql/25-hs-emailaddress.sql | 2 -- .../net/hostsharing/hsadminng/context/Context.java | 4 ++-- .../hs/admin/contact/HsAdminContactRepository.java | 2 +- .../hs/admin/partner/HsAdminPartnerController.java | 12 ++++++------ src/main/resources/db/changelog/050-rbac-base.sql | 6 +++--- .../resources/db/changelog/051-rbac-user-grant.sql | 4 ++-- src/main/resources/db/changelog/080-rbac-global.sql | 2 +- .../db/changelog/113-test-customer-rbac.sql | 2 +- .../resources/db/changelog/133-test-domain-rbac.sql | 2 +- .../resources/db/changelog/200-hs-admin-contact.sql | 4 ++-- .../db/changelog/203-hs-admin-contact-rbac.sql | 2 +- .../HsAdminPartnerControllerAcceptanceTest.java | 8 ++++---- 14 files changed, 26 insertions(+), 30 deletions(-) diff --git a/gradle.properties b/gradle.properties index 852baf23..4ffa0b4c 100644 --- a/gradle.properties +++ b/gradle.properties @@ -3,7 +3,8 @@ postgresql.version = 42.4.1 snakeyaml.version = 1.31 -# TODO: can be removed if all dependencies are JDK 16 compliant +# TODO: can be removed if all dependencies are JDK 16 compliant, check with `gw clean check` +# and check output for "cannot access class ... because module jdk.compiler does not export ..." org.gradle.jvmargs= \ --add-exports jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED \ --add-exports jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED \ diff --git a/sql/24-hs-domain.sql b/sql/24-hs-domain.sql index 07f41be2..d40ec6ef 100644 --- a/sql/24-hs-domain.sql +++ b/sql/24-hs-domain.sql @@ -92,9 +92,6 @@ create trigger createRbacRulesForDomain_Trigger for each row execute procedure createRbacRulesForDomain(); --- TODO: CREATE OR REPLACE FUNCTION deleteRbacRulesForDomain() - - -- create RBAC-restricted view set session session authorization default; -- ALTER TABLE Domain ENABLE ROW LEVEL SECURITY; diff --git a/sql/25-hs-emailaddress.sql b/sql/25-hs-emailaddress.sql index 258d9f0a..0beedce9 100644 --- a/sql/25-hs-emailaddress.sql +++ b/sql/25-hs-emailaddress.sql @@ -77,8 +77,6 @@ create trigger createRbacRulesForEMailAddress_Trigger for each row execute procedure createRbacRulesForEMailAddress(); --- TODO: CREATE OR REPLACE FUNCTION deleteRbacRulesForEMailAddress() - -- create RBAC-restricted view set session session authorization default; diff --git a/src/main/java/net/hostsharing/hsadminng/context/Context.java b/src/main/java/net/hostsharing/hsadminng/context/Context.java index 40ffada5..79f1f566 100644 --- a/src/main/java/net/hostsharing/hsadminng/context/Context.java +++ b/src/main/java/net/hostsharing/hsadminng/context/Context.java @@ -63,7 +63,7 @@ public class Context { cast(:assumedRoles as varchar)); """); query.setParameter("currentTask", shortenToMaxLength(currentTask, 96)); - query.setParameter("currentRequest", shortenToMaxLength(currentRequest, 512)); // TODO.SPEC: length? + query.setParameter("currentRequest", shortenToMaxLength(currentRequest, 512)); // TODO.spec: length? query.setParameter("currentUser", currentUser); query.setParameter("assumedRoles", assumedRoles != null ? assumedRoles : ""); query.executeUpdate(); @@ -91,7 +91,7 @@ public class Context { public UUID[] currentSubjectsUuids() { return (UUID[]) em.createNativeQuery("select currentSubjectsUuids() as uuids") .unwrap(org.hibernate.query.NativeQuery.class) - .addScalar("uuids", UUIDArrayType.INSTANCE) // TODO.BLOG + .addScalar("uuids", UUIDArrayType.INSTANCE) // TODO.blog .getSingleResult(); } diff --git a/src/main/java/net/hostsharing/hsadminng/hs/admin/contact/HsAdminContactRepository.java b/src/main/java/net/hostsharing/hsadminng/hs/admin/contact/HsAdminContactRepository.java index f6569966..729b2207 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/admin/contact/HsAdminContactRepository.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/admin/contact/HsAdminContactRepository.java @@ -16,7 +16,7 @@ public interface HsAdminContactRepository extends Repository findContactByOptionalLabelLike(String label); HsAdminContactEntity save(final HsAdminContactEntity entity); diff --git a/src/main/java/net/hostsharing/hsadminng/hs/admin/partner/HsAdminPartnerController.java b/src/main/java/net/hostsharing/hsadminng/hs/admin/partner/HsAdminPartnerController.java index 082f7cce..53693d27 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/admin/partner/HsAdminPartnerController.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/admin/partner/HsAdminPartnerController.java @@ -37,9 +37,9 @@ public class HsAdminPartnerController implements HsAdminPartnersApi { final String currentUser, final String assumedRoles, final String name) { - // TODO: context.define(currentUser, assumedRoles); + // TODO.feat: context.define(currentUser, assumedRoles); - // TODO: final var entities = partnerRepo.findPartnerByOptionalNameLike(name); + // TODO.feat: final var entities = partnerRepo.findPartnerByOptionalNameLike(name); final var entities = List.of( HsAdminPartnerEntity.builder() @@ -83,13 +83,13 @@ public class HsAdminPartnerController implements HsAdminPartnersApi { final String assumedRoles, final HsAdminPartnerResource body) { - // TODO: context.define(currentUser, assumedRoles); + // TODO.feat: context.define(currentUser, assumedRoles); if (body.getUuid() == null) { body.setUuid(UUID.randomUUID()); } - // TODO: final var saved = partnerRepo.save(map(body, HsAdminPartnerEntity.class)); + // TODO.feat: final var saved = partnerRepo.save(map(body, HsAdminPartnerEntity.class)); final var saved = map(body, HsAdminPartnerEntity.class, PARTNER_RESOURCE_TO_ENTITY_POSTMAPPER); final var uri = @@ -108,9 +108,9 @@ public class HsAdminPartnerController implements HsAdminPartnersApi { final String assumedRoles, final UUID partnerUuid) { - // TODO: context.define(currentUser, assumedRoles); + // TODO.feat: context.define(currentUser, assumedRoles); - // TODO: final var result = partnerRepo.findByUuid(partnerUuid); + // TODO.feat: final var result = partnerRepo.findByUuid(partnerUuid); final var result = partnerUuid.equals(UUID.fromString("3fa85f64-5717-4562-b3fc-2c963f66afa6")) ? null : HsAdminPartnerEntity.builder() diff --git a/src/main/resources/db/changelog/050-rbac-base.sql b/src/main/resources/db/changelog/050-rbac-base.sql index 48dcdb4b..4c77f73c 100644 --- a/src/main/resources/db/changelog/050-rbac-base.sql +++ b/src/main/resources/db/changelog/050-rbac-base.sql @@ -170,7 +170,7 @@ call create_journal('RbacRole'); create type RbacRoleDescriptor as ( - objectTable varchar(63), -- TODO: needed? remove? + objectTable varchar(63), -- for human readability and easier debugging objectUuid uuid, roleType RbacRoleType ); @@ -221,7 +221,7 @@ declare objectUuidOfRole uuid; roleUuid uuid; begin - -- TODO: extract function toRbacRoleDescriptor(roleIdName varchar) + find other occurrences + -- TODO.refact: extract function toRbacRoleDescriptor(roleIdName varchar) + find other occurrences roleParts = overlay(roleIdName placing '#' from length(roleIdName) + 1 - strpos(reverse(roleIdName), '.')); objectTableFromRoleIdName = split_part(roleParts, '#', 1); objectNameFromRoleIdName = split_part(roleParts, '#', 2); @@ -415,7 +415,7 @@ create or replace function isGranted(granteeIds uuid[], grantedId uuid) declare granteeId uuid; begin - -- TODO: needs optimization + -- TODO.perf: needs optimization foreach granteeId in array granteeIds loop if isGranted(granteeId, grantedId) then diff --git a/src/main/resources/db/changelog/051-rbac-user-grant.sql b/src/main/resources/db/changelog/051-rbac-user-grant.sql index d36c9f24..81cadc94 100644 --- a/src/main/resources/db/changelog/051-rbac-user-grant.sql +++ b/src/main/resources/db/changelog/051-rbac-user-grant.sql @@ -30,7 +30,7 @@ begin insert into RbacGrants (grantedByRoleUuid, ascendantUuid, descendantUuid, assumed) values (grantedByRoleUuid, userUuid, roleUuid, doAssume); - -- TODO: What should happen on mupltiple grants? What if options are not the same? + -- TODO.spec: What should happen on mupltiple grants? What if options (doAssume) are not the same? -- Most powerful or latest grant wins? What about managed? -- on conflict do nothing; -- allow granting multiple times end; $$; @@ -53,7 +53,7 @@ begin insert into RbacGrants (grantedByRoleUuid, ascendantUuid, descendantUuid, assumed) values (grantedByRoleUuid, userUuid, grantedRoleUuid, doAssume); - -- TODO: What should happen on mupltiple grants? What if options are not the same? + -- TODO.spec: What should happen on mupltiple grants? What if options (doAssume) are not the same? -- Most powerful or latest grant wins? What about managed? -- on conflict do nothing; -- allow granting multiple times end; $$; diff --git a/src/main/resources/db/changelog/080-rbac-global.sql b/src/main/resources/db/changelog/080-rbac-global.sql index 531bf85c..7d8bad78 100644 --- a/src/main/resources/db/changelog/080-rbac-global.sql +++ b/src/main/resources/db/changelog/080-rbac-global.sql @@ -30,7 +30,7 @@ create or replace function hasGlobalPermission(op RbacOp) returns boolean language sql as $$ - -- TODO: this could to be optimized + -- TODO.perf: this could to be optimized select (select uuid from global) in (select queryAccessibleObjectUuidsOfSubjectIds(op, 'global', currentSubjectsUuids())); $$; diff --git a/src/main/resources/db/changelog/113-test-customer-rbac.sql b/src/main/resources/db/changelog/113-test-customer-rbac.sql index 767506fc..b5600726 100644 --- a/src/main/resources/db/changelog/113-test-customer-rbac.sql +++ b/src/main/resources/db/changelog/113-test-customer-rbac.sql @@ -152,7 +152,7 @@ drop view if exists test_customer_iv; create or replace view test_customer_iv as select target.uuid, target.prefix as idName from test_customer as target; --- TODO: Is it ok that everybody has access to this information? +-- TODO.spec: Is it ok that everybody has access to this information? grant all privileges on test_customer_iv to restricted; /* diff --git a/src/main/resources/db/changelog/133-test-domain-rbac.sql b/src/main/resources/db/changelog/133-test-domain-rbac.sql index 0e562f6e..ae526528 100644 --- a/src/main/resources/db/changelog/133-test-domain-rbac.sql +++ b/src/main/resources/db/changelog/133-test-domain-rbac.sql @@ -168,7 +168,7 @@ drop view if exists test_domain_iv; create or replace view test_domain_iv as select distinct target.uuid, target.name as idName from test_domain as target; --- TODO: Is it ok that everybody has access to this information? +-- TODO.spec: Is it ok that everybody has access to this information? grant all privileges on test_domain_iv to restricted; /* diff --git a/src/main/resources/db/changelog/200-hs-admin-contact.sql b/src/main/resources/db/changelog/200-hs-admin-contact.sql index 4e2c89bd..9fb3d29c 100644 --- a/src/main/resources/db/changelog/200-hs-admin-contact.sql +++ b/src/main/resources/db/changelog/200-hs-admin-contact.sql @@ -9,7 +9,7 @@ create table if not exists hs_admin_contact uuid uuid unique references RbacObject (uuid), label varchar(96) not null, postalAddress text, - emailAddresses text, -- TODO: change to json - phoneNumbers text -- TODO: change to json + emailAddresses text, -- TODO.feat: change to json + phoneNumbers text -- TODO.feat: change to json ); --// diff --git a/src/main/resources/db/changelog/203-hs-admin-contact-rbac.sql b/src/main/resources/db/changelog/203-hs-admin-contact-rbac.sql index 271f836e..4905bfb7 100644 --- a/src/main/resources/db/changelog/203-hs-admin-contact-rbac.sql +++ b/src/main/resources/db/changelog/203-hs-admin-contact-rbac.sql @@ -139,7 +139,7 @@ execute procedure deleteRbacRulesForHsAdminContact(); create or replace view hs_admin_contact_iv as select target.uuid, cleanIdentifier(target.label) as idName from hs_admin_contact as target; --- TODO: Is it ok that everybody has access to this information? +-- TODO.spec: Is it ok that everybody has access to this information? grant all privileges on hs_admin_contact_iv to restricted; /* diff --git a/src/test/java/net/hostsharing/hsadminng/hs/admin/partner/HsAdminPartnerControllerAcceptanceTest.java b/src/test/java/net/hostsharing/hsadminng/hs/admin/partner/HsAdminPartnerControllerAcceptanceTest.java index 204faf8a..aeeb90ee 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/admin/partner/HsAdminPartnerControllerAcceptanceTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/admin/partner/HsAdminPartnerControllerAcceptanceTest.java @@ -117,7 +117,7 @@ class HsAdminPartnerControllerAcceptanceTest { final var newUserUuid = UUID.fromString( location.substring(location.lastIndexOf('/') + 1)); assertThat(newUserUuid).isEqualTo(givenUUID); - // TODO: context.define("partner-admin@ttt.example.com"); + // TODO.feat: context.define("partner-admin@ttt.example.com"); // assertThat(partnerRepository.findByUuid(newUserUuid)) // .hasValueSatisfying(c -> assertThat(c.getPerson().getTradeName()).isEqualTo("Test Corp.")); } @@ -146,7 +146,7 @@ class HsAdminPartnerControllerAcceptanceTest { final var newUserUuid = UUID.fromString( location.substring(location.lastIndexOf('/') + 1)); assertThat(newUserUuid).isNotNull(); - // TODO: context.define("partner-admin@ttt.example.com"); + // TODO.feat: context.define("partner-admin@ttt.example.com"); // assertThat(partnerRepository.findByUuid(newUserUuid)) // .hasValueSatisfying(c -> assertThat(c.getPerson().getTradeName()).isEqualTo("Test Corp.")); } @@ -158,7 +158,7 @@ class HsAdminPartnerControllerAcceptanceTest { @Test void hostsharingAdmin_withoutAssumedRole_canGetArbitraryPartner() { - // TODO: final var givenPartnerUuid = partnerRepository.findPartnerByOptionalNameLike("Ixx").get(0).getUuid(); + // TODO.feat: final var givenPartnerUuid = partnerRepository.findPartnerByOptionalNameLike("Ixx").get(0).getUuid(); final var givenPartnerUuid = UUID.randomUUID(); RestAssured // @formatter:off @@ -178,7 +178,7 @@ class HsAdminPartnerControllerAcceptanceTest { @Test @Accepts({ "Partner:X(Access Control)" }) void normalUser_canNotGetUnrelatedPartner() { - // TODO: final var givenPartnerUuid = partnerRepository.findPartnerByOptionalNameLike("Ixx").get(0).getUuid(); + // TODO.feat: final var givenPartnerUuid = partnerRepository.findPartnerByOptionalNameLike("Ixx").get(0).getUuid(); final UUID givenPartnerUuid = UUID.fromString("3fa85f64-5717-4562-b3fc-2c963f66afa6"); RestAssured // @formatter:off