LogDog: Project READ access for user endpoints.

appengine/logdog/coordinator/auth.go

Index: appengine/logdog/coordinator/auth.go
diff --git a/appengine/logdog/coordinator/auth.go b/appengine/logdog/coordinator/auth.go
index 6c60d88ca59d415af90a354108c0e55131d3f090..6626b12a8684881bc9157f8872cab20941b271f8 100644
--- a/appengine/logdog/coordinator/auth.go
+++ b/appengine/logdog/coordinator/auth.go
@@ -5,11 +5,10 @@
 package coordinator
 
 import (
-	"errors"
 	"fmt"
+	"strings"
 
 	"github.com/luci/gae/service/info"
-	"github.com/luci/luci-go/appengine/logdog/coordinator/config"
 	log "github.com/luci/luci-go/common/logging"
 	"github.com/luci/luci-go/common/proto/logdog/svcconfig"
 	"github.com/luci/luci-go/server/auth"
@@ -17,73 +16,81 @@ import (
 	"golang.org/x/net/context"
 )
 
-// ErrNoAccess is returned if the user has no access to the requested project.
-var ErrNoAccess = config.ErrNoAccess
-
 // IsAdminUser tests whether the current user belongs to the administrative
-// users group. It will return an error if the user does not.
+// users group.
+//

[nodir, 2016/05/19 17:17:20]

I think separating these two paragraphs is unnecessary now

[dnj (Google), 2016/05/19 20:10:46]

I'm okay with it. I like to separate the purpose of the function from its
behavior details.

+// If the user is not, a MembershipError will be returned.
 func IsAdminUser(c context.Context) error {
-	return isMember(c, func(cfg *svcconfig.Coordinator) string {
-		return cfg.AdminAuthGroup
-	})
+	cfg, err := GetServices(c).Config(c)
+	if err != nil {
+		return err
+	}
+	return checkMember(c, cfg.Coordinator.AdminAuthGroup)
 }
 
 // IsServiceUser tests whether the current user belongs to the backend services
-// users group. It will return an error if the user does not.
+// users group.
+//
+// If the user is not, a MembershipError will be returned.
 func IsServiceUser(c context.Context) error {
-	return isMember(c, func(cfg *svcconfig.Coordinator) string {
-		return cfg.ServiceAuthGroup
-	})
+	cfg, err := GetServices(c).Config(c)
+	if err != nil {
+		return err
+	}
+	return checkMember(c, cfg.Coordinator.ServiceAuthGroup)
+}
+
+// IsProjectReader tests whether the current user belongs to one of the
+// project's declared reader groups.
+//
+// If the user is not, a MembershipError will be returned.
+func IsProjectReader(c context.Context, pcfg *svcconfig.ProjectConfig) error {
+	return checkMember(c, pcfg.ReaderAuthGroups...)
 }
 
-func isMember(c context.Context, groupNameFunc func(*svcconfig.Coordinator) string) error {
+func checkMember(c context.Context, groups ...string) error {
 	// On dev-appserver, the superuser has implicit group membership to
 	// everything.
 	if info.Get(c).IsDevAppServer() {
 		if u := auth.CurrentUser(c); u.Superuser {
 			log.Fields{
 				"identity": u.Identity,
+				"groups":   groups,
 			}.Infof(c, "Granting superuser implicit group membership on development server.")
 			return nil
 		}
 	}
 
-	cfg, err := GetServices(c).Config(c)
-	if err != nil {
-		return err
-	}
-
-	if cfg.Coordinator == nil {
-		return errors.New("no coordinator configuration")
-	}
-
-	groupName := groupNameFunc(cfg.Coordinator)
-	if groupName == "" {
-		return errors.New("no auth group is configured")
+	id := auth.CurrentIdentity(c)
+	for _, group := range groups {
+		is, err := auth.IsMember(c, group)
+		if err != nil {
+			return err
+		}
+		if is {
+			log.Fields{
+				"identity": id,
+				"group":    group,
+			}.Debugf(c, "User access granted.")
+			return nil
+		}
 	}
 
-	is, err := auth.IsMember(c, groupName)
-	if err != nil {
-		return err
-	}
-	if !is {
-		return &MembershipError{
-			Identity: auth.CurrentIdentity(c),
-			Group:    groupName,
-		}
+	return &MembershipError{
+		Identity: id,
+		Groups:   groups,
 	}
-	return nil
 }
 
 // MembershipError is an error returned by group membership checking functions
 // if the current identity is not a member of the requested group.
 type MembershipError struct {
 	Identity identity.Identity
-	Group    string
+	Groups   []string
 }
 
 func (e *MembershipError) Error() string {
-	return fmt.Sprintf("user %q is not a member of group %q", e.Identity, e.Group)
+	return fmt.Sprintf("user %q is not a member of [%s]", e.Identity, strings.Join(e.Groups, ", "))
 }
 
 // IsMembershipError returns whether a given error is a membership error.