Skip to content
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
69 changes: 69 additions & 0 deletions __tests__/no-wallet-account.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
// Integration test: a signed-in account with no wallet is not an auth failure.
// 401 stays reserved for a token that is genuinely bad or expired.
require('dotenv').config();
const request = require('supertest');
const { expect } = require('chai');
const chai = require('chai');
const uuid = require('uuid');
const server = require('../server/app');
const seed = require('./seed');

chai.use(require('chai-uuid'));

describe('An account with no wallet yet', () => {
// A keycloak id no wallet row points at: the token verifies, the wallet
// lookup finds nothing.
const strangerToken = uuid.v4();

before(async () => {
await seed.clear();
await seed.seed();
});

it('GET /wallets answers 409 with reason no_wallet', async () => {
const res = await request(server)
.get('/wallets')
.set('Authorization', `Bearer ${strangerToken}`);

expect(res).to.have.property('statusCode', 409);
expect(res.body).to.have.property('reason', 'no_wallet');
expect(res.body.message).to.match(/no wallet yet/);
});

it('redeeming a share link answers 409 with reason no_wallet', async () => {
const link = await request(server)
.post('/action-tokens')
.set('Authorization', `Bearer ${seed.wallet.keycloak_account_id}`)
.send({
recipient_email: 'samwel@example.com',
bundle: { bundle_size: 1 },
sender_wallet: seed.wallet.name,
});
expect(link).to.have.property('statusCode', 201);

const res = await request(server)
.post('/action-tokens/redeem')
.set('Authorization', `Bearer ${strangerToken}`)
.send({ action_token: link.body.action_token });

expect(res).to.have.property('statusCode', 409);
expect(res.body).to.have.property('reason', 'no_wallet');
});

it('POST /wallets is still allowed, so the account can get one', async () => {
const res = await request(server)
.post('/wallets')
.set('Authorization', `Bearer ${strangerToken}`)
.send({ wallet: 'no-wallet-account-test', about: 'a new wallet' });

expect(res.statusCode).to.be.oneOf([200, 201]);
});

it('an account that has a wallet is unaffected', async () => {
const res = await request(server)
.get('/wallets')
.set('Authorization', `Bearer ${seed.wallet.keycloak_account_id}`);

expect(res).to.have.property('statusCode', 200);
});
});
6 changes: 4 additions & 2 deletions __tests__/wallet-get.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,15 @@ describe('Wallet: Get wallets of an account', () => {
expect(res.count).to.eq(11);
});

it('should return 401, user does not exist', async () => {
// The token verifies; the account simply has no wallet row yet. That is a
// 409 with reason no_wallet, not an authentication failure.
it('should return 409, the account has no wallet yet', async () => {
await request(server)
.get('/wallets')
.send({ wallet: 'azAZ.-@0123456789' })
.set('content-type', 'application/json')
.set('Authorization', `Bearer ${uuid.v4()}`)
.expect(401);
.expect(409);
});

it('Get wallets of WalletA without params', async () => {
Expand Down
5 changes: 4 additions & 1 deletion server/utils/HttpError.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,12 @@
*/

class HttpError extends Error {
constructor(code, message, toRollback) {
// `reason` is a stable slug for cases a client must branch on, where the
// status alone is ambiguous. It is sent in the response body.
constructor(code, message, toRollback, reason) {
super(message);
this.code = code;
this.reason = reason;
// set rollback flag, so the transaction of db would rollback when catch this error
// set default to true
this._toRollback = toRollback || true;
Expand Down
13 changes: 11 additions & 2 deletions server/utils/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ exports.errorHandler = (err, req, res, _next) => {
res.status(err.code).send({
code: err.code,
message: err.message,
...(err.reason && { reason: err.reason }),
});
} else if (err instanceof ValidationError) {
res.status(422).send({
Expand All @@ -62,8 +63,16 @@ exports.verifyJWTHandler = exports.handlerWrapper(async (req, res, next) => {
if (req.originalUrl === '/wallets' && req.method === 'POST') {
next();
} else {
log.error('user info not found');
throw new HttpError(401, 'ERROR: Authentication, invalid token received');
// The token verified on the line above. The only thing missing is a
// wallet, so do not call this an authentication failure: 401 stays
// reserved for a token that is genuinely bad or expired.
log.error('no wallet for this account yet');
throw new HttpError(
409,
'ERROR: This account has no wallet yet',
undefined,
'no_wallet',
);
}
} else {
req.wallet_id = wallet.id;
Expand Down
11 changes: 7 additions & 4 deletions server/utils/utils.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,9 @@ describe('routers/utils', () => {
sinon.restore();
});

it('pass corupt token should get response with code 403', async () => {
// The token verifies here; only the wallet is missing. Named for what it
// stubs, not for a corrupt token.
it('a valid token with no wallet yet gets 409 and reason no_wallet', async () => {
const keycloakId = uuid.v4();
const verifyStub = sinon.stub(JWTService, 'verify').returns({
id: keycloakId,
Expand All @@ -194,10 +196,11 @@ describe('routers/utils', () => {
const res = await request(app)
.get('/wallets')
.set('Authorization', `Bearer token`);
expect(res.statusCode).eq(401);
expect(res.statusCode).eq(409);
expect(res.body).eql({
code: 401,
message: 'ERROR: Authentication, invalid token received',
code: 409,
message: 'ERROR: This account has no wallet yet',
reason: 'no_wallet',
});
expect(getWalletIdByKeycloakId.calledOnceWithExactly(keycloakId)).eql(
true,
Expand Down
Loading