Skip to content

Commit

Permalink
fix: Allow any authenticated identity to access security/v1/debug-ena…
Browse files Browse the repository at this point in the history 
…bled (eclipse-kura#4949)

fix: Allow any authenticated user to access security/v1//debug-enabled

Signed-off-by: Nicola Timeus <[email protected]>
  • Loading branch information
@nicolatimeus
nicolatimeus authored Nov 6, 2023
1 parent 9b83ba5 commit d320e80
Show file tree
Hide file tree
Showing 4 changed files with 140 additions and 2 deletions.
1 change: 1 addition & 0 deletions kura/org.eclipse.kura.rest.security.provider/META-INF/MANIFEST.MF
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ Bundle-ActivationPolicy: lazy
Service-Component: OSGI-INF/*.xml
Import-Package: javax.annotation.security;version="1.2.0",
javax.ws.rs;version="2.0.1",
javax.ws.rs.container;version="2.0.1",
javax.ws.rs.core;version="2.0.1",
org.apache.commons.io;version="2.4.0",
org.eclipse.kura;version="[1.3,2.0)",
Expand Down
15 changes: 13 additions & 2 deletions ...r/src/main/java/org/eclipse/kura/internal/rest/security/provider/SecurityRestService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,19 @@
******************************************************************************/
package org.eclipse.kura.internal.rest.security.provider;

import java.util.Optional;

import javax.annotation.security.RolesAllowed;
import javax.ws.rs.GET;
import javax.ws.rs.POST;
import javax.ws.rs.Path;
import javax.ws.rs.Produces;
import javax.ws.rs.WebApplicationException;
import javax.ws.rs.container.ContainerRequestContext;
import javax.ws.rs.core.Context;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.Response.Status;

import org.eclipse.kura.cloudconnection.request.RequestHandler;
import org.eclipse.kura.cloudconnection.request.RequestHandlerRegistry;
Expand Down Expand Up @@ -118,11 +124,16 @@ public Response reloadCommandLineFingerprint() {
* @return true if the debug is permitted. False otherwise.
*/
@GET
@RolesAllowed(REST_ROLE_NAME)
@Path("/debug-enabled")
@Produces(MediaType.APPLICATION_JSON)
public DebugEnabledDTO isDebugEnabled() {
public DebugEnabledDTO isDebugEnabled(final @Context ContainerRequestContext context) {
try {

if (context != null && !Optional.ofNullable(context.getSecurityContext())
.filter(c -> c.getUserPrincipal() != null).isPresent()) {
throw new WebApplicationException(Status.UNAUTHORIZED);
}

logger.debug(DEBUG_MESSSAGE, "isDebugEnabled");
return new DebugEnabledDTO(this.security.isDebugEnabled());
} catch (Exception e) {
Expand Down
2 changes: 2 additions & 0 deletions kura/test/org.eclipse.kura.rest.security.provider.test/META-INF/MANIFEST.MF
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ Require-Capability: osgi.ee;filter:="(&(osgi.ee=JavaSE)(version=1.8))"
Bundle-ActivationPolicy: lazy
Import-Package: javax.annotation;version="1.2.0",
org.eclipse.kura.core.testutil.requesthandler;version="1.1.0",
org.eclipse.kura.core.testutil.service;version="[1.0,2.0)",
org.eclipse.kura.crypto;version="[1.3,2.0)",
org.eclipse.kura.util.wire.test;version="1.1.0",
org.junit;version="[4.12.0,5.0.0)",
org.junit.runner;version="[4.12.0,5.0.0)",
Expand Down
124 changes: 124 additions & 0 deletions ...ain/java/org/eclipse/kura/internal/rest/security/provider/test/SecurityEndpointsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,18 @@
*******************************************************************************/
package org.eclipse.kura.internal.rest.security.provider.test;

import static org.junit.Assert.fail;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.reset;
import static org.mockito.Mockito.when;

import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.Dictionary;
import java.util.Hashtable;
import java.util.List;
import java.util.Optional;
import java.util.concurrent.TimeUnit;

Expand All @@ -32,6 +35,8 @@
import org.eclipse.kura.core.testutil.requesthandler.RestTransport;
import org.eclipse.kura.core.testutil.requesthandler.Transport;
import org.eclipse.kura.core.testutil.requesthandler.Transport.MethodSpec;
import org.eclipse.kura.core.testutil.service.ServiceUtil;
import org.eclipse.kura.crypto.CryptoService;
import org.eclipse.kura.internal.rest.security.provider.SecurityRestService;
import org.eclipse.kura.security.SecurityService;
import org.eclipse.kura.util.wire.test.WireTestUtil;
Expand All @@ -42,6 +47,10 @@
import org.osgi.framework.FrameworkUtil;
import org.osgi.service.cm.Configuration;
import org.osgi.service.cm.ConfigurationAdmin;
import org.osgi.service.useradmin.Group;
import org.osgi.service.useradmin.Role;
import org.osgi.service.useradmin.User;
import org.osgi.service.useradmin.UserAdmin;

@RunWith(Parameterized.class)
public class SecurityEndpointsTest extends AbstractRequestHandlerTest {
Expand Down Expand Up @@ -70,6 +79,7 @@ public SecurityEndpointsTest(Transport transport) {
@Test
public void shouldInvokeReloadSecurityPolicyFingerprintSuccessfully() {
givenSecurityService();
givenRestBasicCredentials("admin:admin");

whenRequestIsPerformed(new MethodSpec(METHOD_SPEC_POST), "/security-policy-fingerprint/reload");

Expand All @@ -80,6 +90,7 @@ public void shouldInvokeReloadSecurityPolicyFingerprintSuccessfully() {
@Test
public void shouldInvokeReloadCommandLineFingerprintSuccessfully() {
givenSecurityService();
givenRestBasicCredentials("admin:admin");

whenRequestIsPerformed(new MethodSpec(METHOD_SPEC_POST), "/command-line-fingerprint/reload");

Expand All @@ -91,6 +102,32 @@ public void shouldInvokeReloadCommandLineFingerprintSuccessfully() {
public void shouldReturnExpectedDebugStatus() {
givenDebugEnabledStatus(true);
givenSecurityService();
givenRestBasicCredentials("admin:admin");

whenRequestIsPerformed(new MethodSpec(METHOD_SPEC_GET), "/debug-enabled");

thenRequestSucceeds();
thenResponseBodyEqualsJson(EXPECTED_DEBUG_ENABLE_TRUE_RESPONSE);
}

@Test
public void shouldNotReturnDebugStatusOverRestIfNotLoggedIn() {
givenDebugEnabledStatus(true);
givenSecurityService();
givenNoRestBasicCredentials();

whenRequestIsPerformed(new MethodSpec(METHOD_SPEC_GET), "/debug-enabled");

thenRestResponseCodeIs(401);
thenMqttResponseCodeIs(200);
}

@Test
public void shouldReturnDebugStatusEvenIfIdentityHasNoPermissions() {
givenDebugEnabledStatus(true);
givenSecurityService();
givenIdentity("foo", Optional.of("bar"), Collections.emptyList(), false);
givenRestBasicCredentials("foo:bar");

whenRequestIsPerformed(new MethodSpec(METHOD_SPEC_GET), "/debug-enabled");

Expand All @@ -101,6 +138,7 @@ public void shouldReturnExpectedDebugStatus() {
@Test
public void shouldRethrowWebApplicationExceptionOnReloadSecurityPolicyFingerprint() throws KuraException {
givenFailingSecurityService();
givenRestBasicCredentials("admin:admin");

whenRequestIsPerformed(new MethodSpec(METHOD_SPEC_POST), "/security-policy-fingerprint/reload");

Expand All @@ -110,6 +148,7 @@ public void shouldRethrowWebApplicationExceptionOnReloadSecurityPolicyFingerprin
@Test
public void shouldRethrowWebApplicationExceptionOnReloadCommandLineFingerprint() throws KuraException {
givenFailingSecurityService();
givenRestBasicCredentials("admin:admin");

whenRequestIsPerformed(new MethodSpec(METHOD_SPEC_POST), "/command-line-fingerprint/reload");

Expand All @@ -119,6 +158,7 @@ public void shouldRethrowWebApplicationExceptionOnReloadCommandLineFingerprint()
@Test
public void shouldRethrowWebApplicationExceptionOnGetDebugStatus() throws KuraException {
givenFailingSecurityService();
givenRestBasicCredentials("admin:admin");

whenRequestIsPerformed(new MethodSpec(METHOD_SPEC_GET), "/debug-enabled");

Expand Down Expand Up @@ -147,6 +187,67 @@ private static void givenFailingSecurityService() throws KuraException {
* Utilities
*/

@SuppressWarnings("unchecked")
private void givenIdentity(final String username, final Optional<String> password, final List<String> roles,
final boolean needsPasswordChange) {
final UserAdmin userAdmin;

try {
userAdmin = ServiceUtil.trackService(UserAdmin.class, Optional.empty()).get(30, TimeUnit.SECONDS);
} catch (Exception e) {
fail("failed to track UserAdmin");
return;
}

final User user = getOrCreateRole(userAdmin, "kura.user." + username, User.class);

if (password.isPresent()) {
try {
final CryptoService cryptoService = ServiceUtil.trackService(CryptoService.class, Optional.empty())
.get(30, TimeUnit.SECONDS);

user.getCredentials().put("kura.password", cryptoService.sha256Hash(password.get()));

} catch (Exception e) {
fail("failed to compute password hash");
}
}

if (needsPasswordChange) {
user.getProperties().put("kura.need.password.change", "true");
} else {
user.getProperties().remove("kura.need.password.change");
}

for (final String role : roles) {
getOrCreateRole(userAdmin, "kura.permission." + role, Group.class).addMember(user);
}
}

private void givenNoRestBasicCredentials() {
if (this.transport instanceof RestTransport) {
((RestTransport) this.transport).setBasicCredentials(Optional.empty());
}
}

private void givenRestBasicCredentials(final String credentials) {
if (this.transport instanceof RestTransport) {
((RestTransport) this.transport).setBasicCredentials(Optional.of(credentials));
}
}

private void thenRestResponseCodeIs(final int expectedStatusCode) {
if (this.transport instanceof RestTransport) {
thenResponseCodeIs(expectedStatusCode);
}
}

private void thenMqttResponseCodeIs(final int expectedStatusCode) {
if (this.transport instanceof MqttTransport) {
thenResponseCodeIs(expectedStatusCode);
}
}

@BeforeClass
public static void setUp() throws Exception {
createSecurityServiceMock();
Expand All @@ -173,4 +274,27 @@ private static void registerSecurityServiceMock() throws Exception {
config.update(properties);
}

@SuppressWarnings("unchecked")
private <T extends Role> T getOrCreateRole(final UserAdmin userAdmin, final String name, final Class<T> classz) {

final Role existing = userAdmin.getRole(name);

if (classz.isInstance(existing)) {
return (T) existing;
}

final int type;

if (classz == User.class) {
type = Role.USER;
} else if (classz == Group.class) {
type = Role.GROUP;
} else {
fail("Unsupported role type");
return null;
}

return (T) userAdmin.createRole(name, type);
}

}

0 comments on commit d320e80

Please sign in to comment.