From fbd0c6d10652e6533084fbda48b30a0f4dd4f1a5 Mon Sep 17 00:00:00 2001
From: Abhineet Jain <abhineet@coder.com>
Date: Fri, 29 Jul 2022 02:38:19 +0000
Subject: [PATCH 1/5] fix: handle getUser error
---
coderd/httpmw/apikey.go | 26 +++++++++++++------
.../components/RequireAuth/RequireAuth.tsx | 5 ++--
.../SignInForm/SignInForm.stories.tsx | 23 ++++++++++++++++
site/src/components/SignInForm/SignInForm.tsx | 2 ++
site/src/pages/LoginPage/LoginPage.tsx | 17 ++++++++----
5 files changed, 58 insertions(+), 15 deletions(-)
diff --git a/coderd/httpmw/apikey.go b/coderd/httpmw/apikey.go
index 6634363e4ce9e..1917a114d303f 100644
--- a/coderd/httpmw/apikey.go
+++ b/coderd/httpmw/apikey.go
@@ -51,6 +51,8 @@ type OAuth2Configs struct {
Github OAuth2Config
}
+const loggedOutErrorMessage string = "You are logged out. Please log in to continue."
+
// ExtractAPIKey requires authentication using a valid API key.
// It handles extending an API key if it comes close to expiry,
// updating the last used time in the database.
@@ -83,7 +85,8 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool
}
if cookieValue == "" {
write(http.StatusUnauthorized, codersdk.Response{
- Message: fmt.Sprintf("Cookie %q or query parameter must be provided.", codersdk.SessionTokenKey),
+ Message: loggedOutErrorMessage,
+ Detail: fmt.Sprintf("Cookie %q or query parameter must be provided.", codersdk.SessionTokenKey),
})
return
}
@@ -91,7 +94,8 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool
// APIKeys are formatted: ID-SECRET
if len(parts) != 2 {
write(http.StatusUnauthorized, codersdk.Response{
- Message: fmt.Sprintf("Invalid %q cookie API key format.", codersdk.SessionTokenKey),
+ Message: loggedOutErrorMessage,
+ Detail: fmt.Sprintf("Invalid %q cookie API key format.", codersdk.SessionTokenKey),
})
return
}
@@ -100,13 +104,15 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool
// Ensuring key lengths are valid.
if len(keyID) != 10 {
write(http.StatusUnauthorized, codersdk.Response{
- Message: fmt.Sprintf("Invalid %q cookie API key id.", codersdk.SessionTokenKey),
+ Message: loggedOutErrorMessage,
+ Detail: fmt.Sprintf("Invalid %q cookie API key id.", codersdk.SessionTokenKey),
})
return
}
if len(keySecret) != 22 {
write(http.StatusUnauthorized, codersdk.Response{
- Message: fmt.Sprintf("Invalid %q cookie API key secret.", codersdk.SessionTokenKey),
+ Message: loggedOutErrorMessage,
+ Detail: fmt.Sprintf("Invalid %q cookie API key secret.", codersdk.SessionTokenKey),
})
return
}
@@ -114,7 +120,8 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool
if err != nil {
if errors.Is(err, sql.ErrNoRows) {
write(http.StatusUnauthorized, codersdk.Response{
- Message: "API key is invalid.",
+ Message: loggedOutErrorMessage,
+ Detail: "API key is invalid.",
})
return
}
@@ -129,7 +136,8 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool
// Checking to see if the secret is valid.
if subtle.ConstantTimeCompare(key.HashedSecret, hashed[:]) != 1 {
write(http.StatusUnauthorized, codersdk.Response{
- Message: "API key secret is invalid.",
+ Message: loggedOutErrorMessage,
+ Detail: "API key secret is invalid.",
})
return
}
@@ -174,7 +182,8 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool
// Checking if the key is expired.
if key.ExpiresAt.Before(now) {
write(http.StatusUnauthorized, codersdk.Response{
- Message: fmt.Sprintf("API key expired at %q.", key.ExpiresAt.String()),
+ Message: loggedOutErrorMessage,
+ Detail: fmt.Sprintf("API key expired at %q.", key.ExpiresAt.String()),
})
return
}
@@ -216,7 +225,8 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool
})
if err != nil {
write(http.StatusInternalServerError, codersdk.Response{
- Message: fmt.Sprintf("API key couldn't update: %s.", err.Error()),
+ Message: loggedOutErrorMessage,
+ Detail: fmt.Sprintf("API key couldn't update: %s.", err.Error()),
})
return
}
diff --git a/site/src/components/RequireAuth/RequireAuth.tsx b/site/src/components/RequireAuth/RequireAuth.tsx
index 1776720d90a7a..9a51ab658d2db 100644
--- a/site/src/components/RequireAuth/RequireAuth.tsx
+++ b/site/src/components/RequireAuth/RequireAuth.tsx
@@ -13,9 +13,10 @@ export const RequireAuth: React.FC<RequireAuthProps> = ({ children }) => {
const xServices = useContext(XServiceContext)
const [authState] = useActor(xServices.authXService)
const location = useLocation()
- const navigateTo = location.pathname === "/" ? "/login" : embedRedirect(location.pathname)
+ const isHomePage = location.pathname === "/"
+ const navigateTo = isHomePage ? "/login" : embedRedirect(location.pathname)
if (authState.matches("signedOut")) {
- return <Navigate to={navigateTo} />
+ return <Navigate to={navigateTo} state={{ isRedirect: !isHomePage }} />
} else if (authState.hasTag("loading")) {
return <FullScreenLoader />
} else {
diff --git a/site/src/components/SignInForm/SignInForm.stories.tsx b/site/src/components/SignInForm/SignInForm.stories.tsx
index 1937dfd066a5b..88378c4acd23a 100644
--- a/site/src/components/SignInForm/SignInForm.stories.tsx
+++ b/site/src/components/SignInForm/SignInForm.stories.tsx
@@ -51,6 +51,17 @@ WithLoginError.args = {
},
}
+export const WithGetUserError = Template.bind({})
+WithGetUserError.args = {
+ ...SignedOut.args,
+ loginErrors: {
+ [LoginErrors.GET_USER_ERROR]: makeMockApiError({
+ message: "You are logged out. Please log in to continue.",
+ detail: "API Key is invalid.",
+ }),
+ },
+}
+
export const WithCheckPermissionsError = Template.bind({})
WithCheckPermissionsError.args = {
...SignedOut.args,
@@ -70,6 +81,18 @@ WithAuthMethodsError.args = {
},
}
+export const WithGetUserAndAuthMethodsError = Template.bind({})
+WithGetUserAndAuthMethodsError.args = {
+ ...SignedOut.args,
+ loginErrors: {
+ [LoginErrors.GET_USER_ERROR]: makeMockApiError({
+ message: "You are logged out. Please log in to continue.",
+ detail: "API Key is invalid.",
+ }),
+ [LoginErrors.GET_METHODS_ERROR]: new Error("Failed to fetch auth methods"),
+ },
+}
+
export const WithGithub = Template.bind({})
WithGithub.args = {
...SignedOut.args,
diff --git a/site/src/components/SignInForm/SignInForm.tsx b/site/src/components/SignInForm/SignInForm.tsx
index a4d75dad63dd7..2860f6f3cedd2 100644
--- a/site/src/components/SignInForm/SignInForm.tsx
+++ b/site/src/components/SignInForm/SignInForm.tsx
@@ -25,6 +25,7 @@ interface BuiltInAuthFormValues {
export enum LoginErrors {
AUTH_ERROR = "authError",
+ GET_USER_ERROR = "getUserError",
CHECK_PERMISSIONS_ERROR = "checkPermissionsError",
GET_METHODS_ERROR = "getMethodsError",
}
@@ -36,6 +37,7 @@ export const Language = {
emailRequired: "Please enter an email address.",
errorMessages: {
[LoginErrors.AUTH_ERROR]: "Incorrect email or password.",
+ [LoginErrors.GET_USER_ERROR]: "Failed to fetch user details.",
[LoginErrors.CHECK_PERMISSIONS_ERROR]: "Unable to fetch user permissions.",
[LoginErrors.GET_METHODS_ERROR]: "Unable to fetch auth methods.",
},
diff --git a/site/src/pages/LoginPage/LoginPage.tsx b/site/src/pages/LoginPage/LoginPage.tsx
index 305012425130d..ecbf3952bbec8 100644
--- a/site/src/pages/LoginPage/LoginPage.tsx
+++ b/site/src/pages/LoginPage/LoginPage.tsx
@@ -4,7 +4,7 @@ import React, { useContext } from "react"
import { Helmet } from "react-helmet"
import { Navigate, useLocation } from "react-router-dom"
import { Footer } from "../../components/Footer/Footer"
-import { SignInForm } from "../../components/SignInForm/SignInForm"
+import { LoginErrors, SignInForm } from "../../components/SignInForm/SignInForm"
import { pageTitle } from "../../util/page"
import { retrieveRedirect } from "../../util/redirect"
import { XServiceContext } from "../../xServices/StateContext"
@@ -28,6 +28,10 @@ export const useStyles = makeStyles((theme) => ({
},
}))
+interface LocationState {
+ isRedirect: boolean
+}
+
export const LoginPage: React.FC = () => {
const styles = useStyles()
const location = useLocation()
@@ -35,12 +39,14 @@ export const LoginPage: React.FC = () => {
const [authState, authSend] = useActor(xServices.authXService)
const isLoading = authState.hasTag("loading")
const redirectTo = retrieveRedirect(location.search)
+ const locationState = location.state ? (location.state as LocationState) : null
+ const isRedirected = locationState !== null ? locationState.isRedirect : false
const onSubmit = async ({ email, password }: { email: string; password: string }) => {
authSend({ type: "SIGN_IN", email, password })
}
- const { authError, checkPermissionsError, getMethodsError } = authState.context
+ const { authError, getUserError, checkPermissionsError, getMethodsError } = authState.context
if (authState.matches("signedIn")) {
return <Navigate to={redirectTo} replace />
@@ -57,9 +63,10 @@ export const LoginPage: React.FC = () => {
redirectTo={redirectTo}
isLoading={isLoading}
loginErrors={{
- authError,
- checkPermissionsError,
- getMethodsError,
+ [LoginErrors.AUTH_ERROR]: authError,
+ [LoginErrors.GET_USER_ERROR]: isRedirected ? getUserError : null,
+ [LoginErrors.CHECK_PERMISSIONS_ERROR]: checkPermissionsError,
+ [LoginErrors.GET_METHODS_ERROR]: getMethodsError,
}}
onSubmit={onSubmit}
/>
From 8dcd9586208bc84356bf6564b20dc2c9e4954713 Mon Sep 17 00:00:00 2001
From: Abhineet Jain <abhineet@coder.com>
Date: Fri, 29 Jul 2022 02:57:00 +0000
Subject: [PATCH 2/5] change language to use sign-out
---
coderd/httpmw/apikey.go | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/coderd/httpmw/apikey.go b/coderd/httpmw/apikey.go
index 1917a114d303f..e1d56539c00a0 100644
--- a/coderd/httpmw/apikey.go
+++ b/coderd/httpmw/apikey.go
@@ -51,7 +51,7 @@ type OAuth2Configs struct {
Github OAuth2Config
}
-const loggedOutErrorMessage string = "You are logged out. Please log in to continue."
+const signedOutErrorMessage string = "You are signed out. Please sign in to continue."
// ExtractAPIKey requires authentication using a valid API key.
// It handles extending an API key if it comes close to expiry,
@@ -85,7 +85,7 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool
}
if cookieValue == "" {
write(http.StatusUnauthorized, codersdk.Response{
- Message: loggedOutErrorMessage,
+ Message: signedOutErrorMessage,
Detail: fmt.Sprintf("Cookie %q or query parameter must be provided.", codersdk.SessionTokenKey),
})
return
@@ -94,7 +94,7 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool
// APIKeys are formatted: ID-SECRET
if len(parts) != 2 {
write(http.StatusUnauthorized, codersdk.Response{
- Message: loggedOutErrorMessage,
+ Message: signedOutErrorMessage,
Detail: fmt.Sprintf("Invalid %q cookie API key format.", codersdk.SessionTokenKey),
})
return
@@ -104,14 +104,14 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool
// Ensuring key lengths are valid.
if len(keyID) != 10 {
write(http.StatusUnauthorized, codersdk.Response{
- Message: loggedOutErrorMessage,
+ Message: signedOutErrorMessage,
Detail: fmt.Sprintf("Invalid %q cookie API key id.", codersdk.SessionTokenKey),
})
return
}
if len(keySecret) != 22 {
write(http.StatusUnauthorized, codersdk.Response{
- Message: loggedOutErrorMessage,
+ Message: signedOutErrorMessage,
Detail: fmt.Sprintf("Invalid %q cookie API key secret.", codersdk.SessionTokenKey),
})
return
@@ -120,7 +120,7 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool
if err != nil {
if errors.Is(err, sql.ErrNoRows) {
write(http.StatusUnauthorized, codersdk.Response{
- Message: loggedOutErrorMessage,
+ Message: signedOutErrorMessage,
Detail: "API key is invalid.",
})
return
@@ -136,7 +136,7 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool
// Checking to see if the secret is valid.
if subtle.ConstantTimeCompare(key.HashedSecret, hashed[:]) != 1 {
write(http.StatusUnauthorized, codersdk.Response{
- Message: loggedOutErrorMessage,
+ Message: signedOutErrorMessage,
Detail: "API key secret is invalid.",
})
return
@@ -182,7 +182,7 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool
// Checking if the key is expired.
if key.ExpiresAt.Before(now) {
write(http.StatusUnauthorized, codersdk.Response{
- Message: loggedOutErrorMessage,
+ Message: signedOutErrorMessage,
Detail: fmt.Sprintf("API key expired at %q.", key.ExpiresAt.String()),
})
return
@@ -225,7 +225,7 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool
})
if err != nil {
write(http.StatusInternalServerError, codersdk.Response{
- Message: loggedOutErrorMessage,
+ Message: signedOutErrorMessage,
Detail: fmt.Sprintf("API key couldn't update: %s.", err.Error()),
})
return
From 52c23fb55a327bc424ec580c04dce383a3506e0b Mon Sep 17 00:00:00 2001
From: Abhineet Jain <abhineet@coder.com>
Date: Fri, 29 Jul 2022 03:04:42 +0000
Subject: [PATCH 3/5] update sign out message
---
coderd/httpmw/apikey.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/coderd/httpmw/apikey.go b/coderd/httpmw/apikey.go
index e1d56539c00a0..ead5759eb1a51 100644
--- a/coderd/httpmw/apikey.go
+++ b/coderd/httpmw/apikey.go
@@ -51,7 +51,7 @@ type OAuth2Configs struct {
Github OAuth2Config
}
-const signedOutErrorMessage string = "You are signed out. Please sign in to continue."
+const signedOutErrorMessage string = "You are signed out or your session has expired. Please sign in again to continue."
// ExtractAPIKey requires authentication using a valid API key.
// It handles extending an API key if it comes close to expiry,
From ade96de46c204deec024473dac1f44708702a1b6 Mon Sep 17 00:00:00 2001
From: Abhineet Jain <abhineet@coder.com>
Date: Fri, 29 Jul 2022 15:57:33 +0000
Subject: [PATCH 4/5] update internal error message
---
coderd/httpmw/apikey.go | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/coderd/httpmw/apikey.go b/coderd/httpmw/apikey.go
index ead5759eb1a51..9ababe6cd45ef 100644
--- a/coderd/httpmw/apikey.go
+++ b/coderd/httpmw/apikey.go
@@ -51,7 +51,10 @@ type OAuth2Configs struct {
Github OAuth2Config
}
-const signedOutErrorMessage string = "You are signed out or your session has expired. Please sign in again to continue."
+const (
+ signedOutErrorMessage string = "You are signed out or your session has expired. Please sign in again to continue."
+ internalErrorMessage string = "An internal error occurred. Please try again or contact the system administrator."
+)
// ExtractAPIKey requires authentication using a valid API key.
// It handles extending an API key if it comes close to expiry,
@@ -126,8 +129,8 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool
return
}
write(http.StatusInternalServerError, codersdk.Response{
- Message: "Internal error fetching API key by id.",
- Detail: err.Error(),
+ Message: internalErrorMessage,
+ Detail: fmt.Sprintf("Internal error fetching API key by id. %s", err.Error()),
})
return
}
@@ -154,7 +157,8 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool
oauthConfig = oauth.Github
default:
write(http.StatusInternalServerError, codersdk.Response{
- Message: fmt.Sprintf("Unexpected authentication type %q.", key.LoginType),
+ Message: internalErrorMessage,
+ Detail: fmt.Sprintf("Unexpected authentication type %q.", key.LoginType),
})
return
}
@@ -225,7 +229,7 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool
})
if err != nil {
write(http.StatusInternalServerError, codersdk.Response{
- Message: signedOutErrorMessage,
+ Message: internalErrorMessage,
Detail: fmt.Sprintf("API key couldn't update: %s.", err.Error()),
})
return
@@ -238,8 +242,8 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool
roles, err := db.GetAuthorizationUserRoles(r.Context(), key.UserID)
if err != nil {
write(http.StatusUnauthorized, codersdk.Response{
- Message: "Internal error fetching user's roles.",
- Detail: err.Error(),
+ Message: internalErrorMessage,
+ Detail: fmt.Sprintf("Internal error fetching user's roles. %s", err.Error()),
})
return
}
From 4bcf5e722249d1072379bf99ba3dfecb667e6950 Mon Sep 17 00:00:00 2001
From: Abhineet Jain <abhineet@coder.com>
Date: Fri, 29 Jul 2022 16:19:48 +0000
Subject: [PATCH 5/5] simplify conditional
---
site/src/pages/LoginPage/LoginPage.tsx | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/site/src/pages/LoginPage/LoginPage.tsx b/site/src/pages/LoginPage/LoginPage.tsx
index ecbf3952bbec8..b84dac9c87106 100644
--- a/site/src/pages/LoginPage/LoginPage.tsx
+++ b/site/src/pages/LoginPage/LoginPage.tsx
@@ -40,7 +40,7 @@ export const LoginPage: React.FC = () => {
const isLoading = authState.hasTag("loading")
const redirectTo = retrieveRedirect(location.search)
const locationState = location.state ? (location.state as LocationState) : null
- const isRedirected = locationState !== null ? locationState.isRedirect : false
+ const isRedirected = locationState ? locationState.isRedirect : false
const onSubmit = async ({ email, password }: { email: string; password: string }) => {
authSend({ type: "SIGN_IN", email, password })