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.

Sign up for GitHub

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

Jump to bottom

refactor: remove support for client registration access tokens #2151

Merged
merged 4 commits into from
Jun 1, 2022
Merged

refactor: remove support for client registration access tokens #2151

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
23a9463
fix: ensure tests always give full output
ThisIsMissEm May 20, 2022
93eb5c3
style: fix formatting of github actions
ThisIsMissEm May 20, 2022
1559682
refactor: remove support for client registration access tokens
ThisIsMissEm May 20, 2022
344e923
refactor: remove support for iframe-based session renewal
ThisIsMissEm May 20, 2022
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
1 change: 0 additions & 1 deletion .prettierrc.json
View file
Open in desktop

This file was deleted.

11 changes: 10 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,16 @@ The following changes have been implemented but not released yet:

### Breaking Changes

ThisIsMissEm marked this conversation as resolved.
Show resolved Hide resolved
- Support for Node.js v12.x has been dropped as that version has reached end-of-life.
- Support for Node.js v12.x has been dropped as that version has reached
end-of-life.
- We've cleaned up dynamic client registration by removing
support for the registrationAccessToken / initialAccessToken
when performing the registration flow as this feature was never fully
implemented. For Solid apps we recommend the use
of a [Public Client Identifier
Document](https://docs.inrupt.com/developer-tools/javascript/client-libraries/tutorial/authenticate-client/)
- We've also removed support for the iframe-based session renewal, which was
never fully implemented.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nicolasmondada does the above look good to you?


## 1.11.9 - 2022-05-25

Expand Down
1 change: 1 addition & 0 deletions jest.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ module.exports = {
// By default we only run unit tests:
"e2e/*",
],
reporters: ["default"],
collectCoverage: true,
coverageReporters: ["text"],
coverageThreshold: {
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"prepublishOnly": "npm run build",
"publish": "lerna publish",
"publish-preview": "lerna publish from-package",
"test": "lerna run --no-bail --ignore e2e-browser test",
"test": "lerna run --stream --concurrency=1 --no-prefix --no-bail --ignore e2e-browser test",
"test:e2e:node": "jest --config jest.e2e.config.js",
"test:e2e:browser": "cd e2e/browser && npm test",
"lerna-version": "lerna version",
Expand Down
181 changes: 0 additions & 181 deletions packages/browser/src/Session.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,6 @@ const mockLocation = (mockedLocation: string) => {
window.history.replaceState = jest.fn();
};

jest.mock("../src/iframe");

describe("Session", () => {
describe("constructor", () => {
it("accepts an empty config", async () => {
Expand Down Expand Up @@ -407,22 +405,6 @@ describe("Session", () => {
);
expect(incomingRedirectHandler).toHaveBeenCalled();
});

it("posts the redirect IRI to the parent if in an iframe", async () => {
// Pretend we are in an iframe.
const frameElement = jest.spyOn(window, "frameElement", "get");
frameElement.mockReturnValueOnce({} as Element);

const mySession = new Session({}, "mySession");
const iframe = jest.requireMock("../src/iframe");
const postIri = jest.spyOn(iframe as any, "postRedirectUrlToParent");
await mySession.handleIncomingRedirect({
url: "https://some.redirect.url?code=someCode&state=someState",
});
expect(postIri).toHaveBeenCalledWith(
"https://some.redirect.url?code=someCode&state=someState"
);
});
});

describe("onError", () => {
Expand Down Expand Up @@ -600,7 +582,6 @@ describe("Session", () => {
clientId: "some client ID",
clientSecret: "some client secret",
redirectUrl: "https://some.redirect/url",
inIframe: false,
},
expect.anything()
);
Expand Down Expand Up @@ -910,168 +891,6 @@ describe("Session", () => {
});
});

describe("on tokenRenewal signal", () => {
it("triggers silent authentication in an iframe when receiving the signal", async () => {
const sessionId = "mySession";
mockLocalStorage({
[KEY_CURRENT_SESSION]: sessionId,
});
mockLocation("https://mock.current/location");
const mockedStorage = new StorageUtility(
mockStorage({
[`${USER_SESSION_PREFIX}:${sessionId}`]: {
isLoggedIn: "true",
},
}),
mockStorage({})
);
const clientAuthentication = mockClientAuthentication({
sessionInfoManager: mockSessionInfoManager(mockedStorage),
});
const validateCurrentSessionPromise = Promise.resolve({
issuer: "https://some.issuer",
clientAppId: "some client ID",
clientAppSecret: "some client secret",
redirectUrl: "https://some.redirect/url",
tokenType: "DPoP",
});
clientAuthentication.validateCurrentSession = jest
.fn()
.mockReturnValue(
validateCurrentSessionPromise
) as typeof clientAuthentication.validateCurrentSession;
const incomingRedirectPromise = Promise.resolve();
clientAuthentication.handleIncomingRedirect = jest
.fn()
.mockReturnValueOnce(
incomingRedirectPromise
) as typeof clientAuthentication.handleIncomingRedirect;
clientAuthentication.login = jest.fn();
const mySession = new Session({ clientAuthentication }, sessionId);
// Send the signal to the session
mySession.emit("tokenRenewal");
await incomingRedirectPromise;
await validateCurrentSessionPromise;

expect(clientAuthentication.login).toHaveBeenCalledWith(
{
sessionId: "mySession",
tokenType: "DPoP",
oidcIssuer: "https://some.issuer",
prompt: "none",
clientId: "some client ID",
clientSecret: "some client secret",
redirectUrl: "https://some.redirect/url",
inIframe: true,
},
expect.anything()
);
// Check that second parameter is of type session
expect(
(clientAuthentication.login as any).mock.calls[0][1]
).toBeInstanceOf(Session);
});

it("sets the updated session info after silently refreshing", async () => {
const clientAuthentication = mockClientAuthentication();
const incomingRedirectPromise = Promise.resolve({
isLoggedIn: true,
webId: "https://some.pod/profile#me",
sessionId: "someSessionId",
expirationDate: 961106400,
});
clientAuthentication.handleIncomingRedirect = jest
.fn()
.mockReturnValueOnce(
incomingRedirectPromise
) as typeof clientAuthentication.handleIncomingRedirect;

const windowAddEventListener = jest.spyOn(window, "addEventListener");
// ../src/iframe is mocked for other tests,
// but we need `setupIframeListener` to actually be executed
// so that the callback gets called:
const iframeMock = jest.requireMock("../src/iframe") as any;
const iframeActual = jest.requireActual("../src/iframe") as any;
iframeMock.setupIframeListener.mockImplementationOnce(
iframeActual.setupIframeListener
);
const mySession = new Session({ clientAuthentication });

// `window.addEventListener` gets called once with ("message", handler)
// — get that handler.
const messageEventHandler = windowAddEventListener.mock
.calls[0][1] as EventListener;
const mockedEvent = {
origin: window.location.origin,
source: null,
data: {
redirectUrl: "http://arbitrary.com",
},
} as MessageEvent;
// This handler will call `clientAuthentication.handleIncomingRedirect`,
// which will return our sessionInfo values:
messageEventHandler(mockedEvent);

await incomingRedirectPromise;
expect(mySession.info.webId).toBe("https://some.pod/profile#me");
expect(mySession.info.sessionId).toBe("someSessionId");
expect(mySession.info.expirationDate).toBe(961106400);
});

it("does not change the existing session if silent authentication failed", async () => {
const clientAuthentication = mockClientAuthentication();
const incomingRedirectPromise = Promise.resolve({
isLoggedIn: false,
});
clientAuthentication.handleIncomingRedirect = jest
.fn()
.mockReturnValueOnce(
incomingRedirectPromise
) as typeof clientAuthentication.handleIncomingRedirect;

const windowAddEventListener = jest.spyOn(window, "addEventListener");
// ../src/iframe is mocked for other tests,
// but we need `setupIframeListener` to actually be executed
// so that the callback gets called:
const iframeMock = jest.requireMock("../src/iframe") as any;
const iframeActual = jest.requireActual("../src/iframe") as any;
iframeMock.setupIframeListener.mockImplementationOnce(
iframeActual.setupIframeListener
);
const mySession = new Session({ clientAuthentication });
// The `any` assertion is necessary because Session.info is not meant to
// be written to; we only do so for tests to pretend we have an existing
// logged-in session that remains logged in after failed silent
// authentication.
(mySession as any).info = {
isLoggedIn: true,
webId: "https://some.pod/profile#me",
sessionId: "someSessionId",
expirationDate: 961106400,
};

// `window.addEventListener` gets called once with ("message", handler)
// — get that handler.
const messageEventHandler = windowAddEventListener.mock
.calls[0][1] as EventListener;
const mockedEvent = {
origin: window.location.origin,
source: null,
data: {
redirectUrl: "http://arbitrary.com",
},
} as MessageEvent;
// This handler will call `clientAuthentication.handleIncomingRedirect`,
// which will return our sessionInfo values:
messageEventHandler(mockedEvent);

await incomingRedirectPromise;
expect(mySession.info.webId).toBe("https://some.pod/profile#me");
expect(mySession.info.sessionId).toBe("someSessionId");
expect(mySession.info.expirationDate).toBe(961106400);
});
});

describe("onSessionExpiration", () => {
it("calls the provided callback when receiving the appropriate event", async () => {
const myCallback = jest.fn();
Expand Down
48 changes: 0 additions & 48 deletions packages/browser/src/Session.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ import { v4 } from "uuid";
import ClientAuthentication from "./ClientAuthentication";
import { getClientAuthenticationWithDependencies } from "./dependencies";
import { KEY_CURRENT_SESSION, KEY_CURRENT_URL } from "./constant";
import { postRedirectUrlToParent, setupIframeListener } from "./iframe";

export interface ISessionOptions {
/**
Expand Down Expand Up @@ -98,11 +97,6 @@ export interface IHandleIncomingRedirectOptions {
export async function silentlyAuthenticate(
sessionId: string,
clientAuthn: ClientAuthentication,
options: {
inIframe?: boolean;
} = {
inIframe: false,
},
session: Session
): Promise<boolean> {
const storedSessionInfo = await clientAuthn.validateCurrentSession(sessionId);
Expand All @@ -121,7 +115,6 @@ export async function silentlyAuthenticate(
clientId: storedSessionInfo.clientAppId,
clientSecret: storedSessionInfo.clientAppSecret,
tokenType: storedSessionInfo.tokenType ?? "DPoP",
inIframe: options.inIframe,
},
session
);
Expand Down Expand Up @@ -194,36 +187,6 @@ export class Session extends EventEmitter {
};
}

// Listen for messages from children iframes.
setupIframeListener(async (redirectUrl: string) => {
const sessionInfo =
await this.clientAuthentication.handleIncomingRedirect(
redirectUrl,
this
);

// If silent authentication was not successful, do nothing;
// the existing session might still be valid for a while,
// and will expire by itself.
if (!isLoggedIn(sessionInfo)) {
return;
}
// After having revalidated the session,
// make sure to apply the new expiration time:
this.setSessionInfo(sessionInfo);
});
// Listen for the 'tokenRenewal' signal to trigger the silent token renewal.
this.on("tokenRenewal", () =>
silentlyAuthenticate(
this.info.sessionId,
this.clientAuthentication,
{
inIframe: true,
},
this
)
);

// When a session is logged in, we want to track its ID in local storage to
// enable silent refresh. The current session ID specifically stored in 'localStorage'
// (as opposed to using our storage abstraction layer) because it is only
Expand Down Expand Up @@ -316,13 +279,6 @@ export class Session extends EventEmitter {
typeof inputOptions === "string" ? { url: inputOptions } : inputOptions;
const url = options.url ?? window.location.href;

if (window.frameElement !== null) {
// This is being loaded from an iframe, so send the redirect
// URL to the parent window on the same origin.
postRedirectUrlToParent(url);
return undefined;
}

this.tokenRequestInProgress = true;
const sessionInfo = await this.clientAuthentication.handleIncomingRedirect(
url,
Expand Down Expand Up @@ -352,13 +308,9 @@ export class Session extends EventEmitter {
// ...if not, then there is no ID token, and so silent authentication cannot happen, but
// if we do have a stored session ID, attempt to re-authenticate now silently.
if (storedSessionId !== null) {
// TODO: iframe-based authentication being still experimental, it is disabled
// by default here. When it settles down, the following could be set to true,
// in which case the unresolving promise afterwards would need to be changed.
const attemptedSilentAuthentication = await silentlyAuthenticate(
storedSessionId,
this.clientAuthentication,
undefined,
this
);
// At this point, we know that the main window will imminently be redirected.
Expand Down
Loading