Fix access-token invalidation for missing relation

[bajtos]

Description

Fix the code invalidating access tokens on user email/password changes to correctly handle the case when the relation "AccessToken belongs to (subclassed) user" is not configured.

cc @ebarault @greaterweb

Related issues

• Problems extending User model with loopback v3.3.0 #3215

Checklist

• New tests added or existing tests modified to cover all changes
• Code conforms with the style
  guide

[ebarault]

👍
@bajtos : not clear to me what part from the multiuser patch specifically broke the default accesstoken --> user belongsto relation inheritance from base User?

[raymondfeng]

LGTM

[superkhau]

Minor nit about the comment/assertion, otherwise LGTM.

[superkhau]

I think we should use two arg style: http://loopback.io//doc/en/contrib/style-guide.html#asserting-a-rejected-promise

[superkhau]

You wrote the style guide entry though so you would know better. ;) What I'm suggesting is we should have something that catches and calls expect.fail() instead of the comment.

[bajtos]

What I'm suggesting is we should have something that catches and calls expect.fail() instead of the comment.

By calling expect.fail, the test would hide the original error and report an unhelpful failure.

Note that we are asserting a resolved promise here, not a rejected promise as is described in the style guide link you pasted.

I suppose I could rewrite this test as follows, would you find it easier to read?

.then(u => {
  u.email = 'updated@example.com';
  return u.save();
})
.then(u => {
  // the test passes when save() does not throw any error
});

[superkhau]

A bit better, but not worth the effort. LGTM. ;)

[greaterweb]

Good work on addressing this, @bajtos!

As far as this PR goes, I've tested this branch in my sample repo and another Loopback project of mine, in both cases the app is no longer crashing and the issue raised is resolved. I'm going to add some more discussions though to the original ticket as I want to clarify a few points on best practices.

[bajtos]

@ebarault @raymondfeng @superkhau thank you for the review!

@greaterweb thank you for verifying my fix in your codebase. Let's get it landed to address the immediate problem and then continue the discussion in the original issue.

[bajtos]

Released in loopback@3.4.0

[bajtos]

@ebarault

not clear to me what part from the multiuser patch specifically broke the default accesstoken --> user belongsto relation inheritance from base User?

It's not your patch that break the relation, it's the application that did not correctly configure this relation. Your patch was just not prepared to handle such misconfiguration, which I fixed here.