Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove internal protectIdentities flag #6606

Merged
merged 1 commit into from
Sep 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
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
5 changes: 5 additions & 0 deletions .changeset/cyan-wolves-cover.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@keystone-next/auth': patch
---

Removed the internal `protectIdentities` variable.
3 changes: 0 additions & 3 deletions packages/auth/src/gql/getBaseAuthSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,12 @@ export function getBaseAuthSchema<I extends string, S extends string>({
listKey,
identityField,
secretField,
protectIdentities,
gqlNames,
secretFieldImpl,
}: {
listKey: string;
identityField: I;
secretField: S;
protectIdentities: boolean;
gqlNames: AuthGqlNames;
secretFieldImpl: SecretFieldImpl;
}): GraphQLSchemaExtension {
Expand Down Expand Up @@ -65,7 +63,6 @@ export function getBaseAuthSchema<I extends string, S extends string>({
identityField,
args[identityField],
secretField,
protectIdentities,
args[secretField],
dbItemAPI
);
Expand Down
12 changes: 2 additions & 10 deletions packages/auth/src/gql/getMagicAuthLinkSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,12 @@ import { getAuthTokenErrorMessage } from '../lib/getErrorMessage';
export function getMagicAuthLinkSchema<I extends string>({
listKey,
identityField,
protectIdentities,
gqlNames,
magicAuthLink,
magicAuthTokenSecretFieldImpl,
}: {
listKey: string;
identityField: I;
protectIdentities: boolean;
gqlNames: AuthGqlNames;
magicAuthLink: AuthTokenTypeConfig;
magicAuthTokenSecretFieldImpl: SecretFieldImpl;
Expand Down Expand Up @@ -62,15 +60,10 @@ export function getMagicAuthLinkSchema<I extends string>({
const tokenType = 'magicAuth';
const identity = args[identityField];

const result = await createAuthToken(
identityField,
protectIdentities,
identity,
dbItemAPI
);
const result = await createAuthToken(identityField, identity, dbItemAPI);

// Note: `success` can be false with no code
// If protectIdentities === true then result.code will *always* be undefined.
// result.code will *always* be undefined.
if (!result.success && result.code) {
const message = getAuthTokenErrorMessage({
identityField,
Expand Down Expand Up @@ -115,7 +108,6 @@ export function getMagicAuthLinkSchema<I extends string>({
tokenType,
identityField,
args[identityField],
protectIdentities,
magicAuthLink.tokensValidForMins,
args.token,
dbItemAPI
Expand Down
13 changes: 2 additions & 11 deletions packages/auth/src/gql/getPasswordResetSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,13 @@ export function getPasswordResetSchema<I extends string, S extends string>({
listKey,
identityField,
secretField,
protectIdentities,
gqlNames,
passwordResetLink,
passwordResetTokenSecretFieldImpl,
}: {
listKey: string;
identityField: I;
secretField: S;
protectIdentities: boolean;
gqlNames: AuthGqlNames;
passwordResetLink: AuthTokenTypeConfig;
passwordResetTokenSecretFieldImpl: SecretFieldImpl;
Expand Down Expand Up @@ -67,15 +65,10 @@ export function getPasswordResetSchema<I extends string, S extends string>({
const tokenType = 'passwordReset';
const identity = args[identityField];

const result = await createAuthToken(
identityField,
protectIdentities,
identity,
dbItemAPI
);
const result = await createAuthToken(identityField, identity, dbItemAPI);

// Note: `success` can be false with no code
// If protectIdentities === true then result.code will *always* be undefined.
// result.code will *always* be undefined.
if (!result.success && result.code) {
const message = getAuthTokenErrorMessage({
identityField,
Expand Down Expand Up @@ -116,7 +109,6 @@ export function getPasswordResetSchema<I extends string, S extends string>({
tokenType,
identityField,
args[identityField],
protectIdentities,
passwordResetLink.tokensValidForMins,
args.token,
dbItemAPI
Expand Down Expand Up @@ -168,7 +160,6 @@ export function getPasswordResetSchema<I extends string, S extends string>({
tokenType,
identityField,
args[identityField],
protectIdentities,
passwordResetLink.tokensValidForMins,
args.token,
dbItemAPI
Expand Down
6 changes: 0 additions & 6 deletions packages/auth/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,6 @@ export function createAuth<GeneratedListTypes extends BaseGeneratedListTypes>({
passwordResetLink,
sessionData,
}: AuthConfig<GeneratedListTypes>) {
// The protectIdentities flag is currently under review to see whether it should be
// part of the createAuth API (in which case its use cases need to be documented and tested)
// or whether always being true is what we want, in which case we can refactor our code
// to match this. -TL
const protectIdentities = true;
const gqlNames: AuthGqlNames = {
// Core
authenticateItemWithPassword: `authenticate${listKey}WithPassword`,
Expand Down Expand Up @@ -163,7 +158,6 @@ export function createAuth<GeneratedListTypes extends BaseGeneratedListTypes>({
const extendGraphqlSchema = getSchemaExtension({
identityField,
listKey,
protectIdentities,
secretField,
gqlNames,
initFirstItem,
Expand Down
5 changes: 2 additions & 3 deletions packages/auth/src/lib/createAuthToken.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,18 @@ function generateToken(length: number): string {
// We don't (currently) make any effort to mitigate the time taken to record the new token or sent the email when successful
export async function createAuthToken(
identityField: string,
protectIdentities: boolean,
identity: string,
dbItemAPI: KeystoneDbAPI<any>[string]
): Promise<
| { success: false; code?: AuthTokenRequestErrorCode }
| { success: true; itemId: string | number; token: string }
> {
const match = await findMatchingIdentity(identityField, identity, dbItemAPI);
// Identity failures with helpful errors (unless it would violate our protectIdentities config)
// Identity failures with helpful errors.
if (match.success) {
return { success: true, itemId: match.item.id, token: generateToken(20) };
} else {
// There is no generic `AUTH_TOKEN_REQUEST_FAILURE` code; it's existance would alow values in the identity field to be probed
return { success: false, code: protectIdentities ? undefined : match.code };
return { success: false, code: undefined };
}
}
6 changes: 2 additions & 4 deletions packages/auth/src/lib/validateAuthToken.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ export async function validateAuthToken(
tokenType: 'passwordReset' | 'magicAuth',
identityField: string,
identity: string,
protectIdentities: boolean,
tokenValidMins: number | undefined,
token: string,
dbItemAPI: KeystoneDbAPI<any>[string]
Expand All @@ -28,23 +27,22 @@ export async function validateAuthToken(
identityField,
identity,
`${tokenType}Token`,
protectIdentities,
token,
dbItemAPI
);
if (!result.success) {
// Rewrite error codes
if (result.code === 'SECRET_NOT_SET') return { success: false, code: 'TOKEN_NOT_SET' };
if (result.code === 'SECRET_MISMATCH') return { success: false, code: 'TOKEN_MISMATCH' };
// Will generally be { success: false, code: 'FAILURE' } due to protectIdentities
// Will generally be { success: false, code: 'FAILURE' }
// Could be due to:
// - Missing identity
// - Missing secret
// - Secret mismatch.
return result as { success: false; code: AuthTokenRedemptionErrorCode };
}

// Now that we know the identity and token are valid, we can always return 'helpful' errors and stop worrying about protectIdentities
// Now that we know the identity and token are valid, we can always return 'helpful' errors and stop worrying about protecting identities.
const { item } = result;
const fieldKeys = { issuedAt: `${tokenType}IssuedAt`, redeemedAt: `${tokenType}RedeemedAt` };

Expand Down
10 changes: 3 additions & 7 deletions packages/auth/src/lib/validateSecret.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ export async function validateSecret(
identityField: string,
identity: string,
secretField: string,
protectIdentities: boolean,
secret: string,
dbItemAPI: KeystoneDbAPI<any>[string]
): Promise<
Expand All @@ -25,18 +24,15 @@ export async function validateSecret(

if (code) {
// See "Identity Protection" in the README as to why this is a thing
if (protectIdentities) {
await secretFieldImpl.generateHash('simulated-password-to-counter-timing-attack');
code = 'FAILURE';
}
return { success: false, code };
await secretFieldImpl.generateHash('simulated-password-to-counter-timing-attack');
return { success: false, code: 'FAILURE' };
}

const { item } = match as { success: true; item: { id: any; [prop: string]: any } };
if (await secretFieldImpl.compare(secret, item[secretField])) {
// Authenticated!
return { success: true, item };
} else {
return { success: false, code: protectIdentities ? 'FAILURE' : 'SECRET_MISMATCH' };
return { success: false, code: 'FAILURE' };
}
}
5 changes: 0 additions & 5 deletions packages/auth/src/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ export const getSchemaExtension =
({
identityField,
listKey,
protectIdentities,
secretField,
gqlNames,
initFirstItem,
Expand All @@ -51,7 +50,6 @@ export const getSchemaExtension =
}: {
identityField: string;
listKey: string;
protectIdentities: boolean;
secretField: string;
gqlNames: AuthGqlNames;
initFirstItem?: InitFirstItemConfig<any>;
Expand All @@ -78,7 +76,6 @@ export const getSchemaExtension =
getBaseAuthSchema({
identityField,
listKey,
protectIdentities,
secretField,
gqlNames,
secretFieldImpl: getSecretFieldImpl(schema, listKey, secretField),
Expand All @@ -95,7 +92,6 @@ export const getSchemaExtension =
getPasswordResetSchema({
identityField,
listKey,
protectIdentities,
secretField,
passwordResetLink,
gqlNames,
Expand All @@ -109,7 +105,6 @@ export const getSchemaExtension =
getMagicAuthLinkSchema({
identityField,
listKey,
protectIdentities,
magicAuthLink,
gqlNames,
magicAuthTokenSecretFieldImpl: getSecretFieldImpl(schema, listKey, 'magicAuthToken'),
Expand Down