[auth] Decode token once

cmds/core-service/main.go

@@ -27,6 +27,7 @@ import (
 	"github.com/interuss/dss/pkg/build"
 	"github.com/interuss/dss/pkg/datastore"
 	"github.com/interuss/dss/pkg/datastore/flags" // Force command line flag registration
+	dsserr "github.com/interuss/dss/pkg/errors"
 	"github.com/interuss/dss/pkg/logging"
 	"github.com/interuss/dss/pkg/rid/application"
 	rid_v1 "github.com/interuss/dss/pkg/rid/server/v1"
@@ -314,12 +315,10 @@ func RunHTTPServer(ctx context.Context, ctxCanceler func(), address, locality st
 		multiRouter.Routers = append(multiRouter.Routers, &scdV1Router)
 	}
 
-	handler := logging.HTTPMiddleware(logger, *dumpRequests,
-		healthyEndpointMiddleware(logger,
-			&multiRouter,
-		))
-
-	handler = authDecoderMiddleware(authorizer, handler)
+	// the middlewares are wrapped and, therefore, executed in the opposite order
+	handler := healthyEndpointMiddleware(logger, &multiRouter)
+	handler = logging.HTTPMiddleware(logger, *dumpRequests, handler)
+	handler = authMiddleware(authorizer, handler)
 
 	httpServer := &http.Server{
 		Addr:              address,
@@ -381,19 +380,33 @@ func healthyEndpointMiddleware(logger *zap.Logger, next http.Handler) http.Handl
 	})
 }
 
-// authDecoderMiddleware decodes the authentication token and adds the Subject claim to the context.
-func authDecoderMiddleware(authorizer *auth.Authorizer, handler http.Handler) http.Handler {
+// authMiddleware decodes the authentication token and passes the claims to the context.
+func authMiddleware(authorizer *auth.Authorizer, handler http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		var ctx context.Context
-
 		claims, err := authorizer.ExtractClaims(r)
+		if err == nil {
+			if !authorizer.AcceptedAudiences[claims.Audience] {
+				err = stacktrace.NewErrorWithCode(dsserr.Unauthenticated, "Invalid access token audience: %v", claims.Audience)
+			}
+		}
+
+		ctx = context.WithValue(r.Context(), auth.CtxAuthKey{}, auth.CtxAuthValue{
+			Claims: claims,
+			Error:  err,
+		})
+
+		var errMsg string
 		if err != nil {
 			//remove the stacktrace using the formatting specifier "%#s"
-			ctx = context.WithValue(r.Context(), logging.CtxAuthError{}, fmt.Sprintf("%#s", err))
-		} else {
-			ctx = context.WithValue(r.Context(), logging.CtxAuthSubject{}, claims.Subject)
+			errMsg = fmt.Sprintf("%#s", err)
 		}
 
+		ctx = context.WithValue(ctx, logging.CtxAuthKey{}, logging.CtxAuthValue{
+			Subject: claims.Subject,
+			ErrMsg:  errMsg,
+		})
+
 		handler.ServeHTTP(w, r.WithContext(ctx))
 	})
 }

pkg/auth/auth.go

@@ -120,10 +120,11 @@ func (r *JWKSResolver) ResolveKeys(ctx context.Context) ([]interface{}, error) {
 
 // Authorizer authorizes incoming requests.
 type Authorizer struct {
-	logger            *zap.Logger
-	keys              []interface{}
-	keyGuard          sync.RWMutex
-	acceptedAudiences map[string]bool
+	logger   *zap.Logger
+	keys     []interface{}
+	keyGuard sync.RWMutex
+
+	AcceptedAudiences map[string]bool
 }
 
 // Configuration bundles up creation-time parameters for an Authorizer instance.
@@ -148,7 +149,7 @@ func NewRSAAuthorizer(ctx context.Context, configuration Configuration) (*Author
 	}
 
 	authorizer := &Authorizer{
-		acceptedAudiences: auds,
+		AcceptedAudiences: auds,
 		logger:            logger,
 		keys:              keys,
 	}
@@ -182,44 +183,46 @@ func (a *Authorizer) setKeys(keys []interface{}) {
 	a.keyGuard.Unlock()
 }
 
+type CtxAuthKey struct{}
+type CtxAuthValue struct {
+	Claims Claims
+	Error  error
+}
+
 // Authorize extracts and verifies bearer tokens from a http.Request.
 func (a *Authorizer) Authorize(_ http.ResponseWriter, r *http.Request, authOptions []api.AuthorizationOption) api.AuthorizationResult {
-	keyClaims, err := a.ExtractClaims(r)
-	if err != nil {
-		return api.AuthorizationResult{Error: stacktrace.PropagateWithCode(err, dsserr.Unauthenticated, "Failed to extract claims from access token")}
-	}
-
-	if !a.acceptedAudiences[keyClaims.Audience] {
-		return api.AuthorizationResult{Error: stacktrace.NewErrorWithCode(dsserr.Unauthenticated, "Invalid access token audience: %v", keyClaims.Audience)}
+	authResults := r.Context().Value(CtxAuthKey{}).(CtxAuthValue)
+	if authResults.Error != nil {
+		return api.AuthorizationResult{Error: stacktrace.PropagateWithCode(authResults.Error, dsserr.Unauthenticated, "Invalid access token")}
 	}
 
-	if pass, missing := validateScopes(authOptions, keyClaims.Scopes); !pass {
+	if pass, missing := validateScopes(authOptions, authResults.Claims.Scopes); !pass {
 		return api.AuthorizationResult{Error: stacktrace.NewErrorWithCode(dsserr.PermissionDenied,
 			"Access token missing scopes (%v) while expecting %v and got %v",
-			missing, describeAuthorizationExpectations(authOptions), strings.Join(keyClaims.Scopes.ToStringSlice(), ", "))}
+			missing, describeAuthorizationExpectations(authOptions), strings.Join(authResults.Claims.Scopes.ToStringSlice(), ", "))}
 	}
 
 	return api.AuthorizationResult{
-		ClientID: &keyClaims.Subject,
-		Scopes:   keyClaims.Scopes.ToStringSlice(),
+		ClientID: &authResults.Claims.Subject,
+		Scopes:   authResults.Claims.Scopes.ToStringSlice(),
 	}
 }
 
-func (a *Authorizer) ExtractClaims(r *http.Request) (claims, error) {
+func (a *Authorizer) ExtractClaims(r *http.Request) (Claims, error) {
 	tknStr, ok := getToken(r)
 	if !ok {
-		return claims{}, stacktrace.NewErrorWithCode(dsserr.Unauthenticated, "Missing access token")
+		return Claims{}, stacktrace.NewErrorWithCode(dsserr.Unauthenticated, "Missing access token")
 	}
 
 	a.keyGuard.RLock()
 	keys := a.keys
 	a.keyGuard.RUnlock()
 	validated := false
 	var err error
-	var keyClaims claims
+	var keyClaims Claims
 
 	for _, key := range keys {
-		keyClaims = claims{}
+		keyClaims = Claims{}
 		key := key
 		_, err = jwt.ParseWithClaims(tknStr, &keyClaims, func(token *jwt.Token) (interface{}, error) {
 			return key, nil
@@ -234,7 +237,7 @@ func (a *Authorizer) ExtractClaims(r *http.Request) (claims, error) {
 		if err == nil { // If we have no keys, errs may be nil
 			err = stacktrace.NewErrorWithCode(dsserr.Unauthenticated, "No keys to validate against")
 		}
-		return claims{}, stacktrace.PropagateWithCode(err, dsserr.Unauthenticated, "Access token validation failed")
+		return Claims{}, stacktrace.PropagateWithCode(err, dsserr.Unauthenticated, "Access token validation failed")
 	}
 
 	return keyClaims, nil

pkg/auth/auth_test.go

@@ -115,7 +115,14 @@ func TestRSAAuthInterceptor(t *testing.T) {
 
 	for i, test := range authTests {
 		t.Run(strconv.Itoa(i), func(t *testing.T) {
-			res := a.Authorize(nil, test.req, []api.AuthorizationOption{})
+			claims, err := a.ExtractClaims(test.req)
+
+			ctx := context.WithValue(test.req.Context(), CtxAuthKey{}, CtxAuthValue{
+				Error:  err,
+				Claims: claims,
+			})
+
+			res := a.Authorize(nil, test.req.WithContext(ctx), []api.AuthorizationOption{})
 			if test.code != stacktrace.ErrorCode(0) && stacktrace.GetCode(res.Error) != test.code {
 				t.Logf("%v", res.Error)
 				t.Errorf("expected: %v, got: %v, with message %s", test.code, stacktrace.GetCode(res.Error), res.Error.Error())
@@ -203,7 +210,7 @@ func TestClaimsValidation(t *testing.T) {
 		Now = time.Now
 	}()
 
-	claims := &claims{}
+	claims := &Claims{}
 
 	require.Error(t, claims.Valid())
 

pkg/auth/claims.go

@@ -61,12 +61,12 @@ func (s *ScopeSet) ToStringSlice() []string {
 	return scopes
 }
 
-type claims struct {
+type Claims struct {
 	jwt.StandardClaims
 	Scopes ScopeSet `json:"scope"`
 }
 
-func (c *claims) Valid() error {
+func (c *Claims) Valid() error {
 	if c.Subject == "" {
 		return errMissingOrEmptySubject
 	}

pkg/auth/claims_test.go

@@ -8,7 +8,7 @@ import (
 )
 
 func TestScopesJSONUnmarshaling(t *testing.T) {
-	claims := &claims{}
+	claims := &Claims{}
 	require.NoError(t, json.Unmarshal([]byte(`{"scope": "one two three"}`), claims))
 	require.Contains(t, claims.Scopes, "one")
 	require.Contains(t, claims.Scopes, "two")

pkg/logging/http.go

@@ -39,8 +39,11 @@ func (w *tracingResponseWriter) WriteHeader(statusCode int) {
 	w.next.WriteHeader(statusCode)
 }
 
-type CtxAuthError struct{}
-type CtxAuthSubject struct{}
+type CtxAuthKey struct{}
+type CtxAuthValue struct {
+	Subject string
+	ErrMsg  string
+}
 
 // HTTPMiddleware installs a logging http.Handler that logs requests and
 // selected aspects of responses to 'logger'.
@@ -72,12 +75,11 @@ func HTTPMiddleware(logger *zap.Logger, dump bool, handler http.Handler) http.Ha
 			}
 		}
 
-		subject, ok := r.Context().Value(CtxAuthSubject{}).(string)
-		if !ok {
-			authErrorMsg := r.Context().Value(CtxAuthError{}).(string)
-			logger = logger.With(zap.String("resp_sub_err", authErrorMsg))
+		authResults := r.Context().Value(CtxAuthKey{}).(CtxAuthValue)
+		if authResults.ErrMsg != "" {
+			logger = logger.With(zap.String("resp_sub_err", authResults.ErrMsg))
 		} else {
-			logger = logger.With(zap.String("req_sub", subject))
+			logger = logger.With(zap.String("req_sub", authResults.Subject))
 		}
 
 		handler.ServeHTTP(trw, r)