fix: shift down require admin credentials check

supabase · Feb 17, 2024 · a8a8ba3 · a8a8ba3
1 parent 3e57b61
commit a8a8ba3
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 12 deletions.
diff --git a/internal/api/auth.go b/internal/api/auth.go
@@ -2,9 +2,7 @@ package api
 
 import (
 	"context"
-	"fmt"
 	"net/http"
-	"time"
 
 	"github.com/gofrs/uuid"
 	jwt "github.com/golang-jwt/jwt"
@@ -39,7 +37,6 @@ func (a *API) requireAdmin(ctx context.Context, w http.ResponseWriter, r *http.R
 	// Find the administrative user
 	claims := getClaims(ctx)
 	if claims == nil {
-		fmt.Printf("[%s] %s %s %d %s\n", time.Now().Format("2006-01-02 15:04:05"), r.Method, r.RequestURI, http.StatusForbidden, "Invalid token")
 		return nil, unauthorizedError("Invalid token")
 	}
 
@@ -50,7 +47,6 @@ func (a *API) requireAdmin(ctx context.Context, w http.ResponseWriter, r *http.R
 		return withAdminUser(ctx, &models.User{Role: claims.Role, Email: storage.NullString(claims.Role)}), nil
 	}
 
-	fmt.Printf("[%s] %s %s %d %s\n", time.Now().Format("2006-01-02 15:04:05"), r.Method, r.RequestURI, http.StatusForbidden, "this token needs role 'supabase_admin' or 'service_role'")
 	return nil, unauthorizedError("User not allowed")
 }
 

diff --git a/internal/api/middleware.go b/internal/api/middleware.go
@@ -151,6 +151,32 @@ func (a *API) requireAdminCredentials(w http.ResponseWriter, req *http.Request)
 	return a.requireAdmin(ctx, w, req)
 }
 
+func (a *API) isAdminUser(w http.ResponseWriter, req *http.Request) (context.Context, bool, error) {
+	t, err := a.extractBearerToken(req)
+	if err != nil || t == "" {
+		// TODO: Not all enpdoints with captcha will require a token, properly handle here
+		return req.Context(), false, nil
+	}
+
+	ctx, err := a.parseJWTClaims(t, req)
+	if err != nil {
+		a.clearCookieTokens(a.config, w)
+		return ctx, false, err
+	}
+
+	claims := getClaims(ctx)
+	if claims == nil {
+		return ctx, false, unauthorizedError("Invalid token")
+	}
+
+	if isStringInSlice(claims.Role, a.config.JWT.AdminRoles) {
+		// successful authentication
+		return ctx, true, nil
+	}
+
+	return ctx, false, nil
+}
+
 func (a *API) requireEmailProvider(w http.ResponseWriter, req *http.Request) (context.Context, error) {
 	ctx := req.Context()
 	config := a.config
@@ -166,17 +192,17 @@ func (a *API) verifyCaptcha(w http.ResponseWriter, req *http.Request) (context.C
 	ctx := req.Context()
 	config := a.config
 
-	if !config.Security.Captcha.Enabled {
+	if !config.Security.Captcha.Enabled || isIgnoreCaptchaRoute(req) {
 		return ctx, nil
 	}
-	if _, err := a.requireAdminCredentials(w, req); err == nil {
-		// skip captcha validation if authorization header contains an admin role
-		return ctx, nil
+
+	ctx, isAdmin, err := a.isAdminUser(w, req)
+	if err != nil {
+		return nil, err
 	}
-	if shouldIgnore := isIgnoreCaptchaRoute(req); shouldIgnore {
+	if isAdmin {
 		return ctx, nil
 	}
-
 	verificationResult, err := security.VerifyRequest(req, strings.TrimSpace(config.Security.Captcha.Secret), config.Security.Captcha.Provider)
 	if err != nil {
 		return nil, internalServerError("captcha verification process failed").WithInternalError(err)

diff --git a/internal/api/middleware_test.go b/internal/api/middleware_test.go
@@ -108,7 +108,7 @@ func (ts *MiddlewareTestSuite) TestVerifyCaptchaValid() {
 
 		w := httptest.NewRecorder()
 
-		afterCtx, err := ts.API.verifyCaptcha(w, req)
+		_, err := ts.API.verifyCaptcha(w, req)
 		require.NoError(ts.T(), err)
 
 		body, err := io.ReadAll(req.Body)
@@ -125,7 +125,6 @@ func (ts *MiddlewareTestSuite) TestVerifyCaptchaValid() {
 
 		// check if body is the same
 		require.Equal(ts.T(), body, buffer.Bytes())
-		require.Equal(ts.T(), afterCtx, beforeCtx)
 	}
 }