From f6b0835982a9ffc3307fa9890b490ca54df8fe88 Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Thu, 18 Aug 2022 17:56:17 -0500 Subject: [PATCH] fix: avoid processing updates to usernames (#3571) - With the support of OIDC we began processing updates to a user's email and username to stay in sync with the upstream provider. This can cause issues in templates that use the user's username as a stable identifier, potentially causing the deletion of user's home volumes. - Fix some faulty error wrapping. --- coderd/userauth.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/coderd/userauth.go b/coderd/userauth.go index 865f2afeb9..58d101671c 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -229,13 +229,13 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) { if link.LinkedID == "" { link, err = api.Database.UpdateUserLinkedID(ctx, database.UpdateUserLinkedIDParams{ UserID: user.ID, - LinkedID: githubLinkedID(ghUser), LoginType: database.LoginTypeGithub, + LinkedID: githubLinkedID(ghUser), }) if err != nil { httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{ Message: "A database error occurred.", - Detail: xerrors.Errorf("update user link: %w", err.Error).Error(), + Detail: fmt.Sprintf("update user link: %s", err.Error()), }) return } @@ -437,13 +437,13 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { if link.LinkedID == "" { link, err = api.Database.UpdateUserLinkedID(ctx, database.UpdateUserLinkedIDParams{ UserID: user.ID, + LoginType: database.LoginTypeOIDC, LinkedID: oidcLinkedID(idToken), - LoginType: database.LoginTypeGithub, }) if err != nil { httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{ Message: "A database error occurred.", - Detail: xerrors.Errorf("update user link: %w", err.Error).Error(), + Detail: fmt.Sprintf("update user link: %s", err.Error()), }) return } @@ -477,9 +477,10 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { // longer sign in until an administrator finds the offending built-in // user and changes their username. user, err = api.Database.UpdateUserProfile(ctx, database.UpdateUserProfileParams{ - ID: user.ID, - Email: claims.Email, - Username: claims.Username, + ID: user.ID, + Email: claims.Email, + // TODO: This should run in a transaction. + Username: user.Username, UpdatedAt: database.Now(), }) if err != nil {