Skip to content

Conversation

@luncht1me
Copy link

Stringify and Parse currentUserData object on save / load.

-- Use Case: When modifying the user instance on a hook, ie afterLogin, with one-time computation of data that we would like persisted via LoopBackAuth storage.

Stringify and Parse currentUserData object on save / load.

-- Use Case: When modifying the user instance on a hook, ie afterLogin, with one-time computation of data that we would like persisted via LoopBackAuth storage.
@luncht1me luncht1me requested a review from bajtos as a code owner September 12, 2017 17:50
@luncht1me
Copy link
Author

I suppose this could be pulled into it's own branch if this functionality isn't desirable over all. But personally, I believe it to be a very handy feature to include. As long as storages are cleared properly this shouldn't pose any security risk.

@bajtos
Copy link
Member

bajtos commented Sep 13, 2017

Hello @luncht1me, thank you for the pull request.

To be honest, I am confused about why are these changes needed? Could you please write a unit-test demonstrating your use case, or at least paste some code snippets showing what are you trying to achieve?

@bajtos bajtos self-assigned this Sep 13, 2017
@luncht1me
Copy link
Author

@bajtos I'll loosely illustrate my specific use-case, why I made this change locally on a project I'm working on.

I have an AngularJS site which has an Admin Dashboard component to it, where the only users for the current scope are administrators.

I wanted a very quick way to show an admin's last login time, so I thought, why not just check their latest AccessToken's created date after login:

// user.js loopback model
User.afterRemote("login", function fetchLastAuth(ctx, instance, next) {
	User.relations.accessTokens.modelTo.findOne({
		order: 'created DESC',
		skip: 1,
		where: {
			userId: instance.userId
		}
	}, function (err, res) {
		if(err) return next(err);
		var created = res && res.created || null
		instance.__data.user.lastLogin = created;
		ctx.result.__data.user.lastLogin = created;

		next();
	})
});

When Angular sees the login result, I then set the modified user to LoopBackAuth, saving the currentUserData by

// AngularJS controller
// usr is caught by an event after login remote method
LoopBackAuth.setUser(LoopBackAuth.accessTokenId, usr.id, usr);
LoopBackAuth.save();

This way, my LoopBackAuth.currentUserData has the lastLogin property that I attached in the login afterRemote, which works great!

Until......

As soon as I refresh my browser, and LoopBackAuth loads, currentUserData and the property I attached to my user is now gone. I don't want to have to do a remote call every time I refresh the admin panel of the web app just to get the last login time when I've already resolved it and saved it to storage on login. My current token is still valid, so my data should still be valid.

Does this go against some design principals you guys have set? I mean, I could always save this data under a different storage key and manually load and clear it... Just figured since LoopBackAuth already had the data, that it would be simplest to allow the currentUserData prop to be saved and fetched and let Loopback clear it on logout automatically.

Let me know what you think may be a better solution to this use case. I'm sure I'm not the only one who would want to attach one-time properties on a user's login.

@bajtos
Copy link
Member

bajtos commented Sep 20, 2017

Thank you for the clarification, I think I am starting to understand your changes.

In the current design, applications should use User.getCurrent or User.getCachedCurrent to access the data of the currently logged-in user. I think it was our intention to avoid persisting this data, because user profile can change on the server between page loads.

As for your lastLogin implementation, it will show the admin the time when their current login session in their current browser started, it will not show the last time somebody logged into their account. In order to provide that information, you need to compute lastLogin value on the server by checking all access tokens associated with the current user. In which case it makes even more sense to not cache this data in the browser.

Thoughts?

@bajtos bajtos closed this Sep 20, 2017
@bajtos bajtos reopened this Sep 20, 2017
@strongloop strongloop deleted a comment from slnode Sep 20, 2017
@strongloop strongloop deleted a comment from slnode Sep 20, 2017
@strongloop strongloop deleted a comment from slnode Sep 20, 2017
@bajtos
Copy link
Member

bajtos commented Mar 23, 2018

I am closing this pull request as abandoned. Feel free to reopen if/when you get time to continue our discussion.

@bajtos bajtos closed this Mar 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants