~bigbes/core-go: Revert "auth: ensure semantic errors are bubbled up to user properly"

1 files changed, 29 insertions(+), 41 deletions(-)

M auth/middleware.go

M auth/middleware.go => auth/middleware.go +29 -41

@@ 100,7 100,8 @@ func (authctx *AuthContext) Access(scope, kind string) error {
 	}
 }
 
-func authError(w http.ResponseWriter, gqlerr *gqlerror.Error, code int) {
+func authError(w http.ResponseWriter, reason string, code int) {
+	gqlerr := gqlerror.Errorf("Authentication error: %s", reason)
 	b, err := json.Marshal(struct {
 		Errors []*gqlerror.Error `json:"errors"`
 	}{


@@ 114,7 115,7 @@ func authError(w http.ResponseWriter, gqlerr *gqlerror.Error, code int) {
 	w.Write(b)
 }
 
-func authForUsername(ctx context.Context, username string) (*AuthContext, *gqlerror.Error) {
+func authForUsername(ctx context.Context, username string) (*AuthContext, error) {
 	var auth AuthContext
 	if err := LookupUser(ctx, username, &auth); err != nil {
 		return nil, err


@@ 123,7 124,7 @@ func authForUsername(ctx context.Context, username string) (*AuthContext, *gqler
 }
 
 // NOTE: This only works for meta.sr.ht (should we move it?)
-func authForOAuthClient(ctx context.Context, clientUUID string) (*AuthContext, *gqlerror.Error) {
+func authForOAuthClient(ctx context.Context, clientUUID string) (*AuthContext, error) {
 	var auth AuthContext
 	if err := database.WithTx(ctx, &sql.TxOptions{
 		Isolation: 0,


@@ 171,13 172,11 @@ func authForOAuthClient(ctx context.Context, clientUUID string) (*AuthContext, *
 		}
 		return nil
 	}); err != nil {
-		gqlerr := err.(*gqlerror.Error)
-		return nil, gqlerr
+		return nil, err
 	}
 
 	if auth.UserType == USER_TYPE_SUSPENDED {
-		return nil, errors.Errorf(
-			errors.AccessDenied,
+		return nil, fmt.Errorf(
 			"Account suspended with the following notice: %s\nContact support",
 			*auth.SuspensionNotice)
 	}


@@ 194,8 193,7 @@ func cookieAuth(cookie *http.Cookie, w http.ResponseWriter,
 	r *http.Request, next http.Handler) {
 	payload := crypto.DecryptWithoutExpiration([]byte(cookie.Value))
 	if payload == nil {
-		err := errors.Errorf(errors.Unauthorized, "Invalid authentication cookie")
-		authError(w, err, http.StatusForbidden)
+		authError(w, "Invalid authentication cookie", http.StatusForbidden)
 		return
 	}
 


@@ 206,13 204,12 @@ func cookieAuth(cookie *http.Cookie, w http.ResponseWriter,
 
 	auth, err := authForUsername(r.Context(), authCookie.Name)
 	if err != nil {
-		authError(w, err, http.StatusForbidden)
+		authError(w, err.Error(), http.StatusForbidden)
 		return
 	}
 
 	if auth.UserType == USER_TYPE_SUSPENDED {
-		authError(w, errors.Errorf(
-			errors.AccessDenied,
+		authError(w, fmt.Sprintf(
 			"Account suspended with the following notice: %s\nContact support",
 			*auth.SuspensionNotice),
 			http.StatusForbidden)


@@ 250,13 247,13 @@ func internalAuth(payload []byte, w http.ResponseWriter, r *http.Request, next h
 		panic(fmt.Errorf("Unable to parse remote address"))
 	}
 	if !config.IsInternalIP(ip) {
-		authError(w, errors.Errorf(errors.Unauthorized, "Invalid source IP %s for internal auth", ip), http.StatusUnauthorized)
+		authError(w, fmt.Sprintf("Invalid source IP %s for internal auth", ip), http.StatusUnauthorized)
 		return
 	}
 
 	payload = crypto.DecryptWithExpiration(payload, 30*time.Second)
 	if payload == nil {
-		authError(w, errors.Errorf(errors.Unauthorized, "Invalid Authorization header (encryption error)"), http.StatusForbidden)
+		authError(w, "Invalid Authorization header (encryption error)", http.StatusForbidden)
 		return
 	}
 


@@ 266,21 263,18 @@ func internalAuth(payload []byte, w http.ResponseWriter, r *http.Request, next h
 	}
 
 	if internalAuth.ClientID == "" || internalAuth.NodeID == "" {
-		authError(w, errors.Errorf(errors.Unauthorized, "Invalid Authorization header (missing Client ID or Node ID)"), http.StatusForbidden)
+		authError(w, "Invalid Authorization header (missing Client ID or Node ID)", http.StatusForbidden)
 	}
 
-	var (
-		auth   *AuthContext
-		gqlerr *gqlerror.Error
-	)
+	var auth *AuthContext
 	if internalAuth.OAuthClientUUID != "" {
-		auth, gqlerr = authForOAuthClient(r.Context(), internalAuth.OAuthClientUUID)
-		if gqlerr == nil {
+		auth, err = authForOAuthClient(r.Context(), internalAuth.OAuthClientUUID)
+		if err == nil {
 			auth.AuthMethod = AUTH_INTERNAL
 		}
 	} else if internalAuth.Name != "" {
-		auth, gqlerr = authForUsername(r.Context(), internalAuth.Name)
-		if gqlerr == nil {
+		auth, err = authForUsername(r.Context(), internalAuth.Name)
+		if err == nil {
 			auth.AuthMethod = AUTH_INTERNAL
 		}
 	} else {


@@ 289,8 283,8 @@ func internalAuth(payload []byte, w http.ResponseWriter, r *http.Request, next h
 		auth = &AuthContext{}
 		auth.AuthMethod = AUTH_ANON_INTERNAL
 	}
-	if gqlerr != nil {
-		authError(w, gqlerr, http.StatusForbidden)
+	if err != nil {
+		authError(w, err.Error(), http.StatusForbidden)
 		return
 	}
 


@@ 406,8 400,8 @@ func FetchMetaProfile(ctx context.Context, username string, user *AuthContext) e
 	})
 }
 
-func LookupUser(ctx context.Context, username string, user *AuthContext) *gqlerror.Error {
-	err := database.WithTx(ctx, &sql.TxOptions{
+func LookupUser(ctx context.Context, username string, user *AuthContext) error {
+	return database.WithTx(ctx, &sql.TxOptions{
 		Isolation: 0,
 		ReadOnly:  true,
 	}, func(tx *sql.Tx) error {


@@ 469,11 463,6 @@ func LookupUser(ctx context.Context, username string, user *AuthContext) *gqlerr
 		}
 		return nil
 	})
-	if err != nil {
-		gqlerr := err.(*gqlerror.Error)
-		return gqlerr
-	}
-	return nil
 }
 
 // Returns true if this token or client ID has been revoked (and therefore


@@ 515,7 504,7 @@ func OAuth2(token string, hash [64]byte, w http.ResponseWriter,
 
 	bt := DecodeBearerToken(token)
 	if bt == nil {
-		authError(w, errors.Errorf(errors.Unauthorized, "Invalid or expired OAuth 2.0 bearer token"), http.StatusForbidden)
+		authError(w, `Invalid or expired OAuth 2.0 bearer token`, http.StatusForbidden)
 		return
 	}
 


@@ 545,16 534,15 @@ func OAuth2(token string, hash [64]byte, w http.ResponseWriter,
 	wg.Wait()
 	if res != 2 {
 		if tempErr != 0 {
-			authError(w, errors.Errorf(errors.InternalError, "Temporary error; try again later"), http.StatusInternalServerError)
+			authError(w, "Temporary error; try again later", http.StatusInternalServerError)
 		} else {
-			authError(w, errors.Errorf(errors.Unauthorized, "Invalid or expired OAuth 2.0 bearer token"), http.StatusForbidden)
+			authError(w, "Invalid or expired OAuth 2.0 bearer token", http.StatusForbidden)
 		}
 		return
 	}
 
 	if auth.UserType == USER_TYPE_SUSPENDED {
-		authError(w, errors.Errorf(
-			errors.AccessDenied,
+		authError(w, fmt.Sprintf(
 			"Account suspended with the following notice: %s\nContact support",
 			*auth.SuspensionNotice), http.StatusForbidden)
 		return


@@ 617,13 605,13 @@ func Middleware(conf ini.File, apiconf string) func(http.Handler) http.Handler {
 			auth := r.Header.Get("Authorization")
 			if auth == "" {
 				w.Header().Set("WWW-Authenticate", "Bearer")
-				authError(w, errors.Errorf(errors.Unauthorized, "Authorization header is required. Expected 'Authorization: Bearer [token]'"), http.StatusUnauthorized)
+				authError(w, `Authorization header is required. Expected 'Authorization: Bearer [token]'`, http.StatusUnauthorized)
 				return
 			}
 
 			z := strings.SplitN(auth, " ", 2)
 			if len(z) != 2 {
-				authError(w, errors.Errorf(errors.Unauthorized, "Invalid Authorization header"), http.StatusBadRequest)
+				authError(w, "Invalid Authorization header", http.StatusBadRequest)
 				return
 			}
 


@@ 637,14 625,14 @@ func Middleware(conf ini.File, apiconf string) func(http.Handler) http.Handler {
 					OAuth2(bearer, hash, w, r, next)
 					return
 				}
-				authError(w, errors.Errorf(errors.Unauthorized, "Invalid OAuth bearer token"), http.StatusBadRequest)
+				authError(w, "Invalid OAuth bearer token", http.StatusBadRequest)
 				return
 			case "internal":
 				payload := []byte(z[1])
 				internalAuth(payload, w, r, next)
 				return
 			default:
-				authError(w, errors.Errorf(errors.Unauthorized, "Invalid Authorization header"), http.StatusBadRequest)
+				authError(w, "Invalid Authorization header", http.StatusBadRequest)
 				return
 			}
 		})