Skip to content

Commit

Permalink
Merge pull request #1667 from balena-io/limit-refreshed-jwt-payload
Browse files Browse the repository at this point in the history 
Fix refresh-token including additional jwt properties
  • Loading branch information
@flowzone-app
flowzone-app[bot] authored Jun 18, 2024
2 parents 6e43afc + 42a4083 commit 83a55a5
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 61 deletions.
13 changes: 12 additions & 1 deletion src/infra/auth/jwt.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,14 @@ export interface TokenUserPayload
authTime?: number;
}

const jwtValidFields = [
...tokenFields,
'authTime',
'twoFactorRequired',
'iat',
'exp',
] satisfies Array<keyof TokenUserPayload | 'iat' | 'exp'>;

export interface ExtraParams {
existingToken?: Partial<TokenUserPayload>;
jwtOptions?: SignOptions;
Expand Down Expand Up @@ -82,7 +90,10 @@ let $getUserTokenDataCallback: GetUserTokenDataFn = async (
const newTokenData = _.pick(userData, tokenFields);

const tokenData: TokenUserPayload = {
...existingToken,
// The existingToken that we pass in is the augmented object that
// the jwt-passport returns, so we need to make sure we are not
// destructuring extra properties.
..._.pick(existingToken, jwtValidFields),
...newTokenData,
};

Expand Down
104 changes: 45 additions & 59 deletions test/04_session.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import { expect } from 'chai';
import { SUPERUSER_EMAIL, SUPERUSER_PASSWORD } from '../src/lib/config.js';
import {
JSON_WEB_TOKEN_LIMIT_EXPIRY_REFRESH,
SUPERUSER_EMAIL,
SUPERUSER_PASSWORD,
} from '../src/lib/config.js';
import { createScopedAccessToken } from '../src/infra/auth/jwt.js';

import * as fixtures from './test-lib/fixtures.js';
Expand All @@ -14,12 +18,8 @@ import {
} from '../src/infra/auth/permissions.js';
import { permissions as pinePermissions, sbvrUtils } from '@balena/pinejs';
const { api } = sbvrUtils;
import type { JwtPayload } from 'jsonwebtoken';
import { decode } from 'jsonwebtoken';
import { setTimeout } from 'timers/promises';

const atob = (x: string) => Buffer.from(x, 'base64').toString('binary');
const parseJwt = (t: string) => JSON.parse(atob(t.split('.')[1]));
import { expectJwt } from './test-lib/api-helpers.js';

export default () => {
versions.test((version) => {
Expand Down Expand Up @@ -60,6 +60,29 @@ export default () => {
await fixtures.clean(this.loadedFixtures);
});

it('/login_ returns 401 when the password is wrong', async function () {
await supertest()
.post('/login_')
.send({
username: SUPERUSER_EMAIL,
password: `${SUPERUSER_PASSWORD}_wrong`,
})
.expect(401);
});

it('/login_ returns a token with only the allowed properties', async function () {
const token = (
await supertest()
.post('/login_')
.send({
username: SUPERUSER_EMAIL,
password: SUPERUSER_PASSWORD,
})
.expect(200)
).text;
expectJwt(token);
});

it('/user/v1/whoami returns a user', async function () {
const user = (await supertest(admin).get('/user/v1/whoami').expect(200))
.body;
Expand Down Expand Up @@ -126,51 +149,28 @@ export default () => {
token = admin.token!;
});

it('should be refreshable with /user/v1/refresh-token', async function () {
it('should be refreshable with /user/v1/refresh-token and not include extra properties', async function () {
// wait 2 seconds to make sure the token is already starting to expire
await setTimeout(2000);

const res = await supertest({ token })
.get('/user/v1/refresh-token')
.expect(200);

const oldDecodedToken = decode(token) as JwtPayload;
const newDecodedToken = decode(res.text) as JwtPayload;
const oldDecodedToken = expectJwt(token);
const newDecodedToken = expectJwt(res.text);
token = res.text;

expect(oldDecodedToken)
.to.be.an('object')
.to.have.property('exp')
.that.is.a('number');
expect(newDecodedToken)
.to.be.an('object')
.to.have.property('exp')
.that.is.a('number');
expect(oldDecodedToken)
.to.be.an('object')
.to.have.property('iat')
.that.is.a('number');
expect(newDecodedToken)
.to.be.an('object')
.to.have.property('iat')
.that.is.a('number');

const oldExp = oldDecodedToken.exp as number;
const newExp = newDecodedToken.exp as number;
expect(newExp).to.be.eq(oldExp);

const oldIat = oldDecodedToken.iat as number;
const newIat = newDecodedToken.iat as number;
expect(newIat).to.be.gt(oldIat);
expect(newIat - oldIat).to.be.gt(2);
if (JSON_WEB_TOKEN_LIMIT_EXPIRY_REFRESH) {
expect(oldDecodedToken.exp, 'exp should not change').to.be.eq(
newDecodedToken.exp,
);

const tokenParts = token.split('.');
expect(tokenParts).to.be.an('array');
expect(tokenParts).to.have.property('length', 3);
const payload = parseJwt(token);
expect(payload).to.have.property('id');
expect(payload).to.not.have.property('username');
expect(payload).to.not.have.property('email');
expect(newDecodedToken.iat, 'should get a newer iat').to.be.gt(
oldDecodedToken.iat,
);
expect(newDecodedToken.iat - oldDecodedToken.iat).to.be.gt(2);
}
});

it('should refresh & update the authTime with a POST to /user/v1/refresh-token using a correct password', async function () {
Expand All @@ -182,11 +182,7 @@ export default () => {
const tokenParts = token.split('.');
expect(tokenParts).to.be.an('array');
expect(tokenParts).to.have.property('length', 3);
const payload = parseJwt(token);
expect(payload).to.have.property('id');
expect(payload).to.not.have.property('username');
expect(payload).to.not.have.property('email');
expect(payload).to.have.property('authTime');
const payload = expectJwt(token);
const initialAuthTime: number = payload.authTime;

const res1 = await supertest(token)
Expand All @@ -198,10 +194,7 @@ export default () => {
const tokenParts1 = token.split('.');
expect(tokenParts1).to.be.an('array');
expect(tokenParts1).to.have.property('length', 3);
const payload1 = parseJwt(token);
expect(payload1).to.have.property('id');
expect(payload1).to.not.have.property('username');
expect(payload1).to.not.have.property('email');
const payload1 = expectJwt(token);
expect(payload1)
.to.have.property('authTime')
.to.be.above(initialAuthTime);
Expand All @@ -215,11 +208,7 @@ export default () => {
const tokenParts = token.split('.');
expect(tokenParts).to.be.an('array');
expect(tokenParts).to.have.property('length', 3);
const payload = parseJwt(token);
expect(payload).to.have.property('id');
expect(payload).to.not.have.property('username');
expect(payload).to.not.have.property('email');
expect(payload).to.have.property('authTime');
const payload = expectJwt(token);
const initialAuthTime: number = payload.authTime;

const res1 = await supertest(token)
Expand All @@ -229,10 +218,7 @@ export default () => {
const tokenParts1 = token.split('.');
expect(tokenParts1).to.be.an('array');
expect(tokenParts1).to.have.property('length', 3);
const payload1 = parseJwt(token);
expect(payload1).to.have.property('id');
expect(payload1).to.not.have.property('username');
expect(payload1).to.not.have.property('email');
const payload1 = expectJwt(token);
expect(payload1)
.to.have.property('authTime')
.that.equals(initialAuthTime);
Expand Down
6 changes: 5 additions & 1 deletion test/test-lib/api-helpers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { expect } from 'chai';
import type { UserObjectParam } from '../test-lib/supertest.js';
import { supertest } from '../test-lib/supertest.js';
import type { TokenUserPayload } from '../../src/index.js';
import type { RequiredField } from '@balena/pinejs/out/sbvr-api/common-types.js';

const version = 'resin';

Expand Down Expand Up @@ -140,7 +141,10 @@ export function expectJwt(tokenOrJwt: string | AnyObject) {
typeof tokenOrJwt === 'string'
? jsonwebtoken.decode(tokenOrJwt)
: tokenOrJwt
) as TokenUserPayload & { iat: number; exp: number };
) as RequiredField<TokenUserPayload, 'authTime'> & {
iat: number;
exp: number;
};
expect(decoded).to.have.property('id').that.is.a('number');
expect(decoded).to.have.property('jwt_secret').that.is.a.string;
expect(decoded.jwt_secret).to.be.a('string').that.has.length(32);
Expand Down

0 comments on commit 83a55a5

Please sign in to comment.