feat: Allow using username in user queries (coder#1221)

Emyrk · web-flow · commit 69e26c40362d · 2022-04-29T11:44:22.000-05:00
* feat: Allow using username in user queries
* Test needs a username/email to not match empty string
diff --git 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)
@@ -27,32 +34,56 @@ 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: "\"user\" must be provided",
+				})
+				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.StatusBadRequest, httpapi.Response{
+						Message: userErrorMessage,
+					})
+					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 {
+					httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{
+						Message: userErrorMessage,
+					})
 					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/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)
 
diff --git 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
@@ -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
 	}
