Skip to content
Open
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
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,5 @@ node_modules/
.idea/
.env
package-lock.json
dist/
types/
5 changes: 5 additions & 0 deletions .mocharc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"extensions": ["ts"],
"spec": "test/**/*.ts",
"require": "ts-node/register"
}
119 changes: 0 additions & 119 deletions index.js

This file was deleted.

127 changes: 127 additions & 0 deletions index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
import dotenv from 'dotenv';
import axios, { AxiosResponse } from 'axios';
import crypto from 'crypto';
import constants from 'constants';

Choose a reason for hiding this comment

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

high

The code imports from the constants npm package, which is an old and unnecessary dependency. The required constants are available directly from Node.js's built-in crypto module. Using the built-in module is more secure and reduces the number of dependencies. You should remove this import and use crypto.constants.RSA_PKCS1_PADDING on line 23. After this change, you can remove the "constants": "0.0.2" dependency from package.json.


dotenv.config();

interface MpesaConfig {
baseUrl: string;
apiKey: string;
publicKey: string;
origin: string;
serviceProviderCode: string;
}

let mpesaConfig: MpesaConfig | undefined;

Choose a reason for hiding this comment

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

critical

The use of a mutable global variable mpesaConfig to store configuration is a significant design flaw in a library. It introduces global state, which is not safe for concurrent operations (e.g., in a web server handling multiple requests) and prevents using the library with multiple different configurations simultaneously. I strongly recommend refactoring to a class-based approach where configuration is managed per instance.

For example:

export class MpesaApi {
  private readonly config: MpesaConfig;

  constructor(config: MpesaConfig) {
    validateConfig(config);
    this.config = config;
  }

  public async initiateC2B(/*...*/) {
    // ... implementation using this.config
  }

  // ... other methods
}


function _getBearerToken(mpesa_public_key: string, mpesa_api_key: string): string {

Choose a reason for hiding this comment

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

medium

The function names like _getBearerToken, initialize_api_from_dotenv, required_config_arg, initiate_c2b, and initiate_b2c use snake_case. The common convention in the TypeScript/JavaScript community is camelCase (e.g., getBearerToken, initializeApiFromDotenv). For consistency within the project (initializeApi is already camelCase) and with the wider ecosystem, I recommend switching to camelCase.

Suggested change
function _getBearerToken(mpesa_public_key: string, mpesa_api_key: string): string {
function getBearerToken(mpesa_public_key: string, mpesa_api_key: string): string {

const publicKey = `-----BEGIN PUBLIC KEY-----\n${mpesa_public_key}\n-----END PUBLIC KEY-----`;
const buffer = Buffer.from(mpesa_api_key);
const encrypted = crypto.publicEncrypt({
key: publicKey,
padding: constants.RSA_PKCS1_PADDING,

Choose a reason for hiding this comment

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

high

This uses the constants object from the deprecated constants package. You should use the constants from the crypto module instead. This requires removing the import constants from 'constants'; on line 4.

Suggested change
padding: constants.RSA_PKCS1_PADDING,
padding: crypto.constants.RSA_PKCS1_PADDING,

}, buffer);
return encrypted.toString("base64");
}

function initialize_api_from_dotenv(): void {
if (!mpesaConfig) {
mpesaConfig = {
baseUrl: process.env.MPESA_API_HOST || '',
apiKey: process.env.MPESA_API_KEY || '',
publicKey: process.env.MPESA_PUBLIC_KEY || '',
origin: process.env.MPESA_ORIGIN || '',
serviceProviderCode: process.env.MPESA_SERVICE_PROVIDER_CODE || ''
};
validateConfig(mpesaConfig);
console.log("Using M-Pesa environment configuration");
} else {
console.log("Using custom M-Pesa configuration");
Comment on lines +38 to +40

Choose a reason for hiding this comment

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

medium

In a library, it's best to avoid writing directly to the console with console.log. This can clutter the output of the consuming application and is not configurable. Consider removing these logs or using a proper logging library that can be controlled by the consumer.

}
}

function required_config_arg(argName: string): string {
return `Please provide a valid ${argName} in the configuration when calling initializeApi()`;
}

function validateConfig(configParams: MpesaConfig): void {
if (!configParams.baseUrl) {
throw required_config_arg("baseUrl");
}
if (!configParams.apiKey) {
throw required_config_arg("apiKey");
}
if (!configParams.publicKey) {
throw required_config_arg("publicKey");
}
if (!configParams.origin) {
throw required_config_arg("origin");
}
if (!configParams.serviceProviderCode) {
throw required_config_arg("serviceProviderCode");
}
}
Comment on lines +48 to +64

Choose a reason for hiding this comment

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

medium

The function validateConfig throws a string when validation fails. It's a best practice in JavaScript/TypeScript to throw Error objects, as they contain valuable information like stack traces and can be identified with instanceof Error.

Suggested change
function validateConfig(configParams: MpesaConfig): void {
if (!configParams.baseUrl) {
throw required_config_arg("baseUrl");
}
if (!configParams.apiKey) {
throw required_config_arg("apiKey");
}
if (!configParams.publicKey) {
throw required_config_arg("publicKey");
}
if (!configParams.origin) {
throw required_config_arg("origin");
}
if (!configParams.serviceProviderCode) {
throw required_config_arg("serviceProviderCode");
}
}
function validateConfig(configParams: MpesaConfig): void {
if (!configParams.baseUrl) {
throw new Error(required_config_arg("baseUrl"));
}
if (!configParams.apiKey) {
throw new Error(required_config_arg("apiKey"));
}
if (!configParams.publicKey) {
throw new Error(required_config_arg("publicKey"));
}
if (!configParams.origin) {
throw new Error(required_config_arg("origin"));
}
if (!configParams.serviceProviderCode) {
throw new Error(required_config_arg("serviceProviderCode"));
}
}


export const initializeApi = (configParams: MpesaConfig): void => {
validateConfig(configParams);
mpesaConfig = configParams;
};

export const initiate_c2b = async (amount: number, msisdn: string, transaction_ref: string, thirdparty_ref: string): Promise<any> => {

Choose a reason for hiding this comment

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

medium

The function returns a Promise<any>, which bypasses type checking for the response data. It's better to define an interface for the expected API response to provide type safety to consumers of your library. Also, consider renaming the function to initiateC2B to follow the camelCase convention.

Suggested change
export const initiate_c2b = async (amount: number, msisdn: string, transaction_ref: string, thirdparty_ref: string): Promise<any> => {
export const initiate_c2b = async (amount: number, msisdn: string, transaction_ref: string, thirdparty_ref: string): Promise<Record<string, any>> => {

initialize_api_from_dotenv();
try {
const response: AxiosResponse = await axios({
method: 'post',
url: `https://${mpesaConfig?.baseUrl}:18352/ipg/v1x/c2bPayment/singleStage/`,
headers: {
'Content-Type': 'application/json',
'Authorization': `Bearer ${_getBearerToken(mpesaConfig?.publicKey || '', mpesaConfig?.apiKey || '')}`,
'Origin': mpesaConfig?.origin || ''
},
data: {
"input_TransactionReference": transaction_ref,
"input_CustomerMSISDN": msisdn,
"input_Amount": amount.toString(),
"input_ThirdPartyReference": thirdparty_ref,
"input_ServiceProviderCode": mpesaConfig?.serviceProviderCode || ''
Comment on lines +79 to +87

Choose a reason for hiding this comment

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

high

The use of optional chaining (?.) and nullish coalescing (|| '') on mpesaConfig suggests it might be undefined. If it were, this would lead to silent failures or obscure errors (e.g., sending an empty public key). The logic should instead guarantee that mpesaConfig is defined at this point, or fail loudly. A guard clause at the start of the function would be more robust.

// At the start of the function:
if (!mpesaConfig) {
    throw new Error("M-Pesa API not initialized. Call initializeApi() or set environment variables before making API calls.");
}

After adding the guard clause, you can safely remove the optional chaining operators.

}
});
return response.data;
} catch (e: any) {
if (e.response?.data) {
throw e.response.data;
} else {
throw e;
}
}
Comment on lines +91 to +97

Choose a reason for hiding this comment

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

medium

The catch block re-throws e.response.data, which is likely a plain object, not an Error instance. This breaks standard error handling conventions and loses the stack trace. It's better to wrap the response data in a new Error object.

    } catch (e: any) {
        if (axios.isAxiosError(e) && e.response?.data) {
            throw new Error(`M-Pesa API Error: ${JSON.stringify(e.response.data)}`);
        }
        throw e;
    }

};

export const initiate_b2c = async (amount: number, msisdn: string, transaction_ref: string, thirdparty_ref: string): Promise<any> => {
initialize_api_from_dotenv();
try {
const response: AxiosResponse = await axios({
method: 'post',
url: `https://${mpesaConfig?.baseUrl}:18345/ipg/v1x/b2cPayment/`,
headers: {
'Content-Type': 'application/json',
'Authorization': `Bearer ${_getBearerToken(mpesaConfig?.publicKey || '', mpesaConfig?.apiKey || '')}`,
'Origin': mpesaConfig?.origin || ''
},
data: {
"input_TransactionReference": transaction_ref,
"input_CustomerMSISDN": msisdn,
"input_Amount": amount.toString(),
"input_ThirdPartyReference": thirdparty_ref,
"input_ServiceProviderCode": mpesaConfig?.serviceProviderCode || ''
}
});
return response.data;
} catch (e: any) {
if (e.response?.data) {
throw e.response.data;
} else {
throw e;
}
}
};
23 changes: 19 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,18 @@
"name": "mpesa-node-api",
"version": "1.1.0",
"description": "Node.js library for M-Pesa API (Mozambique)",
"main": "index.js",
"main": "dist/index.js",
"types": "types/index.d.ts",
"scripts": {
"test": "mocha"
"test": "mocha",
"build": "tsc"
},
"mocha": {
"require": "ts-node/register",
"extension": [
"ts"
],
"spec": "test/**/*.ts"
},
Comment on lines +11 to 17

Choose a reason for hiding this comment

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

medium

You have defined Mocha configuration both in this package.json file and in the new .mocharc.json file. This is redundant and can lead to confusion. It's best to keep the configuration in one place. I recommend keeping .mocharc.json as it's a dedicated file for this purpose, and removing this mocha block from package.json.

"repository": {
"type": "git",
Expand All @@ -20,13 +29,19 @@
},
"homepage": "https://github.com/rosariopfernandes/mpesa-node-api",
"dependencies": {
"axios": "^0.21.1",
"axios": "^1.8.4",
"constants": "0.0.2",
"crypto": "^1.0.1",
"dotenv": "^8.2.0"
},
"devDependencies": {
"@types/chai": "^5.2.1",
"@types/dotenv": "^6.1.1",
"@types/mocha": "^10.0.10",
"@types/node": "^22.13.14",
"chai": "^4.2.0",
"mocha": "^8.0.1"
"mocha": "^11.1.0",
"ts-node": "^9.1.1",
"typescript": "^5.8.2"
}
}
16 changes: 9 additions & 7 deletions test/test.js → test/test.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
const mpesa = require('../index');
const expect = require('chai').expect;
import * as mpesa from '../index';
import { expect } from 'chai';

describe('config', function () {
it('initializeApi throws an error when required config is missing', function () {
const actualConfig = {
baseUrl: "api.mpesa.co.mz"
};
} as any;

try {
mpesa.initializeApi(actualConfig)
mpesa.initializeApi(actualConfig);
} catch (e) {
expect(e).to.equal('Please provide a valid apiKey in the configuration when calling initializeApi()')
expect(e).to.equal('Please provide a valid apiKey in the configuration when calling initializeApi()');
}
});

Expand All @@ -19,10 +20,11 @@ describe('config', function () {
apiKey: "apiKey",
publicKey: "key",
origin: "developer.mpesa.co.mz",
serviceProviderCode: 171717
serviceProviderCode: "171717"
};

expect(function () {
mpesa.initializeApi(actualConfig)
mpesa.initializeApi(actualConfig);
}).to.not.throw();
});
});
20 changes: 20 additions & 0 deletions tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{
"compilerOptions": {
"module": "nodenext",
"moduleResolution": "nodenext",
"target": "esnext",
"declaration": true,
"declarationDir": "./types",
"outDir": "./dist",
"strict": true,
"esModuleInterop": true,
"skipLibCheck": true,
"resolveJsonModule": true,
"sourceMap": true,
"types": ["mocha", "chai"],
},
"include": ["**/*.ts"],
"exclude": [
"node_modules"
]
Comment on lines +17 to +19

Choose a reason for hiding this comment

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

medium

The exclude array is missing the output directories (dist and types). While tsc usually doesn't re-process its own output, explicitly excluding these directories prevents potential issues, especially if build artifacts are not cleaned between runs.

  "exclude": [
    "node_modules",
    "dist",
    "types"
  ]

}