Milo: Add themed page for errors

milo/appengine/buildbot/master.go

 // Copyright 2016 The LUCI Authors. All rights reserved.
 // Use of this source code is governed under the Apache License, Version 2.0
 // that can be found in the LICENSE file.
 
 package buildbot
 
 import (
         "bytes"
         "compress/gzip"
         "encoding/json"
         "fmt"
         "sort"
         "time"
 
         ds "github.com/luci/gae/service/datastore"
         "github.com/luci/luci-go/common/logging"
         "github.com/luci/luci-go/milo/api/resp"
         "github.com/luci/luci-go/milo/appengine/settings"
+        "github.com/luci/luci-go/server/auth"
+        "github.com/luci/luci-go/server/auth/identity"
 
         "golang.org/x/net/context"
 )
 
 func decodeMasterEntry(
         c context.Context, entry *buildbotMasterEntry, master *buildbotMaster) error {
 
         reader, err := gzip.NewReader(bytes.NewReader(entry.Data))
         if err != nil {
                 return err
         }
         defer reader.Close()
         if err = json.NewDecoder(reader).Decode(master); err != nil {
                 return err
         }
         return nil
 }
 
 // getMasterEntry feches the named master and does an ACL check on the
 // current user.
+// It returns:
+// User not logged in, master found, master public: 200
+// User not logged in, master not found: 401
+// User not logged in, master internal: 401
+// User logged in, master found, master internal: 200
+// User logged in, master not found: 404
+// User logged in, master found, master internal: 404
+// Other error: 500
 func getMasterEntry(c context.Context, name string) (*buildbotMasterEntry, error) {
         entry := buildbotMasterEntry{Name: name}
         err := ds.Get(c, &entry)
+        cu := auth.CurrentUser(c)
+
         switch {
         case err == ds.ErrNoSuchEntity:
-                return nil, errMasterNotFound
+                if cu.Identity == identity.AnonymousIdentity {

[estaab, 2016/12/01 00:04:34]

It seems pretty dangerous to have the auth checking code duplicated. If the
logic changes and we forget to update one we can get into trouble. Can you make
this a CheckAccess() method that's shared between both access sites?

[hinoka, 2016/12/01 02:08:54]

Done.

+                        return nil, errNotAuth
+                } else {
+                        return nil, errMasterNotFound
+                }
         case err != nil:
                 logging.WithError(err).Errorf(
                         c, "Encountered error while fetching entry for %s:\n%s", name, err)
                 return nil, err
         }
 
         // Do the ACL check if the entry is internal.
         if entry.Internal {
                 allowed, err := settings.IsAllowedInternal(c)
                 if err != nil {
                         return nil, err
                 }
                 if !allowed {
-                        return nil, errMasterNotFound
+                        if cu.Identity == identity.AnonymousIdentity {
+                                return nil, errNotAuth
+                        } else {
+                                return nil, errMasterNotFound
+                        }
                 }
         }
 
         return &entry, nil
 }
 
 // getMasterJSON fetches the latest known buildbot master data and returns
 // the buildbotMaster struct (if found), whether or not it is internal,
 // the last modified time, and an error if not found.
 func getMasterJSON(c context.Context, name string) (
@@ skipping 54 matching lines @@
                                 Label: bn,
                                 // Go templates escapes this for us, and also
                                 // slashes are not allowed in builder names.
                                 URL: fmt.Sprintf("/buildbot/%s/%s", entry.Name, bn),
                         })
                 }
                 result.Masters = append(result.Masters, ml)
         }
         return result, nil
 }