Split the mutations and make them use an input object instead of different parameters

crates/data-model/src/users.rs

@@ -12,6 +12,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use std::ops::Deref;
+
 use chrono::{DateTime, Duration, Utc};
 use rand::{Rng, SeedableRng};
 use serde::Serialize;
@@ -131,6 +133,13 @@ pub enum UserEmailVerificationState {
     Valid,
 }
 
+impl UserEmailVerificationState {
+    #[must_use]
+    pub fn is_valid(&self) -> bool {
+        matches!(self, Self::Valid)
+    }
+}
+
 #[derive(Debug, Clone, PartialEq, Eq, Serialize)]
 pub struct UserEmailVerification {
     pub id: Ulid,
@@ -140,6 +149,14 @@ pub struct UserEmailVerification {
     pub state: UserEmailVerificationState,
 }
 
+impl Deref for UserEmailVerification {
+    type Target = UserEmailVerificationState;
+
+    fn deref(&self) -> &Self::Target {
+        &self.state
+    }
+}
+
 impl UserEmailVerification {
     #[must_use]
     pub fn samples(now: chrono::DateTime<Utc>, rng: &mut impl Rng) -> Vec<Self> {

crates/graphql/src/mutations/mod.rs

@@ -0,0 +1,28 @@
+// Copyright 2023 The Matrix.org Foundation C.I.C.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+mod user_email;
+
+use async_graphql::MergedObject;
+
+/// The mutations root of the GraphQL interface.
+#[derive(Default, MergedObject)]
+pub struct RootMutations(user_email::UserEmailMutations);
+
+impl RootMutations {
+    #[must_use]
+    pub fn new() -> Self {
+        Self::default()
+    }
+}

crates/graphql/src/mutations/user_email.rs

@@ -13,43 +13,54 @@
 // limitations under the License.
 
 use anyhow::Context as _;
-use async_graphql::{Context, Description, Object, ID};
-use mas_storage::{
-    job::{JobRepositoryExt, ProvisionUserJob, VerifyEmailJob},
-    user::UserEmailRepository,
-    RepositoryAccess,
-};
+use async_graphql::{Context, InputObject, Object, ID};
+use mas_storage::job::{JobRepositoryExt, ProvisionUserJob, VerifyEmailJob};
 
 use crate::{
     model::{NodeType, UserEmail},
     state::ContextExt,
 };
 
-/// The mutations root of the GraphQL interface.
-#[derive(Default, Description)]
-pub struct RootMutations {
+#[derive(Default)]
+pub struct UserEmailMutations {
     _private: (),
 }
 
-impl RootMutations {
-    #[must_use]
-    pub fn new() -> Self {
-        Self::default()
-    }
+/// The input for the `addEmail` mutation
+#[derive(InputObject)]
+struct AddEmailInput {
+    /// The email address to add
+    email: String,
+    /// The ID of the user to add the email address to
+    user_id: ID,
 }
 
-#[Object(use_type_description)]
-impl RootMutations {
+/// The input for the `sendVerificationEmail` mutation
+#[derive(InputObject)]
+struct SendVerificationEmailInput {
+    /// The ID of the email address to verify
+    user_email_id: ID,
+}
+
+/// The input for the `verifyEmail` mutation
+#[derive(InputObject)]
+struct VerifyEmailInput {
+    /// The ID of the email address to verify
+    user_email_id: ID,
+    /// The verification code
+    code: String,
+}
+
+#[Object]
+impl UserEmailMutations {
     /// Add an email address to the specified user
     async fn add_email(
         &self,
         ctx: &Context<'_>,
-
-        #[graphql(desc = "The email address to add")] email: String,
-        #[graphql(desc = "The ID of the user to add the email address to")] user_id: ID,
+        input: AddEmailInput,
     ) -> Result<UserEmail, async_graphql::Error> {
         let state = ctx.state();
-        let id = NodeType::User.extract_ulid(&user_id)?;
+        let id = NodeType::User.extract_ulid(&input.user_id)?;
         let requester = ctx.requester();
 
         let user = requester.user().context("Unauthorized")?;
@@ -63,14 +74,16 @@ impl RootMutations {
         // XXX: this logic should be extracted somewhere else, since most of it is
         // duplicated in mas_handlers
         // Find an existing email address
-        let existing_user_email = repo.user_email().find(user, &email).await?;
+        let existing_user_email = repo.user_email().find(user, &input.email).await?;
         let user_email = if let Some(user_email) = existing_user_email {
             user_email
         } else {
             let clock = state.clock();
             let mut rng = state.rng();
 
-            repo.user_email().add(&mut rng, &clock, user, email).await?
+            repo.user_email()
+                .add(&mut rng, &clock, user, input.email)
+                .await?
         };
 
         // Schedule a job to verify the email address if needed
@@ -89,11 +102,10 @@ impl RootMutations {
     async fn send_verification_email(
         &self,
         ctx: &Context<'_>,
-
-        #[graphql(desc = "The ID of the email address to verify")] user_email_id: ID,
+        input: SendVerificationEmailInput,
     ) -> Result<UserEmail, async_graphql::Error> {
         let state = ctx.state();
-        let user_email_id = NodeType::UserEmail.extract_ulid(&user_email_id)?;
+        let user_email_id = NodeType::UserEmail.extract_ulid(&input.user_email_id)?;
         let requester = ctx.requester();
         let user = requester.user().context("Unauthorized")?;
 
@@ -125,12 +137,10 @@ impl RootMutations {
     async fn verify_email(
         &self,
         ctx: &Context<'_>,
-
-        #[graphql(desc = "The ID of the email address to verify")] user_email_id: ID,
-        #[graphql(desc = "The verification code to submit")] code: String,
+        input: VerifyEmailInput,
     ) -> Result<UserEmail, async_graphql::Error> {
         let state = ctx.state();
-        let user_email_id = NodeType::UserEmail.extract_ulid(&user_email_id)?;
+        let user_email_id = NodeType::UserEmail.extract_ulid(&input.user_email_id)?;
         let requester = ctx.requester();
 
         let user = requester.user().context("Unauthorized")?;
@@ -148,16 +158,26 @@ impl RootMutations {
             return Err(async_graphql::Error::new("Unauthorized"));
         }
 
+        if user_email.confirmed_at.is_some() {
+            // Just return the email address if it's already verified
+            // XXX: should we return an error instead?
+            return Ok(UserEmail(user_email));
+        }
+
         // XXX: this logic should be extracted somewhere else, since most of it is
         // duplicated in mas_handlers
 
         // Find the verification code
         let verification = repo
             .user_email()
-            .find_verification_code(&clock, &user_email, &code)
+            .find_verification_code(&clock, &user_email, &input.code)
             .await?
             .context("Invalid verification code")?;
 
+        if verification.is_valid() {
+            return Err(async_graphql::Error::new("Invalid verification code"));
+        }
+
         // TODO: display nice errors if the code was already consumed or expired
         repo.user_email()
             .consume_verification_code(&clock, verification)

frontend/schema.graphql

@@ -1,3 +1,17 @@
+"""
+The input for the `addEmail` mutation
+"""
+input AddEmailInput {
+  """
+  The email address to add
+  """
+  email: String!
+  """
+  The ID of the user to add the email address to
+  """
+  userId: ID!
+}
+
 """
 An authentication records when a user enter their credential in a browser
 session.
@@ -298,15 +312,15 @@ type RootMutations {
   """
   Add an email address to the specified user
   """
-  addEmail(email: String!, userId: ID!): UserEmail!
+  addEmail(input: AddEmailInput!): UserEmail!
   """
   Send a verification code for an email address
   """
-  sendVerificationEmail(userEmailId: ID!): UserEmail!
+  sendVerificationEmail(input: SendVerificationEmailInput!): UserEmail!
   """
   Submit a verification code for an email address
   """
-  verifyEmail(userEmailId: ID!, code: String!): UserEmail!
+  verifyEmail(input: VerifyEmailInput!): UserEmail!
 }
 
 """
@@ -360,6 +374,16 @@ type RootQuery {
   node(id: ID!): Node
 }
 
+"""
+The input for the `sendVerificationEmail` mutation
+"""
+input SendVerificationEmailInput {
+  """
+  The ID of the email address to verify
+  """
+  userEmailId: ID!
+}
+
 type UpstreamOAuth2Link implements Node & CreationEvent {
   """
   ID of the object.
@@ -584,6 +608,20 @@ type UserEmailEdge {
   node: UserEmail!
 }
 
+"""
+The input for the `verifyEmail` mutation
+"""
+input VerifyEmailInput {
+  """
+  The ID of the email address to verify
+  """
+  userEmailId: ID!
+  """
+  The verification code
+  """
+  code: String!
+}
+
 schema {
   query: RootQuery
   mutation: RootMutations

frontend/src/gql/graphql.ts

@@ -22,6 +22,14 @@ export type Scalars = {
   Url: any;
 };
 
+/** The input for the `addEmail` mutation */
+export type AddEmailInput = {
+  /** The email address to add */
+  email: Scalars['String'];
+  /** The ID of the user to add the email address to */
+  userId: Scalars['ID'];
+};
+
 /**
  * An authentication records when a user enter their credential in a browser
  * session.
@@ -221,21 +229,19 @@ export type RootMutations = {
 
 /** The mutations root of the GraphQL interface. */
 export type RootMutationsAddEmailArgs = {
-  email: Scalars['String'];
-  userId: Scalars['ID'];
+  input: AddEmailInput;
 };
 
 
 /** The mutations root of the GraphQL interface. */
 export type RootMutationsSendVerificationEmailArgs = {
-  userEmailId: Scalars['ID'];
+  input: SendVerificationEmailInput;
 };
 
 
 /** The mutations root of the GraphQL interface. */
 export type RootMutationsVerifyEmailArgs = {
-  code: Scalars['String'];
-  userEmailId: Scalars['ID'];
+  input: VerifyEmailInput;
 };
 
 /** The query root of the GraphQL interface. */
@@ -314,6 +320,12 @@ export type RootQueryUserEmailArgs = {
   id: Scalars['ID'];
 };
 
+/** The input for the `sendVerificationEmail` mutation */
+export type SendVerificationEmailInput = {
+  /** The ID of the email address to verify */
+  userEmailId: Scalars['ID'];
+};
+
 export type UpstreamOAuth2Link = CreationEvent & Node & {
   __typename?: 'UpstreamOAuth2Link';
   /** When the object was created. */
@@ -481,6 +493,14 @@ export type UserEmailEdge = {
   node: UserEmail;
 };
 
+/** The input for the `verifyEmail` mutation */
+export type VerifyEmailInput = {
+  /** The verification code */
+  code: Scalars['String'];
+  /** The ID of the email address to verify */
+  userEmailId: Scalars['ID'];
+};
+
 export type BrowserSession_SessionFragment = { __typename?: 'BrowserSession', id: string, createdAt: any, lastAuthentication?: { __typename?: 'Authentication', id: string, createdAt: any } | null } & { ' $fragmentName'?: 'BrowserSession_SessionFragment' };
 
 export type BrowserSessionList_UserFragment = { __typename?: 'User', browserSessions: { __typename?: 'BrowserSessionConnection', edges: Array<{ __typename?: 'BrowserSessionEdge', cursor: string, node: (

frontend/src/gql/schema.ts

@@ -818,17 +818,7 @@ export default {
             },
             args: [
               {
-                name: "email",
-                type: {
-                  kind: "NON_NULL",
-                  ofType: {
-                    kind: "SCALAR",
-                    name: "Any",
-                  },
-                },
-              },
-              {
-                name: "userId",
+                name: "input",
                 type: {
                   kind: "NON_NULL",
                   ofType: {
@@ -851,7 +841,7 @@ export default {
             },
             args: [
               {
-                name: "userEmailId",
+                name: "input",
                 type: {
                   kind: "NON_NULL",
                   ofType: {
@@ -874,17 +864,7 @@ export default {
             },
             args: [
               {
-                name: "code",
-                type: {
-                  kind: "NON_NULL",
-                  ofType: {
-                    kind: "SCALAR",
-                    name: "Any",
-                  },
-                },
-              },
-              {
-                name: "userEmailId",
+                name: "input",
                 type: {
                   kind: "NON_NULL",
                   ofType: {