From 38ab7f55f1eb6b179740b5b24c6a1b40b80d1e59 Mon Sep 17 00:00:00 2001 From: Chad Elliott Date: Thu, 8 Aug 2024 13:45:51 -0500 Subject: [PATCH 1/6] Added a new controller to handle user impersonation. This allows admin users with the CAN_IMPERSONATE_MEMBERS permission to impersonate other users and easily switch back to their original login. --- .../security/ImpersonationController.java | 152 ++++++++++++++++++ .../security/LocalLoginController.java | 8 +- .../services/permissions/Permission.java | 1 + .../resources/db/dev/R__Load_testing_data.sql | 5 + .../member-directory/AdminMemberCard.jsx | 35 +++- web-ui/src/pages/HomePage.css | 6 + web-ui/src/pages/HomePage.jsx | 7 + 7 files changed, 210 insertions(+), 4 deletions(-) create mode 100644 server/src/main/java/com/objectcomputing/checkins/security/ImpersonationController.java diff --git a/server/src/main/java/com/objectcomputing/checkins/security/ImpersonationController.java b/server/src/main/java/com/objectcomputing/checkins/security/ImpersonationController.java new file mode 100644 index 0000000000..f781184bdd --- /dev/null +++ b/server/src/main/java/com/objectcomputing/checkins/security/ImpersonationController.java @@ -0,0 +1,152 @@ +package com.objectcomputing.checkins.security; + +import com.objectcomputing.checkins.Environments; +import com.objectcomputing.checkins.services.memberprofile.MemberProfile; +import com.objectcomputing.checkins.services.memberprofile.currentuser.CurrentUserServices; +import com.objectcomputing.checkins.services.permissions.Permission; +import com.objectcomputing.checkins.services.permissions.RequiredPermission; +import io.micronaut.http.MutableHttpResponse; +import io.micronaut.http.HttpHeaders; +import io.micronaut.http.cookie.SameSite; +import io.micronaut.http.cookie.Cookie; +import io.micronaut.http.netty.cookies.NettyCookie; +import io.micronaut.security.utils.SecurityService; +import io.micronaut.context.annotation.Requires; +import io.micronaut.context.event.ApplicationEventPublisher; +import io.micronaut.http.HttpRequest; +import io.micronaut.http.HttpResponse; +import io.micronaut.http.HttpStatus; +import io.micronaut.http.MediaType; +import io.micronaut.http.annotation.Consumes; +import io.micronaut.http.annotation.Produces; +import io.micronaut.http.annotation.Controller; +import io.micronaut.http.annotation.Get; +import io.micronaut.http.annotation.Post; +import io.micronaut.scheduling.TaskExecutors; +import io.micronaut.scheduling.annotation.ExecuteOn; +import io.micronaut.security.annotation.Secured; +import io.micronaut.security.authentication.Authentication; +import io.micronaut.security.authentication.AuthenticationResponse; +import io.micronaut.security.authentication.Authenticator; +import io.micronaut.security.authentication.UsernamePasswordCredentials; +import io.micronaut.security.event.LoginFailedEvent; +import io.micronaut.security.event.LoginSuccessfulEvent; +import io.micronaut.security.handlers.LoginHandler; +import io.micronaut.security.rules.SecurityRule; +import reactor.core.publisher.Flux; +import reactor.core.publisher.Mono; + +import java.util.HashMap; +import java.util.Locale; +import java.util.Map; +import java.util.Optional; +import java.util.HashSet; +import java.util.Set; +import java.net.URI; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +@Requires(env = Environments.LOCAL) +@Controller("/impersonation") +@ExecuteOn(TaskExecutors.BLOCKING) +@Secured(SecurityRule.IS_AUTHENTICATED) +public class ImpersonationController { + public static final String originalJWT = "OJWT"; + private static final Logger LOG = LoggerFactory.getLogger(ImpersonationController.class); + protected final Authenticator authenticator; + protected final LoginHandler loginHandler; + protected final ApplicationEventPublisher eventPublisher; + private final CurrentUserServices currentUserServices; + private final SecurityService securityService; + + /** + * @param authenticator {@link Authenticator} collaborator + * @param loginHandler A collaborator which helps to build HTTP response depending on success or failure. + * @param eventPublisher The application event publisher + * @param currentUserServices Current User services + */ + public ImpersonationController(Authenticator authenticator, + LoginHandler loginHandler, + ApplicationEventPublisher eventPublisher, + CurrentUserServices currentUserServices, + SecurityService securityService) { + this.authenticator = authenticator; + this.loginHandler = loginHandler; + this.eventPublisher = eventPublisher; + this.currentUserServices = currentUserServices; + this.securityService = securityService; + } + + @ExecuteOn(TaskExecutors.BLOCKING) + @Consumes({MediaType.APPLICATION_FORM_URLENCODED, MediaType.APPLICATION_JSON}) + @Post("/begin") + @RequiredPermission(Permission.CAN_IMPERSONATE_MEMBERS) + public Mono auth(HttpRequest request, String email) { + if (securityService != null) { + Optional auth = securityService.getAuthentication(); + if (auth.isPresent() && auth.get().getAttributes().get("email") != null) { + HttpHeaders headers = request.getHeaders(); + final Cookie jwt = request.getCookies().get("JWT"); + if (jwt == null) { + // The user is required to be logged in. If this is null, + // we are in an impossible state! + LOG.error("Unable to locate the JWT"); + } else { + UsernamePasswordCredentials usernamePasswordCredentials = new UsernamePasswordCredentials(email, ""); + Flux authenticationResponseFlux = + Flux.from(authenticator.authenticate(request, usernamePasswordCredentials)); + return authenticationResponseFlux.map(authenticationResponse -> { + if (authenticationResponse.isAuthenticated() && authenticationResponse.getAuthentication().isPresent()) { + Authentication authentication = authenticationResponse.getAuthentication().get(); + // Get member profile by work email + MemberProfile memberProfile = currentUserServices.findOrSaveUser("", "", email); + String firstName = memberProfile.getFirstName() != null ? memberProfile.getFirstName() : ""; + String lastName = memberProfile.getLastName() != null ? memberProfile.getLastName() : ""; + + Map newAttributes = new HashMap<>(authentication.getAttributes()); + newAttributes.put("email", memberProfile.getWorkEmail()); + newAttributes.put("name", firstName + ' ' + lastName); + newAttributes.put("picture", ""); + Authentication updatedAuth = Authentication.build(authentication.getName(), authentication.getRoles(), newAttributes); + + eventPublisher.publishEvent(new LoginSuccessfulEvent(updatedAuth, null, Locale.getDefault())); + // Store the old JWT to allow the user to revert the impersonation. + return ((MutableHttpResponse)loginHandler.loginSuccess(updatedAuth, request)).cookie( + new NettyCookie(originalJWT, jwt.getValue()).path("/").sameSite(SameSite.Strict) + .maxAge(jwt.getMaxAge())); + } else { + eventPublisher.publishEvent(new LoginFailedEvent(authenticationResponse, null, null, Locale.getDefault())); + return loginHandler.loginFailed(authenticationResponse, request); + } + }).single(Mono.just(HttpResponse.status(HttpStatus.UNAUTHORIZED))); + } + } else { + LOG.error("Attempted impersonation without authentication."); + } + } + return null; + } + + @ExecuteOn(TaskExecutors.BLOCKING) + @Produces(MediaType.TEXT_HTML) + @Get("/end") + public HttpResponse revert(HttpRequest request) { + HttpHeaders headers = request.getHeaders(); + final Cookie ojwt = request.getCookies().get(originalJWT); + if (ojwt == null) { + return HttpResponse.status(HttpStatus.UNAUTHORIZED); + } else { + // Swap the OJWT back to the JWT and remove the original JWT + Set cookies = new HashSet(); + cookies.add(new NettyCookie("JWT", ojwt.getValue()).path("/") + .sameSite(SameSite.Strict) + .maxAge(ojwt.getMaxAge()).httpOnly()); + cookies.add(new NettyCookie(originalJWT, "").path("/").maxAge(0)); + + // Redirect to "/" while setting the cookies. + return HttpResponse.temporaryRedirect(URI.create("/")) + .cookies(cookies); + } + } +} diff --git a/server/src/main/java/com/objectcomputing/checkins/security/LocalLoginController.java b/server/src/main/java/com/objectcomputing/checkins/security/LocalLoginController.java index 3d6cbf5315..c9b0509113 100644 --- a/server/src/main/java/com/objectcomputing/checkins/security/LocalLoginController.java +++ b/server/src/main/java/com/objectcomputing/checkins/security/LocalLoginController.java @@ -3,6 +3,9 @@ import com.objectcomputing.checkins.Environments; import com.objectcomputing.checkins.services.memberprofile.MemberProfile; import com.objectcomputing.checkins.services.memberprofile.currentuser.CurrentUserServices; +import com.objectcomputing.checkins.security.ImpersonationController; +import io.micronaut.http.MutableHttpResponse; +import io.micronaut.http.netty.cookies.NettyCookie; import io.micronaut.context.annotation.Requires; import io.micronaut.context.event.ApplicationEventPublisher; import io.micronaut.http.HttpRequest; @@ -88,7 +91,10 @@ public Mono auth(HttpRequest request, String email, String role) { Authentication updatedAuth = Authentication.build(authentication.getName(), authentication.getRoles(), newAttributes); eventPublisher.publishEvent(new LoginSuccessfulEvent(updatedAuth, null, Locale.getDefault())); - return loginHandler.loginSuccess(updatedAuth, request); + + // Remove the original JWT on login. + return ((MutableHttpResponse)loginHandler.loginSuccess(updatedAuth, request)) + .cookie(new NettyCookie(ImpersonationController.originalJWT, "").path("/").maxAge(0)); } else { eventPublisher.publishEvent(new LoginFailedEvent(authenticationResponse, null, null, Locale.getDefault())); return loginHandler.loginFailed(authenticationResponse, request); diff --git a/server/src/main/java/com/objectcomputing/checkins/services/permissions/Permission.java b/server/src/main/java/com/objectcomputing/checkins/services/permissions/Permission.java index c375d96b9d..2942074820 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/permissions/Permission.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/permissions/Permission.java @@ -15,6 +15,7 @@ public enum Permission { CAN_VIEW_FEEDBACK_ANSWER("View feedback answers", "Feedback"), CAN_DELETE_ORGANIZATION_MEMBERS("Delete organization members", "User Management"), CAN_CREATE_ORGANIZATION_MEMBERS("Create organization members", "User Management"), + CAN_IMPERSONATE_MEMBERS("Impersonate organization members", "User Management"), CAN_VIEW_ROLE_PERMISSIONS("View role permissions", "Security"), CAN_ASSIGN_ROLE_PERMISSIONS("Assign role permissions", "Security"), CAN_VIEW_PERMISSIONS("View all permissions", "Security"), diff --git a/server/src/main/resources/db/dev/R__Load_testing_data.sql b/server/src/main/resources/db/dev/R__Load_testing_data.sql index f1d4c48caa..534f73bf49 100644 --- a/server/src/main/resources/db/dev/R__Load_testing_data.sql +++ b/server/src/main/resources/db/dev/R__Load_testing_data.sql @@ -850,6 +850,11 @@ insert into role_permissions values ('e8a4fff8-e984-4e59-be84-a713c9fa8d23', 'CAN_CREATE_KUDOS'); +insert into role_permissions + (roleid, permission) +values + ('e8a4fff8-e984-4e59-be84-a713c9fa8d23', 'CAN_IMPERSONATE_MEMBERS'); + -- PDL Permissions insert into role_permissions (roleid, permission) diff --git a/web-ui/src/components/member-directory/AdminMemberCard.jsx b/web-ui/src/components/member-directory/AdminMemberCard.jsx index d57b5afe16..0d19a57aba 100644 --- a/web-ui/src/components/member-directory/AdminMemberCard.jsx +++ b/web-ui/src/components/member-directory/AdminMemberCard.jsx @@ -6,7 +6,7 @@ import MemberModal from './MemberModal'; import { AppContext } from '../../context/AppContext'; import { DELETE_MEMBER_PROFILE, UPDATE_MEMBER_PROFILES, UPDATE_TOAST } from '../../context/actions'; import { selectProfileMap } from '../../context/selectors'; -import { getAvatarURL } from '../../api/api.js'; +import { getAvatarURL, resolve } from '../../api/api.js'; import Avatar from '@mui/material/Avatar'; import PriorityHighIcon from '@mui/icons-material/PriorityHigh'; @@ -73,16 +73,45 @@ const AdminMemberCard = ({ member, index }) => { const handleClose = () => setOpen(false); const handleCloseDeleteConfirmation = () => setOpenDelete(false); - const options = isAdmin ? ['Edit', 'Delete'] : ['Edit']; + const options = () => { + let entries = ['Edit']; + if (isAdmin) { + entries.push('Delete'); + // If we have not already impersonated a user, we can provide that option. + if (document.cookie.indexOf("OJWT=") == -1) { + entries.push('Impersonate'); + } + } + return entries; + } const handleAction = (e, index) => { if (index === 0) { handleOpen(); } else if (index === 1) { handleOpenDeleteConfirmation(); + } else if (index === 2) { + handleImpersonate(); } }; + const handleImpersonate = async () => { + // "log in" as the chosen user with the default role. + const res = await resolve({ + method: 'POST', + url: '/impersonation/begin', + headers: { + 'X-CSRF-Header': csrf, + Accept: 'application/json', + 'Content-Type': 'application/json;charset=UTF-8' + }, + data: { email: workEmail } + }); + + // If that was successful, take the user back to the main page. + if (!res.error) window.location.href = "/"; + } + const handleDeleteMember = async () => { let res = await deleteMember(memberId, csrf); if (res && res.payload && res.payload.status === 200) { @@ -184,7 +213,7 @@ const AdminMemberCard = ({ member, index }) => { { + return document.cookie.indexOf("OJWT=") != -1; + } + return (
@@ -89,6 +94,8 @@ export default function HomePage() { )}
+ {checkForImpersonation() && + }
); } From be769de971dd2ffb0c87b8185ab75db64a10c73e Mon Sep 17 00:00:00 2001 From: Chad Elliott Date: Mon, 12 Aug 2024 12:00:50 -0500 Subject: [PATCH 2/6] Removed unecessary use of HttpHeaders. --- .../checkins/security/ImpersonationController.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/server/src/main/java/com/objectcomputing/checkins/security/ImpersonationController.java b/server/src/main/java/com/objectcomputing/checkins/security/ImpersonationController.java index f781184bdd..6654b7a1a9 100644 --- a/server/src/main/java/com/objectcomputing/checkins/security/ImpersonationController.java +++ b/server/src/main/java/com/objectcomputing/checkins/security/ImpersonationController.java @@ -6,7 +6,6 @@ import com.objectcomputing.checkins.services.permissions.Permission; import com.objectcomputing.checkins.services.permissions.RequiredPermission; import io.micronaut.http.MutableHttpResponse; -import io.micronaut.http.HttpHeaders; import io.micronaut.http.cookie.SameSite; import io.micronaut.http.cookie.Cookie; import io.micronaut.http.netty.cookies.NettyCookie; @@ -86,7 +85,6 @@ public Mono auth(HttpRequest request, String email) { if (securityService != null) { Optional auth = securityService.getAuthentication(); if (auth.isPresent() && auth.get().getAttributes().get("email") != null) { - HttpHeaders headers = request.getHeaders(); final Cookie jwt = request.getCookies().get("JWT"); if (jwt == null) { // The user is required to be logged in. If this is null, @@ -132,7 +130,6 @@ public Mono auth(HttpRequest request, String email) { @Produces(MediaType.TEXT_HTML) @Get("/end") public HttpResponse revert(HttpRequest request) { - HttpHeaders headers = request.getHeaders(); final Cookie ojwt = request.getCookies().get(originalJWT); if (ojwt == null) { return HttpResponse.status(HttpStatus.UNAUTHORIZED); From 59ac55cd349239d5c158df45909372ee00e5bdc3 Mon Sep 17 00:00:00 2001 From: Chad Elliott Date: Mon, 12 Aug 2024 12:01:28 -0500 Subject: [PATCH 3/6] Added tests for the Impersonation Controller. --- .../security/ImpersonationControllerTest.java | 135 ++++++++++++++++++ .../services/fixture/PermissionFixture.java | 3 +- 2 files changed, 137 insertions(+), 1 deletion(-) create mode 100644 server/src/test/java/com/objectcomputing/checkins/security/ImpersonationControllerTest.java diff --git a/server/src/test/java/com/objectcomputing/checkins/security/ImpersonationControllerTest.java b/server/src/test/java/com/objectcomputing/checkins/security/ImpersonationControllerTest.java new file mode 100644 index 0000000000..d420f8d353 --- /dev/null +++ b/server/src/test/java/com/objectcomputing/checkins/security/ImpersonationControllerTest.java @@ -0,0 +1,135 @@ +package com.objectcomputing.checkins.security; + +import com.objectcomputing.checkins.Environments; +import com.objectcomputing.checkins.services.TestContainersSuite; +import com.objectcomputing.checkins.services.fixture.MemberProfileFixture; +import com.objectcomputing.checkins.services.fixture.RoleFixture; +import com.objectcomputing.checkins.services.memberprofile.MemberProfile; +import com.objectcomputing.checkins.services.role.RoleType; +import io.micronaut.http.HttpRequest; +import io.micronaut.http.HttpResponse; +import io.micronaut.http.MutableHttpRequest; +import io.micronaut.http.HttpStatus; +import io.micronaut.http.MediaType; +import io.micronaut.http.cookie.Cookie; +import io.micronaut.http.client.HttpClient; +import io.micronaut.http.client.BlockingHttpClient; +import io.micronaut.http.client.annotation.Client; +import io.micronaut.http.client.exceptions.HttpClientResponseException; +import io.micronaut.test.extensions.junit5.annotation.MicronautTest; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.BeforeEach; + +import jakarta.inject.Inject; +import org.reactivestreams.Publisher; +import reactor.test.StepVerifier; + +import java.util.Map; +import java.util.Set; +import java.util.Iterator; +import org.json.JSONObject; + +import static com.objectcomputing.checkins.services.role.RoleType.Constants.ADMIN_ROLE; +import static com.objectcomputing.checkins.services.role.RoleType.Constants.MEMBER_ROLE; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +@MicronautTest(environments = {Environments.LOCAL, Environments.LOCALTEST}, transactional = false) +class ImpersonationControllerTest extends TestContainersSuite implements MemberProfileFixture, RoleFixture { + + @Client("/impersonation") + @Inject + HttpClient client; + + private MemberProfile nonAdmin; + private MemberProfile admin; + private String jwt = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXUyJ9.eyJjb21wYW55IjoiRnV0dXJlRWQiLCJzdWIiOjEsImlzcyI6Imh0dHA6XC9cL2Z1dHVyZWVkLmRldlwvYXBpXC92MVwvc3R1ZGVudFwvbG9naW5cL3VzZXJuYW1lIiwiaWF0IjoiMTQyNzQyNjc3MSIsImV4cCI6IjE0Mjc0MzAzNzEiLCJuYmYiOiIxNDI3NDI2NzcxIiwianRpIjoiNmFlZDQ3MGFiOGMxYTk0MmE0MTViYTAwOTBlMTFlZTUifQ.MmM2YTUwMjEzYTE0OGNhNjk5Y2Y2MjEwZDdkN2Y1OTQ2NWVhZTdmYmI4OTA5YmM1Y2QwYTMzZjUwNTgwY2Y0MQ"; + + @BeforeEach + void setUp() { + createAndAssignRoles(); + + nonAdmin = createADefaultMemberProfile(); + + admin = createASecondDefaultMemberProfile(); + assignAdminRole(admin); + } + + @Test + void testPostBeginEnd() { + HttpRequest> request = + HttpRequest.POST("/begin", + Map.of("email", nonAdmin.getWorkEmail())) + .contentType(MediaType.APPLICATION_FORM_URLENCODED) + .basicAuth(admin.getWorkEmail(), ADMIN_ROLE); + ((MutableHttpRequest)request).cookie(Cookie.of("JWT", jwt)); + Publisher response = client.retrieve(request); + assertNotNull(response); + final StringBuilder json = new StringBuilder(); + StepVerifier.create(response) + .thenConsumeWhile(resp -> { + assertTrue(resp.contains("\"username\":\"" + + nonAdmin.getWorkEmail())); + assertTrue(!resp.contains(jwt)); + json.append(resp); + return true; + }) + .expectComplete() + .verify(); + + JSONObject jsonObject = new JSONObject(json.toString()); + MutableHttpRequest next = HttpRequest.GET("/end") + .basicAuth(nonAdmin.getWorkEmail(), MEMBER_ROLE); + next.cookies( + Set.of(Cookie.of("OJWT", jwt), + Cookie.of("JWT", jsonObject.get("access_token").toString()))); + response = client.retrieve(next); + assertNotNull(response); + // This just needs to complete in order to verify that it has succeeded. + StepVerifier.create(response) + .thenConsumeWhile(resp -> { + return true; + }) + .expectComplete() + .verify(); + } + + @Test + void testGetEndNoOJWT() { + MutableHttpRequest request = HttpRequest.GET("/end") + .basicAuth(nonAdmin.getWorkEmail(), MEMBER_ROLE); + HttpClientResponseException response = + assertThrows(HttpClientResponseException.class, + () -> client.toBlocking().retrieve(request)); + assertNotNull(response); + assertEquals(HttpStatus.UNAUTHORIZED, response.getStatus()); + } + + @Test + void testPostUnauthorizedBegin() { + HttpRequest> request = + HttpRequest.POST("/begin", + Map.of("email", admin.getWorkEmail())) + .contentType(MediaType.APPLICATION_FORM_URLENCODED); + HttpClientResponseException response = + assertThrows(HttpClientResponseException.class, + () -> client.toBlocking().retrieve(request)); + assertNotNull(response); + assertEquals(HttpStatus.UNAUTHORIZED, response.getStatus()); + assertEquals("Unauthorized", response.getMessage()); + } + + @Test + void testGetUnauthorizedEnd() { + HttpRequest> request = + HttpRequest.GET("/end"); + HttpClientResponseException response = + assertThrows(HttpClientResponseException.class, + () -> client.toBlocking().retrieve(request)); + assertNotNull(response); + assertEquals(HttpStatus.UNAUTHORIZED, response.getStatus()); + assertEquals("Unauthorized", response.getMessage()); + } +} diff --git a/server/src/test/java/com/objectcomputing/checkins/services/fixture/PermissionFixture.java b/server/src/test/java/com/objectcomputing/checkins/services/fixture/PermissionFixture.java index b9da13fb82..e8e219f637 100644 --- a/server/src/test/java/com/objectcomputing/checkins/services/fixture/PermissionFixture.java +++ b/server/src/test/java/com/objectcomputing/checkins/services/fixture/PermissionFixture.java @@ -95,7 +95,8 @@ public interface PermissionFixture extends RolePermissionFixture { Permission.CAN_ADMINISTER_VOLUNTEERING_EVENTS, Permission.CAN_ADMINISTER_DOCUMENTATION, Permission.CAN_ADMINISTER_KUDOS, - Permission.CAN_CREATE_KUDOS + Permission.CAN_CREATE_KUDOS, + Permission.CAN_IMPERSONATE_MEMBERS ); default void setPermissionsForAdmin(UUID roleID) { From 3c4ccc687692c47a7d18e11eb8f7640d211313d4 Mon Sep 17 00:00:00 2001 From: Chad Elliott Date: Mon, 12 Aug 2024 12:10:41 -0500 Subject: [PATCH 4/6] Replace multiple uses of the "JWT" string with a constant. --- .../checkins/security/ImpersonationController.java | 6 ++++-- .../checkins/security/ImpersonationControllerTest.java | 9 ++++++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/com/objectcomputing/checkins/security/ImpersonationController.java b/server/src/main/java/com/objectcomputing/checkins/security/ImpersonationController.java index 6654b7a1a9..d4e320868c 100644 --- a/server/src/main/java/com/objectcomputing/checkins/security/ImpersonationController.java +++ b/server/src/main/java/com/objectcomputing/checkins/security/ImpersonationController.java @@ -51,6 +51,7 @@ @ExecuteOn(TaskExecutors.BLOCKING) @Secured(SecurityRule.IS_AUTHENTICATED) public class ImpersonationController { + public static final String JWT = "JWT"; public static final String originalJWT = "OJWT"; private static final Logger LOG = LoggerFactory.getLogger(ImpersonationController.class); protected final Authenticator authenticator; @@ -64,6 +65,7 @@ public class ImpersonationController { * @param loginHandler A collaborator which helps to build HTTP response depending on success or failure. * @param eventPublisher The application event publisher * @param currentUserServices Current User services + * @param securityService The Security Service */ public ImpersonationController(Authenticator authenticator, LoginHandler loginHandler, @@ -85,7 +87,7 @@ public Mono auth(HttpRequest request, String email) { if (securityService != null) { Optional auth = securityService.getAuthentication(); if (auth.isPresent() && auth.get().getAttributes().get("email") != null) { - final Cookie jwt = request.getCookies().get("JWT"); + final Cookie jwt = request.getCookies().get(JWT); if (jwt == null) { // The user is required to be logged in. If this is null, // we are in an impossible state! @@ -136,7 +138,7 @@ public HttpResponse revert(HttpRequest request) { } else { // Swap the OJWT back to the JWT and remove the original JWT Set cookies = new HashSet(); - cookies.add(new NettyCookie("JWT", ojwt.getValue()).path("/") + cookies.add(new NettyCookie(JWT, ojwt.getValue()).path("/") .sameSite(SameSite.Strict) .maxAge(ojwt.getMaxAge()).httpOnly()); cookies.add(new NettyCookie(originalJWT, "").path("/").maxAge(0)); diff --git a/server/src/test/java/com/objectcomputing/checkins/security/ImpersonationControllerTest.java b/server/src/test/java/com/objectcomputing/checkins/security/ImpersonationControllerTest.java index d420f8d353..cc8375bca5 100644 --- a/server/src/test/java/com/objectcomputing/checkins/security/ImpersonationControllerTest.java +++ b/server/src/test/java/com/objectcomputing/checkins/security/ImpersonationControllerTest.java @@ -6,6 +6,7 @@ import com.objectcomputing.checkins.services.fixture.RoleFixture; import com.objectcomputing.checkins.services.memberprofile.MemberProfile; import com.objectcomputing.checkins.services.role.RoleType; +import com.objectcomputing.checkins.security.ImpersonationController; import io.micronaut.http.HttpRequest; import io.micronaut.http.HttpResponse; import io.micronaut.http.MutableHttpRequest; @@ -64,7 +65,8 @@ void testPostBeginEnd() { Map.of("email", nonAdmin.getWorkEmail())) .contentType(MediaType.APPLICATION_FORM_URLENCODED) .basicAuth(admin.getWorkEmail(), ADMIN_ROLE); - ((MutableHttpRequest)request).cookie(Cookie.of("JWT", jwt)); + ((MutableHttpRequest)request).cookie( + Cookie.of(ImpersonationController.JWT, jwt)); Publisher response = client.retrieve(request); assertNotNull(response); final StringBuilder json = new StringBuilder(); @@ -83,8 +85,9 @@ void testPostBeginEnd() { MutableHttpRequest next = HttpRequest.GET("/end") .basicAuth(nonAdmin.getWorkEmail(), MEMBER_ROLE); next.cookies( - Set.of(Cookie.of("OJWT", jwt), - Cookie.of("JWT", jsonObject.get("access_token").toString()))); + Set.of(Cookie.of(ImpersonationController.originalJWT, jwt), + Cookie.of(ImpersonationController.JWT, + jsonObject.get("access_token").toString()))); response = client.retrieve(next); assertNotNull(response); // This just needs to complete in order to verify that it has succeeded. From 2af9ff25c55bf6c112ff68fda09889f1cc44e3f0 Mon Sep 17 00:00:00 2001 From: Chad Elliott Date: Mon, 12 Aug 2024 13:28:05 -0500 Subject: [PATCH 5/6] Removed redundant @ExecuteOn, specify type in HttpResponse and use the unauthorized() method on HttpResponse. --- .../checkins/security/ImpersonationController.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/com/objectcomputing/checkins/security/ImpersonationController.java b/server/src/main/java/com/objectcomputing/checkins/security/ImpersonationController.java index d4e320868c..32f0234f45 100644 --- a/server/src/main/java/com/objectcomputing/checkins/security/ImpersonationController.java +++ b/server/src/main/java/com/objectcomputing/checkins/security/ImpersonationController.java @@ -14,7 +14,6 @@ import io.micronaut.context.event.ApplicationEventPublisher; import io.micronaut.http.HttpRequest; import io.micronaut.http.HttpResponse; -import io.micronaut.http.HttpStatus; import io.micronaut.http.MediaType; import io.micronaut.http.annotation.Consumes; import io.micronaut.http.annotation.Produces; @@ -79,7 +78,6 @@ public ImpersonationController(Authenticator authenticator, this.securityService = securityService; } - @ExecuteOn(TaskExecutors.BLOCKING) @Consumes({MediaType.APPLICATION_FORM_URLENCODED, MediaType.APPLICATION_JSON}) @Post("/begin") @RequiredPermission(Permission.CAN_IMPERSONATE_MEMBERS) @@ -119,7 +117,7 @@ public Mono auth(HttpRequest request, String email) { eventPublisher.publishEvent(new LoginFailedEvent(authenticationResponse, null, null, Locale.getDefault())); return loginHandler.loginFailed(authenticationResponse, request); } - }).single(Mono.just(HttpResponse.status(HttpStatus.UNAUTHORIZED))); + }).single(Mono.just(HttpResponse.unauthorized())); } } else { LOG.error("Attempted impersonation without authentication."); @@ -128,13 +126,12 @@ public Mono auth(HttpRequest request, String email) { return null; } - @ExecuteOn(TaskExecutors.BLOCKING) @Produces(MediaType.TEXT_HTML) @Get("/end") - public HttpResponse revert(HttpRequest request) { + public HttpResponse revert(HttpRequest request) { final Cookie ojwt = request.getCookies().get(originalJWT); if (ojwt == null) { - return HttpResponse.status(HttpStatus.UNAUTHORIZED); + return HttpResponse.unauthorized(); } else { // Swap the OJWT back to the JWT and remove the original JWT Set cookies = new HashSet(); From 5c987b1b470ae977f4079dca1f34d5f77ed88f50 Mon Sep 17 00:00:00 2001 From: Chad Elliott Date: Tue, 13 Aug 2024 07:08:26 -0500 Subject: [PATCH 6/6] Use the Micronaut DEVELOPMENT environment and return a server error instead of null. --- .../checkins/security/ImpersonationController.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/com/objectcomputing/checkins/security/ImpersonationController.java b/server/src/main/java/com/objectcomputing/checkins/security/ImpersonationController.java index 32f0234f45..31424b97af 100644 --- a/server/src/main/java/com/objectcomputing/checkins/security/ImpersonationController.java +++ b/server/src/main/java/com/objectcomputing/checkins/security/ImpersonationController.java @@ -5,6 +5,7 @@ import com.objectcomputing.checkins.services.memberprofile.currentuser.CurrentUserServices; import com.objectcomputing.checkins.services.permissions.Permission; import com.objectcomputing.checkins.services.permissions.RequiredPermission; +import io.micronaut.context.env.Environment; import io.micronaut.http.MutableHttpResponse; import io.micronaut.http.cookie.SameSite; import io.micronaut.http.cookie.Cookie; @@ -45,7 +46,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -@Requires(env = Environments.LOCAL) +@Requires(env = {Environments.LOCAL, Environment.DEVELOPMENT}) @Controller("/impersonation") @ExecuteOn(TaskExecutors.BLOCKING) @Secured(SecurityRule.IS_AUTHENTICATED) @@ -123,7 +124,7 @@ public Mono auth(HttpRequest request, String email) { LOG.error("Attempted impersonation without authentication."); } } - return null; + return Mono.just(HttpResponse.unauthorized()); } @Produces(MediaType.TEXT_HTML)