Make HTTP middleware easier to use

server/settings/admin/handlers.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 admin implements HTTP routes for settings UI.
 package admin
 
 import (
         "html/template"
         "net"
         "net/http"
 
-        "github.com/julienschmidt/httprouter"
         "golang.org/x/net/context"
 
         "github.com/luci/luci-go/common/errors"
 
         "github.com/luci/luci-go/server/auth"
         "github.com/luci/luci-go/server/auth/identity"
         "github.com/luci/luci-go/server/auth/xsrf"
-        "github.com/luci/luci-go/server/middleware"
+        "github.com/luci/luci-go/server/router"
         "github.com/luci/luci-go/server/templates"
 
         "github.com/luci/luci-go/server/settings/admin/internal/assets"
 )
 
 // InstallHandlers installs HTTP handlers that implement admin UI.
 //
 // `adminAuth` is the method that will be used to authenticate the access
 // (regardless of what's installed in the base context). It must be able to
 // distinguish admins (aka superusers) from non-admins. It is needed because
 // settings UI must be usable even before auth system is configured.
-func InstallHandlers(r *httprouter.Router, base middleware.Base, adminAuth auth.Method) {
+func InstallHandlers(r *router.Router, base router.MiddlewareChain, adminAuth auth.Method) {
         tmpl := &templates.Bundle{
                 Loader:          templates.AssetsLoader(assets.Assets()),
                 DefaultTemplate: "base",
                 FuncMap: template.FuncMap{
                         "includeCSS": func(name string) template.CSS {
                                 return template.CSS(assets.GetAsset(name))
                         },
                 },
                 DefaultArgs: func(c context.Context) (templates.Args, error) {
                         logoutURL, err := auth.LogoutURL(c, "/")
                         if err != nil {
                                 return nil, err
                         }
                         return templates.Args{
                                 "Email":     auth.CurrentUser(c).Email,
                                 "LogoutURL": logoutURL,
                         }, nil
                 },
         }
 
         adminDB := adminBypassDB{
                 auth.ErroringDB{
                         Error: errors.New("admin: unexpected call to auth.DB on admin page"),
                 },
         }
 
-        wrap := func(h middleware.Handler) httprouter.Handle {
-                h = adminOnly(h)
-                h = auth.WithDB(h, func(c context.Context) (auth.DB, error) {
+        rr := r.Subrouter("/admin/settings")
+        rr.Use(append(

[Vadim Sh., 2016/06/18 16:57:20]

I think it should be in reverse now?

base, // basic setup #1
templates.WithTemplates(tmpl), // basic setup #2
auth.Use(auth.Authenticator{adminAuth}), // auth setup #1
auth.WithDB{...}, // auth setup #2
auth.Autologin, // actual auth step
adminOnly // additional auth step

[nishanths, 2016/06/19 02:47:02]

You're right. Done.

+                base,
+                auth.Autologin,
+                adminOnly,
+                auth.WithDB(func(c context.Context) (auth.DB, error) {
                         return adminDB, nil
-                })
-                h = auth.Use(h, auth.Authenticator{adminAuth})
-                h = templates.WithTemplates(h, tmpl)
-                return base(h)
-        }
+                }),
+                auth.Use(auth.Authenticator{adminAuth}),
+                templates.WithTemplates(tmpl),
+        ))
 
-        r.GET("/admin/settings", wrap(indexPage))
-        r.GET("/admin/settings/:SettingsKey", wrap(settingsPageGET))
-        r.POST("/admin/settings/:SettingsKey", wrap(xsrf.WithTokenCheck(settingsPagePOST)))
+        rr.GET("/", nil, indexPage)

[nishanths, 2016/06/19 02:47:02]

This change expects that clients follow redirects. 

Previously: /admin/settings
After CL: /admin/settings/

Is this alright?

[nishanths, 2016/06/20 15:14:29]

Changed in Patch Set 33.

+        rr.GET("/:SettingsKey", nil, settingsPageGET)
+        rr.POST("/:SettingsKey", router.MiddlewareChain{xsrf.WithTokenCheck}, settingsPagePOST)
 }
 
 // replyError sends HTML error page with status 500 on transient errors or 400
 // on fatal ones.
 func replyError(c context.Context, rw http.ResponseWriter, err error) {
         if errors.IsTransient(err) {
                 rw.WriteHeader(http.StatusInternalServerError)
         } else {
                 rw.WriteHeader(http.StatusBadRequest)
         }
@@ skipping 18 matching lines @@
         return "", nil
 }
 
 func (adminBypassDB) IsInWhitelist(c context.Context, ip net.IP, whitelist string) (bool, error) {
         return false, nil
 }
 
 // adminOnly is middleware that ensures authenticated user is local site admin
 // aka superuser. On GAE it grants access only to users that have Editor or
 // Owner roles in the Cloud Project.
-func adminOnly(h middleware.Handler) middleware.Handler {
-        return auth.Autologin(func(c context.Context, rw http.ResponseWriter, r *http.Request, p httprouter.Params) {
-                if !auth.CurrentUser(c).Superuser {
-                        rw.WriteHeader(http.StatusForbidden)
-                        templates.MustRender(c, rw, "pages/access_denied.html", nil)
-                        return
-                }
-                h(c, rw, r, p)
-        })
+func adminOnly(c *router.Context, next router.Handler) {
+        if !auth.CurrentUser(c.Context).Superuser {

[nishanths, 2016/06/19 02:47:02]

Note: auth.Autologin is not coupled with adminOnly any longer. 

+                c.Writer.WriteHeader(http.StatusForbidden)
+                templates.MustRender(c.Context, c.Writer, "pages/access_denied.html", nil)
+                return
+        }
+        next(c)
 }