Merge pull request #183 from eurofurence/issue-182-add-info-self-edit

feat(#182): improve addnl info permission handling

api/openapi-spec/openapi.yaml

@@ -273,7 +273,11 @@ paths:
       tags:
         - additional
       summary: obtain the current additional info for an area
-      description: Returns the current additional info for an area (e.g. regdesk, sponsordesk, overdue, ...). User will need to have permission called {area} to access it.
+      description: | 
+        Returns the current additional info for an area (e.g. regdesk, sponsordesk, overdue, ...).
+        
+        User will need to have permissions on any of their registrations as set in configuration to access it.
+        If configuration allows self_read, then a user can also read this area on their own registration.
       operationId: getAdditionalInfo
       parameters:
         - name: id
@@ -336,9 +340,12 @@ paths:
         - additional
       summary: set the current additional info for an area
       description: |
-        Set the current additional info for an area (e.g. regdesk, sponsordesk, overdue, ...). User will need to have permission "area" to access it.
+        Set the current additional info for an area (e.g. regdesk, sponsordesk, overdue, ...).
         
         You can store an arbitrary json object here, but the length is limited to 1024 characters.
+        
+        User will need to have permissions on any of their registrations as set in configuration to access it. 
+        If configuration allows self_write, then a user can also read the given area on their own registration.
       operationId: setAdditionalInfo
       parameters:
         - name: id
@@ -407,7 +414,10 @@ paths:
         - additional
       summary: remove the current additional info for an area
       description: |
-        Remove the current additional info for an area (e.g. regdesk, sponsordesk, overdue, ...). User will need to have permission "area" to do this.
+        Remove the current additional info for an area (e.g. regdesk, sponsordesk, overdue, ...).
+
+        User will need to have permissions on any of their registrations as set in configuration to access it. 
+        If configuration allows self_write, then a user can also read the given area on their own registration.
       operationId: deleteAdditionalInfo
       parameters:
         - name: id

docs/config-template.yaml

@@ -66,9 +66,22 @@ dues:
 birthday:
   earliest: '1901-01-01'
   latest: '2004-08-24'
-permissions:
-  - regdesk
-  - sponsordesk
+additional_info_areas:
+  # the key is the "area" parameter in the API url (/attendees/{id}/additional-info/{area}).
+  # Key must be [a-z]+. The key "overdue" is reserved for internal use and thus not allowed here.
+  regdesk:
+    permissions:
+      - regdesk
+  sponsordesk:
+    permissions:
+      - sponsordesk
+  shipping:
+    self_read: true # allow a user to read this additional info area on their own registration
+    self_write: true # allow a user to write/delete this additional info area on their own registration
+    # a user needs to have any of the values listed under permissions: in the "permissions" field on their registration
+    # to have read and write access to this area for ALL registrations, not just their own
+    permissions:
+      - sponsordesk
 choices:
   flags:
     hc:

internal/repository/config/config.go

@@ -6,6 +6,7 @@ import (
 	"crypto/rsa"
 	"fmt"
 	"github.com/eurofurence/reg-attendee-service/internal/api/v1/status"
+	"sort"
 	"strings"
 	"time"
 )
@@ -115,11 +116,43 @@ func AllowedOptions() []string {
 	return sortedKeys(Configuration().Choices.Options)
 }
 
+func AdditionalInfoFieldNames() []string {
+	result := make([]string, 0)
+	for k := range Configuration().AdditionalInfo {
+		result = append(result, k)
+	}
+	sort.Strings(result)
+	return result
+}
+
+// AllowedPermissions returns a sorted unique map of all permissions referenced in
+// additional info configurations.
 func AllowedPermissions() []string {
-	if len(Configuration().Permissions) > 0 {
-		return Configuration().Permissions
+	resultMap := make(map[string]bool)
+	for _, v := range Configuration().AdditionalInfo {
+		for _, perm := range v.Permissions {
+			resultMap[perm] = true
+		}
+	}
+
+	result := make([]string, 0)
+	for k := range resultMap {
+		result = append(result, k)
+	}
+	sort.Strings(result)
+	return result
+}
+
+func AdditionalInfoConfiguration(fieldName string) AddInfoConfig {
+	v, ok := Configuration().AdditionalInfo[fieldName]
+	if ok {
+		return v
 	} else {
-		return []string{"regdesk", "sponsordesk"}
+		return AddInfoConfig{
+			SelfRead:    false,
+			SelfWrite:   false,
+			Permissions: []string{},
+		}
 	}
 }
 

internal/repository/config/structure.go

@@ -22,22 +22,22 @@ const HumanDateFormat = "02.01.2006"
 type (
 	// Application is the root configuration type
 	Application struct {
-		Service               ServiceConfig     `yaml:"service"`
-		Server                ServerConfig      `yaml:"server"`
-		Database              DatabaseConfig    `yaml:"database"`
-		Security              SecurityConfig    `yaml:"security"`
-		Logging               LoggingConfig     `yaml:"logging"`
-		Choices               FlagsPkgOptConfig `yaml:"choices"`
-		Permissions           []string          `yaml:"permissions"`
-		TShirtSizes           []string          `yaml:"tshirtsizes"`
-		Birthday              BirthdayConfig    `yaml:"birthday"`
-		GoLive                GoLiveConfig      `yaml:"go_live"`
-		Dues                  DuesConfig        `yaml:"dues"`
-		Countries             []string          `yaml:"countries"`
-		SpokenLanguages       []string          `yaml:"spoken_languages"`
-		RegistrationLanguages []string          `yaml:"registration_languages"`
-		Currency              string            `yaml:"currency"`
-		VatPercent            float64           `yaml:"vat_percent"` // used for manual dues
+		Service               ServiceConfig            `yaml:"service"`
+		Server                ServerConfig             `yaml:"server"`
+		Database              DatabaseConfig           `yaml:"database"`
+		Security              SecurityConfig           `yaml:"security"`
+		Logging               LoggingConfig            `yaml:"logging"`
+		Choices               FlagsPkgOptConfig        `yaml:"choices"`
+		AdditionalInfo        map[string]AddInfoConfig `yaml:"additional_info_areas"` // field name -> config
+		TShirtSizes           []string                 `yaml:"tshirtsizes"`
+		Birthday              BirthdayConfig           `yaml:"birthday"`
+		GoLive                GoLiveConfig             `yaml:"go_live"`
+		Dues                  DuesConfig               `yaml:"dues"`
+		Countries             []string                 `yaml:"countries"`
+		SpokenLanguages       []string                 `yaml:"spoken_languages"`
+		RegistrationLanguages []string                 `yaml:"registration_languages"`
+		Currency              string                   `yaml:"currency"`
+		VatPercent            float64                  `yaml:"vat_percent"` // used for manual dues
 	}
 
 	// ServiceConfig contains configuration values
@@ -130,6 +130,14 @@ type (
 		ConstraintMsg string  `yaml:"constraint_msg"`
 	}
 
+	// AddInfoConfig configures access permissions to an additional info field
+	AddInfoConfig struct {
+		SelfRead    bool     `yaml:"self_read"`
+		SelfWrite   bool     `yaml:"self_write"`
+		Permissions []string `yaml:"permissions"` // name of permission (in admin info) to grant access
+		// could later also add groups
+	}
+
 	// BirthdayConfig is used for validation of attendee supplied birthday
 	//
 	// use it to exclude nonsensical values, or to exclude participants under a minimum age

internal/service/attendeesrv/addinfo.go

@@ -32,3 +32,24 @@ func (s *AttendeeServiceImplData) CanAccessAdditionalInfoArea(ctx context.Contex
 	allowed, err := s.subjectHasAdminPermissionEntry(ctx, loggedInSubject, area...)
 	return allowed, err
 }
+
+func (s *AttendeeServiceImplData) CanAccessOwnAdditionalInfoArea(ctx context.Context, attendeeId uint, wantWriteAccess bool, area string) (bool, error) {
+	att, err := database.GetRepository().GetAttendeeById(ctx, attendeeId)
+	if err != nil {
+		// attendee does not exist is checked later in order to not expose information
+		return false, nil
+	}
+
+	loggedInSubject := ctxvalues.Subject(ctx)
+	if loggedInSubject != "" && loggedInSubject == att.Identity {
+		conf := config.AdditionalInfoConfiguration(area)
+		if wantWriteAccess && conf.SelfWrite {
+			return true, nil
+		}
+		if !wantWriteAccess && conf.SelfRead {
+			return true, nil
+		}
+	}
+
+	return false, nil
+}

internal/service/attendeesrv/interfaces.go

@@ -73,12 +73,17 @@ type AttendeeService interface {
 	// If value is the empty string, the entry is deleted instead.
 	WriteAdditionalInfo(ctx context.Context, attendeeId uint, area string, value string) error
 
-	// CanAccessAdditionalInfoArea checks permission to access additional info for area.
+	// CanAccessAdditionalInfoArea checks permission to access additional info for a whole area.
 	//
 	// Normal users (loaded by identity) need a matching permissions entry in their admin info.
 	// Admins and Api Token can see all areas.
 	CanAccessAdditionalInfoArea(ctx context.Context, area ...string) (bool, error)
 
+	// CanAccessOwnAdditionalInfoArea checks permission to access ones own additional info for a given area
+	//
+	// This is only allowed for areas which have self_read or self_write configured.
+	CanAccessOwnAdditionalInfoArea(ctx context.Context, attendeeId uint, wantWriteAccess bool, area string) (bool, error)
+
 	// GenerateFakeRegistrations creates the specified number of fake registrations in the database.
 	//
 	// Only for use on test systems.

internal/service/attendeesrv/permissions.go

@@ -7,12 +7,12 @@ import (
 	"github.com/eurofurence/reg-attendee-service/internal/repository/database"
 )
 
-func (s *AttendeeServiceImplData) subjectHasAdminPermissionEntry(ctx context.Context, subject string, permission ...string) (bool, error) {
+func (s *AttendeeServiceImplData) subjectHasAdminPermissionEntry(ctx context.Context, subject string, areas ...string) (bool, error) {
 	if subject == "" {
 		return false, errors.New("not a logged in user subject - this is an implementation error")
 	}
-	if len(permission) == 0 {
-		return false, errors.New("must provide valid permissions - this is an implementation error")
+	if len(areas) == 0 {
+		return false, errors.New("must provide valid areas - this is an implementation error")
 	}
 
 	// check that any of the registrations owned by subject have the regdesk permission
@@ -28,14 +28,17 @@ func (s *AttendeeServiceImplData) subjectHasAdminPermissionEntry(ctx context.Con
 
 		permissions := commaSeparatedStrToMap(adminInfo.Permissions, config.AllowedPermissions())
 
-		for _, perm := range permission {
-			if perm == "" {
-				return false, errors.New("must provide valid permissions - this is an implementation error")
+		for _, area := range areas {
+			if area == "" {
+				return false, errors.New("must provide valid area - this is an implementation error")
 			}
 
-			allowed, _ := permissions[perm]
-			if allowed {
-				return true, nil
+			conf := config.AdditionalInfoConfiguration(area)
+			for _, perm := range conf.Permissions {
+				allowed, _ := permissions[perm]
+				if allowed {
+					return true, nil
+				}
 			}
 		}
 	}

internal/web/controller/addinfoctl/addinfoctl.go

@@ -37,7 +37,7 @@ func Create(server chi.Router, attendeeSrv attendeesrv.AttendeeService) {
 }
 
 func getAdditionalInfoHandler(w http.ResponseWriter, r *http.Request) {
-	ctx, id, area, err := ctxIdAreaAllowedAndExists_MustReturn(w, r)
+	ctx, id, area, err := ctxIdAreaAllowedAndExists_MustReturn(w, r, false)
 	if err != nil {
 		return
 	}
@@ -60,7 +60,7 @@ func getAdditionalInfoHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 func writeAdditionalInfoHandler(w http.ResponseWriter, r *http.Request) {
-	ctx, id, area, err := ctxIdAreaAllowedAndExists_MustReturn(w, r)
+	ctx, id, area, err := ctxIdAreaAllowedAndExists_MustReturn(w, r, true)
 	if err != nil {
 		return
 	}
@@ -86,7 +86,7 @@ func writeAdditionalInfoHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 func deleteAdditionalInfoHandler(w http.ResponseWriter, r *http.Request) {
-	ctx, id, area, err := ctxIdAreaAllowedAndExists_MustReturn(w, r)
+	ctx, id, area, err := ctxIdAreaAllowedAndExists_MustReturn(w, r, true)
 	if err != nil {
 		return
 	}
@@ -110,7 +110,7 @@ func deleteAdditionalInfoHandler(w http.ResponseWriter, r *http.Request) {
 	w.WriteHeader(http.StatusNoContent)
 }
 
-func ctxIdAreaAllowedAndExists_MustReturn(w http.ResponseWriter, r *http.Request) (context.Context, uint, string, error) {
+func ctxIdAreaAllowedAndExists_MustReturn(w http.ResponseWriter, r *http.Request, wantWriteAccess bool) (context.Context, uint, string, error) {
 	ctx := r.Context()
 
 	id, area, err := idAndAreaFromVarsValidated_MustReturn(ctx, w, r)
@@ -124,9 +124,16 @@ func ctxIdAreaAllowedAndExists_MustReturn(w http.ResponseWriter, r *http.Request
 		return ctx, id, area, err
 	}
 	if !allowed {
-		culprit := ctxvalues.Subject(ctx)
-		ctlutil.UnauthorizedError(ctx, w, r, "you are not authorized for this additional info area - the attempt has been logged", fmt.Sprintf("unauthorized access attempt for add info area %s by %s", area, culprit))
-		return ctx, id, area, errors.New("forbidden")
+		allowed, err = attendeeService.CanAccessOwnAdditionalInfoArea(ctx, id, wantWriteAccess, area)
+		if err != nil {
+			ctlutil.ErrorHandler(ctx, w, r, "addinfo.read.error", http.StatusInternalServerError, url.Values{})
+			return ctx, id, area, err
+		}
+		if !allowed {
+			culprit := ctxvalues.Subject(ctx)
+			ctlutil.UnauthorizedError(ctx, w, r, "you are not authorized for this additional info area - the attempt has been logged", fmt.Sprintf("unauthorized access attempt for add info area %s by %s", area, culprit))
+			return ctx, id, area, errors.New("forbidden")
+		}
 	}
 
 	_, err = attendeeService.GetAttendee(ctx, id)
@@ -156,7 +163,7 @@ func idAndAreaFromVarsValidated_MustReturn(ctx context.Context, w http.ResponseW
 		ctlutil.ErrorHandler(ctx, w, r, "addinfo.area.invalid", http.StatusBadRequest, url.Values{"area": []string{"the special value 'overdue' is used internally and is forbidden here"}})
 		return uint(id), area, errors.New("invalid additional info area")
 	}
-	if validation.NotInAllowedValues(config.AllowedPermissions(), area) {
+	if validation.NotInAllowedValues(config.AdditionalInfoFieldNames(), area) {
 		aulogging.Logger.Ctx(ctx).Warn().Printf("received additional info area '%s' not listed in configuration", area)
 		ctlutil.ErrorHandler(ctx, w, r, "addinfo.area.unlisted", http.StatusBadRequest, url.Values{"area": []string{"areas must be enabled in configuration"}})
 		return uint(id), area, errors.New("unlisted additional info area")

internal/web/controller/attendeectl/config_test.go

@@ -151,6 +151,10 @@ func (s *MockAttendeeService) CanAccessAdditionalInfoArea(ctx context.Context, a
 	return false, nil
 }
 
+func (s *MockAttendeeService) CanAccessOwnAdditionalInfoArea(ctx context.Context, attendeeId uint, wantWriteAccess bool, area string) (bool, error) {
+	return false, nil
+}
+
 func (s *MockAttendeeService) GenerateFakeRegistrations(ctx context.Context, count uint) error {
 	return nil
 }