From e5a316e40c377a8563e92f62b0773ed34321a91a Mon Sep 17 00:00:00 2001 From: Luke Granger-Brown Date: Fri, 19 Jun 2020 00:26:47 +0100 Subject: [PATCH] Pull out context-stuff from identity into graphqlidentity package --- commands/webui.go | 13 +++-- graphql/graphqlidentity/graphqlidentity.go | 39 ++++++++++++++ graphql/resolvers/errors.go | 6 +++ graphql/resolvers/mutation.go | 59 ++++++++++++---------- graphql/resolvers/repo.go | 10 ++-- identity/context.go | 26 ---------- 6 files changed, 91 insertions(+), 62 deletions(-) create mode 100644 graphql/graphqlidentity/graphqlidentity.go create mode 100644 graphql/resolvers/errors.go delete mode 100644 identity/context.go diff --git a/commands/webui.go b/commands/webui.go index 24bdeced..90679c79 100644 --- a/commands/webui.go +++ b/commands/webui.go @@ -19,6 +19,7 @@ import ( "github.com/spf13/cobra" "github.com/MichaelMure/git-bug/graphql" + "github.com/MichaelMure/git-bug/graphql/graphqlidentity" "github.com/MichaelMure/git-bug/identity" "github.com/MichaelMure/git-bug/repository" "github.com/MichaelMure/git-bug/util/git" @@ -34,10 +35,10 @@ var ( const webUIOpenConfigKey = "git-bug.webui.open" -func authMiddleware(repo repository.RepoCommon, id *identity.Identity) func(http.Handler) http.Handler { +func authMiddleware(id *identity.Identity) func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - ctx := identity.AttachToContext(r.Context(), repo, id) + ctx := graphqlidentity.AttachToContext(r.Context(), id) next.ServeHTTP(w, r.WithContext(ctx)) }) } @@ -83,7 +84,7 @@ func runWebUI(cmd *cobra.Command, args []string) error { router.Path("/gitfile/{hash}").Handler(newGitFileHandler(repo)) router.Path("/upload").Methods("POST").Handler(newGitUploadFileHandler(repo)) router.PathPrefix("/").Handler(http.FileServer(assetsHandler)) - router.Use(authMiddleware(repo, id)) + router.Use(authMiddleware(id)) srv := &http.Server{ Addr: addr, @@ -210,7 +211,11 @@ func newGitUploadFileHandler(repo repository.Repo) http.Handler { } func (gufh *gitUploadFileHandler) ServeHTTP(rw http.ResponseWriter, r *http.Request) { - if identity.ForContext(r.Context(), gufh.repo) == nil { + id, err := graphqlidentity.ForContextUncached(r.Context(), gufh.repo) + if err != nil { + http.Error(rw, fmt.Sprintf("loading identity: %v", err), http.StatusInternalServerError) + return + } else if id == nil { http.Error(rw, fmt.Sprintf("read-only mode or not logged in"), http.StatusForbidden) return } diff --git a/graphql/graphqlidentity/graphqlidentity.go b/graphql/graphqlidentity/graphqlidentity.go new file mode 100644 index 00000000..6851c4a8 --- /dev/null +++ b/graphql/graphqlidentity/graphqlidentity.go @@ -0,0 +1,39 @@ +// Package graphqlidentity contains helpers for managing identities within the GraphQL API. +package graphqlidentity + +import ( + "context" + + "github.com/MichaelMure/git-bug/cache" + "github.com/MichaelMure/git-bug/entity" + "github.com/MichaelMure/git-bug/identity" + "github.com/MichaelMure/git-bug/repository" +) + +// identityCtxKey is a unique context key, accessible only in this package. +var identityCtxKey = &struct{}{} + +// AttachToContext attaches an Identity to a context. +func AttachToContext(ctx context.Context, u *identity.Identity) context.Context { + return context.WithValue(ctx, identityCtxKey, u.Id()) +} + +// ForContext retrieves an IdentityCache from the context, or nil if no identity is present. +// If an error occurs while resolving the identity (e.g. I/O error), then it will be returned. +func ForContext(ctx context.Context, r *cache.RepoCache) (*cache.IdentityCache, error) { + id, ok := ctx.Value(identityCtxKey).(entity.Id) + if !ok { + return nil, nil + } + return r.ResolveIdentity(id) +} + +// ForContextUncached retrieves an Identity from the context, or nil if no identity is present. +// If an error occurs while resolving the identity (e.g. I/O error), then it will be returned. +func ForContextUncached(ctx context.Context, repo repository.Repo) (*identity.Identity, error) { + id, ok := ctx.Value(identityCtxKey).(entity.Id) + if !ok { + return nil, nil + } + return identity.ReadLocal(repo, id) +} diff --git a/graphql/resolvers/errors.go b/graphql/resolvers/errors.go new file mode 100644 index 00000000..2a8024e0 --- /dev/null +++ b/graphql/resolvers/errors.go @@ -0,0 +1,6 @@ +package resolvers + +import "errors" + +// ErrNotAuthenticated is returned to the client if the user requests an action requiring authentication, and they are not authenticated. +var ErrNotAuthenticated = errors.New("not authenticated or read-only") diff --git a/graphql/resolvers/mutation.go b/graphql/resolvers/mutation.go index 62b92aaa..8d2d8081 100644 --- a/graphql/resolvers/mutation.go +++ b/graphql/resolvers/mutation.go @@ -2,18 +2,15 @@ package resolvers import ( "context" - "errors" "time" "github.com/MichaelMure/git-bug/bug" "github.com/MichaelMure/git-bug/cache" "github.com/MichaelMure/git-bug/graphql/graph" + "github.com/MichaelMure/git-bug/graphql/graphqlidentity" "github.com/MichaelMure/git-bug/graphql/models" - "github.com/MichaelMure/git-bug/identity" ) -var ErrNotAuthenticated = errors.New("not authenticated or read-only") - var _ graph.MutationResolver = &mutationResolver{} type mutationResolver struct { @@ -47,13 +44,14 @@ func (r mutationResolver) NewBug(ctx context.Context, input models.NewBugInput) return nil, err } - id := identity.ForContext(ctx, repo) - if id == nil { + id, err := graphqlidentity.ForContext(ctx, repo) + if err != nil { + return nil, err + } else if id == nil { return nil, ErrNotAuthenticated } - author := cache.NewIdentityCache(repo, id) - b, op, err := repo.NewBugRaw(author, time.Now().Unix(), input.Title, input.Message, input.Files, nil) + b, op, err := repo.NewBugRaw(id, time.Now().Unix(), input.Title, input.Message, input.Files, nil) if err != nil { return nil, err } @@ -71,13 +69,14 @@ func (r mutationResolver) AddComment(ctx context.Context, input models.AddCommen return nil, err } - id := identity.ForContext(ctx, repo) - if id == nil { + id, err := graphqlidentity.ForContext(ctx, repo) + if err != nil { + return nil, err + } else if id == nil { return nil, ErrNotAuthenticated } - author := cache.NewIdentityCache(repo, id) - op, err := b.AddCommentRaw(author, time.Now().Unix(), input.Message, input.Files, nil) + op, err := b.AddCommentRaw(id, time.Now().Unix(), input.Message, input.Files, nil) if err != nil { return nil, err } @@ -100,13 +99,14 @@ func (r mutationResolver) ChangeLabels(ctx context.Context, input *models.Change return nil, err } - id := identity.ForContext(ctx, repo) - if id == nil { + id, err := graphqlidentity.ForContext(ctx, repo) + if err != nil { + return nil, err + } else if id == nil { return nil, ErrNotAuthenticated } - author := cache.NewIdentityCache(repo, id) - results, op, err := b.ChangeLabelsRaw(author, time.Now().Unix(), input.Added, input.Removed, nil) + results, op, err := b.ChangeLabelsRaw(id, time.Now().Unix(), input.Added, input.Removed, nil) if err != nil { return nil, err } @@ -135,13 +135,14 @@ func (r mutationResolver) OpenBug(ctx context.Context, input models.OpenBugInput return nil, err } - id := identity.ForContext(ctx, repo) - if id == nil { + id, err := graphqlidentity.ForContext(ctx, repo) + if err != nil { + return nil, err + } else if id == nil { return nil, ErrNotAuthenticated } - author := cache.NewIdentityCache(repo, id) - op, err := b.OpenRaw(author, time.Now().Unix(), nil) + op, err := b.OpenRaw(id, time.Now().Unix(), nil) if err != nil { return nil, err } @@ -164,13 +165,14 @@ func (r mutationResolver) CloseBug(ctx context.Context, input models.CloseBugInp return nil, err } - id := identity.ForContext(ctx, repo) - if id == nil { + id, err := graphqlidentity.ForContext(ctx, repo) + if err != nil { + return nil, err + } else if id == nil { return nil, ErrNotAuthenticated } - author := cache.NewIdentityCache(repo, id) - op, err := b.CloseRaw(author, time.Now().Unix(), nil) + op, err := b.CloseRaw(id, time.Now().Unix(), nil) if err != nil { return nil, err } @@ -193,13 +195,14 @@ func (r mutationResolver) SetTitle(ctx context.Context, input models.SetTitleInp return nil, err } - id := identity.ForContext(ctx, repo) - if id == nil { + id, err := graphqlidentity.ForContext(ctx, repo) + if err != nil { + return nil, err + } else if id == nil { return nil, ErrNotAuthenticated } - author := cache.NewIdentityCache(repo, id) - op, err := b.SetTitleRaw(author, time.Now().Unix(), input.Title, nil) + op, err := b.SetTitleRaw(id, time.Now().Unix(), input.Title, nil) if err != nil { return nil, err } diff --git a/graphql/resolvers/repo.go b/graphql/resolvers/repo.go index ded98636..f68ed3e0 100644 --- a/graphql/resolvers/repo.go +++ b/graphql/resolvers/repo.go @@ -7,8 +7,8 @@ import ( "github.com/MichaelMure/git-bug/entity" "github.com/MichaelMure/git-bug/graphql/connections" "github.com/MichaelMure/git-bug/graphql/graph" + "github.com/MichaelMure/git-bug/graphql/graphqlidentity" "github.com/MichaelMure/git-bug/graphql/models" - "github.com/MichaelMure/git-bug/identity" "github.com/MichaelMure/git-bug/query" ) @@ -151,11 +151,13 @@ func (repoResolver) Identity(_ context.Context, obj *models.Repository, prefix s } func (repoResolver) UserIdentity(ctx context.Context, obj *models.Repository) (models.IdentityWrapper, error) { - id := identity.ForContext(ctx, obj.Repo) - if id == nil { + id, err := graphqlidentity.ForContext(ctx, obj.Repo) + if err != nil { + return nil, err + } else if id == nil { return nil, nil } - return models.NewLoadedIdentity(id), nil + return models.NewLoadedIdentity(id.Identity), nil } func (repoResolver) ValidLabels(_ context.Context, obj *models.Repository, after *string, before *string, first *int, last *int) (*models.LabelConnection, error) { diff --git a/identity/context.go b/identity/context.go deleted file mode 100644 index 619e2c29..00000000 --- a/identity/context.go +++ /dev/null @@ -1,26 +0,0 @@ -package identity - -import ( - "context" - - "github.com/MichaelMure/git-bug/repository" -) - -// identityCtxKey is a unique context key, accessible only in this struct. -type identityCtxKey struct { - repo string -} - -// AttachToContext attaches an Identity to a context. -func AttachToContext(ctx context.Context, r repository.RepoCommon, u *Identity) context.Context { - return context.WithValue(ctx, identityCtxKey{r.GetPath()}, u) -} - -// ForContext retrieves an Identity from the context, or nil if no Identity is present. -func ForContext(ctx context.Context, r repository.RepoCommon) *Identity { - u, ok := ctx.Value(identityCtxKey{r.GetPath()}).(*Identity) - if !ok { - return nil - } - return u -}