5187 jwt refresh token Feature #5589

madaky · 2020-05-28T08:22:50Z

feature refresh token implemented through i̶n̶t̶e̶r̶c̶e̶p̶t̶o̶r̶s̶ service

Implements #5187
See also #5046

Checklist

👉 Read and sign the CLA (Contributor License Agreement) 👈

npm test passes on your machine
New tests added or existing tests modified to cover all changes
Code conforms with the style guide
API Documentation in code was updated
Documentation in /docs/site was updated
Affected artifact templates in packages/cli were updated
Affected example projects in examples/* were updated

dhmlau

@madaky, thanks for the PR. I've reviewed the docs portion of the PR and provided some comments. I'll leave the implementation review to @jannyHou and other team members. Thanks.

extensions/authentication-jwt/README.md

madaky · 2020-05-29T05:25:33Z

@madaky, thanks for the PR. I've reviewed the docs portion of the PR and provided some comments. I'll leave the implementation review to @jannyHou and other team members. Thanks.

Thanks for review :)

jannyHou

@madaky Thank you so much for adding the refresh feature 👍
I left a comment regarding the interceptors in #5589 (comment)

extensions/authentication-jwt/package.json

extensions/authentication-jwt/src/__tests__/fixtures/controllers/user.controller.ts

jannyHou

@madaky Thank you for the update, the structure looks much better now 👍 I left a few comments. Especially #5589 (comment)

extensions/authentication-jwt/src/__tests__/fixtures/controllers/user.controller.ts

extensions/authentication-jwt/src/__tests__/unit/jwt.service.ts

extensions/authentication-jwt/src/keys.ts

extensions/authentication-jwt/src/services/jwt.service.ts

extensions/authentication-jwt/src/services/user.service.ts

jannyHou

@madaky Awesome! The code and test part LGTM.
Could you also update the old docs before merging? Almost there.

dhmlau · 2020-08-19T13:28:34Z

We just switch the contribution method from CLA to DCO, making your contribution easier in the future. Please sign the commits with DCO by amending your commit messages with -s flag and push the changes again. If you're using the command line, you can:

git commit --amend -s
git push --force-with-lease

Please refer to this docs page for details. Thanks!

jannyHou

LGTM!

raymondfeng · 2020-08-21T18:49:55Z

extensions/authentication-jwt/package.json

@@ -28,7 +28,8 @@
    "@loopback/service-proxy": "^2.3.7",
    "@types/bcryptjs": "2.4.2",
    "bcryptjs": "^2.4.3",
-    "jsonwebtoken": "^8.5.1"
+    "jsonwebtoken": "^8.5.1",
+    "@loopback/context": "^3.8.1"


Let's use @loopback/core instead.

raymondfeng · 2020-08-21T18:51:26Z

extensions/authentication-jwt/src/__tests__/fixtures/controllers/user.controller.ts

@@ -142,4 +153,68 @@ export class UserController {
  async whoAmI(): Promise<string> {
    return this.user[securityId];
  }
+  // Routes using refreshtoken
+  @post('/users/refresh/login', {


/users/refresh-token?

Ya can it be like /user/refresh-token/login?

@madaky I think Raymond means remove the login part, just use users/refresh-token, cuz it doesn't do login, it returns tokens.

@raymondfeng I renamed it back to /users/refresh-login, it's essentially a login function. I added doc to explain what it does.

raymondfeng · 2020-08-21T18:53:02Z

extensions/authentication-jwt/src/keys.ts

+export type RefreshGrant = {
+  refreshToken: string;
+};
+
+export const RefreshGrantSchema = {
+  type: 'object',
+  required: ['refreshToken'],
+  properties: {
+    refreshToken: {
+      type: 'string',
+    },
+  },
+};
+export const RefreshGrantRequestBody = {
+  description: 'Reissuing Acess Token',
+  required: true,
+  content: {
+    'application/json': {schema: RefreshGrantSchema},
+  },
+};
+
+export type TokenObject = {
+  accessToken: string;


Can we refactor types/interfaces to types.ts?

OK @raymondfeng Thank you for your suggestion and guidance. I will fix it.

raymondfeng · 2020-08-26T19:12:10Z

extensions/authentication-jwt/src/types.ts

+   *
+   */
+  refreshToken(refreshToken: string): Promise<TokenObject>;
+}


Nitpick: I would like to see descriptive tsdocs for exported types/interfaces/constants.

raymondfeng · 2020-08-26T19:14:23Z

extensions/authentication-jwt/src/services/user.service.ts

-
+/* eslint-disable*/
+import {User, UserRelations} from '../models';
+/* eslint-enable */


For single-line disable, we can use // eslint-disable-next-line no-unused-vars

raymondfeng · 2020-08-26T19:15:03Z

extensions/authentication-jwt/src/keys.ts

+  );
+  export const DATASOURCE_NAME = 'refreshdb';
+  export const REFRESH_REPOSITORY = 'repositories.RefreshTokenRepository';
+}


More tsdocs are welcome.

@raymondfeng I added the docs in the 3rd commit. PTAL

raymondfeng

Nice work, @madaky and @jannyHou 🥇

dougal83

Looks great! Just spotted a few typos. Nice job. 👍

dougal83 · 2020-08-27T02:39:58Z

extensions/authentication-jwt/src/keys.ts

+    'authentication.jwt.refresh.secret',
+  );
+  export const REFRESH_EXPIRES_IN = BindingKey.create<string>(
+    'authentication.jwt.referesh.expires.in.seconds',


Typo? Are instances of 'referesh' meant to be 'refresh'?

thanks @dougal83 for spoting out typos.

dougal83 · 2020-08-27T02:48:19Z

extensions/authentication-jwt/src/keys.ts

+  /**
+   * The default issure used when generating refresh token.
+   */
+  export const REFRESH_ISSURE_VALUE = 'loopback4';


Typo? 'ISSURE' > 'ISSUER'

dougal83 · 2020-08-27T02:50:09Z

extensions/authentication-jwt/src/keys.ts

+   */
+  export const REFRESH_EXPIRES_IN_VALUE = '216000';
+  /**
+   * The default issure used when generating refresh token.


Typo? issure > Issuer

feature refresh token implementation through Service chore(authentication-jwt): readme updated readme.md to use refresh token and extra configurations chore(context): readmemd refactor Apply suggestions from code review Co-authored-by: Diana Lau <[email protected]> feat: implemented awthentication-jwt refreshtoken services Signed-off-by: Madaky <[email protected]>

jannyHou · 2020-08-27T14:18:26Z

Thank you @dougal83 Good catches! fixed.

jannyHou · 2020-08-27T14:48:29Z

Merged 🎉 appreciate @madaky 's contribution and all the feedback again!

madaky requested review from emonddr, hacksparrow and jannyHou as code owners May 28, 2020 08:22

madaky force-pushed the 5187-jwt-refresh-token branch from 17542ee to 3786cb7 Compare May 28, 2020 09:23

bajtos added the community-contribution label May 28, 2020

dhmlau added the Authentication label May 29, 2020

dhmlau reviewed May 29, 2020

View reviewed changes

dhmlau added this to the Jun 2020 milestone Jun 5, 2020

jannyHou reviewed Jun 16, 2020

View reviewed changes

dhmlau assigned jannyHou Jun 22, 2020

madaky force-pushed the 5187-jwt-refresh-token branch from e5c5df7 to 0fe795f Compare June 23, 2020 10:29

agnes512 mentioned this pull request Jun 26, 2020

Monthly Milestone - July 2020 ☀️🍉🍺 #5837

Closed

29 tasks

madaky force-pushed the 5187-jwt-refresh-token branch 2 times, most recently from 6e27c66 to a575d11 Compare July 1, 2020 23:02

dhmlau modified the milestones: Jun 2020, Jul 2020 Jul 2, 2020

jannyHou reviewed Jul 13, 2020

View reviewed changes

madaky force-pushed the 5187-jwt-refresh-token branch 5 times, most recently from 1da2847 to d05307b Compare July 27, 2020 19:39

jannyHou reviewed Jul 27, 2020

View reviewed changes

extensions/authentication-jwt/src/services/user.service.ts Outdated Show resolved Hide resolved

madaky force-pushed the 5187-jwt-refresh-token branch 5 times, most recently from c652b0f to c5be3f2 Compare July 29, 2020 13:06

madaky requested a review from jannyHou July 29, 2020 17:26

jannyHou mentioned this pull request Jul 29, 2020

Monthly Milestone - August 2020 ☀️🌵🌻🌴 #6028

Closed

27 tasks

dhmlau modified the milestones: Jul 2020, Aug 2020 Aug 4, 2020

jannyHou reviewed Aug 11, 2020

View reviewed changes

madaky force-pushed the 5187-jwt-refresh-token branch 2 times, most recently from 1d99caf to cdcb04a Compare August 15, 2020 06:59

madaky force-pushed the 5187-jwt-refresh-token branch 2 times, most recently from a768352 to 8f7cd14 Compare August 20, 2020 15:44

jannyHou approved these changes Aug 20, 2020

View reviewed changes

raymondfeng reviewed Aug 21, 2020

View reviewed changes

jannyHou force-pushed the 5187-jwt-refresh-token branch from b26e888 to 79a3b40 Compare August 26, 2020 18:11

raymondfeng reviewed Aug 26, 2020

View reviewed changes

jannyHou force-pushed the 5187-jwt-refresh-token branch from 3f8e2e1 to 446d693 Compare August 26, 2020 22:33

raymondfeng approved these changes Aug 26, 2020

View reviewed changes

jannyHou force-pushed the 5187-jwt-refresh-token branch from 446d693 to 49a1d1b Compare August 27, 2020 01:55

dougal83 approved these changes Aug 27, 2020

View reviewed changes

jannyHou force-pushed the 5187-jwt-refresh-token branch from 49a1d1b to fa8bf5d Compare August 27, 2020 14:11

jannyHou merged commit 7074182 into loopbackio:master Aug 27, 2020

dhmlau mentioned this pull request Aug 28, 2020

Add refresh in the jwt extension #5187

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

5187 jwt refresh token Feature #5589

5187 jwt refresh token Feature #5589

madaky commented May 28, 2020 •

edited

Loading

dhmlau left a comment

madaky commented May 29, 2020

jannyHou left a comment •

edited

Loading

jannyHou left a comment •

edited

Loading

jannyHou left a comment

dhmlau commented Aug 19, 2020

jannyHou left a comment

raymondfeng Aug 21, 2020

raymondfeng Aug 21, 2020

madaky Aug 24, 2020

jannyHou Aug 26, 2020

jannyHou Aug 26, 2020

raymondfeng Aug 21, 2020

madaky Aug 24, 2020

raymondfeng Aug 26, 2020

raymondfeng Aug 26, 2020

raymondfeng Aug 26, 2020

jannyHou Aug 26, 2020

raymondfeng left a comment

dougal83 left a comment

dougal83 Aug 27, 2020

madaky Aug 27, 2020

dougal83 Aug 27, 2020

dougal83 Aug 27, 2020

jannyHou commented Aug 27, 2020

jannyHou commented Aug 27, 2020

5187 jwt refresh token Feature #5589

5187 jwt refresh token Feature #5589

Conversation

madaky commented May 28, 2020 • edited Loading

Checklist

dhmlau left a comment

Choose a reason for hiding this comment

madaky commented May 29, 2020

jannyHou left a comment • edited Loading

Choose a reason for hiding this comment

jannyHou left a comment • edited Loading

Choose a reason for hiding this comment

jannyHou left a comment

Choose a reason for hiding this comment

dhmlau commented Aug 19, 2020

jannyHou left a 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

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

raymondfeng left a comment

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jannyHou commented Aug 27, 2020

jannyHou commented Aug 27, 2020

madaky commented May 28, 2020 •

edited

Loading

jannyHou left a comment •

edited

Loading

jannyHou left a comment •

edited

Loading