From c6035aaa311d6125f862ddef9b8dabb227110dd4 Mon Sep 17 00:00:00 2001 From: Bryan Phelps Date: Sat, 22 Jan 2022 01:34:24 +0000 Subject: [PATCH 01/14] Initial nav menu + user dropdown --- site/components/Icons/Logout.tsx | 31 ++++++ site/components/Icons/index.ts | 1 + site/components/Navbar/BorderedMenu.tsx | 31 ++++++ site/components/Navbar/UserDropdown.tsx | 131 +++++++++++++++++++++++ site/components/Navbar/index.tsx | 13 +-- site/components/User/UserProfileCard.tsx | 58 ++++++++++ 6 files changed, 256 insertions(+), 9 deletions(-) create mode 100644 site/components/Icons/Logout.tsx create mode 100644 site/components/Navbar/BorderedMenu.tsx create mode 100644 site/components/Navbar/UserDropdown.tsx create mode 100644 site/components/User/UserProfileCard.tsx diff --git a/site/components/Icons/Logout.tsx b/site/components/Icons/Logout.tsx new file mode 100644 index 0000000000000..d4024e73b07c1 --- /dev/null +++ b/site/components/Icons/Logout.tsx @@ -0,0 +1,31 @@ +import SvgIcon, { SvgIconProps } from "@material-ui/core/SvgIcon" +import React from "react" + +export const LogoutIcon = (props: SvgIconProps): JSX.Element => ( + + + + + + +) diff --git a/site/components/Icons/index.ts b/site/components/Icons/index.ts index c135b8db26e79..17f48ebaa4a98 100644 --- a/site/components/Icons/index.ts +++ b/site/components/Icons/index.ts @@ -1,3 +1,4 @@ export { CoderIcon } from "./CoderIcon" export { Logo } from "./Logo" +export * from "./Logout" export { WorkspacesIcon } from "./WorkspacesIcon" diff --git a/site/components/Navbar/BorderedMenu.tsx b/site/components/Navbar/BorderedMenu.tsx new file mode 100644 index 0000000000000..1228d530673a7 --- /dev/null +++ b/site/components/Navbar/BorderedMenu.tsx @@ -0,0 +1,31 @@ +import Popover, { PopoverProps } from "@material-ui/core/Popover" +import { fade, makeStyles } from "@material-ui/core/styles" +import React from "react" + +type BorderedMenuVariant = "manage-dropdown" | "user-dropdown" + +type BorderedMenuProps = Omit & { + variant?: BorderedMenuVariant +} + +export const BorderedMenu: React.FC = ({ children, variant, ...rest }) => { + const styles = useStyles() + + return ( + + {children} + + ) +} + +const useStyles = makeStyles((theme) => ({ + root: { + paddingBottom: theme.spacing(1), + }, + paperRoot: { + width: "292px", + border: `2px solid ${theme.palette.primary.main}`, + borderRadius: 7, + boxShadow: `4px 4px 0px ${fade(theme.palette.primary.main, 0.2)}`, + }, +})) diff --git a/site/components/Navbar/UserDropdown.tsx b/site/components/Navbar/UserDropdown.tsx new file mode 100644 index 0000000000000..5ae23bcef902b --- /dev/null +++ b/site/components/Navbar/UserDropdown.tsx @@ -0,0 +1,131 @@ +import Avatar from "@material-ui/core/Avatar" +import Badge from "@material-ui/core/Badge" +import ListItemIcon from "@material-ui/core/ListItemIcon" +import ListItemText from "@material-ui/core/ListItemText" +import MenuItem from "@material-ui/core/MenuItem" +import { fade, makeStyles } from "@material-ui/core/styles" +//import AccountIcon from "@material-ui/icons/AccountCircleOutlined" +import KeyboardArrowDown from "@material-ui/icons/KeyboardArrowDown" +import KeyboardArrowUp from "@material-ui/icons/KeyboardArrowUp" +import React, { useState } from "react" +import { LogoutIcon } from "../Icons" +import { BorderedMenu } from "./BorderedMenu" +import { UserProfileCard } from "../User/UserProfileCard" + +import { User } from "../../contexts/UserContext" +import Divider from "@material-ui/core/Divider" + +const navHeight = 56 + +export interface UserDropdownProps { + user: User + onSignOut: () => void +} + +export const UserDropdown: React.FC = ({ user, onSignOut }: UserDropdownProps) => { + const styles = useStyles() + + const [anchorEl, setAnchorEl] = useState() + const handleDropdownClick = (ev: React.MouseEvent): void => { + setAnchorEl(ev.currentTarget) + } + const onPopoverClose = () => { + setAnchorEl(undefined) + } + + // TODO: what does this do? + const isSelected = false + + return ( + <> +
+ +
+ {user && ( + + T + + )} + {anchorEl ? ( + + ) : ( + + )} +
+ +
+ + + {user && ( +
+ + + + + + + + + + +
+ )} + + + ) +} + +export const useStyles = makeStyles((theme) => ({ + divider: { + marginTop: theme.spacing(1), + marginBottom: theme.spacing(1), + }, + inner: { + display: "flex", + alignItems: "center", + minWidth: 0, + maxWidth: 300, + }, + + userInfo: { + marginBottom: theme.spacing(1), + }, + arrowIcon: { + color: fade(theme.palette.primary.contrastText, 0.7), + marginLeft: theme.spacing(1), + width: 16, + height: 16, + }, + arrowIconUp: { + color: theme.palette.primary.contrastText, + }, + + menuItem: { + height: 44, + padding: `${theme.spacing(1.5)}px ${theme.spacing(2.75)}px`, + + "&:hover": { + backgroundColor: fade(theme.palette.primary.light, 0.1), + transition: "background-color 0.3s ease", + }, + }, + + icon: { + color: theme.palette.text.secondary, + }, +})) diff --git a/site/components/Navbar/index.tsx b/site/components/Navbar/index.tsx index b9fd1e3d95324..3ff1848d807a3 100644 --- a/site/components/Navbar/index.tsx +++ b/site/components/Navbar/index.tsx @@ -7,12 +7,13 @@ import Link from "next/link" import { User } from "../../contexts/UserContext" import { Logo } from "../Icons" +import { UserDropdown } from "./UserDropdown" export interface NavbarProps { user?: User } -export const Navbar: React.FC = () => { +export const Navbar: React.FC = ({ user }) => { const styles = useStyles() return (
@@ -23,14 +24,8 @@ export const Navbar: React.FC = () => {
-
-
Coder v2
-
-
- - Manage - -
+
+
{user && alert("sign out")} />}
) } diff --git a/site/components/User/UserProfileCard.tsx b/site/components/User/UserProfileCard.tsx new file mode 100644 index 0000000000000..772dbd7057953 --- /dev/null +++ b/site/components/User/UserProfileCard.tsx @@ -0,0 +1,58 @@ +import Avatar from "@material-ui/core/Avatar" +import { makeStyles } from "@material-ui/core/styles" +import Typography from "@material-ui/core/Typography" +import React from "react" + +import { User } from "../../contexts/UserContext" + +interface UserProfileCardProps { + user: User +} + +export const UserProfileCard: React.FC = ({ user }) => { + const styles = useStyles() + + return ( +
+
+ T +
+ {user.username} + {user.email} +
+ ) +} + +const useStyles = makeStyles((theme) => ({ + root: { + paddingTop: theme.spacing(3), + textAlign: "center", + }, + avatarContainer: { + width: "100%", + display: "flex", + alignItems: "center", + justifyContent: "center", + }, + avatar: { + width: 48, + height: 48, + borderRadius: "50%", + marginBottom: theme.spacing(1), + transition: `transform .2s`, + + "&:hover": { + transform: `scale(1.1)`, + }, + }, + userName: { + fontSize: 16, + marginBottom: theme.spacing(0.5), + }, + userEmail: { + fontSize: 14, + letterSpacing: 0.2, + color: theme.palette.text.secondary, + marginBottom: theme.spacing(1.5), + }, +})) From 16a7e575d4001f574f9471d61710ee1afd9c8319 Mon Sep 17 00:00:00 2001 From: Bryan Phelps Date: Sat, 22 Jan 2022 01:43:07 +0000 Subject: [PATCH 02/14] Initial logout route --- coderd/coderd.go | 1 + coderd/users.go | 15 +++++++++++++++ 2 files changed, 16 insertions(+) diff --git a/coderd/coderd.go b/coderd/coderd.go index 16a69a918d683..0be9474b69a0b 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -33,6 +33,7 @@ func New(options *Options) http.Handler { }) r.Post("/user", users.createInitialUser) r.Post("/login", users.loginWithPassword) + r.Post("/logout", logout) // Require an API key and authenticated user for this group. r.Group(func(r chi.Router) { r.Use( diff --git a/coderd/users.go b/coderd/users.go index b5f4004d70f1e..55e36fe116dfa 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -199,6 +199,21 @@ func (users *users) loginWithPassword(rw http.ResponseWriter, r *http.Request) { }) } +// Clear the user's session cookie +func logout(rw http.ResponseWriter, r *http.Request) { + // Get a blank token cookie + cookie := &http.Cookie{ + // MaxAge < 0 means to delete the cookie now + MaxAge: -1, + Name: httpmw.AuthCookie, + Path: "/", + } + + http.SetCookie(rw, cookie) + + render.Status(r, http.StatusOK) +} + // Generates a new ID and secret for an API key. func generateAPIKeyIDSecret() (id string, secret string, err error) { // Length of an API Key ID. From cb6ad184d7bdd8fa4227e38941eada3978c7ef9e Mon Sep 17 00:00:00 2001 From: Bryan Phelps Date: Sat, 22 Jan 2022 01:43:22 +0000 Subject: [PATCH 03/14] API handler for logout --- site/api.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/site/api.ts b/site/api.ts index 82d3303d45988..6677c9495bbc2 100644 --- a/site/api.ts +++ b/site/api.ts @@ -14,6 +14,18 @@ export const login = async (email: string, password: string): Promise => { + const response = await fetch("/api/v2/logout", { + method: "POST", + }) + + return await readOrThrowResponse(response) +} + +const readOrThrowResponse = async (response: Response): Promise => { const body = await response.json() if (!response.ok) { throw new Error(body.message) From b5eec09669059ac0122743020572ad1352676c98 Mon Sep 17 00:00:00 2001 From: Bryan Phelps Date: Sat, 22 Jan 2022 02:02:49 +0000 Subject: [PATCH 04/14] Fix lint issues --- site/api.ts | 15 ++++++++------- site/components/Navbar/UserDropdown.tsx | 10 ++-------- site/components/Navbar/index.tsx | 7 +++---- site/contexts/UserContext.tsx | 25 +++++++++++++++++++++++-- site/pages/index.tsx | 4 ++-- 5 files changed, 38 insertions(+), 23 deletions(-) diff --git a/site/api.ts b/site/api.ts index 6677c9495bbc2..aece323b14aaf 100644 --- a/site/api.ts +++ b/site/api.ts @@ -14,7 +14,12 @@ export const login = async (email: string, password: string): Promise => { @@ -22,14 +27,10 @@ export const logout = async (): Promise => { method: "POST", }) - return await readOrThrowResponse(response) -} - -const readOrThrowResponse = async (response: Response): Promise => { - const body = await response.json() if (!response.ok) { + const body = await response.json() throw new Error(body.message) } - return body + return } diff --git a/site/components/Navbar/UserDropdown.tsx b/site/components/Navbar/UserDropdown.tsx index 5ae23bcef902b..23ec5acb1c25d 100644 --- a/site/components/Navbar/UserDropdown.tsx +++ b/site/components/Navbar/UserDropdown.tsx @@ -4,7 +4,6 @@ import ListItemIcon from "@material-ui/core/ListItemIcon" import ListItemText from "@material-ui/core/ListItemText" import MenuItem from "@material-ui/core/MenuItem" import { fade, makeStyles } from "@material-ui/core/styles" -//import AccountIcon from "@material-ui/icons/AccountCircleOutlined" import KeyboardArrowDown from "@material-ui/icons/KeyboardArrowDown" import KeyboardArrowUp from "@material-ui/icons/KeyboardArrowUp" import React, { useState } from "react" @@ -15,8 +14,6 @@ import { UserProfileCard } from "../User/UserProfileCard" import { User } from "../../contexts/UserContext" import Divider from "@material-ui/core/Divider" -const navHeight = 56 - export interface UserDropdownProps { user: User onSignOut: () => void @@ -33,13 +30,10 @@ export const UserDropdown: React.FC = ({ user, onSignOut }: U setAnchorEl(undefined) } - // TODO: what does this do? - const isSelected = false - return ( <>
- +
{user && ( @@ -73,7 +67,7 @@ export const UserDropdown: React.FC = ({ user, onSignOut }: U > {user && (
- + diff --git a/site/components/Navbar/index.tsx b/site/components/Navbar/index.tsx index 3ff1848d807a3..01100e40eb842 100644 --- a/site/components/Navbar/index.tsx +++ b/site/components/Navbar/index.tsx @@ -1,7 +1,5 @@ import React from "react" import Button from "@material-ui/core/Button" -import List from "@material-ui/core/List" -import ListSubheader from "@material-ui/core/ListSubheader" import { makeStyles } from "@material-ui/core/styles" import Link from "next/link" @@ -11,9 +9,10 @@ import { UserDropdown } from "./UserDropdown" export interface NavbarProps { user?: User + onSignOut: () => void } -export const Navbar: React.FC = ({ user }) => { +export const Navbar: React.FC = ({ user, onSignOut }) => { const styles = useStyles() return (
@@ -25,7 +24,7 @@ export const Navbar: React.FC = ({ user }) => {
-
{user && alert("sign out")} />}
+
{user && }
) } diff --git a/site/contexts/UserContext.tsx b/site/contexts/UserContext.tsx index 544d5dd835701..b5c128067513b 100644 --- a/site/contexts/UserContext.tsx +++ b/site/contexts/UserContext.tsx @@ -2,6 +2,8 @@ import { useRouter } from "next/router" import React, { useContext, useEffect } from "react" import useSWR from "swr" +import * as API from "../api" + export interface User { readonly id: string readonly username: string @@ -12,9 +14,14 @@ export interface User { export interface UserContext { readonly error?: Error readonly me?: User + readonly signOut: () => Promise } -const UserContext = React.createContext({}) +const UserContext = React.createContext({ + signOut: () => { + return Promise.reject("Sign out API not available") + }, +}) export const useUser = (redirectOnError = false): UserContext => { const ctx = useContext(UserContext) @@ -36,13 +43,27 @@ export const useUser = (redirectOnError = false): UserContext => { } export const UserProvider: React.FC = (props) => { - const { data, error } = useSWR("/api/v2/user") + const router = useRouter() + const { data, error, mutate } = useSWR("/api/v2/user") + + const signOut = async () => { + await API.logout() + // Tell SWR to invalidate the cache for the user endpoint + mutate("/api/v2/user") + router.push({ + pathname: "/login", + query: { + redirect: router.asPath, + }, + }) + } return ( {props.children} diff --git a/site/pages/index.tsx b/site/pages/index.tsx index 5780010f734e7..0de405b8a74fb 100644 --- a/site/pages/index.tsx +++ b/site/pages/index.tsx @@ -12,7 +12,7 @@ import { FullScreenLoader } from "../components/Loader/FullScreenLoader" const WorkspacesPage: React.FC = () => { const styles = useStyles() - const { me } = useUser(true) + const { me, signOut } = useUser(true) if (!me) { return @@ -29,7 +29,7 @@ const WorkspacesPage: React.FC = () => { return (
- +
color="primary" From 5c11f7cc1a22b377255c72dd88d95521b0a6ed78 Mon Sep 17 00:00:00 2001 From: Bryan Phelps Date: Sat, 22 Jan 2022 02:44:48 +0000 Subject: [PATCH 05/14] Fix lint / formatting --- site/components/Navbar/UserDropdown.tsx | 6 +++--- site/components/User/UserAvatar.tsx | 25 ++++++++++++++++++++++++ site/components/User/UserProfileCard.tsx | 4 ++-- site/components/User/index.ts | 2 ++ 4 files changed, 32 insertions(+), 5 deletions(-) create mode 100644 site/components/User/UserAvatar.tsx create mode 100644 site/components/User/index.ts diff --git a/site/components/Navbar/UserDropdown.tsx b/site/components/Navbar/UserDropdown.tsx index 23ec5acb1c25d..be002c4741f15 100644 --- a/site/components/Navbar/UserDropdown.tsx +++ b/site/components/Navbar/UserDropdown.tsx @@ -1,5 +1,5 @@ -import Avatar from "@material-ui/core/Avatar" import Badge from "@material-ui/core/Badge" +import Divider from "@material-ui/core/Divider" import ListItemIcon from "@material-ui/core/ListItemIcon" import ListItemText from "@material-ui/core/ListItemText" import MenuItem from "@material-ui/core/MenuItem" @@ -12,7 +12,7 @@ import { BorderedMenu } from "./BorderedMenu" import { UserProfileCard } from "../User/UserProfileCard" import { User } from "../../contexts/UserContext" -import Divider from "@material-ui/core/Divider" +import { UserAvatar } from "../User" export interface UserDropdownProps { user: User @@ -37,7 +37,7 @@ export const UserDropdown: React.FC = ({ user, onSignOut }: U
{user && ( - T + )} {anchorEl ? ( diff --git a/site/components/User/UserAvatar.tsx b/site/components/User/UserAvatar.tsx new file mode 100644 index 0000000000000..020e6eb063ad5 --- /dev/null +++ b/site/components/User/UserAvatar.tsx @@ -0,0 +1,25 @@ +import Avatar from "@material-ui/core/Avatar" +import React from "react" +import { User } from "../../contexts/UserContext" + +export interface UserAvatarProps { + user: User + className?: string +} + +export const UserAvatar: React.FC = ({ user, className }) => { + return {firstLetter(user.username)} +} + +/** + * `firstLetter` extracts the first character and returns it, uppercased + * + * If the string is empty or null, returns an empty string + */ +export const firstLetter = (str: string): string => { + if (str && str.length > 0) { + return str[0].toLocaleUpperCase() + } + + return "" +} diff --git a/site/components/User/UserProfileCard.tsx b/site/components/User/UserProfileCard.tsx index 772dbd7057953..b3cc1ec3deeb9 100644 --- a/site/components/User/UserProfileCard.tsx +++ b/site/components/User/UserProfileCard.tsx @@ -1,9 +1,9 @@ -import Avatar from "@material-ui/core/Avatar" import { makeStyles } from "@material-ui/core/styles" import Typography from "@material-ui/core/Typography" import React from "react" import { User } from "../../contexts/UserContext" +import { UserAvatar } from "./UserAvatar" interface UserProfileCardProps { user: User @@ -15,7 +15,7 @@ export const UserProfileCard: React.FC = ({ user }) => { return (
- T +
{user.username} {user.email} diff --git a/site/components/User/index.ts b/site/components/User/index.ts new file mode 100644 index 0000000000000..324a0afd7a931 --- /dev/null +++ b/site/components/User/index.ts @@ -0,0 +1,2 @@ +export * from "./UserAvatar" +export * from "./UserProfileCard" From f1c3ed05ae1f8ce73ae6376bbc6356629d6b3110 Mon Sep 17 00:00:00 2001 From: Bryan Phelps Date: Sat, 22 Jan 2022 02:49:58 +0000 Subject: [PATCH 06/14] Expand Navbar test to render user icon --- site/components/Navbar/index.test.tsx | 21 +++++++++++++++++++-- site/contexts/UserContext.test.tsx | 10 ++-------- site/test_helpers/index.tsx | 2 ++ site/test_helpers/user.ts | 8 ++++++++ 4 files changed, 31 insertions(+), 10 deletions(-) create mode 100644 site/test_helpers/user.ts diff --git a/site/components/Navbar/index.test.tsx b/site/components/Navbar/index.test.tsx index 9a729fd86e8f8..cd150d1807279 100644 --- a/site/components/Navbar/index.test.tsx +++ b/site/components/Navbar/index.test.tsx @@ -1,15 +1,32 @@ import React from "react" import { screen } from "@testing-library/react" -import { render } from "../../test_helpers" +import { render, MockUser } from "../../test_helpers" import { Navbar } from "./index" describe("Navbar", () => { + const noop = () => { return; } it("renders content", async () => { // When - render() + render() // Then await screen.findAllByText("Coder", { exact: false }) }) + + it("renders profile picture for user", async () => { + // Given + const mockUser = { + ...MockUser, + username: "bryan" + } + + // When + render() + + // Then + // There should be a 'B' avatar! + const element = await screen.findByText("B") + expect(element).toBeDefined() + }) }) diff --git a/site/contexts/UserContext.test.tsx b/site/contexts/UserContext.test.tsx index 8b529841cbbf8..22685b6399468 100644 --- a/site/contexts/UserContext.test.tsx +++ b/site/contexts/UserContext.test.tsx @@ -5,6 +5,7 @@ import { SWRConfig } from "swr" import { render, screen, waitFor } from "@testing-library/react" import { User, UserProvider, useUser } from "./UserContext" +import { MockUser } from "../test_helpers" namespace Helpers { // Helper component that renders out the state of the `useUser` hook. @@ -45,18 +46,11 @@ namespace Helpers { ) } - - export const mockUser: User = { - id: "test-user-id", - username: "TestUser", - email: "test@coder.com", - created_at: "", - } } describe("UserContext", () => { const failingRequest = () => Promise.reject("Failed to load user") - const successfulRequest = () => Promise.resolve(Helpers.mockUser) + const successfulRequest = () => Promise.resolve(MockUser) // Reset the router to '/' before every test beforeEach(() => { diff --git a/site/test_helpers/index.tsx b/site/test_helpers/index.tsx index 1c6c891d986b0..8242832ee7d50 100644 --- a/site/test_helpers/index.tsx +++ b/site/test_helpers/index.tsx @@ -11,3 +11,5 @@ export const WrapperComponent: React.FC = ({ children }) => { export const render = (component: React.ReactElement): RenderResult => { return wrappedRender({component}) } + +export * from "./user" diff --git a/site/test_helpers/user.ts b/site/test_helpers/user.ts new file mode 100644 index 0000000000000..2bd8a08ecc7f0 --- /dev/null +++ b/site/test_helpers/user.ts @@ -0,0 +1,8 @@ +import { User } from "../contexts/UserContext" + +export const MockUser: User = { + id: "test-user-id", + username: "TestUser", + email: "test@coder.com", + created_at: "", +} \ No newline at end of file From a05f476030f12824f7a957d47f0911e6806228cd Mon Sep 17 00:00:00 2001 From: Bryan Phelps Date: Sat, 22 Jan 2022 02:53:03 +0000 Subject: [PATCH 07/14] Cleanup --- coderd/users.go | 1 - site/components/Navbar/index.test.tsx | 6 ++++-- site/test_helpers/user.ts | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/coderd/users.go b/coderd/users.go index 55e36fe116dfa..655e61fb5070c 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -210,7 +210,6 @@ func logout(rw http.ResponseWriter, r *http.Request) { } http.SetCookie(rw, cookie) - render.Status(r, http.StatusOK) } diff --git a/site/components/Navbar/index.test.tsx b/site/components/Navbar/index.test.tsx index cd150d1807279..a4d794cf7847d 100644 --- a/site/components/Navbar/index.test.tsx +++ b/site/components/Navbar/index.test.tsx @@ -5,7 +5,9 @@ import { render, MockUser } from "../../test_helpers" import { Navbar } from "./index" describe("Navbar", () => { - const noop = () => { return; } + const noop = () => { + return + } it("renders content", async () => { // When render() @@ -18,7 +20,7 @@ describe("Navbar", () => { // Given const mockUser = { ...MockUser, - username: "bryan" + username: "bryan", } // When diff --git a/site/test_helpers/user.ts b/site/test_helpers/user.ts index 2bd8a08ecc7f0..652538ef19583 100644 --- a/site/test_helpers/user.ts +++ b/site/test_helpers/user.ts @@ -5,4 +5,4 @@ export const MockUser: User = { username: "TestUser", email: "test@coder.com", created_at: "", -} \ No newline at end of file +} From af001cba22d818a4ba28be03a5393a6340df4324 Mon Sep 17 00:00:00 2001 From: Bryan Phelps Date: Mon, 24 Jan 2022 16:29:02 +0000 Subject: [PATCH 08/14] Remove receiver name completely --- coderd/users.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/users.go b/coderd/users.go index 64cc2f93f52fb..133b8ddc7b557 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -238,7 +238,7 @@ func (users *users) loginWithPassword(rw http.ResponseWriter, r *http.Request) { } // Clear the user's session cookie -func (_ *users) logout(rw http.ResponseWriter, r *http.Request) { +func (*users) logout(rw http.ResponseWriter, r *http.Request) { // Get a blank token cookie cookie := &http.Cookie{ // MaxAge < 0 means to delete the cookie now From 52d0136b89a061c12938fcc88ba7815ab60638e6 Mon Sep 17 00:00:00 2001 From: Bryan Phelps Date: Mon, 24 Jan 2022 17:04:43 +0000 Subject: [PATCH 09/14] Add Logout test --- coderd/users_test.go | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/coderd/users_test.go b/coderd/users_test.go index cd6deda103011..d9533d09df9d1 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -2,12 +2,14 @@ package coderd_test import ( "context" + "net/http" "testing" "github.com/stretchr/testify/require" "github.com/coder/coder/coderd" "github.com/coder/coder/coderd/coderdtest" + "github.com/coder/coder/httpmw" ) func TestUsers(t *testing.T) { @@ -74,3 +76,30 @@ func TestUsers(t *testing.T) { require.Len(t, orgs, 1) }) } + +func TestLogout(t *testing.T) { + t.Parallel() + + t.Run("LogoutShouldClearCookie", func(t *testing.T) { + t.Parallel() + + server := coderdtest.New(t) + fullURL, err := server.URL.Parse("/api/v2/logout") + require.NoError(t, err, "Server URL should parse successfully") + + req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, fullURL.String(), nil) + require.NoError(t, err, "/logout request construction should succeed") + + httpClient := &http.Client{} + + response, err := httpClient.Do(req) + require.NoError(t, err, "/logout request should succeed") + response.Body.Close() + + cookies := response.Cookies() + require.Len(t, cookies, 1, "Exactly one cookie should be returned") + + require.Equal(t, cookies[0].Name, httpmw.AuthCookie, "Cookie should be the auth cookie") + require.Equal(t, cookies[0].MaxAge, -1, "Cookie should be set to delete") + }) +} From 8a828b66f138a4b811a08d2b36b8bff0da2d6166 Mon Sep 17 00:00:00 2001 From: Bryan Phelps Date: Mon, 24 Jan 2022 17:19:06 +0000 Subject: [PATCH 10/14] Fix lint warnings in user context --- site/contexts/UserContext.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/site/contexts/UserContext.tsx b/site/contexts/UserContext.tsx index c44e2ee95ff92..673c767d1e771 100644 --- a/site/contexts/UserContext.tsx +++ b/site/contexts/UserContext.tsx @@ -51,8 +51,8 @@ export const UserProvider: React.FC = (props) => { const signOut = async () => { await API.logout() // Tell SWR to invalidate the cache for the user endpoint - mutate("/api/v2/users/me") - router.push({ + await mutate("/api/v2/users/me") + await router.push({ pathname: "/login", query: { redirect: router.asPath, From 4d8f1310bcd4fcb81c9eaaedc6d54be05e577b99 Mon Sep 17 00:00:00 2001 From: Bryan Phelps Date: Mon, 24 Jan 2022 17:19:24 +0000 Subject: [PATCH 11/14] Add router as dependency --- site/contexts/UserContext.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/contexts/UserContext.tsx b/site/contexts/UserContext.tsx index 673c767d1e771..12085e7e5a0da 100644 --- a/site/contexts/UserContext.tsx +++ b/site/contexts/UserContext.tsx @@ -39,7 +39,7 @@ export const useUser = (redirectOnError = false): UserContext => { }, }) } - }, [redirectOnError, requestError]) + }, [redirectOnError, requestError, router]) return ctx } From 4dcd1fd0a441611e47aad3735c299eb7f8cf8494 Mon Sep 17 00:00:00 2001 From: Bryan Phelps Date: Mon, 24 Jan 2022 18:33:17 +0000 Subject: [PATCH 12/14] Stub out client tests --- codersdk/client.go | 18 ++++++++++++++++++ codersdk/client_test.go | 30 ++++++++++++++++++++++++++++++ codersdk/users.go | 13 +++++++++++++ codersdk/users_test.go | 8 ++++++++ 4 files changed, 69 insertions(+) create mode 100644 codersdk/client_test.go diff --git a/codersdk/client.go b/codersdk/client.go index b4931a91e8b91..b24b5b439dbcb 100644 --- a/codersdk/client.go +++ b/codersdk/client.go @@ -47,6 +47,24 @@ func (c *Client) SetSessionToken(token string) error { return nil } +// ClearSessionToken clears tokens and authentication context in the current cline.t +func (c *Client) ClearSessionToken() error { + // If there is no cookie jar defined, just leave it be + if c.httpClient.Jar == nil { + return nil + } + + // Otherwise, just clear it out + var err error + c.httpClient.Jar, err = cookiejar.New(nil) + + if err != nil { + return err + } + + return nil +} + // request performs an HTTP request with the body provided. // The caller is responsible for closing the response body. func (c *Client) request(ctx context.Context, method, path string, body interface{}) (*http.Response, error) { diff --git a/codersdk/client_test.go b/codersdk/client_test.go new file mode 100644 index 0000000000000..f04513c21a228 --- /dev/null +++ b/codersdk/client_test.go @@ -0,0 +1,30 @@ +package codersdk_test + +import ( + "net/url" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/coder/coder/codersdk" +) + +var ( + serverURL = &url.URL{ + Scheme: "https", + Host: "coder.com", + } +) + +func TestClient(t *testing.T) { + t.Parallel() + t.Run("SetClearSessionToken", func(t *testing.T) { + t.Parallel() + client := codersdk.New(serverURL) + err := client.SetSessionToken("test-session") + require.NoError(t, err, "Setting a session token should be successful") + + err = client.ClearSessionToken() + require.NoError(t, err, "Clearing a session token should be successful") + }) +} diff --git a/codersdk/users.go b/codersdk/users.go index abe62107b90f6..f47e7383f60af 100644 --- a/codersdk/users.go +++ b/codersdk/users.go @@ -44,6 +44,19 @@ func (c *Client) LoginWithPassword(ctx context.Context, req coderd.LoginWithPass return resp, nil } +// Logout calls the /logout API +// Call `ClearSessionToken()` to clear the session token of the client. +func (c *Client) Logout(ctx context.Context) error { + // Since `LoginWithPassword` doesn't actually set a SessionToken + // (it requires a call to SetSessionToken), this is essentially a no-op + res, err := c.request(ctx, http.MethodPost, "/api/v2/logout", nil) + if err != nil { + return err + } + defer res.Body.Close() + return nil +} + // User returns a user for the ID provided. // If the ID string is empty, the current user will be returned. func (c *Client) User(ctx context.Context, id string) (coderd.User, error) { diff --git a/codersdk/users_test.go b/codersdk/users_test.go index ee59e97330bd5..a42aa630c0353 100644 --- a/codersdk/users_test.go +++ b/codersdk/users_test.go @@ -47,4 +47,12 @@ func TestUsers(t *testing.T) { require.NoError(t, err) require.Len(t, orgs, 1) }) + + t.Run("LogoutIsSuccessful", func(t *testing.T) { + t.Parallel() + server := coderdtest.New(t) + _ = server.RandomInitialUser(t) + err := server.Client.Logout(context.Background()) + require.NoError(t, err) + }) } From aa84624ed871463434314ed3dc6cc277ea216c03 Mon Sep 17 00:00:00 2001 From: Bryan Phelps Date: Mon, 24 Jan 2022 18:42:22 +0000 Subject: [PATCH 13/14] Remove ClearSessionToken + test --- codersdk/client.go | 18 ------------------ codersdk/client_test.go | 30 ------------------------------ 2 files changed, 48 deletions(-) delete mode 100644 codersdk/client_test.go diff --git a/codersdk/client.go b/codersdk/client.go index b24b5b439dbcb..b4931a91e8b91 100644 --- a/codersdk/client.go +++ b/codersdk/client.go @@ -47,24 +47,6 @@ func (c *Client) SetSessionToken(token string) error { return nil } -// ClearSessionToken clears tokens and authentication context in the current cline.t -func (c *Client) ClearSessionToken() error { - // If there is no cookie jar defined, just leave it be - if c.httpClient.Jar == nil { - return nil - } - - // Otherwise, just clear it out - var err error - c.httpClient.Jar, err = cookiejar.New(nil) - - if err != nil { - return err - } - - return nil -} - // request performs an HTTP request with the body provided. // The caller is responsible for closing the response body. func (c *Client) request(ctx context.Context, method, path string, body interface{}) (*http.Response, error) { diff --git a/codersdk/client_test.go b/codersdk/client_test.go deleted file mode 100644 index f04513c21a228..0000000000000 --- a/codersdk/client_test.go +++ /dev/null @@ -1,30 +0,0 @@ -package codersdk_test - -import ( - "net/url" - "testing" - - "github.com/stretchr/testify/require" - - "github.com/coder/coder/codersdk" -) - -var ( - serverURL = &url.URL{ - Scheme: "https", - Host: "coder.com", - } -) - -func TestClient(t *testing.T) { - t.Parallel() - t.Run("SetClearSessionToken", func(t *testing.T) { - t.Parallel() - client := codersdk.New(serverURL) - err := client.SetSessionToken("test-session") - require.NoError(t, err, "Setting a session token should be successful") - - err = client.ClearSessionToken() - require.NoError(t, err, "Clearing a session token should be successful") - }) -} From 00df3d962b8e9c55714fb69a24484535aeaad51d Mon Sep 17 00:00:00 2001 From: Bryan Phelps Date: Tue, 25 Jan 2022 00:40:57 +0000 Subject: [PATCH 14/14] Fix dependency causing infinite loop in tests --- site/contexts/UserContext.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/contexts/UserContext.tsx b/site/contexts/UserContext.tsx index 12085e7e5a0da..673c767d1e771 100644 --- a/site/contexts/UserContext.tsx +++ b/site/contexts/UserContext.tsx @@ -39,7 +39,7 @@ export const useUser = (redirectOnError = false): UserContext => { }, }) } - }, [redirectOnError, requestError, router]) + }, [redirectOnError, requestError]) return ctx }