Skip to content
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

MF-1718 - Use static code analysis in CI #1729

Merged
merged 26 commits into from
Apr 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
3177d1a
things, twins, and logger lint fixed
AryanGodara Feb 21, 2023
24d8b6d
all services updated, auth jwt not working, ineffectual assignment issue
AryanGodara Feb 22, 2023
3e10361
handle error from grpc server in endpointtest
AryanGodara Feb 22, 2023
6fa690a
temp commit, auth/jwt needs to be resolved
AryanGodara Mar 1, 2023
34a21ca
revert back to jwt v4 temporarily
AryanGodara Mar 3, 2023
bfc1dcc
updated jwt tokenizer
AryanGodara Mar 3, 2023
21f797d
resolve EOF error for httptest requests
AryanGodara Mar 3, 2023
f089e41
fix auth jwt, update to registeredclaims
AryanGodara Mar 4, 2023
6a48d7a
fix ineffective assignment, auth/api/grpc endpoint failing
AryanGodara Mar 4, 2023
b9a8ffe
temp commit, remove later
AryanGodara Mar 29, 2023
ea41d83
fix grpc server setup
AryanGodara Apr 1, 2023
1709ad5
resolve golangci tests, remove debug statements
AryanGodara Apr 3, 2023
a12b21c
update golangci version and modify linters used
AryanGodara Apr 3, 2023
1cd3baa
fix failing tests
AryanGodara Apr 4, 2023
9b15e30
fix grpc server for setup tests
AryanGodara Apr 5, 2023
c79f217
fix logging and errors inlined
AryanGodara Apr 5, 2023
84b982b
fix remarks, update grpc setup_test
AryanGodara Apr 7, 2023
b40e835
fix setup_test
AryanGodara Apr 7, 2023
27fd8c5
update setup_test grpc
AryanGodara Apr 9, 2023
90b7454
fix data race
AryanGodara Apr 9, 2023
bf0fb24
update setup_test grpc
AryanGodara Apr 12, 2023
5ac6558
fix grpc setup down to single simple function
AryanGodara Apr 12, 2023
e903ec3
fix linting issues
AryanGodara Apr 12, 2023
ad060b6
resolve pr comments
AryanGodara Apr 14, 2023
51bd182
fix tests, handle returned errors, go mod tidy vendor
AryanGodara Apr 18, 2023
d121bd0
fix errors from new linters
AryanGodara Apr 20, 2023
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
48 changes: 18 additions & 30 deletions auth/api/grpc/endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,12 @@ package grpc_test
import (
"context"
"fmt"
"net"
"testing"
"time"

"github.com/mainflux/mainflux"
"github.com/mainflux/mainflux/auth"
grpcapi "github.com/mainflux/mainflux/auth/api/grpc"
"github.com/mainflux/mainflux/auth/jwt"
"github.com/mainflux/mainflux/auth/mocks"
"github.com/mainflux/mainflux/pkg/uuid"
"github.com/opentracing/opentracing-go/mocktracer"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -44,30 +41,11 @@ const (

var svc auth.Service

func newService() auth.Service {
repo := mocks.NewKeyRepository()
groupRepo := mocks.NewGroupRepository()
idProvider := uuid.NewMock()

mockAuthzDB := map[string][]mocks.MockSubjectSet{}
mockAuthzDB[id] = append(mockAuthzDB[id], mocks.MockSubjectSet{Object: authoritiesObj, Relation: memberRelation})
ketoMock := mocks.NewKetoMock(mockAuthzDB)

t := jwt.New(secret)

return auth.New(repo, groupRepo, idProvider, t, ketoMock, loginDuration)
}

func startGRPCServer(svc auth.Service, port int) {
listener, _ := net.Listen("tcp", fmt.Sprintf(":%d", port))
server := grpc.NewServer()
mainflux.RegisterAuthServiceServer(server, grpcapi.NewServer(mocktracer.New(), svc))
go server.Serve(listener)
}

func TestIssue(t *testing.T) {
authAddr := fmt.Sprintf("localhost:%d", port)
conn, _ := grpc.Dial(authAddr, grpc.WithTransportCredentials(insecure.NewCredentials()))
conn, err := grpc.Dial(authAddr, grpc.WithTransportCredentials(insecure.NewCredentials()))
require.Nil(t, err, fmt.Sprintf("got unexpected error while creating client connection: %s", err))

client := grpcapi.NewClient(mocktracer.New(), conn, time.Second)

cases := []struct {
Expand Down Expand Up @@ -139,7 +117,9 @@ func TestIdentify(t *testing.T) {
require.Nil(t, err, fmt.Sprintf("Issuing API key expected to succeed: %s", err))

authAddr := fmt.Sprintf("localhost:%d", port)
conn, _ := grpc.Dial(authAddr, grpc.WithTransportCredentials(insecure.NewCredentials()))
conn, err := grpc.Dial(authAddr, grpc.WithTransportCredentials(insecure.NewCredentials()))
require.Nil(t, err, fmt.Sprintf("got unexpected error while creating client connection: %s", err))

client := grpcapi.NewClient(mocktracer.New(), conn, time.Second)

cases := []struct {
Expand Down Expand Up @@ -202,7 +182,9 @@ func TestAuthorize(t *testing.T) {
require.Nil(t, err, fmt.Sprintf("Issuing user key expected to succeed: %s", err))

authAddr := fmt.Sprintf("localhost:%d", port)
conn, _ := grpc.Dial(authAddr, grpc.WithTransportCredentials(insecure.NewCredentials()))
conn, err := grpc.Dial(authAddr, grpc.WithTransportCredentials(insecure.NewCredentials()))
require.Nil(t, err, fmt.Sprintf("got unexpected error while creating client connection: %s", err))

client := grpcapi.NewClient(mocktracer.New(), conn, time.Second)

cases := []struct {
Expand Down Expand Up @@ -283,7 +265,9 @@ func TestAddPolicy(t *testing.T) {
require.Nil(t, err, fmt.Sprintf("Issuing user key expected to succeed: %s", err))

authAddr := fmt.Sprintf("localhost:%d", port)
conn, _ := grpc.Dial(authAddr, grpc.WithTransportCredentials(insecure.NewCredentials()))
conn, err := grpc.Dial(authAddr, grpc.WithTransportCredentials(insecure.NewCredentials()))
require.Nil(t, err, fmt.Sprintf("got unexpected error while creating client connection: %s", err))

client := grpcapi.NewClient(mocktracer.New(), conn, time.Second)

groupAdminObj := "groupadmin"
Expand Down Expand Up @@ -336,7 +320,9 @@ func TestDeletePolicy(t *testing.T) {
require.Nil(t, err, fmt.Sprintf("Issuing user key expected to succeed: %s", err))

authAddr := fmt.Sprintf("localhost:%d", port)
conn, _ := grpc.Dial(authAddr, grpc.WithTransportCredentials(insecure.NewCredentials()))
conn, err := grpc.Dial(authAddr, grpc.WithTransportCredentials(insecure.NewCredentials()))
require.Nil(t, err, fmt.Sprintf("got unexpected error while creating client connection: %s", err))

client := grpcapi.NewClient(mocktracer.New(), conn, time.Second)

readRelation := "read"
Expand Down Expand Up @@ -452,7 +438,9 @@ func TestMembers(t *testing.T) {
}

authAddr := fmt.Sprintf("localhost:%d", port)
conn, _ := grpc.Dial(authAddr, grpc.WithTransportCredentials(insecure.NewCredentials()))
conn, err := grpc.Dial(authAddr, grpc.WithTransportCredentials(insecure.NewCredentials()))
require.Nil(t, err, fmt.Sprintf("got unexpected error while creating client connection: %s", err))

client := grpcapi.NewClient(mocktracer.New(), conn, time.Second)

for _, tc := range cases {
Expand Down
47 changes: 46 additions & 1 deletion auth/api/grpc/setup_test.go
AryanGodara marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,60 @@
package grpc_test

import (
"fmt"
"log"
"net"
"os"
"testing"

"github.com/mainflux/mainflux"
"github.com/mainflux/mainflux/auth"
grpcapi "github.com/mainflux/mainflux/auth/api/grpc"
"github.com/mainflux/mainflux/auth/jwt"
"github.com/mainflux/mainflux/auth/mocks"
"github.com/mainflux/mainflux/pkg/uuid"
"github.com/opentracing/opentracing-go/mocktracer"
"google.golang.org/grpc"
)

func TestMain(m *testing.M) {
serverErr := make(chan error)

listener, err := net.Listen("tcp", fmt.Sprintf(":%d", port))
if err != nil {
log.Fatalf("got unexpected error while creating new listerner: %s", err)
}

svc = newService()
startGRPCServer(svc, port)
server := grpc.NewServer()
mainflux.RegisterAuthServiceServer(server, grpcapi.NewServer(mocktracer.New(), svc))

// Start gRPC server in detached mode.
go func() {
serverErr <- server.Serve(listener)
}()

code := m.Run()

server.GracefulStop()
err = <-serverErr
if err != nil {
log.Fatalln("gPRC Server Terminated : ", err)
}
close(serverErr)
os.Exit(code)
}

func newService() auth.Service {
repo := mocks.NewKeyRepository()
groupRepo := mocks.NewGroupRepository()
idProvider := uuid.NewMock()

mockAuthzDB := map[string][]mocks.MockSubjectSet{}
mockAuthzDB[id] = append(mockAuthzDB[id], mocks.MockSubjectSet{Object: authoritiesObj, Relation: memberRelation})
ketoMock := mocks.NewKetoMock(mockAuthzDB)

tokenizer := jwt.New(secret)

return auth.New(repo, groupRepo, idProvider, tokenizer, ketoMock, loginDuration)
}
1 change: 1 addition & 0 deletions auth/api/http/groups/endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ type testRequest struct {

func (tr testRequest) make() (*http.Response, error) {
req, err := http.NewRequest(tr.method, tr.url, tr.body)
req.Close = true
if err != nil {
return nil, err
}
Expand Down
25 changes: 14 additions & 11 deletions auth/jwt/tokenizer.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
const issuerName = "mainflux.auth"

type claims struct {
jwt.StandardClaims
jwt.RegisteredClaims
IssuerID string `json:"issuer_id,omitempty"`
Type *uint32 `json:"type,omitempty"`
}
Expand All @@ -24,7 +24,7 @@ func (c claims) Valid() error {
return errors.ErrMalformedEntity
}

return c.StandardClaims.Valid()
return c.RegisteredClaims.Valid()
}

type tokenizer struct {
Expand All @@ -38,20 +38,20 @@ func New(secret string) auth.Tokenizer {

func (svc tokenizer) Issue(key auth.Key) (string, error) {
claims := claims{
StandardClaims: jwt.StandardClaims{
RegisteredClaims: jwt.RegisteredClaims{
Issuer: issuerName,
Subject: key.Subject,
IssuedAt: key.IssuedAt.UTC().Unix(),
IssuedAt: &jwt.NumericDate{Time: key.IssuedAt.UTC()},
},
IssuerID: key.IssuerID,
Type: &key.Type,
}

if !key.ExpiresAt.IsZero() {
claims.ExpiresAt = key.ExpiresAt.UTC().Unix()
claims.ExpiresAt = &jwt.NumericDate{Time: key.ExpiresAt.UTC()}
}
if key.ID != "" {
claims.Id = key.ID
claims.ID = key.ID
}

token := jwt.NewWithClaims(jwt.SigningMethodHS256, claims)
Expand All @@ -70,6 +70,7 @@ func (svc tokenizer) Parse(token string) (auth.Key, error) {
if err != nil {
if e, ok := err.(*jwt.ValidationError); ok && e.Errors == jwt.ValidationErrorExpired {
// Expired User key needs to be revoked.

if c.Type != nil && *c.Type == auth.APIKey {
return c.toKey(), auth.ErrAPIKeyExpired
}
Expand All @@ -83,18 +84,20 @@ func (svc tokenizer) Parse(token string) (auth.Key, error) {

func (c claims) toKey() auth.Key {
key := auth.Key{
ID: c.Id,
ID: c.ID,
IssuerID: c.IssuerID,
Subject: c.Subject,
IssuedAt: time.Unix(c.IssuedAt, 0).UTC(),
IssuedAt: c.IssuedAt.Time.UTC(),
}
if c.ExpiresAt != 0 {
key.ExpiresAt = time.Unix(c.ExpiresAt, 0).UTC()

key.ExpiresAt = time.Time{}
if c.ExpiresAt != nil && c.ExpiresAt.Time.UTC().Unix() != 0 {
key.ExpiresAt = c.ExpiresAt.Time.UTC()
}

// Default type is 0.
if c.Type != nil {
key.Type = *c.Type
key.Type = *(c.Type)
}

return key
Expand Down
2 changes: 1 addition & 1 deletion auth/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ var (
)

const (
// LoginKey is temporary User key received on successfull login.
// LoginKey is temporary User key received on successful login.
LoginKey uint32 = iota
// RecoveryKey represents a key for resseting password.
RecoveryKey
Expand Down
12 changes: 10 additions & 2 deletions auth/postgres/groups.go
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,11 @@ func (gr groupRepository) Assign(ctx context.Context, groupID, groupType string,
dbg.UpdatedAt = created

if _, err := tx.NamedExecContext(ctx, qIns, dbg); err != nil {
tx.Rollback()
if rollbackErr := tx.Rollback(); rollbackErr != nil {
err = errors.Wrap(err, rollbackErr)
return errors.Wrap(auth.ErrAssignToGroup, err)
}

pgErr, ok := err.(*pgconn.PgError)
if ok {
switch pgErr.Code {
Expand Down Expand Up @@ -479,7 +483,11 @@ func (gr groupRepository) Unassign(ctx context.Context, groupID string, ids ...s
}

if _, err := tx.NamedExecContext(ctx, qDel, dbg); err != nil {
tx.Rollback()
if rollbackErr := tx.Rollback(); rollbackErr != nil {
err = errors.Wrap(rollbackErr, err)
return errors.Wrap(auth.ErrAssignToGroup, err)
}

pgErr, ok := err.(*pgconn.PgError)
if ok {
switch pgErr.Code {
Expand Down
2 changes: 1 addition & 1 deletion auth/postgres/groups_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ func TestGroupRetrieveByID(t *testing.T) {
require.Nil(t, err, fmt.Sprintf("got unexpected error: %s", err))
assert.True(t, retrieved.ID == group1.ID, fmt.Sprintf("Save group, ID: expected %s got %s\n", group1.ID, retrieved.ID))

// Round to milliseconds as otherwise saving and retriving from DB
// Round to milliseconds as otherwise saving and retrieving from DB
// adds rounding error.
creationTime := time.Now().UTC().Round(time.Millisecond)
group2 := auth.Group{
Expand Down
4 changes: 2 additions & 2 deletions auth/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ func (svc service) AddPolicies(ctx context.Context, token, object string, subjec
for _, subjectID := range subjectIDs {
for _, relation := range relations {
if err := svc.AddPolicy(ctx, PolicyReq{Object: object, Relation: relation, Subject: subjectID}); err != nil {
errs = errors.Wrap(fmt.Errorf("cannot add '%s' policy on object '%s' for subject '%s': %s", relation, object, subjectID, err), errs)
errs = errors.Wrap(fmt.Errorf("cannot add '%s' policy on object '%s' for subject '%s': %w", relation, object, subjectID, err), errs)
}
}
}
Expand All @@ -209,7 +209,7 @@ func (svc service) DeletePolicies(ctx context.Context, token, object string, sub
for _, subjectID := range subjectIDs {
for _, relation := range relations {
if err := svc.DeletePolicy(ctx, PolicyReq{Object: object, Relation: relation, Subject: subjectID}); err != nil {
errs = errors.Wrap(fmt.Errorf("cannot delete '%s' policy on object '%s' for subject '%s': %s", relation, object, subjectID, err), errs)
errs = errors.Wrap(fmt.Errorf("cannot delete '%s' policy on object '%s' for subject '%s': %w", relation, object, subjectID, err), errs)
}
}
}
Expand Down
11 changes: 6 additions & 5 deletions auth/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ const (
id = "testID"
groupName = "mfx"
description = "Description"
read = "read"

memberRelation = "member"
authoritiesObj = "authorities"
Expand Down Expand Up @@ -939,8 +940,8 @@ func TestAssign(t *testing.T) {

// check access control policies things members.
subjectSet := fmt.Sprintf("%s:%s#%s", "members", group.ID, memberRelation)
err = svc.Authorize(context.Background(), auth.PolicyReq{Object: mid, Relation: "read", Subject: subjectSet})
assert.Nil(t, err, fmt.Sprintf("entites having an access to group %s must have %s policy on %s: %s", group.ID, "read", mid, err))
err = svc.Authorize(context.Background(), auth.PolicyReq{Object: mid, Relation: read, Subject: subjectSet})
assert.Nil(t, err, fmt.Sprintf("entites having an access to group %s must have %s policy on %s: %s", group.ID, read, mid, err))
err = svc.Authorize(context.Background(), auth.PolicyReq{Object: mid, Relation: "write", Subject: subjectSet})
assert.Nil(t, err, fmt.Sprintf("entites having an access to group %s must have %s policy on %s: %s", group.ID, "write", mid, err))
err = svc.Authorize(context.Background(), auth.PolicyReq{Object: mid, Relation: "delete", Subject: subjectSet})
Expand Down Expand Up @@ -1082,7 +1083,7 @@ func TestAddPolicies(t *testing.T) {
assert.Nil(t, err, fmt.Sprintf("got unexpected error: %s", err))

tmpID := "tmpid"
readPolicy := "read"
readPolicy := read
AryanGodara marked this conversation as resolved.
Show resolved Hide resolved
writePolicy := "write"
deletePolicy := "delete"

Expand Down Expand Up @@ -1167,7 +1168,7 @@ func TestDeletePolicies(t *testing.T) {
assert.Nil(t, err, fmt.Sprintf("got unexpected error: %s", err))

tmpID := "tmpid"
readPolicy := "read"
readPolicy := read
writePolicy := "write"
deletePolicy := "delete"
memberPolicy := "member"
Expand Down Expand Up @@ -1253,7 +1254,7 @@ func TestListPolicies(t *testing.T) {
_, apiToken, err := svc.Issue(context.Background(), secret, key)
assert.Nil(t, err, fmt.Sprintf("Issuing user's key expected to succeed: %s", err))

readPolicy := "read"
readPolicy := read
pageLen := 15

// Add arbitrary policies to the user.
Expand Down
6 changes: 4 additions & 2 deletions bootstrap/api/endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -967,7 +967,9 @@ func TestList(t *testing.T) {
assert.Equal(t, tc.status, res.StatusCode, fmt.Sprintf("%s: expected status code %d got %d", tc.desc, tc.status, res.StatusCode))
var body configPage

json.NewDecoder(res.Body).Decode(&body)
err = json.NewDecoder(res.Body).Decode(&body)
assert.Nil(t, err, fmt.Sprintf("%s: unexpected error while decoding response body: %s", tc.desc, err))

assert.Nil(t, err, fmt.Sprintf("%s: unexpected error %s", tc.desc, err))
assert.ElementsMatch(t, tc.res.Configs, body.Configs, fmt.Sprintf("%s: expected response '%s' got '%s'", tc.desc, tc.res.Configs, body.Configs))
assert.Equal(t, tc.res.Total, body.Total, fmt.Sprintf("%s: expected response total '%d' got '%d'", tc.desc, tc.res.Total, body.Total))
Expand Down Expand Up @@ -1157,7 +1159,7 @@ func TestBootstrap(t *testing.T) {
assert.Nil(t, err, fmt.Sprintf("%s: unexpected error %s", tc.desc, err))
if tc.secure && tc.status == http.StatusOK {
body, err = dec(body)
assert.Nil(t, err, fmt.Sprintf("%sGot unexpected error: %s\n", tc.desc, err))
assert.Nil(t, err, fmt.Sprintf("%s: unexpected error while decoding body: %s", tc.desc, err))
}

data := strings.Trim(string(body), "\n")
Expand Down
4 changes: 0 additions & 4 deletions bootstrap/mocks/configs.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,6 @@ func (crm *configRepositoryMock) RetrieveAll(token string, filter bootstrap.Filt

configs := make([]bootstrap.Config, 0)

if offset < 0 || limit <= 0 {
return bootstrap.ConfigsPage{}
}

first := uint64(offset) + 1
last := first + uint64(limit)
var state bootstrap.State = emptyState
Expand Down
2 changes: 1 addition & 1 deletion bootstrap/postgres/configs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -598,7 +598,7 @@ func TestRemoveThing(t *testing.T) {
assert.Nil(t, err, fmt.Sprintf("Saving config expected to succeed: %s.\n", err))
for i := 0; i < 2; i++ {
err := repo.RemoveThing(saved)
assert.Nil(t, err, fmt.Sprintf("an unexpected error occured: %s\n", err))
assert.Nil(t, err, fmt.Sprintf("an unexpected error occurred: %s\n", err))
}
}

Expand Down
Loading