review and move api endpoints secured check

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)
+	// SecuredCheck() only worked with TokenAuth: Removed
-	if err != nil {
-		http.Error(rw, err.Error(), http.StatusForbidden)
-		return
-	}
 
 	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)
+	// SecuredCheck() only worked with TokenAuth: Removed
-	if err != nil {
-		http.Error(rw, err.Error(), http.StatusForbidden)
-		return
-	}
 
 	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)
+	// SecuredCheck() only worked with TokenAuth: Removed
-	if err != nil {
-		http.Error(rw, err.Error(), http.StatusForbidden)
-		return
-	}
 
 	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)
+	// SecuredCheck() only worked with TokenAuth: Removed
-	if err != nil {
-		http.Error(rw, err.Error(), http.StatusForbidden)
-		return
-	}
 
 	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)
+	// SecuredCheck() only worked with TokenAuth: Removed
-	if err != nil {
-		http.Error(rw, err.Error(), http.StatusForbidden)
-		return
-	}
 
 	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)
+	// SecuredCheck() only worked with TokenAuth: Removed
-	if err != nil {
-		http.Error(rw, err.Error(), http.StatusForbidden)
-		return
-	}
 
 	user := repository.GetUserFromContext(r.Context())
 	if !user.HasRole(schema.RoleAdmin) {

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)
+			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."))
+			onfailure(rw, r, errors.New("Too many login attempts, try again in a few minutes."))
-				return
+			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
+}

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"`