Skip to content

Commit 06bde90

Browse files
DylanTinianovDylanTinianov
authored
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
1 parent 0d9fe33 commit 06bde90

File tree

8 files changed

+35
-30
lines changed

8 files changed

+35
-30
lines changed

‎auth/auth.go

Lines changed: 21 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"github.com/getsentry/sentry-go"
2929
"github.com/google/uuid"
3030
"github.com/pkg/errors"
31+
"sync"
3132
)
3233

3334
// An Authenticator manages user authentication for the Playground API.
@@ -37,6 +38,7 @@ import (
3738
type Authenticator struct {
3839
store storage.Store
3940
sessionName string
41+
lock sync.Mutex
4042
}
4143

4244
// NewAuthenticator returns a new authenticator instance.
@@ -57,39 +59,32 @@ const userIDKey = "userID"
5759
// GetOrCreateUser gets an existing user from the current session or creates a
5860
// new user and session if a session does not already exist.
5961
func (a *Authenticator) GetOrCreateUser(ctx context.Context) (*model.User, error) {
60-
session := sessions.Get(ctx, a.sessionName)
61-
62-
var user *model.User
63-
var err error
62+
a.lock.Lock()
63+
defer a.lock.Unlock()
6464

65-
userLoaded := false
66-
67-
if !session.IsNew {
68-
// Try to load existing user
69-
if session.Values[userIDKey] != nil {
70-
user, err = a.getCurrentUser(session.Values[userIDKey].(string))
71-
if err != nil {
72-
sentry.CaptureException(errors.New(fmt.Sprintf(
73-
"Failed to load user id %s from session\n", session.Values[userIDKey].(string))))
74-
} else {
75-
userLoaded = true
76-
}
77-
}
78-
}
65+
session := sessions.Get(ctx, a.sessionName)
7966

80-
if !userLoaded {
81-
// Create new user
82-
user, err = a.createNewUser()
67+
if session.Values[userIDKey] == nil {
68+
// Create new user since UserID for cookie has not been created yet
69+
user, err := a.createNewUser()
8370
if err != nil {
8471
return nil, errors.Wrap(err, "failed to create new user")
8572
}
8673

8774
session.Values[userIDKey] = user.ID.String()
75+
76+
err = sessions.Save(ctx, session)
77+
if err != nil {
78+
fmt.Println("Failed to save session!")
79+
return nil, errors.Wrap(err, "failed to save userID to session")
80+
}
8881
}
8982

90-
err = sessions.Save(ctx, session)
83+
user, err := a.getCurrentUser(session.Values[userIDKey].(string))
9184
if err != nil {
92-
return nil, errors.Wrap(err, "failed to update session")
85+
sentry.CaptureException(errors.New(fmt.Sprintf(
86+
"Failed to load user id %s from session\n", session.Values[userIDKey].(string))))
87+
return nil, errors.New("failed to load user id from session")
9388
}
9489

9590
return user, nil
@@ -101,6 +96,9 @@ func (a *Authenticator) GetOrCreateUser(ctx context.Context) (*model.User, error
10196
// This function checks for access using both the new and legacy authentication schemes. If
10297
// a user has legacy access, their authentication is then migrated to use the new scheme.
10398
func (a *Authenticator) CheckProjectAccess(ctx context.Context, proj *model.Project) error {
99+
a.lock.Lock()
100+
defer a.lock.Unlock()
101+
104102
session := sessions.Get(ctx, a.sessionName)
105103

106104
if session.Values[userIDKey] == nil {

‎e2eTest/project_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@ func TestProjects(t *testing.T) {
257257
})
258258

259259
t.Run("Maximum projects limit", func(t *testing.T) {
260-
const MaxProjectsLimit = 10
260+
const MaxProjectsLimit = 50
261261
const additionalAttempts = 5 // Try to create projects over the limit
262262

263263
c := newClient()

‎handler.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ func GraphQLHandler(resolver *Resolver, middlewares ...graphql.ResponseMiddlewar
4242
}
4343

4444
srv.SetRecoverFunc(func(ctx context.Context, err interface{}) (userMessage error) {
45+
fmt.Println("Handler Recover: ", fmt.Errorf("panic: %v, stack: %s", err, string(debug.Stack())).Error())
4546
sentry.CaptureException(fmt.Errorf("panic: %v, stack: %s", err, string(debug.Stack())))
4647
return errors.ServerErr
4748
})

‎middleware/errors/errors.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ package errors
2121
import (
2222
"context"
2323
"errors"
24+
"fmt"
2425
"github.com/dapperlabs/flow-playground-api/telemetry"
2526

2627
"github.com/99designs/gqlgen/graphql"
@@ -65,6 +66,7 @@ func Middleware(entry *logrus.Entry, localHub *sentry.Hub) graphql.ResponseMiddl
6566
} else if errors.As(err, &authErr) {
6667
res.Extensions["code"] = "AUTHORIZATION_ERROR"
6768
} else {
69+
fmt.Println("Middleware errors: ", err.Error())
6870
localHub.CaptureException(err)
6971
telemetry.ServerErrorCounter.Inc()
7072
res.Errors[i].Message = ServerErr.Error()

‎middleware/sessions/sessions.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ package sessions
2020

2121
import (
2222
"context"
23+
"fmt"
2324
"net/http"
2425

2526
"github.com/gorilla/sessions"
@@ -53,6 +54,8 @@ func Get(ctx context.Context, name string) *sessions.Session {
5354
// ignore error because a session is always returned even if one does not exist
5455
session, _ := store.Get(httpcontext.Request(ctx), name)
5556

57+
_ = Save(ctx, session) // Pre save in case it's not saved elsewhere
58+
5659
return session
5760
}
5861

@@ -63,6 +66,7 @@ func Save(ctx context.Context, session *sessions.Session) error {
6366
httpcontext.Writer(ctx),
6467
)
6568
if err != nil {
69+
fmt.Println("Sessions Save(): failed to save session:", err.Error())
6670
return err
6771
}
6872

‎resolver.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ func (r *mutationResolver) authorize(ctx context.Context, ID uuid.UUID) error {
9999
}
100100

101101
if err := r.auth.CheckProjectAccess(ctx, proj); err != nil {
102-
return userErr.NewUserError("not authorized")
102+
return userErr.NewAuthorizationError("not authorized")
103103
}
104104

105105
return nil
@@ -108,7 +108,7 @@ func (r *mutationResolver) authorize(ctx context.Context, ID uuid.UUID) error {
108108
func (r *mutationResolver) CreateProject(ctx context.Context, input model.NewProject) (*model.Project, error) {
109109
user, err := r.auth.GetOrCreateUser(ctx)
110110
if err != nil {
111-
return nil, errors.Wrap(err, "failed to get or create user")
111+
return nil, userErr.NewAuthorizationError(err.Error())
112112
}
113113

114114
proj, err := r.projects.Create(user, input)
@@ -446,7 +446,7 @@ func (r *queryResolver) Account(_ context.Context, address model.Address, projec
446446
func (r *queryResolver) ProjectList(ctx context.Context) (*model.ProjectList, error) {
447447
user, err := r.auth.GetOrCreateUser(ctx)
448448
if err != nil {
449-
return nil, err
449+
return nil, userErr.NewAuthorizationError(err.Error())
450450
}
451451

452452
return r.projects.GetProjectListForUser(user.ID, r.auth, ctx)

‎server/config/playground.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ type PlaygroundConfig struct {
3232
LedgerCacheSize int `default:"128"`
3333
PlaygroundBaseURL string `default:"http://localhost:3000"`
3434
ForceMigration bool `default:"false"`
35-
MaxProjectsLimit int `default:"10"`
35+
MaxProjectsLimit int `default:"50"`
3636
StaleProjectDays int `default:"90"`
3737
StorageBackend string
3838
}

‎server/server.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ import (
4444
"github.com/dapperlabs/flow-playground-api/middleware/sessions"
4545
"github.com/dapperlabs/flow-playground-api/storage"
4646

47-
gqlPlayground "github.com/99designs/gqlgen/graphql/playground"
4847
"github.com/Masterminds/semver"
4948
stackdriver "github.com/TV4/logrus-stackdriver-formatter"
5049
"github.com/getsentry/sentry-go"
@@ -115,7 +114,7 @@ func main() {
115114
if conf.Debug {
116115
logger := httplog.NewLogger("playground-api", httplog.Options{Concise: true, JSON: true})
117116
router.Use(httplog.RequestLogger(logger))
118-
router.Handle("/", gqlPlayground.Handler("GraphQL playground", "/query"))
117+
//router.Handle("/", gqlPlayground.Handler("GraphQL playground", "/query"))
119118
}
120119

121120
if config.Telemetry().TracingEnabled {
@@ -163,6 +162,7 @@ func main() {
163162
defer func() {
164163
err := recover()
165164
if err != nil {
165+
fmt.Println("Server Recovered: ", err)
166166
localHub.Recover(err)
167167
sentry.Flush(time.Second * 5)
168168
}

0 commit comments

Comments
 (0)