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

fix: ip check fallback following axios regression #2054

Merged
merged 2 commits into from Dec 20, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
97 changes: 76 additions & 21 deletions src/app/accounts/update-account-ip.ts
@@ -1,9 +1,15 @@
import { getIpConfig } from "@config"
import { RepositoryError } from "@domain/errors"
import { IpFetcherServiceError } from "@domain/ipfetcher"
import { ErrorLevel } from "@domain/shared"
import { IpFetcher } from "@services/ipfetcher"
import { AccountsIpRepository } from "@services/mongoose/accounts-ips"
import { asyncRunInSpan, SemanticAttributes } from "@services/tracing"
import {
addAttributesToCurrentSpan,
asyncRunInSpan,
recordExceptionInCurrentSpan,
SemanticAttributes,
} from "@services/tracing"

const accountsIp = AccountsIpRepository()

Expand All @@ -17,28 +23,28 @@ export const updateAccountIPsInfo = async ({
logger: Logger
}): Promise<void | RepositoryError> =>
asyncRunInSpan(
"app.users.updateAccountIPsInfo",
"app.accounts.updateAccountIPsInfo",
Copy link
Contributor

@dolcalmi dolcalmi Dec 20, 2022

Choose a reason for hiding this comment

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

this wrapper is not necessary because in sessionContext we are using Accounts.updateAccountIPsInfo which is already a wrapper

image

{
attributes: {
[SemanticAttributes.CODE_FUNCTION]: "updateAccountIPsInfo",
[SemanticAttributes.CODE_NAMESPACE]: "app.users",
[SemanticAttributes.CODE_NAMESPACE]: "app.accounts",
},
},
async () => {
const ipConfig = getIpConfig()

const lastConnection = new Date()

const userIP = await accountsIp.findById(accountId)
if (userIP instanceof RepositoryError) return userIP
const accountIP = await accountsIp.findById(accountId)
if (accountIP instanceof RepositoryError) return accountIP

if (!ip || !ipConfig.ipRecordingEnabled) {
const result = await accountsIp.update(userIP)
const result = await accountsIp.update(accountIP)

if (result instanceof Error) {
logger.error(
{ result, accountId, ip },
"impossible to update user last connection",
"impossible to update account last connection",
)

return result
Expand All @@ -47,31 +53,80 @@ export const updateAccountIPsInfo = async ({
return
}

const lastIP = userIP.lastIPs.find((ipObject) => ipObject.ip === ip)
let ipInfo: IPType

if (lastIP) {
lastIP.lastConnection = lastConnection
const ipFromDb = accountIP.lastIPs.find((ipObject) => ipObject.ip === ip)

if (ipFromDb) {
ipInfo = ipFromDb
ipInfo.lastConnection = lastConnection
} else {
let ipInfo: IPType = {
ipInfo = {
ip,
firstConnection: lastConnection,
lastConnection: lastConnection,
}
}

if (
ipConfig.proxyCheckingEnabled &&
(ipInfo.isoCode === undefined ||
ipInfo.proxy === undefined ||
ipInfo.asn === undefined)
) {
const ipFetcher = IpFetcher()
const ipFetcherInfo = await ipFetcher.fetchIPInfo(ip)

if (ipFetcherInfo instanceof IpFetcherServiceError) {
recordExceptionInCurrentSpan({
error: ipFetcherInfo.message,
level: ErrorLevel.Warn,
attributes: {
ip,
accountId,
},
})

logger.error({ accountId, ip }, "impossible to get ip detail")
return ipFetcherInfo
}

// deep copy
const ipFetcherInfoForOtel = JSON.parse(JSON.stringify(ipFetcherInfo))

for (const key in ipFetcherInfoForOtel) {
ipFetcherInfoForOtel["proxycheck." + key] = ipFetcherInfoForOtel[key]
delete ipFetcherInfoForOtel[key]
}
Copy link
Contributor

Choose a reason for hiding this comment

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

this can be replaced by a .map


addAttributesToCurrentSpan(ipFetcherInfoForOtel)

if (ipConfig.proxyCheckingEnabled) {
const ipFetcher = IpFetcher()
const ipFetcherInfo = await ipFetcher.fetchIPInfo(ip)
if (
ipFetcherInfo.isoCode === undefined ||
ipFetcherInfo.proxy === undefined ||
ipFetcherInfo.asn === undefined
nicolasburtey marked this conversation as resolved.
Show resolved Hide resolved
) {
const error = `missing mandatory fields. isoCode: ${ipFetcherInfo.isoCode}, proxy: ${ipFetcherInfo.proxy}, asn: ${ipFetcherInfo.asn}`
recordExceptionInCurrentSpan({
error,
level: ErrorLevel.Warn,
attributes: {
ip,
accountId,
},
})
} else {
// using Object.assign instead of ... because of conflict with mongoose hidden properties
ipInfo = Object.assign(ipInfo, ipFetcherInfo)

if (ipFetcherInfo instanceof IpFetcherServiceError) {
logger.error({ accountId, ip }, "impossible to get ip detail")
return ipFetcherInfo
}
// removing current ip from lastIPs - if it exists
accountIP.lastIPs = accountIP.lastIPs.filter((ipDb) => ipDb.ip !== ip)

ipInfo = { ...ipInfo, ...ipFetcherInfo }
// adding it back with the correct info
accountIP.lastIPs.push(ipInfo)
}
userIP.lastIPs.push(ipInfo)
}
const result = await accountsIp.update(userIP)
const result = await accountsIp.update(accountIP)

if (result instanceof Error) {
logger.error({ result, accountId, ip }, "impossible to update ip")
Expand Down
7 changes: 6 additions & 1 deletion src/app/auth/request-phone-code.ts
@@ -1,5 +1,6 @@
import { getTestAccounts } from "@config"
import { getTestAccounts, getTwilioConfig } from "@config"
import { TestAccountsChecker } from "@domain/accounts/test-accounts-checker"
import { NotImplementedError } from "@domain/errors"
import { RateLimitConfig } from "@domain/rate-limit"
import { RateLimiterExceededError } from "@domain/rate-limit/errors"
import { consumeLimiter } from "@services/rate-limit"
Expand Down Expand Up @@ -69,6 +70,10 @@ export const requestPhoneCode = async ({
return true
}

if (getTwilioConfig().accountSid === "AC_twilio_id") {
return new NotImplementedError("use test account for local dev and tests")
}

return TwilioClient().initiateVerify(phone)
}

Expand Down
5 changes: 2 additions & 3 deletions src/config/yaml.ts
Expand Up @@ -264,9 +264,8 @@ export const getBuildVersions = (): {
export const PROXY_CHECK_APIKEY = yamlConfig?.PROXY_CHECK_APIKEY

export const getIpConfig = (config = yamlConfig): IpConfig => ({
ipRecordingEnabled:
process.env.NODE_ENV === "test" ? false : config.ipRecording?.enabled,
proxyCheckingEnabled: config.ipRecording?.proxyChecking?.enabled,
ipRecordingEnabled: config.ipRecording.enabled,
proxyCheckingEnabled: config.ipRecording.proxyChecking.enabled,
})

export const getApolloConfig = (config = yamlConfig): ApolloConfig => config.apollo
Expand Down
27 changes: 16 additions & 11 deletions src/services/ipfetcher/index.ts
@@ -1,24 +1,29 @@
import axios from "axios"
import { UnknownIpFetcherServiceError } from "@domain/ipfetcher"
import { PROXY_CHECK_APIKEY } from "@config"
import { addAttributesToCurrentSpan } from "@services/tracing"

export const IpFetcher = (): IIpFetcherService => {
const fetchIPInfo = async (ip: string): Promise<IPInfo | IpFetcherServiceError> => {
try {
const { data } = await axios.get(
`https://proxycheck.io/v2/${ip}?key=${PROXY_CHECK_APIKEY}&vpn=1&asn=1`,
)
const proxy = !!(data[ip] && data[ip].proxy && data[ip].proxy === "yes")
const isoCode = data[ip] && data[ip].isocode
const params: { [id: string]: string } = {
vpn: "1",
asn: "1",
time: "1",
risk: "1",
}

if (PROXY_CHECK_APIKEY) {
params["key"] = PROXY_CHECK_APIKEY
}

addAttributesToCurrentSpan({
...data[ip],
isoCode,
proxy,
status: data.status,
const { data } = await axios.request({
url: `https://proxycheck.io/v2/${ip}`,
params,
})

const proxy = !!(data[ip] && data[ip].proxy && data[ip].proxy === "yes")
const isoCode = data[ip] && data[ip].isocode

return { ...data[ip], isoCode, proxy, status: data.status }
} catch (err) {
return new UnknownIpFetcherServiceError(err)
Expand Down