Skip to content

Commit 2ee406d

Browse files
EmyrkEmyrk
authored
feat: log out and redirect user when converting to oidc (coder#8347)
* feat: log out user on conver to oidc Log out user and redirect to login page and log out user when they convert to oidc.
1 parent 90a3deb commit 2ee406d

5 files changed

Lines changed: 76 additions & 21 deletions

File tree

‎coderd/coderd.go‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -632,12 +632,18 @@ func New(options *Options) *API {
632632
r.Post("/login", api.postLogin)
633633
r.Route("/oauth2", func(r chi.Router) {
634634
r.Route("/github", func(r chi.Router) {
635-
r.Use(httpmw.ExtractOAuth2(options.GithubOAuth2Config, options.HTTPClient, nil))
635+
r.Use(
636+
httpmw.ExtractOAuth2(options.GithubOAuth2Config, options.HTTPClient, nil),
637+
apiKeyMiddlewareOptional,
638+
)
636639
r.Get("/callback", api.userOAuth2Github)
637640
})
638641
})
639642
r.Route("/oidc/callback", func(r chi.Router) {
640-
r.Use(httpmw.ExtractOAuth2(options.OIDCConfig, options.HTTPClient, oidcAuthURLParams))
643+
r.Use(
644+
httpmw.ExtractOAuth2(options.OIDCConfig, options.HTTPClient, oidcAuthURLParams),
645+
apiKeyMiddlewareOptional,
646+
)
641647
r.Get("/", api.userOIDC)
642648
})
643649
})

‎coderd/userauth.go‎

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1110,6 +1110,7 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C
11101110
cookies []*http.Cookie
11111111
)
11121112

1113+
var isConvertLoginType bool
11131114
err := api.Database.InTx(func(tx database.Store) error {
11141115
var (
11151116
link database.UserLink
@@ -1130,6 +1131,7 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C
11301131
return err
11311132
}
11321133
params.User = user
1134+
isConvertLoginType = true
11331135
}
11341136

11351137
if user.ID == uuid.Nil && !params.AllowSignups {
@@ -1292,18 +1294,44 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C
12921294
return nil, database.APIKey{}, xerrors.Errorf("in tx: %w", err)
12931295
}
12941296

1295-
//nolint:gocritic
1296-
cookie, key, err := api.createAPIKey(dbauthz.AsSystemRestricted(ctx), apikey.CreateParams{
1297-
UserID: user.ID,
1298-
LoginType: params.LoginType,
1299-
DeploymentValues: api.DeploymentValues,
1300-
RemoteAddr: r.RemoteAddr,
1301-
})
1302-
if err != nil {
1303-
return nil, database.APIKey{}, xerrors.Errorf("create API key: %w", err)
1297+
var key database.APIKey
1298+
if oldKey, ok := httpmw.APIKeyOptional(r); ok && isConvertLoginType {
1299+
// If this is a convert login type, and it succeeds, then delete the old
1300+
// session. Force the user to log back in.
1301+
err := api.Database.DeleteAPIKeyByID(r.Context(), oldKey.ID)
1302+
if err != nil {
1303+
// Do not block this login if we fail to delete the old API key.
1304+
// Just delete the cookie and continue.
1305+
api.Logger.Warn(r.Context(), "failed to delete old API key in convert to oidc",
1306+
slog.Error(err),
1307+
slog.F("old_api_key_id", oldKey.ID),
1308+
slog.F("user_id", user.ID),
1309+
)
1310+
}
1311+
cookies = append(cookies, &http.Cookie{
1312+
Name: codersdk.SessionTokenCookie,
1313+
Path: "/",
1314+
MaxAge: -1,
1315+
Secure: api.SecureAuthCookie,
1316+
HttpOnly: true,
1317+
})
1318+
key = oldKey
1319+
} else {
1320+
//nolint:gocritic
1321+
cookie, newKey, err := api.createAPIKey(dbauthz.AsSystemRestricted(ctx), apikey.CreateParams{
1322+
UserID: user.ID,
1323+
LoginType: params.LoginType,
1324+
DeploymentValues: api.DeploymentValues,
1325+
RemoteAddr: r.RemoteAddr,
1326+
})
1327+
if err != nil {
1328+
return nil, database.APIKey{}, xerrors.Errorf("create API key: %w", err)
1329+
}
1330+
cookies = append(cookies, cookie)
1331+
key = *newKey
13041332
}
13051333

1306-
return append(cookies, cookie), *key, nil
1334+
return cookies, key, nil
13071335
}
13081336

13091337
// convertUserToOauth will convert a user from password base loginType to

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ const useStyles = makeStyles((theme) => ({
3737
fontWeight: 600,
3838
},
3939
},
40-
error: {
40+
alert: {
4141
marginBottom: theme.spacing(4),
4242
},
4343
divider: {
@@ -69,6 +69,7 @@ export interface SignInFormProps {
6969
isSigningIn: boolean
7070
redirectTo: string
7171
error?: unknown
72+
info?: string
7273
authMethods?: AuthMethods
7374
onSubmit: (credentials: { email: string; password: string }) => void
7475
// initialTouched is only used for testing the error state of the form.
@@ -80,6 +81,7 @@ export const SignInForm: FC<React.PropsWithChildren<SignInFormProps>> = ({
8081
redirectTo,
8182
isSigningIn,
8283
error,
84+
info,
8385
onSubmit,
8486
initialTouched,
8587
}) => {
@@ -100,10 +102,15 @@ export const SignInForm: FC<React.PropsWithChildren<SignInFormProps>> = ({
100102
<strong>{commonTranslation.t("coder")}</strong>
101103
</h1>
102104
<Maybe condition={error !== undefined}>
103-
<div className={styles.error}>
105+
<div className={styles.alert}>
104106
<ErrorAlert error={error} />
105107
</div>
106108
</Maybe>
109+
<Maybe condition={Boolean(info) && info !== "" && error === undefined}>
110+
<div className={styles.alert}>
111+
<Alert severity="info">{info}</Alert>
112+
</div>
113+
</Maybe>
107114
<Maybe condition={passwordEnabled && showPasswordAuth}>
108115
<PasswordSignInForm
109116
onSubmit={onSubmit}

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ export const LoginPageView: FC<LoginPageViewProps> = ({
2525
const { error } = context
2626
const data = context.data as UnauthenticatedData
2727
const styles = useStyles()
28+
// This allows messages to be displayed at the top of the sign in form.
29+
// Helpful for any redirects that want to inform the user of something.
30+
const info = new URLSearchParams(location.search).get("info") || undefined
2831

2932
return isLoading ? (
3033
<FullScreenLoader />
@@ -37,6 +40,7 @@ export const LoginPageView: FC<LoginPageViewProps> = ({
3740
redirectTo={redirectTo}
3841
isSigningIn={isSigningIn}
3942
error={error}
43+
info={info}
4044
onSubmit={onSignIn}
4145
/>
4246
<footer className={styles.footer}>

‎site/src/pages/UserSettingsPage/SecurityPage/SingleSignOnSection.tsx‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@ import Box from "@mui/material/Box"
55
import GitHubIcon from "@mui/icons-material/GitHub"
66
import KeyIcon from "@mui/icons-material/VpnKey"
77
import Button from "@mui/material/Button"
8-
import { useLocation } from "react-router-dom"
9-
import { retrieveRedirect } from "utils/redirect"
108
import Typography from "@mui/material/Typography"
119
import { convertToOAUTH } from "api/api"
1210
import { AuthMethods, LoginType, UserLoginType } from "api/typesGenerated"
@@ -27,19 +25,31 @@ type LoginTypeConfirmation =
2725
selectedType: LoginType
2826
}
2927

30-
export const redirectToOIDCAuth = (stateString: string, redirectTo: string) => {
31-
window.location.href = `/api/v2/users/oidc/callback?oidc_merge_state=${stateString}&redirect=${redirectTo}`
28+
export const redirectToOIDCAuth = (
29+
toType: string,
30+
stateString: string,
31+
redirectTo: string,
32+
) => {
33+
window.location.href = `/api/v2/users/${toType}/callback?oidc_merge_state=${stateString}&redirect=${redirectTo}`
3234
}
3335

3436
export const useSingleSignOnSection = () => {
35-
const location = useLocation()
36-
const redirectTo = retrieveRedirect(location.search)
3737
const [loginTypeConfirmation, setLoginTypeConfirmation] =
3838
useState<LoginTypeConfirmation>({ open: false, selectedType: undefined })
3939

4040
const mutation = useMutation(convertToOAUTH, {
4141
onSuccess: (data) => {
42-
redirectToOIDCAuth(data.state_string, encodeURIComponent(redirectTo))
42+
const loginTypeMsg =
43+
data.to_type === "github" ? "Github" : "OpenID Connect"
44+
redirectToOIDCAuth(
45+
data.to_type,
46+
data.state_string,
47+
// The redirect on success should be back to the login page with a nice message.
48+
// The user should be logged out if this worked.
49+
encodeURIComponent(
50+
`/login?info=Login type has been changed to ${loginTypeMsg}. Log in again using the new method.`,
51+
),
52+
)
4353
},
4454
})
4555

0 commit comments

Comments
 (0)