fix(coderd)!: restrict OIDC email fallback to first-time account link… · coder/coder@ffe7645
@@ -1036,7 +1036,16 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) {
10361036 })
10371037return
10381038 }
1039-user, link, err := findLinkedUser(ctx, api.Database, githubLinkedID(ghUser), verifiedEmail.GetEmail())
1039+user, link, err := findLinkedUser(ctx, api.Database, githubLinkedID(ghUser), database.LoginTypeGithub, verifiedEmail.GetEmail())
1040+if errors.Is(err, errLinkedIDAlreadyBound) {
1041+logger.Warn(ctx, "oauth2: blocked login, account already linked to different identity",
1042+slog.F("email", verifiedEmail.GetEmail()),
1043+ )
1044+httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{
1045+Message: "This account is already linked to a different identity provider subject.",
1046+ })
1047+return
1048+ }
10401049if err != nil {
10411050logger.Error(ctx, "oauth2: unable to find linked user", slog.F("gh_user", ghUser.Name), slog.Error(err))
10421051httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
@@ -1436,7 +1445,22 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
14361445 }
14371446ctx = slog.With(ctx, slog.F("email", email), slog.F("username", username), slog.F("name", name))
143814471439-user, link, err := findLinkedUser(ctx, api.Database, oidcLinkedID(idToken), email)
1448+user, link, err := findLinkedUser(ctx, api.Database, oidcLinkedID(idToken), database.LoginTypeOIDC, email)
1449+if errors.Is(err, errLinkedIDAlreadyBound) {
1450+logger.Warn(ctx, "oauth2: blocked login, account already linked to different identity",
1451+slog.F("email", email),
1452+ )
1453+site.RenderStaticErrorPage(rw, r, site.ErrorPageData{
1454+Status: http.StatusForbidden,
1455+HideStatus: true,
1456+Title: "Account already linked",
1457+Description: "This account is already linked to a different identity provider subject. Contact your administrator.",
1458+Actions: []site.Action{
1459+ {URL: "/login", Text: "Back to login"},
1460+ },
1461+ })
1462+return
1463+ }
14401464if err != nil {
14411465logger.Error(ctx, "oauth2: unable to find linked user", slog.F("email", email), slog.Error(err))
14421466httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
@@ -1870,6 +1894,31 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C
18701894if err != nil {
18711895return xerrors.Errorf("update user link: %w", err)
18721896 }
1897+1898+// Defense-in-depth: if a concurrent transaction backfilled
1899+// linked_id between findLinkedUser and this point, reject the
1900+// login with a 403 instead of letting it bubble up as a 500.
1901+if link.LinkedID != "" && link.LinkedID != params.LinkedID {
1902+return &idpsync.HTTPError{
1903+Code: http.StatusForbidden,
1904+Msg: "Account already linked",
1905+Detail: "This account is already linked to a different identity provider subject. Contact your administrator.",
1906+RenderStaticPage: true,
1907+ }
1908+ }
1909+1910+// Backfill linked_id for legacy links.
1911+if link.LinkedID == "" && params.LinkedID != "" {
1912+//nolint:gocritic // System needs to update the user link.
1913+link, err = tx.UpdateUserLinkedID(dbauthz.AsSystemRestricted(ctx), database.UpdateUserLinkedIDParams{
1914+LinkedID: params.LinkedID,
1915+UserID: user.ID,
1916+LoginType: params.LoginType,
1917+ })
1918+if err != nil {
1919+return xerrors.Errorf("backfill user linked id: %w", err)
1920+ }
1921+ }
18731922 }
1874192318751924err = api.IDPSync.SyncOrganizations(ctx, tx, user, params.OrganizationSync)
@@ -2090,9 +2139,17 @@ func oidcLinkedID(tok *oidc.IDToken) string {
20902139return strings.Join([]string{tok.Issuer, tok.Subject}, "||")
20912140}
209221412142+// errLinkedIDAlreadyBound is returned by findLinkedUser when the user
2143+// found by email already has a user_link with a different linked_id.
2144+var errLinkedIDAlreadyBound = xerrors.New("user account is already linked to a different identity provider subject")
2145+20932146// findLinkedUser tries to find a user by their unique OAuth-linked ID.
2094-// If it doesn't not find it, it returns the user by their email.
2095-func findLinkedUser(ctx context.Context, db database.Store, linkedID string, emails ...string) (database.User, database.UserLink, error) {
2147+// If it does not find a match, it falls back to email-based lookup.
2148+// The email fallback is restricted to first-time account linking and
2149+// legacy links (empty linked_id) only. If the user found by email
2150+// already has a link with a different linked_id, errLinkedIDAlreadyBound
2151+// is returned to prevent account takeover via IdP email reuse.
2152+func findLinkedUser(ctx context.Context, db database.Store, linkedID string, loginType database.LoginType, emails ...string) (database.User, database.UserLink, error) {
20962153var (
20972154user database.User
20982155link database.UserLink
@@ -2137,12 +2194,19 @@ func findLinkedUser(ctx context.Context, db database.Store, linkedID string, ema
21372194// possible that a user_link exists without a populated 'linked_id'.
21382195link, err = db.GetUserLinkByUserIDLoginType(ctx, database.GetUserLinkByUserIDLoginTypeParams{
21392196UserID: user.ID,
2140-LoginType: user.LoginType,
2197+LoginType: loginType,
21412198 })
21422199if err != nil && !errors.Is(err, sql.ErrNoRows) {
21432200return database.User{}, database.UserLink{}, xerrors.Errorf("get user link by user id and login type: %w", err)
21442201 }
214522022203+// Block email fallback when an existing link has a different linked_id.
2204+// Prevents account takeover via IdP email reuse; first-time and legacy
2205+// (empty linked_id) links pass through.
2206+if err == nil && link.LinkedID != "" && link.LinkedID != linkedID {
2207+return database.User{}, database.UserLink{}, errLinkedIDAlreadyBound
2208+ }
2209+21462210return user, link, nil
21472211}
21482212