fix: validate user email on creation

Validate the user email when creating a new identity.

Closes #312.

Signed-off-by: Utku Ozdemir <utku.ozdemir@siderolabs.com>

frontend/src/notification.ts

@@ -11,29 +11,37 @@ export type Notification = {
 
 export const notification: Ref<Notification | null> = shallowRef(null);
 
-export const showError = (title: string, body?: string) => {
+export const showError = (title: string, body?: string, ref?: Ref<Notification | null>) => {
   console.error(`error occurred: title: ${title}, body: ${body}`)
 
-  notification.value = {
+  if (!ref) {
+    ref = notification;
+  }
+
+  ref.value = {
     props: {
       title: title,
       body: body,
       type: "error",
       close: () => {
-        notification.value = null;
+        ref.value = null;
       }
     }
   }
 };
 
-export const showSuccess = (title: string, body?: string) => {
-  notification.value = {
+export const showSuccess = (title: string, body?: string, ref?: Ref<Notification | null>) => {
+  if (!ref) {
+    ref = notification;
+  }
+
+  ref.value = {
     props: {
       title: title,
       body: body,
       type: "success",
       close: () => {
-        notification.value = null;
+        ref.value = null;
       }
     }
   }

frontend/src/views/omni/Modals/UserCreate.vue

@@ -13,24 +13,29 @@ included in the LICENSE file.
       <close-button @click="close"/>
     </div>
 
-    <div class="flex gap-2 items-center">
-      <t-input title="User Email" class="flex-1 h-full" placeholder="..." v-model="identity"/>
-      <t-select-list
-          class="h-full"
-          title="Role"
-          :values="roles"
-          :defaultValue="RoleReader"
-          @checkedValue="value => role = value"
-      />
-      <t-button type="highlighted" @click="handleUserCreate" :disabled="!canManageUsers && authType !== AuthType.SAML" class="w-32 h-9">Create User</t-button>
+    <div class="flex flex-col w-full h-full gap-4">
+      <t-notification v-if="notification" v-bind="notification.props"/>
+
+      <div class="flex gap-2 items-center">
+        <t-input title="User Email" class="flex-1 h-full" placeholder="..." v-model="identity"/>
+        <t-select-list
+            class="h-full"
+            title="Role"
+            :values="roles"
+            :defaultValue="RoleReader"
+            @checkedValue="value => role = value"
+        />
+        <t-button type="highlighted" @click="handleUserCreate" :disabled="!canManageUsers && authType !== AuthType.SAML" class="w-32 h-9">Create User</t-button>
+      </div>
     </div>
+
   </div>
 </template>
 
 <script setup lang="ts">
-import { Ref, ref } from "vue";
+import { Ref, ref, shallowRef } from "vue";
 import { createUser } from "@/methods/user";
-import { showError, showSuccess } from "@/notification";
+import { Notification, showError, showSuccess } from "@/notification";
 import { useRouter } from "vue-router";
 import { RoleNone, RoleReader, RoleOperator, RoleAdmin } from "@/api/resources";
 
@@ -40,25 +45,28 @@ import { canManageUsers } from "@/methods/auth";
 import TSelectList from "@/components/common/SelectList/TSelectList.vue";
 import TInput from "@/components/common/TInput/TInput.vue";
 import { AuthType, authType } from "@/methods";
+import TNotification from "@/components/common/Notification/TNotification.vue";
+
+const notification: Ref<Notification | null> = shallowRef(null);
 
 const identity = ref("");
 const router = useRouter();
 
 const roles = [RoleNone, RoleReader, RoleOperator, RoleAdmin]
 
-const role: Ref<string> = ref(RoleReader);
+const role: Ref<string> = ref(RoleReader)
 
 const handleUserCreate = async () => {
   if (identity.value === "") {
-    showError("Failed to Create User", "User email is not defined");
+    showError("Failed to Create User", "User email is not defined", notification);
 
     return;
   }
 
   try {
     await createUser(identity.value, role.value);
   } catch (e) {
-    showError("Failed to Create User", e.message);
+    showError("Failed to Create User", e.message, notification);
 
     return;
   }

internal/backend/runtime/omni/export_test.go

@@ -30,8 +30,8 @@ func EtcdElections(ctx context.Context, client *clientv3.Client, electionKey str
 	return etcdElections(ctx, client, electionKey, logger, f)
 }
 
-func ClusterValidationOptions(st state.State, config *config.Params) []validated.StateOption {
-	return clusterValidationOptions(st, config)
+func ClusterValidationOptions(st state.State, etcdBackupConfig config.EtcdBackupParams, embeddedDiscoveryServiceConfig config.EmbeddedDiscoveryServiceParams) []validated.StateOption {
+	return clusterValidationOptions(st, etcdBackupConfig, embeddedDiscoveryServiceConfig)
 }
 
 func RelationLabelsValidationOptions() []validated.StateOption {
@@ -46,8 +46,8 @@ func MachineSetNodeValidationOptions(st state.State) []validated.StateOption {
 	return machineSetNodeValidationOptions(st)
 }
 
-func IdentityValidationOptions() []validated.StateOption {
-	return identityValidationOptions()
+func IdentityValidationOptions(samlConfig config.SAMLParams) []validated.StateOption {
+	return identityValidationOptions(samlConfig)
 }
 
 func ExposedServiceValidationOptions() []validated.StateOption {

internal/backend/runtime/omni/omni.go

@@ -270,14 +270,14 @@ func New(talosClientFactory *talos.ClientFactory, dnsService *dns.Service, workl
 
 	metricsRegistry.MustRegister(expvarCollector)
 
-	validationOptions := clusterValidationOptions(resourceState, config.Config)
+	validationOptions := clusterValidationOptions(resourceState, config.Config.EtcdBackup, config.Config.EmbeddedDiscoveryService)
 	validationOptions = append(validationOptions, relationLabelsValidationOptions()...)
 	validationOptions = append(validationOptions, accessPolicyValidationOptions()...)
 	validationOptions = append(validationOptions, aclValidationOptions(resourceState)...)
 	validationOptions = append(validationOptions, roleValidationOptions()...)
 	validationOptions = append(validationOptions, machineSetNodeValidationOptions(resourceState)...)
 	validationOptions = append(validationOptions, machineSetValidationOptions(resourceState, storeFactory)...)
-	validationOptions = append(validationOptions, identityValidationOptions()...)
+	validationOptions = append(validationOptions, identityValidationOptions(config.Config.Auth.SAML)...)
 	validationOptions = append(validationOptions, exposedServiceValidationOptions()...)
 	validationOptions = append(validationOptions, configPatchValidationOptions(resourceState)...)
 	validationOptions = append(validationOptions, etcdManualBackupValidationOptions()...)

internal/backend/runtime/omni/state_validation.go

@@ -9,6 +9,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"net/mail"
 	"strings"
 	"time"
 	"unicode"
@@ -36,11 +37,7 @@ import (
 // Validation is only syntactic - they are checked whether they are valid semver strings.
 //
 //nolint:gocognit,gocyclo,cyclop
-func clusterValidationOptions(st state.State, config *config.Params) []validated.StateOption {
-	if config == nil {
-		panic("config is nil")
-	}
-
+func clusterValidationOptions(st state.State, etcdBackupConfig config.EtcdBackupParams, embeddedDiscoveryServiceConfig config.EmbeddedDiscoveryServiceParams) []validated.StateOption {
 	validateVersions := func(ctx context.Context, res *omni.Cluster, skipTalosVersion, skipKubernetesVersion bool) error {
 		if skipTalosVersion && skipKubernetesVersion {
 			return nil
@@ -94,16 +91,16 @@ func clusterValidationOptions(st state.State, config *config.Params) []validated
 	validateBackupInterval := func(res *omni.Cluster) error {
 		if conf := res.TypedSpec().Value.GetBackupConfiguration(); conf != nil {
 			switch conf := conf.GetInterval().AsDuration(); {
-			case conf < config.EtcdBackup.MinInterval:
+			case conf < etcdBackupConfig.MinInterval:
 				return fmt.Errorf(
 					"backup interval must be greater than %s, actual %s",
-					config.EtcdBackup.MinInterval.String(),
+					etcdBackupConfig.MinInterval.String(),
 					conf.String(),
 				)
-			case conf > config.EtcdBackup.MaxInterval:
+			case conf > etcdBackupConfig.MaxInterval:
 				return fmt.Errorf(
 					"backup interval must be less than %s, actual %s",
-					config.EtcdBackup.MaxInterval.String(),
+					etcdBackupConfig.MaxInterval.String(),
 					conf.String(),
 				)
 			}
@@ -120,7 +117,7 @@ func clusterValidationOptions(st state.State, config *config.Params) []validated
 
 		// if this is a create operation or if the setting is changed, validate that the feature is available
 		if oldRes == nil || oldRes.TypedSpec().Value.GetFeatures().GetUseEmbeddedDiscoveryService() != newValue {
-			if !config.EmbeddedDiscoveryService.Enabled {
+			if !embeddedDiscoveryServiceConfig.Enabled {
 				return errors.New("embedded discovery service is not enabled")
 			}
 		}
@@ -600,17 +597,28 @@ func hasUppercaseLetters(s string) bool {
 	return false
 }
 
-func identityValidationOptions() []validated.StateOption {
+func identityValidationOptions(samlConfig config.SAMLParams) []validated.StateOption {
 	return []validated.StateOption{
-		validated.WithCreateValidations(validated.NewCreateValidationForType(func(_ context.Context, res *authres.Identity, _ ...state.CreateOption) error {
+		validated.WithCreateValidations(validated.NewCreateValidationForType(func(ctx context.Context, res *authres.Identity, _ ...state.CreateOption) error {
+			var errs error
+
 			if hasUppercaseLetters(res.Metadata().ID()) {
-				return errors.New("identity id must be lowercase")
+				errs = multierror.Append(errs, errors.New("must be lowercase"))
 			}
 
-			return nil
+			// allow non-email identities for internal actors and for users coming from the SAML provider
+			if samlConfig.Enabled || actor.ContextIsInternalActor(ctx) {
+				return nil
+			}
+
+			if _, err := mail.ParseAddress(res.Metadata().ID()); err != nil {
+				errs = multierror.Append(errs, fmt.Errorf("not a valid email address: "+res.Metadata().ID()))
+			}
+
+			return errs
 		})),
 		validated.WithUpdateValidations(validated.NewUpdateValidationForType(func(ctx context.Context, res *authres.Identity, newRes *authres.Identity, _ ...state.UpdateOption) error {
-			if !config.Config.Auth.SAML.Enabled || actor.ContextIsInternalActor(ctx) {
+			if !samlConfig.Enabled || actor.ContextIsInternalActor(ctx) {
 				return nil
 			}