Skip to content

Commit

Permalink
chore: use retrieveRequestParams where possible (#1818)
Browse files Browse the repository at this point in the history
## What kind of change does this PR introduce?
* Move the reading the requestBody into the captcha middleware, which is
in the api package so it can use `retrieveRequestParams`
* Use `retrieveRequestParams` in the admin delete route

## What is the current behavior?

Please link any relevant issues here.

## What is the new behavior?

Feel free to include screenshots if it includes visual changes.

## Additional context

Add any other context or screenshots.
  • Loading branch information
kangmingtay authored Oct 28, 2024
1 parent 158e473 commit 3b03472
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 36 deletions.
20 changes: 8 additions & 12 deletions internal/api/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package api

import (
"context"
"encoding/json"
"net/http"
"time"

Expand All @@ -15,6 +14,7 @@ import (
"github.com/supabase/auth/internal/models"
"github.com/supabase/auth/internal/observability"
"github.com/supabase/auth/internal/storage"
"github.com/supabase/auth/internal/utilities"
"golang.org/x/crypto/bcrypt"
)

Expand Down Expand Up @@ -512,21 +512,17 @@ func (a *API) adminUserDelete(w http.ResponseWriter, r *http.Request) error {
user := getUser(ctx)
adminUser := getAdminUser(ctx)

var err error
// ShouldSoftDelete defaults to false
params := &adminUserDeleteParams{}
body, err := getBodyBytes(r)
if err != nil {
return internalServerError("Could not read body").WithInternalError(err)
}
if len(body) > 0 {
if err := json.Unmarshal(body, params); err != nil {
return badRequestError(ErrorCodeBadJSON, "Could not read params: %v", err)
if body, _ := utilities.GetBodyBytes(r); len(body) != 0 {
// we only want to parse the body if it's not empty
// retrieveRequestParams will handle any errors with stream
if err := retrieveRequestParams(r, params); err != nil {
return err
}
} else {
params.ShouldSoftDelete = false
}

err = a.db.Transaction(func(tx *storage.Connection) error {
err := a.db.Transaction(func(tx *storage.Connection) error {
if terr := models.NewAuditLogEntry(r, tx, adminUser, models.UserDeletedAction, "", map[string]interface{}{
"user_id": user.ID,
"user_email": user.Email,
Expand Down
10 changes: 4 additions & 6 deletions internal/api/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/pkg/errors"
"github.com/supabase/auth/internal/conf"
"github.com/supabase/auth/internal/models"
"github.com/supabase/auth/internal/security"
"github.com/supabase/auth/internal/utilities"
)

Expand Down Expand Up @@ -57,11 +58,6 @@ func isStringInSlice(checkValue string, list []string) bool {
return false
}

// getBodyBytes returns a byte array of the request's Body.
func getBodyBytes(req *http.Request) ([]byte, error) {
return utilities.GetBodyBytes(req)
}

type RequestParams interface {
AdminUserParams |
CreateSSOProviderParams |
Expand All @@ -82,6 +78,8 @@ type RequestParams interface {
VerifyFactorParams |
VerifyParams |
adminUserUpdateFactorParams |
adminUserDeleteParams |
security.GotrueRequest |
ChallengeFactorParams |
struct {
Email string `json:"email"`
Expand All @@ -94,7 +92,7 @@ type RequestParams interface {

// retrieveRequestParams is a generic method that unmarshals the request body into the params struct provided
func retrieveRequestParams[A RequestParams](r *http.Request, params *A) error {
body, err := getBodyBytes(r)
body, err := utilities.GetBodyBytes(r)
if err != nil {
return internalServerError("Could not read body into byte slice").WithInternalError(err)
}
Expand Down
12 changes: 7 additions & 5 deletions internal/api/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/supabase/auth/internal/models"
"github.com/supabase/auth/internal/observability"
"github.com/supabase/auth/internal/security"
"github.com/supabase/auth/internal/utilities"

"github.com/didip/tollbooth/v5"
"github.com/didip/tollbooth/v5/limiter"
Expand Down Expand Up @@ -116,12 +117,13 @@ func (a *API) verifyCaptcha(w http.ResponseWriter, req *http.Request) (context.C
return ctx, nil
}

verificationResult, err := security.VerifyRequest(req, strings.TrimSpace(config.Security.Captcha.Secret), config.Security.Captcha.Provider)
if err != nil {
if strings.Contains(err.Error(), "request body was not JSON") {
return nil, badRequestError(ErrorCodeValidationFailed, "Request body for CAPTCHA verification was not a valid JSON object")
}
body := &security.GotrueRequest{}
if err := retrieveRequestParams(req, body); err != nil {
return nil, err
}

verificationResult, err := security.VerifyRequest(body, utilities.GetIPAddress(req), strings.TrimSpace(config.Security.Captcha.Secret), config.Security.Captcha.Provider)
if err != nil {
return nil, internalServerError("captcha verification process failed").WithInternalError(err)
}

Expand Down
15 changes: 2 additions & 13 deletions internal/security/captcha.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"time"

"fmt"

"github.com/pkg/errors"
"github.com/supabase/auth/internal/utilities"
)
Expand Down Expand Up @@ -45,25 +46,13 @@ func init() {
Client = &http.Client{Timeout: defaultTimeout}
}

func VerifyRequest(r *http.Request, secretKey, captchaProvider string) (VerificationResponse, error) {
bodyBytes, err := utilities.GetBodyBytes(r)
if err != nil {
return VerificationResponse{}, err
}

var requestBody GotrueRequest

if err := json.Unmarshal(bodyBytes, &requestBody); err != nil {
return VerificationResponse{}, errors.Wrap(err, "request body was not JSON")
}

func VerifyRequest(requestBody *GotrueRequest, clientIP, secretKey, captchaProvider string) (VerificationResponse, error) {
captchaResponse := strings.TrimSpace(requestBody.Security.Token)

if captchaResponse == "" {
return VerificationResponse{}, errors.New("no captcha response (captcha_token) found in request")
}

clientIP := utilities.GetIPAddress(r)
captchaURL, err := GetCaptchaURL(captchaProvider)
if err != nil {
return VerificationResponse{}, err
Expand Down

0 comments on commit 3b03472

Please sign in to comment.