Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
132 changes: 131 additions & 1 deletion account-kit/signer/src/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ import {
type SessionManagerParams,
} from "./session/manager.js";
import type { SessionManagerEvents } from "./session/types";
import type { AuthParams } from "./signer";
import type { AuthParams, AuthStepResult } from "./signer";
import { SolanaSigner } from "./solanaSigner.js";
import {
AlchemySignerStatus,
Expand Down Expand Up @@ -312,6 +312,34 @@ export abstract class BaseAlchemySigner<TClient extends BaseSignerClient>
}
);

authenticateStep: (params: AuthParams) => Promise<AuthStepResult> =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit because this also starts and progresses the flow should the name be handleAuth, authenticateOrAdvance, or progressAuth?

Should we break up starting and progressing authentication into two calls? Such as login and verify?

SignerLogger.profiled(
"BaseAlchemySigner.authenticateStep",
async (params) => {
const { type } = params;
const result = (() => {
switch (type) {
case "email":
return this.authenticateWithEmailNew(params);
case "otp":
return this.authenticateWithOtpNew(params);
default:
throw new Error("type not implemented");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Minor) I'd recommend using assertNever to check that all types are handled, which also means explicitly writing out the other cases. That way, if we add a new type of auth step the type system will make us explicitly acknowledge it here. I'm thinking something like:

          switch (type) {
            case "email":
              return this.authenticateWithEmailNew(params);
            case "otp":
              return this.authenticateWithOtpNew(params);
            case "passkey":
            case "oauth":
            case "oauthReturn":
              throw new Error(`Unsupported auth step type: ${type}`);
            default:
              assertNever(type, `Unknown auth step type: ${type}`);
          }

Over time we've had a number of issues in the sdk where we added a new union arm and forgot to add handling for it somewhere, so IMO it's worth the extra hassle.

(I also suggest including the type in the error message to make debugging easier if this happens.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh yeah good call, will updated 👍

}
})();

this.trackAuthenticateType(params);

return result.catch((error) => {
this.store.setState({
error: toErrorInfo(error),
status: AlchemySignerStatus.DISCONNECTED,
});
throw error;
});
}
);

private trackAuthenticateType = (params: AuthParams) => {
const { type } = params;
switch (type) {
Expand Down Expand Up @@ -836,6 +864,53 @@ export abstract class BaseAlchemySigner<TClient extends BaseSignerClient>
return new SolanaSigner(this.inner);
};

private authenticateWithEmailNew = async (
params: Extract<AuthParams, { type: "email" }>
): Promise<AuthStepResult> => {
if ("bundle" in params) {
const user = await this.completeEmailAuth(params);
return {
status: AlchemySignerStatus.CONNECTED,
user,
};
}

if (!("email" in params)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check isn't strictly necessary because the type system should supposedly enforce that email is present in this branch, but doesn't hurt I suppose.

throw new Error("Email is required");
}

const { orgId, otpId, multiFactors, isNewUser } =
await this.initOrCreateEmailUser(
params.email,
params.emailMode ?? "otp",
params.multiFactors,
params.redirectParams
);

const isMfaRequired = multiFactors ? multiFactors?.length > 0 : false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Very nit) No need for the ?., we know multiFactors is present in that branch.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah good catch


this.sessionManager.setTemporarySession({
orgId,
isNewUser,
isMfaRequired,
});

this.store.setState({
status: AlchemySignerStatus.AWAITING_EMAIL_AUTH,
otpId,
error: null,
mfaStatus: {
mfaRequired: isMfaRequired,
mfaFactorId: multiFactors?.[0]?.multiFactorId,
},
});

return {
status: AlchemySignerStatus.AWAITING_EMAIL_AUTH,
user: undefined,
};
};

private authenticateWithEmail = async (
params: Extract<AuthParams, { type: "email" }>
): Promise<User> => {
Expand Down Expand Up @@ -993,6 +1068,61 @@ export abstract class BaseAlchemySigner<TClient extends BaseSignerClient>
return user;
};

private authenticateWithOtpNew = async (
args: Extract<AuthParams, { type: "otp" }>
): Promise<AuthStepResult> => {
const tempSession = this.sessionManager.getTemporarySession();
const { orgId, isNewUser, isMfaRequired } = tempSession ?? {};
const { otpId } = this.store.getState();
if (!orgId) {
throw new Error("orgId not found in session");
}
if (!otpId) {
throw new Error("otpId not found in session");
}
if (isMfaRequired && !args.multiFactors) {
throw new Error(`MFA is required.`);
}

const response = await this.inner.submitOtpCode({
orgId,
otpId,
otpCode: args.otpCode,
expirationSeconds: this.getExpirationSeconds(),
multiFactors: args.multiFactors,
});

if (response.mfaRequired) {
this.handleMfaRequired(response.encryptedPayload, response.multiFactors);

return {
status: AlchemySignerStatus.AWAITING_MFA_AUTH,
user: undefined,
multiFactors: response.multiFactors,
};
}

const user = await this.inner.completeAuthWithBundle({
bundle: response.bundle,
orgId,
connectedEventName: "connectedOtp",
authenticatingType: "otp",
});

this.emitNewUserEvent(isNewUser);
if (tempSession) {
this.sessionManager.setTemporarySession({
...tempSession,
isNewUser: false,
});
}

return {
user,
status: AlchemySignerStatus.CONNECTED,
};
};

private handleOauthReturn = ({
bundle,
orgId,
Expand Down
19 changes: 19 additions & 0 deletions account-kit/signer/src/signer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,27 @@ import {
import type {
CredentialCreationOptionOverrides,
VerifyMfaParams,
User,
MfaFactor,
} from "./client/types.js";
import { SessionManagerParamsSchema } from "./session/manager.js";
import type { AlchemySignerStatus } from "./types.js";

export type AuthStepResult =
| {
status: AlchemySignerStatus.CONNECTED;
user: User;
}
| {
status: AlchemySignerStatus.AWAITING_EMAIL_AUTH;
user: undefined;
}
| {
status: AlchemySignerStatus.AWAITING_MFA_AUTH;
user: undefined;
multiFactors: MfaFactor[];
// encryptedPayload: string; // I don't think we want to expose this to the user
};

export type AuthParams =
| {
Expand Down
Loading