From 9973aa9ffa47586985a5736a3ea1f32641555862 Mon Sep 17 00:00:00 2001 From: Jan Eitzinger Date: Thu, 20 Nov 2025 07:48:45 +0100 Subject: [PATCH] Refactor api package --- internal/api/job.go | 129 ++++++++++++++++++++++-------------- internal/api/memorystore.go | 7 -- internal/api/rest.go | 123 ++++++++++++++++++++++------------ internal/api/user.go | 106 ++++++++++++++++++++++------- 4 files changed, 243 insertions(+), 122 deletions(-) diff --git a/internal/api/job.go b/internal/api/job.go index 2ce2a3a..3ec32ae 100644 --- a/internal/api/job.go +++ b/internal/api/job.go @@ -29,6 +29,12 @@ import ( "github.com/gorilla/mux" ) +const ( + // secondsPerDay is the number of seconds in 24 hours. + // Used for duplicate job detection within a day window. + secondsPerDay = 86400 +) + // StopJobApiRequest model type StopJobApiRequest struct { JobId *int64 `json:"jobId" example:"123000"` @@ -113,7 +119,8 @@ func (api *RestApi) getJobs(rw http.ResponseWriter, r *http.Request) { for key, vals := range r.URL.Query() { switch key { - // TODO: add project filter + case "project": + filter.Project = &model.StringInput{Eq: &vals[0]} case "state": for _, s := range vals { state := schema.JobState(s) @@ -363,7 +370,7 @@ func (api *RestApi) getJobById(rw http.ResponseWriter, r *http.Request) { var metrics GetJobApiRequest if err = decode(r.Body, &metrics); err != nil { - http.Error(rw, err.Error(), http.StatusBadRequest) + handleError(fmt.Errorf("decoding request failed: %w", err), http.StatusBadRequest, rw) return } @@ -434,30 +441,32 @@ func (api *RestApi) getJobById(rw http.ResponseWriter, r *http.Request) { func (api *RestApi) editMeta(rw http.ResponseWriter, r *http.Request) { id, err := strconv.ParseInt(mux.Vars(r)["id"], 10, 64) if err != nil { - http.Error(rw, err.Error(), http.StatusBadRequest) + handleError(fmt.Errorf("parsing job ID failed: %w", err), http.StatusBadRequest, rw) return } job, err := api.JobRepository.FindById(r.Context(), id) if err != nil { - http.Error(rw, err.Error(), http.StatusNotFound) + handleError(fmt.Errorf("finding job failed: %w", err), http.StatusNotFound, rw) return } var req EditMetaRequest if err := decode(r.Body, &req); err != nil { - http.Error(rw, err.Error(), http.StatusBadRequest) + handleError(fmt.Errorf("decoding request failed: %w", err), http.StatusBadRequest, rw) return } if err := api.JobRepository.UpdateMetadata(job, req.Key, req.Value); err != nil { - http.Error(rw, err.Error(), http.StatusInternalServerError) + handleError(fmt.Errorf("updating metadata failed: %w", err), http.StatusInternalServerError, rw) return } rw.Header().Add("Content-Type", "application/json") rw.WriteHeader(http.StatusOK) - json.NewEncoder(rw).Encode(job) + if err := json.NewEncoder(rw).Encode(job); err != nil { + cclog.Errorf("Failed to encode job response: %v", err) + } } // tagJob godoc @@ -480,32 +489,32 @@ func (api *RestApi) editMeta(rw http.ResponseWriter, r *http.Request) { func (api *RestApi) tagJob(rw http.ResponseWriter, r *http.Request) { id, err := strconv.ParseInt(mux.Vars(r)["id"], 10, 64) if err != nil { - http.Error(rw, err.Error(), http.StatusBadRequest) + handleError(fmt.Errorf("parsing job ID failed: %w", err), http.StatusBadRequest, rw) return } job, err := api.JobRepository.FindById(r.Context(), id) if err != nil { - http.Error(rw, err.Error(), http.StatusNotFound) + handleError(fmt.Errorf("finding job failed: %w", err), http.StatusNotFound, rw) return } job.Tags, err = api.JobRepository.GetTags(repository.GetUserFromContext(r.Context()), job.ID) if err != nil { - http.Error(rw, err.Error(), http.StatusInternalServerError) + handleError(fmt.Errorf("getting tags failed: %w", err), http.StatusInternalServerError, rw) return } var req TagJobApiRequest if err := decode(r.Body, &req); err != nil { - http.Error(rw, err.Error(), http.StatusBadRequest) + handleError(fmt.Errorf("decoding request failed: %w", err), http.StatusBadRequest, rw) return } for _, tag := range req { tagId, err := api.JobRepository.AddTagOrCreate(repository.GetUserFromContext(r.Context()), *job.ID, tag.Type, tag.Name, tag.Scope) if err != nil { - http.Error(rw, err.Error(), http.StatusInternalServerError) + handleError(fmt.Errorf("adding tag failed: %w", err), http.StatusInternalServerError, rw) return } @@ -519,7 +528,9 @@ func (api *RestApi) tagJob(rw http.ResponseWriter, r *http.Request) { rw.Header().Add("Content-Type", "application/json") rw.WriteHeader(http.StatusOK) - json.NewEncoder(rw).Encode(job) + if err := json.NewEncoder(rw).Encode(job); err != nil { + cclog.Errorf("Failed to encode job response: %v", err) + } } // removeTagJob godoc @@ -542,25 +553,25 @@ func (api *RestApi) tagJob(rw http.ResponseWriter, r *http.Request) { func (api *RestApi) removeTagJob(rw http.ResponseWriter, r *http.Request) { id, err := strconv.ParseInt(mux.Vars(r)["id"], 10, 64) if err != nil { - http.Error(rw, err.Error(), http.StatusBadRequest) + handleError(fmt.Errorf("parsing job ID failed: %w", err), http.StatusBadRequest, rw) return } job, err := api.JobRepository.FindById(r.Context(), id) if err != nil { - http.Error(rw, err.Error(), http.StatusNotFound) + handleError(fmt.Errorf("finding job failed: %w", err), http.StatusNotFound, rw) return } job.Tags, err = api.JobRepository.GetTags(repository.GetUserFromContext(r.Context()), job.ID) if err != nil { - http.Error(rw, err.Error(), http.StatusInternalServerError) + handleError(fmt.Errorf("getting tags failed: %w", err), http.StatusInternalServerError, rw) return } var req TagJobApiRequest if err := decode(r.Body, &req); err != nil { - http.Error(rw, err.Error(), http.StatusBadRequest) + handleError(fmt.Errorf("decoding request failed: %w", err), http.StatusBadRequest, rw) return } @@ -573,7 +584,7 @@ func (api *RestApi) removeTagJob(rw http.ResponseWriter, r *http.Request) { remainingTags, err := api.JobRepository.RemoveJobTagByRequest(repository.GetUserFromContext(r.Context()), *job.ID, rtag.Type, rtag.Name, rtag.Scope) if err != nil { - http.Error(rw, err.Error(), http.StatusInternalServerError) + handleError(fmt.Errorf("removing tag failed: %w", err), http.StatusInternalServerError, rw) return } @@ -582,7 +593,9 @@ func (api *RestApi) removeTagJob(rw http.ResponseWriter, r *http.Request) { rw.Header().Add("Content-Type", "application/json") rw.WriteHeader(http.StatusOK) - json.NewEncoder(rw).Encode(job) + if err := json.NewEncoder(rw).Encode(job); err != nil { + cclog.Errorf("Failed to encode job response: %v", err) + } } // removeTags godoc @@ -604,7 +617,7 @@ func (api *RestApi) removeTagJob(rw http.ResponseWriter, r *http.Request) { func (api *RestApi) removeTags(rw http.ResponseWriter, r *http.Request) { var req TagJobApiRequest if err := decode(r.Body, &req); err != nil { - http.Error(rw, err.Error(), http.StatusBadRequest) + handleError(fmt.Errorf("decoding request failed: %w", err), http.StatusBadRequest, rw) return } @@ -619,11 +632,10 @@ func (api *RestApi) removeTags(rw http.ResponseWriter, r *http.Request) { err := api.JobRepository.RemoveTagByRequest(rtag.Type, rtag.Name, rtag.Scope) if err != nil { - http.Error(rw, err.Error(), http.StatusInternalServerError) + handleError(fmt.Errorf("removing tag failed: %w", err), http.StatusInternalServerError, rw) return - } else { - currentCount++ } + currentCount++ } rw.WriteHeader(http.StatusOK) @@ -674,9 +686,11 @@ func (api *RestApi) startJob(rw http.ResponseWriter, r *http.Request) { if err != nil && err != sql.ErrNoRows { handleError(fmt.Errorf("checking for duplicate failed: %w", err), http.StatusInternalServerError, rw) return - } else if err == nil { + } + if err == nil { for _, job := range jobs { - if (req.StartTime - job.StartTime) < 86400 { + // Check if jobs are within the same day (prevent duplicates) + if (req.StartTime - job.StartTime) < secondsPerDay { handleError(fmt.Errorf("a job with that jobId, cluster and startTime already exists: dbid: %d, jobid: %d", job.ID, job.JobID), http.StatusUnprocessableEntity, rw) return } @@ -693,7 +707,6 @@ func (api *RestApi) startJob(rw http.ResponseWriter, r *http.Request) { for _, tag := range req.Tags { if _, err := api.JobRepository.AddTagOrCreate(repository.GetUserFromContext(r.Context()), id, tag.Type, tag.Name, tag.Scope); err != nil { - http.Error(rw, err.Error(), http.StatusInternalServerError) handleError(fmt.Errorf("adding tag to new job %d failed: %w", id, err), http.StatusInternalServerError, rw) return } @@ -702,9 +715,11 @@ func (api *RestApi) startJob(rw http.ResponseWriter, r *http.Request) { cclog.Printf("new job (id: %d): cluster=%s, jobId=%d, user=%s, startTime=%d", id, req.Cluster, req.JobID, req.User, req.StartTime) rw.Header().Add("Content-Type", "application/json") rw.WriteHeader(http.StatusCreated) - json.NewEncoder(rw).Encode(DefaultApiResponse{ + if err := json.NewEncoder(rw).Encode(DefaultApiResponse{ Message: "success", - }) + }); err != nil { + cclog.Errorf("Failed to encode response: %v", err) + } } // stopJobByRequest godoc @@ -742,12 +757,14 @@ func (api *RestApi) stopJobByRequest(rw http.ResponseWriter, r *http.Request) { // cclog.Printf("loading db job for stopJobByRequest... : stopJobApiRequest=%v", req) job, err = api.JobRepository.Find(req.JobId, req.Cluster, req.StartTime) if err != nil { - job, err = api.JobRepository.FindCached(req.JobId, req.Cluster, req.StartTime) - // FIXME: Previous error is hidden - if err != nil { - handleError(fmt.Errorf("finding job failed: %w", err), http.StatusUnprocessableEntity, rw) + // Try cached jobs if not found in main repository + cachedJob, cachedErr := api.JobRepository.FindCached(req.JobId, req.Cluster, req.StartTime) + if cachedErr != nil { + // Combine both errors for better debugging + handleError(fmt.Errorf("finding job failed: %w (cached lookup also failed: %v)", err, cachedErr), http.StatusNotFound, rw) return } + job = cachedJob } api.checkAndHandleStopJob(rw, job, req) @@ -790,9 +807,11 @@ func (api *RestApi) deleteJobById(rw http.ResponseWriter, r *http.Request) { } rw.Header().Add("Content-Type", "application/json") rw.WriteHeader(http.StatusOK) - json.NewEncoder(rw).Encode(DefaultApiResponse{ + if err := json.NewEncoder(rw).Encode(DefaultApiResponse{ Message: fmt.Sprintf("Successfully deleted job %s", id), - }) + }); err != nil { + cclog.Errorf("Failed to encode response: %v", err) + } } // deleteJobByRequest godoc @@ -841,9 +860,11 @@ func (api *RestApi) deleteJobByRequest(rw http.ResponseWriter, r *http.Request) rw.Header().Add("Content-Type", "application/json") rw.WriteHeader(http.StatusOK) - json.NewEncoder(rw).Encode(DefaultApiResponse{ + if err := json.NewEncoder(rw).Encode(DefaultApiResponse{ Message: fmt.Sprintf("Successfully deleted job %d", job.ID), - }) + }); err != nil { + cclog.Errorf("Failed to encode response: %v", err) + } } // deleteJobBefore godoc @@ -885,9 +906,11 @@ func (api *RestApi) deleteJobBefore(rw http.ResponseWriter, r *http.Request) { rw.Header().Add("Content-Type", "application/json") rw.WriteHeader(http.StatusOK) - json.NewEncoder(rw).Encode(DefaultApiResponse{ + if err := json.NewEncoder(rw).Encode(DefaultApiResponse{ Message: fmt.Sprintf("Successfully deleted %d jobs", cnt), - }) + }); err != nil { + cclog.Errorf("Failed to encode response: %v", err) + } } func (api *RestApi) checkAndHandleStopJob(rw http.ResponseWriter, job *schema.Job, req StopJobApiRequest) { @@ -897,7 +920,7 @@ func (api *RestApi) checkAndHandleStopJob(rw http.ResponseWriter, job *schema.Jo return } - if job == nil || job.StartTime > req.StopTime { + if job.StartTime > req.StopTime { handleError(fmt.Errorf("jobId %d (id %d) on %s : stopTime %d must be larger/equal than startTime %d", job.JobID, job.ID, job.Cluster, req.StopTime, job.StartTime), http.StatusBadRequest, rw) return } @@ -913,14 +936,14 @@ func (api *RestApi) checkAndHandleStopJob(rw http.ResponseWriter, job *schema.Jo job.Duration = int32(req.StopTime - job.StartTime) job.State = req.State api.JobRepository.Mutex.Lock() + defer api.JobRepository.Mutex.Unlock() + if err := api.JobRepository.Stop(*job.ID, job.Duration, job.State, job.MonitoringStatus); err != nil { if err := api.JobRepository.StopCached(*job.ID, job.Duration, job.State, job.MonitoringStatus); err != nil { - api.JobRepository.Mutex.Unlock() handleError(fmt.Errorf("jobId %d (id %d) on %s : marking job as '%s' (duration: %d) in DB failed: %w", job.JobID, job.ID, job.Cluster, job.State, job.Duration, err), http.StatusInternalServerError, rw) return } } - api.JobRepository.Mutex.Unlock() cclog.Printf("archiving job... (dbid: %d): cluster=%s, jobId=%d, user=%s, startTime=%d, duration=%d, state=%s", job.ID, job.Cluster, job.JobID, job.User, job.StartTime, job.Duration, job.State) @@ -929,7 +952,9 @@ func (api *RestApi) checkAndHandleStopJob(rw http.ResponseWriter, job *schema.Jo // writing to the filesystem fails, the client will not know. rw.Header().Add("Content-Type", "application/json") rw.WriteHeader(http.StatusOK) - json.NewEncoder(rw).Encode(job) + if err := json.NewEncoder(rw).Encode(job); err != nil { + cclog.Errorf("Failed to encode job response: %v", err) + } // Monitoring is disabled... if job.MonitoringStatus == schema.MonitoringStatusDisabled { @@ -947,7 +972,7 @@ func (api *RestApi) getJobMetrics(rw http.ResponseWriter, r *http.Request) { for _, scope := range r.URL.Query()["scope"] { var s schema.MetricScope if err := s.UnmarshalGQL(scope); err != nil { - http.Error(rw, err.Error(), http.StatusBadRequest) + handleError(fmt.Errorf("unmarshaling scope failed: %w", err), http.StatusBadRequest, rw) return } scopes = append(scopes, s) @@ -956,7 +981,7 @@ func (api *RestApi) getJobMetrics(rw http.ResponseWriter, r *http.Request) { rw.Header().Add("Content-Type", "application/json") rw.WriteHeader(http.StatusOK) - type Respone struct { + type Response struct { Data *struct { JobMetrics []*model.JobMetricWithName `json:"jobMetrics"` } `json:"data"` @@ -968,17 +993,21 @@ func (api *RestApi) getJobMetrics(rw http.ResponseWriter, r *http.Request) { resolver := graph.GetResolverInstance() data, err := resolver.Query().JobMetrics(r.Context(), id, metrics, scopes, nil) if err != nil { - json.NewEncoder(rw).Encode(Respone{ + if err := json.NewEncoder(rw).Encode(Response{ Error: &struct { - Message string "json:\"message\"" + Message string `json:"message"` }{Message: err.Error()}, - }) + }); err != nil { + cclog.Errorf("Failed to encode error response: %v", err) + } return } - json.NewEncoder(rw).Encode(Respone{ + if err := json.NewEncoder(rw).Encode(Response{ Data: &struct { - JobMetrics []*model.JobMetricWithName "json:\"jobMetrics\"" + JobMetrics []*model.JobMetricWithName `json:"jobMetrics"` }{JobMetrics: data}, - }) + }); err != nil { + cclog.Errorf("Failed to encode response: %v", err) + } } diff --git a/internal/api/memorystore.go b/internal/api/memorystore.go index 4d598ee..1b88379 100644 --- a/internal/api/memorystore.go +++ b/internal/api/memorystore.go @@ -50,13 +50,6 @@ func freeMetrics(rw http.ResponseWriter, r *http.Request) { return } - // // TODO: lastCheckpoint might be modified by different go-routines. - // // Load it using the sync/atomic package? - // freeUpTo := lastCheckpoint.Unix() - // if to < freeUpTo { - // freeUpTo = to - // } - bodyDec := json.NewDecoder(r.Body) var selectors [][]string err = bodyDec.Decode(&selectors) diff --git a/internal/api/rest.go b/internal/api/rest.go index 907e737..dcc4b2d 100644 --- a/internal/api/rest.go +++ b/internal/api/rest.go @@ -2,6 +2,11 @@ // All rights reserved. This file is part of cc-backend. // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file. + +// Package api provides the REST API layer for ClusterCockpit. +// It handles HTTP requests for job management, user administration, +// cluster queries, node state updates, and metrics storage operations. +// The API supports both JWT token authentication and session-based authentication. package api import ( @@ -11,6 +16,7 @@ import ( "net/http" "os" "path/filepath" + "strings" "sync" "github.com/ClusterCockpit/cc-backend/internal/auth" @@ -39,10 +45,19 @@ import ( // @in header // @name X-Auth-Token +const ( + noticeFilePath = "./var/notice.txt" + noticeFilePerms = 0o644 +) + type RestApi struct { JobRepository *repository.JobRepository Authentication *auth.Authentication MachineStateDir string + // RepositoryMutex protects job creation operations from race conditions + // when checking for duplicate jobs during startJob API calls. + // It prevents concurrent job starts with the same jobId/cluster/startTime + // from creating duplicate entries in the database. RepositoryMutex sync.Mutex } @@ -66,7 +81,6 @@ func (api *RestApi) MountApiRoutes(r *mux.Router) { // Job Handler r.HandleFunc("/jobs/start_job/", api.startJob).Methods(http.MethodPost, http.MethodPut) r.HandleFunc("/jobs/stop_job/", api.stopJobByRequest).Methods(http.MethodPost, http.MethodPut) - // r.HandleFunc("/jobs/import/", api.importJob).Methods(http.MethodPost, http.MethodPut) r.HandleFunc("/jobs/", api.getJobs).Methods(http.MethodGet) r.HandleFunc("/jobs/{id}", api.getJobById).Methods(http.MethodPost) r.HandleFunc("/jobs/{id}", api.getCompleteJobById).Methods(http.MethodGet) @@ -97,15 +111,11 @@ func (api *RestApi) MountUserApiRoutes(r *mux.Router) { func (api *RestApi) MountMetricStoreApiRoutes(r *mux.Router) { // REST API Uses TokenAuth + // Note: StrictSlash handles trailing slash variations automatically r.HandleFunc("/api/free", freeMetrics).Methods(http.MethodPost) r.HandleFunc("/api/write", writeMetrics).Methods(http.MethodPost) r.HandleFunc("/api/debug", debugMetrics).Methods(http.MethodGet) r.HandleFunc("/api/healthcheck", metricsHealth).Methods(http.MethodGet) - // Same endpoints but with trailing slash - r.HandleFunc("/api/free/", freeMetrics).Methods(http.MethodPost) - r.HandleFunc("/api/write/", writeMetrics).Methods(http.MethodPost) - r.HandleFunc("/api/debug/", debugMetrics).Methods(http.MethodGet) - r.HandleFunc("/api/healthcheck/", metricsHealth).Methods(http.MethodGet) } func (api *RestApi) MountConfigApiRoutes(r *mux.Router) { @@ -146,10 +156,12 @@ func handleError(err error, statusCode int, rw http.ResponseWriter) { cclog.Warnf("REST ERROR : %s", err.Error()) rw.Header().Add("Content-Type", "application/json") rw.WriteHeader(statusCode) - json.NewEncoder(rw).Encode(ErrorResponse{ + if err := json.NewEncoder(rw).Encode(ErrorResponse{ Status: http.StatusText(statusCode), Error: err.Error(), - }) + }); err != nil { + cclog.Errorf("Failed to encode error response: %v", err) + } } func decode(r io.Reader, val any) error { @@ -162,41 +174,41 @@ func (api *RestApi) editNotice(rw http.ResponseWriter, r *http.Request) { // SecuredCheck() only worked with TokenAuth: Removed if user := repository.GetUserFromContext(r.Context()); !user.HasRole(schema.RoleAdmin) { - http.Error(rw, "Only admins are allowed to update the notice.txt file", http.StatusForbidden) + handleError(fmt.Errorf("only admins are allowed to update the notice.txt file"), http.StatusForbidden, rw) return } // Get Value newContent := r.FormValue("new-content") - // Check FIle - noticeExists := util.CheckFileExists("./var/notice.txt") + // Validate content length to prevent DoS + if len(newContent) > 10000 { + handleError(fmt.Errorf("notice content exceeds maximum length of 10000 characters"), http.StatusBadRequest, rw) + return + } + + // Check File + noticeExists := util.CheckFileExists(noticeFilePath) if !noticeExists { - ntxt, err := os.Create("./var/notice.txt") + ntxt, err := os.Create(noticeFilePath) if err != nil { - cclog.Errorf("Creating ./var/notice.txt failed: %s", err.Error()) - http.Error(rw, err.Error(), http.StatusUnprocessableEntity) + handleError(fmt.Errorf("creating notice file failed: %w", err), http.StatusInternalServerError, rw) return } ntxt.Close() } + if err := os.WriteFile(noticeFilePath, []byte(newContent), noticeFilePerms); err != nil { + handleError(fmt.Errorf("writing to notice file failed: %w", err), http.StatusInternalServerError, rw) + return + } + + rw.Header().Set("Content-Type", "text/plain") + rw.WriteHeader(http.StatusOK) if newContent != "" { - if err := os.WriteFile("./var/notice.txt", []byte(newContent), 0o666); err != nil { - cclog.Errorf("Writing to ./var/notice.txt failed: %s", err.Error()) - http.Error(rw, err.Error(), http.StatusUnprocessableEntity) - return - } else { - rw.Write([]byte("Update Notice Content Success")) - } + rw.Write([]byte("Update Notice Content Success")) } else { - if err := os.WriteFile("./var/notice.txt", []byte(""), 0o666); err != nil { - cclog.Errorf("Writing to ./var/notice.txt failed: %s", err.Error()) - http.Error(rw, err.Error(), http.StatusUnprocessableEntity) - return - } else { - rw.Write([]byte("Empty Notice Content Success")) - } + rw.Write([]byte("Empty Notice Content Success")) } } @@ -206,21 +218,20 @@ func (api *RestApi) getJWT(rw http.ResponseWriter, r *http.Request) { me := repository.GetUserFromContext(r.Context()) if !me.HasRole(schema.RoleAdmin) { if username != me.Username { - http.Error(rw, "Only admins are allowed to sign JWTs not for themselves", - http.StatusForbidden) + handleError(fmt.Errorf("only admins are allowed to sign JWTs not for themselves"), http.StatusForbidden, rw) return } } user, err := repository.GetUserRepository().GetUser(username) if err != nil { - http.Error(rw, err.Error(), http.StatusUnprocessableEntity) + handleError(fmt.Errorf("getting user failed: %w", err), http.StatusNotFound, rw) return } jwt, err := api.Authentication.JwtAuth.ProvideJWT(user) if err != nil { - http.Error(rw, err.Error(), http.StatusUnprocessableEntity) + handleError(fmt.Errorf("providing JWT failed: %w", err), http.StatusInternalServerError, rw) return } @@ -233,17 +244,20 @@ func (api *RestApi) getRoles(rw http.ResponseWriter, r *http.Request) { user := repository.GetUserFromContext(r.Context()) if !user.HasRole(schema.RoleAdmin) { - http.Error(rw, "only admins are allowed to fetch a list of roles", http.StatusForbidden) + handleError(fmt.Errorf("only admins are allowed to fetch a list of roles"), http.StatusForbidden, rw) return } roles, err := schema.GetValidRoles(user) if err != nil { - http.Error(rw, err.Error(), http.StatusInternalServerError) + handleError(fmt.Errorf("getting valid roles failed: %w", err), http.StatusInternalServerError, rw) return } - json.NewEncoder(rw).Encode(roles) + rw.Header().Set("Content-Type", "application/json") + if err := json.NewEncoder(rw).Encode(roles); err != nil { + cclog.Errorf("Failed to encode roles response: %v", err) + } } func (api *RestApi) updateConfiguration(rw http.ResponseWriter, r *http.Request) { @@ -251,38 +265,50 @@ func (api *RestApi) updateConfiguration(rw http.ResponseWriter, r *http.Request) key, value := r.FormValue("key"), r.FormValue("value") if err := repository.GetUserCfgRepo().UpdateConfig(key, value, repository.GetUserFromContext(r.Context())); err != nil { - http.Error(rw, err.Error(), http.StatusUnprocessableEntity) + handleError(fmt.Errorf("updating configuration failed: %w", err), http.StatusInternalServerError, rw) return } + rw.WriteHeader(http.StatusOK) rw.Write([]byte("success")) } func (api *RestApi) putMachineState(rw http.ResponseWriter, r *http.Request) { if api.MachineStateDir == "" { - http.Error(rw, "REST > machine state not enabled", http.StatusNotFound) + handleError(fmt.Errorf("machine state not enabled"), http.StatusNotFound, rw) return } vars := mux.Vars(r) cluster := vars["cluster"] host := vars["host"] + + // Validate cluster and host to prevent path traversal attacks + if strings.Contains(cluster, "..") || strings.Contains(cluster, "/") || strings.Contains(cluster, "\\") { + handleError(fmt.Errorf("invalid cluster name"), http.StatusBadRequest, rw) + return + } + if strings.Contains(host, "..") || strings.Contains(host, "/") || strings.Contains(host, "\\") { + handleError(fmt.Errorf("invalid host name"), http.StatusBadRequest, rw) + return + } + dir := filepath.Join(api.MachineStateDir, cluster) if err := os.MkdirAll(dir, 0o755); err != nil { - http.Error(rw, err.Error(), http.StatusInternalServerError) + handleError(fmt.Errorf("creating directory failed: %w", err), http.StatusInternalServerError, rw) return } filename := filepath.Join(dir, fmt.Sprintf("%s.json", host)) f, err := os.Create(filename) if err != nil { - http.Error(rw, err.Error(), http.StatusInternalServerError) + handleError(fmt.Errorf("creating file failed: %w", err), http.StatusInternalServerError, rw) return } defer f.Close() if _, err := io.Copy(f, r.Body); err != nil { - http.Error(rw, err.Error(), http.StatusInternalServerError) + handleError(fmt.Errorf("writing file failed: %w", err), http.StatusInternalServerError, rw) return } @@ -291,12 +317,25 @@ func (api *RestApi) putMachineState(rw http.ResponseWriter, r *http.Request) { func (api *RestApi) getMachineState(rw http.ResponseWriter, r *http.Request) { if api.MachineStateDir == "" { - http.Error(rw, "REST > machine state not enabled", http.StatusNotFound) + handleError(fmt.Errorf("machine state not enabled"), http.StatusNotFound, rw) return } vars := mux.Vars(r) - filename := filepath.Join(api.MachineStateDir, vars["cluster"], fmt.Sprintf("%s.json", vars["host"])) + cluster := vars["cluster"] + host := vars["host"] + + // Validate cluster and host to prevent path traversal attacks + if strings.Contains(cluster, "..") || strings.Contains(cluster, "/") || strings.Contains(cluster, "\\") { + handleError(fmt.Errorf("invalid cluster name"), http.StatusBadRequest, rw) + return + } + if strings.Contains(host, "..") || strings.Contains(host, "/") || strings.Contains(host, "\\") { + handleError(fmt.Errorf("invalid host name"), http.StatusBadRequest, rw) + return + } + + filename := filepath.Join(api.MachineStateDir, cluster, fmt.Sprintf("%s.json", host)) // Sets the content-type and 'Last-Modified' Header and so on automatically http.ServeFile(rw, r, filename) diff --git a/internal/api/user.go b/internal/api/user.go index 7e17e36..85b4ba3 100644 --- a/internal/api/user.go +++ b/internal/api/user.go @@ -40,24 +40,42 @@ func (api *RestApi) getUsers(rw http.ResponseWriter, r *http.Request) { // SecuredCheck() only worked with TokenAuth: Removed if user := repository.GetUserFromContext(r.Context()); !user.HasRole(schema.RoleAdmin) { - http.Error(rw, "Only admins are allowed to fetch a list of users", http.StatusForbidden) + handleError(fmt.Errorf("only admins are allowed to fetch a list of users"), http.StatusForbidden, rw) return } users, err := repository.GetUserRepository().ListUsers(r.URL.Query().Get("not-just-user") == "true") if err != nil { - http.Error(rw, err.Error(), http.StatusInternalServerError) + handleError(fmt.Errorf("listing users failed: %w", err), http.StatusInternalServerError, rw) return } - json.NewEncoder(rw).Encode(users) + rw.Header().Set("Content-Type", "application/json") + if err := json.NewEncoder(rw).Encode(users); err != nil { + cclog.Errorf("Failed to encode users response: %v", err) + } } +// updateUser godoc +// @summary Update user roles and projects +// @tags User +// @description Allows admins to add/remove roles and projects for a user +// @produce plain +// @param id path string true "Username" +// @param add-role formData string false "Role to add" +// @param remove-role formData string false "Role to remove" +// @param add-project formData string false "Project to add" +// @param remove-project formData string false "Project to remove" +// @success 200 {string} string "Success message" +// @failure 403 {object} api.ErrorResponse "Forbidden" +// @failure 422 {object} api.ErrorResponse "Unprocessable Entity" +// @security ApiKeyAuth +// @router /api/user/{id} [post] func (api *RestApi) updateUser(rw http.ResponseWriter, r *http.Request) { // SecuredCheck() only worked with TokenAuth: Removed if user := repository.GetUserFromContext(r.Context()); !user.HasRole(schema.RoleAdmin) { - http.Error(rw, "Only admins are allowed to update a user", http.StatusForbidden) + handleError(fmt.Errorf("only admins are allowed to update a user"), http.StatusForbidden, rw) return } @@ -67,43 +85,70 @@ func (api *RestApi) updateUser(rw http.ResponseWriter, r *http.Request) { newproj := r.FormValue("add-project") delproj := r.FormValue("remove-project") - // TODO: Handle anything but roles... + rw.Header().Set("Content-Type", "application/json") + + // Handle role updates if newrole != "" { if err := repository.GetUserRepository().AddRole(r.Context(), mux.Vars(r)["id"], newrole); err != nil { - http.Error(rw, err.Error(), http.StatusUnprocessableEntity) + handleError(fmt.Errorf("adding role failed: %w", err), http.StatusUnprocessableEntity, rw) return } - rw.Write([]byte("Add Role Success")) + if err := json.NewEncoder(rw).Encode(DefaultApiResponse{Message: "Add Role Success"}); err != nil { + cclog.Errorf("Failed to encode response: %v", err) + } } else if delrole != "" { if err := repository.GetUserRepository().RemoveRole(r.Context(), mux.Vars(r)["id"], delrole); err != nil { - http.Error(rw, err.Error(), http.StatusUnprocessableEntity) + handleError(fmt.Errorf("removing role failed: %w", err), http.StatusUnprocessableEntity, rw) return } - rw.Write([]byte("Remove Role Success")) + if err := json.NewEncoder(rw).Encode(DefaultApiResponse{Message: "Remove Role Success"}); err != nil { + cclog.Errorf("Failed to encode response: %v", err) + } } else if newproj != "" { if err := repository.GetUserRepository().AddProject(r.Context(), mux.Vars(r)["id"], newproj); err != nil { - http.Error(rw, err.Error(), http.StatusUnprocessableEntity) + handleError(fmt.Errorf("adding project failed: %w", err), http.StatusUnprocessableEntity, rw) return } - rw.Write([]byte("Add Project Success")) + if err := json.NewEncoder(rw).Encode(DefaultApiResponse{Message: "Add Project Success"}); err != nil { + cclog.Errorf("Failed to encode response: %v", err) + } } else if delproj != "" { if err := repository.GetUserRepository().RemoveProject(r.Context(), mux.Vars(r)["id"], delproj); err != nil { - http.Error(rw, err.Error(), http.StatusUnprocessableEntity) + handleError(fmt.Errorf("removing project failed: %w", err), http.StatusUnprocessableEntity, rw) return } - rw.Write([]byte("Remove Project Success")) + if err := json.NewEncoder(rw).Encode(DefaultApiResponse{Message: "Remove Project Success"}); err != nil { + cclog.Errorf("Failed to encode response: %v", err) + } } else { - http.Error(rw, "Not Add or Del [role|project]?", http.StatusInternalServerError) + handleError(fmt.Errorf("no operation specified: must provide add-role, remove-role, add-project, or remove-project"), http.StatusBadRequest, rw) } } +// createUser godoc +// @summary Create a new user +// @tags User +// @description Creates a new user with specified credentials and role +// @produce plain +// @param username formData string true "Username" +// @param password formData string false "Password (not required for API users)" +// @param role formData string true "User role" +// @param name formData string false "Full name" +// @param email formData string false "Email address" +// @param project formData string false "Project (required for managers)" +// @success 200 {string} string "Success message" +// @failure 400 {object} api.ErrorResponse "Bad Request" +// @failure 403 {object} api.ErrorResponse "Forbidden" +// @failure 422 {object} api.ErrorResponse "Unprocessable Entity" +// @security ApiKeyAuth +// @router /api/users/ [post] func (api *RestApi) createUser(rw http.ResponseWriter, r *http.Request) { // SecuredCheck() only worked with TokenAuth: Removed rw.Header().Set("Content-Type", "text/plain") me := repository.GetUserFromContext(r.Context()) if !me.HasRole(schema.RoleAdmin) { - http.Error(rw, "Only admins are allowed to create new users", http.StatusForbidden) + handleError(fmt.Errorf("only admins are allowed to create new users"), http.StatusForbidden, rw) return } @@ -111,18 +156,22 @@ func (api *RestApi) createUser(rw http.ResponseWriter, r *http.Request) { r.FormValue("password"), r.FormValue("role"), r.FormValue("name"), r.FormValue("email"), r.FormValue("project") + // Validate username length + if len(username) == 0 || len(username) > 100 { + handleError(fmt.Errorf("username must be between 1 and 100 characters"), http.StatusBadRequest, rw) + return + } + if len(password) == 0 && role != schema.GetRoleString(schema.RoleApi) { - http.Error(rw, "Only API users are allowed to have a blank password (login will be impossible)", http.StatusBadRequest) + handleError(fmt.Errorf("only API users are allowed to have a blank password (login will be impossible)"), http.StatusBadRequest, rw) return } if len(project) != 0 && role != schema.GetRoleString(schema.RoleManager) { - http.Error(rw, "only managers require a project (can be changed later)", - http.StatusBadRequest) + handleError(fmt.Errorf("only managers require a project (can be changed later)"), http.StatusBadRequest, rw) return } else if len(project) == 0 && role == schema.GetRoleString(schema.RoleManager) { - http.Error(rw, "managers require a project to manage (can be changed later)", - http.StatusBadRequest) + handleError(fmt.Errorf("managers require a project to manage (can be changed later)"), http.StatusBadRequest, rw) return } @@ -134,24 +183,35 @@ func (api *RestApi) createUser(rw http.ResponseWriter, r *http.Request) { Projects: []string{project}, Roles: []string{role}, }); err != nil { - http.Error(rw, err.Error(), http.StatusUnprocessableEntity) + handleError(fmt.Errorf("adding user failed: %w", err), http.StatusUnprocessableEntity, rw) return } fmt.Fprintf(rw, "User %v successfully created!\n", username) } +// deleteUser godoc +// @summary Delete a user +// @tags User +// @description Deletes a user from the system +// @produce plain +// @param username formData string true "Username to delete" +// @success 200 {string} string "Success" +// @failure 403 {object} api.ErrorResponse "Forbidden" +// @failure 422 {object} api.ErrorResponse "Unprocessable Entity" +// @security ApiKeyAuth +// @router /api/users/ [delete] func (api *RestApi) deleteUser(rw http.ResponseWriter, r *http.Request) { // SecuredCheck() only worked with TokenAuth: Removed if user := repository.GetUserFromContext(r.Context()); !user.HasRole(schema.RoleAdmin) { - http.Error(rw, "Only admins are allowed to delete a user", http.StatusForbidden) + handleError(fmt.Errorf("only admins are allowed to delete a user"), http.StatusForbidden, rw) return } username := r.FormValue("username") if err := repository.GetUserRepository().DelUser(username); err != nil { - http.Error(rw, err.Error(), http.StatusUnprocessableEntity) + handleError(fmt.Errorf("deleting user failed: %w", err), http.StatusUnprocessableEntity, rw) return }