[POC] add refreshtoken to extend accessToken #537

derdeka · 2020-02-07T15:58:10Z

Proof of concept for loopbackio/loopback-next#4573

Early feedback welcome.

Basic Idea:

Short-lived accessToken (e.g. 10 minutes) and very long-lived refreshToken (e.g. 48 hours) which are issued at login time
the accessToken giving access to resources
the refreshToken can be used to to create a new accessToken
the refreshToken can be revoked, e.g. by logout

Signed-off-by: derdeka <[email protected]>

derdeka · 2020-02-10T15:34:08Z

Requesting feedback on this PR. If this is the right direction, i'll add some tests.

dougal83

I think perhaps dropping ttl and possibly even creation date from refresh token model and replacing with a straight expiry date might be better. Extending token could be a reissue with updated expiry date. Revocation of token would be setting the expiry date in the past, forcing a login. Hope that makes sense. I'll double check tomorrow. But it makes sense to me right now.

dougal83 · 2020-02-12T23:55:15Z

packages/shopping/src/models/user.model.ts

@@ -51,6 +52,9 @@ export class User extends Entity {
  @hasOne(() => UserCredentials)
  userCredentials: UserCredentials;

+  @hasMany(() => UserRefreshtoken)


Why not hasOne? Can user have multiple?

Currently the refreshToken is newly generated at login time. As you can login multiple times or from multiple browsers/devices this needs to be a hasMany relation.

Alternatively we could share the refreshToken accross browsers/devices, than a hasOne would fit. But this might decrease security!?

WDYT?

For simplicity, sharing a single token across devices would have a single point of failure, any detected breach would log out all devices. Could be annoying to user.

Your solution is more flexible/complex, but might think about identifying the authorised device linked to the refresh token maybe? Identifying devices, allows the token to be individually invalidated (i.e. Lost device can have access revoked, although any serious security will remote wipe devices). If one token is compromised, is there any practical difference in access compromised? To get updated refresh token we need to log in regardless don't we? Hmm will need to think more.

Both solutions allow:

Local - individual devices to be logged out(delete local token)

Server - all devices to be logged out(expire one token vs. expire all tokens)

Server - update permissions (update one token vs. update all tokens)

Multiple refresh token adds:

Server - log out single device(given identity is known, expire the individual token)

Do what you think fits the scenario best. I'm just offering thoughts for consideration.

achrinza

@dougal83 +1 for using an expiration instead of TTL. As per-RF7519, the claim should be named exp.

emonddr · 2020-02-13T14:26:48Z

packages/shopping/src/controllers/specs/user-controller.specs.ts

+  properties: {
+    refreshtoken: {
+      type: 'string',
+      minLength: 8,


minLength: 8

Usually the jwt tokens we generate are very long. What is the reasoning for 8? Why not 1? Or why specify this at all? Just curious

@emonddr If you look at the generateToken function in
https://github.com/strongloop/loopback4-example-shopping/pull/537/files#diff-90f4ee64422c3d610fc44a185e6d0781R47

A refresh token is the id of a UserRefreshToken record, not a token generated by jwt service.

I would prefer a pattern check to the token, but i'm still not sure what is the best way to generate a refreshToken. Currently it's a objectId which might not be the best practice. On a mysql/postgresql database i would just use a UUID as refreshToken and use a regular expression for validation.

Do you have any other ideas how to generate a revokeable token?

emonddr · 2020-02-13T14:36:09Z

packages/shopping/src/services/refreshtoken.service.ts

@@ -0,0 +1,69 @@
+// Copyright IBM Corp. 2020. All Rights Reserved.


@derdeka , cool stuff. I was going to just increase the token lifetime from 10 minutes to 6 hours in my last PR ;).

I am wondering :

are you going to add mocha tests?

have you interacted with the shopping cart API Explorer to see how this will affect the user experience?
Developers may want to use the API Explorer to interact with the REST API to debug how things work
for extended periods of time. It would annoy me when the jwt token would expire in 10 minutes. So I would
set it for 6 hours while I debug and play with the REST API. How will these changes affect this?

What needs to change in the Shoppy application (UI front-end) that was recently added?

@emonddr All good questions. Above @derdeka said he'd add tests after initial feedback.

I think the general idea is for the refresh token to hold the session open on a device for a period of time(say 6 hours, two days or even longer) without the need to log back in. When your access token runs out after 10 minutes, then the refresh token would be used to get new access token without the need to log in again. This would also need to be designed so that the refresh token can be revoked and force a login. Is that correct @derdeka?

are you going to add mocha tests?

I'll add some tests.

have you interacted with the shopping cart API Explorer to see how this will affect the user experience?

On a angular application i would use a serviceworker or service to periodically refresh the accesstoken. Is there something similar in the shopping example?

Developers may want to use the API Explorer to interact with the REST API to debug how things work
for extended periods of time. It would annoy me when the jwt token would expire in 10 minutes. So I would
set it for 6 hours while I debug and play with the REST API. How will these changes affect this?

Good catch, i have currently no concept for this. My use case is a regular user on a angular SPA.

What needs to change in the Shoppy application (UI front-end) that was recently added?

I did not have a deep look how the frontend is currently working. I guess a serviceworker could work here too?

jannyHou · 2020-02-13T15:40:12Z

packages/shopping/src/services/refreshtoken.service.ts

+      const {creation, ttl} = await this.userRefreshtokenRepository.findById(
+        refreshtoken,
+        {
+          where: {


usually findById should return only one result, any reason adding an additional where filter here?

The idea is to check if the refreshToken is valid for / belongsTo the current user, validated by the authorization system. It's a kind of additional security check.

I recently learned from @raymondfeng that findById does not respect the where condition. I have to do the check differently.

jannyHou · 2020-02-13T16:10:17Z

packages/shopping/src/controllers/user.controller.ts

+  })
+  @authenticate('jwt')
+  // TODO(derdeka) find out why @authorize.allowAuthenticated() is not working
+  async refresh(


When would the refresh function get called? I am trying to illustrate the flow of how a refresh token works:

first a user login

then it gets the token and refresh token

then how does it decide when to call the refresh endpoint to extend the login time?

then how does it decide when to call the refresh endpoint to extend the login time?

In a angular application i would use a serviceworker or service with a timer to call this endpoint periodically in background.

jannyHou

@derdeka Appreciate your contribution 🎉 I left a comment in https://github.com/strongloop/loopback4-example-shopping/pull/537/files#r378960812, otherwise LGTM.

raymondfeng · 2020-02-26T22:40:22Z

@derdeka Do see opportunity to add some code to loopbackio/loopback-next#4746?

dhmlau · 2020-10-07T14:19:33Z

@derdeka, the @loopback/authentication-jwt extension now has the refresh token capability. Would you be interested to rework this PR to use that instead? Thanks.

cc @jannyHou

mrmodise · 2020-10-28T16:12:10Z

@derdeka, the @loopback/authentication-jwt extension now has the refresh token capability. Would you be interested to rework this PR to use that instead? Thanks.

cc @jannyHou

Based on this comment I recommend we change the status of the PR to draft?

jannyHou · 2020-11-09T18:38:24Z

@mrmodise I am ok with making the POC PR a draft, or if the gaps are big, creating a new PR is probably better. I would respect @derdeka 's decision on how to process the feature.

stale · 2021-07-09T02:00:28Z

This pull request has been marked stale because it has not seen activity within two months. It will be closed within 14 days of being stale unless there is new activity.

stale · 2021-07-23T02:06:12Z

This pull request has been closed due to continued inactivity. If you are interested in finishing the proposed changes, then feel free to re-open this pull request or open a new one.

derdeka force-pushed the feature/refreshtoken branch 2 times, most recently from a8f6698 to 487e699 Compare February 10, 2020 15:14

derdeka added 2 commits February 10, 2020 16:17

feat(loopback4-example-shopping): add refreshtoken

3b2ec13

Signed-off-by: derdeka <[email protected]>

feat(loopback4-example-shopping): add refreshtoken - refactoring

980a065

Signed-off-by: derdeka <[email protected]>

derdeka force-pushed the feature/refreshtoken branch from 487e699 to 980a065 Compare February 10, 2020 15:17

derdeka marked this pull request as ready for review February 10, 2020 15:32

derdeka requested review from hacksparrow and jannyHou as code owners February 10, 2020 15:32

derdeka requested review from dougal83 and achrinza February 10, 2020 16:19

raymondfeng approved these changes Feb 12, 2020

View reviewed changes

dougal83 reviewed Feb 12, 2020

View reviewed changes

achrinza requested changes Feb 13, 2020

View reviewed changes

emonddr suggested changes Feb 13, 2020

View reviewed changes

jannyHou reviewed Feb 13, 2020

View reviewed changes

derdeka mentioned this pull request Feb 25, 2020

feat(authentication): extend TokenService for revokeable tokens loopbackio/loopback-next#4746

Merged

7 tasks

dhmlau added the community-contribution label Feb 27, 2020

jannyHou mentioned this pull request Apr 13, 2020

feat: add jwt auth extension loopbackio/loopback-next#5046

Merged

7 tasks

jannyHou mentioned this pull request Apr 21, 2020

Add refresh in the jwt extension loopbackio/loopback-next#5187

Closed

1 task

dhmlau mentioned this pull request Apr 29, 2020

[DO NOT MERGE] feat(docs): May milestone loopbackio/loopback-next#5189

Closed

7 tasks

mrmodise marked this pull request as draft November 9, 2020 19:45

stale bot added the stale label Jul 9, 2021

stale bot closed this Jul 23, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[POC] add refreshtoken to extend accessToken #537

[POC] add refreshtoken to extend accessToken #537

derdeka commented Feb 7, 2020

derdeka commented Feb 10, 2020

dougal83 left a comment

dougal83 Feb 12, 2020

derdeka Feb 14, 2020

dougal83 Feb 14, 2020 •

edited

Loading

achrinza left a comment •

edited

Loading

emonddr Feb 13, 2020

jannyHou Feb 13, 2020

derdeka Feb 14, 2020

emonddr Feb 13, 2020

dougal83 Feb 13, 2020

derdeka Feb 14, 2020

jannyHou Feb 13, 2020

derdeka Feb 14, 2020

derdeka Feb 27, 2020 •

edited

Loading

jannyHou Feb 13, 2020 •

edited

Loading

derdeka Feb 14, 2020

jannyHou left a comment

raymondfeng commented Feb 26, 2020

dhmlau commented Oct 7, 2020

mrmodise commented Oct 28, 2020

jannyHou commented Nov 9, 2020 •

edited

Loading

stale bot commented Jul 9, 2021

stale bot commented Jul 23, 2021

		@@ -0,0 +1,69 @@
		// Copyright IBM Corp. 2020. All Rights Reserved.

[POC] add refreshtoken to extend accessToken #537

[POC] add refreshtoken to extend accessToken #537

Conversation

derdeka commented Feb 7, 2020

derdeka commented Feb 10, 2020

dougal83 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dougal83 Feb 14, 2020 • edited Loading

Choose a reason for hiding this comment

achrinza left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

derdeka Feb 27, 2020 • edited Loading

Choose a reason for hiding this comment

jannyHou Feb 13, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jannyHou left a comment

Choose a reason for hiding this comment

raymondfeng commented Feb 26, 2020

dhmlau commented Oct 7, 2020

mrmodise commented Oct 28, 2020

jannyHou commented Nov 9, 2020 • edited Loading

stale bot commented Jul 9, 2021

stale bot commented Jul 23, 2021

dougal83 Feb 14, 2020 •

edited

Loading

achrinza left a comment •

edited

Loading

derdeka Feb 27, 2020 •

edited

Loading

jannyHou Feb 13, 2020 •

edited

Loading

jannyHou commented Nov 9, 2020 •

edited

Loading