Change graphql Go handlers to pluck identity out of context instead.

Luke Granger-Brown created 5 years ago

Change summary

commands/webui.go             |  27 +++++++--
graphql/config/config.go      |   7 --
graphql/graphql_test.go       |   3 
graphql/handler.go            |   5 -
graphql/resolvers/mutation.go | 107 ++++++++++++++++++++++--------------
graphql/resolvers/repo.go     |  16 ++---
graphql/resolvers/root.go     |  14 +---
7 files changed, 98 insertions(+), 81 deletions(-)

Detailed changes

commands/webui.go

@@ -19,7 +19,6 @@ import (
 	"github.com/spf13/cobra"
 
 	"github.com/MichaelMure/git-bug/graphql"
-	"github.com/MichaelMure/git-bug/graphql/config"
 	"github.com/MichaelMure/git-bug/identity"
 	"github.com/MichaelMure/git-bug/repository"
 	"github.com/MichaelMure/git-bug/util/git"
@@ -35,6 +34,15 @@ var (
 
 const webUIOpenConfigKey = "git-bug.webui.open"
 
+func authMiddleware(repo repository.RepoCommon, 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)
+			next.ServeHTTP(w, r.WithContext(ctx))
+		})
+	}
+}
+
 func runWebUI(cmd *cobra.Command, args []string) error {
 	if webUIPort == 0 {
 		var err error
@@ -44,9 +52,12 @@ func runWebUI(cmd *cobra.Command, args []string) error {
 		}
 	}
 
+	var id *identity.Identity
 	if !webUIReadOnly {
 		// Verify that we have an identity.
-		if _, err := identity.GetUserIdentity(repo); err != nil {
+		var err error
+		id, err = identity.GetUserIdentity(repo)
+		if err != nil {
 			return err
 		}
 	}
@@ -56,7 +67,7 @@ func runWebUI(cmd *cobra.Command, args []string) error {
 
 	router := mux.NewRouter()
 
-	graphqlHandler, err := graphql.NewHandler(repo, config.Config{ReadOnly: webUIReadOnly})
+	graphqlHandler, err := graphql.NewHandler(repo)
 	if err != nil {
 		return err
 	}
@@ -70,10 +81,9 @@ func runWebUI(cmd *cobra.Command, args []string) error {
 	router.Path("/playground").Handler(playground.Handler("git-bug", "/graphql"))
 	router.Path("/graphql").Handler(graphqlHandler)
 	router.Path("/gitfile/{hash}").Handler(newGitFileHandler(repo))
-	if !webUIReadOnly {
-		router.Path("/upload").Methods("POST").Handler(newGitUploadFileHandler(repo))
-	}
+	router.Path("/upload").Methods("POST").Handler(newGitUploadFileHandler(repo))
 	router.PathPrefix("/").Handler(http.FileServer(assetsHandler))
+	router.Use(authMiddleware(repo, id))
 
 	srv := &http.Server{
 		Addr:    addr,
@@ -200,6 +210,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 {
+		http.Error(rw, fmt.Sprintf("read-only mode or not logged in"), http.StatusForbidden)
+		return
+	}
+
 	// 100MB (github limit)
 	var maxUploadSize int64 = 100 * 1000 * 1000
 	r.Body = http.MaxBytesReader(rw, r.Body, maxUploadSize)

graphql/config/config.go

@@ -1,7 +0,0 @@
-// Package config contains configuration for GraphQL stuff.
-package config
-
-// Config holds configuration elements.
-type Config struct {
-	ReadOnly bool
-}

graphql/graphql_test.go

@@ -5,7 +5,6 @@ import (
 
 	"github.com/99designs/gqlgen/client"
 
-	"github.com/MichaelMure/git-bug/graphql/config"
 	"github.com/MichaelMure/git-bug/graphql/models"
 	"github.com/MichaelMure/git-bug/misc/random_bugs"
 	"github.com/MichaelMure/git-bug/repository"
@@ -17,7 +16,7 @@ func TestQueries(t *testing.T) {
 
 	random_bugs.FillRepoWithSeed(repo, 10, 42)
 
-	handler, err := NewHandler(repo, config.Config{})
+	handler, err := NewHandler(repo)
 	if err != nil {
 		t.Fatal(err)
 	}

graphql/handler.go

@@ -8,7 +8,6 @@ import (
 
 	"github.com/99designs/gqlgen/graphql/handler"
 
-	"github.com/MichaelMure/git-bug/graphql/config"
 	"github.com/MichaelMure/git-bug/graphql/graph"
 	"github.com/MichaelMure/git-bug/graphql/resolvers"
 	"github.com/MichaelMure/git-bug/repository"
@@ -20,9 +19,9 @@ type Handler struct {
 	*resolvers.RootResolver
 }
 
-func NewHandler(repo repository.ClockedRepo, cfg config.Config) (Handler, error) {
+func NewHandler(repo repository.ClockedRepo) (Handler, error) {
 	h := Handler{
-		RootResolver: resolvers.NewRootResolver(cfg),
+		RootResolver: resolvers.NewRootResolver(),
 	}
 
 	err := h.RootResolver.RegisterDefaultRepository(repo)

graphql/resolvers/mutation.go

@@ -2,36 +2,17 @@ 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/models"
-	"github.com/vektah/gqlparser/gqlerror"
+	"github.com/MichaelMure/git-bug/identity"
 )
 
-var _ graph.MutationResolver = &readonlyMutationResolver{}
-
-type readonlyMutationResolver struct{}
-
-func (readonlyMutationResolver) NewBug(_ context.Context, _ models.NewBugInput) (*models.NewBugPayload, error) {
-	return nil, gqlerror.Errorf("readonly mode")
-}
-func (readonlyMutationResolver) AddComment(_ context.Context, input models.AddCommentInput) (*models.AddCommentPayload, error) {
-	return nil, gqlerror.Errorf("readonly mode")
-}
-func (readonlyMutationResolver) ChangeLabels(_ context.Context, input *models.ChangeLabelInput) (*models.ChangeLabelPayload, error) {
-	return nil, gqlerror.Errorf("readonly mode")
-}
-func (readonlyMutationResolver) OpenBug(_ context.Context, input models.OpenBugInput) (*models.OpenBugPayload, error) {
-	return nil, gqlerror.Errorf("readonly mode")
-}
-func (readonlyMutationResolver) CloseBug(_ context.Context, input models.CloseBugInput) (*models.CloseBugPayload, error) {
-	return nil, gqlerror.Errorf("readonly mode")
-}
-func (readonlyMutationResolver) SetTitle(_ context.Context, input models.SetTitleInput) (*models.SetTitlePayload, error) {
-	return nil, gqlerror.Errorf("readonly mode")
-}
+var ErrNotAuthenticated = errors.New("not authenticated or read-only")
 
 var _ graph.MutationResolver = &mutationResolver{}
 
@@ -47,22 +28,32 @@ func (r mutationResolver) getRepo(ref *string) (*cache.RepoCache, error) {
 	return r.cache.DefaultRepo()
 }
 
-func (r mutationResolver) getBug(repoRef *string, bugPrefix string) (*cache.BugCache, error) {
+func (r mutationResolver) getBug(repoRef *string, bugPrefix string) (*cache.RepoCache, *cache.BugCache, error) {
 	repo, err := r.getRepo(repoRef)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 
-	return repo.ResolveBugPrefix(bugPrefix)
+	bug, err := repo.ResolveBugPrefix(bugPrefix)
+	if err != nil {
+		return nil, nil, err
+	}
+	return repo, bug, nil
 }
 
-func (r mutationResolver) NewBug(_ context.Context, input models.NewBugInput) (*models.NewBugPayload, error) {
+func (r mutationResolver) NewBug(ctx context.Context, input models.NewBugInput) (*models.NewBugPayload, error) {
 	repo, err := r.getRepo(input.RepoRef)
 	if err != nil {
 		return nil, err
 	}
 
-	b, op, err := repo.NewBugWithFiles(input.Title, input.Message, input.Files)
+	id := identity.ForContext(ctx, repo)
+	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)
 	if err != nil {
 		return nil, err
 	}
@@ -74,13 +65,19 @@ func (r mutationResolver) NewBug(_ context.Context, input models.NewBugInput) (*
 	}, nil
 }
 
-func (r mutationResolver) AddComment(_ context.Context, input models.AddCommentInput) (*models.AddCommentPayload, error) {
-	b, err := r.getBug(input.RepoRef, input.Prefix)
+func (r mutationResolver) AddComment(ctx context.Context, input models.AddCommentInput) (*models.AddCommentPayload, error) {
+	repo, b, err := r.getBug(input.RepoRef, input.Prefix)
 	if err != nil {
 		return nil, err
 	}
 
-	op, err := b.AddCommentWithFiles(input.Message, input.Files)
+	id := identity.ForContext(ctx, repo)
+	if id == nil {
+		return nil, ErrNotAuthenticated
+	}
+	author := cache.NewIdentityCache(repo, id)
+
+	op, err := b.AddCommentRaw(author, time.Now().Unix(), input.Message, input.Files, nil)
 	if err != nil {
 		return nil, err
 	}
@@ -97,13 +94,19 @@ func (r mutationResolver) AddComment(_ context.Context, input models.AddCommentI
 	}, nil
 }
 
-func (r mutationResolver) ChangeLabels(_ context.Context, input *models.ChangeLabelInput) (*models.ChangeLabelPayload, error) {
-	b, err := r.getBug(input.RepoRef, input.Prefix)
+func (r mutationResolver) ChangeLabels(ctx context.Context, input *models.ChangeLabelInput) (*models.ChangeLabelPayload, error) {
+	repo, b, err := r.getBug(input.RepoRef, input.Prefix)
 	if err != nil {
 		return nil, err
 	}
 
-	results, op, err := b.ChangeLabels(input.Added, input.Removed)
+	id := identity.ForContext(ctx, repo)
+	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)
 	if err != nil {
 		return nil, err
 	}
@@ -126,13 +129,19 @@ func (r mutationResolver) ChangeLabels(_ context.Context, input *models.ChangeLa
 	}, nil
 }
 
-func (r mutationResolver) OpenBug(_ context.Context, input models.OpenBugInput) (*models.OpenBugPayload, error) {
-	b, err := r.getBug(input.RepoRef, input.Prefix)
+func (r mutationResolver) OpenBug(ctx context.Context, input models.OpenBugInput) (*models.OpenBugPayload, error) {
+	repo, b, err := r.getBug(input.RepoRef, input.Prefix)
 	if err != nil {
 		return nil, err
 	}
 
-	op, err := b.Open()
+	id := identity.ForContext(ctx, repo)
+	if id == nil {
+		return nil, ErrNotAuthenticated
+	}
+	author := cache.NewIdentityCache(repo, id)
+
+	op, err := b.OpenRaw(author, time.Now().Unix(), nil)
 	if err != nil {
 		return nil, err
 	}
@@ -149,13 +158,19 @@ func (r mutationResolver) OpenBug(_ context.Context, input models.OpenBugInput)
 	}, nil
 }
 
-func (r mutationResolver) CloseBug(_ context.Context, input models.CloseBugInput) (*models.CloseBugPayload, error) {
-	b, err := r.getBug(input.RepoRef, input.Prefix)
+func (r mutationResolver) CloseBug(ctx context.Context, input models.CloseBugInput) (*models.CloseBugPayload, error) {
+	repo, b, err := r.getBug(input.RepoRef, input.Prefix)
 	if err != nil {
 		return nil, err
 	}
 
-	op, err := b.Close()
+	id := identity.ForContext(ctx, repo)
+	if id == nil {
+		return nil, ErrNotAuthenticated
+	}
+	author := cache.NewIdentityCache(repo, id)
+
+	op, err := b.CloseRaw(author, time.Now().Unix(), nil)
 	if err != nil {
 		return nil, err
 	}
@@ -172,13 +187,19 @@ func (r mutationResolver) CloseBug(_ context.Context, input models.CloseBugInput
 	}, nil
 }
 
-func (r mutationResolver) SetTitle(_ context.Context, input models.SetTitleInput) (*models.SetTitlePayload, error) {
-	b, err := r.getBug(input.RepoRef, input.Prefix)
+func (r mutationResolver) SetTitle(ctx context.Context, input models.SetTitleInput) (*models.SetTitlePayload, error) {
+	repo, b, err := r.getBug(input.RepoRef, input.Prefix)
 	if err != nil {
 		return nil, err
 	}
 
-	op, err := b.SetTitle(input.Title)
+	id := identity.ForContext(ctx, repo)
+	if id == nil {
+		return nil, ErrNotAuthenticated
+	}
+	author := cache.NewIdentityCache(repo, id)
+
+	op, err := b.SetTitleRaw(author, time.Now().Unix(), input.Title, nil)
 	if err != nil {
 		return nil, err
 	}

graphql/resolvers/repo.go

@@ -5,16 +5,16 @@ import (
 
 	"github.com/MichaelMure/git-bug/bug"
 	"github.com/MichaelMure/git-bug/entity"
-	"github.com/MichaelMure/git-bug/graphql/config"
 	"github.com/MichaelMure/git-bug/graphql/connections"
 	"github.com/MichaelMure/git-bug/graphql/graph"
 	"github.com/MichaelMure/git-bug/graphql/models"
+	"github.com/MichaelMure/git-bug/identity"
 	"github.com/MichaelMure/git-bug/query"
 )
 
 var _ graph.RepositoryResolver = &repoResolver{}
 
-type repoResolver struct{ cfg config.Config }
+type repoResolver struct{}
 
 func (repoResolver) Name(_ context.Context, obj *models.Repository) (*string, error) {
 	name := obj.Repo.Name()
@@ -150,16 +150,12 @@ func (repoResolver) Identity(_ context.Context, obj *models.Repository, prefix s
 	return models.NewLazyIdentity(obj.Repo, excerpt), nil
 }
 
-func (r repoResolver) UserIdentity(_ context.Context, obj *models.Repository) (models.IdentityWrapper, error) {
-	if r.cfg.ReadOnly {
+func (repoResolver) UserIdentity(ctx context.Context, obj *models.Repository) (models.IdentityWrapper, error) {
+	id := identity.ForContext(ctx, obj.Repo)
+	if id == nil {
 		return nil, nil
 	}
-	excerpt, err := obj.Repo.GetUserIdentityExcerpt()
-	if err != nil {
-		return nil, err
-	}
-
-	return models.NewLazyIdentity(obj.Repo, excerpt), nil
+	return models.NewLoadedIdentity(id), nil
 }
 
 func (repoResolver) ValidLabels(_ context.Context, obj *models.Repository, after *string, before *string, first *int, last *int) (*models.LabelConnection, error) {

graphql/resolvers/root.go

@@ -3,7 +3,6 @@ package resolvers
 
 import (
 	"github.com/MichaelMure/git-bug/cache"
-	"github.com/MichaelMure/git-bug/graphql/config"
 	"github.com/MichaelMure/git-bug/graphql/graph"
 )
 
@@ -11,13 +10,11 @@ var _ graph.ResolverRoot = &RootResolver{}
 
 type RootResolver struct {
 	cache.MultiRepoCache
-	cfg config.Config
 }
 
-func NewRootResolver(cfg config.Config) *RootResolver {
+func NewRootResolver() *RootResolver {
 	return &RootResolver{
 		MultiRepoCache: cache.NewMultiRepoCache(),
-		cfg:            cfg,
 	}
 }
 
@@ -28,16 +25,13 @@ func (r RootResolver) Query() graph.QueryResolver {
 }
 
 func (r RootResolver) Mutation() graph.MutationResolver {
-	if r.cfg.ReadOnly {
-		return &readonlyMutationResolver{}
-	}
 	return &mutationResolver{
 		cache: &r.MultiRepoCache,
 	}
 }
 
-func (r RootResolver) Repository() graph.RepositoryResolver {
-	return &repoResolver{r.cfg}
+func (RootResolver) Repository() graph.RepositoryResolver {
+	return &repoResolver{}
 }
 
 func (RootResolver) Bug() graph.BugResolver {
@@ -56,7 +50,7 @@ func (RootResolver) Label() graph.LabelResolver {
 	return &labelResolver{}
 }
 
-func (RootResolver) Identity() graph.IdentityResolver {
+func (r RootResolver) Identity() graph.IdentityResolver {
 	return &identityResolver{}
 }