Merge pull request #1186 from softwaremill/invalidate-session

Invalidate session on password change/logout

backend/src/main/scala/com/softwaremill/bootzooka/security/ApiKeyModel.scala

@@ -22,6 +22,10 @@ class ApiKeyModel {
       .option
   }
 
+  def deleteAllForUser(id: Id @@ User): ConnectionIO[Unit] = {
+    sql"""DELETE FROM api_keys WHERE user_id = $id""".update.run.void
+  }
+
   def delete(id: Id @@ ApiKey): ConnectionIO[Unit] = {
     sql"""DELETE FROM api_keys WHERE id = $id""".update.run.void
   }

backend/src/main/scala/com/softwaremill/bootzooka/security/ApiKeyService.scala

@@ -21,6 +21,18 @@ class ApiKeyService(apiKeyModel: ApiKeyModel, idGenerator: IdGenerator, clock: C
       _ <- logger.debug[ConnectionIO](s"Creating a new api key for user $userId, valid until: $validUntil")
       _ <- apiKeyModel.insert(apiKey)
     } yield apiKey
+
+  def invalidate(id: Id @@ ApiKey): ConnectionIO[Unit] =
+    for {
+      _ <- logger.debug[ConnectionIO](s"Invalidating api key $id")
+      _ <- apiKeyModel.delete(id)
+    } yield ()
+
+  def invalidateAllForUser(userId: Id @@ User): ConnectionIO[Unit] =
+    for {
+      _ <- logger.debug[ConnectionIO](s"Invalidating all api keys for user $userId")
+      _ <- apiKeyModel.deleteAllForUser(userId)
+    } yield ()
 }
 
 case class ApiKey(id: Id @@ ApiKey, userId: Id @@ User, createdOn: Instant, validUntil: Instant)

backend/src/main/scala/com/softwaremill/bootzooka/user/UserApi.scala

@@ -7,7 +7,7 @@ import com.softwaremill.bootzooka.infrastructure.Doobie._
 import com.softwaremill.bootzooka.infrastructure.Json._
 import com.softwaremill.bootzooka.metrics.Metrics
 import com.softwaremill.bootzooka.security.{ApiKey, Auth}
-import com.softwaremill.bootzooka.util.ServerEndpoints
+import com.softwaremill.bootzooka.util.{Id, RichString, ServerEndpoints}
 import doobie.util.transactor.Transactor
 
 import java.time.Instant
@@ -44,15 +44,28 @@ class UserApi(http: Http, auth: Auth[ApiKey], userService: UserService, xa: Tran
 
   private val authedEndpoint = secureEndpoint.serverSecurityLogic(authData => auth(authData).toOut)
 
+  private val logoutEndpoint = authedEndpoint.post
+    .in(UserPath / "logout")
+    .in(jsonBody[Logout_IN])
+    .out(jsonBody[Logout_OUT])
+    .serverLogic(_ =>
+      data =>
+        (for {
+          _ <- userService
+            .logout(data.apiKey.asId[ApiKey])
+            .transact(xa)
+        } yield Logout_OUT()).toOut
+    )
+
   private val changePasswordEndpoint = authedEndpoint.post
     .in(UserPath / "changepassword")
     .in(jsonBody[ChangePassword_IN])
     .out(jsonBody[ChangePassword_OUT])
     .serverLogic(id =>
       data =>
         (for {
-          _ <- userService.changePassword(id, data.currentPassword, data.newPassword).transact(xa)
-        } yield ChangePassword_OUT()).toOut
+          apiKey <- userService.changePassword(id, data.currentPassword, data.newPassword).transact(xa)
+        } yield ChangePassword_OUT(apiKey.id)).toOut
     )
 
   private val getUserEndpoint = authedEndpoint.get
@@ -81,6 +94,7 @@ class UserApi(http: Http, auth: Auth[ApiKey], userService: UserService, xa: Tran
       .of(
         registerUserEndpoint,
         loginEndpoint,
+        logoutEndpoint,
         changePasswordEndpoint,
         getUserEndpoint,
         updateUserEndpoint
@@ -93,11 +107,14 @@ object UserApi {
   case class Register_OUT(apiKey: String)
 
   case class ChangePassword_IN(currentPassword: String, newPassword: String)
-  case class ChangePassword_OUT()
+  case class ChangePassword_OUT(apiKey: String)
 
   case class Login_IN(loginOrEmail: String, password: String, apiKeyValidHours: Option[Int])
   case class Login_OUT(apiKey: String)
 
+  case class Logout_IN(apiKey: String)
+  case class Logout_OUT()
+
   case class UpdateUser_IN(login: String, email: String)
   case class UpdateUser_OUT()
 

backend/src/main/scala/com/softwaremill/bootzooka/user/UserService.scala

@@ -71,6 +71,8 @@ class UserService(
     } yield apiKey
   }
 
+  def logout(id: Id @@ ApiKey): ConnectionIO[Unit] = apiKeyService.invalidate(id)
+
   def changeUser(userId: Id @@ User, newLogin: String, newEmail: String): ConnectionIO[Unit] = {
     val newLoginClean = newLogin.trim()
     val newEmailClean = newEmail.trim()
@@ -130,19 +132,39 @@ class UserService(
     }
   }
 
-  def changePassword(userId: Id @@ User, currentPassword: String, newPassword: String): ConnectionIO[Unit] = {
-    def validateNewPassword() =
+  def changePassword(userId: Id @@ User, currentPassword: String, newPassword: String): ConnectionIO[ApiKey] = {
+    def validateUserPassword(userId: Id @@ User, currentPassword: String): ConnectionIO[User] = {
+      for {
+        user <- userOrNotFound(userModel.findById(userId))
+        _ <- verifyPassword(user, currentPassword, validationErrorMsg = "Incorrect current password")
+      } yield user
+    }
+
+    def validateNewPassword(): ConnectionIO[Unit] =
       UserValidator(None, None, Some(newPassword)).as[ConnectionIO]
 
+    def updateUserPassword(user: User, newPassword: String): ConnectionIO[Unit] = {
+      for {
+        _ <- logger.debug[ConnectionIO](s"Changing password for user: ${user.id}")
+        _ <- userModel.updatePassword(user.id, User.hashPassword(newPassword))
+        confirmationEmail = emailTemplates.passwordChangeNotification(user.login)
+        _ <- emailScheduler(EmailData(user.emailLowerCased, confirmationEmail))
+      } yield ()
+    }
+
+    def invalidateKeysAndCreateNew(user: User): ConnectionIO[ApiKey] = {
+      for {
+        _ <- apiKeyService.invalidateAllForUser(user.id)
+        apiKey <- apiKeyService.create(user.id, config.defaultApiKeyValid)
+      } yield apiKey
+    }
+
     for {
-      user <- userOrNotFound(userModel.findById(userId))
-      _ <- verifyPassword(user, currentPassword, validationErrorMsg = "Incorrect current password")
+      user <- validateUserPassword(userId, currentPassword)
       _ <- validateNewPassword()
-      _ <- logger.debug[ConnectionIO](s"Changing password for user: $userId")
-      _ <- userModel.updatePassword(userId, User.hashPassword(newPassword))
-      confirmationEmail = emailTemplates.passwordChangeNotification(user.login)
-      _ <- emailScheduler(EmailData(user.emailLowerCased, confirmationEmail))
-    } yield ()
+      _ <- updateUserPassword(user, newPassword)
+      apiKey <- invalidateKeysAndCreateNew(user)
+    } yield apiKey
   }
 
   private def userOrNotFound(op: ConnectionIO[Option[User]]): ConnectionIO[User] = {

backend/src/test/scala/com/softwaremill/bootzooka/test/Requests.scala

@@ -41,6 +41,15 @@ class Requests(backend: SttpBackend[IO, Any]) extends TestSupport {
       .unwrap
   }
 
+  def logoutUser(apiKey: String): Response[Either[String, String]] = {
+    basicRequest
+      .post(uri"$basePath/user/logout")
+      .body(Logout_IN(apiKey).asJson.noSpaces)
+      .header("Authorization", s"Bearer $apiKey")
+      .send(backend)
+      .unwrap
+  }
+
   def getUser(apiKey: String): client3.Response[Either[String, String]] = {
     basicRequest
       .get(uri"$basePath/user")

backend/src/test/scala/com/softwaremill/bootzooka/user/UserApiTest.scala

@@ -168,6 +168,23 @@ class UserApiTest extends BaseTest with Eventually with TestDependencies with Te
     requests.getUser("invalid").code shouldBe StatusCode.Unauthorized
   }
 
+  "/user/logout" should "logout the user" in {
+    // given
+    val RegisteredUser(_, _, _, apiKey) = requests.newRegisteredUsed()
+
+    // when
+    val response = requests.logoutUser(apiKey)
+
+    // then
+    response.code shouldBe StatusCode.Ok
+
+    // when
+    val responseAfterLogout = requests.getUser(apiKey)
+
+    // then
+    responseAfterLogout.code shouldBe StatusCode.Unauthorized
+  }
+
   "/user/changepassword" should "change the password" in {
     // given
     val RegisteredUser(login, _, password, apiKey) = requests.newRegisteredUsed()
@@ -182,6 +199,24 @@ class UserApiTest extends BaseTest with Eventually with TestDependencies with Te
     requests.loginUser(login, newPassword, None).code shouldBe StatusCode.Ok
   }
 
+  "/user/changepassword" should "create new session and invalidate all existing user's sessions" in {
+    // given
+    val RegisteredUser(login, _, password, apiKey1) = requests.newRegisteredUsed()
+    val newPassword = password + password
+
+    // login again to create another session
+    val apiKey2 = requests.loginUser(login, password, Some(3)).body.shouldDeserializeTo[Login_OUT].apiKey
+
+    // when
+    val response = requests.changePassword(apiKey1, password, newPassword)
+
+    // then
+    val newApiKey = response.body.shouldDeserializeTo[ChangePassword_OUT].apiKey
+    requests.getUser(newApiKey).code shouldBe StatusCode.Ok
+    requests.getUser(apiKey1).code shouldBe StatusCode.Unauthorized
+    requests.getUser(apiKey2).code shouldBe StatusCode.Unauthorized
+  }
+
   "/user/changepassword" should "not change the password if the current is invalid" in {
     // given
     val RegisteredUser(login, _, password, apiKey) = requests.newRegisteredUsed()

ui/src/main/Top/Top.test.tsx

@@ -1,23 +1,27 @@
-import { render, screen } from "@testing-library/react";
+import { screen } from "@testing-library/react";
 import { userEvent } from "@testing-library/user-event";
 import { MemoryRouter } from "react-router-dom";
 import { UserContext, UserState, initialUserState } from "contexts";
 import { Top } from "./Top";
+import { userService } from "../../services";
+import { renderWithClient } from "../../tests";
 
 const loggedUserState: UserState = {
   apiKey: "test-api-key",
   user: { login: "user-login", email: "[email protected]", createdOn: "2020-10-09T09:57:17.995288Z" },
   loggedIn: true,
 };
 
+jest.mock("services");
+
 const dispatch = jest.fn();
 
 beforeEach(() => {
   jest.clearAllMocks();
 });
 
 test("renders brand name", () => {
-  render(
+  renderWithClient(
     <MemoryRouter initialEntries={[""]}>
       <UserContext.Provider value={{ state: initialUserState, dispatch }}>
         <Top />
@@ -29,7 +33,7 @@ test("renders brand name", () => {
 });
 
 test("renders nav bar unlogged user", () => {
-  render(
+  renderWithClient(
     <MemoryRouter initialEntries={["/main"]}>
       <UserContext.Provider value={{ state: initialUserState, dispatch }}>
         <Top />
@@ -44,7 +48,7 @@ test("renders nav bar unlogged user", () => {
 });
 
 test("renders nav bar for logged user", () => {
-  render(
+  renderWithClient(
     <MemoryRouter initialEntries={["/main"]}>
       <UserContext.Provider value={{ state: loggedUserState, dispatch }}>
         <Top />
@@ -59,7 +63,9 @@ test("renders nav bar for logged user", () => {
 });
 
 test("handles logout logged user", async () => {
-  render(
+  (userService.logout as jest.Mock).mockResolvedValueOnce({});
+
+  renderWithClient(
     <MemoryRouter initialEntries={["/main"]}>
       <UserContext.Provider value={{ state: loggedUserState, dispatch }}>
         <Top />

ui/src/main/Top/Top.tsx

@@ -5,14 +5,18 @@ import Container from "react-bootstrap/Container";
 import { Link } from "react-router-dom";
 import { BiPowerOff, BiHappy } from "react-icons/bi";
 import { UserContext } from "contexts";
+import { useMutation } from "react-query";
+import { userService } from "../../services";
 
 export const Top: React.FC = () => {
   const {
-    state: { user, loggedIn },
+    state: { user, loggedIn, apiKey },
     dispatch,
   } = React.useContext(UserContext);
 
-  const handleLogOut = () => dispatch({ type: "LOG_OUT" });
+  const handleLogOut = useMutation(() => userService.logout(apiKey), {
+    onSuccess: () => dispatch({ type: "LOG_OUT" }),
+  });
 
   return (
     <Navbar variant="dark" bg="dark" sticky="top" collapseOnSelect expand="lg">
@@ -36,7 +40,7 @@ export const Top: React.FC = () => {
                   <BiHappy />
                   &nbsp;{user?.login}
                 </Nav.Link>{" "}
-                <Nav.Link className="text-lg-end" onClick={handleLogOut}>
+                <Nav.Link className="text-lg-end" onClick={() => handleLogOut.mutate()}>
                   <BiPowerOff />
                   &nbsp;Logout
                 </Nav.Link>

ui/src/pages/Profile/components/PasswordDetails.test.tsx

@@ -29,7 +29,8 @@ test("renders header", () => {
 });
 
 test("handles change password success", async () => {
-  (userService.changePassword as jest.Mock).mockResolvedValueOnce({});
+  const apiKey = "test-api-key";
+  (userService.changePassword as jest.Mock).mockResolvedValueOnce({ apiKey });
 
   renderWithClient(
     <UserContext.Provider value={{ state: mockState, dispatch }}>
@@ -49,6 +50,7 @@ test("handles change password success", async () => {
     currentPassword: "test-password",
     newPassword: "test-new-password",
   });
+  expect(dispatch).toHaveBeenCalledWith({ apiKey: "test-api-key", type: "SET_API_KEY" });
 });
 
 test("handles change password error", async () => {

ui/src/pages/Profile/components/PasswordDetails.tsx

@@ -24,12 +24,21 @@ type PasswordDetailsParams = Yup.InferType<typeof validationSchema>;
 export const PasswordDetails: React.FC = () => {
   const {
     state: { apiKey },
+    dispatch,
   } = React.useContext(UserContext);
 
-  const mutation = useMutation(({ currentPassword, newPassword }: PasswordDetailsParams) =>
-    userService.changePassword(apiKey, { currentPassword, newPassword }),
+  const mutation = useMutation(
+    ({ currentPassword, newPassword }: PasswordDetailsParams) =>
+      userService.changePassword(apiKey, { currentPassword, newPassword }),
+    {
+      onSuccess: ({ apiKey }) => dispatch({ type: "SET_API_KEY", apiKey }),
+    },
   );
 
+  React.useEffect(() => {
+    localStorage.setItem("apiKey", apiKey || "");
+  }, [apiKey]);
+
   return (
     <Container className="py-5">
       <Row>

ui/src/services/UserService/UserService.test.ts

@@ -27,6 +27,21 @@ test("logs in user", async () => {
   expect(axios.post).toHaveBeenCalledWith("api/v1/user/login", { ...params, apiKeyValidHours: 1 });
 });
 
+test("logs out user", async () => {
+  const data = {};
+  const testApiKey = "test-api-key";
+
+  (axios.request as jest.Mock).mockResolvedValueOnce({ data });
+
+  await expect(userService.logout(testApiKey)).resolves.toEqual(data);
+  expect(axios.request).toHaveBeenCalledWith({
+    headers: { Authorization: `Bearer ${testApiKey}` },
+    method: "POST",
+    url: "api/v1/user/logout",
+    data: { apiKey: testApiKey },
+  });
+});
+
 test("gets current user", async () => {
   const data = { login: "test-login", email: "test-email", createdOn: "test-date" };
   const testApiKey = "test-api-key";
@@ -58,7 +73,7 @@ test("changes profile details", async () => {
 });
 
 test("changes password", async () => {
-  const data = {};
+  const data = { apiKey: "new-api-key" };
   const testApiKey = "test-api-key";
   const params = { currentPassword: "test-current-password", newPassword: "test-new-password" };