Skip to content

Commit

Permalink
chore: fix lint issues iteration #1
Browse files Browse the repository at this point in the history
  • Loading branch information
Sanad authored and shaj13 committed May 21, 2020
1 parent 3254f04 commit 3bbb61c
Show file tree
Hide file tree
Showing 26 changed files with 130 additions and 86 deletions.
1 change: 1 addition & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ jobs:
steps:
- checkout
- run: make install
- run: make lint
- run: make cover
- run: make deploy-cover

10 changes: 10 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,13 @@ linters-settings:
local-prefixes: "github.com/shaj13/go-guardian/"
issues:
exclude-use-default: false
exclude-rules:

- text: "G304: Potential file inclusion via variable"
linters:
- gosec

- path: _test\.go
linters:
- errcheck
- gosec
3 changes: 2 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ deploy-cover:

lint:
./bin/golangci-lint run -c .golangci.yml ./...

lint-fix:
@FILES="$(shell find . -type f -name '*.go' -not -path "./vendor/*")"; goimports -local "github.com/shaj13/go-guardian/" -w $$FILES
./bin/golangci-lint run -c .golangci.yml ./... --fix
./bin/golangci-lint run -c .golangci.yml ./... --fix
36 changes: 21 additions & 15 deletions auth/authenticator.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,26 +7,32 @@ import (
"sync"
)

// ErrNoMatch is returned by Authenticator when request not authenticated, and all registered Strategies returned errors.
var ErrNoMatch = errors.New("authenticator: No authentication strategy matched to request all Strategies returned errors")

// DisabledPath is a soft error similar to EOF. returned by Authenticator when a attempting to authenticate request have a disabled path.
// Authenticator return DisabledPath only to signal the caller.
// The caller should continue the request flow, and never return the error to the end users.
var DisabledPath = errors.New("authenticator: Disabled Path")

// NOOP is a soft error similar to EOF, returned by strategies that have NoOpAuthenticate function to indicate there no op,
// and signal authenticator to unauthenticate the request.
var NOOP = errors.New("NOOP")
var (
// ErrNoMatch is returned by Authenticator when request not authenticated,
// and all registered Strategies returned errors.
ErrNoMatch = errors.New("authenticator: No authentication strategy matched")

// ErrDisabledPath is a soft error similar to EOF.
// returned by Authenticator when a attempting to authenticate request have a disabled path.
// Authenticator return DisabledPath only to signal the caller.
// The caller should continue the request flow, and never return the error to the end users.
ErrDisabledPath = errors.New("authenticator: Disabled Path")

// ErrNOOP is a soft error similar to EOF,
// returned by strategies that have NoOpAuthenticate function to indicate there no op,
// and signal authenticator to unauthenticate the request.
ErrNOOP = errors.New("NOOP")
)

// Authenticator carry the registered authentication strategies, and represents the first API to authenticate received requests.
// Authenticator carry the registered authentication strategies,
// and represents the first API to authenticate received requests.
type Authenticator interface {
// Authenticate dispatch the request to the registered authentication strategies,
// and return user information from the first strategy that successfully authenticates the request.
// Otherwise, an aggregated error returned.
// if request attempt to visit a disabled path, error DisabledPath returned to signal the caller,
// if request attempt to visit a disabled path, ErrDisabledPath returned to signal the caller,
// Otherwise, start the authentication process.
// See DisabledPath documentation for more info.
// See ErrDisabledPath documentation for more info.
//
// NOTICE: Authenticate does not guarantee the order strategies run in.
Authenticate(r *http.Request) (Info, error)
Expand All @@ -49,7 +55,7 @@ type authenticator struct {
func (a *authenticator) Authenticate(r *http.Request) (Info, error) {
// check if request to a disabled path
if a.disabledPath(r.RequestURI) {
return nil, DisabledPath
return nil, ErrDisabledPath
}

var info Info
Expand Down
10 changes: 5 additions & 5 deletions auth/authenticator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ func TestAuthenticator(t *testing.T) {
name string
strategies []Strategy
paths []string
userId string
userID string
ExpetedErr bool
}{
{
Expand All @@ -39,7 +39,7 @@ func TestAuthenticator(t *testing.T) {
strategy{returnErr: true},
},
ExpetedErr: false,
userId: "1",
userID: "1",
},
{
name: "it return DisabledPath when path disabled",
Expand All @@ -61,8 +61,8 @@ func TestAuthenticator(t *testing.T) {
for _, p := range tt.paths {
r.RequestURI = p
_, err := authenticator.Authenticate(r)
if err != DisabledPath {
t.Errorf("Expected %v, Got %v, For Path %s", DisabledPath, err, p)
if err != ErrDisabledPath {
t.Errorf("Expected %v, Got %v, For Path %s", ErrDisabledPath, err, p)
continue
}
}
Expand All @@ -76,7 +76,7 @@ func TestAuthenticator(t *testing.T) {
return
}

assert.Equal(t, tt.userId, info.ID())
assert.Equal(t, tt.userID, info.ID())
})
}
}
Expand Down
2 changes: 1 addition & 1 deletion auth/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,4 @@ func ExampleRevoke() {
fmt.Println(err)
// Output:
// <nil>
}
}
2 changes: 1 addition & 1 deletion auth/strategies/basic/basic.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ var ErrMissingPrams = errors.New("basic: Request missing BasicAuth")
// commonly used when enable/add strategy to go-passport authenticator.
const StrategyKey = auth.StrategyKey("Basic.Strategy")

// Authenticate decalre custom function to authenticate request using user credentials.
// Authenticate declare custom function to authenticate request using user credentials.
// the authenticate function invoked by Authenticate Strategy method after extracting user credentials
// to compare against DB or ather service, if extracting user credentials from request failed a nil info
// with ErrMissingPrams returned, Otherwise, return Authenticate invocation result.
Expand Down
1 change: 1 addition & 0 deletions auth/strategies/basic/basic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/shaj13/go-guardian/auth"
)

//nolint:goconst
func Test(t *testing.T) {
auth := func(ctx context.Context, r *http.Request, userName, password string) (auth.Info, error) {
if userName == "test" && password == "test" {
Expand Down
5 changes: 3 additions & 2 deletions auth/strategies/basic/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"github.com/shaj13/go-guardian/auth"
)

func Example() {
strategy := Authenticate(exampleAuthFunc)
authenticator := auth.New()
Expand All @@ -20,11 +21,11 @@ func Example() {

req.SetBasicAuth("test", "1234")
_, err = authenticator.Authenticate(req)
fmt.Println(err)
fmt.Println(err.(auth.Error).Errors()[1])

// Output:
// 10 <nil>
// authenticator: No authentication strategy matched to request all Strategies returned errors: [Invalid credentials, ]
// Invalid credentials
}

func exampleAuthFunc(ctx context.Context, r *http.Request, userName, password string) (auth.Info, error) {
Expand Down
3 changes: 2 additions & 1 deletion auth/strategies/bearer/bearer.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ var (
// ErrInvalidToken indicate a hit of an invalid bearer token format.
// And it's returned by Token function.
ErrInvalidToken = errors.New("bearer: Invalid bearer token")
// ErrTokenNotFound is returned by authenticating functions for both cached and static bearer strategies when token not found in their store.
// ErrTokenNotFound is returned by authenticating functions for bearer strategies,
// when token not found in their store.
ErrTokenNotFound = errors.New("barer: Token does not exists")
)

Expand Down
3 changes: 2 additions & 1 deletion auth/strategies/bearer/bearer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ import (
"net/http"
"testing"

"github.com/shaj13/go-guardian/auth"
"github.com/stretchr/testify/assert"

"github.com/shaj13/go-guardian/auth"
)

func TestAuthenticateMethod(t *testing.T) {
Expand Down
6 changes: 3 additions & 3 deletions auth/strategies/bearer/cached.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
// commonly used when enable/add strategy to go-passport authenticator.
const CachedStrategyKey = auth.StrategyKey("Bearer.Cached.Strategy")

// Authenticate decalre custom function to authenticate request using token.
// Authenticate declare custom function to authenticate request using token.
// The authenticate function invoked by Authenticate Strategy method when
// The token does not exist in the cahce and the invocation result will be cached, unless an error returned.
// Use NoOpAuthenticate instead to refresh/mangae token directly using cache or Append function.
Expand Down Expand Up @@ -80,9 +80,9 @@ func (c *cachedToken) Revoke(token string, r *http.Request) error {
return c.cache.Delete(token, r)
}

// NoOpAuthenticate implements Authenticate function, it return nil, auth.NOOP error,
// NoOpAuthenticate implements Authenticate function, it return nil, auth.ErrNOOP,
// commonly used when token refreshed/mangaed directly using cache or Append function,
// and there is no need to parse token and authenticate request.
func NoOpAuthenticate(ctx context.Context, r *http.Request, token string) (auth.Info, error) {
return nil, auth.NOOP
return nil, auth.ErrNOOP
}
15 changes: 8 additions & 7 deletions auth/strategies/bearer/cached_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ import (
"net/http"
"testing"

"github.com/stretchr/testify/assert"

"github.com/shaj13/go-guardian/auth"
"github.com/shaj13/go-guardian/storage"
"github.com/stretchr/testify/assert"
)

func TestNewCahced(t *testing.T) {
Expand Down Expand Up @@ -42,7 +43,7 @@ func TestNewCahced(t *testing.T) {
name: "it return error when cache store return error",
expectedErr: true,
cache: make(mockCache),
authFunc: func(ctx context.Context, r *http.Request, token string) (auth.Info, error) { return nil, nil },
authFunc: func(_ context.Context, _ *http.Request, _ string) (auth.Info, error) { return nil, nil },
token: "store-error",
panic: false,
info: nil,
Expand All @@ -51,7 +52,7 @@ func TestNewCahced(t *testing.T) {
name: "it return error when cache return invalid type",
expectedErr: true,
cache: make(mockCache),
authFunc: func(ctx context.Context, r *http.Request, token string) (auth.Info, error) { return nil, nil },
authFunc: func(_ context.Context, _ *http.Request, _ string) (auth.Info, error) { return nil, nil },
panic: false,
info: "sample-data",
token: "valid",
Expand Down Expand Up @@ -89,11 +90,11 @@ func TestNewCahced(t *testing.T) {
return
}

strat := NewCachedToken(tt.authFunc, tt.cache)
strategy := NewCachedToken(tt.authFunc, tt.cache)
r, _ := http.NewRequest("GET", "/", nil)
r.Header.Set("Authorization", "Bearer "+tt.token)
_ = tt.cache.Store(tt.token, tt.info, r)
info, err := strat.Authenticate(r.Context(), r)
info, err := strategy.Authenticate(r.Context(), r)
if tt.expectedErr {
assert.Error(t, err)
return
Expand All @@ -105,9 +106,9 @@ func TestNewCahced(t *testing.T) {

func TestCahcedTokenAppend(t *testing.T) {
cache := make(mockCache)
strat := &cachedToken{cache: cache}
strategy := &cachedToken{cache: cache}
info := auth.NewDefaultUser("1", "2", nil, nil)
strat.Append("test-append", info, nil)
strategy.Append("test-append", info, nil)
cachedInfo, ok, _ := cache.Load("test-append", nil)
assert.True(t, ok)
assert.Equal(t, info, cachedInfo)
Expand Down
4 changes: 2 additions & 2 deletions auth/strategies/bearer/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func ExampleNoOpAuthenticate() {

// demonstrate a user attempt to login
r, _ := http.NewRequest("GET", "/login", nil)
// user verified and add the user token to startegy using append or cache
// user verified and add the user token to strategy using append or cache
cache.Store("token", auth.NewDefaultUser("example", "1", nil, nil), r)

// first request where authentication decision added to cached
Expand All @@ -90,7 +90,7 @@ func ExampleNoOpAuthenticate() {
fmt.Println(err, info.ID())

// second request where authentication decision expired and user must login again
time.Sleep(time.Microsecond * 600)
time.Sleep(time.Microsecond * 800)
info, err = strategy.Authenticate(r.Context(), r)
fmt.Println(err, info)
// Output:
Expand Down
12 changes: 9 additions & 3 deletions auth/strategies/bearer/static.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,19 @@ func (s *Static) authenticate(ctx context.Context, _ *http.Request, token string
}

// Authenticate user request against predefined tokens by verifying request token existence in the static Map.
// Once token found auth.Info returned with a nil error, Otherwise, a nil auth.Info and ErrTokenNotFound returned.
// Once token found auth.Info returned with a nil error,
// Otherwise, a nil auth.Info and ErrTokenNotFound returned.
func (s *Static) Authenticate(ctx context.Context, r *http.Request) (auth.Info, error) {
return authenticateFunc(s.authenticate).authenticate(ctx, r)
}

// Append add new token to static store.
func (s *Static) Append(token string, info auth.Info, _ *http.Request) error {
s.Store(token, info)
return nil
}

// Revoke delete token from static store.
func (s *Static) Revoke(token string, _ *http.Request) error {
s.Delete(token)
return nil
Expand Down Expand Up @@ -76,15 +79,18 @@ func NewStaticFromFile(path string) (auth.Strategy, error) {
}

if len(record) < 3 {
return nil, fmt.Errorf("static: record must have at least 3 columns (token, username, id), Record: %v", record)
return nil, fmt.Errorf(
"static: record must have at least 3 columns (token, username, id), Record: %v",
record,
)
}

if record[0] == "" {
return nil, fmt.Errorf("static: a non empty token is required, Record: %v", record)
}

// if token Contains Bearer remove it
record[0] = strings.TrimLeft(record[0], "Bearer ")
record[0] = strings.TrimPrefix(record[0], "Bearer ")

if _, ok := tokens[record[0]]; ok {
return nil, fmt.Errorf("static: token already exists, Record: %v", record)
Expand Down
Loading

0 comments on commit 3bbb61c

Please sign in to comment.