Skip to content

Commit

Permalink
fix: improve mfa.enroll return types (#956)
Browse files Browse the repository at this point in the history
## What kind of change does this PR introduce?

Bug fix for the TypeScript types.

## What is the current behavior?

- Fixes #955

## What is the new behavior?

Use [TypeScript's function
overloading](https://www.typescriptlang.org/docs/handbook/2/functions.html#function-overloads)
to return

- TOTP data when `mfa.enroll` was passed `factorType: 'totp'`,
- phone data when `mfa.enroll` was passed `factorType: 'phone'`.

This fixes a breaking change in the types in
b957c30.

## Additional context

This PR would also expose the new following types:

- `MFAEnrollTOTPParams`
- `MFAEnrollPhoneParams`
- `AuthMFAEnrollTOTPResponse`
- `AuthMFAEnrollPhoneResponse`
- `AuthMFAEnrollErrorResponse` (it might be better to roll this type in
with the other two).

It might be better to keep these types private, since once they're
exposed, you can't easily remove them again without creating a breaking
change.
  • Loading branch information
aloisklink authored Sep 17, 2024
1 parent 96c7f09 commit 8a1ec06
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 63 deletions.
11 changes: 11 additions & 0 deletions src/GoTrueClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,11 @@ import type {
LockFunc,
UserIdentity,
SignInAnonymouslyCredentials,
MFAEnrollTOTPParams,
AuthMFAEnrollTOTPResponse,
AuthMFAEnrollErrorResponse,
MFAEnrollPhoneParams,
AuthMFAEnrollPhoneResponse,
} from './lib/types'

polyfillGlobalThis() // Make "globalThis" available
Expand Down Expand Up @@ -2353,6 +2358,12 @@ export default class GoTrueClient {
/**
* {@see GoTrueMFAApi#enroll}
*/
private async _enroll(
params: MFAEnrollTOTPParams
): Promise<AuthMFAEnrollTOTPResponse | AuthMFAEnrollErrorResponse>
private async _enroll(
params: MFAEnrollPhoneParams
): Promise<AuthMFAEnrollPhoneResponse | AuthMFAEnrollErrorResponse>
private async _enroll(params: MFAEnrollParams): Promise<AuthMFAEnrollResponse> {
try {
return await this._useSession(async (result) => {
Expand Down
135 changes: 72 additions & 63 deletions src/lib/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -800,23 +800,23 @@ export type GenerateLinkType =
| 'email_change_current'
| 'email_change_new'

export type MFAEnrollParams =
| {
/** The type of factor being enrolled. */
factorType: 'totp'
/** Domain which the user is enrolled with. */
issuer?: string
/** Human readable name assigned to the factor. */
friendlyName?: string
}
| {
/** The type of factor being enrolled. */
factorType: 'phone'
/** Human readable name assigned to the factor. */
friendlyName?: string
/** Phone number associated with a factor. Number should conform to E.164 format */
phone: string
}
export type MFAEnrollTOTPParams = {
/** The type of factor being enrolled. */
factorType: 'totp'
/** Domain which the user is enrolled with. */
issuer?: string
/** Human readable name assigned to the factor. */
friendlyName?: string
}
export type MFAEnrollPhoneParams = {
/** The type of factor being enrolled. */
factorType: 'phone'
/** Human readable name assigned to the factor. */
friendlyName?: string
/** Phone number associated with a factor. Number should conform to E.164 format */
phone: string
}
export type MFAEnrollParams = MFAEnrollTOTPParams | MFAEnrollPhoneParams

export type MFAUnenrollParams = {
/** ID of the factor being unenrolled. */
Expand Down Expand Up @@ -873,56 +873,59 @@ export type AuthMFAVerifyResponse =
error: AuthError
}

export type AuthMFAEnrollResponse =
| {
data: {
/** ID of the factor that was just enrolled (in an unverified state). */
id: string

/** Type of MFA factor.*/
type: 'totp'

/** TOTP enrollment information. */
totp: {
/** Contains a QR code encoding the authenticator URI. You can
* convert it to a URL by prepending `data:image/svg+xml;utf-8,` to
* the value. Avoid logging this value to the console. */
qr_code: string

/** The TOTP secret (also encoded in the QR code). Show this secret
* in a password-style field to the user, in case they are unable to
* scan the QR code. Avoid logging this value to the console. */
secret: string

/** The authenticator URI encoded within the QR code, should you need
* to use it. Avoid loggin this value to the console. */
uri: string
}
/** Friendly name of the factor, useful for distinguishing between factors **/
friendly_name?: string
}
error: null
export type AuthMFAEnrollTOTPResponse = {
data: {
/** ID of the factor that was just enrolled (in an unverified state). */
id: string

/** Type of MFA factor.*/
type: 'totp'

/** TOTP enrollment information. */
totp: {
/** Contains a QR code encoding the authenticator URI. You can
* convert it to a URL by prepending `data:image/svg+xml;utf-8,` to
* the value. Avoid logging this value to the console. */
qr_code: string

/** The TOTP secret (also encoded in the QR code). Show this secret
* in a password-style field to the user, in case they are unable to
* scan the QR code. Avoid logging this value to the console. */
secret: string

/** The authenticator URI encoded within the QR code, should you need
* to use it. Avoid loggin this value to the console. */
uri: string
}
| {
data: {
/** ID of the factor that was just enrolled (in an unverified state). */
id: string
/** Friendly name of the factor, useful for distinguishing between factors **/
friendly_name?: string
}
error: null
}
export type AuthMFAEnrollPhoneResponse = {
data: {
/** ID of the factor that was just enrolled (in an unverified state). */
id: string

/** Type of MFA factor. */
type: 'phone'
/** Type of MFA factor. */
type: 'phone'

/** Friendly name of the factor, useful for distinguishing between factors **/
friendly_name?: string
/** Friendly name of the factor, useful for distinguishing between factors **/
friendly_name?: string

/** Phone number of the MFA factor in E.164 format. Used to send messages */
phone: string
}
error: null
}
| {
data: null
error: AuthError
}
/** Phone number of the MFA factor in E.164 format. Used to send messages */
phone: string
}
error: null
}
export type AuthMFAEnrollErrorResponse = {
data: null
error: AuthError
}
export type AuthMFAEnrollResponse =
| AuthMFAEnrollTOTPResponse
| AuthMFAEnrollPhoneResponse
| AuthMFAEnrollErrorResponse

export type AuthMFAUnenrollResponse =
| {
Expand Down Expand Up @@ -1007,6 +1010,12 @@ export interface GoTrueMFAApi {
* Upon verifying a factor, all other sessions are logged out and the current session's authenticator level is promoted to `aal2`.
*
*/
enroll(
params: MFAEnrollTOTPParams
): Promise<AuthMFAEnrollTOTPResponse | AuthMFAEnrollErrorResponse>
enroll(
params: MFAEnrollPhoneParams
): Promise<AuthMFAEnrollPhoneResponse | AuthMFAEnrollErrorResponse>
enroll(params: MFAEnrollParams): Promise<AuthMFAEnrollResponse>

/**
Expand Down
14 changes: 14 additions & 0 deletions test/GoTrueClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -904,6 +904,20 @@ describe('User management', () => {
})
})

describe('MFA', () => {
test('enroll({factorType: "totp"}) returns totp', async () => {
const { data, error } = await authWithSession.mfa.enroll({
factorType: 'totp',
})

if (error) {
throw error
}

expect(data.totp.qr_code).not.toBeNull()
})
})

describe('GoTrueClient with storageisServer = true', () => {
const originalWarn = console.warn
let warnings: any[][] = []
Expand Down

0 comments on commit 8a1ec06

Please sign in to comment.