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

[change] Do not send new SMS unless needed #656 #665

Merged
merged 3 commits into from
Jun 29, 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
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import LoadingContext from "../../utils/loading-context";
import {
createMobilePhoneTokenUrl,
verifyMobilePhoneTokenUrl,
mobilePhoneTokenStatusUrl,
} from "../../constants";
import getErrorText from "../../utils/get-error-text";
import logError from "../../utils/log-error";
Expand Down Expand Up @@ -70,7 +71,9 @@ export default class MobilePhoneVerification extends React.Component {
this.setState({phone_number});
// send token via SMS only if user needs to verify
if (!is_verified && settings.mobile_phone_verification) {
await this.createPhoneToken();
if (!(await this.activePhoneToken())) {
await this.createPhoneToken();
}
}
}
setLoading(false);
Expand Down Expand Up @@ -171,6 +174,37 @@ export default class MobilePhoneVerification extends React.Component {
});
}

async activePhoneToken() {
const {orgSlug, language, userData} = this.props;
const url = mobilePhoneTokenStatusUrl(orgSlug);
return axios({
method: "get",
headers: {
"content-type": "application/x-www-form-urlencoded",
"accept-language": getLanguageHeaders(language),
Authorization: `Bearer ${userData.auth_token}`,
},
url,
})
.then((data) => data.active)
.catch((error) => {
if (
error.response &&
error.response.status === 404 &&
error.response.data &&
error.response.data.response_code !== "INVALID_ORGANIZATION"
) {
// This is kept for backward compatibility with older versions of OpenWISP RADIUS
// that does not have API endpoint for checking phone token status.
return false;
}
const errorText = getErrorText(error);
logError(error, errorText);
toast.error(errorText);
return errorText;
});
}

async resendPhoneToken() {
const {setLoading} = this.context;
setLoading(true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ describe("Mobile Phone Token verification: standard flow", () => {
);
// console mocking
validateToken.mockClear();
jest
.spyOn(MobilePhoneVerification.prototype, "activePhoneToken")
.mockReturnValue(false);
originalError = console.error;
lastConsoleOutuput = null;
setLoading.mockReset();
Expand Down Expand Up @@ -131,6 +134,129 @@ describe("Mobile Phone Token verification: standard flow", () => {
expect(wrapper.instance().hasPhoneTokenBeenSent()).toBe(true);
});

it("should check if active token is present", async () => {
validateToken.mockReturnValue(true);
axios.mockImplementation(() =>
Promise.resolve({
status: 200,
active: true,
}),
);
wrapper = createShallowComponent(props);
await tick();
expect(
MobilePhoneVerification.prototype.activePhoneToken.mock.calls.length,
).toBe(1);
});

it("should not send token if active token is present", async () => {
MobilePhoneVerification.prototype.activePhoneToken.mockRestore();
jest.spyOn(MobilePhoneVerification.prototype, "createPhoneToken");
validateToken.mockReturnValue(true);
axios.mockReset();
axios.mockImplementation(() =>
Promise.resolve({
status: 200,
active: true,
}),
);
wrapper = createShallowComponent(props);
await tick();
expect(
MobilePhoneVerification.prototype.createPhoneToken.mock.calls.length,
).toBe(0);
});

it("should not show error if active phone token returns 404", async () => {
// This is kept for backward compatibility with older versions of OpenWISP RADIUS
// that does not have API endpoint for checking phone token status.
MobilePhoneVerification.prototype.activePhoneToken.mockRestore();
jest
.spyOn(MobilePhoneVerification.prototype, "createPhoneToken")
.mockReturnValue(true);
axios.mockRestore();
axios.mockImplementationOnce(() =>
Promise.reject({
response: {
status: 404,
statusText: "NOT FOUND",
data: {
non_field_errors: ["Not Found"],
},
},
}),
);
validateToken.mockReturnValue(true);
jest.spyOn(toast, "error");
wrapper = createShallowComponent(props);
await tick();
expect(logError.mock.calls.length).toBe(0);
expect(toast.error.mock.calls.length).toBe(0);
expect(
MobilePhoneVerification.prototype.createPhoneToken.mock.calls.length,
).toBe(1);
});

it("should not execute createPhoneToken if invalid organization", async () => {
MobilePhoneVerification.prototype.activePhoneToken.mockRestore();
jest
.spyOn(MobilePhoneVerification.prototype, "createPhoneToken")
.mockReturnValue(true);
axios.mockRestore();
axios.mockImplementationOnce(() =>
Promise.reject({
response: {
status: 404,
statusText: "NOT FOUND",
data: {
non_field_errors: ["Not Found"],
response_code: "INVALID_ORGANIZATION",
},
},
}),
);
validateToken.mockReturnValue(true);
jest.spyOn(toast, "error");
wrapper = createShallowComponent(props);
await tick();
expect(
MobilePhoneVerification.prototype.createPhoneToken.mock.calls.length,
).toBe(0);
expect(logError.mock.calls.length).toBe(1);
expect(toast.error.mock.calls.length).toBe(1);
expect(toast.error).toHaveBeenCalledWith("Not Found");
});

it("should show error on if active phone token check fails", async () => {
MobilePhoneVerification.prototype.activePhoneToken.mockRestore();
axios.mockImplementationOnce(() =>
Promise.reject({
response: {
status: 400,
statusText: "BAD REQUEST",
data: {
non_field_errors: ["Bad request"],
},
},
}),
);
validateToken.mockReturnValue(true);
jest.spyOn(toast, "error");
wrapper = createShallowComponent(props);
await tick();
expect(logError).toHaveBeenCalledWith(
{
response: {
data: {non_field_errors: ["Bad request"]},
status: 400,
statusText: "BAD REQUEST",
},
},
"Bad request",
);
expect(toast.error.mock.calls.length).toBe(1);
});

it("should resend token successfully", async () => {
jest.spyOn(MobilePhoneVerification.prototype, "resendPhoneToken");
jest.spyOn(toast, "info");
Expand Down Expand Up @@ -317,6 +443,9 @@ describe("Mobile Phone Token verification: corner cases", () => {
setLoading: PropTypes.func,
};
props = createTestProps();
jest
.spyOn(MobilePhoneVerification.prototype, "activePhoneToken")
.mockReturnValue(false);
validateToken.mockClear();
});

Expand Down
2 changes: 2 additions & 0 deletions client/constants/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ export const getUserRadiusSessionsUrl = (orgSlug) =>
`${prefix}/${orgSlug}/account/session`;
export const createMobilePhoneTokenUrl = (orgSlug) =>
`${prefix}/${orgSlug}/account/phone/token`;
export const mobilePhoneTokenStatusUrl = (orgSlug) =>
`${prefix}/${orgSlug}/account/phone/token/status`;
export const verifyMobilePhoneTokenUrl = (orgSlug) =>
`${prefix}/${orgSlug}/account/phone/verify`;
export const mobilePhoneChangeUrl = (orgSlug) =>
Expand Down
58 changes: 58 additions & 0 deletions server/controllers/mobile-phone-token-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,64 @@ export const createMobilePhoneToken = (req, res) => {
}
};

export const mobilePhoneTokenStatus = (req, res) => {
const reqOrg = req.params.organization;
const validSlug = config.some((org) => {
if (org.slug === reqOrg) {
// merge default config and custom config
const conf = merge(defaultConfig, org);
const {host} = conf;
const url = reverse("mobile_phone_token_status", getSlug(conf));
const timeout = conf.timeout * 1000;
// make AJAX request
axios({
method: "get",
headers: {
"content-type": "application/x-www-form-urlencoded",
Authorization: req.headers.authorization,
"accept-language": req.headers["accept-language"],
},
url: `${host}${url}/`,
timeout,
})
.then((response) => {
res
.status(response.status)
.type("application/json")
.send(response.data);
})
.catch((error) => {
logResponseError(error);
// forward error
try {
res
.status(error.response.status)
.type("application/json")
.send(error.response.data);
} catch (err) {
res.status(500).type("application/json").send({
response_code: "INTERNAL_SERVER_ERROR",
});
}
});
}
return org.slug === reqOrg;
});
if (!validSlug) {
res
.status(404)
.type("application/json")
.send({
// The response code is different from other implementations because
// the frontend expects 404 also when the phone token status API
// is not implemented in OpenWISP RADIUS. Thus, the response
// code is used to distinguish between the two scenarios.
response_code: "INVALID_ORGANIZATION",
non_field_errors: ["Not found."],
});
}
};

export const verifyMobilePhoneToken = (req, res) => {
const reqOrg = req.params.organization;
const validSlug = config.some((org) => {
Expand Down
2 changes: 2 additions & 0 deletions server/routes/account.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import validateToken from "../controllers/validate-token-controller";
import {
createMobilePhoneToken,
verifyMobilePhoneToken,
mobilePhoneTokenStatus,
} from "../controllers/mobile-phone-token-controller";
import mobilePhoneNumberChange from "../controllers/mobile-phone-number-change-controller";
import errorHandler from "../utils/error-handler";
Expand All @@ -23,6 +24,7 @@ router.post("/password/reset", errorHandler(passwordReset));
router.post("/", errorHandler(registration));
router.get("/session/", errorHandler(getUserRadiusSessions));
router.post("/phone/token", errorHandler(createMobilePhoneToken));
router.get("/phone/token/status", errorHandler(mobilePhoneTokenStatus));
router.post("/phone/verify", errorHandler(verifyMobilePhoneToken));
router.post("/phone/change", errorHandler(mobilePhoneNumberChange));

Expand Down
1 change: 1 addition & 0 deletions server/utils/openwisp-urls.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const paths = {
validate_auth_token: "/account/token/validate",
user_radius_sessions: "/account/session",
create_mobile_phone_token: "/account/phone/token",
mobile_phone_token_status: "/account/phone/token/active",
verify_mobile_phone_token: "/account/phone/verify",
mobile_phone_number_change: "/account/phone/change",
plans: "/plan",
Expand Down