Skip to content

delete redundant txn cookies impl, tests and some linting changes #2077

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
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
238 changes: 128 additions & 110 deletions src/server/auth-client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -518,20 +518,22 @@ ca/T0LLtgmbMmxSv/MmzIg==
updatedSessionCookie!.value,
secret
);
expect(updatedSessionCookieValue).toEqual(expect.objectContaining({
user: {
sub: DEFAULT.sub
},
tokenSet: {
accessToken: "at_123",
refreshToken: "rt_123",
expiresAt: expect.any(Number)
},
internal: {
sid: DEFAULT.sid,
createdAt: expect.any(Number)
}
}));
expect(updatedSessionCookieValue).toEqual(
expect.objectContaining({
user: {
sub: DEFAULT.sub
},
tokenSet: {
accessToken: "at_123",
refreshToken: "rt_123",
expiresAt: expect.any(Number)
},
internal: {
sid: DEFAULT.sid,
createdAt: expect.any(Number)
}
})
);

// assert that the session expiry has been extended by the inactivity duration
expect(updatedSessionCookie?.maxAge).toEqual(1800);
Expand Down Expand Up @@ -1025,13 +1027,15 @@ ca/T0LLtgmbMmxSv/MmzIg==
expect(transactionCookie).toBeDefined();
expect(
(await decrypt(transactionCookie!.value, secret)).payload
).toEqual(expect.objectContaining({
nonce: authorizationUrl.searchParams.get("nonce"),
codeVerifier: expect.any(String),
responseType: "code",
state: authorizationUrl.searchParams.get("state"),
returnTo: "/"
}));
).toEqual(
expect.objectContaining({
nonce: authorizationUrl.searchParams.get("nonce"),
codeVerifier: expect.any(String),
responseType: "code",
state: authorizationUrl.searchParams.get("state"),
returnTo: "/"
})
);
});

it("should forward the configured authorization parameters to the authorization server", async () => {
Expand Down Expand Up @@ -1585,13 +1589,15 @@ ca/T0LLtgmbMmxSv/MmzIg==
expect(transactionCookie).toBeDefined();
expect(
(await decrypt(transactionCookie!.value, secret)).payload
).toEqual(expect.objectContaining({
nonce: expect.any(String),
codeVerifier: expect.any(String),
responseType: "code",
state,
returnTo: "/"
}));
).toEqual(
expect.objectContaining({
nonce: expect.any(String),
codeVerifier: expect.any(String),
responseType: "code",
state,
returnTo: "/"
})
);
});

describe("custom parameters to the authorization server", async () => {
Expand Down Expand Up @@ -1664,13 +1670,15 @@ ca/T0LLtgmbMmxSv/MmzIg==
expect(transactionCookie).toBeDefined();
expect(
(await decrypt(transactionCookie!.value, secret)).payload
).toEqual(expect.objectContaining({
nonce: expect.any(String),
codeVerifier: expect.any(String),
responseType: "code",
state,
returnTo: "/"
}));
).toEqual(
expect.objectContaining({
nonce: expect.any(String),
codeVerifier: expect.any(String),
responseType: "code",
state,
returnTo: "/"
})
);
});

it("should forward custom parameters set in the configuration to the authorization server", async () => {
Expand Down Expand Up @@ -1744,13 +1752,15 @@ ca/T0LLtgmbMmxSv/MmzIg==
expect(transactionCookie).toBeDefined();
expect(
(await decrypt(transactionCookie!.value, secret)).payload
).toEqual(expect.objectContaining({
nonce: expect.any(String),
codeVerifier: expect.any(String),
responseType: "code",
state,
returnTo: "/"
}));
).toEqual(
expect.objectContaining({
nonce: expect.any(String),
codeVerifier: expect.any(String),
responseType: "code",
state,
returnTo: "/"
})
);
});
});
});
Expand Down Expand Up @@ -2298,21 +2308,23 @@ ca/T0LLtgmbMmxSv/MmzIg==
const sessionCookie = response.cookies.get("__session");
expect(sessionCookie).toBeDefined();
const { payload: session } = await decrypt(sessionCookie!.value, secret);
expect(session).toEqual(expect.objectContaining({
user: {
sub: DEFAULT.sub
},
tokenSet: {
accessToken: DEFAULT.accessToken,
refreshToken: DEFAULT.refreshToken,
idToken: expect.stringMatching(/^eyJhbGciOiJSUzI1NiJ9\..+\..+$/),
expiresAt: expect.any(Number)
},
internal: {
sid: expect.any(String),
createdAt: expect.any(Number)
}
}));
expect(session).toEqual(
expect.objectContaining({
user: {
sub: DEFAULT.sub
},
tokenSet: {
accessToken: DEFAULT.accessToken,
refreshToken: DEFAULT.refreshToken,
idToken: expect.stringMatching(/^eyJhbGciOiJSUzI1NiJ9\..+\..+$/),
expiresAt: expect.any(Number)
},
internal: {
sid: expect.any(String),
createdAt: expect.any(Number)
}
})
);

// validate the transaction cookie has been removed
const transactionCookie = response.cookies.get(`__txn_${state}`);
Expand Down Expand Up @@ -2409,21 +2421,23 @@ ca/T0LLtgmbMmxSv/MmzIg==
const sessionCookie = response.cookies.get("__session");
expect(sessionCookie).toBeDefined();
const { payload: session } = await decrypt(sessionCookie!.value, secret);
expect(session).toEqual(expect.objectContaining({
user: {
sub: DEFAULT.sub
},
tokenSet: {
accessToken: DEFAULT.accessToken,
idToken: expect.any(String),
refreshToken: DEFAULT.refreshToken,
expiresAt: expect.any(Number)
},
internal: {
sid: expect.any(String),
createdAt: expect.any(Number)
}
}));
expect(session).toEqual(
expect.objectContaining({
user: {
sub: DEFAULT.sub
},
tokenSet: {
accessToken: DEFAULT.accessToken,
idToken: expect.any(String),
refreshToken: DEFAULT.refreshToken,
expiresAt: expect.any(Number)
},
internal: {
sid: expect.any(String),
createdAt: expect.any(Number)
}
})
);

// validate the transaction cookie has been removed
const transactionCookie = response.cookies.get(`__txn_${state}`);
Expand Down Expand Up @@ -2747,8 +2761,8 @@ ca/T0LLtgmbMmxSv/MmzIg==
returnTo: "/dashboard"
};
const maxAge = 60 * 60; // 1 hour
const expiration = Math.floor(Date.now() / 1000 + maxAge);
const expiration = Math.floor(Date.now() / 1000 + maxAge);

headers.set(
"cookie",
`__txn_${state}=${await encrypt(transactionState, secret, expiration)}`
Expand Down Expand Up @@ -3260,24 +3274,26 @@ ca/T0LLtgmbMmxSv/MmzIg==
sessionCookie!.value,
secret
);
expect(session).toEqual(expect.objectContaining({
user: {
sub: DEFAULT.sub,
name: "John Doe",
email: "[email protected]",
custom: "value"
},
tokenSet: {
accessToken: DEFAULT.accessToken,
refreshToken: DEFAULT.refreshToken,
idToken: expect.any(String),
expiresAt: expect.any(Number)
},
internal: {
sid: expect.any(String),
createdAt: expect.any(Number)
}
}));
expect(session).toEqual(
expect.objectContaining({
user: {
sub: DEFAULT.sub,
name: "John Doe",
email: "[email protected]",
custom: "value"
},
tokenSet: {
accessToken: DEFAULT.accessToken,
refreshToken: DEFAULT.refreshToken,
idToken: expect.any(String),
expiresAt: expect.any(Number)
},
internal: {
sid: expect.any(String),
createdAt: expect.any(Number)
}
})
);
});

it("should not call the hook if the session is not established", async () => {
Expand Down Expand Up @@ -3392,24 +3408,26 @@ ca/T0LLtgmbMmxSv/MmzIg==
sessionCookie!.value,
secret
);
expect(session).toEqual(expect.objectContaining({
user: {
sub: DEFAULT.sub,
name: "John Doe",
email: "[email protected]",
custom: "value"
},
tokenSet: {
accessToken: DEFAULT.accessToken,
refreshToken: DEFAULT.refreshToken,
idToken: expect.any(String),
expiresAt: expect.any(Number)
},
internal: {
sid: expect.any(String),
createdAt: expect.any(Number)
}
}));
expect(session).toEqual(
expect.objectContaining({
user: {
sub: DEFAULT.sub,
name: "John Doe",
email: "[email protected]",
custom: "value"
},
tokenSet: {
accessToken: DEFAULT.accessToken,
refreshToken: DEFAULT.refreshToken,
idToken: expect.any(String),
expiresAt: expect.any(Number)
},
internal: {
sid: expect.any(String),
createdAt: expect.any(Number)
}
})
);
});
});
});
Expand Down
7 changes: 7 additions & 0 deletions src/server/auth-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,10 @@ export class AuthClient {

const res = NextResponse.redirect(url);
await this.sessionStore.delete(req.cookies, res.cookies);

// Clear any orphaned transaction cookies
await this.transactionStore.deleteAll(req.cookies, res.cookies);

return res;
}

Expand All @@ -438,6 +442,9 @@ export class AuthClient {
const res = NextResponse.redirect(url);
await this.sessionStore.delete(req.cookies, res.cookies);

// Clear any orphaned transaction cookies
await this.transactionStore.deleteAll(req.cookies, res.cookies);

return res;
}

Expand Down
10 changes: 5 additions & 5 deletions src/server/cookies.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,24 +31,24 @@ describe("encrypt/decrypt", async () => {
const payload = { key: "value" };
const expiration = Math.floor(Date.now() / 1000 - 60); // 60 seconds in the past
const encrypted = await encrypt(payload, secret, expiration);
await expect(() =>
decrypt(encrypted, secret)
).rejects.toThrowError(`"exp" claim timestamp check failed`);
await expect(() => decrypt(encrypted, secret)).rejects.toThrowError(
`"exp" claim timestamp check failed`
);
});

it("should fail to encrypt if a secret is not provided", async () => {
const payload = { key: "value" };
const maxAge = 60 * 60; // 1 hour in seconds
const expiration = Math.floor(Date.now() / 1000 + maxAge);

await expect(() => encrypt(payload, "", expiration)).rejects.toThrowError();
});

it("should fail to decrypt if a secret is not provided", async () => {
const payload = { key: "value" };
const maxAge = 60 * 60; // 1 hour in seconds
const expiration = Math.floor(Date.now() / 1000 + maxAge);

const encrypted = await encrypt(payload, secret, expiration);
await expect(() => decrypt(encrypted, "")).rejects.toThrowError();
});
Expand Down
Loading