auth: Refactor how authentication methods are passed to server/auth library.

server/auth/auth.go

 // Copyright 2015 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 auth
 
 import (
         "fmt"
         "net/http"
 
         "golang.org/x/net/context"
 
         "github.com/luci/luci-go/common/errors"
         "github.com/luci/luci-go/common/logging"
 
         "github.com/luci/luci-go/server/auth/delegation"
         "github.com/luci/luci-go/server/auth/identity"
         "github.com/luci/luci-go/server/auth/signing"
+        "github.com/luci/luci-go/server/router"
 )
 
 var (
         // ErrNotConfigured is returned by Authenticate if auth library wasn't
         // properly initialized (see SetConfig).
         ErrNotConfigured = errors.New("auth: the library is not properly configured")
 
         // ErrNoUsersAPI is returned by LoginURL and LogoutURL if none of
         // the authentication methods support UsersAPI.
         ErrNoUsersAPI = errors.New("auth: methods do not support login or logout URL")
 
         // ErrBadClientID is returned by Authenticate if caller is using
         // non-whitelisted OAuth2 client. More info is in the log.
         ErrBadClientID = errors.New("auth: OAuth client_id is not whitelisted")
 
         // ErrIPNotWhitelisted is returned when an account is restricted by an IP
         // whitelist and request's remote_addr is not in it.
         ErrIPNotWhitelisted = errors.New("auth: IP is not whitelisted")
 )
 
-// Method implements particular kind of low level authentication mechanism for
-// incoming requests. It may also optionally implement UsersAPI (if the method
-// support login and logout URLs). Use type sniffing to figure out.
+// Method implements a particular kind of low-level authentication mechanism.
+//
+// It may also optionally implement UsersAPI (if the method support login and
+// logout URLs).
+//
+// Methods are not usually used directly, but passed to Authenticator{...} that
+// knows how to apply them.
 type Method interface {
         // Authenticate extracts user information from the incoming request.
+        //
         // It returns:
         //   * (*User, nil) on success.
         //   * (nil, nil) if the method is not applicable.
         //   * (nil, error) if the method is applicable, but credentials are invalid.
         Authenticate(context.Context, *http.Request) (*User, error)
 }
 
 // UsersAPI may be additionally implemented by Method if it supports login and
 // logout URLs.
 type UsersAPI interface {
@@ skipping 27 matching lines @@
 
         // Picture is URL of the user avatar. Optional, default "".
         Picture string
 
         // ClientID is the ID of the pre-registered OAuth2 client so its identity can
         // be verified. Used only by authentication methods based on OAuth2.
         // See https://developers.google.com/console/help/#generatingoauth2 for more.
         ClientID string
 }
 
-// Authenticator perform authentication of incoming requests. It is stateless
-// object that just describes what methods to try when authenticating current
-// request. It is fine to create it on per-request basis.
-type Authenticator []Method
+// Authenticator perform authentication of incoming requests.

[iannucci, 2017/04/19 21:58:34]

nit: performs

[Vadim Sh., 2017/04/19 22:32:47]

Done.

+//
+// It is stateless object configured with a list of methods to try when

[iannucci, 2017/04/19 21:58:34]

nit: It is a

[Vadim Sh., 2017/04/19 22:32:46]

Done.

+// authenticating incoming requests. It implements Authenticate method that
+// performs high-level authentication logic using the provided list of low-level
+// auth methods.

[iannucci, 2017/04/19 21:58:34]

How often do we actually have more than one Method?

[Vadim Sh., 2017/04/19 22:32:46]

Not often, that's why the recommended way to use this stuff is through
Authenticate(Method) function. I decided to keep Authenticator{...} public for
users that really need multiple methods. I'll add a clarifying comment that this
is "advanced API".

+type Authenticator struct {
+        Methods []Method // a list of authentication methods to try
+}
 
-// Authenticate authenticates incoming requests and returns new context.Context
-// with State stored into it. Returns error if credentials are provided, but
-// invalid. If no credentials are provided (i.e. the request is anonymous),
-// finishes successfully, but in that case State.Identity() will return
-// AnonymousIdentity.
-func (a Authenticator) Authenticate(c context.Context, r *http.Request) (context.Context, error) {
+// GetMiddleware returns a middleware that uses this Authenticator for
+// authentication.
+//
+// It uses a.Authenticate internally and handles errors appropriately.
+func (a *Authenticator) GetMiddleware() router.Middleware {
+        return func(c *router.Context, next router.Handler) {
+                ctx, err := a.Authenticate(c.Context, c.Request)
+                switch {
+                case errors.IsTransient(err):
+                        replyError(c.Context, c.Writer, 500, fmt.Sprintf("Transient error during authentication - %s", err))
+                case err != nil:
+                        replyError(c.Context, c.Writer, 401, fmt.Sprintf("Authentication error - %s", err))
+                default:
+                        c.Context = ctx
+                        next(c)
+                }
+        }
+}
+
+// Authenticate authenticates the requests and adds State into the context.
+//
+// Returns an error if credentials are provided, but invalid. If no credentials
+// are provided (i.e. the request is anonymous), finishes successfully, but in
+// that case State.Identity() returns AnonymousIdentity.
+func (a *Authenticator) Authenticate(c context.Context, r *http.Request) (context.Context, error) {
         report := durationReporter(c, authenticateDuration)
 
-        // Make it known to handlers what authentication methods are allowed.
-        // This is important for LoginURL and LogoutURL functions.
-        c = SetAuthenticator(c, a)
-
         // We will need working DB factory below to check IP whitelist.
         cfg := GetConfig(c)
-        if cfg == nil || cfg.DBProvider == nil {
+        if cfg == nil || cfg.DBProvider == nil || len(a.Methods) == 0 {
                 report(ErrNotConfigured, "ERROR_NOT_CONFIGURED")
                 return nil, ErrNotConfigured
         }
 
         // Pick first authentication method that applies.
-        var s state
-        for _, m := range a {
+        s := state{authenticator: a}
+        for _, m := range a.Methods {
                 var err error
                 s.user, err = m.Authenticate(c, r)
                 if err != nil {
                         report(err, "ERROR_BROKEN_CREDS") // e.g. malformed OAuth token
                         return nil, err
                 }
                 if s.user != nil {
                         if err = s.user.Identity.Validate(); err != nil {
                                 report(err, "ERROR_BROKEN_IDENTITY") // a weird looking email address
                                 return nil, err
@@ skipping 102 matching lines @@
                         "peerID":      s.peerIdent,
                         "delegatedID": delegatedIdentity,
                 }.Debugf(c, "auth: Using delegation")
         }
 
         // Inject auth state.
         report(nil, "SUCCESS")
         return WithState(c, &s), nil
 }
 
-// usersAPI returns implementation of UsersAPI by examining Methods. Returns nil
-// if none of Methods implement UsersAPI.
-func (a Authenticator) usersAPI() UsersAPI {
-        for _, m := range a {
+// usersAPI returns implementation of UsersAPI by examining Methods.
+//
+// Returns nil if none of Methods implement UsersAPI.
+func (a *Authenticator) usersAPI() UsersAPI {
+        for _, m := range a.Methods {
                 if api, ok := m.(UsersAPI); ok {
                         return api
                 }
         }
         return nil
 }
 
 // LoginURL returns a URL that, when visited, prompts the user to sign in,
 // then redirects the user to the URL specified by dest.
-func (a Authenticator) LoginURL(c context.Context, dest string) (string, error) {
+//
+// Returns ErrNoUsersAPI if none of the authentication methods support login
+// URLs.
+func (a *Authenticator) LoginURL(c context.Context, dest string) (string, error) {
         if api := a.usersAPI(); api != nil {
                 return api.LoginURL(c, dest)
         }
         return "", ErrNoUsersAPI
 }
 
-// LogoutURL returns a URL that, when visited, signs the user out,
-// then redirects the user to the URL specified by dest.
-func (a Authenticator) LogoutURL(c context.Context, dest string) (string, error) {
+// LogoutURL returns a URL that, when visited, signs the user out, then
+// redirects the user to the URL specified by dest.
+//
+// Returns ErrNoUsersAPI if none of the authentication methods support login
+// URLs.
+func (a *Authenticator) LogoutURL(c context.Context, dest string) (string, error) {
         if api := a.usersAPI(); api != nil {
                 return api.LogoutURL(c, dest)
         }
         return "", ErrNoUsersAPI
 }
 
 ////
 
+// replyError logs the error and writes it to ResponseWriter.
+func replyError(c context.Context, rw http.ResponseWriter, code int, msg string) {
+        logging.Errorf(c, "HTTP %d: %s", code, msg)
+        http.Error(rw, msg, code)

[iannucci, 2017/04/19 21:58:34]

Can this leak some secret info from msg -> user?

[Vadim Sh., 2017/04/19 22:32:47]

I don't think it can. But just to be sure, removed error propagation to the
client (keeping it in the log).

+}
+
 // getOwnServiceIdentity returns 'service:<appID>' identity of the current
 // service.
 func getOwnServiceIdentity(c context.Context, signer signing.Signer) (identity.Identity, error) {
         if signer == nil {
                 return "", ErrNotConfigured
         }
         serviceInfo, err := signer.ServiceInfo(c)
         if err != nil {
                 return "", err
         }
         return identity.MakeIdentity("service:" + serviceInfo.AppID)
 }