fix: sort companies before comparing since map-iteration is not stable

.gitlab-ci.yml

@@ -20,15 +20,15 @@ stages:
 
 deps:
   stage: deps
-  image: golang:1.13
+  image: golang:1.14
   script:
-    - go get -mod=readonly
+    - go mod download
 
 test:
   stage: test
   dependencies:
     - deps
-  image: golang:1.13
+  image: golang:1.14
   script:
     - go fmt $(go list ./...)
     - go vet $(go list ./...)

README.md

@@ -1,3 +1,3 @@
 # Shiny authz-client
 
-[![Build Status](https://gitlab.com/unboundsoftware/shiny/authz-client/badges/master/pipeline.svg)](https://gitlab.com/unboundsoftware/shiny/authz-client/commits/master)[![coverage report](https://gitlab.com/unboundsoftware/shiny/authz-client/badges/master/coverage.svg)](https://gitlab.com/unboundsoftware/shiny/authz-client/commits/master)
+[![Build Status](https://gitlab.com/unboundsoftware/shiny/authz_client/badges/master/pipeline.svg)](https://gitlab.com/unboundsoftware/shiny/authz_client/commits/master)[![coverage report](https://gitlab.com/unboundsoftware/shiny/authz_client/badges/master/coverage.svg)](https://gitlab.com/unboundsoftware/shiny/authz_client/commits/master)

client.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"net/http"
+	"reflect"
 )
 
 // CompanyPrivileges contains the privileges for a combination of email address and company id
@@ -18,24 +19,11 @@ type CompanyPrivileges struct {
 	Supplier   bool `json:"supplier"`
 }
 
-// PrivilegeAdded is the event sent when a new privilege is added
-type PrivilegeAdded struct {
-	Email      string `json:"email"`
-	CompanyID  string `json:"companyId"`
-	Admin      bool   `json:"admin"`
-	Company    bool   `json:"company"`
-	Consumer   bool   `json:"consumer"`
-	Time       bool   `json:"time"`
-	Invoicing  bool   `json:"invoicing"`
-	Accounting bool   `json:"accounting"`
-	Supplier   bool   `json:"supplier"`
-}
-
 // PrivilegeHandler processes PrivilegeAdded-events and fetches the initial set of privileges from an authz-service
 type PrivilegeHandler struct {
 	client     *http.Client
 	baseURL    string
-	privileges map[string]map[string]CompanyPrivileges
+	privileges map[string]map[string]*CompanyPrivileges
 }
 
 // OptsFunc is used to configure the PrivilegeHandler
@@ -53,7 +41,7 @@ func New(opts ...OptsFunc) *PrivilegeHandler {
 	handler := &PrivilegeHandler{
 		client:     &http.Client{},
 		baseURL:    "http://authz-service",
-		privileges: map[string]map[string]CompanyPrivileges{},
+		privileges: map[string]map[string]*CompanyPrivileges{},
 	}
 	for _, opt := range opts {
 		opt(handler)
@@ -82,29 +70,59 @@ func (h *PrivilegeHandler) Fetch() error {
 
 // Process privilege-related events and update the internal state
 func (h *PrivilegeHandler) Process(msg interface{}) bool {
-	if ev, ok := msg.(*PrivilegeAdded); ok {
-		h.setPrivileges(ev)
+	switch ev := msg.(type) {
+	case *UserAdded:
+		if priv, exists := h.privileges[ev.Email]; exists {
+			priv[ev.CompanyID] = &CompanyPrivileges{}
+		} else {
+			h.privileges[ev.Email] = map[string]*CompanyPrivileges{
+				ev.CompanyID: {},
+			}
+		}
 		return true
+	case *UserRemoved:
+		if priv, exists := h.privileges[ev.Email]; exists {
+			delete(priv, ev.CompanyID)
+		}
+		return true
+	case *PrivilegeAdded:
+		h.setPrivileges(ev.Email, ev.CompanyID, ev.Privilege, true)
+		return true
+	case *PrivilegeRemoved:
+		h.setPrivileges(ev.Email, ev.CompanyID, ev.Privilege, false)
+		return true
+	default:
+		fmt.Printf("Got unexpected message type (%s): '%+v'\n", reflect.TypeOf(msg).String(), msg)
+		return false
 	}
-	return false
 }
 
-func (h *PrivilegeHandler) setPrivileges(ev *PrivilegeAdded) {
-	if priv, exists := h.privileges[ev.Email]; exists {
-		priv[ev.CompanyID] = CompanyPrivileges{
-			Admin:      ev.Admin,
-			Company:    ev.Company,
-			Consumer:   ev.Consumer,
-			Time:       ev.Time,
-			Invoicing:  ev.Invoicing,
-			Accounting: ev.Accounting,
-			Supplier:   ev.Supplier,
+func (h *PrivilegeHandler) setPrivileges(email, companyId string, privilege Privilege, set bool) {
+	if priv, exists := h.privileges[email]; exists {
+		if c, exists := priv[companyId]; exists {
+			switch privilege {
+			case PrivilegeAdmin:
+				c.Admin = set
+			case PrivilegeCompany:
+				c.Company = set
+			case PrivilegeConsumer:
+				c.Consumer = set
+			case PrivilegeTime:
+				c.Time = set
+			case PrivilegeInvoicing:
+				c.Invoicing = set
+			case PrivilegeAccounting:
+				c.Accounting = set
+			case PrivilegeSupplier:
+				c.Supplier = set
+			}
+		} else {
+			priv[companyId] = &CompanyPrivileges{}
+			h.setPrivileges(email, companyId, privilege, set)
 		}
 	} else {
-		h.privileges[ev.Email] = map[string]CompanyPrivileges{
-			ev.CompanyID: {},
-		}
-		h.setPrivileges(ev)
+		h.privileges[email] = map[string]*CompanyPrivileges{}
+		h.setPrivileges(email, companyId, privilege, set)
 	}
 }
 
@@ -113,10 +131,21 @@ func (h *PrivilegeHandler) CompaniesByUser(email string, predicate func(privileg
 	var result []string
 	if p, exists := h.privileges[email]; exists {
 		for k, v := range p {
-			if predicate(v) {
+			if predicate(*v) {
 				result = append(result, k)
 			}
 		}
 	}
 	return result
 }
+
+// IsAllowed return true if the provided predicate return true for the privileges matching the provided email and companyID, return false otherwise
+func (h *PrivilegeHandler) IsAllowed(email, companyID string, predicate func(privileges CompanyPrivileges) bool) bool {
+	if p, exists := h.privileges[email]; exists {
+		if v, exists := p[companyID]; exists {
+			return predicate(*v)
+		}
+	}
+
+	return false
+}

client_test.go

@@ -5,6 +5,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"net/http"
 	"net/http/httptest"
+	"sort"
 	"testing"
 )
 
@@ -16,6 +17,75 @@ func TestPrivilegeHandler_Process_InvalidType(t *testing.T) {
 	assert.False(t, result)
 }
 
+func TestPrivilegeHandler_Process_PrivilegeRemoved(t *testing.T) {
+	handler := New(WithBaseURL("base"))
+
+	result := handler.Process(&PrivilegeAdded{
+		Email:     "jim@example.org",
+		CompanyID: "abc-123",
+		Privilege: PrivilegeAdmin,
+	})
+	assert.True(t, result)
+
+	companies := handler.CompaniesByUser("jim@example.org", func(privileges CompanyPrivileges) bool {
+		return privileges.Admin
+	})
+
+	assert.Equal(t, []string{"abc-123"}, companies)
+
+	result = handler.Process(&PrivilegeRemoved{
+		Email:     "jim@example.org",
+		CompanyID: "abc-123",
+		Privilege: PrivilegeAdmin,
+	})
+	assert.True(t, result)
+
+	companies = handler.CompaniesByUser("jim@example.org", func(privileges CompanyPrivileges) bool {
+		return privileges.Admin
+	})
+
+	assert.Empty(t, companies)
+}
+
+func TestPrivilegeHandler_Process_UserAdded_And_UserRemoved(t *testing.T) {
+	handler := New(WithBaseURL("base"))
+
+	result := handler.Process(&UserAdded{
+		Email:     "jim@example.org",
+		CompanyID: "abc-123",
+	})
+	assert.True(t, result)
+
+	result = handler.Process(&UserAdded{
+		Email:     "jim@example.org",
+		CompanyID: "abc-456",
+	})
+	assert.True(t, result)
+
+	companies := handler.CompaniesByUser("jim@example.org", func(privileges CompanyPrivileges) bool {
+		return true
+	})
+	sort.Strings(companies)
+	assert.Equal(t, []string{"abc-123", "abc-456"}, companies)
+
+	result = handler.Process(&UserRemoved{
+		Email:     "jim@example.org",
+		CompanyID: "abc-123",
+	})
+	assert.True(t, result)
+
+	result = handler.Process(&UserRemoved{
+		Email:     "jim@example.org",
+		CompanyID: "abc-456",
+	})
+	assert.True(t, result)
+
+	companies = handler.CompaniesByUser("jim@example.org", func(privileges CompanyPrivileges) bool {
+		return true
+	})
+	assert.Empty(t, companies)
+}
+
 func TestPrivilegeHandler_GetCompanies_Email_Not_Found(t *testing.T) {
 	handler := New(WithBaseURL("base"))
 
@@ -29,16 +99,9 @@ func TestPrivilegeHandler_GetCompanies_Email_Not_Found(t *testing.T) {
 func TestPrivilegeHandler_GetCompanies_No_Companies_Found(t *testing.T) {
 	handler := New(WithBaseURL("base"))
 
-	result := handler.Process(&PrivilegeAdded{
-		Email:      "jim@example.org",
-		CompanyID:  "abc-123",
-		Admin:      false,
-		Company:    false,
-		Consumer:   false,
-		Time:       false,
-		Invoicing:  false,
-		Accounting: false,
-		Supplier:   false,
+	result := handler.Process(&UserAdded{
+		Email:     "jim@example.org",
+		CompanyID: "abc-123",
 	})
 	assert.True(t, result)
 
@@ -47,21 +110,32 @@ func TestPrivilegeHandler_GetCompanies_No_Companies_Found(t *testing.T) {
 	})
 
 	assert.Empty(t, companies)
+
+	companies = handler.CompaniesByUser("jim@example.org", func(privileges CompanyPrivileges) bool {
+		return true
+	})
+
+	assert.Equal(t, []string{"abc-123"}, companies)
+
+	result = handler.Process(&UserRemoved{
+		Email:     "jim@example.org",
+		CompanyID: "abc-123",
+	})
+	assert.True(t, result)
+
+	companies = handler.CompaniesByUser("jim@example.org", func(privileges CompanyPrivileges) bool {
+		return true
+	})
+	assert.Empty(t, companies)
 }
 
 func TestPrivilegeHandler_GetCompanies_Company_With_Company_Access_Found(t *testing.T) {
 	handler := New(WithBaseURL("base"))
 
 	result := handler.Process(&PrivilegeAdded{
-		Email:      "jim@example.org",
-		CompanyID:  "abc-123",
-		Admin:      false,
-		Company:    true,
-		Consumer:   false,
-		Time:       false,
-		Invoicing:  false,
-		Accounting: false,
-		Supplier:   false,
+		Email:     "jim@example.org",
+		CompanyID: "abc-123",
+		Privilege: PrivilegeCompany,
 	})
 	assert.True(t, result)
 
@@ -76,25 +150,81 @@ func TestPrivilegeHandler_GetCompanies_Company_With_Admin_Access_Found(t *testin
 	handler := New(WithBaseURL("base"))
 
 	result := handler.Process(&PrivilegeAdded{
-		Email:      "jim@example.org",
-		CompanyID:  "abc-123",
-		Admin:      true,
-		Company:    false,
-		Consumer:   false,
-		Time:       false,
-		Invoicing:  false,
-		Accounting: false,
-		Supplier:   false,
+		Email:     "jim@example.org",
+		CompanyID: "abc-123",
+		Privilege: PrivilegeConsumer,
 	})
 	assert.True(t, result)
 
 	companies := handler.CompaniesByUser("jim@example.org", func(privileges CompanyPrivileges) bool {
-		return privileges.Admin
+		return privileges.Consumer
 	})
 
 	assert.Equal(t, []string{"abc-123"}, companies)
 }
 
+func TestPrivilegeHandler_IsAllowed_Return_False_If_No_Privileges(t *testing.T) {
+	handler := New(WithBaseURL("base"))
+
+	result := handler.IsAllowed("jim@example.org", "abc-123", func(privileges CompanyPrivileges) bool {
+		return privileges.Company
+	})
+
+	assert.False(t, result)
+}
+
+func TestPrivilegeHandler_IsAllowed_Return_True_If_Privilege_Exists(t *testing.T) {
+	handler := New(WithBaseURL("base"))
+
+	handler.Process(&PrivilegeAdded{
+		Email:     "jim@example.org",
+		CompanyID: "abc-123",
+		Privilege: PrivilegeTime,
+	})
+
+	result := handler.IsAllowed("jim@example.org", "abc-123", func(privileges CompanyPrivileges) bool {
+		return privileges.Time
+	})
+
+	assert.True(t, result)
+
+	handler.Process(&PrivilegeAdded{
+		Email:     "jim@example.org",
+		CompanyID: "abc-123",
+		Privilege: PrivilegeInvoicing,
+	})
+
+	result = handler.IsAllowed("jim@example.org", "abc-123", func(privileges CompanyPrivileges) bool {
+		return privileges.Invoicing
+	})
+
+	assert.True(t, result)
+
+	handler.Process(&PrivilegeAdded{
+		Email:     "jim@example.org",
+		CompanyID: "abc-123",
+		Privilege: PrivilegeAccounting,
+	})
+
+	result = handler.IsAllowed("jim@example.org", "abc-123", func(privileges CompanyPrivileges) bool {
+		return privileges.Accounting
+	})
+
+	assert.True(t, result)
+
+	handler.Process(&PrivilegeAdded{
+		Email:     "jim@example.org",
+		CompanyID: "abc-123",
+		Privilege: PrivilegeSupplier,
+	})
+
+	result = handler.IsAllowed("jim@example.org", "abc-123", func(privileges CompanyPrivileges) bool {
+		return privileges.Supplier
+	})
+
+	assert.True(t, result)
+}
+
 func TestPrivilegeHandler_Fetch_Error_Response(t *testing.T) {
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(500)
@@ -106,7 +236,7 @@ func TestPrivilegeHandler_Fetch_Error_Response(t *testing.T) {
 	server.Close()
 
 	err := handler.Fetch()
-	assert.EqualError(t, err, fmt.Sprintf("Get http://%s/authz: dial tcp %s: connect: connection refused", baseURL, baseURL))
+	assert.EqualError(t, err, fmt.Sprintf("Get \"http://%s/authz\": dial tcp %s: connect: connection refused", baseURL, baseURL))
 }
 
 func TestPrivilegeHandler_Fetch_Error_Unreadable_Body(t *testing.T) {
@@ -160,7 +290,7 @@ func TestPrivilegeHandler_Fetch_Valid(t *testing.T) {
 
 	err := handler.Fetch()
 	assert.NoError(t, err)
-	expectedPrivileges := map[string]map[string]CompanyPrivileges{
+	expectedPrivileges := map[string]map[string]*CompanyPrivileges{
 		"jim@example.org": {
 			"00010203-0405-4607-8809-0a0b0c0d0e0f": {
 				Admin:      false,

events.go

@@ -0,0 +1,62 @@
+package client
+
+// UserAdded is the event sent when a new user is added to a company
+type UserAdded struct {
+	Email     string `json:"email"`
+	CompanyID string `json:"companyId"`
+}
+
+// UserRemoved is the event sent when a user is removed from a company
+type UserRemoved struct {
+	Email     string `json:"email"`
+	CompanyID string `json:"companyId"`
+}
+
+// Privilege is an enumeration of all available privileges
+type Privilege string
+
+const (
+	PrivilegeAdmin      = "ADMIN"
+	PrivilegeCompany    = "COMPANY"
+	PrivilegeConsumer   = "CONSUMER"
+	PrivilegeTime       = "TIME"
+	PrivilegeInvoicing  = "INVOICING"
+	PrivilegeAccounting = "ACCOUNTING"
+	PrivilegeSupplier   = "SUPPLIER"
+)
+
+var AllPrivilege = []Privilege{
+	PrivilegeAdmin,
+	PrivilegeCompany,
+	PrivilegeConsumer,
+	PrivilegeTime,
+	PrivilegeInvoicing,
+	PrivilegeAccounting,
+	PrivilegeSupplier,
+}
+
+func (e Privilege) IsValid() bool {
+	switch e {
+	case PrivilegeAdmin, PrivilegeCompany, PrivilegeConsumer, PrivilegeTime, PrivilegeInvoicing, PrivilegeAccounting, PrivilegeSupplier:
+		return true
+	}
+	return false
+}
+
+func (e Privilege) String() string {
+	return string(e)
+}
+
+// PrivilegeAdded is the event sent when a new privilege is added
+type PrivilegeAdded struct {
+	Email     string    `json:"email"`
+	CompanyID string    `json:"companyId"`
+	Privilege Privilege `json:"privilege"`
+}
+
+// PrivilegeRemoved is the event sent when a privilege is removed
+type PrivilegeRemoved struct {
+	Email     string    `json:"email"`
+	CompanyID string    `json:"companyId"`
+	Privilege Privilege `json:"privilege"`
+}

events_test.go

@@ -0,0 +1,110 @@
+package client
+
+import "testing"
+
+func TestPrivilege_IsValid(t *testing.T) {
+	tests := []struct {
+		name string
+		e    Privilege
+		want bool
+	}{
+		{
+			name: "Admin",
+			e:    "ADMIN",
+			want: true,
+		},
+		{
+			name: "Company",
+			e:    "COMPANY",
+			want: true,
+		},
+		{
+			name: "Consumer",
+			e:    "CONSUMER",
+			want: true,
+		},
+		{
+			name: "Time",
+			e:    "TIME",
+			want: true,
+		},
+		{
+			name: "Invoicing",
+			e:    "INVOICING",
+			want: true,
+		},
+		{
+			name: "Accounting",
+			e:    "ACCOUNTING",
+			want: true,
+		},
+		{
+			name: "Supplier",
+			e:    "SUPPLIER",
+			want: true,
+		},
+		{
+			name: "Invalid",
+			e:    "BLUTTI",
+			want: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := tt.e.IsValid(); got != tt.want {
+				t.Errorf("IsValid() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestPrivilege_String(t *testing.T) {
+	tests := []struct {
+		name string
+		e    Privilege
+		want string
+	}{
+		{
+			name: "Admin",
+			e:    "ADMIN",
+			want: "ADMIN",
+		},
+		{
+			name: "Company",
+			e:    "COMPANY",
+			want: "COMPANY",
+		},
+		{
+			name: "Consumer",
+			e:    "CONSUMER",
+			want: "CONSUMER",
+		},
+		{
+			name: "Time",
+			e:    "TIME",
+			want: "TIME",
+		},
+		{
+			name: "Invoicing",
+			e:    "INVOICING",
+			want: "INVOICING",
+		},
+		{
+			name: "Accounting",
+			e:    "ACCOUNTING",
+			want: "ACCOUNTING",
+		},
+		{
+			name: "Supplier",
+			e:    "SUPPLIER",
+			want: "SUPPLIER",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := tt.e.String(); got != tt.want {
+				t.Errorf("String() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}