Skip to content

Commit 866276d

Browse files
Feature: formalize redirectTarget across stores. Add ability to set redirectTarget with requireAuthentication method (#3006)
* test: add tests for ember_simple_auth-redirectTarget for both with and without fastboot * test: improve visibility of FASTBOOT_DISABLED tests * test: allow adaptive store, store backend override. Generate tests for each type of store * wip: use macros to parameterize the session-store * chore: build test-app with correct session-store based on FASTBOOT_DISABLED env * feat(ember-simple-auth): add an ability to persist redirectTarget for all stores * test(test-esa): fix unit tests. Change cookieName, tests use the actual session-store:cookie for testing * test(test-app): add unit tests for redirectTarget
1 parent 29ebd33 commit 866276d

File tree

21 files changed

+334
-104
lines changed

21 files changed

+334
-104
lines changed

.github/workflows/ci.yml

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,11 @@ jobs:
101101
strategy:
102102
fail-fast: false
103103
matrix:
104+
FASTBOOT_DISABLED:
105+
# ember-cli-fastboot doesn't parse the `process.env.FASTBOOT_DISABLED` and we must set it to an empty string for 'false'
106+
- '' # false
107+
- 'true'
108+
104109
ember-version:
105110
- ember-lts-4.12
106111
- ember-lts-5.4
@@ -119,12 +124,12 @@ jobs:
119124
${{ runner.os }}-pnpm-
120125
121126
- run: pnpm install
122-
- name: test ${{ matrix.ember-version }}
127+
- name: (FASTBOOT_DISABLED=${{ matrix.FASTBOOT_DISABLED }}) ${{ matrix.ember-version }}
123128
env:
129+
FASTBOOT_DISABLED: ${{ matrix.FASTBOOT_DISABLED }}
124130
EMBER_SERVER_COMMAND: pnpm -F test-app test:one ${{ matrix.ember-version }} --- pnpm start
125131
run: pnpm -F playwright-tests test:e2e:chromium
126132

127-
128133
allow-fail-try-scenarios:
129134
name: ${{ matrix.workspace }} ${{ matrix.test-suite }} - Allowed to fail
130135
runs-on: ubuntu-latest

packages/ember-simple-auth/src/-internals/routing.js

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,20 +2,21 @@ import isFastBoot from '../utils/is-fastboot';
22
import location from '../utils/location';
33
import { isTesting } from '@embroider/macros';
44

5-
export function requireAuthentication(owner, transition) {
5+
export function requireAuthentication(owner, transition, extraArgs) {
66
let sessionService = owner.lookup('service:session');
77
let isAuthenticated = sessionService.get('isAuthenticated');
88
if (!isAuthenticated) {
9-
if (transition && isFastBoot(owner)) {
10-
const fastbootService = owner.lookup('service:fastboot');
11-
const cookiesService = owner.lookup('service:cookies');
12-
cookiesService.write('ember_simple_auth-redirectTarget', transition.intent.url, {
13-
path: '/',
14-
secure: fastbootService.get('request.protocol') === 'https',
15-
});
16-
} else if (transition) {
9+
const internalSession = sessionService.session;
10+
let redirectTarget = extraArgs?.redirectTarget;
11+
12+
if (transition) {
1713
sessionService.set('attemptedTransition', transition);
14+
15+
if (!redirectTarget) {
16+
redirectTarget = transition.intent.url;
17+
}
1818
}
19+
internalSession.setRedirectTarget(redirectTarget);
1920
}
2021
return isAuthenticated;
2122
}
@@ -33,8 +34,8 @@ export function prohibitAuthentication(owner, routeIfAlreadyAuthenticated) {
3334
export function handleSessionAuthenticated(owner, routeAfterAuthentication) {
3435
let sessionService = owner.lookup('service:session');
3536
let attemptedTransition = sessionService.get('attemptedTransition');
36-
let cookiesService = owner.lookup('service:cookies');
37-
const redirectTarget = cookiesService.read('ember_simple_auth-redirectTarget');
37+
const internalSession = sessionService.session;
38+
const redirectTarget = internalSession.getRedirectTarget();
3839

3940
let routerService = owner.lookup('service:router');
4041

@@ -43,7 +44,7 @@ export function handleSessionAuthenticated(owner, routeAfterAuthentication) {
4344
sessionService.set('attemptedTransition', null);
4445
} else if (redirectTarget) {
4546
routerService.transitionTo(redirectTarget);
46-
cookiesService.clear('ember_simple_auth-redirectTarget');
47+
internalSession.clearRedirectTarget();
4748
} else {
4849
routerService.transitionTo(routeAfterAuthentication);
4950
}

packages/ember-simple-auth/src/internal-session.js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ export default ObjectProxy.extend({
4949
isAuthenticated: false,
5050
attemptedTransition: null,
5151
sessionEvents: null,
52+
redirectTarget: null,
5253

5354
init() {
5455
this._super(...arguments);
@@ -285,4 +286,16 @@ export default ObjectProxy.extend({
285286
trigger(event, value) {
286287
this.sessionEvents.dispatchEvent(event, value);
287288
},
289+
290+
setRedirectTarget(url) {
291+
this.store.setRedirectTarget(url);
292+
},
293+
294+
getRedirectTarget() {
295+
return this.store.getRedirectTarget();
296+
},
297+
298+
clearRedirectTarget() {
299+
return this.store.clearRedirectTarget();
300+
},
288301
});

packages/ember-simple-auth/src/services/session.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
} from '../-internals/routing';
1313
import type Transition from '@ember/routing/transition';
1414
import { alias, readOnly } from '@ember/object/computed';
15+
import type EsaBaseSessionStore from '../session-stores/base';
1516

1617
const SESSION_DATA_KEY_PREFIX = /^data\./;
1718

@@ -38,6 +39,14 @@ type InternalSessionMock<Data> = {
3839
prohibitAuthentication: (routeOrCallback: RouteOrCallback) => boolean;
3940
restore: () => Promise<void>;
4041
set(key: string, value: any): void;
42+
setRedirectTarget: EsaBaseSessionStore['setRedirectTarget'];
43+
getRedirectTarget: EsaBaseSessionStore['getRedirectTarget'];
44+
clearRedirectTarget: EsaBaseSessionStore['clearRedirectTarget'];
45+
};
46+
47+
export type ExtraAuthenticationArgs = {
48+
redirectTarget?: string;
49+
[key: string]: unknown;
4150
};
4251

4352
export type DefaultDataShape = {
@@ -236,9 +245,13 @@ export default class SessionService<Data = DefaultDataShape> extends Service {
236245
@return {Boolean} true when the session is authenticated, false otherwise
237246
@public
238247
*/
239-
requireAuthentication(transition: Transition, routeOrCallback: RouteOrCallback) {
248+
requireAuthentication(
249+
transition: Transition,
250+
routeOrCallback: RouteOrCallback,
251+
extraArgs: ExtraAuthenticationArgs = {}
252+
) {
240253
assertSetupHasBeenCalled(this._setupIsCalled);
241-
let isAuthenticated = requireAuthentication(getOwner(this), transition);
254+
let isAuthenticated = requireAuthentication(getOwner(this), transition, extraArgs);
242255
if (!isAuthenticated) {
243256
if (typeof routeOrCallback === 'string') {
244257
triggerAuthentication(getOwner(this), routeOrCallback);

packages/ember-simple-auth/src/session-stores/adaptive.ts

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import Base from './base';
55
import type CookiesService from 'ember-cookies/services/cookies';
66
import type CookieStore from './cookie';
77
import type LocalStorageStore from './local-storage';
8+
import { debug } from '@ember/debug';
89

910
const LOCAL_STORAGE_TEST_KEY = '_ember_simple_auth_test_key';
1011

@@ -154,7 +155,7 @@ export default class AdaptiveStore extends Base {
154155

155156
declare _store: CookieStore | LocalStorageStore;
156157

157-
__isLocalStorageAvailable = null;
158+
__isLocalStorageAvailable: boolean | null = null;
158159
get _isLocalStorageAvailable() {
159160
if (this.__isLocalStorageAvailable !== null) {
160161
return this.__isLocalStorageAvailable;
@@ -163,8 +164,15 @@ export default class AdaptiveStore extends Base {
163164
}
164165
}
165166

166-
init(properties: any) {
167-
super.init(properties);
167+
init(emberOwner: any, __isLocalStorageAvailable?: boolean | null) {
168+
super.init(emberOwner);
169+
170+
if (typeof __isLocalStorageAvailable == 'boolean') {
171+
this.__isLocalStorageAvailable = __isLocalStorageAvailable;
172+
debug(
173+
`ember-simple-auth: session-store received __isLocalStorageAvailable: ${__isLocalStorageAvailable}`
174+
);
175+
}
168176

169177
let owner = getOwner(this) as any;
170178
if (owner && !this.hasOwnProperty('_fastboot')) {
@@ -247,6 +255,18 @@ export default class AdaptiveStore extends Base {
247255
clear() {
248256
return this.get('_store').clear();
249257
}
258+
259+
setRedirectTarget(url: string) {
260+
this.get('_store').setRedirectTarget(url);
261+
}
262+
263+
getRedirectTarget() {
264+
return this.get('_store').getRedirectTarget();
265+
}
266+
267+
clearRedirectTarget() {
268+
return this.get('_store').clearRedirectTarget();
269+
}
250270
}
251271

252272
function testLocalStorageAvailable() {

packages/ember-simple-auth/src/session-stores/base.ts

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ class SessionStoreEventTarget extends EsaEventTarget<SessionEvents> {}
1818
@extends Ember.Object
1919
@public
2020
*/
21-
export default class EsaBaseSessionStore extends EmberObject {
21+
export default abstract class EsaBaseSessionStore extends EmberObject {
2222
sessionStoreEvents = new SessionStoreEventTarget();
2323
/**
2424
Triggered when the session store's data changes due to an external event,
@@ -45,9 +45,7 @@ export default class EsaBaseSessionStore extends EmberObject {
4545
@return {Promise} A promise that resolves when the data has successfully been persisted and rejects otherwise.
4646
@public
4747
*/
48-
persist(..._args: any[]): Promise<unknown> {
49-
return Promise.reject();
50-
}
48+
abstract persist(..._args: any[]): Promise<unknown>;
5149

5250
/**
5351
Returns all data currently stored as a plain object.
@@ -60,9 +58,7 @@ export default class EsaBaseSessionStore extends EmberObject {
6058
@return {Promise} A promise that resolves with the data currently persisted in the store when the data has been restored successfully and rejects otherwise.
6159
@public
6260
*/
63-
restore(..._args: any[]): Promise<unknown> {
64-
return Promise.reject();
65-
}
61+
abstract restore(): Promise<unknown>;
6662

6763
/**
6864
Clears the store.
@@ -75,9 +71,11 @@ export default class EsaBaseSessionStore extends EmberObject {
7571
@return {Promise} A promise that resolves when the store has been cleared successfully and rejects otherwise.
7672
@public
7773
*/
78-
clear(..._args: any[]): Promise<unknown> {
79-
return Promise.reject();
80-
}
74+
abstract clear(): Promise<unknown>;
75+
76+
abstract setRedirectTarget(urL: string): void;
77+
abstract getRedirectTarget(): string | null;
78+
abstract clearRedirectTarget(): void;
8179

8280
on<Event extends keyof SessionEvents>(event: Event, cb: EventListener<SessionEvents, Event>) {
8381
this.sessionStoreEvents.addEventListener(event, cb);

packages/ember-simple-auth/src/session-stores/cookie.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,4 +368,19 @@ export default class CookieStore extends BaseStore {
368368
this._write(data, expiration);
369369
}
370370
}
371+
372+
setRedirectTarget(url: string) {
373+
this.get('_cookies').write(`${this.cookieName}-redirectTarget`, url, {
374+
path: '/',
375+
secure: this._secureCookies(),
376+
});
377+
}
378+
379+
getRedirectTarget() {
380+
return this.get('_cookies').read(`${this.cookieName}-redirectTarget`) || null;
381+
}
382+
383+
clearRedirectTarget() {
384+
return this.get('_cookies').clear(`${this.cookieName}-redirectTarget`);
385+
}
371386
}

packages/ember-simple-auth/src/session-stores/ephemeral.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,4 +61,14 @@ export default class EphemeralStore extends BaseStore {
6161

6262
return Promise.resolve();
6363
}
64+
65+
setRedirectTarget() {
66+
// noop
67+
}
68+
getRedirectTarget() {
69+
return null;
70+
}
71+
clearRedirectTarget() {
72+
return null;
73+
}
6474
}

packages/ember-simple-auth/src/session-stores/local-storage.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,4 +113,16 @@ export default class LocalStorageStore extends BaseStore {
113113
});
114114
}
115115
}
116+
117+
setRedirectTarget(url: string) {
118+
localStorage.setItem(`${this.key}-redirectTarget`, url);
119+
}
120+
121+
getRedirectTarget() {
122+
return localStorage.getItem(`${this.key}-redirectTarget`);
123+
}
124+
125+
clearRedirectTarget() {
126+
return localStorage.removeItem(`${this.key}-redirectTarget`);
127+
}
116128
}

packages/ember-simple-auth/src/session-stores/session-storage.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,4 +108,16 @@ export default class SessionStorageStore extends BaseStore {
108108
});
109109
}
110110
}
111+
112+
setRedirectTarget(url: string) {
113+
sessionStorage.setItem(`${this.key}-redirectTarget`, url);
114+
}
115+
116+
getRedirectTarget() {
117+
return sessionStorage.getItem(`${this.key}-redirectTarget`);
118+
}
119+
120+
clearRedirectTarget() {
121+
return sessionStorage.removeItem(`${this.key}-redirectTarget`);
122+
}
111123
}

0 commit comments

Comments
 (0)