Fix User Auth and Session (#322)

* debug project list * debug project list * debug project list * debug project list * debug project list * debug project list * debug project list * debug project list * debug project list * debug project list * debug project list * debug project list * save session * Increase project limit * Add auth lock * Add auth lock * debug response * Remove debug prints * Remove immutable counter * fix project limit test
onflow · Aug 29, 2023 · 06bde90 · 06bde90
1 parent 0d9fe33
commit 06bde90
Show file tree

Hide file tree

Showing 8 changed files with 35 additions and 30 deletions.
diff --git a/auth/auth.go b/auth/auth.go
@@ -28,6 +28,7 @@ import (
 	"github.com/getsentry/sentry-go"
 	"github.com/google/uuid"
 	"github.com/pkg/errors"
+	"sync"
 )
 
 // An Authenticator manages user authentication for the Playground API.
@@ -37,6 +38,7 @@ import (
 type Authenticator struct {
 	store       storage.Store
 	sessionName string
+	lock        sync.Mutex
 }
 
 // NewAuthenticator returns a new authenticator instance.
@@ -57,39 +59,32 @@ const userIDKey = "userID"
 // GetOrCreateUser gets an existing user from the current session or creates a
 // new user and session if a session does not already exist.
 func (a *Authenticator) GetOrCreateUser(ctx context.Context) (*model.User, error) {
-	session := sessions.Get(ctx, a.sessionName)
-
-	var user *model.User
-	var err error
+	a.lock.Lock()
+	defer a.lock.Unlock()
 
-	userLoaded := false
-
-	if !session.IsNew {
-		// Try to load existing user
-		if session.Values[userIDKey] != nil {
-			user, err = a.getCurrentUser(session.Values[userIDKey].(string))
-			if err != nil {
-				sentry.CaptureException(errors.New(fmt.Sprintf(
-					"Failed to load user id %s from session\n", session.Values[userIDKey].(string))))
-			} else {
-				userLoaded = true
-			}
-		}
-	}
+	session := sessions.Get(ctx, a.sessionName)
 
-	if !userLoaded {
-		// Create new user
-		user, err = a.createNewUser()
+	if session.Values[userIDKey] == nil {
+		// Create new user since UserID for cookie has not been created yet
+		user, err := a.createNewUser()
 		if err != nil {
 			return nil, errors.Wrap(err, "failed to create new user")
 		}
 
 		session.Values[userIDKey] = user.ID.String()
+
+		err = sessions.Save(ctx, session)
+		if err != nil {
+			fmt.Println("Failed to save session!")
+			return nil, errors.Wrap(err, "failed to save userID to session")
+		}
 	}
 
-	err = sessions.Save(ctx, session)
+	user, err := a.getCurrentUser(session.Values[userIDKey].(string))
 	if err != nil {
-		return nil, errors.Wrap(err, "failed to update session")
+		sentry.CaptureException(errors.New(fmt.Sprintf(
+			"Failed to load user id %s from session\n", session.Values[userIDKey].(string))))
+		return nil, errors.New("failed to load user id from session")
 	}
 
 	return user, nil
@@ -101,6 +96,9 @@ func (a *Authenticator) GetOrCreateUser(ctx context.Context) (*model.User, error
 // This function checks for access using both the new and legacy authentication schemes. If
 // a user has legacy access, their authentication is then migrated to use the new scheme.
 func (a *Authenticator) CheckProjectAccess(ctx context.Context, proj *model.Project) error {
+	a.lock.Lock()
+	defer a.lock.Unlock()
+
 	session := sessions.Get(ctx, a.sessionName)
 
 	if session.Values[userIDKey] == nil {

diff --git a/e2eTest/project_test.go b/e2eTest/project_test.go
@@ -257,7 +257,7 @@ func TestProjects(t *testing.T) {
 	})
 
 	t.Run("Maximum projects limit", func(t *testing.T) {
-		const MaxProjectsLimit = 10
+		const MaxProjectsLimit = 50
 		const additionalAttempts = 5 // Try to create projects over the limit
 
 		c := newClient()

diff --git a/handler.go b/handler.go
@@ -42,6 +42,7 @@ func GraphQLHandler(resolver *Resolver, middlewares ...graphql.ResponseMiddlewar
 	}
 
 	srv.SetRecoverFunc(func(ctx context.Context, err interface{}) (userMessage error) {
+		fmt.Println("Handler Recover: ", fmt.Errorf("panic: %v, stack: %s", err, string(debug.Stack())).Error())
 		sentry.CaptureException(fmt.Errorf("panic: %v, stack: %s", err, string(debug.Stack())))
 		return errors.ServerErr
 	})

diff --git a/middleware/errors/errors.go b/middleware/errors/errors.go
@@ -21,6 +21,7 @@ package errors
 import (
 	"context"
 	"errors"
+	"fmt"
 	"github.com/dapperlabs/flow-playground-api/telemetry"
 
 	"github.com/99designs/gqlgen/graphql"
@@ -65,6 +66,7 @@ func Middleware(entry *logrus.Entry, localHub *sentry.Hub) graphql.ResponseMiddl
 				} else if errors.As(err, &authErr) {
 					res.Extensions["code"] = "AUTHORIZATION_ERROR"
 				} else {
+					fmt.Println("Middleware errors: ", err.Error())
 					localHub.CaptureException(err)
 					telemetry.ServerErrorCounter.Inc()
 					res.Errors[i].Message = ServerErr.Error()

diff --git a/middleware/sessions/sessions.go b/middleware/sessions/sessions.go
@@ -20,6 +20,7 @@ package sessions
 
 import (
 	"context"
+	"fmt"
 	"net/http"
 
 	"github.com/gorilla/sessions"
@@ -53,6 +54,8 @@ func Get(ctx context.Context, name string) *sessions.Session {
 	// ignore error because a session is always returned even if one does not exist
 	session, _ := store.Get(httpcontext.Request(ctx), name)
 
+	_ = Save(ctx, session) // Pre save in case it's not saved elsewhere
+
 	return session
 }
 
@@ -63,6 +66,7 @@ func Save(ctx context.Context, session *sessions.Session) error {
 		httpcontext.Writer(ctx),
 	)
 	if err != nil {
+		fmt.Println("Sessions Save(): failed to save session:", err.Error())
 		return err
 	}
 

diff --git a/resolver.go b/resolver.go
@@ -99,7 +99,7 @@ func (r *mutationResolver) authorize(ctx context.Context, ID uuid.UUID) error {
 	}
 
 	if err := r.auth.CheckProjectAccess(ctx, proj); err != nil {
-		return userErr.NewUserError("not authorized")
+		return userErr.NewAuthorizationError("not authorized")
 	}
 
 	return nil
@@ -108,7 +108,7 @@ func (r *mutationResolver) authorize(ctx context.Context, ID uuid.UUID) error {
 func (r *mutationResolver) CreateProject(ctx context.Context, input model.NewProject) (*model.Project, error) {
 	user, err := r.auth.GetOrCreateUser(ctx)
 	if err != nil {
-		return nil, errors.Wrap(err, "failed to get or create user")
+		return nil, userErr.NewAuthorizationError(err.Error())
 	}
 
 	proj, err := r.projects.Create(user, input)
@@ -446,7 +446,7 @@ func (r *queryResolver) Account(_ context.Context, address model.Address, projec
 func (r *queryResolver) ProjectList(ctx context.Context) (*model.ProjectList, error) {
 	user, err := r.auth.GetOrCreateUser(ctx)
 	if err != nil {
-		return nil, err
+		return nil, userErr.NewAuthorizationError(err.Error())
 	}
 
 	return r.projects.GetProjectListForUser(user.ID, r.auth, ctx)

diff --git a/server/config/playground.go b/server/config/playground.go
@@ -32,7 +32,7 @@ type PlaygroundConfig struct {
 	LedgerCacheSize            int           `default:"128"`
 	PlaygroundBaseURL          string        `default:"http://localhost:3000"`
 	ForceMigration             bool          `default:"false"`
-	MaxProjectsLimit           int           `default:"10"`
+	MaxProjectsLimit           int           `default:"50"`
 	StaleProjectDays           int           `default:"90"`
 	StorageBackend             string
 }

diff --git a/server/server.go b/server/server.go
@@ -44,7 +44,6 @@ import (
 	"github.com/dapperlabs/flow-playground-api/middleware/sessions"
 	"github.com/dapperlabs/flow-playground-api/storage"
 
-	gqlPlayground "github.com/99designs/gqlgen/graphql/playground"
 	"github.com/Masterminds/semver"
 	stackdriver "github.com/TV4/logrus-stackdriver-formatter"
 	"github.com/getsentry/sentry-go"
@@ -115,7 +114,7 @@ func main() {
 	if conf.Debug {
 		logger := httplog.NewLogger("playground-api", httplog.Options{Concise: true, JSON: true})
 		router.Use(httplog.RequestLogger(logger))
-		router.Handle("/", gqlPlayground.Handler("GraphQL playground", "/query"))
+		//router.Handle("/", gqlPlayground.Handler("GraphQL playground", "/query"))
 	}
 
 	if config.Telemetry().TracingEnabled {
@@ -163,6 +162,7 @@ func main() {
 		defer func() {
 			err := recover()
 			if err != nil {
+				fmt.Println("Server Recovered: ", err)
 				localHub.Recover(err)
 				sentry.Flush(time.Second * 5)
 			}