From 2d9a547c395c1e82ddde6ed5c47312037a07996a Mon Sep 17 00:00:00 2001
From: Drew DeVault <sir@cmpwn.com>
Date: Wed, 25 Aug 2021 10:56:27 +0200
Subject: [PATCH] auth: harden grants structure

This replaces the Access map with a data type whose default value does
not grant any permissions, so that it's more difficult to create an
insecure auth context by mistake.
---
 auth/bearer.go       | 48 +++++++++++++++++++++++++++++++++++++++++---
 auth/middleware.go   |  6 +++---
 server/directives.go | 12 +++--------
 3 files changed, 51 insertions(+), 15 deletions(-)

diff --git a/auth/bearer.go b/auth/bearer.go
index 940f9d5276261f26ea6027fd990b61df6fdf4590..5cb0b6d04f3ec77031e4859cf87ec3047bfddbe7 100644
--- a/auth/bearer.go
+++ b/auth/bearer.go
@@ -80,10 +80,25 @@ func DecodeBearerToken(token string) *BearerToken {
 	return &bt
 }
 
-func DecodeGrants(ctx context.Context, grants string) map[string]string {
+const (
+	RO = "RO"
+	RW = "RW"
+)
+
+type Grants struct {
+	all     bool
+	grants  map[string]string
+	encoded string
+}
+
+func DecodeGrants(ctx context.Context, grants string) Grants {
 	if grants == "" {
 		// All permissions
-		return nil
+		return Grants{
+			all:     true,
+			grants:  nil,
+			encoded: "",
+		}
 	}
 	accessMap := make(map[string]string)
 	for _, grant := range strings.Split(grants, " ") {
@@ -108,5 +123,32 @@ func DecodeGrants(ctx context.Context, grants string) map[string]string {
 			accessMap[scope] = access
 		}
 	}
-	return accessMap
+	return Grants{
+		all:     false,
+		grants:  accessMap,
+		encoded: grants,
+	}
+}
+
+func (g *Grants) Has(grant string, mode string) bool {
+	if mode != RO && mode != RW {
+		panic("Invalid access mode")
+	}
+
+	if g.all {
+		return true
+	}
+
+	if access, ok := g.grants[grant]; !ok {
+		return false
+	} else {
+		if mode == RO {
+			return true
+		}
+		return mode == access
+	}
+}
+
+func (g *Grants) Encode() string {
+	return g.encoded
 }
diff --git a/auth/middleware.go b/auth/middleware.go
index 0483e6ca61948b9d25f8327a6e32b90f43028534..b794aaaed768e04c073b35d69986989910ab3499 100644
--- a/auth/middleware.go
+++ b/auth/middleware.go
@@ -77,7 +77,7 @@ type AuthContext struct {
 
 	// Only filled out if AuthMethod == AUTH_OAUTH2 or AUTH_WEBHOOK
 	BearerToken *BearerToken
-	Access      map[string]string
+	Grants      Grants
 	TokenHash   [64]byte
 }
 
@@ -554,7 +554,7 @@ func OAuth2(token string, hash [64]byte, w http.ResponseWriter,
 	auth.AuthMethod = AUTH_OAUTH2
 	auth.BearerToken = bt
 	auth.TokenHash = hash
-	auth.Access = DecodeGrants(r.Context(), bt.Grants)
+	auth.Grants = DecodeGrants(r.Context(), bt.Grants)
 
 	ctx := context.WithValue(r.Context(), userCtxKey, &auth)
 	r = r.WithContext(ctx)
@@ -666,7 +666,7 @@ func WebhookAuth(ctx context.Context, auth *AuthContext,
 	whAuth := *auth
 	whAuth.AuthMethod = AUTH_WEBHOOK
 	whAuth.TokenHash = tokenHash
-	whAuth.Access = DecodeGrants(ctx, grants)
+	whAuth.Grants = DecodeGrants(ctx, grants)
 	whAuth.BearerToken = &BearerToken{}
 	if clientID != nil {
 		whAuth.BearerToken.ClientID = *clientID
diff --git a/server/directives.go b/server/directives.go
index 4eb296c897976a9c209eb4d7e5801698fa0fa0bc..778de157b3436b724b947b1621dcc5b8c79ef94a 100644
--- a/server/directives.go
+++ b/server/directives.go
@@ -44,25 +44,19 @@ func Access(ctx context.Context, obj interface{}, next graphql.Resolver,
 	case auth.AUTH_INTERNAL, auth.AUTH_COOKIE:
 		return next(ctx)
 	case auth.AUTH_OAUTH_LEGACY:
-		if kind == "RO" {
+		if kind == auth.RO {
 			// Only legacy tokens with "*" scopes ever get this far
 			return next(ctx)
 		}
 	case auth.AUTH_WEBHOOK:
-		if kind != "RO" {
+		if kind != auth.RO {
 			return nil, fmt.Errorf("Access to read/write resolver denied for webhook")
 		}
 		fallthrough
 	case auth.AUTH_OAUTH2:
-		if authctx.Access == nil {
+		if authctx.Grants.Has(scope, kind) {
 			return next(ctx)
 		}
-		if access, ok := authctx.Access[scope]; !ok {
-			break
-		} else if access == "RO" && kind == "RW" {
-			break
-		}
-		return next(ctx)
 	default:
 		panic(fmt.Errorf("Unknown auth method for access check"))
 	}