use hard-coded names for saml certificates (#10341)

This preserves the ability to use custom file names -- required for backwards compatibility -- but forces names upon all newly uploaded certificates, avoiding clashes with other configuration files and skipping the need for file safety checks.

app/saml.go

@@ -15,6 +15,12 @@ import (
 	"github.com/mattermost/mattermost-server/utils/fileutils"
 )
 
+const (
+	SamlPublicCertificateName = "saml-public.crt"
+	SamlPrivateKeyName        = "saml-private.key"
+	SamlIdpCertificateName    = "saml-idp.crt"
+)
+
 func (a *App) GetSamlMetadata() (string, *model.AppError) {
 	if a.Saml == nil {
 		err := model.NewAppError("GetSamlMetadata", "api.admin.saml.not_available.app_error", nil, "", http.StatusNotImplemented)
@@ -28,13 +34,7 @@ func (a *App) GetSamlMetadata() (string, *model.AppError) {
 	return result, nil
 }
 
-func WriteSamlFile(fileData *multipart.FileHeader) *model.AppError {
-	filename := filepath.Base(fileData.Filename)
-
-	if filename == "." || filename == string(filepath.Separator) {
-		return model.NewAppError("AddSamlCertificate", "api.admin.add_certificate.saving.app_error", nil, "", http.StatusBadRequest)
-	}
-
+func WriteSamlFile(filename string, fileData *multipart.FileHeader) *model.AppError {
 	file, err := fileData.Open()
 	if err != nil {
 		return model.NewAppError("AddSamlCertificate", "api.admin.add_certificate.open.app_error", nil, err.Error(), http.StatusInternalServerError)
@@ -53,12 +53,12 @@ func WriteSamlFile(fileData *multipart.FileHeader) *model.AppError {
 }
 
 func (a *App) AddSamlPublicCertificate(fileData *multipart.FileHeader) *model.AppError {
-	if err := WriteSamlFile(fileData); err != nil {
+	if err := WriteSamlFile(SamlPublicCertificateName, fileData); err != nil {
 		return err
 	}
 
 	cfg := a.Config().Clone()
-	*cfg.SamlSettings.PublicCertificateFile = fileData.Filename
+	*cfg.SamlSettings.PublicCertificateFile = SamlPublicCertificateName
 
 	if err := cfg.IsValid(); err != nil {
 		return err
@@ -71,12 +71,12 @@ func (a *App) AddSamlPublicCertificate(fileData *multipart.FileHeader) *model.Ap
 }
 
 func (a *App) AddSamlPrivateCertificate(fileData *multipart.FileHeader) *model.AppError {
-	if err := WriteSamlFile(fileData); err != nil {
+	if err := WriteSamlFile(SamlPrivateKeyName, fileData); err != nil {
 		return err
 	}
 
 	cfg := a.Config().Clone()
-	*cfg.SamlSettings.PrivateKeyFile = fileData.Filename
+	*cfg.SamlSettings.PrivateKeyFile = SamlPrivateKeyName
 
 	if err := cfg.IsValid(); err != nil {
 		return err
@@ -89,12 +89,12 @@ func (a *App) AddSamlPrivateCertificate(fileData *multipart.FileHeader) *model.A
 }
 
 func (a *App) AddSamlIdpCertificate(fileData *multipart.FileHeader) *model.AppError {
-	if err := WriteSamlFile(fileData); err != nil {
+	if err := WriteSamlFile(SamlIdpCertificateName, fileData); err != nil {
 		return err
 	}
 
 	cfg := a.Config().Clone()
-	*cfg.SamlSettings.IdpCertificateFile = fileData.Filename
+	*cfg.SamlSettings.IdpCertificateFile = SamlIdpCertificateName
 
 	if err := cfg.IsValid(); err != nil {
 		return err
@@ -107,14 +107,8 @@ func (a *App) AddSamlIdpCertificate(fileData *multipart.FileHeader) *model.AppEr
 }
 
 func RemoveSamlFile(filename string) *model.AppError {
-	filename = filepath.Base(filename)
-
-	if filename == "." || filename == string(filepath.Separator) {
-		return model.NewAppError("AddSamlCertificate", "api.admin.remove_certificate.delete.app_error", nil, "", http.StatusBadRequest)
-	}
-
 	if err := os.Remove(fileutils.FindConfigFile(filename)); err != nil {
-		return model.NewAppError("removeCertificate", "api.admin.remove_certificate.delete.app_error", map[string]interface{}{"Filename": filename}, err.Error(), http.StatusInternalServerError)
+		return model.NewAppError("removeCertificate", "api.admin.remove_certificate.delete.app_error", map[string]interface{}{"Filename": filename}, filename+": "+err.Error(), http.StatusInternalServerError)
 	}
 
 	return nil

i18n/en.json

@@ -105,7 +105,7 @@
   },
   {
     "id": "api.admin.remove_certificate.delete.app_error",
-    "translation": "An error occurred while deleting the certificate. Make sure the file config/{{.Filename}} exists."
+    "translation": "An error occurred while deleting the certificate."
   },
   {
     "id": "api.admin.saml.metadata.app_error",

model/client4.go

@@ -2944,6 +2944,7 @@ func samlFileToMultipart(data []byte, filename string) ([]byte, *multipart.Write
 }
 
 // UploadSamlIdpCertificate will upload an IDP certificate for SAML and set the config to use it.
+// The filename parameter is deprecated and ignored: the server will pick a hard-coded filename when writing to disk.
 func (c *Client4) UploadSamlIdpCertificate(data []byte, filename string) (bool, *Response) {
 	body, writer, err := samlFileToMultipart(data, filename)
 	if err != nil {
@@ -2955,6 +2956,7 @@ func (c *Client4) UploadSamlIdpCertificate(data []byte, filename string) (bool,
 }
 
 // UploadSamlPublicCertificate will upload a public certificate for SAML and set the config to use it.
+// The filename parameter is deprecated and ignored: the server will pick a hard-coded filename when writing to disk.
 func (c *Client4) UploadSamlPublicCertificate(data []byte, filename string) (bool, *Response) {
 	body, writer, err := samlFileToMultipart(data, filename)
 	if err != nil {
@@ -2966,6 +2968,7 @@ func (c *Client4) UploadSamlPublicCertificate(data []byte, filename string) (boo
 }
 
 // UploadSamlPrivateCertificate will upload a private key for SAML and set the config to use it.
+// The filename parameter is deprecated and ignored: the server will pick a hard-coded filename when writing to disk.
 func (c *Client4) UploadSamlPrivateCertificate(data []byte, filename string) (bool, *Response) {
 	body, writer, err := samlFileToMultipart(data, filename)
 	if err != nil {