From 01d9fc73daec7863800cf48e4a6744f43299bb91 Mon Sep 17 00:00:00 2001 From: Max Shaposhnik Date: Tue, 18 Sep 2018 17:23:59 +0300 Subject: [PATCH] Limit scope of the machine token signed requests --- .../pom.xml | 8 ++ .../server/MachineAuthModule.java | 2 + .../server/MachineLoginFilter.java | 8 +- .../server/MachineTokenAccessFilter.java | 59 +++++++++++++++ .../server/MachineTokenAuthorizedSubject.java | 43 +++++++++++ .../server/MachineLoginFilterTest.java | 27 +++++++ .../server/MachineTokenAccessFilterTest.java | 73 +++++++++++++++++++ .../MachineTokenAuthorizedSubjectTest.java | 60 +++++++++++++++ 8 files changed, 276 insertions(+), 4 deletions(-) create mode 100644 multiuser/machine-auth/che-multiuser-machine-authentication/src/main/java/org/eclipse/che/multiuser/machine/authentication/server/MachineTokenAccessFilter.java create mode 100644 multiuser/machine-auth/che-multiuser-machine-authentication/src/main/java/org/eclipse/che/multiuser/machine/authentication/server/MachineTokenAuthorizedSubject.java create mode 100644 multiuser/machine-auth/che-multiuser-machine-authentication/src/test/java/org/eclipse/che/multiuser/machine/authentication/server/MachineTokenAccessFilterTest.java create mode 100644 multiuser/machine-auth/che-multiuser-machine-authentication/src/test/java/org/eclipse/che/multiuser/machine/authentication/server/MachineTokenAuthorizedSubjectTest.java diff --git a/multiuser/machine-auth/che-multiuser-machine-authentication/pom.xml b/multiuser/machine-auth/che-multiuser-machine-authentication/pom.xml index 5f474fcdbc..48776f1c22 100644 --- a/multiuser/machine-auth/che-multiuser-machine-authentication/pom.xml +++ b/multiuser/machine-auth/che-multiuser-machine-authentication/pom.xml @@ -62,6 +62,10 @@ javax.servlet javax.servlet-api + + javax.ws.rs + javax.ws.rs-api + org.eclipse.che.core che-core-api-core @@ -122,6 +126,10 @@ org.eclipse.persistence javax.persistence + + org.everrest + everrest-core + org.slf4j slf4j-api diff --git a/multiuser/machine-auth/che-multiuser-machine-authentication/src/main/java/org/eclipse/che/multiuser/machine/authentication/server/MachineAuthModule.java b/multiuser/machine-auth/che-multiuser-machine-authentication/src/main/java/org/eclipse/che/multiuser/machine/authentication/server/MachineAuthModule.java index 8d4f67afc2..20ecf4f68c 100644 --- a/multiuser/machine-auth/che-multiuser-machine-authentication/src/main/java/org/eclipse/che/multiuser/machine/authentication/server/MachineAuthModule.java +++ b/multiuser/machine-auth/che-multiuser-machine-authentication/src/main/java/org/eclipse/che/multiuser/machine/authentication/server/MachineAuthModule.java @@ -35,6 +35,8 @@ public class MachineAuthModule extends AbstractModule { bind(MachineTokenProvider.class).to(MachineTokenProviderImpl.class); + bind(MachineTokenAccessFilter.class); + bind(SignatureKeyManager.class); bind(SignatureKeyDao.class).to(JpaSignatureKeyDao.class); bind(JpaSignatureKeyDao.RemoveKeyPairsBeforeWorkspaceRemovedEventSubscriber.class) diff --git a/multiuser/machine-auth/che-multiuser-machine-authentication/src/main/java/org/eclipse/che/multiuser/machine/authentication/server/MachineLoginFilter.java b/multiuser/machine-auth/che-multiuser-machine-authentication/src/main/java/org/eclipse/che/multiuser/machine/authentication/server/MachineLoginFilter.java index c074f715ea..be7dad2fb7 100644 --- a/multiuser/machine-auth/che-multiuser-machine-authentication/src/main/java/org/eclipse/che/multiuser/machine/authentication/server/MachineLoginFilter.java +++ b/multiuser/machine-auth/che-multiuser-machine-authentication/src/main/java/org/eclipse/che/multiuser/machine/authentication/server/MachineLoginFilter.java @@ -15,6 +15,7 @@ import static com.google.common.base.Strings.isNullOrEmpty; import static java.lang.String.format; import static javax.servlet.http.HttpServletResponse.SC_UNAUTHORIZED; import static org.eclipse.che.multiuser.machine.authentication.shared.Constants.USER_ID_CLAIM; +import static org.eclipse.che.multiuser.machine.authentication.shared.Constants.WORKSPACE_ID_CLAIM; import io.jsonwebtoken.Claims; import io.jsonwebtoken.JwtException; @@ -41,7 +42,6 @@ import org.eclipse.che.commons.auth.token.RequestTokenExtractor; import org.eclipse.che.commons.env.EnvironmentContext; import org.eclipse.che.commons.subject.Subject; import org.eclipse.che.commons.subject.SubjectImpl; -import org.eclipse.che.multiuser.api.permission.server.AuthorizedSubject; import org.eclipse.che.multiuser.api.permission.server.PermissionChecker; /** @@ -121,9 +121,9 @@ public class MachineLoginFilter implements Filter { final String userId = claims.get(USER_ID_CLAIM, String.class); // check if user with such id exists final String userName = userManager.getById(userId).getName(); - - return new AuthorizedSubject( - new SubjectImpl(userName, userId, token, false), permissionChecker); + final String workspaceId = claims.get(WORKSPACE_ID_CLAIM, String.class); + return new MachineTokenAuthorizedSubject( + new SubjectImpl(userName, userId, token, false), permissionChecker, workspaceId); } /** Sets given error code with err message into give response. */ diff --git a/multiuser/machine-auth/che-multiuser-machine-authentication/src/main/java/org/eclipse/che/multiuser/machine/authentication/server/MachineTokenAccessFilter.java b/multiuser/machine-auth/che-multiuser-machine-authentication/src/main/java/org/eclipse/che/multiuser/machine/authentication/server/MachineTokenAccessFilter.java new file mode 100644 index 0000000000..3cacc1e832 --- /dev/null +++ b/multiuser/machine-auth/che-multiuser-machine-authentication/src/main/java/org/eclipse/che/multiuser/machine/authentication/server/MachineTokenAccessFilter.java @@ -0,0 +1,59 @@ +/* + * Copyright (c) 2012-2018 Red Hat, Inc. + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ +package org.eclipse.che.multiuser.machine.authentication.server; + +import static java.util.Arrays.asList; + +import com.google.common.collect.HashMultimap; +import com.google.common.collect.SetMultimap; +import javax.ws.rs.Path; +import org.eclipse.che.api.core.ForbiddenException; +import org.eclipse.che.commons.env.EnvironmentContext; +import org.eclipse.che.everrest.CheMethodInvokerFilter; +import org.everrest.core.Filter; +import org.everrest.core.resource.GenericResourceMethod; + +/** + * Limits set of methods which can be invoked using machine token signed requests. + * + * @author Max Shaposhnik (mshaposh@redhat.com) + */ +@Filter +@Path("/{path:.*}") +public class MachineTokenAccessFilter extends CheMethodInvokerFilter { + + private final SetMultimap allowedMethodsByPath = HashMultimap.create(); + + public MachineTokenAccessFilter() { + allowedMethodsByPath.putAll( + "/workspace", asList("getByKey", "addProject", "updateProject", "deleteProject")); + allowedMethodsByPath.putAll("/ssh", asList("getPair", "generatePair")); + allowedMethodsByPath.putAll( + "/factory", + asList("getFactoryJson", "getFactory", "getFactoryByAttribute", "resolveFactory")); + allowedMethodsByPath.put("/preferences", "find"); + allowedMethodsByPath.put("/activity", "active"); + } + + @Override + protected void filter(GenericResourceMethod genericMethodResource, Object[] arguments) + throws ForbiddenException { + if (!(EnvironmentContext.getCurrent().getSubject() instanceof MachineTokenAuthorizedSubject)) { + return; + } + if (!allowedMethodsByPath + .get(genericMethodResource.getParentResource().getPathValue().getPath()) + .contains(genericMethodResource.getMethod().getName())) { + throw new ForbiddenException("This operation cannot be performed using machine token."); + } + } +} diff --git a/multiuser/machine-auth/che-multiuser-machine-authentication/src/main/java/org/eclipse/che/multiuser/machine/authentication/server/MachineTokenAuthorizedSubject.java b/multiuser/machine-auth/che-multiuser-machine-authentication/src/main/java/org/eclipse/che/multiuser/machine/authentication/server/MachineTokenAuthorizedSubject.java new file mode 100644 index 0000000000..b37b39710c --- /dev/null +++ b/multiuser/machine-auth/che-multiuser-machine-authentication/src/main/java/org/eclipse/che/multiuser/machine/authentication/server/MachineTokenAuthorizedSubject.java @@ -0,0 +1,43 @@ +/* + * Copyright (c) 2012-2018 Red Hat, Inc. + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ +package org.eclipse.che.multiuser.machine.authentication.server; + +import org.eclipse.che.commons.subject.Subject; +import org.eclipse.che.multiuser.api.permission.server.AuthorizedSubject; +import org.eclipse.che.multiuser.api.permission.server.PermissionChecker; +import org.eclipse.che.multiuser.permission.workspace.server.WorkspaceDomain; + +/** + * An implementation of {@link Subject} which should be used when request was signed by machine + * token. This implementation limits all workspace related permissions to the single workspace for + * which the machine token was issued. + * + * @author Max Shaposhnik (mshaposh@redhat.com) + */ +public class MachineTokenAuthorizedSubject extends AuthorizedSubject { + + private final String claimsWorkspaceId; + + public MachineTokenAuthorizedSubject( + Subject baseSubject, PermissionChecker permissionChecker, String claimsWorkspaceId) { + super(baseSubject, permissionChecker); + this.claimsWorkspaceId = claimsWorkspaceId; + } + + @Override + public boolean hasPermission(String domain, String instance, String action) { + if (domain.equals(WorkspaceDomain.DOMAIN_ID) && !instance.equals(claimsWorkspaceId)) { + return false; + } + return super.hasPermission(domain, instance, action); + } +} diff --git a/multiuser/machine-auth/che-multiuser-machine-authentication/src/test/java/org/eclipse/che/multiuser/machine/authentication/server/MachineLoginFilterTest.java b/multiuser/machine-auth/che-multiuser-machine-authentication/src/test/java/org/eclipse/che/multiuser/machine/authentication/server/MachineLoginFilterTest.java index 81b08680e3..a894a0d003 100644 --- a/multiuser/machine-auth/che-multiuser-machine-authentication/src/test/java/org/eclipse/che/multiuser/machine/authentication/server/MachineLoginFilterTest.java +++ b/multiuser/machine-auth/che-multiuser-machine-authentication/src/test/java/org/eclipse/che/multiuser/machine/authentication/server/MachineLoginFilterTest.java @@ -24,6 +24,7 @@ import static org.mockito.Mockito.when; import io.jsonwebtoken.Claims; import io.jsonwebtoken.Jwts; +import io.jsonwebtoken.impl.DefaultClaims; import java.security.KeyPair; import java.security.KeyPairGenerator; import java.util.HashMap; @@ -139,6 +140,32 @@ public class MachineLoginFilterTest { + " JWT validity cannot be asserted and should not be trusted."); } + @Test + public void testNotProceedRequestWhenNoWorkspaceIdClaim() throws Exception { + final HttpServletRequest requestMock = getRequestMock(); + final KeyPairGenerator kpg = KeyPairGenerator.getInstance(SIGNATURE_ALGORITHM); + kpg.initialize(KEY_SIZE); + final KeyPair pair = kpg.generateKeyPair(); + final Claims badClaims = new DefaultClaims(); + badClaims.put(Constants.USER_ID_CLAIM, SUBJECT.getUserId()); + badClaims.put(Claims.ID, "84123-132-fn31"); + final String token = + Jwts.builder() + .setClaims(badClaims) + .setHeader(HEADER) + .signWith(RS512, pair.getPrivate()) + .compact(); + when(tokenExtractorMock.getToken(any(HttpServletRequest.class))).thenReturn(token); + + machineLoginFilter.doFilter(requestMock, responseMock, chainMock); + + verify(tokenExtractorMock).getToken(any(HttpServletRequest.class)); + verify(responseMock) + .sendError( + 401, + "Authentication with machine token failed cause: Unable to fetch signature key pair: no workspace id present in token"); + } + @Test public void testProceedRequestWhenEmptyTokenProvided() throws Exception { final HttpServletRequest requestMock = getRequestMock(); diff --git a/multiuser/machine-auth/che-multiuser-machine-authentication/src/test/java/org/eclipse/che/multiuser/machine/authentication/server/MachineTokenAccessFilterTest.java b/multiuser/machine-auth/che-multiuser-machine-authentication/src/test/java/org/eclipse/che/multiuser/machine/authentication/server/MachineTokenAccessFilterTest.java new file mode 100644 index 0000000000..655a7c3bae --- /dev/null +++ b/multiuser/machine-auth/che-multiuser-machine-authentication/src/test/java/org/eclipse/che/multiuser/machine/authentication/server/MachineTokenAccessFilterTest.java @@ -0,0 +1,73 @@ +/* + * Copyright (c) 2012-2018 Red Hat, Inc. + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ +package org.eclipse.che.multiuser.machine.authentication.server; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.when; + +import java.lang.reflect.Method; +import org.eclipse.che.api.core.ForbiddenException; +import org.eclipse.che.api.workspace.server.WorkspaceService; +import org.eclipse.che.commons.env.EnvironmentContext; +import org.eclipse.che.multiuser.api.permission.server.AuthorizedSubject; +import org.everrest.core.impl.resource.PathValue; +import org.everrest.core.resource.GenericResourceMethod; +import org.everrest.core.resource.ResourceDescriptor; +import org.mockito.Mock; +import org.mockito.testng.MockitoTestNGListener; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Listeners; +import org.testng.annotations.Test; + +@Listeners(MockitoTestNGListener.class) +public class MachineTokenAccessFilterTest { + + @Mock EnvironmentContext environmentContext; + + @Mock GenericResourceMethod genericMethodResource; + + @Mock MachineTokenAuthorizedSubject machineTokenAuthorizedSubject; + + @Mock AuthorizedSubject authorizedSubject; + + MachineTokenAccessFilter filter; + + @BeforeMethod + private void setUp() { + filter = new MachineTokenAccessFilter(); + } + + @Test + public void shouldNotLimitAccessIfSubjectIsNotMachineAuthorized() throws Exception { + when(environmentContext.getSubject()).thenReturn(authorizedSubject); + EnvironmentContext.setCurrent(environmentContext); + filter.filter(genericMethodResource, new Object[] {}); + verifyZeroInteractions(genericMethodResource); + } + + @Test(expectedExceptions = ForbiddenException.class) + public void shouldLimitAccessIfMethodIsNotAllowed() throws Exception { + when(environmentContext.getSubject()).thenReturn(machineTokenAuthorizedSubject); + EnvironmentContext.setCurrent(environmentContext); + Method method = WorkspaceService.class.getMethod("getServiceDescriptor"); + ResourceDescriptor descriptor = mock(ResourceDescriptor.class); + PathValue pathValue = mock(PathValue.class); + + when(genericMethodResource.getMethod()).thenReturn(method); + when(descriptor.getPathValue()).thenReturn(pathValue); + when(genericMethodResource.getParentResource()).thenReturn(descriptor); + when(pathValue.getPath()).thenReturn("/workspace"); + + filter.filter(genericMethodResource, new Object[] {}); + } +} diff --git a/multiuser/machine-auth/che-multiuser-machine-authentication/src/test/java/org/eclipse/che/multiuser/machine/authentication/server/MachineTokenAuthorizedSubjectTest.java b/multiuser/machine-auth/che-multiuser-machine-authentication/src/test/java/org/eclipse/che/multiuser/machine/authentication/server/MachineTokenAuthorizedSubjectTest.java new file mode 100644 index 0000000000..87e5c723bd --- /dev/null +++ b/multiuser/machine-auth/che-multiuser-machine-authentication/src/test/java/org/eclipse/che/multiuser/machine/authentication/server/MachineTokenAuthorizedSubjectTest.java @@ -0,0 +1,60 @@ +/* + * Copyright (c) 2012-2018 Red Hat, Inc. + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ +package org.eclipse.che.multiuser.machine.authentication.server; + +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static org.testng.Assert.assertFalse; + +import org.eclipse.che.commons.subject.Subject; +import org.eclipse.che.multiuser.api.permission.server.PermissionChecker; +import org.eclipse.che.multiuser.api.permission.server.SystemDomain; +import org.eclipse.che.multiuser.permission.workspace.server.WorkspaceDomain; +import org.mockito.Mock; +import org.mockito.testng.MockitoTestNGListener; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Listeners; +import org.testng.annotations.Test; + +@Listeners(MockitoTestNGListener.class) +public class MachineTokenAuthorizedSubjectTest { + + private static final String WS_ID = "ws123"; + private static final String USER_ID = "user123"; + @Mock private PermissionChecker permissionChecker; + + @Mock Subject baseSubject; + + private MachineTokenAuthorizedSubject subject; + + @BeforeMethod + private void setUp() { + when(baseSubject.getUserId()).thenReturn(USER_ID); + subject = new MachineTokenAuthorizedSubject(baseSubject, permissionChecker, WS_ID); + } + + @Test + public void shouldRejectPermissionsFromAnotherWorkspace() { + assertFalse( + subject.hasPermission(WorkspaceDomain.DOMAIN_ID, "another_ws", WorkspaceDomain.READ)); + } + + @Test + public void shouldRequestPermissionsFromBaseSubjectForNonWorkspaceDomains() throws Exception { + subject.hasPermission(SystemDomain.DOMAIN_ID, "", SystemDomain.MANAGE_SYSTEM_ACTION); + verify(permissionChecker, atLeastOnce()) + .hasPermission( + eq(USER_ID), eq(SystemDomain.DOMAIN_ID), eq(""), eq(SystemDomain.MANAGE_SYSTEM_ACTION)); + } +}