fix: ensure auth redirects throw an error (#561)

* fix: ensure auth redirects throw an error * fix: add line removed by linter
hatchet-dev · Jun 6, 2024 · f4be542 · f4be542
1 parent 3b1232c
commit f4be542
Show file tree

Hide file tree

Showing 11 changed files with 72 additions and 38 deletions.
diff --git a/api/v1/server/authn/middleware.go b/api/v1/server/authn/middleware.go
@@ -10,6 +10,7 @@ import (
 	"github.com/rs/zerolog"
 
 	"github.com/hatchet-dev/hatchet/api/v1/server/middleware"
+	"github.com/hatchet-dev/hatchet/api/v1/server/middleware/redirect"
 	"github.com/hatchet-dev/hatchet/internal/config/server"
 	"github.com/hatchet-dev/hatchet/internal/repository/prisma/db"
 )
@@ -82,13 +83,13 @@ func (a *AuthN) handleNoAuth(c echo.Context) error {
 	if err != nil {
 		a.l.Debug().Err(err).Msg("error getting session")
 
-		return GetRedirectWithError(c, a.l, err, "Could not log in. Please try again and make sure cookies are enabled.")
+		return redirect.GetRedirectWithError(c, a.l, err, "Could not log in. Please try again and make sure cookies are enabled.")
 	}
 
 	if auth, ok := session.Values["authenticated"].(bool); ok && auth {
 		a.l.Debug().Msgf("user was authenticated when no security schemes permit auth")
 
-		return GetRedirectNoError(c, a.config.Runtime.ServerURL)
+		return redirect.GetRedirectNoError(c, a.config.Runtime.ServerURL)
 	}
 
 	// set unauthenticated session in context

diff --git a/api/v1/server/handlers/github-app/oauth_callback.go b/api/v1/server/handlers/github-app/oauth_callback.go
@@ -8,6 +8,7 @@ import (
 	"github.com/labstack/echo/v4"
 
 	"github.com/hatchet-dev/hatchet/api/v1/server/authn"
+	"github.com/hatchet-dev/hatchet/api/v1/server/middleware/redirect"
 	"github.com/hatchet-dev/hatchet/api/v1/server/oas/gen"
 	"github.com/hatchet-dev/hatchet/internal/repository"
 	"github.com/hatchet-dev/hatchet/internal/repository/prisma/db"
@@ -20,23 +21,23 @@ func (g *GithubAppService) UserUpdateGithubAppOauthCallback(ctx echo.Context, _
 	ghApp, err := GetGithubAppConfig(g.config)
 
 	if err != nil {
-		return nil, authn.GetRedirectWithError(ctx, g.config.Logger, err, "Github app is misconfigured on this Hatchet instance.")
+		return nil, redirect.GetRedirectWithError(ctx, g.config.Logger, err, "Github app is misconfigured on this Hatchet instance.")
 	}
 
 	isValid, isOAuthTriggered, err := authn.NewSessionHelpers(g.config).ValidateOAuthState(ctx, "github")
 
 	if err != nil || !isValid {
-		return nil, authn.GetRedirectWithError(ctx, g.config.Logger, err, "Could not link Github account. Please try again and make sure cookies are enabled.")
+		return nil, redirect.GetRedirectWithError(ctx, g.config.Logger, err, "Could not link Github account. Please try again and make sure cookies are enabled.")
 	}
 
 	token, err := ghApp.Exchange(context.Background(), ctx.Request().URL.Query().Get("code"))
 
 	if err != nil {
-		return nil, authn.GetRedirectWithError(ctx, g.config.Logger, err, "Forbidden")
+		return nil, redirect.GetRedirectWithError(ctx, g.config.Logger, err, "Forbidden")
 	}
 
 	if !token.Valid() {
-		return nil, authn.GetRedirectWithError(ctx, g.config.Logger, fmt.Errorf("invalid token"), "Forbidden")
+		return nil, redirect.GetRedirectWithError(ctx, g.config.Logger, fmt.Errorf("invalid token"), "Forbidden")
 	}
 
 	ghClient := github.NewClient(ghApp.Client(context.Background(), token))
@@ -48,7 +49,7 @@ func (g *GithubAppService) UserUpdateGithubAppOauthCallback(ctx echo.Context, _
 	}
 
 	if githubUser.ID == nil {
-		return nil, authn.GetRedirectWithError(ctx, g.config.Logger, err, "Could not get Github user ID.")
+		return nil, redirect.GetRedirectWithError(ctx, g.config.Logger, err, "Could not get Github user ID.")
 	}
 
 	expiresAt := token.Expiry
@@ -75,7 +76,7 @@ func (g *GithubAppService) UserUpdateGithubAppOauthCallback(ctx echo.Context, _
 	})
 
 	if err != nil {
-		return nil, authn.GetRedirectWithError(ctx, g.config.Logger, err, "Internal error.")
+		return nil, redirect.GetRedirectWithError(ctx, g.config.Logger, err, "Internal error.")
 	}
 
 	var url string

diff --git a/api/v1/server/handlers/github-app/oauth_start.go b/api/v1/server/handlers/github-app/oauth_start.go
@@ -5,6 +5,7 @@ import (
 	"golang.org/x/oauth2"
 
 	"github.com/hatchet-dev/hatchet/api/v1/server/authn"
+	"github.com/hatchet-dev/hatchet/api/v1/server/middleware/redirect"
 	"github.com/hatchet-dev/hatchet/api/v1/server/oas/gen"
 )
 
@@ -13,13 +14,13 @@ func (g *GithubAppService) UserUpdateGithubAppOauthStart(ctx echo.Context, _ gen
 	ghApp, err := GetGithubAppConfig(g.config)
 
 	if err != nil {
-		return nil, authn.GetRedirectWithError(ctx, g.config.Logger, err, "Github app is misconfigured on this Hatchet instance.")
+		return nil, redirect.GetRedirectWithError(ctx, g.config.Logger, err, "Github app is misconfigured on this Hatchet instance.")
 	}
 
 	state, err := authn.NewSessionHelpers(g.config).SaveOAuthState(ctx, "github")
 
 	if err != nil {
-		return nil, authn.GetRedirectWithError(ctx, g.config.Logger, err, "Could not get cookie. Please make sure cookies are enabled.")
+		return nil, redirect.GetRedirectWithError(ctx, g.config.Logger, err, "Could not get cookie. Please make sure cookies are enabled.")
 	}
 
 	url := ghApp.AuthCodeURL(state, oauth2.AccessTypeOffline)

diff --git a/api/v1/server/handlers/slack-app/oauth_callback.go b/api/v1/server/handlers/slack-app/oauth_callback.go
@@ -7,6 +7,7 @@ import (
 	"github.com/slack-go/slack"
 
 	"github.com/hatchet-dev/hatchet/api/v1/server/authn"
+	"github.com/hatchet-dev/hatchet/api/v1/server/middleware/redirect"
 	"github.com/hatchet-dev/hatchet/api/v1/server/oas/gen"
 	"github.com/hatchet-dev/hatchet/internal/repository"
 )
@@ -16,15 +17,15 @@ func (g *SlackAppService) UserUpdateSlackOauthCallback(ctx echo.Context, _ gen.U
 	oauth, ok := g.config.AdditionalOAuthConfigs["slack"]
 
 	if !ok {
-		return nil, authn.GetRedirectWithError(ctx, g.config.Logger, nil, "Slack OAuth is not configured on this Hatchet instance.")
+		return nil, redirect.GetRedirectWithError(ctx, g.config.Logger, nil, "Slack OAuth is not configured on this Hatchet instance.")
 	}
 
 	sh := authn.NewSessionHelpers(g.config)
 
 	tenantId, err := sh.GetKey(ctx, "tenant")
 
 	if err != nil {
-		return nil, authn.GetRedirectWithError(ctx, g.config.Logger, err, "Could not link Slack account. Please try again and make sure cookies are enabled.")
+		return nil, redirect.GetRedirectWithError(ctx, g.config.Logger, err, "Could not link Slack account. Please try again and make sure cookies are enabled.")
 	}
 
 	defer func() {
@@ -38,7 +39,7 @@ func (g *SlackAppService) UserUpdateSlackOauthCallback(ctx echo.Context, _ gen.U
 	isValid, _, err := sh.ValidateOAuthState(ctx, "slack")
 
 	if err != nil || !isValid {
-		return nil, authn.GetRedirectWithError(ctx, g.config.Logger, err, "Could not link Slack account. Please try again and make sure cookies are enabled.")
+		return nil, redirect.GetRedirectWithError(ctx, g.config.Logger, err, "Could not link Slack account. Please try again and make sure cookies are enabled.")
 	}
 
 	resp, err := slack.GetOAuthV2ResponseContext(
@@ -51,13 +52,13 @@ func (g *SlackAppService) UserUpdateSlackOauthCallback(ctx echo.Context, _ gen.U
 	)
 
 	if err != nil {
-		return nil, authn.GetRedirectWithError(ctx, g.config.Logger, err, "Forbidden")
+		return nil, redirect.GetRedirectWithError(ctx, g.config.Logger, err, "Forbidden")
 	}
 
 	webhookURLEncrypted, err := g.config.Encryption.Encrypt([]byte(resp.IncomingWebhook.URL), "incoming_webhook_url")
 
 	if err != nil {
-		return nil, authn.GetRedirectWithError(ctx, g.config.Logger, err, "Could not link Slack account. An internal error occurred.")
+		return nil, redirect.GetRedirectWithError(ctx, g.config.Logger, err, "Could not link Slack account. An internal error occurred.")
 	}
 
 	_, err = g.config.APIRepository.Slack().UpsertSlackWebhook(
@@ -72,7 +73,7 @@ func (g *SlackAppService) UserUpdateSlackOauthCallback(ctx echo.Context, _ gen.U
 	)
 
 	if err != nil {
-		return nil, authn.GetRedirectWithError(ctx, g.config.Logger, err, "Could not link Slack account. An internal error occurred.")
+		return nil, redirect.GetRedirectWithError(ctx, g.config.Logger, err, "Could not link Slack account. An internal error occurred.")
 	}
 
 	return gen.UserUpdateSlackOauthCallback302Response{

diff --git a/api/v1/server/handlers/slack-app/oauth_start.go b/api/v1/server/handlers/slack-app/oauth_start.go
@@ -5,6 +5,7 @@ import (
 	"golang.org/x/oauth2"
 
 	"github.com/hatchet-dev/hatchet/api/v1/server/authn"
+	"github.com/hatchet-dev/hatchet/api/v1/server/middleware/redirect"
 	"github.com/hatchet-dev/hatchet/api/v1/server/oas/gen"
 	"github.com/hatchet-dev/hatchet/internal/repository/prisma/db"
 )
@@ -16,19 +17,19 @@ func (g *SlackAppService) UserUpdateSlackOauthStart(ctx echo.Context, _ gen.User
 	oauth, ok := g.config.AdditionalOAuthConfigs["slack"]
 
 	if !ok {
-		return nil, authn.GetRedirectWithError(ctx, g.config.Logger, nil, "Slack OAuth is not configured on this Hatchet instance.")
+		return nil, redirect.GetRedirectWithError(ctx, g.config.Logger, nil, "Slack OAuth is not configured on this Hatchet instance.")
 	}
 
 	sh := authn.NewSessionHelpers(g.config)
 
 	if err := sh.SaveKV(ctx, "tenant", tenant.ID); err != nil {
-		return nil, authn.GetRedirectWithError(ctx, g.config.Logger, err, "Could not get cookie. Please make sure cookies are enabled.")
+		return nil, redirect.GetRedirectWithError(ctx, g.config.Logger, err, "Could not get cookie. Please make sure cookies are enabled.")
 	}
 
 	state, err := sh.SaveOAuthState(ctx, "slack")
 
 	if err != nil {
-		return nil, authn.GetRedirectWithError(ctx, g.config.Logger, err, "Could not get cookie. Please make sure cookies are enabled.")
+		return nil, redirect.GetRedirectWithError(ctx, g.config.Logger, err, "Could not get cookie. Please make sure cookies are enabled.")
 	}
 
 	url := oauth.AuthCodeURL(state, oauth2.AccessTypeOffline)

diff --git a/api/v1/server/handlers/users/github_oauth_callback.go b/api/v1/server/handlers/users/github_oauth_callback.go
@@ -10,6 +10,7 @@ import (
 	"golang.org/x/oauth2"
 
 	"github.com/hatchet-dev/hatchet/api/v1/server/authn"
+	"github.com/hatchet-dev/hatchet/api/v1/server/middleware/redirect"
 	"github.com/hatchet-dev/hatchet/api/v1/server/oas/gen"
 	"github.com/hatchet-dev/hatchet/internal/config/server"
 	"github.com/hatchet-dev/hatchet/internal/repository"
@@ -21,37 +22,37 @@ func (u *UserService) UserUpdateGithubOauthCallback(ctx echo.Context, _ gen.User
 	isValid, _, err := authn.NewSessionHelpers(u.config).ValidateOAuthState(ctx, "github")
 
 	if err != nil || !isValid {
-		return nil, authn.GetRedirectWithError(ctx, u.config.Logger, err, "Could not log in. Please try again and make sure cookies are enabled.")
+		return nil, redirect.GetRedirectWithError(ctx, u.config.Logger, err, "Could not log in. Please try again and make sure cookies are enabled.")
 	}
 
 	token, err := u.config.Auth.GithubOAuthConfig.Exchange(context.Background(), ctx.Request().URL.Query().Get("code"))
 
 	if err != nil {
-		return nil, authn.GetRedirectWithError(ctx, u.config.Logger, err, "Forbidden")
+		return nil, redirect.GetRedirectWithError(ctx, u.config.Logger, err, "Forbidden")
 	}
 
 	if !token.Valid() {
-		return nil, authn.GetRedirectWithError(ctx, u.config.Logger, fmt.Errorf("invalid token"), "Forbidden")
+		return nil, redirect.GetRedirectWithError(ctx, u.config.Logger, fmt.Errorf("invalid token"), "Forbidden")
 	}
 
 	user, err := u.upsertGithubUserFromToken(u.config, token)
 
 	if err != nil {
 		if errors.Is(err, ErrGithubNotVerified) {
-			return nil, authn.GetRedirectWithError(ctx, u.config.Logger, err, "Please verify your email on Github.")
+			return nil, redirect.GetRedirectWithError(ctx, u.config.Logger, err, "Please verify your email on Github.")
 		}
 
 		if errors.Is(err, ErrGithubNoEmail) {
-			return nil, authn.GetRedirectWithError(ctx, u.config.Logger, err, "Github user must have an email.")
+			return nil, redirect.GetRedirectWithError(ctx, u.config.Logger, err, "Github user must have an email.")
 		}
 
-		return nil, authn.GetRedirectWithError(ctx, u.config.Logger, err, "Internal error.")
+		return nil, redirect.GetRedirectWithError(ctx, u.config.Logger, err, "Internal error.")
 	}
 
 	err = authn.NewSessionHelpers(u.config).SaveAuthenticated(ctx, user)
 
 	if err != nil {
-		return nil, authn.GetRedirectWithError(ctx, u.config.Logger, err, "Internal error.")
+		return nil, redirect.GetRedirectWithError(ctx, u.config.Logger, err, "Internal error.")
 	}
 
 	return gen.UserUpdateGithubOauthCallback302Response{

diff --git a/api/v1/server/handlers/users/github_oauth_start.go b/api/v1/server/handlers/users/github_oauth_start.go
@@ -4,6 +4,7 @@ import (
 	"github.com/labstack/echo/v4"
 
 	"github.com/hatchet-dev/hatchet/api/v1/server/authn"
+	"github.com/hatchet-dev/hatchet/api/v1/server/middleware/redirect"
 	"github.com/hatchet-dev/hatchet/api/v1/server/oas/gen"
 )
 
@@ -12,7 +13,7 @@ func (u *UserService) UserUpdateGithubOauthStart(ctx echo.Context, _ gen.UserUpd
 	state, err := authn.NewSessionHelpers(u.config).SaveOAuthState(ctx, "github")
 
 	if err != nil {
-		return nil, authn.GetRedirectWithError(ctx, u.config.Logger, err, "Could not get cookie. Please make sure cookies are enabled.")
+		return nil, redirect.GetRedirectWithError(ctx, u.config.Logger, err, "Could not get cookie. Please make sure cookies are enabled.")
 	}
 
 	url := u.config.Auth.GithubOAuthConfig.AuthCodeURL(state)

diff --git a/api/v1/server/handlers/users/google_oauth_callback.go b/api/v1/server/handlers/users/google_oauth_callback.go
@@ -11,6 +11,7 @@ import (
 	"golang.org/x/oauth2"
 
 	"github.com/hatchet-dev/hatchet/api/v1/server/authn"
+	"github.com/hatchet-dev/hatchet/api/v1/server/middleware/redirect"
 	"github.com/hatchet-dev/hatchet/api/v1/server/oas/gen"
 	"github.com/hatchet-dev/hatchet/internal/config/server"
 	"github.com/hatchet-dev/hatchet/internal/repository"
@@ -22,29 +23,29 @@ func (u *UserService) UserUpdateGoogleOauthCallback(ctx echo.Context, _ gen.User
 	isValid, _, err := authn.NewSessionHelpers(u.config).ValidateOAuthState(ctx, "google")
 
 	if err != nil || !isValid {
-		return nil, authn.GetRedirectWithError(ctx, u.config.Logger, err, "Could not log in. Please try again and make sure cookies are enabled.")
+		return nil, redirect.GetRedirectWithError(ctx, u.config.Logger, err, "Could not log in. Please try again and make sure cookies are enabled.")
 	}
 
 	token, err := u.config.Auth.GoogleOAuthConfig.Exchange(context.Background(), ctx.Request().URL.Query().Get("code"))
 
 	if err != nil {
-		return nil, authn.GetRedirectWithError(ctx, u.config.Logger, err, "Forbidden")
+		return nil, redirect.GetRedirectWithError(ctx, u.config.Logger, err, "Forbidden")
 	}
 
 	if !token.Valid() {
-		return nil, authn.GetRedirectWithError(ctx, u.config.Logger, fmt.Errorf("invalid token"), "Forbidden")
+		return nil, redirect.GetRedirectWithError(ctx, u.config.Logger, fmt.Errorf("invalid token"), "Forbidden")
 	}
 
 	user, err := u.upsertGoogleUserFromToken(u.config, token)
 
 	if err != nil {
-		return nil, authn.GetRedirectWithError(ctx, u.config.Logger, err, "Internal error.")
+		return nil, redirect.GetRedirectWithError(ctx, u.config.Logger, err, "Internal error.")
 	}
 
 	err = authn.NewSessionHelpers(u.config).SaveAuthenticated(ctx, user)
 
 	if err != nil {
-		return nil, authn.GetRedirectWithError(ctx, u.config.Logger, err, "Internal error.")
+		return nil, redirect.GetRedirectWithError(ctx, u.config.Logger, err, "Internal error.")
 	}
 
 	return gen.UserUpdateGoogleOauthCallback302Response{

diff --git a/api/v1/server/handlers/users/google_oauth_start.go b/api/v1/server/handlers/users/google_oauth_start.go
@@ -4,6 +4,7 @@ import (
 	"github.com/labstack/echo/v4"
 
 	"github.com/hatchet-dev/hatchet/api/v1/server/authn"
+	"github.com/hatchet-dev/hatchet/api/v1/server/middleware/redirect"
 	"github.com/hatchet-dev/hatchet/api/v1/server/oas/gen"
 )
 
@@ -12,7 +13,7 @@ func (u *UserService) UserUpdateGoogleOauthStart(ctx echo.Context, _ gen.UserUpd
 	state, err := authn.NewSessionHelpers(u.config).SaveOAuthState(ctx, "google")
 
 	if err != nil {
-		return nil, authn.GetRedirectWithError(ctx, u.config.Logger, err, "Could not get cookie. Please make sure cookies are enabled.")
+		return nil, redirect.GetRedirectWithError(ctx, u.config.Logger, err, "Could not get cookie. Please make sure cookies are enabled.")
 	}
 
 	url := u.config.Auth.GoogleOAuthConfig.AuthCodeURL(state)

diff --git a/api/v1/server/middleware/middleware.go b/api/v1/server/middleware/middleware.go
@@ -1,6 +1,7 @@
 package middleware
 
 import (
+	"errors"
 	"fmt"
 	"net/http"
 
@@ -9,6 +10,8 @@ import (
 	"github.com/getkin/kin-openapi/routers/gorillamux"
 	lru "github.com/hashicorp/golang-lru/v2"
 	"github.com/labstack/echo/v4"
+
+	"github.com/hatchet-dev/hatchet/api/v1/server/middleware/redirect"
 )
 
 type SecurityRequirement interface {
@@ -173,8 +176,14 @@ func (m *MiddlewareHandler) Middleware() (echo.MiddlewareFunc, error) {
 				m.cache.Add(getCacheKey(req), routeInfo)
 			}
 
-			for _, m := range m.mws {
-				if err := m(routeInfo)(c); err != nil {
+			for _, middlewareFunc := range m.mws {
+				if err := middlewareFunc(routeInfo)(c); err != nil {
+					// in the case of a redirect, we don't want to return an error but we want to stop the
+					// middleware chain
+					if errors.Is(err, redirect.ErrRedirect) {
+						return nil
+					}
+
 					return err
 				}
 			}

diff --git a/api/v1/server/authn/redirect.go → ...v1/server/middleware/redirect/redirect.go b/api/v1/server/authn/redirect.go → ...v1/server/middleware/redirect/redirect.go
@@ -1,15 +1,31 @@
-package authn
+package redirect
 
 import (
+	"errors"
+
 	"github.com/labstack/echo/v4"
 	"github.com/rs/zerolog"
 )
 
+var ErrRedirect = errors.New("redirecting")
+
 func GetRedirectWithError(ctx echo.Context, l *zerolog.Logger, internalErr error, userErr string) error {
 	l.Err(internalErr).Msgf("redirecting with error")
-	return ctx.Redirect(302, "/auth/login?error="+userErr)
+	err := ctx.Redirect(302, "/auth/login?error="+userErr)
+
+	if err != nil {
+		return err
+	}
+
+	return ErrRedirect
 }
 
 func GetRedirectNoError(ctx echo.Context, serverURL string) error {
-	return ctx.Redirect(302, serverURL)
+	err := ctx.Redirect(302, serverURL)
+
+	if err != nil {
+		return err
+	}
+
+	return ErrRedirect
 }