Skip to content

Commit ecac935

Browse files
timgentclaude
timgent
and
claude
committed
Refactor client expiration check to use ISessionInternalInfo
Move client expiration data (clientExpiresAt) into ISessionInternalInfo so it flows through SessionInfoManager.get() → validateCurrentSession() → silentlyAuthenticate(), eliminating the need for a separate isClientExpired method, constructor changes, and extra mock plumbing in ClientAuthentication. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 3251a82 commit ecac935

9 files changed

Lines changed: 106 additions & 204 deletions

File tree

‎packages/browser/src/ClientAuthentication.spec.ts‎

Lines changed: 11 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -92,20 +92,17 @@ describe("ClientAuthentication", () => {
9292
logoutHandler: mockLogoutHandler(defaultMockStorage),
9393
sessionInfoManager: mockSessionInfoManager(defaultMockStorage),
9494
issuerConfigFetcher: mockDefaultIssuerConfigFetcher(),
95-
storage: defaultMockStorage,
9695
};
9796

9897
function getClientAuthentication(
9998
mocks: Partial<typeof defaultMocks> = defaultMocks,
10099
): ClientAuthentication {
101-
const storage = mocks.storage ?? defaultMocks.storage;
102100
return new ClientAuthentication(
103101
mocks.loginHandler ?? defaultMocks.loginHandler,
104102
mocks.redirectHandler ?? defaultMocks.redirectHandler,
105103
mocks.logoutHandler ?? defaultMocks.logoutHandler,
106104
mocks.sessionInfoManager ?? defaultMocks.sessionInfoManager,
107105
mocks.issuerConfigFetcher ?? defaultMocks.issuerConfigFetcher,
108-
storage,
109106
);
110107
}
111108

@@ -605,128 +602,36 @@ describe("ClientAuthentication", () => {
605602
});
606603
});
607604

608-
describe("isClientExpired", () => {
609-
it("returns true when a confidential client has an expired timestamp", async () => {
610-
const sessionId = "mySession";
611-
const expiredTimestamp = Math.floor(Date.now() / 1000) - 1000; // 1000 seconds ago
612-
const mockedStorage = new StorageUtility(
613-
mockStorage({
614-
[`${USER_SESSION_PREFIX}:${sessionId}`]: {
615-
isLoggedIn: "true",
616-
},
617-
}),
618-
mockStorage({
619-
[`${USER_SESSION_PREFIX}:${sessionId}`]: {
620-
clientId: "some-client-id",
621-
clientSecret: "some-secret",
622-
expiresAt: String(expiredTimestamp),
623-
},
624-
}),
625-
);
626-
const clientAuthn = getClientAuthentication({
627-
sessionInfoManager: mockSessionInfoManager(mockedStorage),
628-
storage: mockedStorage,
629-
});
630-
631-
await expect(clientAuthn.isClientExpired(sessionId)).resolves.toBe(true);
632-
});
633-
634-
it("returns false when a confidential client has a valid timestamp", async () => {
635-
const sessionId = "mySession";
636-
const futureTimestamp = Math.floor(Date.now() / 1000) + 10000; // 10000 seconds in future
637-
const mockedStorage = new StorageUtility(
638-
mockStorage({
639-
[`${USER_SESSION_PREFIX}:${sessionId}`]: {
640-
isLoggedIn: "true",
641-
},
642-
}),
643-
mockStorage({
644-
[`${USER_SESSION_PREFIX}:${sessionId}`]: {
645-
clientId: "some-client-id",
646-
clientSecret: "some-secret",
647-
expiresAt: String(futureTimestamp),
648-
},
649-
}),
650-
);
651-
const clientAuthn = getClientAuthentication({
652-
sessionInfoManager: mockSessionInfoManager(mockedStorage),
653-
storage: mockedStorage,
654-
});
655-
656-
await expect(clientAuthn.isClientExpired(sessionId)).resolves.toBe(false);
657-
});
658-
659-
it("returns false when a confidential client never expires (expiresAt = 0)", async () => {
605+
describe("validateCurrentSession", () => {
606+
it("returns clientExpiresAt when expiresAt is in storage", async () => {
660607
const sessionId = "mySession";
608+
const expiresAt = Math.floor(Date.now() / 1000) + 10000;
661609
const mockedStorage = new StorageUtility(
662610
mockStorage({
663611
[`${USER_SESSION_PREFIX}:${sessionId}`]: {
664612
isLoggedIn: "true",
613+
webId: "https://my.pod/profile#me",
665614
},
666615
}),
667616
mockStorage({
668617
[`${USER_SESSION_PREFIX}:${sessionId}`]: {
669-
clientId: "some-client-id",
618+
clientId: "https://some.app/registration",
670619
clientSecret: "some-secret",
671-
expiresAt: "0",
672-
},
673-
}),
674-
);
675-
const clientAuthn = getClientAuthentication({
676-
sessionInfoManager: mockSessionInfoManager(mockedStorage),
677-
storage: mockedStorage,
678-
});
679-
680-
await expect(clientAuthn.isClientExpired(sessionId)).resolves.toBe(false);
681-
});
682-
683-
it("returns false for public clients (no secret) regardless of expiration", async () => {
684-
const sessionId = "mySession";
685-
const expiredTimestamp = Math.floor(Date.now() / 1000) - 1000;
686-
const mockedStorage = new StorageUtility(
687-
mockStorage({
688-
[`${USER_SESSION_PREFIX}:${sessionId}`]: {
689-
isLoggedIn: "true",
690-
},
691-
}),
692-
mockStorage({
693-
[`${USER_SESSION_PREFIX}:${sessionId}`]: {
694-
clientId: "some-client-id",
695-
// No clientSecret - public client
696-
expiresAt: String(expiredTimestamp),
620+
issuer: "https://some.issuer",
621+
expiresAt: String(expiresAt),
697622
},
698623
}),
699624
);
700625
const clientAuthn = getClientAuthentication({
701626
sessionInfoManager: mockSessionInfoManager(mockedStorage),
702-
storage: mockedStorage,
703627
});
704628

705-
await expect(clientAuthn.isClientExpired(sessionId)).resolves.toBe(false);
706-
});
707-
708-
it("returns true for legacy clients with missing expiresAt (confidential)", async () => {
709-
const sessionId = "mySession";
710-
const mockedStorage = new StorageUtility(
711-
mockStorage({
712-
[`${USER_SESSION_PREFIX}:${sessionId}`]: {
713-
isLoggedIn: "true",
714-
},
715-
}),
716-
mockStorage({
717-
[`${USER_SESSION_PREFIX}:${sessionId}`]: {
718-
clientId: "some-client-id",
719-
clientSecret: "some-secret",
720-
// No expiresAt - legacy case
721-
},
629+
const result = await clientAuthn.validateCurrentSession(sessionId);
630+
expect(result).toStrictEqual(
631+
expect.objectContaining({
632+
clientExpiresAt: expiresAt,
722633
}),
723634
);
724-
const clientAuthn = getClientAuthentication({
725-
sessionInfoManager: mockSessionInfoManager(mockedStorage),
726-
storage: mockedStorage,
727-
});
728-
729-
await expect(clientAuthn.isClientExpired(sessionId)).resolves.toBe(true);
730635
});
731636
});
732637
});

‎packages/browser/src/ClientAuthentication.ts‎

Lines changed: 0 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,6 @@ import type {
2828
ISessionInfo,
2929
ISessionInternalInfo,
3030
ILoginOptions,
31-
IStorageUtility,
32-
ILoginHandler,
33-
IIncomingRedirectHandler,
34-
ILogoutHandler,
35-
ISessionInfoManager,
36-
IIssuerConfigFetcher,
3731
} from "@inrupt/solid-client-authn-core";
3832
import {
3933
EVENTS,
@@ -48,23 +42,6 @@ import type { EventEmitter } from "events";
4842
* @hidden
4943
*/
5044
export default class ClientAuthentication extends ClientAuthenticationBase {
51-
constructor(
52-
loginHandler: ILoginHandler,
53-
redirectHandler: IIncomingRedirectHandler,
54-
logoutHandler: ILogoutHandler,
55-
sessionInfoManager: ISessionInfoManager,
56-
issuerConfigFetcher: IIssuerConfigFetcher,
57-
protected storageUtility: IStorageUtility,
58-
) {
59-
super(
60-
loginHandler,
61-
redirectHandler,
62-
logoutHandler,
63-
sessionInfoManager,
64-
issuerConfigFetcher,
65-
);
66-
}
67-
6845
// Define these functions as properties so that they don't get accidentally re-bound.
6946
// Isn't Javascript fun?
7047
login = async (
@@ -118,38 +95,6 @@ export default class ClientAuthentication extends ClientAuthenticationBase {
11895
return sessionInfo;
11996
};
12097

121-
/**
122-
* Checks if the stored client registration has expired.
123-
* Returns true if the client is a confidential client (has a secret) and has expired.
124-
* Returns false for public clients or clients that haven't expired.
125-
*/
126-
isClientExpired = async (sessionId: string): Promise<boolean> => {
127-
const [clientSecret, expiresAt] = await Promise.all([
128-
this.storageUtility.getForUser(sessionId, "clientSecret", {
129-
secure: false,
130-
}),
131-
this.storageUtility.getForUser(sessionId, "expiresAt", { secure: false }),
132-
]);
133-
134-
// Expiration only applies to confidential clients (those with secrets)
135-
if (clientSecret === undefined) {
136-
return false;
137-
}
138-
139-
// -1 identifies legacy cases when a value should have been stored but wasn't
140-
// Treat as expired
141-
const expirationDate =
142-
expiresAt !== undefined ? Number.parseInt(expiresAt, 10) : -1;
143-
144-
// expirationDate === 0 means the client registration never expires
145-
if (expirationDate === 0) {
146-
return false;
147-
}
148-
149-
// Check if current time (in seconds) is past the expiration date
150-
return Math.floor(Date.now() / 1000) > expirationDate;
151-
};
152-
15398
handleIncomingRedirect = async (
15499
url: string,
155100
eventEmitter: EventEmitter,

‎packages/browser/src/Session.spec.ts‎

Lines changed: 10 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -458,21 +458,17 @@ describe("Session", () => {
458458
.mockReturnValueOnce(
459459
incomingRedirectPromise,
460460
) as typeof clientAuthentication.handleIncomingRedirect;
461-
const validateCurrentSessionPromise = Promise.resolve(
462-
"https://some.issuer/",
463-
);
461+
const validateCurrentSessionPromise = Promise.resolve({
462+
issuer: "https://some.issuer/",
463+
clientAppId: "some client ID",
464+
redirectUrl: "https://some.redirect/url",
465+
tokenType: "DPoP",
466+
});
464467
clientAuthentication.validateCurrentSession = jest
465468
.fn()
466469
.mockReturnValue(
467470
validateCurrentSessionPromise,
468471
) as typeof clientAuthentication.validateCurrentSession;
469-
// Mock isClientExpired to return false (not expired)
470-
const isClientExpiredPromise = Promise.resolve(false);
471-
clientAuthentication.isClientExpired = jest
472-
.fn()
473-
.mockReturnValue(
474-
isClientExpiredPromise,
475-
) as typeof clientAuthentication.isClientExpired;
476472

477473
const mySession = new Session({ clientAuthentication });
478474
// eslint-disable-next-line no-void
@@ -482,7 +478,6 @@ describe("Session", () => {
482478
});
483479
await incomingRedirectPromise;
484480
await validateCurrentSessionPromise;
485-
await isClientExpiredPromise;
486481
expect(window.localStorage.getItem(KEY_CURRENT_URL)).toBe(
487482
"https://mock.current/location",
488483
);
@@ -544,6 +539,7 @@ describe("Session", () => {
544539
issuer: "https://some.issuer",
545540
clientAppId: "some client ID",
546541
clientAppSecret: "some client secret",
542+
clientExpiresAt: Math.floor(Date.now() / 1000) + 10000,
547543
redirectUrl: "https://some.redirect/url",
548544
tokenType: "DPoP",
549545
});
@@ -552,13 +548,6 @@ describe("Session", () => {
552548
.mockReturnValue(
553549
validateCurrentSessionPromise,
554550
) as typeof clientAuthentication.validateCurrentSession;
555-
// Mock isClientExpired to return false (not expired)
556-
const isClientExpiredPromise = Promise.resolve(false);
557-
clientAuthentication.isClientExpired = jest
558-
.fn()
559-
.mockReturnValue(
560-
isClientExpiredPromise,
561-
) as typeof clientAuthentication.isClientExpired;
562551
const incomingRedirectPromise = Promise.resolve();
563552
clientAuthentication.handleIncomingRedirect = jest
564553
.fn()
@@ -575,7 +564,6 @@ describe("Session", () => {
575564
});
576565
await incomingRedirectPromise;
577566
await validateCurrentSessionPromise;
578-
await isClientExpiredPromise;
579567
expect(clientAuthentication.login).toHaveBeenCalledWith(
580568
{
581569
sessionId: "mySession",
@@ -796,11 +784,12 @@ describe("Session", () => {
796784
sessionInfoManager: mockSessionInfoManager(mockedStorage),
797785
});
798786

799-
// Mock validateCurrentSession to return valid session info
787+
// Mock validateCurrentSession to return session info with an expired client
800788
const validateCurrentSessionPromise = Promise.resolve({
801789
issuer: "https://some.issuer",
802790
clientAppId: "some client ID",
803791
clientAppSecret: "some client secret",
792+
clientExpiresAt: Math.floor(Date.now() / 1000) - 1000,
804793
redirectUrl: "https://some.redirect/url",
805794
tokenType: "DPoP",
806795
});
@@ -810,11 +799,6 @@ describe("Session", () => {
810799
validateCurrentSessionPromise,
811800
) as typeof clientAuthentication.validateCurrentSession;
812801

813-
// Mock isClientExpired to return true (expired client)
814-
clientAuthentication.isClientExpired = (
815-
jest.fn() as any
816-
).mockResolvedValue(true);
817-
818802
const incomingRedirectPromise = Promise.resolve();
819803
clientAuthentication.handleIncomingRedirect = jest
820804
.fn()
@@ -864,13 +848,10 @@ describe("Session", () => {
864848
issuer: "https://some.issuer",
865849
clientAppId: "some client ID",
866850
clientAppSecret: "some client secret",
851+
clientExpiresAt: Math.floor(Date.now() / 1000) - 1000,
867852
redirectUrl: "https://some.redirect/url",
868853
});
869854

870-
clientAuthentication.isClientExpired = (
871-
jest.fn() as any
872-
).mockResolvedValue(true);
873-
874855
clientAuthentication.handleIncomingRedirect = (
875856
jest.fn() as any
876857
).mockResolvedValue(undefined);

‎packages/browser/src/Session.ts‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -86,12 +86,16 @@ export async function silentlyAuthenticate(
8686
): Promise<boolean> {
8787
const storedSessionInfo = await clientAuthn.validateCurrentSession(sessionId);
8888
if (storedSessionInfo !== null) {
89-
// Check if the client registration has expired before attempting silent auth
90-
const clientExpired = await clientAuthn.isClientExpired(sessionId);
91-
if (clientExpired) {
92-
// Clear the stored session to prevent retry loops
93-
window.localStorage.removeItem(KEY_CURRENT_SESSION);
94-
return false;
89+
// Check if the client registration has expired before attempting silent auth.
90+
// Expiration only applies to confidential clients (those with a secret).
91+
// clientExpiresAt === 0 means the registration never expires.
92+
// clientExpiresAt === undefined with a secret means legacy data — treat as expired.
93+
if (storedSessionInfo.clientAppSecret !== undefined) {
94+
const expiresAt = storedSessionInfo.clientExpiresAt ?? -1;
95+
if (expiresAt !== 0 && Math.floor(Date.now() / 1000) > expiresAt) {
96+
window.localStorage.removeItem(KEY_CURRENT_SESSION);
97+
return false;
98+
}
9599
}
96100

97101
// It can be really useful to save the user's current browser location,

‎packages/browser/src/__mocks__/ClientAuthentication.ts‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
// SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
2020
//
2121

22-
import { jest } from "@jest/globals";
2322
import type {
2423
IIssuerConfigFetcher,
2524
ILoginHandler,
@@ -52,16 +51,11 @@ export const mockClientAuthentication = (
5251
mocks?: Partial<ClientAuthnDependencies>,
5352
): ClientAuthentication => {
5453
const storage = mocks?.storage ?? mockStorageUtility({});
55-
const clientAuthn = new ClientAuthentication(
54+
return new ClientAuthentication(
5655
mocks?.loginhandler ?? mockLoginHandler(),
5756
mocks?.redirectHandler ?? mockIncomingRedirectHandler(),
5857
mocks?.logoutHandler ?? mockLogoutHandler(storage),
5958
mocks?.sessionInfoManager ?? mockSessionInfoManager(storage),
6059
mocks?.issuerConfigFetcher ?? IssuerConfigFetcherMock,
61-
storage,
6260
);
63-
// Add default mock for isClientExpired
64-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
65-
clientAuthn.isClientExpired = (jest.fn() as any).mockResolvedValue(false);
66-
return clientAuthn;
6761
};

‎packages/browser/src/dependencies.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,5 @@ export function getClientAuthenticationWithDependencies(dependencies: {
105105
new IWaterfallLogoutHandler(sessionInfoManager, redirector),
106106
sessionInfoManager,
107107
issuerConfigFetcher,
108-
storageUtility,
109108
);
110109
}

0 commit comments

Comments
 (0)