Repository: preslavmihaylov/todocheck
Pull request #128 (2021-01-08 16:47:24)

extracted authentication middleware into issuetracker component
One part of the `issuetracker` refactoring is to move the authentication middleware logic into the `issuetracker` component.

This would enable easier onboarding of new issue trackers as all routines an issue tracker needs to support are provided by the `issuetracker.IssueTracker` interface. It's much easier this way than having a mental model of the whole codebase to know that there is some sneaky component somewhere, instrumenting an authentication header.

Related to #82


Patch:
diff --git a/authmanager/authmiddleware/authmiddleware.go b/authmanager/authmiddleware/authmiddleware.go
deleted file mode 100644
index c7b48d8..0000000
--- a/authmanager/authmiddleware/authmiddleware.go
+++ /dev/null
@@ -1,66 +0,0 @@
-package authmiddleware
-
-import (
-	"net/http"
-
-	"github.com/preslavmihaylov/todocheck/config"
-)
-
-// Func used as callback to plug-in necessary auth headers
-type Func func(r *http.Request)
-
-// For creates a new auth middleware Func based on the given configuration
-func For(cfg *config.Local) Func {
-	if cfg.Auth == nil || cfg.Auth.Type == config.AuthTypeNone {
-		return noAuthMiddleware()
-	}
-
-	assertInvariant(cfg.Auth.Token != "", "invariant violated. No token found for auth middleware")
-	if cfg.Auth.Type == config.AuthTypeOffline {
-		return authorizationTokenMiddleware(cfg.Auth.Token)
-	} else if cfg.IssueTracker == config.IssueTrackerGithub && cfg.Auth.Type == config.AuthTypeAPIToken {
-		return authorizationTokenMiddleware(cfg.Auth.Token)
-	} else if cfg.IssueTracker == config.IssueTrackerGitlab && cfg.Auth.Type == config.AuthTypeAPIToken {
-		return gitlabAPITokenMiddleware(cfg.Auth.Token)
-	} else if cfg.IssueTracker == config.IssueTrackerPivotal && cfg.Auth.Type == config.AuthTypeAPIToken {
-		return pivotalTrackerAPITokenMiddleware(cfg.Auth.Token)
-	} else if cfg.IssueTracker == config.IssueTrackerRedmine && cfg.Auth.Type == config.AuthTypeAPIToken {
-		return redmineAPITokenMiddleware(cfg.Auth.Token)
-	}
-
-	panic("couldn't derive authentication middleware based on the given local configuration")
-}
-
-func noAuthMiddleware() Func {
-	return func(r *http.Request) {}
-}
-
-func authorizationTokenMiddleware(token string) Func {
-	return func(r *http.Request) {
-		r.Header.Add("Authorization", "Bearer "+token)
-	}
-}
-
-func gitlabAPITokenMiddleware(token string) Func {
-	return func(r *http.Request) {
-		r.Header.Add("PRIVATE-TOKEN", token)
-	}
-}
-
-func pivotalTrackerAPITokenMiddleware(token string) Func {
-	return func(r *http.Request) {
-		r.Header.Add("X-TrackerToken", token)
-	}
-}
-
-func redmineAPITokenMiddleware(token string) Func {
-	return func(r *http.Request) {
-		r.Header.Add("X-Redmine-API-Key", token)
-	}
-}
-
-func assertInvariant(condition bool, msg string) {
-	if !condition {
-		panic(msg)
-	}
-}
diff --git a/common/common.go b/common/common.go
index 7de5352..9f66376 100644
--- a/common/common.go
+++ b/common/common.go
@@ -25,3 +25,10 @@ func ArrayAsRegexAnyMatchExpression(todos []string) string {
 	}
 	return fmt.Sprintf("(?:%s)", strings.Join(todos, "|"))
 }
+
+// Assert panics if the provided condition is not met
+func Assert(condition bool, msg string) {
+	if !condition {
+		panic("Invariant violated: " + msg)
+	}
+}
diff --git a/config/auth.go b/config/auth.go
index 4414ec5..f4fcb07 100644
--- a/config/auth.go
+++ b/config/auth.go
@@ -56,3 +56,7 @@ func (a AuthType) IsValid() bool {
 
 	return false
 }
+
+func (a AuthType) String() string {
+	return string(a)
+}
diff --git a/fetcher/fetcher.go b/fetcher/fetcher.go
index a23ac02..e0e94c7 100644
--- a/fetcher/fetcher.go
+++ b/fetcher/fetcher.go
@@ -6,7 +6,6 @@ import (
 	"io/ioutil"
 	"net/http"
 
-	"github.com/preslavmihaylov/todocheck/authmanager/authmiddleware"
 	"github.com/preslavmihaylov/todocheck/issuetracker"
 	"github.com/preslavmihaylov/todocheck/issuetracker/taskstatus"
 )
@@ -14,12 +13,11 @@ import (
 // Fetcher for task statuses by contacting task management web apps' rest api
 type Fetcher struct {
 	issuetracker.IssueTracker
-	authMiddleware authmiddleware.Func
 }
 
 // NewFetcher instance
-func NewFetcher(issueTracker issuetracker.IssueTracker, authMw authmiddleware.Func) *Fetcher {
-	return &Fetcher{issueTracker, authMw}
+func NewFetcher(issueTracker issuetracker.IssueTracker) *Fetcher {
+	return &Fetcher{issueTracker}
 }
 
 // Fetch a task's status based on task ID
@@ -29,7 +27,10 @@ func (f *Fetcher) Fetch(taskID string) (taskstatus.TaskStatus, error) {
 		return taskstatus.None, fmt.Errorf("failed creating new GET request: %w", err)
 	}
 
-	f.authMiddleware(req)
+	err = f.IssueTracker.InstrumentMiddleware(req)
+	if err != nil {
+		return taskstatus.None, fmt.Errorf("couldn't instrument authentication middleware: %w", err)
+	}
 
 	hclient := &http.Client{}
 	resp, err := hclient.Do(req)
diff --git a/issuetracker/factory/factory.go b/issuetracker/factory/factory.go
index e1a562d..be2b5ea 100644
--- a/issuetracker/factory/factory.go
+++ b/issuetracker/factory/factory.go
@@ -5,27 +5,27 @@ import (
 
 	"github.com/preslavmihaylov/todocheck/config"
 	"github.com/preslavmihaylov/todocheck/issuetracker"
-	"github.com/preslavmihaylov/todocheck/issuetracker/github"
-	"github.com/preslavmihaylov/todocheck/issuetracker/gitlab"
-	"github.com/preslavmihaylov/todocheck/issuetracker/jira"
-	"github.com/preslavmihaylov/todocheck/issuetracker/pivotaltracker"
-	"github.com/preslavmihaylov/todocheck/issuetracker/redmine"
+	"github.com/preslavmihaylov/todocheck/issuetracker/internal/github"
+	"github.com/preslavmihaylov/todocheck/issuetracker/internal/gitlab"
+	"github.com/preslavmihaylov/todocheck/issuetracker/internal/jira"
+	"github.com/preslavmihaylov/todocheck/issuetracker/internal/pivotaltracker"
+	"github.com/preslavmihaylov/todocheck/issuetracker/internal/redmine"
 )
 
 // NewIssueTrackerFrom is a static factory method for creating an issuetracker.IssueTracker instance based on the chosen issue tracker type
 // in the configuration
-func NewIssueTrackerFrom(issueTrackerType config.IssueTracker, origin string) (issuetracker.IssueTracker, error) {
+func NewIssueTrackerFrom(issueTrackerType config.IssueTracker, authCfg *config.Auth, origin string) (issuetracker.IssueTracker, error) {
 	switch issueTrackerType {
 	case config.IssueTrackerGithub:
-		return &github.IssueTracker{Origin: origin}, nil
+		return &github.IssueTracker{Origin: origin, AuthCfg: authCfg}, nil
 	case config.IssueTrackerJira:
-		return &jira.IssueTracker{Origin: origin}, nil
+		return &jira.IssueTracker{Origin: origin, AuthCfg: authCfg}, nil
 	case config.IssueTrackerGitlab:
-		return &gitlab.IssueTracker{Origin: origin}, nil
+		return &gitlab.IssueTracker{Origin: origin, AuthCfg: authCfg}, nil
 	case config.IssueTrackerRedmine:
-		return &redmine.IssueTracker{Origin: origin}, nil
+		return &redmine.IssueTracker{Origin: origin, AuthCfg: authCfg}, nil
 	case config.IssueTrackerPivotal:
-		return &pivotaltracker.IssueTracker{Origin: origin}, nil
+		return &pivotaltracker.IssueTracker{Origin: origin, AuthCfg: authCfg}, nil
 	}
 
 	return nil, errors.New("unknown issue tracker " + string(issueTrackerType))
diff --git a/issuetracker/github/github.go b/issuetracker/internal/github/github.go
similarity index 73%
rename from issuetracker/github/github.go
rename to issuetracker/internal/github/github.go
index 1385bfc..42ead10 100644
--- a/issuetracker/github/github.go
+++ b/issuetracker/internal/github/github.go
@@ -2,15 +2,18 @@ package github
 
 import (
 	"fmt"
+	"net/http"
 	"strings"
 
 	"github.com/preslavmihaylov/todocheck/common"
+	"github.com/preslavmihaylov/todocheck/config"
 	"github.com/preslavmihaylov/todocheck/issuetracker"
 )
 
 // IssueTracker implementation for integrating with public & private github issue trackers
 type IssueTracker struct {
-	Origin string
+	Origin  string
+	AuthCfg *config.Auth
 }
 
 // TaskModel returns the model representing a deserialized github task
@@ -30,6 +33,19 @@ func (it *IssueTracker) Exists() bool {
 	return true
 }
 
+// InstrumentMiddleware is a hook to instrument any necessary middleware for connecting with the issue tracker
+func (it *IssueTracker) InstrumentMiddleware(r *http.Request) error {
+	if it.AuthCfg == nil || it.AuthCfg.Type == config.AuthTypeNone {
+		return nil
+	} else if it.AuthCfg.Type != config.AuthTypeAPIToken {
+		return fmt.Errorf("unsupported authentication token type for github: %s", it.AuthCfg.Type)
+	}
+
+	common.Assert(it.AuthCfg.Token != "", "authentication token is empty")
+	r.Header.Add("Authorization", "Bearer "+it.AuthCfg.Token)
+	return nil
+}
+
 // taskURLFrom taskID returns the url for the target github task ID to fetch
 func (it *IssueTracker) taskURLFrom(taskID string) string {
 	if strings.HasPrefix(taskID, "#") {
diff --git a/issuetracker/github/github_test.go b/issuetracker/internal/github/github_test.go
similarity index 100%
rename from issuetracker/github/github_test.go
rename to issuetracker/internal/github/github_test.go
diff --git a/issuetracker/github/task.go b/issuetracker/internal/github/task.go
similarity index 100%
rename from issuetracker/github/task.go
rename to issuetracker/internal/github/task.go
diff --git a/issuetracker/gitlab/gitlab.go b/issuetracker/internal/gitlab/gitlab.go
similarity index 71%
rename from issuetracker/gitlab/gitlab.go
rename to issuetracker/internal/gitlab/gitlab.go
index 769c744..ead9cf1 100644
--- a/issuetracker/gitlab/gitlab.go
+++ b/issuetracker/internal/gitlab/gitlab.go
@@ -2,16 +2,19 @@ package gitlab
 
 import (
 	"fmt"
+	"net/http"
 	"net/url"
 	"strings"
 
 	"github.com/preslavmihaylov/todocheck/common"
+	"github.com/preslavmihaylov/todocheck/config"
 	"github.com/preslavmihaylov/todocheck/issuetracker"
 )
 
 // IssueTracker is an issue tracker implementation for integrating with public & private gitlab issue trackers
 type IssueTracker struct {
-	Origin string
+	Origin  string
+	AuthCfg *config.Auth
 }
 
 // TaskModel returns the model representing a deserialized gitlab task
@@ -30,6 +33,19 @@ func (it *IssueTracker) Exists() bool {
 	return true
 }
 
+// InstrumentMiddleware is a hook to instrument any necessary middleware for connecting with the issue tracker
+func (it *IssueTracker) InstrumentMiddleware(r *http.Request) error {
+	if it.AuthCfg == nil || it.AuthCfg.Type == config.AuthTypeNone {
+		return nil
+	} else if it.AuthCfg.Type != config.AuthTypeAPIToken {
+		return fmt.Errorf("unsupported authentication token type for gitlab: %s", it.AuthCfg.Type)
+	}
+
+	common.Assert(it.AuthCfg.Token != "", "authentication token is empty")
+	r.Header.Add("PRIVATE-TOKEN", it.AuthCfg.Token)
+	return nil
+}
+
 // TaskURLFrom taskID returns the url for the target gitlab task ID to fetch
 func (it *IssueTracker) taskURLFrom(taskID string) string {
 	if strings.HasPrefix(taskID, "#") {
diff --git a/issuetracker/gitlab/gitlab_test.go b/issuetracker/internal/gitlab/gitlab_test.go
similarity index 100%
rename from issuetracker/gitlab/gitlab_test.go
rename to issuetracker/internal/gitlab/gitlab_test.go
diff --git a/issuetracker/gitlab/task.go b/issuetracker/internal/gitlab/task.go
similarity index 100%
rename from issuetracker/gitlab/task.go
rename to issuetracker/internal/gitlab/task.go
diff --git a/issuetracker/jira/jira.go b/issuetracker/internal/jira/jira.go
similarity index 59%
rename from issuetracker/jira/jira.go
rename to issuetracker/internal/jira/jira.go
index 730291d..389381a 100644
--- a/issuetracker/jira/jira.go
+++ b/issuetracker/internal/jira/jira.go
@@ -2,13 +2,17 @@ package jira
 
 import (
 	"fmt"
+	"net/http"
 
+	"github.com/preslavmihaylov/todocheck/common"
+	"github.com/preslavmihaylov/todocheck/config"
 	"github.com/preslavmihaylov/todocheck/issuetracker"
 )
 
 // IssueTracker is an issue tracker implementation for integrating with private Jira servers
 type IssueTracker struct {
-	Origin string
+	Origin  string
+	AuthCfg *config.Auth
 }
 
 // TaskModel returns the model representing a deserialized Jira task
@@ -27,6 +31,19 @@ func (it *IssueTracker) Exists() bool {
 	return true
 }
 
+// InstrumentMiddleware is a hook to instrument any necessary middleware for connecting with the issue tracker
+func (it *IssueTracker) InstrumentMiddleware(r *http.Request) error {
+	if it.AuthCfg == nil || it.AuthCfg.Type == config.AuthTypeNone {
+		return nil
+	} else if it.AuthCfg.Type != config.AuthTypeOffline {
+		return fmt.Errorf("unsupported authentication token type for jira: %s", it.AuthCfg.Type)
+	}
+
+	common.Assert(it.AuthCfg.Token != "", "authentication token is empty")
+	r.Header.Add("Authorization", "Bearer "+it.AuthCfg.Token)
+	return nil
+}
+
 // TaskURLFrom taskID returns the url for the target Jira task ID to fetch
 func (it *IssueTracker) taskURLFrom(taskID string) string {
 	return taskID
diff --git a/issuetracker/jira/task.go b/issuetracker/internal/jira/task.go
similarity index 100%
rename from issuetracker/jira/task.go
rename to issuetracker/internal/jira/task.go
diff --git a/issuetracker/pivotaltracker/pivotaltracker.go b/issuetracker/internal/pivotaltracker/pivotaltracker.go
similarity index 69%
rename from issuetracker/pivotaltracker/pivotaltracker.go
rename to issuetracker/internal/pivotaltracker/pivotaltracker.go
index 1e19952..68493c8 100644
--- a/issuetracker/pivotaltracker/pivotaltracker.go
+++ b/issuetracker/internal/pivotaltracker/pivotaltracker.go
@@ -2,15 +2,18 @@ package pivotaltracker
 
 import (
 	"fmt"
+	"net/http"
 	"strings"
 
 	"github.com/preslavmihaylov/todocheck/common"
+	"github.com/preslavmihaylov/todocheck/config"
 	"github.com/preslavmihaylov/todocheck/issuetracker"
 )
 
 // IssueTracker implementation for integrating with public pivotaltracker issue trackers
 type IssueTracker struct {
-	Origin string
+	Origin  string
+	AuthCfg *config.Auth
 }
 
 // TaskModel returns the model representing a deserialized pivotaltracker task
@@ -29,6 +32,19 @@ func (it *IssueTracker) Exists() bool {
 	return true
 }
 
+// InstrumentMiddleware is a hook to instrument any necessary middleware for connecting with the issue tracker
+func (it *IssueTracker) InstrumentMiddleware(r *http.Request) error {
+	if it.AuthCfg == nil || it.AuthCfg.Type == config.AuthTypeNone {
+		return nil
+	} else if it.AuthCfg.Type != config.AuthTypeAPIToken {
+		return fmt.Errorf("unsupported authentication token type for pivotaltracker: %s", it.AuthCfg.Type)
+	}
+
+	common.Assert(it.AuthCfg.Token != "", "authentication token is empty")
+	r.Header.Add("X-TrackerToken", it.AuthCfg.Token)
+	return nil
+}
+
 // TaskURLFrom taskID returns the url for the target pivotaltracker task ID to fetch
 func (it *IssueTracker) taskURLFrom(taskID string) string {
 	if strings.HasPrefix(taskID, "#") {
diff --git a/issuetracker/pivotaltracker/pivotaltracker_test.go b/issuetracker/internal/pivotaltracker/pivotaltracker_test.go
similarity index 100%
rename from issuetracker/pivotaltracker/pivotaltracker_test.go
rename to issuetracker/internal/pivotaltracker/pivotaltracker_test.go
diff --git a/issuetracker/pivotaltracker/task.go b/issuetracker/internal/pivotaltracker/task.go
similarity index 100%
rename from issuetracker/pivotaltracker/task.go
rename to issuetracker/internal/pivotaltracker/task.go
diff --git a/issuetracker/redmine/redmine.go b/issuetracker/internal/redmine/redmine.go
similarity index 62%
rename from issuetracker/redmine/redmine.go
rename to issuetracker/internal/redmine/redmine.go
index 64b3d68..36cf256 100644
--- a/issuetracker/redmine/redmine.go
+++ b/issuetracker/internal/redmine/redmine.go
@@ -2,14 +2,18 @@ package redmine
 
 import (
 	"fmt"
+	"net/http"
 	"strings"
 
+	"github.com/preslavmihaylov/todocheck/common"
+	"github.com/preslavmihaylov/todocheck/config"
 	"github.com/preslavmihaylov/todocheck/issuetracker"
 )
 
 // IssueTracker implementation for integrating with public & private redmine issue trackers
 type IssueTracker struct {
-	Origin string
+	Origin  string
+	AuthCfg *config.Auth
 }
 
 // TaskModel returns the model representing a deserialized redmine task
@@ -28,6 +32,19 @@ func (it *IssueTracker) Exists() bool {
 	return true
 }
 
+// InstrumentMiddleware is a hook to instrument any necessary middleware for connecting with the issue tracker
+func (it *IssueTracker) InstrumentMiddleware(r *http.Request) error {
+	if it.AuthCfg.Type == config.AuthTypeNone {
+		return nil
+	} else if it.AuthCfg.Type != config.AuthTypeAPIToken {
+		return fmt.Errorf("unsupported authentication token type for redmine: %s", it.AuthCfg.Type)
+	}
+
+	common.Assert(it.AuthCfg.Token != "", "authentication token is empty")
+	r.Header.Add("X-Redmine-API-Key", it.AuthCfg.Token)
+	return nil
+}
+
 // TaskURLFrom taskID returns the url for the target redmine task ID to fetch
 func (it *IssueTracker) taskURLFrom(taskID string) string {
 	if strings.HasPrefix(taskID, "#") {
diff --git a/issuetracker/redmine/task.go b/issuetracker/internal/redmine/task.go
similarity index 100%
rename from issuetracker/redmine/task.go
rename to issuetracker/internal/redmine/task.go
diff --git a/issuetracker/issuetracker.go b/issuetracker/issuetracker.go
index a04038d..9e33a87 100644
--- a/issuetracker/issuetracker.go
+++ b/issuetracker/issuetracker.go
@@ -2,6 +2,7 @@ package issuetracker
 
 import (
 	"errors"
+	"net/http"
 
 	"github.com/preslavmihaylov/todocheck/issuetracker/taskstatus"
 )
@@ -25,4 +26,7 @@ type IssueTracker interface {
 
 	// Exists verifies if the issue tracker exists based on the provided configuration
 	Exists() bool
+
+	// InstrumentMiddleware is a hook to instrument any necessary middleware for connecting with the issue tracker
+	InstrumentMiddleware(r *http.Request) error
 }
diff --git a/main.go b/main.go
index 38b2b9f..a882a96 100644
--- a/main.go
+++ b/main.go
@@ -9,7 +9,6 @@ import (
 
 	"github.com/fatih/color"
 	"github.com/preslavmihaylov/todocheck/authmanager"
-	"github.com/preslavmihaylov/todocheck/authmanager/authmiddleware"
 	todocheckerrors "github.com/preslavmihaylov/todocheck/checker/errors"
 	"github.com/preslavmihaylov/todocheck/config"
 	"github.com/preslavmihaylov/todocheck/fetcher"
@@ -51,7 +50,7 @@ func main() {
 		log.Fatalf("couldn't acquire token from config: %s\n", err)
 	}
 
-	tracker, err := factory.NewIssueTrackerFrom(localCfg.IssueTracker, localCfg.Origin)
+	tracker, err := factory.NewIssueTrackerFrom(localCfg.IssueTracker, localCfg.Auth, localCfg.Origin)
 	if err != nil {
 		log.Fatalf("couldn't create new issue tracker: %s\n", err)
 	}
@@ -64,7 +63,7 @@ func main() {
 		os.Exit(1)
 	}
 
-	f := fetcher.NewFetcher(tracker, authmiddleware.For(localCfg))
+	f := fetcher.NewFetcher(tracker)
 
 	todoErrs := []*todocheckerrors.TODO{}
 	traverser := todoerrs.NewTraverser(f, localCfg.IgnoredPaths, localCfg.CustomTodos, func(todoErr *todocheckerrors.TODO) error {


Test patch:
diff --git a/testing/scenariobuilder/issuetracker/issuetracker.go b/testing/scenariobuilder/issuetracker/issuetracker.go
index 7de814c..a2d79da 100644
--- a/testing/scenariobuilder/issuetracker/issuetracker.go
+++ b/testing/scenariobuilder/issuetracker/issuetracker.go
@@ -3,7 +3,7 @@ package issuetracker
 import (
 	"encoding/json"
 
-	"github.com/preslavmihaylov/todocheck/issuetracker/jira"
+	"github.com/preslavmihaylov/todocheck/testing/scenariobuilder/issuetracker/jira"
 )
 
 // Type is an enum specifying the target mock issue tracker to use
diff --git a/testing/scenariobuilder/issuetracker/jira/task.go b/testing/scenariobuilder/issuetracker/jira/task.go
new file mode 100644
index 0000000..ac825eb
--- /dev/null
+++ b/testing/scenariobuilder/issuetracker/jira/task.go
@@ -0,0 +1,30 @@
+package jira
+
+import "github.com/preslavmihaylov/todocheck/issuetracker/taskstatus"
+
+// Status for Jira tasks
+type Status struct {
+	Name string `json:"name"`
+}
+
+// Fields for Jira tasks
+type Fields struct {
+	Status `json:"status"`
+}
+
+// Task JSON model as returned by the Jira Rest API
+type Task struct {
+	Fields `json:"fields"`
+}
+
+// GetStatus of jira task, based on underlying structure
+func (t *Task) GetStatus() taskstatus.TaskStatus {
+	switch t.Fields.Status.Name {
+	case "Done":
+		fallthrough
+	case "Closed":
+		return taskstatus.Closed
+	default:
+		return taskstatus.Open
+	}
+}
diff --git a/validation/validation_test.go b/validation/validation_test.go
index ba24402..b3e358b 100644
--- a/validation/validation_test.go
+++ b/validation/validation_test.go
@@ -1,6 +1,7 @@
 package validation
 
 import (
+	"net/http"
 	"testing"
 
 	"github.com/preslavmihaylov/todocheck/config"
@@ -24,6 +25,11 @@ func (m *mockIssueTracker) Exists() bool {
 	return true
 }
 
+// InstrumentMiddleware is a hook to instrument any necessary middleware for connecting with the issue tracker
+func (m *mockIssueTracker) InstrumentMiddleware(r *http.Request) error {
+	panic("not implemented") // TODO: Implement
+}
+
 func TestInvalidOrigins(t *testing.T) {
 	invalidConfigPaths := []string{
 		"./fixtures/invalid/invalid_github_https.yaml",