make avatar lookup occur at image request (#10540)

speed up page generation by making avatar lookup occur at the browser
not at page generation

* Protect against evil email address ".."

* hash the complete email address

Signed-off-by: Andrew Thornton <art27@cantab.net>

Co-Authored-By: Lauris BH <lauris@nix.lv>

13 files changed, 154 insertions, 21 deletions

diff --git a/models/avatar.go b/models/avatar.go
new file mode 100644
index 0000000000..311d714629
--- /dev/null
+++ b/models/avatar.go
@@ -0,0 +1,48 @@
+// Copyright 2020 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package models
+
+import (
+	"crypto/md5"
+	"fmt"
+	"net/url"
+	"strings"
+
+	"code.gitea.io/gitea/modules/cache"
+	"code.gitea.io/gitea/modules/setting"
+)
+
+// EmailHash represents a pre-generated hash map
+type EmailHash struct {
+	Hash  string `xorm:"pk varchar(32)"`
+	Email string `xorm:"UNIQUE NOT NULL"`
+}
+
+// GetEmailForHash converts a provided md5sum to the email
+func GetEmailForHash(md5Sum string) (string, error) {
+	return cache.GetString("Avatar:"+md5Sum, func() (string, error) {
+		emailHash := EmailHash{
+			Hash: strings.ToLower(strings.TrimSpace(md5Sum)),
+		}
+
+		_, err := x.Get(&emailHash)
+		return emailHash.Email, err
+	})
+}
+
+// AvatarLink returns an avatar link for a provided email
+func AvatarLink(email string) string {
+	lowerEmail := strings.ToLower(strings.TrimSpace(email))
+	sum := fmt.Sprintf("%x", md5.Sum([]byte(lowerEmail)))
+	_, _ = cache.GetString("Avatar:"+sum, func() (string, error) {
+		emailHash := &EmailHash{
+			Email: lowerEmail,
+			Hash:  sum,
+		}
+		_, _ = x.Insert(emailHash)
+		return lowerEmail, nil
+	})
+	return setting.AppSubURL + "/avatar/" + url.PathEscape(sum)
+}
diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go
index c554121e85..3f18a18c6d 100644
--- a/models/migrations/migrations.go
+++ b/models/migrations/migrations.go
@@ -198,6 +198,8 @@ var migrations = []Migration{
 	NewMigration("Add IsSystemWebhook column to webhooks table", addSystemWebhookColumn),
 	// v132 -> v133
 	NewMigration("Add Branch Protection Protected Files Column", addBranchProtectionProtectedFilesColumn),
+	// v133 -> v134
+	NewMigration("Add EmailHash Table", addEmailHashTable),
 }
 
 // Migrate database to current version
diff --git a/models/migrations/v133.go b/models/migrations/v133.go
new file mode 100644
index 0000000000..ea0411d470
--- /dev/null
+++ b/models/migrations/v133.go
@@ -0,0 +1,16 @@
+// Copyright 2020 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package migrations
+
+import "xorm.io/xorm"
+
+func addEmailHashTable(x *xorm.Engine) error {
+	// EmailHash represents a pre-generated hash map
+	type EmailHash struct {
+		Hash  string `xorm:"pk varchar(32)"`
+		Email string `xorm:"UNIQUE NOT NULL"`
+	}
+	return x.Sync2(new(EmailHash))
+}
diff --git a/models/models.go b/models/models.go
index d2872422e3..3bf7713955 100644
--- a/models/models.go
+++ b/models/models.go
@@ -124,6 +124,7 @@ func init() {
 		new(OAuth2Grant),
 		new(Task),
 		new(LanguageStat),
+		new(EmailHash),
 	)
 
 	gonicNames := []string{"SSL", "UID"}
diff --git a/modules/base/tool.go b/modules/base/tool.go
index 86606c8bee..157bd9bc3d 100644
--- a/modules/base/tool.go
+++ b/modules/base/tool.go
@@ -193,11 +193,32 @@ func SizedAvatarLink(email string, size int) string {
 	return avatarURL.String()
 }
 
-// AvatarLink returns relative avatar link to the site domain by given email,
-// which includes app sub-url as prefix. However, it is possible
-// to return full URL if user enables Gravatar-like service.
-func AvatarLink(email string) string {
-	return SizedAvatarLink(email, DefaultAvatarSize)
+// SizedAvatarLinkWithDomain returns a sized link to the avatar for the given email
+// address.
+func SizedAvatarLinkWithDomain(email string, size int) string {
+	var avatarURL *url.URL
+	if setting.EnableFederatedAvatar && setting.LibravatarService != nil {
+		var err error
+		avatarURL, err = libravatarURL(email)
+		if err != nil {
+			return DefaultAvatarLink()
+		}
+	} else if !setting.DisableGravatar {
+		// copy GravatarSourceURL, because we will modify its Path.
+		copyOfGravatarSourceURL := *setting.GravatarSourceURL
+		avatarURL = &copyOfGravatarSourceURL
+		avatarURL.Path = path.Join(avatarURL.Path, HashEmail(email))
+	} else {
+		return DefaultAvatarLink()
+	}
+
+	vals := avatarURL.Query()
+	vals.Set("d", "identicon")
+	if size != DefaultAvatarSize {
+		vals.Set("s", strconv.Itoa(size))
+	}
+	avatarURL.RawQuery = vals.Encode()
+	return avatarURL.String()
 }
 
 // FileSize calculates the file size and generate user-friendly string.
diff --git a/modules/base/tool_test.go b/modules/base/tool_test.go
index 075b5ed817..9c1a79e3f2 100644
--- a/modules/base/tool_test.go
+++ b/modules/base/tool_test.go
@@ -90,17 +90,6 @@ func TestSizedAvatarLink(t *testing.T) {
 	)
 }
 
-func TestAvatarLink(t *testing.T) {
-	disableGravatar()
-	assert.Equal(t, "/img/avatar_default.png", AvatarLink("gitea@example.com"))
-
-	enableGravatar(t)
-	assert.Equal(t,
-		"https://secure.gravatar.com/avatar/353cbad9b58e69c96154ad99f92bedc7?d=identicon",
-		AvatarLink("gitea@example.com"),
-	)
-}
-
 func TestFileSize(t *testing.T) {
 	var size int64 = 512
 	assert.Equal(t, "512 B", FileSize(size))
diff --git a/modules/cache/cache.go b/modules/cache/cache.go
index e3a905e3fa..859f4a4b47 100644
--- a/modules/cache/cache.go
+++ b/modules/cache/cache.go
@@ -41,6 +41,34 @@ func NewContext() error {
 	return err
 }
 
+// GetString returns the key value from cache with callback when no key exists in cache
+func GetString(key string, getFunc func() (string, error)) (string, error) {
+	if conn == nil || setting.CacheService.TTL == 0 {
+		return getFunc()
+	}
+	if !conn.IsExist(key) {
+		var (
+			value string
+			err   error
+		)
+		if value, err = getFunc(); err != nil {
+			return value, err
+		}
+		err = conn.Put(key, value, int64(setting.CacheService.TTL.Seconds()))
+		if err != nil {
+			return "", err
+		}
+	}
+	value := conn.Get(key)
+	if v, ok := value.(string); ok {
+		return v, nil
+	}
+	if v, ok := value.(fmt.Stringer); ok {
+		return v.String(), nil
+	}
+	return fmt.Sprintf("%s", conn.Get(key)), nil
+}
+
 // GetInt returns key value from cache with callback when no key exists in cache
 func GetInt(key string, getFunc func() (int, error)) (int, error) {
 	if conn == nil || setting.CacheService.TTL == 0 {
diff --git a/modules/repository/commits.go b/modules/repository/commits.go
index 7345aaae24..e02f3d11ca 100644
--- a/modules/repository/commits.go
+++ b/modules/repository/commits.go
@@ -10,7 +10,6 @@ import (
 	"time"
 
 	"code.gitea.io/gitea/models"
-	"code.gitea.io/gitea/modules/base"
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/log"
 	api "code.gitea.io/gitea/modules/structs"
@@ -124,7 +123,7 @@ func (pc *PushCommits) AvatarLink(email string) string {
 		var err error
 		u, err = models.GetUserByEmail(email)
 		if err != nil {
-			pc.avatars[email] = base.AvatarLink(email)
+			pc.avatars[email] = models.AvatarLink(email)
 			if !models.IsErrUserNotExist(err) {
 				log.Error("GetUserByEmail: %v", err)
 				return ""
diff --git a/modules/repository/commits_test.go b/modules/repository/commits_test.go
index 2f61ce3329..cb00e19c2e 100644
--- a/modules/repository/commits_test.go
+++ b/modules/repository/commits_test.go
@@ -6,6 +6,8 @@ package repository
 
 import (
 	"container/list"
+	"crypto/md5"
+	"fmt"
 	"testing"
 	"time"
 
@@ -114,7 +116,7 @@ func TestPushCommits_AvatarLink(t *testing.T) {
 		pushCommits.AvatarLink("user2@example.com"))
 
 	assert.Equal(t,
-		"https://secure.gravatar.com/avatar/19ade630b94e1e0535b3df7387434154?d=identicon",
+		"/avatar/"+fmt.Sprintf("%x", md5.Sum([]byte("nonexistent@example.com"))),
 		pushCommits.AvatarLink("nonexistent@example.com"))
 }
 
diff --git a/modules/templates/helper.go b/modules/templates/helper.go
index 9d3206934e..b5b4987427 100644
--- a/modules/templates/helper.go
+++ b/modules/templates/helper.go
@@ -85,7 +85,7 @@ func NewFuncMap() []template.FuncMap {
 		"AllowedReactions": func() []string {
 			return setting.UI.Reactions
 		},
-		"AvatarLink":    base.AvatarLink,
+		"AvatarLink":    models.AvatarLink,
 		"Safe":          Safe,
 		"SafeJS":        SafeJS,
 		"Str2html":      Str2html,
diff --git a/routers/repo/blame.go b/routers/repo/blame.go
index f5a2a548e3..beed59ea97 100644
--- a/routers/repo/blame.go
+++ b/routers/repo/blame.go
@@ -230,7 +230,7 @@ func renderBlame(ctx *context.Context, blameParts []git.BlamePart, commitNames m
 					}
 					avatar = fmt.Sprintf(`<a href="%s/%s"><img class="ui avatar image" src="%s" title="%s" alt=""/></a>`, setting.AppSubURL, url.PathEscape(commit.User.Name), commit.User.RelAvatarLink(), html.EscapeString(authorName))
 				} else {
-					avatar = fmt.Sprintf(`<img class="ui avatar image" src="%s" title="%s"/>`, html.EscapeString(base.AvatarLink(commit.Author.Email)), html.EscapeString(commit.Author.Name))
+					avatar = fmt.Sprintf(`<img class="ui avatar image" src="%s" title="%s"/>`, html.EscapeString(models.AvatarLink(commit.Author.Email)), html.EscapeString(commit.Author.Name))
 				}
 				commitInfo.WriteString(fmt.Sprintf(`<div class="blame-info%s"><div class="blame-data"><div class="blame-avatar">%s</div><div class="blame-message"><a href="%s/commit/%s" title="%[5]s">%[5]s</a></div><div class="blame-time">%s</div></div></div>`, attr, avatar, repoLink, part.Sha, html.EscapeString(commit.CommitMessage), commitSince))
 			} else {
diff --git a/routers/routes/routes.go b/routers/routes/routes.go
index 093edcd920..459aa4d09f 100644
--- a/routers/routes/routes.go
+++ b/routers/routes/routes.go
@@ -417,6 +417,8 @@ func RegisterRoutes(m *macaron.Macaron) {
 	})
 	// ***** END: User *****
 
+	m.Get("/avatar/:hash", user.AvatarByEmailHash)
+
 	adminReq := context.Toggle(&context.ToggleOptions{SignInRequired: true, AdminRequired: true})
 
 	// ***** START: Admin *****
diff --git a/routers/user/avatar.go b/routers/user/avatar.go
index 045206c50a..32d05f03cc 100644
--- a/routers/user/avatar.go
+++ b/routers/user/avatar.go
@@ -5,10 +5,12 @@
 package user
 
 import (
+	"errors"
 	"strconv"
 	"strings"
 
 	"code.gitea.io/gitea/models"
+	"code.gitea.io/gitea/modules/base"
 	"code.gitea.io/gitea/modules/context"
 	"code.gitea.io/gitea/modules/log"
 )
@@ -41,3 +43,26 @@ func Avatar(ctx *context.Context) {
 
 	ctx.Redirect(user.RealSizedAvatarLink(size))
 }
+
+// AvatarByEmailHash redirects the browser to the appropriate Avatar link
+func AvatarByEmailHash(ctx *context.Context) {
+	hash := ctx.Params(":hash")
+	if len(hash) == 0 {
+		ctx.ServerError("invalid avatar hash", errors.New("hash cannot be empty"))
+		return
+	}
+	email, err := models.GetEmailForHash(hash)
+	if err != nil {
+		ctx.ServerError("invalid avatar hash", err)
+		return
+	}
+	if len(email) == 0 {
+		ctx.Redirect(base.DefaultAvatarLink())
+		return
+	}
+	size := ctx.QueryInt("size")
+	if size == 0 {
+		size = base.DefaultAvatarSize
+	}
+	ctx.Redirect(base.SizedAvatarLinkWithDomain(email, size))
+}

path: root/lib/private/group.php

<?php
/**
 * ownCloud
 *
 * @author Frank Karlitschek
 * @copyright 2012 Frank Karlitschek frank@owncloud.org
 *
 * This library is free software; you can redistribute it and/or
 * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
 * License as published by the Free Software Foundation; either
 * version 3 of the License, or any later version.
 *
 * This library is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 * GNU AFFERO GENERAL PUBLIC LICENSE for more details.
 *
 * You should have received a copy of the GNU Affero General Public
 * License along with this library.  If not, see <http://www.gnu.org/licenses/>.
 *
 */

/**
 * This class provides all methods needed for managing groups.
 *
 * Hooks provided:
 *   pre_createGroup(&run, gid)
 *   post_createGroup(gid)
 *   pre_deleteGroup(&run, gid)
 *   post_deleteGroup(gid)
 *   pre_addToGroup(&run, uid, gid)
 *   post_addToGroup(uid, gid)
 *   pre_removeFromGroup(&run, uid, gid)
 *   post_removeFromGroup(uid, gid)
 */
class OC_Group {

	/**
	 * @return \OC\Group\Manager
	 * @deprecated Use \OC::$server->getGroupManager();
	 */
	public static function getManager() {
		return \OC::$server->getGroupManager();
	}

	/**
	 * @return \OC\User\Manager
	 * @deprecated Use \OC::$server->getUserManager()
	 */
	private static function getUserManager() {
		return \OC::$server->getUserManager();
	}

	/**
	 * set the group backend
	 * @param \OC_Group_Backend $backend  The backend to use for user managment
	 * @return bool
	 */
	public static function useBackend($backend) {
		self::getManager()->addBackend($backend);
		return true;
	}

	/**
	 * remove all used backends
	 */
	public static function clearBackends() {
		self::getManager()->clearBackends();
	}

	/**
	 * Try to create a new group
	 * @param string $gid The name of the group to create
	 * @return bool
	 *
	 * Tries to create a new group. If the group name already exists, false will
	 * be returned. Basic checking of Group name
	 * @deprecated Use \OC::$server->getGroupManager()->createGroup() instead
	 */
	public static function createGroup($gid) {
		if (self::getManager()->createGroup($gid)) {
			return true;
		} else {
			return false;
		}
	}

	/**
	 * delete a group
	 * @param string $gid gid of the group to delete
	 * @return bool
	 *
	 * Deletes a group and removes it from the group_user-table
	 * @deprecated Use \OC::$server->getGroupManager()->delete() instead
	 */
	public static function deleteGroup($gid) {
		$group = self::getManager()->get($gid);
		if ($group) {
			if ($group->delete()) {
				return true;
			}
		}
		return false;
	}

	/**
	 * is user in group?
	 * @param string $uid uid of the user
	 * @param string $gid gid of the group
	 * @return bool
	 *
	 * Checks whether the user is member of a group or not.
	 * @deprecated Use \OC::$server->getGroupManager->inGroup($user);
	 */
	public static function inGroup($uid, $gid) {
		$group = self::getManager()->get($gid);
		$user = self::getUserManager()->get($uid);
		if ($group and $user) {
			return $group->inGroup($user);
		}
		return false;
	}

	/**
	 * Add a user to a group
	 * @param string $uid Name of the user to add to group
	 * @param string $gid Name of the group in which add the user
	 * @return bool
	 *
	 * Adds a user to a group.
	 * @deprecated Use \OC::$server->getGroupManager->addUser();
	 */
	public static function addToGroup($uid, $gid) {
		$group = self::getManager()->get($gid);
		$user = self::getUserManager()->get($uid);
		if ($group and $user) {
			$group->addUser($user);
			return true;
		} else {
			return false;
		}
	}

	/**
	 * Removes a user from a group
	 * @param string $uid Name of the user to remove from group
	 * @param string $gid Name of the group from which remove the user
	 * @return bool
	 *
	 * removes the user from a group.
	 */
	public static function removeFromGroup($uid, $gid) {
		$group = self::getManager()->get($gid);
		$user = self::getUserManager()->get($uid);
		if ($group and $user) {
			OC_Hook::emit("OC_Group", "pre_removeFromGroup", array("run" => true, "uid" => $uid, "gid" => $gid));
			$group->removeUser($user);
			OC_Hook::emit("OC_User", "post_removeFromGroup", array("uid" => $uid, "gid" => $gid));
			return true;
		} else {
			return false;
		}
	}

	/**
	 * Get all groups a user belongs to
	 * @param string $uid Name of the user
	 * @return array an array of group names
	 *
	 * This function fetches all groups a user belongs to. It does not check
	 * if the user exists at all.
	 * @deprecated Use \OC::$server->getGroupManager->getuserGroupIds($user)
	 */
	public static function getUserGroups($uid) {
		$user = self::getUserManager()->get($uid);
		if ($user) {
			return self::getManager()->getUserGroupIds($user);
		} else {
			return array();
		}
	}

	/**
	 * get a list of all groups
	 * @param string $search
	 * @param int|null $limit
	 * @param int|null $offset
	 * @return array an array of group names
	 *
	 * Returns a list with all groups
	 */
	public static function getGroups($search = '', $limit = null, $offset = null) {
		$groups = self::getManager()->search($search, $limit, $offset);
		$groupIds = array();
		foreach ($groups as $group) {
			$groupIds[] = $group->getGID();
		}
		return $groupIds;
	}

	/**
	 * check if a group exists
	 *
	 * @param string $gid
	 * @return bool
	 * @deprecated Use \OC::$server->getGroupManager->groupExists($gid)
	 */
	public static function groupExists($gid) {
		return self::getManager()->groupExists($gid);
	}

	/**
	 * get a list of all users in a group
	 * @param string $gid
	 * @param string $search
	 * @param int $limit
	 * @param int $offset
	 * @return array an array of user ids
	 */
	public static function usersInGroup($gid, $search = '', $limit = -1, $offset = 0) {
		$group = self::getManager()->get($gid);
		if ($group) {
			$users = $group->searchUsers($search, $limit, $offset);
			$userIds = array();
			foreach ($users as $user) {
				$userIds[] = $user->getUID();
			}
			return $userIds;
		} else {
			return array();
		}
	}

	/**
	 * get a list of all users in several groups
	 * @param string[] $gids
	 * @param string $search
	 * @param int $limit
	 * @param int $offset
	 * @return array an array of user ids
	 */
	public static function usersInGroups($gids, $search = '', $limit = -1, $offset = 0) {
		$users = array();
		foreach ($gids as $gid) {
			// TODO Need to apply limits to groups as total
			$users = array_merge(array_diff(self::usersInGroup($gid, $search, $limit, $offset), $users), $users);
		}
		return $users;
	}

	/**
	 * get a list of all display names in a group
	 * @param string $gid
	 * @param string $search
	 * @param int $limit
	 * @param int $offset
	 * @return array an array of display names (value) and user ids(key)
	 * @deprecated Use \OC::$server->getGroupManager->displayNamesInGroup($gid, $search, $limit, $offset)
	 */
	public static function displayNamesInGroup($gid, $search = '', $limit = -1, $offset = 0) {
		return self::getManager()->displayNamesInGroup($gid, $search, $limit, $offset);
	}

	/**
	 * get a list of all display names in several groups
	 * @param array $gids
	 * @param string $search
	 * @param int $limit
	 * @param int $offset
	 * @return array an array of display names (Key) user ids (value)
	 */
	public static function displayNamesInGroups($gids, $search = '', $limit = -1, $offset = 0) {
		$displayNames = array();
		foreach ($gids as $gid) {
			// TODO Need to apply limits to groups as total
			$diff = array_diff(
				self::displayNamesInGroup($gid, $search, $limit, $offset),
				$displayNames
			);
			if ($diff) {
				// A fix for LDAP users. array_merge loses keys...
				$displayNames = $diff + $displayNames;
			}
		}
		return $displayNames;
	}
}