Merge pull request #432 from jmpsec/saml-2-fixes

Fix for SAML authentication and redirect loops
jmpsec · Apr 7, 2024 · 86e7b60 · 86e7b60
2 parents 7daad2f + 3d82cdc
commit 86e7b60
Show file tree

Hide file tree

Showing 4 changed files with 68 additions and 32 deletions.
diff --git a/admin/auth.go b/admin/auth.go
@@ -5,8 +5,10 @@ import (
 	"log"
 	"net/http"
 
+	"github.com/crewjam/saml/samlsp"
 	"github.com/jmpsec/osctrl/admin/sessions"
 	"github.com/jmpsec/osctrl/settings"
+	"github.com/jmpsec/osctrl/users"
 )
 
 const (
@@ -43,40 +45,60 @@ func handlerAuthCheck(h http.Handler) http.Handler {
 			// Access granted
 			h.ServeHTTP(w, r.WithContext(ctx))
 		case settings.AuthSAML:
-			_, err := samlMiddleware.Session.GetSession(r)
+			samlSession, err := samlMiddleware.Session.GetSession(r)
 			if err != nil {
 				log.Printf("GetSession %v", err)
-			}
-			cookiev, err := r.Cookie(authCookieName)
-			if err != nil {
-				log.Printf("error extracting JWT data: %v", err)
 				http.Redirect(w, r, samlConfig.LoginURL, http.StatusFound)
 				return
 			}
-			jwtdata, err := parseJWTFromCookie(samlData.KeyPair, cookiev.Value)
-			if err != nil {
-				log.Printf("error parsing JWT: %v", err)
-				http.Redirect(w, r, samlConfig.LoginURL, http.StatusFound)
+			if samlSession == nil {
+				log.Printf("GetSession %v", err)
+				http.Redirect(w, r, samlConfig.LogoutURL, http.StatusFound)
+				return
+			}
+			jwtSessionClaims, ok := samlSession.(samlsp.JWTSessionClaims)
+			if !ok {
+				log.Printf("JWTSessionClaims %v", err)
+				return
+			}
+			samlUser := jwtSessionClaims.Subject
+			if samlUser == "" {
+				log.Printf("SAML user is empty")
 				return
 			}
 			// Check if user is already authenticated
 			authenticated, session := sessionsmgr.CheckAuth(r)
 			if !authenticated {
 				// Create user if it does not exist
-				if !adminUsers.Exists(jwtdata.Username) {
-					log.Printf("user not found: %s", jwtdata.Username)
-					http.Redirect(w, r, forbiddenPath, http.StatusFound)
-					return
-				}
-				u, err := adminUsers.Get(jwtdata.Username)
-				if err != nil {
-					log.Printf("error getting user %s: %v", jwtdata.Username, err)
-					http.Redirect(w, r, forbiddenPath, http.StatusFound)
-					return
+				var u users.AdminUser
+				if !adminUsers.Exists(samlUser) {
+					if !samlConfig.JITProvision {
+						log.Printf("user not found: %s", samlUser)
+						http.Redirect(w, r, forbiddenPath, http.StatusFound)
+						return
+					}
+					u, err = adminUsers.New(samlUser, "", samlUser, "", "", false)
+					if err != nil {
+						log.Printf("error creating user %s: %v", samlUser, err)
+						http.Redirect(w, r, forbiddenPath, http.StatusFound)
+						return
+					}
+					if err := adminUsers.Create(u); err != nil {
+						log.Printf("error creating user %s: %v", samlUser, err)
+						http.Redirect(w, r, forbiddenPath, http.StatusFound)
+						return
+					}
+				} else {
+					u, err = adminUsers.Get(samlUser)
+					if err != nil {
+						log.Printf("error getting user %s: %v", samlUser, err)
+						http.Redirect(w, r, forbiddenPath, http.StatusFound)
+						return
+					}
 				}
 				access, err := adminUsers.GetEnvAccess(u.Username, u.DefaultEnv)
 				if err != nil {
-					log.Printf("error getting access for %s: %v", jwtdata.Username, err)
+					log.Printf("error getting access for %s: %v", samlUser, err)
 					http.Redirect(w, r, forbiddenPath, http.StatusFound)
 					return
 				}

diff --git a/admin/main.go b/admin/main.go
@@ -53,6 +53,8 @@ const (
 	healthPath string = "/health"
 	// Default endpoint to handle Login
 	loginPath string = "/login"
+	// Default endpoint to handle Logout
+	logoutPath string = "/logout"
 	// Default endpoint to handle HTTP(500) errors
 	errorPath string = "/error"
 	// Default endpoint to handle Forbidden(403) errors
@@ -760,10 +762,13 @@ func osctrlAdminService() {
 		log.Println("DebugService: Unauthenticated content")
 	}
 	// Admin: login only if local auth is enabled
-	if adminConfig.Auth != settings.AuthNone {
+	if adminConfig.Auth != settings.AuthNone && adminConfig.Auth != settings.AuthSAML {
 		// login
 		routerAdmin.HandleFunc(loginPath, handlersAdmin.LoginHandler).Methods("GET")
 		routerAdmin.HandleFunc(loginPath, handlersAdmin.LoginPOSTHandler).Methods("POST")
+		routerAdmin.HandleFunc(logoutPath, func(w http.ResponseWriter, r *http.Request) {
+			http.Redirect(w, r, loginPath, http.StatusFound)
+		}).Methods("GET")
 	}
 	// Admin: health of service
 	routerAdmin.HandleFunc(healthPath, handlersAdmin.HealthHandler).Methods("GET")
@@ -861,10 +866,16 @@ func osctrlAdminService() {
 	routerAdmin.Handle("/profile", handlerAuthCheck(http.HandlerFunc(handlersAdmin.EditProfileGETHandler))).Methods("GET")
 	routerAdmin.Handle("/profile", handlerAuthCheck(http.HandlerFunc(handlersAdmin.EditProfilePOSTHandler))).Methods("POST")
 	// logout
-	routerAdmin.Handle("/logout", handlerAuthCheck(http.HandlerFunc(handlersAdmin.LogoutPOSTHandler))).Methods("POST")
+	routerAdmin.Handle(logoutPath, handlerAuthCheck(http.HandlerFunc(handlersAdmin.LogoutPOSTHandler))).Methods("POST")
 	// SAML ACS
 	if adminConfig.Auth == settings.AuthSAML {
 		routerAdmin.PathPrefix("/saml/").Handler(samlMiddleware)
+		routerAdmin.HandleFunc(loginPath, func(w http.ResponseWriter, r *http.Request) {
+			http.Redirect(w, r, samlConfig.LoginURL, http.StatusFound)
+		}).Methods("GET")
+		routerAdmin.HandleFunc(logoutPath, func(w http.ResponseWriter, r *http.Request) {
+			http.Redirect(w, r, samlConfig.LogoutURL, http.StatusFound)
+		}).Methods("GET")
 	}
 	// Launch HTTP server for admin
 	serviceAdmin := adminConfig.Listener + ":" + adminConfig.Port

diff --git a/admin/saml.go b/admin/saml.go
@@ -17,15 +17,13 @@ import (
 
 // JSONConfigurationSAML to keep all SAML details for auth
 type JSONConfigurationSAML struct {
-	CertPath    string `json:"certpath"`
-	KeyPath     string `json:"keypath"`
-	MetaDataURL string `json:"metadataurl"`
-	RootURL     string `json:"rooturl"`
-	LoginURL    string `json:"loginurl"`
-	TokenName   string `json:"nametoken"`
-	EmailAttr   string `json:"attremail"`
-	UserAttr    string `json:"attruser"`
-	DisplayAttr string `json:"attrdisplay"`
+	CertPath     string `json:"certpath"`
+	KeyPath      string `json:"keypath"`
+	MetaDataURL  string `json:"metadataurl"`
+	RootURL      string `json:"rooturl"`
+	LoginURL     string `json:"loginurl"`
+	LogoutURL    string `json:"logouturl"`
+	JITProvision bool   `json:"jitprovision"`
 }
 
 // Structure to keep all SAML related data
@@ -80,3 +78,8 @@ func keypairSAML(config JSONConfigurationSAML) (samlThings, error) {
 	}
 	return data, nil
 }
+
+// Function to serve as login redirect
+func loginSAML(w http.ResponseWriter, r *http.Request, samlConfig JSONConfigurationSAML) {
+	http.Redirect(w, r, samlConfig.LoginURL, http.StatusFound)
+}
diff --git a/admin/static/js/login.js b/admin/static/js/login.js
@@ -19,7 +19,7 @@ function sendLogout() {
   var data = {
     csrftoken: _csrf
   };
-  sendPostRequest(data, _url, '/login', false);
+  sendPostRequest(data, _url, '/logout', false);
 }
 
 $("#login_password").keyup(function(event) {