Skip to content

WIP: Add golanci/golint to github actions #18

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,12 @@ jobs:
go vet ./...
go test -v ./...
go build ./...
lint:
name: lint
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: golangci-lint
uses: golangci/golangci-lint-action@v2
with:
version: v1.29
14 changes: 9 additions & 5 deletions ecdsa.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ var (
ErrECDSAVerification = errors.New("crypto/ecdsa: verification error")
)

// Implements the ECDSA family of signing methods signing methods
// SigningMethodECDSA Implements the ECDSA family of signing methods signing methods
// Expects *ecdsa.PrivateKey for signing and *ecdsa.PublicKey for verification
type SigningMethodECDSA struct {
Name string
Expand Down Expand Up @@ -53,7 +53,7 @@ func (m *SigningMethodECDSA) Alg() string {
return m.Name
}

// Implements the Verify method from SigningMethod
// Verify Implements the Verify method from SigningMethod
// For this verify method, key must be an ecdsa.PublicKey struct
func (m *SigningMethodECDSA) Verify(signingString, signature string, key interface{}) error {
var err error
Expand Down Expand Up @@ -85,7 +85,9 @@ func (m *SigningMethodECDSA) Verify(signingString, signature string, key interfa
return ErrHashUnavailable
}
hasher := m.Hash.New()
hasher.Write([]byte(signingString))
if _, err = hasher.Write([]byte(signingString)); err != nil {
return err
}

// Verify the signature
if verifystatus := ecdsa.Verify(ecdsaKey, hasher.Sum(nil), r, s); verifystatus {
Expand All @@ -95,7 +97,7 @@ func (m *SigningMethodECDSA) Verify(signingString, signature string, key interfa
return ErrECDSAVerification
}

// Implements the Sign method from SigningMethod
// Sign Implements the Sign method from SigningMethod
// For this signing method, key must be an ecdsa.PrivateKey struct
func (m *SigningMethodECDSA) Sign(signingString string, key interface{}) (string, error) {
// Get the key
Expand All @@ -113,7 +115,9 @@ func (m *SigningMethodECDSA) Sign(signingString string, key interface{}) (string
}

hasher := m.Hash.New()
hasher.Write([]byte(signingString))
if _, err := hasher.Write([]byte(signingString)); err != nil {
return "", err
}

// Sign the string and return r, s
if r, s, err := ecdsa.Sign(rand.Reader, ecdsaKey, hasher.Sum(nil)); err == nil {
Expand Down
10 changes: 7 additions & 3 deletions hmac.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,9 @@ func (m *SigningMethodHMAC) Verify(signingString, signature string, key interfac
// by reproducing the signature from the signing string and key, then
// comparing that against the provided signature.
hasher := hmac.New(m.Hash.New, keyBytes)
hasher.Write([]byte(signingString))
if _, err = hasher.Write([]byte(signingString)); err != nil {
return err
}
if !hmac.Equal(sig, hasher.Sum(nil)) {
return ErrSignatureInvalid
}
Expand All @@ -77,7 +79,7 @@ func (m *SigningMethodHMAC) Verify(signingString, signature string, key interfac
return nil
}

// Implements the Sign method from SigningMethod for this signing method.
// Sign Implements the Sign method from SigningMethod for this signing method.
// Key must be []byte
func (m *SigningMethodHMAC) Sign(signingString string, key interface{}) (string, error) {
if keyBytes, ok := key.([]byte); ok {
Expand All @@ -86,7 +88,9 @@ func (m *SigningMethodHMAC) Sign(signingString string, key interface{}) (string,
}

hasher := hmac.New(m.Hash.New, keyBytes)
hasher.Write([]byte(signingString))
if _, err := hasher.Write([]byte(signingString)); err != nil {
return "", err
}

return EncodeSegment(hasher.Sum(nil)), nil
}
Expand Down
25 changes: 16 additions & 9 deletions http_example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ var (
serverPort int
// storing sample username/password pairs
// don't do this on a real server
users = map[string]string{
"test": "known",
}
//users = map[string]string{
// "test": "known",
//}
)

// read the key files before starting http handlers
Expand Down Expand Up @@ -65,7 +65,7 @@ func init() {
}()
}

var start func()
//var start func()

func fatal(err error) {
if err != nil {
Expand Down Expand Up @@ -101,8 +101,12 @@ func Example_getTokenViaHTTP() {

// Read the token out of the response body
buf := new(bytes.Buffer)
io.Copy(buf, res.Body)
res.Body.Close()
if _, err = io.Copy(buf, res.Body); err != nil {
fatal(err)
}
if err = res.Body.Close(); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not very useful to panic in this situation.
An error when closing a response.Body can be ignored - the client has gone anyway.

This is an example where using golangci-lint does not improve the code,
but makes it unnecessarily convoluted.

The Go stdlib wil notpass golangci-lint.

fatal(err)
}
tokenString := strings.TrimSpace(buf.String())

// Parse the token
Expand Down Expand Up @@ -136,8 +140,12 @@ func Example_useTokenViaHTTP() {

// Read the response body
buf := new(bytes.Buffer)
io.Copy(buf, res.Body)
res.Body.Close()
if _, err = io.Copy(buf, res.Body); err != nil {
panic(err)
}
if err = res.Body.Close(); err != nil {
panic(err)
}
fmt.Println(buf.String())

// Output: Welcome, foo
Expand Down Expand Up @@ -214,5 +222,4 @@ func restrictedHandler(w http.ResponseWriter, r *http.Request) {

// Token is valid
fmt.Fprintln(w, "Welcome,", token.Claims.(*CustomClaimsExample).Name)
return
}
57 changes: 27 additions & 30 deletions map_claims_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,55 +7,52 @@ import (
func TestVerifyAud(t *testing.T) {
var nilInterface interface{}
var nilListInterface []interface{}
var intListInterface interface{} = []int{1,2,3}
type test struct{
Name string
MapClaims MapClaims
Expected bool
var intListInterface interface{} = []int{1, 2, 3}
type test struct {
Name string
MapClaims MapClaims
Expected bool
Comparison string
Required bool
Required bool
}
tests := []test{
// Matching Claim in aud
// Required = true
{ Name: "String Aud matching required", MapClaims: MapClaims{"aud": "example.com"}, Expected: true , Required: true, Comparison: "example.com"},
{ Name: "[]String Aud with match required", MapClaims: MapClaims{"aud": []string{"example.com", "example.example.com"}}, Expected: true, Required: true, Comparison: "example.com"},
{Name: "String Aud matching required", MapClaims: MapClaims{"aud": "example.com"}, Expected: true, Required: true, Comparison: "example.com"},
{Name: "[]String Aud with match required", MapClaims: MapClaims{"aud": []string{"example.com", "example.example.com"}}, Expected: true, Required: true, Comparison: "example.com"},

// Required = false
{ Name: "String Aud with match not required", MapClaims: MapClaims{"aud": "example.com"}, Expected: true , Required: false, Comparison: "example.com"},
{ Name: "Empty String Aud with match not required", MapClaims: MapClaims{}, Expected: true , Required: false, Comparison: "example.com"},
{ Name: "Empty String Aud with match not required", MapClaims: MapClaims{"aud": ""}, Expected: true , Required: false, Comparison: "example.com"},
{ Name: "Nil String Aud with match not required", MapClaims: MapClaims{"aud": nil}, Expected: true , Required: false, Comparison: "example.com"},
{Name: "String Aud with match not required", MapClaims: MapClaims{"aud": "example.com"}, Expected: true, Required: false, Comparison: "example.com"},
{Name: "Empty String Aud with match not required", MapClaims: MapClaims{}, Expected: true, Required: false, Comparison: "example.com"},
{Name: "Empty String Aud with match not required", MapClaims: MapClaims{"aud": ""}, Expected: true, Required: false, Comparison: "example.com"},
{Name: "Nil String Aud with match not required", MapClaims: MapClaims{"aud": nil}, Expected: true, Required: false, Comparison: "example.com"},

{ Name: "[]String Aud with match not required", MapClaims: MapClaims{"aud": []string{"example.com", "example.example.com"}}, Expected: true, Required: false, Comparison: "example.com"},
{ Name: "Empty []String Aud with match not required", MapClaims: MapClaims{"aud": []string{}}, Expected: true, Required: false, Comparison: "example.com"},
{Name: "[]String Aud with match not required", MapClaims: MapClaims{"aud": []string{"example.com", "example.example.com"}}, Expected: true, Required: false, Comparison: "example.com"},
{Name: "Empty []String Aud with match not required", MapClaims: MapClaims{"aud": []string{}}, Expected: true, Required: false, Comparison: "example.com"},

// Non-Matching Claim in aud
// Required = true
{ Name: "String Aud without match required", MapClaims: MapClaims{"aud": "not.example.com"}, Expected: false, Required: true, Comparison: "example.com"},
{ Name: "Empty String Aud without match required", MapClaims: MapClaims{"aud": ""}, Expected: false, Required: true, Comparison: "example.com"},
{ Name: "[]String Aud without match required", MapClaims: MapClaims{"aud": []string{"not.example.com", "example.example.com"}}, Expected: false, Required: true, Comparison: "example.com"},
{ Name: "Empty []String Aud without match required", MapClaims: MapClaims{"aud": []string{""}}, Expected: false, Required: true, Comparison: "example.com"},
{ Name: "String Aud without match not required", MapClaims: MapClaims{"aud": "not.example.com"}, Expected: false, Required: true, Comparison: "example.com"},
{ Name: "Empty String Aud without match not required", MapClaims: MapClaims{"aud": ""}, Expected: false, Required: true, Comparison: "example.com"},
{ Name: "[]String Aud without match not required", MapClaims: MapClaims{"aud": []string{"not.example.com", "example.example.com"}}, Expected: false, Required: true, Comparison: "example.com"},
{Name: "String Aud without match required", MapClaims: MapClaims{"aud": "not.example.com"}, Expected: false, Required: true, Comparison: "example.com"},
{Name: "Empty String Aud without match required", MapClaims: MapClaims{"aud": ""}, Expected: false, Required: true, Comparison: "example.com"},
{Name: "[]String Aud without match required", MapClaims: MapClaims{"aud": []string{"not.example.com", "example.example.com"}}, Expected: false, Required: true, Comparison: "example.com"},
{Name: "Empty []String Aud without match required", MapClaims: MapClaims{"aud": []string{""}}, Expected: false, Required: true, Comparison: "example.com"},
{Name: "String Aud without match not required", MapClaims: MapClaims{"aud": "not.example.com"}, Expected: false, Required: true, Comparison: "example.com"},
{Name: "Empty String Aud without match not required", MapClaims: MapClaims{"aud": ""}, Expected: false, Required: true, Comparison: "example.com"},
{Name: "[]String Aud without match not required", MapClaims: MapClaims{"aud": []string{"not.example.com", "example.example.com"}}, Expected: false, Required: true, Comparison: "example.com"},

// Required = false
{ Name: "Empty []String Aud without match required", MapClaims: MapClaims{"aud": []string{""}}, Expected: false, Required: true, Comparison: "example.com"},
{Name: "Empty []String Aud without match required", MapClaims: MapClaims{"aud": []string{""}}, Expected: false, Required: true, Comparison: "example.com"},

// []interface{}
{ Name: "Empty []interface{} Aud without match required", MapClaims: MapClaims{"aud": nilListInterface}, Expected: true, Required: false, Comparison: "example.com"},
{ Name: "[]interface{} Aud wit match required", MapClaims: MapClaims{"aud": []interface{}{"a", "foo", "example.com"}}, Expected: true, Required: true, Comparison: "example.com"},
{ Name: "[]interface{} Aud wit match but invalid types", MapClaims: MapClaims{"aud": []interface{}{"a", 5, "example.com"}}, Expected: false, Required: true, Comparison: "example.com"},
{ Name: "[]interface{} Aud int wit match required", MapClaims: MapClaims{"aud": intListInterface}, Expected: false, Required: true, Comparison: "example.com"},

{Name: "Empty []interface{} Aud without match required", MapClaims: MapClaims{"aud": nilListInterface}, Expected: true, Required: false, Comparison: "example.com"},
{Name: "[]interface{} Aud wit match required", MapClaims: MapClaims{"aud": []interface{}{"a", "foo", "example.com"}}, Expected: true, Required: true, Comparison: "example.com"},
{Name: "[]interface{} Aud wit match but invalid types", MapClaims: MapClaims{"aud": []interface{}{"a", 5, "example.com"}}, Expected: false, Required: true, Comparison: "example.com"},
{Name: "[]interface{} Aud int wit match required", MapClaims: MapClaims{"aud": intListInterface}, Expected: false, Required: true, Comparison: "example.com"},

// interface{}
{ Name: "Empty interface{} Aud without match not required", MapClaims: MapClaims{"aud": nilInterface}, Expected: true, Required: false, Comparison: "example.com"},

{Name: "Empty interface{} Aud without match not required", MapClaims: MapClaims{"aud": nilInterface}, Expected: true, Required: false, Comparison: "example.com"},
}


for _, test := range tests {
t.Run(test.Name, func(t *testing.T) {
got := test.MapClaims.VerifyAudience(test.Comparison, test.Required)
Expand Down
14 changes: 8 additions & 6 deletions request/extractor.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,19 @@ import (
"net/http"
)

// Errors
// ErrNoTokenInRequest Errors
var (
ErrNoTokenInRequest = errors.New("no token present in request")
)

// Interface for extracting a token from an HTTP request.
// Extractor Interface for extracting a token from an HTTP request.
// The ExtractToken method should return a token string or an error.
// If no token is present, you must return ErrNoTokenInRequest.
type Extractor interface {
ExtractToken(*http.Request) (string, error)
}

// Extractor for finding a token in a header. Looks at each specified
// HeaderExtractor for finding a token in a header. Looks at each specified
// header in order until there's a match
type HeaderExtractor []string

Expand All @@ -31,14 +31,16 @@ func (e HeaderExtractor) ExtractToken(req *http.Request) (string, error) {
return "", ErrNoTokenInRequest
}

// Extract token from request arguments. This includes a POSTed form or
// ArgumentExtractor Extract token from request arguments. This includes a POSTed form or
// GET URL arguments. Argument names are tried in order until there's a match.
// This extractor calls `ParseMultipartForm` on the request
type ArgumentExtractor []string

func (e ArgumentExtractor) ExtractToken(req *http.Request) (string, error) {
// Make sure form is parsed
req.ParseMultipartForm(10e6)
if err := req.ParseMultipartForm(10e6); err != nil {
return "", err
}

// loop over arg names and return the first one that contains data
for _, arg := range e {
Expand All @@ -50,7 +52,7 @@ func (e ArgumentExtractor) ExtractToken(req *http.Request) (string, error) {
return "", ErrNoTokenInRequest
}

// Tries Extractors in order until one returns a token string or an error occurs
// MultiExtractor Tries Extractors in order until one returns a token string or an error occurs
type MultiExtractor []Extractor

func (e MultiExtractor) ExtractToken(req *http.Request) (string, error) {
Expand Down
12 changes: 8 additions & 4 deletions rsa.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func (m *SigningMethodRSA) Alg() string {
return m.Name
}

// Implements the Verify method from SigningMethod
// Verify Implements the Verify method from SigningMethod
// For this signing method, must be an *rsa.PublicKey structure.
func (m *SigningMethodRSA) Verify(signingString, signature string, key interface{}) error {
var err error
Expand All @@ -67,13 +67,15 @@ func (m *SigningMethodRSA) Verify(signingString, signature string, key interface
return ErrHashUnavailable
}
hasher := m.Hash.New()
hasher.Write([]byte(signingString))
if _, err = hasher.Write([]byte(signingString)); err != nil {
return err
}

// Verify the signature
return rsa.VerifyPKCS1v15(rsaKey, m.Hash, hasher.Sum(nil), sig)
}

// Implements the Sign method from SigningMethod
// Sign Implements the Sign method from SigningMethod
// For this signing method, must be an *rsa.PrivateKey structure.
func (m *SigningMethodRSA) Sign(signingString string, key interface{}) (string, error) {
var rsaKey *rsa.PrivateKey
Expand All @@ -90,7 +92,9 @@ func (m *SigningMethodRSA) Sign(signingString string, key interface{}) (string,
}

hasher := m.Hash.New()
hasher.Write([]byte(signingString))
if _, err := hasher.Write([]byte(signingString)); err != nil {
return "", err
}

// Sign the string and return the encoded bytes
if sigBytes, err := rsa.SignPKCS1v15(rand.Reader, rsaKey, m.Hash, hasher.Sum(nil)); err == nil {
Expand Down
4 changes: 2 additions & 2 deletions rsa_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ var (
ErrNotRSAPublicKey = errors.New("Key is not a valid RSA public key")
)

// Parse PEM encoded PKCS1 or PKCS8 private key
// ParseRSAPrivateKeyFromPEM Parse PEM encoded PKCS1 or PKCS8 private key
func ParseRSAPrivateKeyFromPEM(key []byte) (*rsa.PrivateKey, error) {
var err error

Expand All @@ -39,7 +39,7 @@ func ParseRSAPrivateKeyFromPEM(key []byte) (*rsa.PrivateKey, error) {
return pkey, nil
}

// Parse PEM encoded PKCS1 or PKCS8 private key protected with password
// ParseRSAPrivateKeyFromPEMWithPassword Parse PEM encoded PKCS1 or PKCS8 private key protected with password
func ParseRSAPrivateKeyFromPEMWithPassword(key []byte, password string) (*rsa.PrivateKey, error) {
var err error

Expand Down