Skip to content

Commit e94fe20

Browse files
AbhineetJainAbhineetJain
authored
fix: handle getUser error (#3285)
1 parent 4658b3f commit e94fe20

File tree

5 files changed

+67
-20
lines changed

5 files changed

+67
-20
lines changed

‎coderd/httpmw/apikey.go

+27-13
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,11 @@ type OAuth2Configs struct {
5151
Github OAuth2Config
5252
}
5353

54+
const (
55+
signedOutErrorMessage string = "You are signed out or your session has expired. Please sign in again to continue."
56+
internalErrorMessage string = "An internal error occurred. Please try again or contact the system administrator."
57+
)
58+
5459
// ExtractAPIKey requires authentication using a valid API key.
5560
// It handles extending an API key if it comes close to expiry,
5661
// updating the last used time in the database.
@@ -83,15 +88,17 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool
8388
}
8489
if cookieValue == "" {
8590
write(http.StatusUnauthorized, codersdk.Response{
86-
Message: fmt.Sprintf("Cookie %q or query parameter must be provided.", codersdk.SessionTokenKey),
91+
Message: signedOutErrorMessage,
92+
Detail: fmt.Sprintf("Cookie %q or query parameter must be provided.", codersdk.SessionTokenKey),
8793
})
8894
return
8995
}
9096
parts := strings.Split(cookieValue, "-")
9197
// APIKeys are formatted: ID-SECRET
9298
if len(parts) != 2 {
9399
write(http.StatusUnauthorized, codersdk.Response{
94-
Message: fmt.Sprintf("Invalid %q cookie API key format.", codersdk.SessionTokenKey),
100+
Message: signedOutErrorMessage,
101+
Detail: fmt.Sprintf("Invalid %q cookie API key format.", codersdk.SessionTokenKey),
95102
})
96103
return
97104
}
@@ -100,27 +107,30 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool
100107
// Ensuring key lengths are valid.
101108
if len(keyID) != 10 {
102109
write(http.StatusUnauthorized, codersdk.Response{
103-
Message: fmt.Sprintf("Invalid %q cookie API key id.", codersdk.SessionTokenKey),
110+
Message: signedOutErrorMessage,
111+
Detail: fmt.Sprintf("Invalid %q cookie API key id.", codersdk.SessionTokenKey),
104112
})
105113
return
106114
}
107115
if len(keySecret) != 22 {
108116
write(http.StatusUnauthorized, codersdk.Response{
109-
Message: fmt.Sprintf("Invalid %q cookie API key secret.", codersdk.SessionTokenKey),
117+
Message: signedOutErrorMessage,
118+
Detail: fmt.Sprintf("Invalid %q cookie API key secret.", codersdk.SessionTokenKey),
110119
})
111120
return
112121
}
113122
key, err := db.GetAPIKeyByID(r.Context(), keyID)
114123
if err != nil {
115124
if errors.Is(err, sql.ErrNoRows) {
116125
write(http.StatusUnauthorized, codersdk.Response{
117-
Message: "API key is invalid.",
126+
Message: signedOutErrorMessage,
127+
Detail: "API key is invalid.",
118128
})
119129
return
120130
}
121131
write(http.StatusInternalServerError, codersdk.Response{
122-
Message: "Internal error fetching API key by id.",
123-
Detail: err.Error(),
132+
Message: internalErrorMessage,
133+
Detail: fmt.Sprintf("Internal error fetching API key by id. %s", err.Error()),
124134
})
125135
return
126136
}
@@ -129,7 +139,8 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool
129139
// Checking to see if the secret is valid.
130140
if subtle.ConstantTimeCompare(key.HashedSecret, hashed[:]) != 1 {
131141
write(http.StatusUnauthorized, codersdk.Response{
132-
Message: "API key secret is invalid.",
142+
Message: signedOutErrorMessage,
143+
Detail: "API key secret is invalid.",
133144
})
134145
return
135146
}
@@ -146,7 +157,8 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool
146157
oauthConfig = oauth.Github
147158
default:
148159
write(http.StatusInternalServerError, codersdk.Response{
149-
Message: fmt.Sprintf("Unexpected authentication type %q.", key.LoginType),
160+
Message: internalErrorMessage,
161+
Detail: fmt.Sprintf("Unexpected authentication type %q.", key.LoginType),
150162
})
151163
return
152164
}
@@ -174,7 +186,8 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool
174186
// Checking if the key is expired.
175187
if key.ExpiresAt.Before(now) {
176188
write(http.StatusUnauthorized, codersdk.Response{
177-
Message: fmt.Sprintf("API key expired at %q.", key.ExpiresAt.String()),
189+
Message: signedOutErrorMessage,
190+
Detail: fmt.Sprintf("API key expired at %q.", key.ExpiresAt.String()),
178191
})
179192
return
180193
}
@@ -216,7 +229,8 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool
216229
})
217230
if err != nil {
218231
write(http.StatusInternalServerError, codersdk.Response{
219-
Message: fmt.Sprintf("API key couldn't update: %s.", err.Error()),
232+
Message: internalErrorMessage,
233+
Detail: fmt.Sprintf("API key couldn't update: %s.", err.Error()),
220234
})
221235
return
222236
}
@@ -228,8 +242,8 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs, redirectToLogin bool
228242
roles, err := db.GetAuthorizationUserRoles(r.Context(), key.UserID)
229243
if err != nil {
230244
write(http.StatusUnauthorized, codersdk.Response{
231-
Message: "Internal error fetching user's roles.",
232-
Detail: err.Error(),
245+
Message: internalErrorMessage,
246+
Detail: fmt.Sprintf("Internal error fetching user's roles. %s", err.Error()),
233247
})
234248
return
235249
}

‎site/src/components/RequireAuth/RequireAuth.tsx

+3-2
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,10 @@ export const RequireAuth: React.FC<RequireAuthProps> = ({ children }) => {
1313
const xServices = useContext(XServiceContext)
1414
const [authState] = useActor(xServices.authXService)
1515
const location = useLocation()
16-
const navigateTo = location.pathname === "/" ? "/login" : embedRedirect(location.pathname)
16+
const isHomePage = location.pathname === "/"
17+
const navigateTo = isHomePage ? "/login" : embedRedirect(location.pathname)
1718
if (authState.matches("signedOut")) {
18-
return <Navigate to={navigateTo} />
19+
return <Navigate to={navigateTo} state={{ isRedirect: !isHomePage }} />
1920
} else if (authState.hasTag("loading")) {
2021
return <FullScreenLoader />
2122
} else {

‎site/src/components/SignInForm/SignInForm.stories.tsx

+23
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,17 @@ WithLoginError.args = {
5151
},
5252
}
5353

54+
export const WithGetUserError = Template.bind({})
55+
WithGetUserError.args = {
56+
...SignedOut.args,
57+
loginErrors: {
58+
[LoginErrors.GET_USER_ERROR]: makeMockApiError({
59+
message: "You are logged out. Please log in to continue.",
60+
detail: "API Key is invalid.",
61+
}),
62+
},
63+
}
64+
5465
export const WithCheckPermissionsError = Template.bind({})
5566
WithCheckPermissionsError.args = {
5667
...SignedOut.args,
@@ -70,6 +81,18 @@ WithAuthMethodsError.args = {
7081
},
7182
}
7283

84+
export const WithGetUserAndAuthMethodsError = Template.bind({})
85+
WithGetUserAndAuthMethodsError.args = {
86+
...SignedOut.args,
87+
loginErrors: {
88+
[LoginErrors.GET_USER_ERROR]: makeMockApiError({
89+
message: "You are logged out. Please log in to continue.",
90+
detail: "API Key is invalid.",
91+
}),
92+
[LoginErrors.GET_METHODS_ERROR]: new Error("Failed to fetch auth methods"),
93+
},
94+
}
95+
7396
export const WithGithub = Template.bind({})
7497
WithGithub.args = {
7598
...SignedOut.args,

‎site/src/components/SignInForm/SignInForm.tsx

+2
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ interface BuiltInAuthFormValues {
2525

2626
export enum LoginErrors {
2727
AUTH_ERROR = "authError",
28+
GET_USER_ERROR = "getUserError",
2829
CHECK_PERMISSIONS_ERROR = "checkPermissionsError",
2930
GET_METHODS_ERROR = "getMethodsError",
3031
}
@@ -36,6 +37,7 @@ export const Language = {
3637
emailRequired: "Please enter an email address.",
3738
errorMessages: {
3839
[LoginErrors.AUTH_ERROR]: "Incorrect email or password.",
40+
[LoginErrors.GET_USER_ERROR]: "Failed to fetch user details.",
3941
[LoginErrors.CHECK_PERMISSIONS_ERROR]: "Unable to fetch user permissions.",
4042
[LoginErrors.GET_METHODS_ERROR]: "Unable to fetch auth methods.",
4143
},

‎site/src/pages/LoginPage/LoginPage.tsx

+12-5
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import React, { useContext } from "react"
44
import { Helmet } from "react-helmet"
55
import { Navigate, useLocation } from "react-router-dom"
66
import { Footer } from "../../components/Footer/Footer"
7-
import { SignInForm } from "../../components/SignInForm/SignInForm"
7+
import { LoginErrors, SignInForm } from "../../components/SignInForm/SignInForm"
88
import { pageTitle } from "../../util/page"
99
import { retrieveRedirect } from "../../util/redirect"
1010
import { XServiceContext } from "../../xServices/StateContext"
@@ -28,19 +28,25 @@ export const useStyles = makeStyles((theme) => ({
2828
},
2929
}))
3030

31+
interface LocationState {
32+
isRedirect: boolean
33+
}
34+
3135
export const LoginPage: React.FC = () => {
3236
const styles = useStyles()
3337
const location = useLocation()
3438
const xServices = useContext(XServiceContext)
3539
const [authState, authSend] = useActor(xServices.authXService)
3640
const isLoading = authState.hasTag("loading")
3741
const redirectTo = retrieveRedirect(location.search)
42+
const locationState = location.state ? (location.state as LocationState) : null
43+
const isRedirected = locationState ? locationState.isRedirect : false
3844

3945
const onSubmit = async ({ email, password }: { email: string; password: string }) => {
4046
authSend({ type: "SIGN_IN", email, password })
4147
}
4248

43-
const { authError, checkPermissionsError, getMethodsError } = authState.context
49+
const { authError, getUserError, checkPermissionsError, getMethodsError } = authState.context
4450

4551
if (authState.matches("signedIn")) {
4652
return <Navigate to={redirectTo} replace />
@@ -57,9 +63,10 @@ export const LoginPage: React.FC = () => {
5763
redirectTo={redirectTo}
5864
isLoading={isLoading}
5965
loginErrors={{
60-
authError,
61-
checkPermissionsError,
62-
getMethodsError,
66+
[LoginErrors.AUTH_ERROR]: authError,
67+
[LoginErrors.GET_USER_ERROR]: isRedirected ? getUserError : null,
68+
[LoginErrors.CHECK_PERMISSIONS_ERROR]: checkPermissionsError,
69+
[LoginErrors.GET_METHODS_ERROR]: getMethodsError,
6370
}}
6471
onSubmit={onSubmit}
6572
/>

0 commit comments

Comments
 (0)