Skip to content

Commit

Permalink
case insenstive error checking, removing dependency on grafana pkg (#8)
Browse files Browse the repository at this point in the history
  • Loading branch information
dskatz authored Dec 21, 2021
1 parent 67c6554 commit 94fc053
Show file tree
Hide file tree
Showing 8 changed files with 59 additions and 140 deletions.
3 changes: 2 additions & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
FROM golang:1.16 as build
WORKDIR /go/src/app
COPY go.mod go.sum .
RUN go mod download
COPY . .
RUN go get -d -v ./...
RUN go build -o /go/bin/app

FROM debian:buster-slim
Expand Down
11 changes: 4 additions & 7 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,14 @@ module github.com/kanopy-platform/grafana-auth-proxy
go 1.16

require (
github.com/go-macaron/session v1.0.2 // indirect
github.com/go-sql-driver/mysql v1.6.0 // indirect
github.com/gosimple/slug v1.11.2 // indirect
github.com/grafana/grafana v6.1.6+incompatible
github.com/grafana/grafana-api-golang-client v0.0.0-20211005011003-c69abe946fa6
github.com/inconshreveable/log15 v0.0.0-20201112154412-8562bdadbbac // indirect
github.com/gopherjs/gopherjs v0.0.0-20190430165422-3e4dfb77656c // indirect
github.com/grafana/grafana-api-golang-client v0.1.0
github.com/kanopy-platform/k8s-auth-portal v0.1.0
github.com/sirupsen/logrus v1.8.1
github.com/smartystreets/assertions v1.0.1 // indirect
github.com/spf13/cobra v1.2.1
github.com/spf13/viper v1.8.1
github.com/stretchr/testify v1.7.0
github.com/teris-io/shortid v0.0.0-20201117134242-e59966efd125 // indirect
golang.org/x/crypto v0.0.0-20201124201722-c8d3bf9c5392 // indirect
gopkg.in/square/go-jose.v2 v2.6.0
)
104 changes: 6 additions & 98 deletions go.sum

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions internal/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ func (s *Server) handleRoot() http.HandlerFunc {
if err != nil {
// if an upsert fails we still allow the user to login as it will be assigned to
// the configured default Org and Role
log.Infof("err: %v", err)
log.Infof("failed to update role %s in orgID %d for user %s", string(role), orgID, login)
}
}
Expand Down
7 changes: 3 additions & 4 deletions internal/server/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"testing"

gapi "github.com/grafana/grafana-api-golang-client"
"github.com/grafana/grafana/pkg/models"
"github.com/kanopy-platform/grafana-auth-proxy/internal/jwt"
"github.com/kanopy-platform/grafana-auth-proxy/pkg/config"
"github.com/kanopy-platform/grafana-auth-proxy/pkg/grafana"
Expand Down Expand Up @@ -38,7 +37,7 @@ func TestTokenValidations(t *testing.T) {
defer backendServer.Close()
backendURL, _ := url.Parse(backendServer.URL)

client := grafana.NewMockClient(gapi.User{Login: "jhon", ID: 1}, map[int64]models.RoleType{})
client := grafana.NewMockClient(gapi.User{Login: "jhon", ID: 1}, map[int64]grafana.RoleType{})

tests := []struct {
name string
Expand Down Expand Up @@ -127,8 +126,8 @@ func TestHandleRoot(t *testing.T) {
},
}

orgRoleMap := map[int64]models.RoleType{
1: models.ROLE_EDITOR,
orgRoleMap := map[int64]grafana.RoleType{
1: grafana.ROLE_EDITOR,
}

client := grafana.NewMockClient(gapi.User{Login: "jhon", ID: 1}, orgRoleMap)
Expand Down
43 changes: 29 additions & 14 deletions pkg/grafana/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,28 @@ import (
"strings"

gapi "github.com/grafana/grafana-api-golang-client"
"github.com/grafana/grafana/pkg/models"
"github.com/kanopy-platform/grafana-auth-proxy/pkg/config"
"github.com/kanopy-platform/k8s-auth-portal/pkg/random"
log "github.com/sirupsen/logrus"
)

var (
ErrRoleNotValid = errors.New("role is not valid")
ErrInvalidURL = errors.New("url is nil")
ErrRoleNotValid = errors.New("role is not valid")
ErrUserNotFound = errors.New("user not found")
ErrOrgUserAlreadyMember = errors.New("user is already member of this organization")
)

type userOrgsRoleMap map[int64]models.RoleType
type RoleType string

// Grafana pkgs cannot be safely imported as dependencies.
const (
ROLE_VIEWER RoleType = "Viewer"
ROLE_EDITOR RoleType = "Editor"
ROLE_ADMIN RoleType = "Admin"
)

type userOrgsRoleMap map[int64]RoleType

type Client struct {
client GAPIClient
Expand All @@ -34,7 +45,7 @@ func NewClient(baseURL *url.URL, cfg gapi.Config) (*Client, error) {
newClient := &Client{}

if baseURL == nil {
return nil, errors.New("url is nil")
return nil, ErrInvalidURL
}

client, err := gapi.New(baseURL.String(), cfg)
Expand All @@ -51,7 +62,10 @@ func NewClient(baseURL *url.URL, cfg gapi.Config) (*Client, error) {
func (c *Client) LookupUser(loginOrEmail string) (gapi.User, error) {
user, err := c.client.UserByEmail(loginOrEmail)
if err != nil {
if !strings.Contains(err.Error(), "User not found") {

// the error message is longer but given that this is constrained
// to a returned message from an specific call, it's better to keep it short
if !strings.Contains(strings.ToLower(err.Error()), ErrUserNotFound.Error()) {
return user, err
}
}
Expand Down Expand Up @@ -102,7 +116,8 @@ func (c *Client) UpsertOrgUser(orgID int64, user gapi.User, role string) error {

err := c.AddOrgUser(orgID, user.Login, role)
if err != nil {
if !strings.Contains(err.Error(), "User is already member of this organization") {

if !strings.Contains(strings.ToLower(err.Error()), ErrOrgUserAlreadyMember.Error()) {
return err
} else {
isOrgMember = true
Expand Down Expand Up @@ -138,11 +153,11 @@ func (c *Client) UpdateOrgUserAuthz(user gapi.User, groups config.Groups) (userO

for _, org := range group.Orgs {
// Check if the users has a more permissive role and apply that instead
if !isRoleAssignable(userOrgsRole[org.ID], models.RoleType(org.Role)) {
if !isRoleAssignable(userOrgsRole[org.ID], RoleType(org.Role)) {
continue
}

userOrgsRole[org.ID] = models.RoleType(org.Role)
userOrgsRole[org.ID] = RoleType(org.Role)
}
}

Expand Down Expand Up @@ -180,16 +195,16 @@ func (c *Client) GetOrCreateUser(login string) (gapi.User, error) {
return user, nil
}

func isRoleAssignable(currentRole models.RoleType, incomingRole models.RoleType) bool {
func isRoleAssignable(currentRole RoleType, incomingRole RoleType) bool {
// role hierarchy
roleHierarchy := map[models.RoleType]int{
models.ROLE_VIEWER: 0,
models.ROLE_EDITOR: 1,
models.ROLE_ADMIN: 2,
roleHierarchy := map[RoleType]int{
ROLE_VIEWER: 0,
ROLE_EDITOR: 1,
ROLE_ADMIN: 2,
}

// If the incoming role is less than ( less privilege ) than the currently assigned role ( more privilege ), skip this mapping.
if currentRole != "" && roleHierarchy[models.RoleType(incomingRole)] < roleHierarchy[currentRole] {
if currentRole != "" && roleHierarchy[RoleType(incomingRole)] < roleHierarchy[currentRole] {
return false
}

Expand Down
27 changes: 13 additions & 14 deletions pkg/grafana/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"testing"

gapi "github.com/grafana/grafana-api-golang-client"
"github.com/grafana/grafana/pkg/models"
"github.com/kanopy-platform/grafana-auth-proxy/pkg/config"
"github.com/stretchr/testify/assert"
)
Expand Down Expand Up @@ -47,7 +46,7 @@ func TestAddOrgUser(t *testing.T) {
user := newUser("foo", 1)

orgRoleMap := userOrgsRoleMap{
1: models.ROLE_EDITOR,
1: ROLE_EDITOR,
}
client := NewMockClient(user, orgRoleMap)

Expand All @@ -64,7 +63,7 @@ func TestUpsertOrgUser(t *testing.T) {
user := newUser("foo", 1)

orgRoleMap := userOrgsRoleMap{
1: models.ROLE_EDITOR,
1: ROLE_EDITOR,
}

client := NewMockClient(user, orgRoleMap)
Expand Down Expand Up @@ -335,16 +334,16 @@ func TestGetOrCreateUser(t *testing.T) {

func TestIsRoleAssignable(t *testing.T) {
// table test to validate isRoleAssignable(currentRole, incomingRole)
assert.True(t, isRoleAssignable("", models.ROLE_VIEWER))
assert.True(t, isRoleAssignable(models.ROLE_VIEWER, models.ROLE_EDITOR))
assert.True(t, isRoleAssignable(models.ROLE_VIEWER, models.ROLE_ADMIN))
assert.True(t, isRoleAssignable(models.ROLE_EDITOR, models.ROLE_ADMIN))
assert.False(t, isRoleAssignable(models.ROLE_ADMIN, models.ROLE_EDITOR))
assert.False(t, isRoleAssignable(models.ROLE_ADMIN, models.ROLE_VIEWER))
assert.False(t, isRoleAssignable(models.ROLE_EDITOR, models.ROLE_VIEWER))
assert.True(t, isRoleAssignable(models.ROLE_VIEWER, models.ROLE_VIEWER))

roles := map[int64]models.RoleType{}
assert.True(t, isRoleAssignable(roles[0], models.ROLE_VIEWER))
assert.True(t, isRoleAssignable("", ROLE_VIEWER))
assert.True(t, isRoleAssignable(ROLE_VIEWER, ROLE_EDITOR))
assert.True(t, isRoleAssignable(ROLE_VIEWER, ROLE_ADMIN))
assert.True(t, isRoleAssignable(ROLE_EDITOR, ROLE_ADMIN))
assert.False(t, isRoleAssignable(ROLE_ADMIN, ROLE_EDITOR))
assert.False(t, isRoleAssignable(ROLE_ADMIN, ROLE_VIEWER))
assert.False(t, isRoleAssignable(ROLE_EDITOR, ROLE_VIEWER))
assert.True(t, isRoleAssignable(ROLE_VIEWER, ROLE_VIEWER))

roles := map[int64]RoleType{}
assert.True(t, isRoleAssignable(roles[0], ROLE_VIEWER))

}
3 changes: 1 addition & 2 deletions pkg/grafana/mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"errors"

gapi "github.com/grafana/grafana-api-golang-client"
"github.com/grafana/grafana/pkg/models"
"github.com/stretchr/testify/mock"
)

Expand Down Expand Up @@ -60,7 +59,7 @@ func (c *mockGAPIClient) UpdateUserPermissions(id int64, isAdmin bool) error {
}

// MockClient returns a Client using a mocked GAPIClient underneat
func NewMockClient(user gapi.User, orgRoleMap map[int64]models.RoleType) *Client {
func NewMockClient(user gapi.User, orgRoleMap map[int64]RoleType) *Client {
return &Client{
client: &mockGAPIClient{
user: user,
Expand Down

0 comments on commit 94fc053

Please sign in to comment.