Skip to content

Commit cdaaa36

Browse files
authored
fix: prettify types and improved typesafety for MFA methods (#1116)
## What kind of change does this PR introduce? This is a preparatory PR for adding webauthn support, which will have different params/responses across the `client.mfa.{enroll/challenge/verify}()` methods. At the moment, some parameters in the calls are present in the type even when they are not valid for that factor/method type, and the response signature that the function returns does not match what's actually being returned. Also, in some instances of the types returned, we can assert some values in the return types as being one property of the valid union types in that property, for example: in the call to `client.mfa.listFactors()` an object returns which contains:` ```json all: [], totp: [], phone: [], ``` If the developer tries to iterate through this list, the `factor_type` will be a union of all the possible values in `.all`, and status will be `verified | unverified` which is correct. but in `.phone`, we *know* that it's of type `phone` and that its status is `verified`, thus, we can assert this in the type, the `Factor` signature has been updated to accept 2 generics (which are optional), which enable us to set specific shapes in different parts of the library, some other examples: - In `.enroll()`, we *know* for a *fact* that the `.type()` of the factor will match the `factorType` that was passed in the function call, - Similarly, in the `AuthMFAChallengeResponse`, we *know* for a fact that the `.type` property will always be `totp` or `phone` based on what parameters were passed. I believe these type revamps greately enhance the Developer Experience & enable the developer to not make any mistakes while accessing those fields. It will also enable us to add specific documentation/links that will appear in intellisense depending on which parameters were passed to each function. ## What is the current behavior? Most calls to the backend result in either: ```typescript type ExampleCalltoT = {data: T, error: null} | {data: null, error: AuthError} ``` or ```typescript type ExampleCalltoT = {data: T, error: null} | {data: ...keys of T set to null, error: AuthError} ``` Also, there was no distinction in calls to `.enroll()` and `.challenge()` ## What is the new behavior? - Consolidate response patterns into `RequestResult<T>` and `RequestResultSafeDestructure<T>` - Provide better type inference and IDE support with `Prettify<T>` utility - Add specific overloads to each function so that all the possible parameters show up in the IDE, then narrow down as soon as you lock into a specific parameter. - Update AMR Methods to include everything that's supported by the GoTrue backend. ### Key improvements 1. **Generic response utilities**: - `RequestResult<T>` - Standard result type with data/error pattern - `RequestResultSafeDestructure<T>` - Allows safe destructuring with null safety - `Prettify<T>` - Improves IDE hover experience by resolving mapped types 2. **Enhanced type safety**: - Stronger typing for MFA factor types using discriminated unions - Better type narrowing for factor verification status - Consistent error type handling across all responses 3. **Internally breaking change**: - `_callRefreshToken` now returns `{ data: Session, error: null }` instead of `{ session: Session, error: null }`, for consistency, this will not affect users as it's an internal utility (tests have been adjusted to accomodate this) ## Additional context This refactoring improves developer experience by: - Making response types more predictable and easier to work with - Providing better IntelliSense/autocomplete in IDEs - Reducing the likelihood of type-related bugs - Making the codebase more maintainable All existing tests pass with minimal adjustments to accommodate the internal API change, and to prepare for the next PR here that adds webauthn types, which require that some function calls have distinct parameters/responses to not confuse users and guide them to use the library correctly using the TYPES as a source of truth, so the developer can use the api naturally just through the types.
1 parent 568001b commit cdaaa36

File tree

6 files changed

+368
-436
lines changed

6 files changed

+368
-436
lines changed

packages/core/auth-js/src/GoTrueClient.ts

Lines changed: 26 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ import { polyfillGlobalThis } from './lib/polyfills'
5555
import { version } from './lib/version'
5656
import { LockAcquireTimeoutError, navigatorLock } from './lib/locks'
5757

58-
import type {
58+
import {
5959
AuthChangeEvent,
6060
AuthResponse,
6161
AuthResponsePassword,
@@ -113,6 +113,8 @@ import type {
113113
Web3Credentials,
114114
EthereumWeb3Credentials,
115115
EthereumWallet,
116+
FactorType,
117+
FactorTypes,
116118
} from './lib/types'
117119
import { stringToUint8Array, bytesToBase64URL } from './lib/base64url'
118120
import {
@@ -1578,7 +1580,7 @@ export default class GoTrueClient {
15781580
return { data: { session: currentSession }, error: null }
15791581
}
15801582

1581-
const { session, error } = await this._callRefreshToken(currentSession.refresh_token)
1583+
const { data: session, error } = await this._callRefreshToken(currentSession.refresh_token)
15821584
if (error) {
15831585
return { data: { session: null }, error }
15841586
}
@@ -1757,7 +1759,7 @@ export default class GoTrueClient {
17571759
}
17581760

17591761
if (hasExpired) {
1760-
const { session: refreshedSession, error } = await this._callRefreshToken(
1762+
const { data: refreshedSession, error } = await this._callRefreshToken(
17611763
currentSession.refresh_token
17621764
)
17631765
if (error) {
@@ -1827,7 +1829,7 @@ export default class GoTrueClient {
18271829
throw new AuthSessionMissingError()
18281830
}
18291831

1830-
const { session, error } = await this._callRefreshToken(currentSession.refresh_token)
1832+
const { data: session, error } = await this._callRefreshToken(currentSession.refresh_token)
18311833
if (error) {
18321834
return { data: { user: null, session: null }, error: error }
18331835
}
@@ -1963,7 +1965,7 @@ export default class GoTrueClient {
19631965
expires_in: expiresIn,
19641966
expires_at: expiresAt,
19651967
refresh_token,
1966-
token_type,
1968+
token_type: token_type as 'bearer',
19671969
user: data.user,
19681970
}
19691971

@@ -2478,7 +2480,7 @@ export default class GoTrueClient {
24782480
await this._saveSession(data.session)
24792481
await this._notifyAllSubscribers('TOKEN_REFRESHED', data.session)
24802482

2481-
const result = { session: data.session, error: null }
2483+
const result = { data: data.session, error: null }
24822484

24832485
this.refreshingDeferred.resolve(result)
24842486

@@ -2487,7 +2489,7 @@ export default class GoTrueClient {
24872489
this._debug(debugName, 'error', error)
24882490

24892491
if (isAuthError(error)) {
2490-
const result = { session: null, error }
2492+
const result = { data: null, error }
24912493

24922494
if (!isAuthRetryableFetchError(error)) {
24932495
await this._removeSession()
@@ -2997,7 +2999,9 @@ export default class GoTrueClient {
29972999
/**
29983000
* {@see GoTrueMFAApi#challenge}
29993001
*/
3000-
private async _challenge(params: MFAChallengeParams): Promise<AuthMFAChallengeResponse> {
3002+
private async _challenge<T extends FactorType>(
3003+
params: MFAChallengeParams
3004+
): Promise<AuthMFAChallengeResponse<T>> {
30013005
return this._acquireLock(-1, async () => {
30023006
try {
30033007
return await this._useSession(async (result) => {
@@ -3011,7 +3015,7 @@ export default class GoTrueClient {
30113015
'POST',
30123016
`${this.url}/factors/${params.factorId}/challenge`,
30133017
{
3014-
body: { channel: params.channel },
3018+
body: 'channel' in params ? { channel: params.channel } : {},
30153019
headers: this.headers,
30163020
jwt: sessionData?.session?.access_token,
30173021
}
@@ -3062,20 +3066,21 @@ export default class GoTrueClient {
30623066
return { data: null, error: userError }
30633067
}
30643068

3065-
const factors = user?.factors || []
3066-
const totp = factors.filter(
3067-
(factor) => factor.factor_type === 'totp' && factor.status === 'verified'
3068-
)
3069-
const phone = factors.filter(
3070-
(factor) => factor.factor_type === 'phone' && factor.status === 'verified'
3071-
)
3069+
const data: AuthMFAListFactorsResponse['data'] = {
3070+
all: [],
3071+
phone: [],
3072+
totp: [],
3073+
}
3074+
3075+
for (const factor of user?.factors ?? []) {
3076+
data.all.push(factor)
3077+
if (factor.status === 'verified') {
3078+
;(data[factor.factor_type] as typeof factor[]).push(factor)
3079+
}
3080+
}
30723081

30733082
return {
3074-
data: {
3075-
all: factors,
3076-
totp,
3077-
phone,
3078-
},
3083+
data,
30793084
error: null,
30803085
}
30813086
}

packages/core/auth-js/src/lib/errors.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ export class AuthWeakPasswordError extends CustomAuthError {
147147
*/
148148
reasons: WeakPasswordReasons[]
149149

150-
constructor(message: string, status: number, reasons: string[]) {
150+
constructor(message: string, status: number, reasons: WeakPasswordReasons[]) {
151151
super(message, 'AuthWeakPasswordError', status, 'weak_password')
152152

153153
this.reasons = reasons

0 commit comments

Comments
 (0)