From 16db9bd1a29382a1bc556d1f31c1ef1c4352673d Mon Sep 17 00:00:00 2001 From: exterr2f Date: Mon, 10 Mar 2025 08:15:42 +0100 Subject: [PATCH 1/7] Fix node filter: Use EXISTS with Eq for exact match and LIKE for Contains --- internal/repository/jobFind.go | 4 ++-- internal/repository/jobQuery.go | 9 ++++++++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/internal/repository/jobFind.go b/internal/repository/jobFind.go index 0354df0..ea5e1e9 100644 --- a/internal/repository/jobFind.go +++ b/internal/repository/jobFind.go @@ -194,11 +194,11 @@ func (r *JobRepository) FindConcurrentJobs( queryRunning := query.Where("job.job_state = ?").Where("(job.start_time BETWEEN ? AND ? OR job.start_time < ?)", "running", startTimeTail, stopTimeTail, startTime) - queryRunning = queryRunning.Where("job.resources LIKE ?", fmt.Sprint("%", hostname, "%")) + queryRunning = queryRunning.Where("EXISTS (SELECT 1 FROM json_each(job.resources) WHERE json_extract(value, '$.hostname') = ?)", hostname) query = query.Where("job.job_state != ?").Where("((job.start_time BETWEEN ? AND ?) OR (job.start_time + job.duration) BETWEEN ? AND ? OR (job.start_time < ?) AND (job.start_time + job.duration) > ?)", "running", startTimeTail, stopTimeTail, startTimeFront, stopTimeTail, startTime, stopTime) - query = query.Where("job.resources LIKE ?", fmt.Sprint("%", hostname, "%")) + query = query.Where("EXISTS (SELECT 1 FROM json_each(job.resources) WHERE json_extract(value, '$.hostname') = ?)", hostname) rows, err := query.RunWith(r.stmtCache).Query() if err != nil { diff --git a/internal/repository/jobQuery.go b/internal/repository/jobQuery.go index b43b569..7d1d9eb 100644 --- a/internal/repository/jobQuery.go +++ b/internal/repository/jobQuery.go @@ -194,7 +194,14 @@ func BuildWhereClause(filter *model.JobFilter, query sq.SelectBuilder) sq.Select query = buildIntCondition("job.num_hwthreads", filter.NumHWThreads, query) } if filter.Node != nil { - query = buildStringCondition("job.resources", filter.Node, query) + log.Infof("Applying node filter: %v", filter.Node) + if filter.Node.Eq != nil { + query = query.Where("EXISTS (SELECT 1 FROM json_each(job.resources) WHERE json_extract(value, '$.hostname') = ?)", *filter.Node.Eq) + } else if filter.Node.Contains != nil { + query = query.Where("EXISTS (SELECT 1 FROM json_each(job.resources) WHERE json_extract(value, '$.hostname') LIKE ?)", "%"+*filter.Node.Contains+"%") + } else { + query = buildStringCondition("job.resources", filter.Node, query) + } } if filter.Energy != nil { query = buildFloatCondition("job.energy", filter.Energy, query) From 02946cf0b4fff18f5abaca9d978f1c4c918e2fd1 Mon Sep 17 00:00:00 2001 From: Christoph Kluge Date: Mon, 7 Apr 2025 17:03:23 +0200 Subject: [PATCH 2/7] fix: fix nodelist filter result displaying wrong information - missing svelte iteration key added --- web/frontend/src/systems/NodeList.svelte | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/frontend/src/systems/NodeList.svelte b/web/frontend/src/systems/NodeList.svelte index ca22d57..c8a5e51 100644 --- a/web/frontend/src/systems/NodeList.svelte +++ b/web/frontend/src/systems/NodeList.svelte @@ -205,7 +205,7 @@ {:else} - {#each nodes as nodeData} + {#each nodes as nodeData (nodeData.host)} {:else} From d770292be8612937a3dc41794c84801f61e20eba Mon Sep 17 00:00:00 2001 From: Christoph Kluge Date: Tue, 8 Apr 2025 14:52:07 +0200 Subject: [PATCH 3/7] feat: add nodename matcher select to filter, defaults to equal match - see PR !353 --- internal/repository/jobFind.go | 2 ++ internal/repository/jobQuery.go | 34 ++++++++++++++----- web/frontend/src/generic/Filters.svelte | 21 ++++++++---- .../src/generic/filters/Resources.svelte | 27 +++++++++++++-- 4 files changed, 67 insertions(+), 17 deletions(-) diff --git a/internal/repository/jobFind.go b/internal/repository/jobFind.go index ea5e1e9..1e2ccb8 100644 --- a/internal/repository/jobFind.go +++ b/internal/repository/jobFind.go @@ -194,10 +194,12 @@ func (r *JobRepository) FindConcurrentJobs( queryRunning := query.Where("job.job_state = ?").Where("(job.start_time BETWEEN ? AND ? OR job.start_time < ?)", "running", startTimeTail, stopTimeTail, startTime) + // Get At Least One Exact Hostname Match from JSON Resources Array in Database queryRunning = queryRunning.Where("EXISTS (SELECT 1 FROM json_each(job.resources) WHERE json_extract(value, '$.hostname') = ?)", hostname) query = query.Where("job.job_state != ?").Where("((job.start_time BETWEEN ? AND ?) OR (job.start_time + job.duration) BETWEEN ? AND ? OR (job.start_time < ?) AND (job.start_time + job.duration) > ?)", "running", startTimeTail, stopTimeTail, startTimeFront, stopTimeTail, startTime, stopTime) + // Get At Least One Exact Hostname Match from JSON Resources Array in Database query = query.Where("EXISTS (SELECT 1 FROM json_each(job.resources) WHERE json_extract(value, '$.hostname') = ?)", hostname) rows, err := query.RunWith(r.stmtCache).Query() diff --git a/internal/repository/jobQuery.go b/internal/repository/jobQuery.go index f081297..d169b6f 100644 --- a/internal/repository/jobQuery.go +++ b/internal/repository/jobQuery.go @@ -67,7 +67,8 @@ func (r *JobRepository) QueryJobs( rows, err := query.RunWith(r.stmtCache).Query() if err != nil { - log.Errorf("Error while running query: %v", err) + queryString, queryVars, _ := query.ToSql() + log.Errorf("Error while running query '%s' %v: %v", queryString, queryVars, err) return nil, err } @@ -197,14 +198,7 @@ func BuildWhereClause(filter *model.JobFilter, query sq.SelectBuilder) sq.Select query = buildIntCondition("job.num_hwthreads", filter.NumHWThreads, query) } if filter.Node != nil { - log.Infof("Applying node filter: %v", filter.Node) - if filter.Node.Eq != nil { - query = query.Where("EXISTS (SELECT 1 FROM json_each(job.resources) WHERE json_extract(value, '$.hostname') = ?)", *filter.Node.Eq) - } else if filter.Node.Contains != nil { - query = query.Where("EXISTS (SELECT 1 FROM json_each(job.resources) WHERE json_extract(value, '$.hostname') LIKE ?)", "%"+*filter.Node.Contains+"%") - } else { - query = buildStringCondition("job.resources", filter.Node, query) - } + query = buildResourceJsonCondition("hostname", filter.Node, query) } if filter.Energy != nil { query = buildFloatCondition("job.energy", filter.Energy, query) @@ -306,6 +300,28 @@ func buildMetaJsonCondition(jsonField string, cond *model.StringInput, query sq. return query } +func buildResourceJsonCondition(jsonField string, cond *model.StringInput, query sq.SelectBuilder) sq.SelectBuilder { + // Verify and Search Only in Valid Jsons + query = query.Where("JSON_VALID(resources)") + // add "AND" Sql query Block for field match + if cond.Eq != nil { + return query.Where("EXISTS (SELECT 1 FROM json_each(job.resources) WHERE json_extract(value, \"$."+jsonField+"\") = ?)", *cond.Eq) + } + if cond.Neq != nil { // Currently Unused + return query.Where("EXISTS (SELECT 1 FROM json_each(job.resources) WHERE json_extract(value, \"$."+jsonField+"\") != ?)", *cond.Neq) + } + if cond.StartsWith != nil { // Currently Unused + return query.Where("EXISTS (SELECT 1 FROM json_each(job.resources) WHERE json_extract(value, \"$."+jsonField+"\")) LIKE ?)", fmt.Sprint(*cond.StartsWith, "%")) + } + if cond.EndsWith != nil { // Currently Unused + return query.Where("EXISTS (SELECT 1 FROM json_each(job.resources) WHERE json_extract(value, \"$."+jsonField+"\") LIKE ?)", fmt.Sprint("%", *cond.EndsWith)) + } + if cond.Contains != nil { + return query.Where("EXISTS (SELECT 1 FROM json_each(job.resources) WHERE json_extract(value, \"$."+jsonField+"\") LIKE ?)", fmt.Sprint("%", *cond.Contains, "%")) + } + return query +} + var ( matchFirstCap = regexp.MustCompile("(.)([A-Z][a-z]+)") matchAllCap = regexp.MustCompile("([a-z0-9])([A-Z])") diff --git a/web/frontend/src/generic/Filters.svelte b/web/frontend/src/generic/Filters.svelte index 4a9be3e..74640ae 100644 --- a/web/frontend/src/generic/Filters.svelte +++ b/web/frontend/src/generic/Filters.svelte @@ -53,10 +53,16 @@ { range: "last30d", rangeLabel: "Last 30 days"} ]; + const nodeMatchLabels = { + eq: "", + contains: " Contains", + } + let filters = { projectMatch: filterPresets.projectMatch || "contains", userMatch: filterPresets.userMatch || "contains", jobIdMatch: filterPresets.jobIdMatch || "eq", + nodeMatch: filterPresets.nodeMatch || "eq", cluster: filterPresets.cluster || null, partition: filterPresets.partition || null, @@ -106,7 +112,7 @@ let items = []; if (filters.cluster) items.push({ cluster: { eq: filters.cluster } }); - if (filters.node) items.push({ node: { contains: filters.node } }); + if (filters.node) items.push({ node: { [filters.nodeMatch]: filters.node } }); if (filters.partition) items.push({ partition: { eq: filters.partition } }); if (filters.states.length != allJobStates.length) items.push({ state: filters.states }); @@ -178,6 +184,8 @@ let opts = []; if (filters.cluster) opts.push(`cluster=${filters.cluster}`); if (filters.node) opts.push(`node=${filters.node}`); + if (filters.node && filters.nodeMatch != "eq") // "eq" is default-case + opts.push(`nodeMatch=${filters.nodeMatch}`); if (filters.partition) opts.push(`partition=${filters.partition}`); if (filters.states.length != allJobStates.length) for (let state of filters.states) opts.push(`state=${state}`); @@ -196,7 +204,7 @@ opts.push(`jobId=${singleJobId}`); } if (filters.jobIdMatch != "eq") - opts.push(`jobIdMatch=${filters.jobIdMatch}`); + opts.push(`jobIdMatch=${filters.jobIdMatch}`); // "eq" is default-case for (let tag of filters.tags) opts.push(`tag=${tag}`); if (filters.duration.from && filters.duration.to) opts.push(`duration=${filters.duration.from}-${filters.duration.to}`); @@ -218,13 +226,13 @@ } else { for (let singleUser of filters.user) opts.push(`user=${singleUser}`); } - if (filters.userMatch != "contains") + if (filters.userMatch != "contains") // "contains" is default-case opts.push(`userMatch=${filters.userMatch}`); if (filters.project) opts.push(`project=${filters.project}`); + if (filters.project && filters.projectMatch != "contains") // "contains" is default-case + opts.push(`projectMatch=${filters.projectMatch}`); if (filters.jobName) opts.push(`jobName=${filters.jobName}`); if (filters.arrayJobId) opts.push(`arrayJobId=${filters.arrayJobId}`); - if (filters.project && filters.projectMatch != "contains") - opts.push(`projectMatch=${filters.projectMatch}`); if (filters.stats.length != 0) for (let stat of filters.stats) { opts.push(`stat=${stat.field}-${stat.from}-${stat.to}`); @@ -386,7 +394,7 @@ {#if filters.node != null} (isResourcesOpen = true)}> - Node: {filters.node} + Node{nodeMatchLabels[filters.nodeMatch]}: {filters.node} {/if} @@ -449,6 +457,7 @@ bind:numHWThreads={filters.numHWThreads} bind:numAccelerators={filters.numAccelerators} bind:namedNode={filters.node} + bind:nodeMatch={filters.nodeMatch} bind:isNodesModified bind:isHwthreadsModified bind:isAccsModified diff --git a/web/frontend/src/generic/filters/Resources.svelte b/web/frontend/src/generic/filters/Resources.svelte index 750c4a6..443dda7 100644 --- a/web/frontend/src/generic/filters/Resources.svelte +++ b/web/frontend/src/generic/filters/Resources.svelte @@ -24,6 +24,7 @@ ModalBody, ModalHeader, ModalFooter, + Input } from "@sveltestrap/sveltestrap"; import DoubleRangeSlider from "../select/DoubleRangeSlider.svelte"; @@ -40,11 +41,18 @@ export let isHwthreadsModified = false; export let isAccsModified = false; export let namedNode = null; + export let nodeMatch = "eq" let pendingNumNodes = numNodes, pendingNumHWThreads = numHWThreads, pendingNumAccelerators = numAccelerators, - pendingNamedNode = namedNode; + pendingNamedNode = namedNode, + pendingNodeMatch = nodeMatch; + + const nodeMatchLabels = { + eq: "Equal To", + contains: "Contains", + } const findMaxNumAccels = (clusters) => clusters.reduce( @@ -145,7 +153,17 @@ Select number of utilized Resources
Named Node
- +
+ +
+ + {#each Object.entries(nodeMatchLabels) as [nodeMatchKey, nodeMatchLabel]} + + {/each} + +
Number of Nodes
{ @@ -215,11 +233,13 @@ to: pendingNumAccelerators.to, }; namedNode = pendingNamedNode; + nodeMatch = pendingNodeMatch; dispatch("set-filter", { numNodes, numHWThreads, numAccelerators, namedNode, + nodeMatch }); }} > @@ -233,6 +253,7 @@ pendingNumHWThreads = { from: null, to: null }; pendingNumAccelerators = { from: null, to: null }; pendingNamedNode = null; + pendingNodeMatch = null; numNodes = { from: pendingNumNodes.from, to: pendingNumNodes.to }; numHWThreads = { from: pendingNumHWThreads.from, @@ -246,11 +267,13 @@ isHwthreadsModified = false; isAccsModified = false; namedNode = pendingNamedNode; + nodeMatch = pendingNodeMatch; dispatch("set-filter", { numNodes, numHWThreads, numAccelerators, namedNode, + nodeMatch }); }}>Reset From a6784b5549722466299541a8377c25800a83f318 Mon Sep 17 00:00:00 2001 From: Christoph Kluge Date: Tue, 8 Apr 2025 16:00:07 +0200 Subject: [PATCH 4/7] fix: reintroduce statstable id natural sort order - see Use natural sort order for IDs in statistics tables #369 --- web/frontend/src/job/statstab/StatsTableEntry.svelte | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/web/frontend/src/job/statstab/StatsTableEntry.svelte b/web/frontend/src/job/statstab/StatsTableEntry.svelte index b39eacb..c3161a5 100644 --- a/web/frontend/src/job/statstab/StatsTableEntry.svelte +++ b/web/frontend/src/job/statstab/StatsTableEntry.svelte @@ -41,7 +41,9 @@ if (a == null || b == null) return -1; if (field === "id") { - return s.dir != "up" ? a[field].localeCompare(b[field]) : b[field].localeCompare(a[field]) + return s.dir != "up" ? + a[field].localeCompare(b[field], undefined, {numeric: true, sensitivity: 'base'}) : + b[field].localeCompare(a[field], undefined, {numeric: true, sensitivity: 'base'}) } else { return s.dir != "up" ? a.data[field] - b.data[field] From fb6a4c3b874114883a49aacdd408d0c17dfc0a20 Mon Sep 17 00:00:00 2001 From: Christoph Kluge Date: Wed, 9 Apr 2025 16:00:27 +0200 Subject: [PATCH 5/7] review and move api endpoints secured check --- internal/api/rest.go | 95 +++++-------------------------------------- internal/auth/auth.go | 80 ++++++++++++++++++++++++++++++++++-- pkg/schema/config.go | 5 ++- 3 files changed, 91 insertions(+), 89 deletions(-) diff --git a/internal/api/rest.go b/internal/api/rest.go index 712a0b3..352dd94 100644 --- a/internal/api/rest.go +++ b/internal/api/rest.go @@ -69,11 +69,10 @@ func New() *RestApi { func (api *RestApi) MountApiRoutes(r *mux.Router) { r.StrictSlash(true) - + // REST API Uses TokenAuth 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) @@ -83,7 +82,6 @@ func (api *RestApi) MountApiRoutes(r *mux.Router) { r.HandleFunc("/jobs/delete_job/", api.deleteJobByRequest).Methods(http.MethodDelete) r.HandleFunc("/jobs/delete_job/{id}", api.deleteJobById).Methods(http.MethodDelete) r.HandleFunc("/jobs/delete_job_before/{ts}", api.deleteJobBefore).Methods(http.MethodDelete) - r.HandleFunc("/clusters/", api.getClusters).Methods(http.MethodGet) if api.MachineStateDir != "" { @@ -94,7 +92,7 @@ func (api *RestApi) MountApiRoutes(r *mux.Router) { func (api *RestApi) MountUserApiRoutes(r *mux.Router) { r.StrictSlash(true) - + // REST API Uses TokenAuth 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) @@ -103,7 +101,7 @@ func (api *RestApi) MountUserApiRoutes(r *mux.Router) { func (api *RestApi) MountConfigApiRoutes(r *mux.Router) { r.StrictSlash(true) - + // Settings Frontend Uses SessionAuth if api.Authentication != nil { r.HandleFunc("/roles/", api.getRoles).Methods(http.MethodGet) r.HandleFunc("/users/", api.createUser).Methods(http.MethodPost, http.MethodPut) @@ -116,7 +114,7 @@ func (api *RestApi) MountConfigApiRoutes(r *mux.Router) { func (api *RestApi) MountFrontendApiRoutes(r *mux.Router) { r.StrictSlash(true) - + // Settings Frontrend Uses SessionAuth if api.Authentication != nil { r.HandleFunc("/jwt/", api.getJWT).Methods(http.MethodGet) r.HandleFunc("/configuration/", api.updateConfiguration).Methods(http.MethodPost) @@ -221,44 +219,6 @@ func decode(r io.Reader, val interface{}) error { return dec.Decode(val) } -func securedCheck(r *http.Request) error { - user := repository.GetUserFromContext(r.Context()) - if user == nil { - return fmt.Errorf("no user in context") - } - - if user.AuthType == schema.AuthToken { - // If nothing declared in config: deny all request to this endpoint - if config.Keys.ApiAllowedIPs == nil || len(config.Keys.ApiAllowedIPs) == 0 { - return fmt.Errorf("missing configuration key ApiAllowedIPs") - } - - if config.Keys.ApiAllowedIPs[0] == "*" { - return nil - } - - // extract IP address - IPAddress := r.Header.Get("X-Real-Ip") - if IPAddress == "" { - IPAddress = r.Header.Get("X-Forwarded-For") - } - if IPAddress == "" { - IPAddress = r.RemoteAddr - } - - if strings.Contains(IPAddress, ":") { - IPAddress = strings.Split(IPAddress, ":")[0] - } - - // check if IP is allowed - if !util.Contains(config.Keys.ApiAllowedIPs, IPAddress) { - return fmt.Errorf("unknown ip: %v", IPAddress) - } - } - - return nil -} - // getClusters godoc // @summary Lists all cluster configs // @tags Cluster query @@ -1093,7 +1053,6 @@ func (api *RestApi) getJobMetrics(rw http.ResponseWriter, r *http.Request) { // @summary Adds a new user // @tags User // @description User specified in form data will be saved to database. -// @description Only accessible from IPs registered with apiAllowedIPs configuration option. // @accept mpfd // @produce plain // @param username formData string true "Unique user ID" @@ -1111,11 +1070,7 @@ func (api *RestApi) getJobMetrics(rw http.ResponseWriter, r *http.Request) { // @security ApiKeyAuth // @router /users/ [post] func (api *RestApi) createUser(rw http.ResponseWriter, r *http.Request) { - err := securedCheck(r) - if err != nil { - http.Error(rw, err.Error(), http.StatusForbidden) - return - } + // SecuredCheck() only worked with TokenAuth: Removed rw.Header().Set("Content-Type", "text/plain") me := repository.GetUserFromContext(r.Context()) @@ -1162,7 +1117,6 @@ func (api *RestApi) createUser(rw http.ResponseWriter, r *http.Request) { // @summary Deletes a user // @tags User // @description User defined by username in form data will be deleted from database. -// @description Only accessible from IPs registered with apiAllowedIPs configuration option. // @accept mpfd // @produce plain // @param username formData string true "User ID to delete" @@ -1175,11 +1129,7 @@ func (api *RestApi) createUser(rw http.ResponseWriter, r *http.Request) { // @security ApiKeyAuth // @router /users/ [delete] func (api *RestApi) deleteUser(rw http.ResponseWriter, r *http.Request) { - err := securedCheck(r) - if err != nil { - http.Error(rw, err.Error(), http.StatusForbidden) - return - } + // 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) @@ -1200,7 +1150,6 @@ func (api *RestApi) deleteUser(rw http.ResponseWriter, r *http.Request) { // @tags User // @description Returns a JSON-encoded list of users. // @description Required query-parameter defines if all users or only users with additional special roles are returned. -// @description Only accessible from IPs registered with apiAllowedIPs configuration option. // @produce json // @param not-just-user query bool true "If returned list should contain all users or only users with additional special roles" // @success 200 {array} api.ApiReturnedUser "List of users returned successfully" @@ -1211,11 +1160,7 @@ func (api *RestApi) deleteUser(rw http.ResponseWriter, r *http.Request) { // @security ApiKeyAuth // @router /users/ [get] func (api *RestApi) getUsers(rw http.ResponseWriter, r *http.Request) { - err := securedCheck(r) - if err != nil { - http.Error(rw, err.Error(), http.StatusForbidden) - return - } + // 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) @@ -1236,7 +1181,6 @@ func (api *RestApi) getUsers(rw http.ResponseWriter, r *http.Request) { // @tags User // @description Modifies user defined by username (id) in one of four possible ways. // @description If more than one formValue is set then only the highest priority field is used. -// @description Only accessible from IPs registered with apiAllowedIPs configuration option. // @accept mpfd // @produce plain // @param id path string true "Database ID of User" @@ -1253,11 +1197,7 @@ func (api *RestApi) getUsers(rw http.ResponseWriter, r *http.Request) { // @security ApiKeyAuth // @router /user/{id} [post] func (api *RestApi) updateUser(rw http.ResponseWriter, r *http.Request) { - err := securedCheck(r) - if err != nil { - http.Error(rw, err.Error(), http.StatusForbidden) - return - } + // 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) @@ -1305,7 +1245,6 @@ func (api *RestApi) updateUser(rw http.ResponseWriter, r *http.Request) { // @tags User // @description Modifies the content of notice.txt, shown as notice box on the homepage. // @description If more than one formValue is set then only the highest priority field is used. -// @description Only accessible from IPs registered with apiAllowedIPs configuration option. // @accept mpfd // @produce plain // @param new-content formData string false "Priority 1: New content to display" @@ -1318,11 +1257,7 @@ func (api *RestApi) updateUser(rw http.ResponseWriter, r *http.Request) { // @security ApiKeyAuth // @router /notice/ [post] func (api *RestApi) editNotice(rw http.ResponseWriter, r *http.Request) { - err := securedCheck(r) - if err != nil { - http.Error(rw, err.Error(), http.StatusForbidden) - return - } + // 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) @@ -1364,12 +1299,6 @@ func (api *RestApi) editNotice(rw http.ResponseWriter, r *http.Request) { } func (api *RestApi) getJWT(rw http.ResponseWriter, r *http.Request) { - err := securedCheck(r) - if err != nil { - http.Error(rw, err.Error(), http.StatusForbidden) - return - } - rw.Header().Set("Content-Type", "text/plain") username := r.FormValue("username") me := repository.GetUserFromContext(r.Context()) @@ -1398,11 +1327,7 @@ func (api *RestApi) getJWT(rw http.ResponseWriter, r *http.Request) { } func (api *RestApi) getRoles(rw http.ResponseWriter, r *http.Request) { - err := securedCheck(r) - if err != nil { - http.Error(rw, err.Error(), http.StatusForbidden) - return - } + // SecuredCheck() only worked with TokenAuth: Removed user := repository.GetUserFromContext(r.Context()) if !user.HasRole(schema.RoleAdmin) { diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 262204c..d5e48ac 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -10,9 +10,11 @@ import ( "database/sql" "encoding/base64" "errors" + "fmt" "net" "net/http" "os" + "strings" "sync" "time" @@ -20,6 +22,7 @@ import ( "github.com/ClusterCockpit/cc-backend/internal/config" "github.com/ClusterCockpit/cc-backend/internal/repository" + "github.com/ClusterCockpit/cc-backend/internal/util" "github.com/ClusterCockpit/cc-backend/pkg/log" "github.com/ClusterCockpit/cc-backend/pkg/schema" "github.com/gorilla/sessions" @@ -233,9 +236,9 @@ func (auth *Authentication) Login( limiter := getIPUserLimiter(ip, username) if !limiter.Allow() { - log.Warnf("AUTH/RATE > Too many login attempts for combination IP: %s, Username: %s", ip, username) - onfailure(rw, r, errors.New("Too many login attempts, try again in a few minutes.")) - return + log.Warnf("AUTH/RATE > Too many login attempts for combination IP: %s, Username: %s", ip, username) + onfailure(rw, r, errors.New("Too many login attempts, try again in a few minutes.")) + return } var dbUser *schema.User @@ -325,6 +328,14 @@ func (auth *Authentication) AuthApi( onfailure(rw, r, err) return } + + ipErr := securedCheck(user, "api", r) + if ipErr != nil { + log.Infof("auth api -> secured check failed: %s", err.Error()) + onfailure(rw, r, ipErr) + return + } + if user != nil { switch { case len(user.Roles) == 1: @@ -360,6 +371,14 @@ func (auth *Authentication) AuthUserApi( onfailure(rw, r, err) return } + + ipErr := securedCheck(user, "userapi", r) + if ipErr != nil { + log.Infof("auth user api -> secured check failed: %s", err.Error()) + onfailure(rw, r, ipErr) + return + } + if user != nil { switch { case len(user.Roles) == 1: @@ -445,3 +464,58 @@ func (auth *Authentication) Logout(onsuccess http.Handler) http.Handler { onsuccess.ServeHTTP(rw, r) }) } + +// Helper Moved To MiddleWare Auth Handlers +func securedCheck(user *schema.User, checkEndpoint string, r *http.Request) error { + if user == nil { + return fmt.Errorf("no user for secured check") + } + + // extract IP address for checking + IPAddress := r.Header.Get("X-Real-Ip") + if IPAddress == "" { + IPAddress = r.Header.Get("X-Forwarded-For") + } + if IPAddress == "" { + IPAddress = r.RemoteAddr + } + + if strings.Contains(IPAddress, ":") { + IPAddress = strings.Split(IPAddress, ":")[0] + } + + // Used for checking TokenAuth'd Requests Only: Remove '== schema.AuthToken'-Condition + if checkEndpoint == "api" { + // If nothing declared in config: deny all request to this api endpoint + if config.Keys.ApiAllowedIPs == nil || len(config.Keys.ApiAllowedIPs) == 0 { + return fmt.Errorf("missing configuration key ApiAllowedIPs") + } + // If wildcard declared in config: Continue + if config.Keys.ApiAllowedIPs[0] == "*" { + return nil + } + // check if IP is allowed + if !util.Contains(config.Keys.ApiAllowedIPs, IPAddress) { + return fmt.Errorf("unknown ip: %v", IPAddress) + } + + } else if checkEndpoint == "userapi" { + // If nothing declared in config: deny all request to this api endpoint + if config.Keys.UserApiAllowedIPs == nil || len(config.Keys.UserApiAllowedIPs) == 0 { + return fmt.Errorf("missing configuration key UserApiAllowedIPs") + } + // If wildcard declared in config: Continue + if config.Keys.UserApiAllowedIPs[0] == "*" { + return nil + } + // check if IP is allowed + if !util.Contains(config.Keys.UserApiAllowedIPs, IPAddress) { + return fmt.Errorf("unknown user ip: %v", IPAddress) + } + + } else { + return fmt.Errorf("unknown checkEndpoint for secured check") + } + + return nil +} diff --git a/pkg/schema/config.go b/pkg/schema/config.go index f9116cf..16b4219 100644 --- a/pkg/schema/config.go +++ b/pkg/schema/config.go @@ -100,9 +100,12 @@ type ProgramConfig struct { // Address where the http (or https) server will listen on (for example: 'localhost:80'). Addr string `json:"addr"` - // Addresses from which secured API endpoints can be reached + // Addresses from which secured admin API endpoints can be reached, can be wildcard "*" ApiAllowedIPs []string `json:"apiAllowedIPs"` + // Addresses from which secured admin API endpoints can be reached, can be wildcard "*" + UserApiAllowedIPs []string `json:"userApiAllowedIPs"` + // Drop root permissions once .env was read and the port was taken. User string `json:"user"` Group string `json:"group"` From 25d3325049ad9060399da27d2e3e9807aaa0c630 Mon Sep 17 00:00:00 2001 From: Christoph Kluge Date: Mon, 14 Apr 2025 11:36:03 +0200 Subject: [PATCH 6/7] add getUsers to admin REST api --- internal/api/rest.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/internal/api/rest.go b/internal/api/rest.go index 352dd94..0fa4611 100644 --- a/internal/api/rest.go +++ b/internal/api/rest.go @@ -70,6 +70,11 @@ func New() *RestApi { func (api *RestApi) MountApiRoutes(r *mux.Router) { r.StrictSlash(true) // REST API Uses TokenAuth + // User List + r.HandleFunc("/users/", api.getUsers).Methods(http.MethodGet) + // Cluster List + r.HandleFunc("/clusters/", api.getClusters).Methods(http.MethodGet) + // 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) @@ -82,7 +87,6 @@ func (api *RestApi) MountApiRoutes(r *mux.Router) { r.HandleFunc("/jobs/delete_job/", api.deleteJobByRequest).Methods(http.MethodDelete) r.HandleFunc("/jobs/delete_job/{id}", api.deleteJobById).Methods(http.MethodDelete) r.HandleFunc("/jobs/delete_job_before/{ts}", api.deleteJobBefore).Methods(http.MethodDelete) - r.HandleFunc("/clusters/", api.getClusters).Methods(http.MethodGet) if api.MachineStateDir != "" { r.HandleFunc("/machine_state/{cluster}/{host}", api.getMachineState).Methods(http.MethodGet) From 1755a4a7dfd851f211669fe155a75afe1373aa6f Mon Sep 17 00:00:00 2001 From: Christoph Kluge Date: Mon, 14 Apr 2025 11:58:42 +0200 Subject: [PATCH 7/7] remove separate userapiallowedips config and check --- internal/auth/auth.go | 53 +++++++++++-------------------------------- pkg/schema/config.go | 3 --- 2 files changed, 13 insertions(+), 43 deletions(-) diff --git a/internal/auth/auth.go b/internal/auth/auth.go index d5e48ac..9201315 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -329,7 +329,7 @@ func (auth *Authentication) AuthApi( return } - ipErr := securedCheck(user, "api", r) + ipErr := securedCheck(user, r) if ipErr != nil { log.Infof("auth api -> secured check failed: %s", err.Error()) onfailure(rw, r, ipErr) @@ -372,13 +372,6 @@ func (auth *Authentication) AuthUserApi( return } - ipErr := securedCheck(user, "userapi", r) - if ipErr != nil { - log.Infof("auth user api -> secured check failed: %s", err.Error()) - onfailure(rw, r, ipErr) - return - } - if user != nil { switch { case len(user.Roles) == 1: @@ -466,7 +459,7 @@ func (auth *Authentication) Logout(onsuccess http.Handler) http.Handler { } // Helper Moved To MiddleWare Auth Handlers -func securedCheck(user *schema.User, checkEndpoint string, r *http.Request) error { +func securedCheck(user *schema.User, r *http.Request) error { if user == nil { return fmt.Errorf("no user for secured check") } @@ -484,37 +477,17 @@ func securedCheck(user *schema.User, checkEndpoint string, r *http.Request) erro IPAddress = strings.Split(IPAddress, ":")[0] } - // Used for checking TokenAuth'd Requests Only: Remove '== schema.AuthToken'-Condition - if checkEndpoint == "api" { - // If nothing declared in config: deny all request to this api endpoint - if config.Keys.ApiAllowedIPs == nil || len(config.Keys.ApiAllowedIPs) == 0 { - return fmt.Errorf("missing configuration key ApiAllowedIPs") - } - // If wildcard declared in config: Continue - if config.Keys.ApiAllowedIPs[0] == "*" { - return nil - } - // check if IP is allowed - if !util.Contains(config.Keys.ApiAllowedIPs, IPAddress) { - return fmt.Errorf("unknown ip: %v", IPAddress) - } - - } else if checkEndpoint == "userapi" { - // If nothing declared in config: deny all request to this api endpoint - if config.Keys.UserApiAllowedIPs == nil || len(config.Keys.UserApiAllowedIPs) == 0 { - return fmt.Errorf("missing configuration key UserApiAllowedIPs") - } - // If wildcard declared in config: Continue - if config.Keys.UserApiAllowedIPs[0] == "*" { - return nil - } - // check if IP is allowed - if !util.Contains(config.Keys.UserApiAllowedIPs, IPAddress) { - return fmt.Errorf("unknown user ip: %v", IPAddress) - } - - } else { - return fmt.Errorf("unknown checkEndpoint for secured check") + // If nothing declared in config: deny all request to this api endpoint + if len(config.Keys.ApiAllowedIPs) == 0 { + return fmt.Errorf("missing configuration key ApiAllowedIPs") + } + // If wildcard declared in config: Continue + if config.Keys.ApiAllowedIPs[0] == "*" { + return nil + } + // check if IP is allowed + if !util.Contains(config.Keys.ApiAllowedIPs, IPAddress) { + return fmt.Errorf("unknown ip: %v", IPAddress) } return nil diff --git a/pkg/schema/config.go b/pkg/schema/config.go index 16b4219..27d11be 100644 --- a/pkg/schema/config.go +++ b/pkg/schema/config.go @@ -103,9 +103,6 @@ type ProgramConfig struct { // Addresses from which secured admin API endpoints can be reached, can be wildcard "*" ApiAllowedIPs []string `json:"apiAllowedIPs"` - // Addresses from which secured admin API endpoints can be reached, can be wildcard "*" - UserApiAllowedIPs []string `json:"userApiAllowedIPs"` - // Drop root permissions once .env was read and the port was taken. User string `json:"user"` Group string `json:"group"`