Merge pull request #86 from eurofurence/issue-33-further-impl

Issue 33 further impl

README.md

@@ -9,6 +9,20 @@ Implemented in go.
 Command line arguments
 ```-config <path-to-config-file> [-migrate-database]```
 
+## Configuration File
+
+There is a template configuration file under `docs/config.example.yaml`. Copy it to `config.yaml` in the service
+root (or wherever your `-config` argument points), and edit it to match your requirements.
+
+The sensitive values in the configuration file can also be specified via environment variables, so they can be
+configured using a kubernetes secret or vault integration. If set, the environment variables override any
+values in the configuration file, which are then allowed to be missing or empty.
+
+| Environment variable   | Overrides configuration value |
+|------------------------|-------------------------------|
+| REG_SECRET_DB_PASSWORD | database.password             |
+| REG_SECRET_API_TOKEN   | database.password             |
+
 ## Installation
 
 This service uses go modules to provide dependency management, see `go.mod`.

api/openapi-spec/openapi.yaml

@@ -71,25 +71,15 @@ paths:
       description: |-
         Obtain a list of all or selected groups, including their members.       
         
-        Admin or Api Key authorization: can see all groups if the "show"
-        query parameter is set to "all".
+        Admin or Api Key authorization: can see all groups.
         
-        User authorization will only show groups visible to the current user, and only allows
-        "show" value "public".
         Normal users: can only see groups visible to them. If public groups are enabled in configuration,
         this means all groups that are public, not full, and from which the user wasn't banned. Not all fields
         will be filled.
         
         Note: both admins and normal users can always use the findMyGroup operation to get the group they are in.
       operationId: listGroups
       parameters:
-        - name: show
-          in: query
-          description: set to "all" to request admin access rather than normal visibility rules. If permissions aren't sufficient, will result in a 403. Set to "public" to request only groups with flag "public".
-          schema:
-            type: string
-            example: public
-            default: public
         - name: member_ids
           in: query
           description: a comma separated list of badge numbers. The result will be limited to all groups that contain at least one of these badge numbers as members. Only admins can set this parameter, it is ignored for normal attendees to avoid leaking information.
@@ -271,7 +261,7 @@ paths:
       tags:
         - groups
       summary: obtain a group by uuid
-      description: Returns a single group
+      description: Returns a single group. You must be a member of the group or an admin in order to have access.
       operationId: getGroupById
       parameters:
         - name: uuid
@@ -435,12 +425,6 @@ paths:
             application/json:
               schema:
                 $ref: '#/components/schemas/Error'
-        '409':
-          description: Group still contains members other than the owner. Must remove them first to ensure proper notifications.
-          content:
-            application/json:
-              schema:
-                $ref: '#/components/schemas/Error'
         '500':
           description: An unexpected error occurred. This includes database errors. A best effort attempt is made to return details in the body.
           content:
@@ -568,7 +552,7 @@ paths:
               schema:
                 $ref: '#/components/schemas/Error'
         '409':
-          description: Duplicate assignment, this attendee is already in another group, or has been individually assigned to a room already.
+          description: Duplicate assignment, this attendee is already in another group.
           content:
             application/json:
               schema:
@@ -1161,9 +1145,7 @@ paths:
         - rooms
       summary: add a whole group
       description: |-
-        Adds a group to a room.
-        
-        This locks the group against membership changes done by regular attendees. Admins may still change group membership.
+        Adds a group to a room. This is just a convenience function.
         
         Admin only.
       operationId: addGroupToRoom
@@ -1229,7 +1211,9 @@ paths:
         - rooms
       summary: remove a whole group
       description: |-
-        Removes the group from the room.
+        Removes the group from the room. This is just a convenience function.
+        
+        If some members of the group are not in the room, their room assignments are left untouched.
         
         Admin only.
       operationId: removeGroupFromRoom
@@ -1395,7 +1379,7 @@ components:
           example: 6
         owner:
           type: integer
-          description: the badge number of the group owner. Must be a member of the group. If you are not an admin, you can only create groups with yourself as owner.
+          description: the badge number of the group owner. Must be a member of the group. If you are not an admin, you can only create groups with yourself as owner. When changing group owners, the current owner can assign any of the other members to become group owner.
         members:
           type: array
           description: the current group members. READ ONLY, provided for ease of use of the API, but completely ignored in all write requests. Please use the relevant subresource API endpoints to manipulate group membership.

docs/config.example.yaml

@@ -1,8 +1,20 @@
 service:
-  attendee_service_url: 'http://localhost:9091' # no trailing slash
+  attendee_service_url: 'http://localhost:9091' # base url to attendee service - no trailing slash
+  # the maximum size of room groups that attendees can form
+  #
+  # Should usually be set to the largest available room size, unless you have just one room that is much larger
+  # than all the others, then it may make sense to limit the group size to more typical room sizes.
   max_group_size: 6
+  # allowed flags for groups.
+  #
+  # Flag "public" means a group is visible to approved attendees, who can then request to join it. If you
+  # do not allow this flag here, no public groups will be supported.
   group_flags:
     - public
+  # allowed flags for rooms.
+  #
+  # This service does not react to the flags, but UIs and exports may rely on presence of certain flags to
+  # function correctly.
   room_flags:
     - handicapped
     - final
@@ -11,7 +23,7 @@ server:
 database:
   use: 'mysql' # or inmemory
   username: 'demouser'
-  password: 'demopw'
+  password: 'demopw' # can also leave blank and set REG_SECRET_DB_PASSWORD
   database: 'tcp(localhost:3306)/dbname'
   parameters:
     - 'charset=utf8mb4'
@@ -21,6 +33,8 @@ database:
 security:
   cors:
     disable: false
+  fixed_token:
+    api: 'put_secure_random_string_here_for_api_token' # can also leave unset and set REG_SECRET_API_TOKEN
   oidc:
     id_token_cookie_name: JWT
     access_token_cookie_name: AUTH
@@ -37,8 +51,10 @@ security:
         mwIDAQAB
         -----END PUBLIC KEY-----
 logging:
-  style: ecs
+  style: ecs # or plain
   severity: INFO
+# this section is currently unused. It was used by the old email style hotel booking, see
+# https://github.com/eurofurence/reg-hotel-booking
 go_live:
   public:
     start_iso_datetime: 1995-06-30T11:11:11+02:00

internal/api/v1/apimodel.go

@@ -5,8 +5,8 @@ import "time"
 var _ = time.Now
 
 type Error struct {
-	// The time at which the error occurred.
-	Timestamp time.Time `json:"timestamp"`
+	// The time at which the error occurred, formatted as ISO datetime according to spec.
+	Timestamp string `json:"timestamp"`
 	// An internal trace id assigned to the error. Used to find logs associated with errors across our services. Display to the user as something to communicate to us with inquiries about the error.
 	Requestid string `json:"requestid"`
 	// A keyed description of the error. We do not write human readable text here because the user interface will be multi language.  At this time, there are these values: - auth.unauthorized (token missing completely or invalid) - auth.forbidden (permissions missing)
@@ -25,9 +25,9 @@ type Group struct {
 	// Optional comments the owner wishes to make regarding the group. Not processed in any way.
 	Comments *string `yaml:"comments,omitempty" json:"comments,omitempty"`
 	// if set higher than 0 (the default), will limit the number of people that can join the group. Note that there is also a configuration item that globally limits the size of groups, e.g. to the maximum room size.
-	MaximumSize *int32 `yaml:"maximum_size,omitempty" json:"maximum_size,omitempty"`
+	MaximumSize int64 `yaml:"maximum_size" json:"maximum_size"`
 	// the badge number of the group owner. Must be a member of the group. If you are not an admin, you can only create groups with yourself as owner.
-	Owner int32 `yaml:"owner" json:"owner"`
+	Owner int64 `yaml:"owner" json:"owner"`
 	// the current group members. READ ONLY, provided for ease of use of the API, but completely ignored in all write requests. Please use the relevant subresource API endpoints to manipulate group membership.
 	Members []Member `yaml:"members,omitempty" json:"members,omitempty"`
 	// the current outstanding invites for this group. READ ONLY, provided for ease of use of the API, but completely ignored in all write requests. Please use the relevant subresource API endpoints to send/revoke invites.
@@ -41,8 +41,10 @@ type GroupCreate struct {
 	Flags []string `yaml:"flags,omitempty" json:"flags,omitempty"`
 	// Optional comments the owner wishes to make regarding the group. Not processed in any way.
 	Comments *string `yaml:"comments,omitempty" json:"comments,omitempty"`
+	// if set higher than 0 (the default), will limit the number of people that can join the group. Note that there is also a configuration item that globally limits the size of groups, e.g. to the maximum room size.
+	MaximumSize int64 `yaml:"maximum_size" json:"maximum_size"`
 	// the badge number of the group owner. If you are not an admin, you can only create groups with yourself as owner. Defaults to yourself.
-	Owner int32 `yaml:"owner" json:"owner"`
+	Owner int64 `yaml:"owner" json:"owner"`
 }
 
 type GroupList struct {
@@ -51,7 +53,7 @@ type GroupList struct {
 
 type Member struct {
 	// badge number (id in the attendee service).
-	ID int32 `yaml:"id" json:"id"`
+	ID int64 `yaml:"id" json:"id"`
 	// The nickname of the attendee, proxied from that attendee service.
 	Nickname string `yaml:"nickname" json:"nickname"`
 	// A url to obtain the avatar for this attendee, points to an image such as a png or jpg. May require the same authentication this API expects.
@@ -70,7 +72,7 @@ type Room struct {
 	// Optional comment. Not processed in any way.
 	Comments *string `yaml:"comments,omitempty" json:"comments,omitempty"`
 	// the maximum room size, usually the number of sleeping spots/beds in the room.
-	Size int32 `yaml:"size" json:"size"`
+	Size int64 `yaml:"size" json:"size"`
 	// the assigned room members. READ ONLY, provided for ease of use of the API, but completely ignored in all write requests. Please use the relevant subresource API endpoints to manipulate individual or group assignments.
 	Members []Member `yaml:"members,omitempty" json:"members,omitempty"`
 }

internal/application/app/application.go

@@ -42,6 +42,18 @@ func (a *Application) Run() error {
 		aulogging.ErrorErrf(ctx, err, "failed to load configuration - bailing out: %s", err.Error())
 		return err
 	}
+	aulogging.Info(ctx, "configuration file successfully loaded")
+
+	aulogging.Info(ctx, "adding configuration defaults")
+	conf.AddDefaults()
+	aulogging.Info(ctx, "applying environment variable overrides")
+	conf.ApplyEnvironmentOverrides()
+	aulogging.Info(ctx, "validating configuration")
+	err = conf.Validate()
+	if err != nil {
+		aulogging.ErrorErrf(ctx, err, "failed to validate configuration - bailing out: %s", err.Error())
+		return err
+	}
 
 	if conf.Database.Use == config.Mysql {
 		connectString := dbrepo.MysqlConnectString(conf.Database.Username, conf.Database.Password, conf.Database.Database, conf.Database.Parameters)

internal/application/common/errors.go

@@ -15,6 +15,7 @@ type APIError interface {
 	error
 	Status() int
 	Response() modelsv1.Error
+	InternalCauses() []error // not for sending to the client, but useful for logging
 }
 
 // ErrorMessageCode is a key to use for error messages in frontends or other automated systems interacting
@@ -44,32 +45,32 @@ const (
 
 // construct specific API errors
 
-func NewBadRequest(ctx context.Context, message ErrorMessageCode, details url.Values) APIError {
-	return NewAPIError(ctx, http.StatusBadRequest, message, details)
+func NewBadRequest(ctx context.Context, message ErrorMessageCode, details url.Values, internalCauses ...error) APIError {
+	return NewAPIError(ctx, http.StatusBadRequest, message, details, internalCauses...)
 }
 
-func NewUnauthorized(ctx context.Context, message ErrorMessageCode, details url.Values) APIError {
-	return NewAPIError(ctx, http.StatusUnauthorized, message, details)
+func NewUnauthorized(ctx context.Context, message ErrorMessageCode, details url.Values, internalCauses ...error) APIError {
+	return NewAPIError(ctx, http.StatusUnauthorized, message, details, internalCauses...)
 }
 
-func NewForbidden(ctx context.Context, message ErrorMessageCode, details url.Values) APIError {
-	return NewAPIError(ctx, http.StatusForbidden, message, details)
+func NewForbidden(ctx context.Context, message ErrorMessageCode, details url.Values, internalCauses ...error) APIError {
+	return NewAPIError(ctx, http.StatusForbidden, message, details, internalCauses...)
 }
 
-func NewNotFound(ctx context.Context, message ErrorMessageCode, details url.Values) APIError {
-	return NewAPIError(ctx, http.StatusNotFound, message, details)
+func NewNotFound(ctx context.Context, message ErrorMessageCode, details url.Values, internalCauses ...error) APIError {
+	return NewAPIError(ctx, http.StatusNotFound, message, details, internalCauses...)
 }
 
-func NewConflict(ctx context.Context, message ErrorMessageCode, details url.Values) APIError {
-	return NewAPIError(ctx, http.StatusConflict, message, details)
+func NewConflict(ctx context.Context, message ErrorMessageCode, details url.Values, internalCauses ...error) APIError {
+	return NewAPIError(ctx, http.StatusConflict, message, details, internalCauses...)
 }
 
-func NewInternalServerError(ctx context.Context, message ErrorMessageCode, details url.Values) APIError {
-	return NewAPIError(ctx, http.StatusInternalServerError, message, details)
+func NewInternalServerError(ctx context.Context, message ErrorMessageCode, details url.Values, internalCauses ...error) APIError {
+	return NewAPIError(ctx, http.StatusInternalServerError, message, details, internalCauses...)
 }
 
-func NewBadGateway(ctx context.Context, message ErrorMessageCode, details url.Values) APIError {
-	return NewAPIError(ctx, http.StatusBadGateway, message, details)
+func NewBadGateway(ctx context.Context, message ErrorMessageCode, details url.Values, internalCauses ...error) APIError {
+	return NewAPIError(ctx, http.StatusBadGateway, message, details, internalCauses...)
 }
 
 // check for API errors
@@ -107,24 +108,29 @@ func IsAPIError(err error) bool {
 	return ok
 }
 
+const isoDateTimeFormat = "2006-01-02T15:04:05-07:00"
+
 // NewAPIError creates a generic API error from directly provided information.
-func NewAPIError(ctx context.Context, status int, message ErrorMessageCode, details url.Values) APIError {
+func NewAPIError(ctx context.Context, status int, message ErrorMessageCode, details url.Values, internalCauses ...error) APIError {
+
 	return &StatusError{
 		errStatus: status,
 		response: modelsv1.Error{
-			Timestamp: time.Now(),
+			Timestamp: time.Now().Format(isoDateTimeFormat),
 			Requestid: GetRequestID(ctx),
 			Message:   string(message),
 			Details:   details,
 		},
+		internalCauses: internalCauses,
 	}
 }
 
 var _ error = (*StatusError)(nil)
 
 type StatusError struct {
-	errStatus int
-	response  modelsv1.Error
+	errStatus      int
+	response       modelsv1.Error
+	internalCauses []error
 }
 
 func (se *StatusError) Error() string {
@@ -139,6 +145,10 @@ func (se *StatusError) Response() modelsv1.Error {
 	return se.response
 }
 
+func (se *StatusError) InternalCauses() []error {
+	return se.internalCauses
+}
+
 func isAPIErrorWithStatus(status int, err error) bool {
 	apiError, ok := err.(APIError)
 	return ok && status == apiError.Status()

internal/application/web/endpoint.go

@@ -13,7 +13,8 @@ type (
 	Endpoint[Req, Res any]   func(ctx context.Context, request *Req, w http.ResponseWriter) (*Res, error)
 )
 
-func CreateHandler[Req, Res any](endpoint Endpoint[Req, Res],
+func CreateHandler[Req, Res any](
+	endpoint Endpoint[Req, Res],
 	requestHandler RequestHandler[Req],
 	responseHandler ResponseHandler[Res],
 ) http.Handler {
@@ -36,24 +37,25 @@ func CreateHandler[Req, Res any](endpoint Endpoint[Req, Res],
 		defer func() {
 			err := r.Body.Close()
 			if err != nil {
-				aulogging.ErrorErrf(ctx, err, "Error when closing the request body. [error]: %v", err)
+				aulogging.WarnErrf(ctx, err, "error while closing the request body: %v", err)
 			}
 		}()
 
 		request, err := requestHandler(r, w)
 		if err != nil {
-			aulogging.ErrorErrf(ctx, err, "An error occurred while parsing the request. [error]: %v", err)
+			SendErrorResponse(ctx, w, err)
 			return
 		}
 
 		response, err := endpoint(ctx, request, w)
 		if err != nil {
-			aulogging.ErrorErrf(ctx, err, "An error occurred during the request. [error]: %v", err)
+			SendErrorResponse(ctx, w, err)
 			return
 		}
 
 		if err := responseHandler(ctx, response, w); err != nil {
-			aulogging.ErrorErrf(ctx, err, "An error occurred during the handling of the response. [error]: %v", err)
+			// cannot SendErrorResponse(ctx, w, err) - likely already have started sending in responseHandler
+			aulogging.ErrorErrf(ctx, err, "An error occurred during the handling of the response - response may have been incomplete. [error]: %v", err)
 		}
 	})
 }

internal/application/web/response.go

@@ -3,6 +3,7 @@ package web
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 	aulogging "github.com/StephanHCB/go-autumn-logging"
 	"github.com/eurofurence/reg-room-service/internal/application/common"
 	"net/http"
@@ -45,6 +46,11 @@ func SendErrorResponse(ctx context.Context, w http.ResponseWriter, err error) {
 // which contains relevant information about the failed request to the client.
 // The function will also set the http status according to the provided status.
 func SendAPIErrorResponse(ctx context.Context, w http.ResponseWriter, apiErr common.APIError) {
+	aulogging.InfoErrf(ctx, apiErr, fmt.Sprintf("api response status %d: %v", apiErr.Status(), apiErr))
+	for _, cause := range apiErr.InternalCauses() {
+		aulogging.InfoErrf(ctx, cause, fmt.Sprintf("... caused by: %v", cause))
+	}
+
 	w.WriteHeader(apiErr.Status())
 
 	EncodeToJSON(ctx, w, apiErr.Response())