> On March 24, 2017, 4:23 p.m., Anthony Baker wrote: > > Did you test Pulse in non-embedded mode w/ security?
Haven't. Will try it out. Theoretically, we didn't change anythign in that aspect. It should behave the same. > On March 24, 2017, 4:23 p.m., Anthony Baker wrote: > > geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/PulseAppListener.java > > Line 495 (original) > > <https://reviews.apache.org/r/57894/diff/1/?file=1673144#file1673144line495> > > > > Will this break pulse when not run in embedded mode? I don't quite know why previously we need to get the username/password from a proerty file. I thought user log in with a username/password and we use the same username/password to connect to jmx. This is probably before Integrated security where you need two different sets of credentials > On March 24, 2017, 4:23 p.m., Anthony Baker wrote: > > geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/data/IClusterUpdater.java > > Lines 36 (patched) > > <https://reviews.apache.org/r/57894/diff/1/?file=1673147#file1673147line36> > > > > Seems like it would be better for connect to return a JMXConnection. True. realized that too after I committed it. > On March 24, 2017, 4:23 p.m., Anthony Baker wrote: > > geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/data/IClusterUpdater.java > > Lines 38 (patched) > > <https://reviews.apache.org/r/57894/diff/1/?file=1673147#file1673147line38> > > > > I don't think a default implementation should return null. won't need this anymore. > On March 24, 2017, 4:23 p.m., Anthony Baker wrote: > > geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthenticationProvider.java > > Line 67 (original) > > <https://reviews.apache.org/r/57894/diff/1/?file=1673150#file1673150line73> > > > > Why is this case removed? It's just unnessary, the exception is caught by the catch block immediatly below, so eventually, it's throwing a BadCredentialsException. - Jinmei ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57894/#review170024 ----------------------------------------------------------- On March 24, 2017, 4:03 p.m., Jinmei Liao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57894/ > ----------------------------------------------------------- > > (Updated March 24, 2017, 4:03 p.m.) > > > Review request for geode, Anthony Baker, Jared Stewart, Kevin Duling, Ken > Howe, Kirk Lund, and Patrick Rhomberg. > > > Repository: geode > > > Description > ------- > > * using local mbs server connection will bypass all the mbean security checks > * do not update the mbean attribute since pulse user has no cluster:write > privilege at all > * Created EmbeddedPulseRule for tests > > > Diffs > ----- > > > geode-assembly/src/test/java/org/apache/geode/test/dunit/rules/EmbeddedPulseRule.java > PRE-CREATION > > geode-assembly/src/test/java/org/apache/geode/test/dunit/rules/HttpClientRule.java > d0b54b9de5759e1ba3ce78f4efaa547d391c29cf > > geode-assembly/src/test/java/org/apache/geode/tools/pulse/PulseVerificationTest.java > 0b3b1cef8fa54c8dcb4d2ec2e4f49e9f10d729a7 > > geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/PulseAppListener.java > e1666ecf38331983d8c47c4b46b3b7e87faaf854 > > geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/controllers/PulseController.java > 12b6172cbbc79651e972179532f2b79623a1992e > > geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/data/Cluster.java > e71388d134c96549ee9995c4c874615ee66fe7c1 > > geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/data/IClusterUpdater.java > 3ec820769a897bfb4f10a76a86dd2ce54fba3b70 > > geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/data/JMXDataUpdater.java > 9633b9a1f50df051b8fb9b4f4787a1d25a0ab019 > > geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/data/Repository.java > b228e4a754100fe07c9dbec232d5e88809aefeef > > geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthenticationProvider.java > 4d300f04ff82f701509d44b83dd46698dbc6035e > > geode-pulse/src/test/java/org/apache/geode/tools/pulse/controllers/PulseControllerJUnitTest.java > ddd799f5158e1f4213b0b6b2de8e1853ba65ab74 > > > Diff: https://reviews.apache.org/r/57894/diff/1/ > > > Testing > ------- > > precheckin successful, manul pulse testing > > > Thanks, > > Jinmei Liao > >