RESTWS-941: Any authenticated user should be able to read available m…

…odules (#610)
openmrs · Jun 4, 2024 · 6bd939a · 6bd939a
1 parent 58cc4a9
commit 6bd939a
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 11 deletions.
diff --git a/...openmrs/module/webservices/rest/web/v1_0/resource/openmrs1_8/ModuleActionResource1_8.java b/...openmrs/module/webservices/rest/web/v1_0/resource/openmrs1_8/ModuleActionResource1_8.java
@@ -88,7 +88,7 @@ public Object create(SimpleObject post, RequestContext context) throws ResponseE
 			throw new IllegalRequestException("Cannot execute action " + action.getAction() + " on empty set of modules.");
 		} else {
 			if (action.getAction() == Action.INSTALL) {
-				if (installUri == null || !ResourceUtils.isUrl(installUri)) {
+				if (!ResourceUtils.isUrl(installUri)) {
 					throw new IllegalRequestException("The installUri needs to be a URL for this action to be performed");
 				}
 			}
@@ -171,11 +171,7 @@ private Module installModule(Collection<Module> modules, String installUri, Serv
 		}
 		catch (MalformedURLException e) {
 			throw new APIException(e.getMessage(), e);
-		}
-		catch (IOException e) {
-			throw new APIException(e.getMessage(), e);
-		}
-		finally {
+		} finally {
 			if (tempModule == null && moduleFile != null) {
 				FileUtils.deleteQuietly(moduleFile);
 			}

diff --git a/...a/org/openmrs/module/webservices/rest/web/v1_0/resource/openmrs1_8/ModuleResource1_8.java b/...a/org/openmrs/module/webservices/rest/web/v1_0/resource/openmrs1_8/ModuleResource1_8.java
@@ -46,15 +46,15 @@ public class ModuleResource1_8 extends BaseDelegatingReadableResource<Module> im
 
 	private ModuleFactoryWrapper moduleFactoryWrapper = new ModuleFactoryWrapper();
 
-	private String moduleActionLink = ModuleActionResource1_8.class.getAnnotation(Resource.class).name();
+	private final String moduleActionLink = ModuleActionResource1_8.class.getAnnotation(Resource.class).name();
 
 	public void setModuleFactoryWrapper(ModuleFactoryWrapper moduleFactoryWrapper) {
 		this.moduleFactoryWrapper = moduleFactoryWrapper;
 	}
 
 	@Override
 	public Module getByUniqueId(String uniqueId) {
-		moduleFactoryWrapper.checkPrivilege();
+		moduleFactoryWrapper.checkReadonlyPrivilege();
 		return moduleFactoryWrapper.getModuleById(uniqueId);
 	}
 
@@ -136,11 +136,11 @@ public Model getGETModel(Representation rep) {
 	}
 
 	/**
-	 * @see org.openmrs.module.webservices.rest.web.resource.impl.DelegatingCrudResource#doGetAll(org.openmrs.module.webservices.rest.web.RequestContext)
+	 * @see org.openmrs.module.webservices.rest.web.resource.impl.BaseDelegatingReadableResource#doGetAll(RequestContext)
 	 */
 	@Override
 	public NeedsPaging<Module> doGetAll(RequestContext context) throws ResponseException {
-		moduleFactoryWrapper.checkPrivilege();
+		moduleFactoryWrapper.checkReadonlyPrivilege();
 		return new NeedsPaging<Module>(new ArrayList<Module>(moduleFactoryWrapper.getLoadedModules()), context);
 	}
 
@@ -185,7 +185,7 @@ public Object upload(MultipartFile file, RequestContext context) throws Response
 				module = moduleFactoryWrapper.loadModule(moduleFile);
 				moduleFactoryWrapper.startModule(module, servletContext);
 
-				if (existingModule != null && dependentModulesStopped.size() > 0
+				if (existingModule != null && !dependentModulesStopped.isEmpty()
 				        && moduleFactoryWrapper.isModuleStarted(module)) {
 					startModules(dependentModulesStopped, existingModule, servletContext);
 				}

diff --git a/omod-common/src/main/java/org/openmrs/module/webservices/helper/ModuleFactoryWrapper.java b/omod-common/src/main/java/org/openmrs/module/webservices/helper/ModuleFactoryWrapper.java
@@ -161,4 +161,10 @@ public void checkPrivilege() throws APIAuthenticationException {
 			throw new APIAuthenticationException("Privilege required: " + PrivilegeConstants.MANAGE_MODULES);
 		}
 	}
+
+	public void checkReadonlyPrivilege() throws APIAuthenticationException {
+		if (!Context.isAuthenticated()) {
+			throw new APIAuthenticationException("User must be authenticated");
+		}
+	}
 }