Skip to content

Commit

Permalink
fix: protect ops endpoints requiring admin (#122)
Browse files Browse the repository at this point in the history
* fix: protect ops endpoints requiring admin
  • Loading branch information
dbarrosop authored Oct 28, 2022
1 parent da8215a commit 097abb4
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 10 deletions.
9 changes: 9 additions & 0 deletions cmd/serve.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package cmd

import (
"fmt"
"net/url"
"os"
"time"

Expand All @@ -10,6 +12,7 @@ import (
"github.com/nhost/hasura-storage/controller"
"github.com/nhost/hasura-storage/image"
"github.com/nhost/hasura-storage/metadata"
"github.com/nhost/hasura-storage/middleware/auth"
"github.com/nhost/hasura-storage/middleware/cdn/fastly"
"github.com/nhost/hasura-storage/migrations"
"github.com/nhost/hasura-storage/storage"
Expand Down Expand Up @@ -88,8 +91,14 @@ func getGin(
publicURL, apiRootPrefix, hasuraAdminSecret, metadataStorage, contentStorage, imageTransformer, logger,
)

opsPath, err := url.JoinPath(apiRootPrefix, "ops")
if err != nil {
return nil, fmt.Errorf("problem trying to compute ops prefix path: %w", err)
}

middlewares := []gin.HandlerFunc{
ginLogger(logger),
auth.NeedsAdmin(opsPath, hasuraAdminSecret),
}

fastlyService := viper.GetString(fastlyServiceFlag)
Expand Down
26 changes: 16 additions & 10 deletions controller/openapi.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# yaml-language-server: $schema=https://raw.githubusercontent.com/OAI/OpenAPI-Specification/main/schemas/v3.0/schema.yaml
---
openapi: 3.0.0
openapi: 3.0
info:
description: Hasura Storage is amazing
version: 1.0.0
Expand All @@ -18,13 +18,19 @@ servers:
default: '8000'
security:
- Authorization: []
- X-Hasura-Admin-Secret: []
components:
securitySchemes:
Authorization:
type: http
scheme: bearer
bearerFormat: JWT
description: API key to authorize requests.
X-Hasura-Admin-Secret:
type: apiKey
name: X-Hasura-Admin-Secret
in: header
description: Hasura admin secret
schemas:
VersionInformation:
type: object
Expand Down Expand Up @@ -120,7 +126,7 @@ paths:
application/json:
schema:
$ref: '#/components/schemas/VersionInformation'

/files/:
post:
summary: Upload one or more files
Expand Down Expand Up @@ -450,8 +456,8 @@ paths:
1. Set isUploaded flag to false
2. Replace contents of the file in the storage backend
3. Update file metadata (size, mime-type, isUploaded, etc)
Each individual state is atomic but if a step fails, previous steps won't be undone
Each individual state is atomic but if a step fails, previous steps won't be undone
tags:
- storage
security:
Expand Down Expand Up @@ -762,7 +768,7 @@ paths:
tags:
- operations
security:
- Authorization: []
- X-Hasura-Admin-Secret: []
responses:
'200':
description: Successfully computed orphaned files
Expand All @@ -789,7 +795,7 @@ paths:
tags:
- operations
security:
- Authorization: []
- X-Hasura-Admin-Secret: []
responses:
'200':
description: Successfully deleted orphaned files
Expand All @@ -816,7 +822,7 @@ paths:
tags:
- operations
security:
- Authorization: []
- X-Hasura-Admin-Secret: []
responses:
'200':
description: Successfully computed broken metadata
Expand All @@ -835,15 +841,15 @@ paths:
application/json:
schema:
$ref: '#/components/schemas/Error'

/ops/delete-broken-metadata:
post:
summary: Delete broken metadata
description: Broken metadata is defined as metadata that has isUploaded = true but there is no file in the storage matching it
tags:
- operations
security:
- Authorization: []
- X-Hasura-Admin-Secret: []
responses:
'200':
description: Successfully deleted broken metadata
Expand All @@ -870,7 +876,7 @@ paths:
tags:
- operations
security:
- Authorization: []
- X-Hasura-Admin-Secret: []
responses:
'200':
description: Successfully checked files not uploaded
Expand Down
25 changes: 25 additions & 0 deletions middleware/auth/needs_admin.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package auth

import (
"net/http"
"strings"

"github.com/gin-gonic/gin"
)

func isAdmin(hasuraAdminSecret string, header http.Header) bool {
return header.Get("X-Hasura-Admin-Secret") == hasuraAdminSecret &&
(header.Get("X-Hasura-Role") == "admin" ||
header.Get("X-Hasura-Role") == "")
}

func NeedsAdmin(prefixPath, hasuraAdminSecret string) gin.HandlerFunc {
return func(ctx *gin.Context) {
if strings.HasPrefix(ctx.Request.URL.Path, prefixPath) &&
!isAdmin(hasuraAdminSecret, ctx.Request.Header) {
ctx.AbortWithStatus(http.StatusUnauthorized)
return
}
ctx.Next()
}
}
73 changes: 73 additions & 0 deletions middleware/auth/needs_admin_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
package auth //nolint: testpackage

import (
"net/http"
"testing"
)

func TestIsAdmin(t *testing.T) {
t.Parallel()

cases := []struct {
name string
reqHeader http.Header
isAdmin bool
}{
{
name: "correct secret and empty role",
reqHeader: http.Header{
"X-Hasura-Admin-Secret": []string{"secret"},
},
isAdmin: true,
},
{
name: "correct secret and role",
reqHeader: http.Header{
"X-Hasura-Admin-Secret": []string{"secret"},
"X-Hasura-Role": []string{"admin"},
},
isAdmin: true,
},
{
name: "correct secret and wrong role",
reqHeader: http.Header{
"X-Hasura-Admin-Secret": []string{"secret"},
"X-Hasura-Role": []string{"user"},
},
isAdmin: false,
},
{
name: "wrong secret and correct role",
reqHeader: http.Header{
"X-Hasura-Admin-Secret": []string{"woooo"},
"X-Hasura-Role": []string{"admin"},
},
isAdmin: false,
},
{
name: "wrong secret and no role",
reqHeader: http.Header{
"X-Hasura-Admin-Secret": []string{"woooo"},
},
isAdmin: false,
},
{
name: "no headers",
reqHeader: http.Header{},
isAdmin: false,
},
}

for _, tc := range cases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
tc := tc

got := isAdmin("secret", tc.reqHeader)
if got != tc.isAdmin {
t.Errorf("isAdmin() = %v, want %v", got, tc.isAdmin)
}
})
}
}

0 comments on commit 097abb4

Please sign in to comment.