Skip to content
This repository has been archived by the owner on May 13, 2024. It is now read-only.

fix(auth): refactor current user handling into service #1201

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

czosel
Copy link
Contributor

@czosel czosel commented Apr 29, 2024

Writing instances of JS classes directly to the data property
of the ember simple auth service is problematic, because ember
simple auth always saves the serialized form of the class in
local storage. In our case, this caused issues when the application
is open in multiple tabs, and one tab triggers a token refresh: In that
case, the initialization of the class instance is not triggered, and the
serialized data is read directly from local storage.

This refactors the current implementation to use a separate service, as
suggested by the ember-simple-auth docs:
https://github.com/mainmatter/ember-simple-auth/blob/master/guides/managing-current-user.md

MitanOmar and others added 3 commits April 29, 2024 13:45
Writing instances of JS classes directly to the `data` property
of the ember simple auth service is problematic, because ember
simple auth always saves the _serialized_ form of the class in
local storage. In our case, this caused issues when the application
is open in multiple tabs, and on tab triggers a token refresh: In that
case, the initialization of the class instance is not triggered, and the
serialized data is read directly from local storage.

This refactors the current implementation to use a separate service, as
suggested by the ember-simple-auth docs:
https://github.com/mainmatter/ember-simple-auth/blob/master/guides/managing-current-user.md
@czosel czosel changed the title chore: bump simple auth fix(auth): refactor current user handling into service May 8, 2024
@czosel czosel requested a review from MitanOmar May 8, 2024 15:36
Copy link
Contributor

@derrabauke derrabauke left a comment

Choose a reason for hiding this comment

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

LGTM! 👌

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants