vyommani commented on code in PR #1266:
URL: https://github.com/apache/ranger/pull/1266#discussion_r4143872754


##########
security-admin/src/test/java/org/apache/ranger/biz/TestServiceMgr.java:
##########
@@ -385,4 +386,138 @@ public void 
test13_validateConfig_allowsLegitimateExternalUrls() throws Exceptio
         Assertions.assertNotNull(response);
         Assertions.assertNotEquals(VXResponse.STATUS_ERROR, 
response.getStatusCode());
     }
+
+    @ParameterizedTest
+    @MethodSource("ssrfBypassConfigValues")
+    public void test14_lookupResource_rejectsSSRFBypassAttempts(String 
configKey, String configValue) throws Exception {
+        ServiceMgr mgr = new ServiceMgr();
+        ServiceDBStore svcDBStore = mock(ServiceDBStore.class);
+        RangerServiceService svcService = mock(RangerServiceService.class);
+        TimedExecutor exec = mock(TimedExecutor.class);
+        setField(mgr, ServiceMgr.class, "svcDBStore", svcDBStore);
+        setField(mgr, ServiceMgr.class, "rangerSvcService", svcService);
+        setField(mgr, ServiceMgr.class, "timedExecutor", exec);
+
+        Map<String, String> bypassConfigs = new HashMap<>();
+        bypassConfigs.put(configKey, configValue);
+
+        RangerService svc = new RangerService();
+        svc.setType("knox");
+        svc.setName("knox-test");
+        svc.setConfigs(bypassConfigs);
+
+        when(svcDBStore.getServiceByName("knox-test")).thenReturn(svc);
+        
when(svcService.getConfigsWithDecryptedPassword(any(RangerService.class))).thenReturn(bypassConfigs);
+
+        ResourceLookupContext ctx = new ResourceLookupContext();
+        Exception thrown = Assertions.assertThrows(Exception.class, () -> 
mgr.lookupResource("knox-test", ctx, null));
+        Assertions.assertTrue(
+                thrown.getMessage().contains("is not allowed") || 
thrown.getMessage().contains("blocked host detected"),
+                "Unexpected message: " + thrown.getMessage());
+    }
+
+    @Test
+    public void 
test15_validateConfig_requireSavedService_rejectsMissingService() throws 
Exception {
+        ServiceMgr mgr = new ServiceMgr();
+        ServiceDBStore db = mock(ServiceDBStore.class);
+        RangerServiceService svcService = mock(RangerServiceService.class);
+        setField(mgr, ServiceMgr.class, "svcDBStore", db);
+        setField(mgr, ServiceMgr.class, "rangerSvcService", svcService);
+
+        
PropertiesUtil.getPropertiesMap().put(ServiceMgr.PROP_SERVICE_CONFIG_VALIDATE_REQUIRE_SAVED_SERVICE,
 "true");
+        try {
+            Map<String, String> cfg = new HashMap<>();
+            cfg.put("knox.url", "https://knox.example.com:8443/gateway";);
+
+            RangerService svc = new RangerService();
+            svc.setName("knox-prod");
+            svc.setType("knox");
+            svc.setConfigs(cfg);
+
+            
when(svcService.getConfigsWithDecryptedPassword(any(RangerService.class))).thenReturn(cfg);
+            when(db.getServiceByName("knox-prod")).thenReturn(null);
+
+            Exception thrown = Assertions.assertThrows(Exception.class, () -> 
mgr.validateConfig(svc, null));
+            Assertions.assertTrue(thrown.getMessage().contains("requires a 
saved service"));
+        } finally {
+            
PropertiesUtil.getPropertiesMap().remove(ServiceMgr.PROP_SERVICE_CONFIG_VALIDATE_REQUIRE_SAVED_SERVICE);
+        }
+    }
+
+    @Test
+    public void 
test16_validateConfig_requireSavedService_rejectsEndpointMismatch() throws 
Exception {
+        ServiceMgr mgr = new ServiceMgr();
+        ServiceDBStore db = mock(ServiceDBStore.class);
+        RangerServiceService svcService = mock(RangerServiceService.class);
+        setField(mgr, ServiceMgr.class, "svcDBStore", db);
+        setField(mgr, ServiceMgr.class, "rangerSvcService", svcService);
+
+        
PropertiesUtil.getPropertiesMap().put(ServiceMgr.PROP_SERVICE_CONFIG_VALIDATE_REQUIRE_SAVED_SERVICE,
 "true");
+        try {
+            Map<String, String> savedCfg = new HashMap<>();
+            savedCfg.put("knox.url", "https://knox.example.com:8443/gateway";);
+
+            Map<String, String> requestCfg = new HashMap<>();
+            requestCfg.put("knox.url", 
"https://attacker.example.com:8443/gateway";);
+
+            RangerService saved = new RangerService();
+            saved.setName("knox-prod");
+            saved.setType("knox");
+            saved.setConfigs(savedCfg);
+
+            RangerService request = new RangerService();
+            request.setName("knox-prod");
+            request.setType("knox");
+            request.setConfigs(requestCfg);
+
+            when(db.getServiceByName("knox-prod")).thenReturn(saved);
+            
when(svcService.getConfigsWithDecryptedPassword(saved)).thenReturn(savedCfg);
+            
when(svcService.getConfigsWithDecryptedPassword(request)).thenReturn(requestCfg);
+
+            Exception thrown = Assertions.assertThrows(Exception.class, () -> 
mgr.validateConfig(request, null));
+            Assertions.assertTrue(thrown.getMessage().contains("must match the 
saved service configuration"));
+        } finally {
+            
PropertiesUtil.getPropertiesMap().remove(ServiceMgr.PROP_SERVICE_CONFIG_VALIDATE_REQUIRE_SAVED_SERVICE);
+        }
+    }
+
+    @Test
+    public void 
test17_validateConfig_requireSavedService_allowsMatchingEndpoint() throws 
Exception {

Review Comment:
   This test catches every exception and only checks that the message is not 
the saved-service error. validateConfig can fail for another reason and the 
test still passes.
   
   Please assert that validateConfig returns a non-error response, and let an 
unexpected exception fail the test. Cases for a different port, a 
comma-separated second host, and a jdbc: URL whose host extractHost cannot 
parse would cover the gaps above.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to