From 01e3aacec95ce35f1d52a57c1d2f21e3f6641ec9 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 29 Apr 2022 11:06:57 -0500 Subject: [PATCH 1/4] feat: Allow using username in user queries --- coderd/httpmw/userparam.go | 56 ++++++++++++++++++++++++++++---------- coderd/users_test.go | 43 +++++++++++++++++++++++++---- codersdk/users.go | 11 +++++++- 3 files changed, 89 insertions(+), 21 deletions(-) diff --git a/coderd/httpmw/userparam.go b/coderd/httpmw/userparam.go index 4342cbb695c64..b5f2fcf015dd4 100644 --- a/coderd/httpmw/userparam.go +++ b/coderd/httpmw/userparam.go @@ -27,32 +27,60 @@ func UserParam(r *http.Request) database.User { func ExtractUserParam(db database.Store) func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { - var userID uuid.UUID - if chi.URLParam(r, "user") == "me" { - userID = APIKey(r).UserID + var user database.User + var err error + + // userQuery is either a uuid, a username, or 'me' + userQuery := chi.URLParam(r, "user") + if userQuery == "" { + httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ + Message: fmt.Sprintf("%q must be provided", "user"), + }) + return + } + + if userQuery == "me" { + user, err = db.GetUserByID(r.Context(), APIKey(r).UserID) + if err != nil { + httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ + Message: fmt.Sprintf("get user: %s", err.Error()), + }) + return + } + } else if userID, err := uuid.Parse(userQuery); err == nil { + // If the userQuery is a valid uuid + user, err = db.GetUserByID(r.Context(), userID) + if err != nil { + httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ + Message: fmt.Sprintf("get user: %s", err.Error()), + }) + return + } } else { - var ok bool - userID, ok = parseUUID(rw, r, "user") - if !ok { + // Try as a username last + user, err = db.GetUserByEmailOrUsername(r.Context(), database.GetUserByEmailOrUsernameParams{ + Username: userQuery, + }) + if err != nil { + // If the error is no rows, they might have inputted something + // that is not a username or uuid. Regardless, let's not indicate if + // the user exists or not. Just lump all these errors into + // something generic. + httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ + Message: fmt.Sprint("\"user\" must be a uuid or username"), + }) return } } apiKey := APIKey(r) - if apiKey.UserID != userID { + if apiKey.UserID != user.ID { httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ Message: "getting non-personal users isn't supported yet", }) return } - user, err := db.GetUserByID(r.Context(), userID) - if err != nil { - httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ - Message: fmt.Sprintf("get user: %s", err.Error()), - }) - } - ctx := context.WithValue(r.Context(), userParamContextKey{}, user) next.ServeHTTP(rw, r.WithContext(ctx)) }) diff --git a/coderd/users_test.go b/coderd/users_test.go index 536c716b36f57..531ac93be7aa6 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -431,14 +431,45 @@ func TestPutUserSuspend(t *testing.T) { }) } -func TestUserByName(t *testing.T) { +func TestGetUser(t *testing.T) { t.Parallel() - client := coderdtest.New(t, nil) - firstUser := coderdtest.CreateFirstUser(t, client) - user, err := client.User(context.Background(), codersdk.Me) - require.NoError(t, err) - require.Equal(t, firstUser.OrganizationID, user.OrganizationIDs[0]) + t.Run("ByMe", func(t *testing.T) { + t.Parallel() + + client := coderdtest.New(t, nil) + firstUser := coderdtest.CreateFirstUser(t, client) + + user, err := client.User(context.Background(), codersdk.Me) + require.NoError(t, err) + require.Equal(t, firstUser.UserID, user.ID) + require.Equal(t, firstUser.OrganizationID, user.OrganizationIDs[0]) + }) + + t.Run("ByID", func(t *testing.T) { + t.Parallel() + + client := coderdtest.New(t, nil) + firstUser := coderdtest.CreateFirstUser(t, client) + + user, err := client.User(context.Background(), firstUser.UserID) + require.NoError(t, err) + require.Equal(t, firstUser.UserID, user.ID) + require.Equal(t, firstUser.OrganizationID, user.OrganizationIDs[0]) + }) + + t.Run("ByUsername", func(t *testing.T) { + t.Parallel() + + client := coderdtest.New(t, nil) + firstUser := coderdtest.CreateFirstUser(t, client) + exp, err := client.User(context.Background(), firstUser.UserID) + require.NoError(t, err) + + user, err := client.UserByUsername(context.Background(), exp.Username) + require.NoError(t, err) + require.Equal(t, exp, user) + }) } func TestGetUsers(t *testing.T) { diff --git a/codersdk/users.go b/codersdk/users.go index a2608c8bdec3a..e2c8261febdec 100644 --- a/codersdk/users.go +++ b/codersdk/users.go @@ -274,7 +274,16 @@ func (c *Client) Logout(ctx context.Context) error { // User returns a user for the ID provided. // If the uuid is nil, the current user will be returned. func (c *Client) User(ctx context.Context, id uuid.UUID) (User, error) { - res, err := c.request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/users/%s", uuidOrMe(id)), nil) + return c.userByIdentifier(ctx, uuidOrMe(id)) +} + +// UserByUsername returns a user for the username provided. +func (c *Client) UserByUsername(ctx context.Context, username string) (User, error) { + return c.userByIdentifier(ctx, username) +} + +func (c *Client) userByIdentifier(ctx context.Context, ident string) (User, error) { + res, err := c.request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/users/%s", ident), nil) if err != nil { return User{}, err } From f3f285b63c9550c75f0c196f91a981448c3172e7 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 29 Apr 2022 11:09:12 -0500 Subject: [PATCH 2/4] Fix a string --- coderd/httpmw/userparam.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/coderd/httpmw/userparam.go b/coderd/httpmw/userparam.go index b5f2fcf015dd4..c6bbe0885dba0 100644 --- a/coderd/httpmw/userparam.go +++ b/coderd/httpmw/userparam.go @@ -34,7 +34,7 @@ func ExtractUserParam(db database.Store) func(http.Handler) http.Handler { userQuery := chi.URLParam(r, "user") if userQuery == "" { httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ - Message: fmt.Sprintf("%q must be provided", "user"), + Message: "\"user\" must be provided", }) return } @@ -67,7 +67,7 @@ func ExtractUserParam(db database.Store) func(http.Handler) http.Handler { // the user exists or not. Just lump all these errors into // something generic. httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ - Message: fmt.Sprint("\"user\" must be a uuid or username"), + Message: "\"user\" must be a uuid or username", }) return } From 8d30f1e1048020bd2f2fe1c454457b12b0622358 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 29 Apr 2022 11:20:33 -0500 Subject: [PATCH 3/4] Use same error always --- coderd/httpmw/userparam.go | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/coderd/httpmw/userparam.go b/coderd/httpmw/userparam.go index c6bbe0885dba0..ba91414de8f4c 100644 --- a/coderd/httpmw/userparam.go +++ b/coderd/httpmw/userparam.go @@ -14,6 +14,13 @@ import ( type userParamContextKey struct{} +const ( + // userErrorMessage is a constant so that no information about the state + // of the queried user can be gained. We return the same error if the user + // does not exist, or if the input is just garbage. + userErrorMessage = "\"user\" must be an existing uuid or username" +) + // UserParam returns the user from the ExtractUserParam handler. func UserParam(r *http.Request) database.User { user, ok := r.Context().Value(userParamContextKey{}).(database.User) @@ -51,8 +58,8 @@ func ExtractUserParam(db database.Store) func(http.Handler) http.Handler { // If the userQuery is a valid uuid user, err = db.GetUserByID(r.Context(), userID) if err != nil { - httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ - Message: fmt.Sprintf("get user: %s", err.Error()), + httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ + Message: userErrorMessage, }) return } @@ -62,12 +69,8 @@ func ExtractUserParam(db database.Store) func(http.Handler) http.Handler { Username: userQuery, }) if err != nil { - // If the error is no rows, they might have inputted something - // that is not a username or uuid. Regardless, let's not indicate if - // the user exists or not. Just lump all these errors into - // something generic. httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ - Message: "\"user\" must be a uuid or username", + Message: userErrorMessage, }) return } From 290d1a3fe7bb25d7208a5c4061f04ca3232cd203 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 29 Apr 2022 11:29:12 -0500 Subject: [PATCH 4/4] Test needs a username/email to not match empty string --- coderd/httpmw/userparam_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/coderd/httpmw/userparam_test.go b/coderd/httpmw/userparam_test.go index 6a7ccd69285af..48e1da72e2142 100644 --- a/coderd/httpmw/userparam_test.go +++ b/coderd/httpmw/userparam_test.go @@ -34,7 +34,9 @@ func TestUserParam(t *testing.T) { }) user, err := db.InsertUser(r.Context(), database.InsertUserParams{ - ID: uuid.New(), + ID: uuid.New(), + Email: "admin@email.com", + Username: "admin", }) require.NoError(t, err)