fix(coderd)!: restrict OIDC email fallback to first-time account link… · coder/coder@ed7e924
@@ -1033,7 +1033,16 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) {
10331033 })
10341034return
10351035 }
1036-user, link, err := findLinkedUser(ctx, api.Database, githubLinkedID(ghUser), verifiedEmail.GetEmail())
1036+user, link, err := findLinkedUser(ctx, api.Database, githubLinkedID(ghUser), database.LoginTypeGithub, verifiedEmail.GetEmail())
1037+if errors.Is(err, errLinkedIDAlreadyBound) {
1038+logger.Warn(ctx, "oauth2: blocked login, account already linked to different identity",
1039+slog.F("email", verifiedEmail.GetEmail()),
1040+ )
1041+httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{
1042+Message: "This account is already linked to a different identity provider subject.",
1043+ })
1044+return
1045+ }
10371046if err != nil {
10381047logger.Error(ctx, "oauth2: unable to find linked user", slog.F("gh_user", ghUser.Name), slog.Error(err))
10391048httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
@@ -1418,7 +1427,21 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
1418142714191428ctx = slog.With(ctx, slog.F("email", email), slog.F("username", username), slog.F("name", name))
142014291421-user, link, err := findLinkedUser(ctx, api.Database, oidcLinkedID(idToken), email)
1430+user, link, err := findLinkedUser(ctx, api.Database, oidcLinkedID(idToken), database.LoginTypeOIDC, email)
1431+if errors.Is(err, errLinkedIDAlreadyBound) {
1432+logger.Warn(ctx, "oauth2: blocked login, account already linked to different identity",
1433+slog.F("email", email),
1434+ )
1435+site.RenderStaticErrorPage(rw, r, site.ErrorPageData{
1436+Status: http.StatusForbidden,
1437+HideStatus: true,
1438+Title: "Account already linked",
1439+Description: "This account is already linked to a different identity provider subject. Contact your administrator.",
1440+AdditionalButtonLink: "/login",
1441+AdditionalButtonText: "Back to login",
1442+ })
1443+return
1444+ }
14221445if err != nil {
14231446logger.Error(ctx, "oauth2: unable to find linked user", slog.F("email", email), slog.Error(err))
14241447httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
@@ -1851,6 +1874,31 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C
18511874if err != nil {
18521875return xerrors.Errorf("update user link: %w", err)
18531876 }
1877+1878+// Defense-in-depth: if a concurrent transaction backfilled
1879+// linked_id between findLinkedUser and this point, reject the
1880+// login with a 403 instead of letting it bubble up as a 500.
1881+if link.LinkedID != "" && link.LinkedID != params.LinkedID {
1882+return &idpsync.HTTPError{
1883+Code: http.StatusForbidden,
1884+Msg: "Account already linked",
1885+Detail: "This account is already linked to a different identity provider subject. Contact your administrator.",
1886+RenderStaticPage: true,
1887+ }
1888+ }
1889+1890+// Backfill linked_id for legacy links.
1891+if link.LinkedID == "" && params.LinkedID != "" {
1892+//nolint:gocritic // System needs to update the user link.
1893+link, err = tx.UpdateUserLinkedID(dbauthz.AsSystemRestricted(ctx), database.UpdateUserLinkedIDParams{
1894+LinkedID: params.LinkedID,
1895+UserID: user.ID,
1896+LoginType: params.LoginType,
1897+ })
1898+if err != nil {
1899+return xerrors.Errorf("backfill user linked id: %w", err)
1900+ }
1901+ }
18541902 }
1855190318561904err = api.IDPSync.SyncOrganizations(ctx, tx, user, params.OrganizationSync)
@@ -2071,9 +2119,17 @@ func oidcLinkedID(tok *oidc.IDToken) string {
20712119return strings.Join([]string{tok.Issuer, tok.Subject}, "||")
20722120}
207321212122+// errLinkedIDAlreadyBound is returned by findLinkedUser when the user
2123+// found by email already has a user_link with a different linked_id.
2124+var errLinkedIDAlreadyBound = xerrors.New("user account is already linked to a different identity provider subject")
2125+20742126// findLinkedUser tries to find a user by their unique OAuth-linked ID.
2075-// If it doesn't not find it, it returns the user by their email.
2076-func findLinkedUser(ctx context.Context, db database.Store, linkedID string, emails ...string) (database.User, database.UserLink, error) {
2127+// If it does not find a match, it falls back to email-based lookup.
2128+// The email fallback is restricted to first-time account linking and
2129+// legacy links (empty linked_id) only. If the user found by email
2130+// already has a link with a different linked_id, errLinkedIDAlreadyBound
2131+// is returned to prevent account takeover via IdP email reuse.
2132+func findLinkedUser(ctx context.Context, db database.Store, linkedID string, loginType database.LoginType, emails ...string) (database.User, database.UserLink, error) {
20772133var (
20782134user database.User
20792135link database.UserLink
@@ -2118,12 +2174,19 @@ func findLinkedUser(ctx context.Context, db database.Store, linkedID string, ema
21182174// possible that a user_link exists without a populated 'linked_id'.
21192175link, err = db.GetUserLinkByUserIDLoginType(ctx, database.GetUserLinkByUserIDLoginTypeParams{
21202176UserID: user.ID,
2121-LoginType: user.LoginType,
2177+LoginType: loginType,
21222178 })
21232179if err != nil && !errors.Is(err, sql.ErrNoRows) {
21242180return database.User{}, database.UserLink{}, xerrors.Errorf("get user link by user id and login type: %w", err)
21252181 }
212621822183+// Block email fallback when an existing link has a different linked_id.
2184+// Prevents account takeover via IdP email reuse; first-time and legacy
2185+// (empty linked_id) links pass through.
2186+if err == nil && link.LinkedID != "" && link.LinkedID != linkedID {
2187+return database.User{}, database.UserLink{}, errLinkedIDAlreadyBound
2188+ }
2189+21272190return user, link, nil
21282191}
21292192