Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ID-1099]getUser will get new token when existed token expired. #913

Merged
merged 7 commits into from
Oct 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions packages/passport/sdk/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
"@metamask/detect-provider": "^2.0.0",
"axios": "^1.3.5",
"ethers": "^5.7.2",
"jwt-decode": "^3.1.2",
"magic-sdk": "^13.3.1",
"oidc-client-ts": "^2.2.1"
},
Expand All @@ -28,6 +29,7 @@
"@swc/jest": "^0.2.24",
Copy link
Contributor

Choose a reason for hiding this comment

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

We'll also need to add this devDependency to packages/passport/package.json (seems as though jwt-decode was somehow missed 🤔)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

might be because it already included in yarn.lock: https://github.com/immutable/ts-immutable-sdk/blob/main/yarn.lock#L21022

I will add the package json anyway. would be in dependency thought as the code need to be run within the build sdk.

"@types/axios": "^0.14.0",
"@types/jest": "^29.4.3",
"@types/jwt-encode": "^1.0.1",
"@types/node": "^18.14.2",
"@types/react": "^18.0.28",
"@types/react-dom": "^18.0.11",
Expand All @@ -37,6 +39,7 @@
"eslint": "^8.40.0",
"jest": "^29.4.3",
"jest-environment-jsdom": "^29.4.3",
"jwt-encode": "^1.0.1",
"msw": "^1.2.2",
"prettier": "^2.8.7",
"rollup": "^3.17.2",
Expand Down
7 changes: 3 additions & 4 deletions packages/passport/sdk/src/Passport.int.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { Magic } from 'magic-sdk';
import { UserManager } from 'oidc-client-ts';
import { TransactionRequest } from '@ethersproject/providers';
import { Environment, ImmutableConfiguration } from '@imtbl/config';
import { mockValidIdToken } from './token.test';
import { Passport } from './Passport';
import { RequestArguments } from './zkEvm/types';
import {
Expand All @@ -27,7 +28,7 @@ const mockOidcUser = {
nickname: 'test',
},
expired: false,
id_token: 'idToken123',
id_token: mockValidIdToken,
access_token: 'accessToken123',
refresh_token: 'refreshToken123',
};
Expand Down Expand Up @@ -134,8 +135,7 @@ describe('Passport', () => {
it('registers the user and returns the ether key', async () => {
mockSigninPopup.mockResolvedValue(mockOidcUser);
mockGetUser.mockResolvedValueOnce(null);
mockGetUser.mockResolvedValueOnce(mockOidcUser);
mockSigninSilent.mockResolvedValue(mockOidcUserZkevm);
mockGetUser.mockResolvedValueOnce(mockOidcUserZkevm);
useMswHandlers([
mswHandlers.counterfactualAddress.success,
]);
Expand All @@ -148,7 +148,6 @@ describe('Passport', () => {

expect(accounts).toEqual([mockOidcUserZkevm.profile.passport.zkevm_eth_address]);
expect(mockGetUser).toHaveBeenCalledTimes(2);
expect(mockSigninSilent).toHaveBeenCalledTimes(1);
expect(mockMagicRequest).toHaveBeenCalledTimes(3);
});

Expand Down
56 changes: 53 additions & 3 deletions packages/passport/sdk/src/authManager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@ import AuthManager from './authManager';
import { PassportError, PassportErrorType } from './errors/passportError';
import { PassportConfiguration } from './config';
import { mockUser, mockUserImx, mockUserZkEvm } from './test/mocks';
import { isTokenExpired } from './token';

jest.mock('oidc-client-ts');
jest.mock('./token');

const baseConfig = new ImmutableConfiguration({
environment: Environment.SANDBOX,
Expand All @@ -18,10 +20,9 @@ const config = new PassportConfiguration({
scope: 'email profile',
});

const mockOidcUser: OidcUser = {
const commonOidcUser: OidcUser = {
id_token: mockUser.idToken,
access_token: mockUser.accessToken,
refresh_token: mockUser.refreshToken,
token_type: 'Bearer',
scope: 'openid',
expires_in: 167222,
Expand All @@ -30,9 +31,25 @@ const mockOidcUser: OidcUser = {
email: mockUser.profile.email,
nickname: mockUser.profile.nickname,
},
} as OidcUser;

const mockOidcUser: OidcUser = {
...commonOidcUser,
refresh_token: mockUser.refreshToken,
expired: false,
} as OidcUser;

const mockOidcExpiredUser: OidcUser = {
...commonOidcUser,
refresh_token: mockUser.refreshToken,
expired: true,
} as OidcUser;

const mockOidcExpiredNoRefreshTokenUser: OidcUser = {
...commonOidcUser,
expired: true,
} as OidcUser;

const imxProfileData = {
imx_eth_address: mockUserImx.imx.ethAddress,
imx_stark_address: mockUserImx.imx.starkAddress,
Expand Down Expand Up @@ -251,7 +268,8 @@ describe('AuthManager', () => {
});

it('should return null if user is returned', async () => {
getUserMock.mockReturnValue(mockOidcUser);
getUserMock.mockReturnValue(mockOidcExpiredUser);
(isTokenExpired as jest.Mock).mockReturnValue(true);
signinSilentMock.mockResolvedValue(null);

const result = await authManager.loginSilent();
Expand Down Expand Up @@ -318,12 +336,44 @@ describe('AuthManager', () => {
describe('getUser', () => {
it('should retrieve the user from the userManager and return the domain model', async () => {
getUserMock.mockReturnValue(mockOidcUser);
(isTokenExpired as jest.Mock).mockReturnValue(false);

const result = await authManager.getUser();

expect(result).toEqual(mockUser);
});

it('should call signinSilent and returns user when user token is expired with the refresh token', async () => {
getUserMock.mockReturnValue(mockOidcExpiredUser);
(isTokenExpired as jest.Mock).mockReturnValue(true);
signinSilentMock.mockResolvedValue(mockOidcUser);

const result = await authManager.getUser();

expect(signinSilentMock).toBeCalledTimes(1);
expect(result).toEqual(mockUser);
});

it('should return null when the user token is expired without refresh token', async () => {
getUserMock.mockReturnValue(mockOidcExpiredNoRefreshTokenUser);
(isTokenExpired as jest.Mock).mockReturnValue(true);

const result = await authManager.getUser();

expect(signinSilentMock).toBeCalledTimes(0);
expect(result).toEqual(null);
});

it('should return null when the user token is expired with the refresh token, but signinSilent returns null', async () => {
getUserMock.mockReturnValue(mockOidcExpiredUser);
(isTokenExpired as jest.Mock).mockReturnValue(true);
signinSilentMock.mockResolvedValue(null);
const result = await authManager.getUser();

expect(signinSilentMock).toBeCalledTimes(1);
expect(result).toEqual(null);
});

it('should return null if no user is returned', async () => {
getUserMock.mockReturnValue(null);

Expand Down
37 changes: 23 additions & 14 deletions packages/passport/sdk/src/authManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ import {
WebStorageStateStore,
} from 'oidc-client-ts';
import axios from 'axios';
import jwt_decode from 'jwt-decode';
carmen0208 marked this conversation as resolved.
Show resolved Hide resolved
import DeviceCredentialsManager from 'storage/device_credentials_manager';
import * as crypto from 'crypto';
import jwt_decode from 'jwt-decode';
import { isTokenExpired } from './token';
import { PassportErrorType, withPassportError } from './errors/passportError';
import {
PassportMetadata,
Expand Down Expand Up @@ -369,24 +370,32 @@ export default class AuthManager {
}

public async loginSilent(): Promise<User | null> {
return withPassportError<User | null>(async () => {
const existedUser = await this.getUser();
if (!existedUser) {
return null;
}
const oidcUser = await this.userManager.signinSilent();
if (!oidcUser) {
return null;
}
return withPassportError<User | null>(async () => this.getUser(), PassportErrorType.SILENT_LOGIN_ERROR);
}

private async getWebUser() : Promise<User | null> {
const oidcUser = await this.userManager.getUser();
if (!oidcUser) {
return null;
}
const tokenExpired = isTokenExpired(oidcUser);
if (!tokenExpired) {
return AuthManager.mapOidcUserToDomainModel(oidcUser);
}, PassportErrorType.SILENT_LOGIN_ERROR);
}
if (oidcUser.refresh_token) {
const newOidcUser = await this.userManager.signinSilent();
if (newOidcUser) {
return AuthManager.mapOidcUserToDomainModel(newOidcUser);
}
}
return null;
}

public async getUser(): Promise<User | null> {
return withPassportError<User | null>(async () => {
const oidcUser = await this.userManager.getUser();
if (oidcUser) {
return AuthManager.mapOidcUserToDomainModel(oidcUser);
const user = await this.getWebUser();
if (user) {
return user;
}

const deviceToken = this.deviceCredentialsManager.getCredentials();
Expand Down
58 changes: 58 additions & 0 deletions packages/passport/sdk/src/token.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import encode from 'jwt-encode';
import {
User as OidcUser,
} from 'oidc-client-ts';
import { isIdTokenExpired, isTokenExpired } from './token';

const now = Math.floor(Date.now() / 1000);
const oneHourLater = now + 3600;
const oneHourBefore = now - 3600;

const mockExpiredIdToken = encode({
iat: oneHourBefore,
exp: oneHourBefore,
}, 'secret');
export const mockValidIdToken = encode({
iat: now,
exp: oneHourLater,
}, 'secret');

describe('isIdTokenExpired', () => {
it('should return false if idToken is undefined', () => {
expect(isIdTokenExpired(undefined)).toBe(false);
});

it('should return true if idToken is expired', () => {
expect(isIdTokenExpired(mockExpiredIdToken)).toBe(true);
});

it('should return false if idToken is not expired', () => {
expect(isIdTokenExpired(mockValidIdToken)).toBe(false);
});
});

describe('isTokenExpired', () => {
it('should return true if expired is true', () => {
const user = {
id_token: mockValidIdToken,
expired: true,
} as unknown as OidcUser;
expect(isTokenExpired(user)).toBe(true);
});

it('should return false if idToken is valid', () => {
const user = {
id_token: mockValidIdToken,
expired: false,
} as unknown as OidcUser;
expect(isTokenExpired(user)).toBe(false);
});

it('should return true idToken is expired', () => {
const user = {
id_token: mockExpiredIdToken,
expired: false,
} as unknown as OidcUser;
expect(isTokenExpired(user)).toBe(true);
});
});
22 changes: 22 additions & 0 deletions packages/passport/sdk/src/token.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { IdTokenPayload } from 'types';
import jwt_decode from 'jwt-decode';
haydenfowler marked this conversation as resolved.
Show resolved Hide resolved
import {
User as OidcUser,
} from 'oidc-client-ts';

export function isIdTokenExpired(idToken: string | undefined): boolean {
if (!idToken) {
return false;
}
const decodedToken: IdTokenPayload = jwt_decode(idToken);
const now = Math.floor(Date.now() / 1000);
return decodedToken.exp < now;
}

export function isTokenExpired(oidcUser: OidcUser): boolean {
const { id_token: idToken, expired } = oidcUser;
if (expired) {
return true;
}
return isIdTokenExpired(idToken);
}
1 change: 1 addition & 0 deletions packages/passport/sdk/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ export type IdTokenPayload = {
nickname: string;
aud: string;
sub: string;
exp: number;
};

export type DeviceErrorResponse = {
Expand Down
26 changes: 26 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3644,6 +3644,7 @@ __metadata:
"@swc/jest": ^0.2.24
"@types/axios": ^0.14.0
"@types/jest": ^29.4.3
"@types/jwt-encode": ^1.0.1
"@types/node": ^18.14.2
"@types/react": ^18.0.28
"@types/react-dom": ^18.0.11
Expand All @@ -3655,6 +3656,8 @@ __metadata:
ethers: ^5.7.2
jest: ^29.4.3
jest-environment-jsdom: ^29.4.3
jwt-decode: ^3.1.2
jwt-encode: ^1.0.1
magic-sdk: ^13.3.1
msw: ^1.2.2
oidc-client-ts: ^2.2.1
Expand Down Expand Up @@ -9780,6 +9783,13 @@ __metadata:
languageName: node
linkType: hard

"@types/jwt-encode@npm:^1.0.1":
version: 1.0.1
resolution: "@types/jwt-encode@npm:1.0.1"
checksum: 63bdfe9ebe0fab5a3635a4219d9e7667b15fbd48534fa07a1d3c9036c30f06ce8c7bcae1d4b17e293186b1c281d2ff4ef943877f5a92fcc0a5cd0d9b6823c5a1
languageName: node
linkType: hard

"@types/keyv@npm:^3.1.4":
version: 3.1.4
resolution: "@types/keyv@npm:3.1.4"
Expand Down Expand Up @@ -21026,6 +21036,15 @@ __metadata:
languageName: node
linkType: hard

"jwt-encode@npm:^1.0.1":
version: 1.0.1
resolution: "jwt-encode@npm:1.0.1"
dependencies:
ts.cryptojs256: ^1.0.1
checksum: 440e0e80a45fcad5e2e2542e632bbf7692d50a2574ca895e0961e8a6c08dc91b537b2e7bcd1bbd1c9a01ca937287d6078dd1e04dcc63832facbe8abb9f6291e6
languageName: node
linkType: hard

"keccak@npm:^3.0.0, keccak@npm:^3.0.2":
version: 3.0.3
resolution: "keccak@npm:3.0.3"
Expand Down Expand Up @@ -28949,6 +28968,13 @@ __metadata:
languageName: node
linkType: hard

"ts.cryptojs256@npm:^1.0.1":
version: 1.0.1
resolution: "ts.cryptojs256@npm:1.0.1"
checksum: 94f5a3d7e779e7cb533226731bb0d3cf6ed0a3f8e4032e44e80f9a2de68c57c86db1a39e69c412a4012c769186232b586d554fd863bbebd6d431d606d8980c72
languageName: node
linkType: hard

"tsc-watch@npm:^6.0.0":
version: 6.0.4
resolution: "tsc-watch@npm:6.0.4"
Expand Down