Skip to content

Commit 596605e

Browse files
EmyrkEmyrk
committed
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 33bdc23 commit 596605e

File tree

5 files changed

+74
-19
lines changed

5 files changed

+74
-19
lines changed

‎coderd/coderd.go

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

‎coderd/userauth.go

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

1099+
var isConvertLoginType bool
10991100
err := api.Database.InTx(func(tx database.Store) error {
11001101
var (
11011102
link database.UserLink
@@ -1116,6 +1117,7 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C
11161117
return err
11171118
}
11181119
params.User = user
1120+
isConvertLoginType = true
11191121
}
11201122

11211123
if user.ID == uuid.Nil && !params.AllowSignups {
@@ -1284,18 +1286,44 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C
12841286
return nil, database.APIKey{}, xerrors.Errorf("in tx: %w", err)
12851287
}
12861288

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

1298-
return append(cookies, cookie), *key, nil
1326+
return cookies, key, nil
12991327
}
13001328

13011329
// 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: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,19 +27,29 @@ type LoginTypeConfirmation =
2727
selectedType: LoginType
2828
}
2929

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

3438
export const useSingleSignOnSection = () => {
35-
const location = useLocation()
36-
const redirectTo = retrieveRedirect(location.search)
3739
const [loginTypeConfirmation, setLoginTypeConfirmation] =
3840
useState<LoginTypeConfirmation>({ open: false, selectedType: undefined })
3941

4042
const mutation = useMutation(convertToOAUTH, {
4143
onSuccess: (data) => {
42-
redirectToOIDCAuth(data.state_string, encodeURIComponent(redirectTo))
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 ${data.to_type}. Log in again using the new method.`,
51+
),
52+
)
4353
},
4454
})
4555

0 commit comments

Comments
 (0)