Skip to content

Commit

Permalink
feat: Remove sharingRespected property from namespace protection [DHI… (
Browse files Browse the repository at this point in the history
#15777)

* feat: Remove sharingRespected property from namespace protection [DHIS2-16215]

* feat: update e2e code

* feat: merge ifs [DHIS2-16215]

* feat: addd update and delete tests [DHIS2-16215]
  • Loading branch information
david-mackessy authored Nov 28, 2023
1 parent d73533e commit 94e857d
Show file tree
Hide file tree
Showing 9 changed files with 258 additions and 67 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -67,36 +67,25 @@ public enum ProtectionType {

private final String namespace;

private final boolean sharingRespected;

private final ProtectionType reads;

private final ProtectionType writes;

private final Set<String> authorities;

public DatastoreNamespaceProtection(
String namespace, ProtectionType readWrite, boolean sharingRespected, String... authorities) {
this(namespace, readWrite, readWrite, sharingRespected, authorities);
String namespace, ProtectionType readWrite, String... authorities) {
this(namespace, readWrite, readWrite, authorities);
}

public DatastoreNamespaceProtection(
String namespace,
ProtectionType reads,
ProtectionType writes,
boolean sharingRespected,
String... authorities) {
this(namespace, reads, writes, sharingRespected, new HashSet<>(asList(authorities)));
String namespace, ProtectionType reads, ProtectionType writes, String... authorities) {
this(namespace, reads, writes, new HashSet<>(asList(authorities)));
}

public DatastoreNamespaceProtection(
String namespace,
ProtectionType reads,
ProtectionType writes,
boolean sharingRespected,
Set<String> authorities) {
String namespace, ProtectionType reads, ProtectionType writes, Set<String> authorities) {
this.namespace = namespace;
this.sharingRespected = sharingRespected;
this.reads = reads;
this.writes = writes;
this.authorities = authorities;
Expand All @@ -110,10 +99,6 @@ public String getNamespace() {
* @return true when the {@link org.hisp.dhis.user.sharing.Sharing} of a {@link DatastoreEntry}
* should be checked in addition to authority based checks, else false.
*/
public boolean isSharingRespected() {
return sharingRespected;
}

public Set<String> getAuthorities() {
return authorities;
}
Expand All @@ -129,7 +114,6 @@ public ProtectionType getWrites() {
@Override
public String toString() {
return String.format(
"KeyJsonNamespaceProtection{%s r:%s w:%s [%s]%s}",
namespace, reads, writes, authorities, (sharingRespected ? "!" : ""));
"KeyJsonNamespaceProtection{%s r:%s w:%s [%s]}", namespace, reads, writes, authorities);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,6 @@ public class AndroidSettingsApp {
public AndroidSettingsApp(DatastoreService service) {
service.addProtection(
new DatastoreNamespaceProtection(
NAMESPACE, ProtectionType.NONE, ProtectionType.RESTRICTED, false, AUTHORITY));
NAMESPACE, ProtectionType.NONE, ProtectionType.RESTRICTED, AUTHORITY));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -415,8 +415,7 @@ private void registerDatastoreProtection(App app) {
? new String[] {WEB_MAINTENANCE_APPMANAGER_AUTHORITY}
: new String[] {WEB_MAINTENANCE_APPMANAGER_AUTHORITY, app.getSeeAppAuthority()};
datastoreService.addProtection(
new DatastoreNamespaceProtection(
namespace, ProtectionType.RESTRICTED, true, authorities));
new DatastoreNamespaceProtection(namespace, ProtectionType.RESTRICTED, authorities));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,11 +133,7 @@ public void addEntry(DatastoreEntry entry) throws ConflictException, BadRequestE
@Transactional
public void updateEntry(DatastoreEntry entry) throws BadRequestException {
validateEntry(entry);
DatastoreNamespaceProtection protection = protectionByNamespace.get(entry.getNamespace());
Runnable update =
protection == null || protection.isSharingRespected()
? () -> store.update(entry)
: () -> store.updateNoAcl(entry);
Runnable update = () -> store.updateNoAcl(entry);
writeProtectedIn(entry.getNamespace(), () -> singletonList(entry), update);
}

Expand Down Expand Up @@ -195,14 +191,10 @@ private <T> T readProtectedIn(String namespace, T whenHidden, Supplier<T> read)
DatastoreNamespaceProtection protection = protectionByNamespace.get(namespace);
if (userHasNamespaceReadAccess(protection)) {
T res = read.get();
if (isDatastoreEntryAndProtectionSharingRespected(res, protection)
|| (isDatastoreEntryWithNoProtection(res, protection))) {
DatastoreEntry entry = (DatastoreEntry) res;
if (!aclService.canRead(currentUserService.getCurrentUser(), entry)) {
throw new AccessDeniedException(
String.format(
"Access denied for key '%s' in namespace '%s'", entry.getKey(), namespace));
}
if (res instanceof DatastoreEntry de
&& (!aclService.canRead(currentUserService.getCurrentUser(), de))) {
throw new AccessDeniedException(
String.format("Access denied for key '%s' in namespace '%s'", de.getKey(), namespace));
}
return res;
} else if (protection.getReads() == ProtectionType.RESTRICTED) {
Expand All @@ -211,17 +203,6 @@ private <T> T readProtectedIn(String namespace, T whenHidden, Supplier<T> read)
return whenHidden;
}

private <T> boolean isDatastoreEntryWithNoProtection(
T res, DatastoreNamespaceProtection protection) {
return (res instanceof DatastoreEntry) && (protection == null);
}

private <T> boolean isDatastoreEntryAndProtectionSharingRespected(
T res, DatastoreNamespaceProtection protection) {
return (res instanceof DatastoreEntry)
&& (protection != null && protection.isSharingRespected());
}

private boolean userHasNamespaceReadAccess(DatastoreNamespaceProtection protection) {
return protection == null
|| protection.getReads() == ProtectionType.NONE
Expand All @@ -234,12 +215,9 @@ private void writeProtectedIn(
if (protection == null || protection.getWrites() == ProtectionType.NONE) {
write.run();
} else if (currentUserHasAuthority(protection.getAuthorities())) {
// might also need to check sharing
if (protection.isSharingRespected()) {
for (DatastoreEntry entry : whenSharing.get()) {
if (!aclService.canWrite(currentUserService.getCurrentUser(), entry)) {
throw accessDeniedTo(namespace, entry.getKey());
}
for (DatastoreEntry entry : whenSharing.get()) {
if (!aclService.canWrite(currentUserService.getCurrentUser(), entry)) {
throw accessDeniedTo(namespace, entry.getKey());
}
}
write.run();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ public DefaultMetadataDatastoreService(DatastoreStore store, DatastoreService se
new DatastoreNamespaceProtection(
MetadataDatastoreService.METADATA_STORE_NS,
ProtectionType.HIDDEN,
false,
MetadataDatastoreService.METADATA_SYNC_AUTHORITY));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,6 @@ public DefaultOrgUnitProfileService(
ORG_UNIT_PROFILE_NAMESPACE,
DatastoreNamespaceProtection.ProtectionType.NONE,
DatastoreNamespaceProtection.ProtectionType.RESTRICTED,
false,
"ALL",
ORG_UNIT_PROFILE_AUTHORITY));
}
Expand Down
Loading

0 comments on commit 94e857d

Please sign in to comment.