-
Notifications
You must be signed in to change notification settings - Fork 11
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
Add startTrackingVerified(), getVerifiedLocationToken(), and onTokenUpdated() #178
Changes from all commits
58eb2da
9f080ed
d8d6796
edb52aa
2623cd5
3f3aefb
d1f515b
26e1c0a
1f2d176
f4bb957
944e5e8
56b5769
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,7 +6,14 @@ import Logger from '../logger'; | |
import Session from '../session'; | ||
import Storage from '../storage'; | ||
|
||
import type { RadarTrackParams, RadarTrackVerifiedResponse } from '../types'; | ||
import type { RadarStartTrackingVerifiedParams, RadarTrackParams, RadarTrackVerifiedResponse } from '../types'; | ||
|
||
let tokenTimeoutId: any | null = null; | ||
let tokenCallback: ((token: RadarTrackVerifiedResponse) => void) | null = null; | ||
let lastToken: RadarTrackVerifiedResponse | null = null; | ||
let lastTokenNow: number = 0; | ||
let expectedCountryCode: string | null = null; | ||
let expectedStateCode: string | null = null; | ||
Comment on lines
+12
to
+16
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is probably fine to define the scoped variables like this, but there are a couple issues:
If we need to track the last call time across the page-load, probably need to put it in localstorage. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't need this to work across page load like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If these aren't sensitive it's probably fine. Maybe the user-provided ones should be static variables (callback and expected codes) |
||
|
||
class VerifyAPI { | ||
static async trackVerified(params: RadarTrackParams, encrypted: Boolean = false) { | ||
|
@@ -41,6 +48,8 @@ class VerifyAPI { | |
stopped: true, | ||
userId, | ||
encrypted, | ||
expectedCountryCode, | ||
expectedStateCode, | ||
}; | ||
|
||
let userAgent = navigator.userAgent; | ||
|
@@ -53,7 +62,7 @@ class VerifyAPI { | |
host: apple ? 'https://radar-verify.com:52516' : 'http://localhost:52516', | ||
}); | ||
|
||
let { user, events, token, expiresAt } = response; | ||
let { user, events, token, expiresAt, expiresIn, passed, failureReasons, _id } = response; | ||
let location; | ||
if (user && user.location && user.location.coordinates && user.locationAccuracy) { | ||
location = { | ||
|
@@ -62,12 +71,8 @@ class VerifyAPI { | |
accuracy: user.locationAccuracy, | ||
}; | ||
} | ||
let passed; | ||
let expiresIn; | ||
if (expiresAt) { | ||
expiresAt = new Date(expiresAt); | ||
passed = user?.fraud?.passed && user?.country?.passed && user?.state?.passed; | ||
expiresIn = (expiresAt.getTime() - Date.now()) / 1000; | ||
} | ||
|
||
const trackRes = { | ||
|
@@ -78,14 +83,86 @@ class VerifyAPI { | |
expiresAt, | ||
expiresIn, | ||
passed, | ||
failureReasons, | ||
_id, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
} as RadarTrackVerifiedResponse; | ||
|
||
if (options.debug) { | ||
trackRes.response = response; | ||
} | ||
|
||
lastToken = trackRes; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. token is the entire response? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. Deserialized to |
||
lastTokenNow = performance.now(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The problem with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We do something similar on the mobile SDKs ( |
||
|
||
if (tokenCallback) { | ||
tokenCallback(trackRes); | ||
} | ||
|
||
return trackRes; | ||
} | ||
|
||
static async startTrackingVerified(params: RadarStartTrackingVerifiedParams) { | ||
const doTrackVerified = async () => { | ||
const trackRes = await this.trackVerified({}); | ||
|
||
const { interval } = params; | ||
|
||
let expiresIn = 0; | ||
let minInterval = interval; | ||
|
||
if (trackRes) { | ||
expiresIn = (trackRes.expiresIn || expiresIn); | ||
|
||
// if expiresIn is shorter than interval, override interval | ||
minInterval = Math.min(expiresIn, interval); | ||
} | ||
|
||
// re-request early to maximize the likelihood that a cached token is available | ||
if (minInterval > 20) { | ||
minInterval = minInterval - 10; | ||
} | ||
|
||
// min interval is 10 seconds | ||
if (minInterval < 10) { | ||
minInterval = 10; | ||
Comment on lines
+120
to
+127
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so if I give the interval param as 21 then it is effectively 11 but if I give 15 it is effectively 15? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah. I think that's an edge case, but if it bothers us, I can update |
||
} | ||
|
||
if (tokenTimeoutId) { | ||
clearTimeout(tokenTimeoutId); | ||
} | ||
|
||
tokenTimeoutId = setTimeout(doTrackVerified, minInterval * 1000); | ||
}; | ||
|
||
doTrackVerified(); | ||
} | ||
|
||
static stopTrackingVerified() { | ||
if (tokenTimeoutId) { | ||
clearTimeout(tokenTimeoutId); | ||
} | ||
} | ||
|
||
static async getVerifiedLocationToken() { | ||
const lastTokenElapsed = (performance.now() - lastTokenNow) / 1000; | ||
|
||
if (lastToken) { | ||
if (lastTokenElapsed < (lastToken.expiresIn || 0)) { | ||
return lastToken; | ||
} | ||
} | ||
|
||
return this.trackVerified({}); | ||
} | ||
|
||
static setExpectedJurisdiction(countryCode?: string, stateCode?: string) { | ||
expectedCountryCode = countryCode || null; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If these are marked as optional in the definition, we wont need the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was getting errors because There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was getting errors because There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I think you can just define them as: let expectedCountryCode?: string; |
||
expectedStateCode = stateCode || null; | ||
} | ||
|
||
static onTokenUpdated(callback: (token: RadarTrackVerifiedResponse) => void) { | ||
tokenCallback = callback; | ||
} | ||
} | ||
|
||
export default VerifyAPI; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
export default '4.3.5-beta.0'; | ||
export default '4.3.5'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we namespace these as
verified
instead oftoken
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, it's probably preferable to mark things as optional instead of
null
Otherwise it needs to be explicitly set to
null