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

Save last ip, user-agent and fingerprint on users #499

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Redm4x
Copy link
Contributor

@Redm4x Redm4x commented Nov 26, 2024

Implement basic fingerprinting

  • Added the clientInfoMiddleware middleware to all routes to add the ip, user-agent and fingerprint to the request context.
  • Save the last ip, user-agent and fingerprint in the database (userSetting).
  • Updated hono to the latest version to get getConnInfo from hono/node-server.
    • Changes to the predictedBlockDate, predictedDateHeight, providerDeployments and transactionList endpoints were to fix typing issues that surfaced after upgrading.

The fingerprinting is done server-side and is inspired by express-fingerprint. It uses information from the following sources to generate a hash:

  • Client IP
  • User-Agent
  • Accept, Accept-Encoding and Accept-Language headers

@Redm4x Redm4x force-pushed the feat/user-fingerprint branch 3 times, most recently from bf34dbb to b079f0c Compare November 26, 2024 11:08
@@ -0,0 +1,3 @@
ALTER TABLE "userSetting" ADD COLUMN "lastIp" varchar(255);--> statement-breakpoint
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
ALTER TABLE "userSetting" ADD COLUMN "lastIp" varchar(255);--> statement-breakpoint
ALTER TABLE "userSetting" ADD COLUMN "last_ip" varchar(255);--> statement-breakpoint

Copy link
Contributor

Choose a reason for hiding this comment

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

cols are named in snake case

Copy link
Contributor

Choose a reason for hiding this comment

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

@@ -86,7 +99,10 @@ export async function getSettingsOrInit({ anonymousUserId, userId, wantedUsernam
email: email,
emailVerified: emailVerified,
stripeCustomerId: null,
subscribedToNewsletter: subscribedToNewsletter
subscribedToNewsletter: subscribedToNewsletter,
Copy link
Contributor

@ygrishajev ygrishajev Nov 26, 2024

Choose a reason for hiding this comment

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

Suggested change
subscribedToNewsletter: subscribedToNewsletter,
subscribedToNewsletter,

@@ -125,14 +141,26 @@ export async function getSettingsOrInit({ anonymousUserId, userId, wantedUsernam
email: email,
emailVerified: emailVerified,
stripeCustomerId: null,
subscribedToNewsletter: subscribedToNewsletter
subscribedToNewsletter: subscribedToNewsletter,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
subscribedToNewsletter: subscribedToNewsletter,
subscribedToNewsletter,

const user = await this.userRepository.create({
lastIp: ip,
lastUserAgent: userAgent,
lastFingerprint: lastFingerprint
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
lastFingerprint: lastFingerprint
lastFingerprint

@@ -20,8 +20,12 @@ export class UserController {
private readonly staleAnonymousUsersCleanerService: StaleAnonymousUsersCleanerService
) {}

async create(): Promise<AnonymousUserResponseOutput> {
const user = await this.userRepository.create();
async create(ip: string | undefined, userAgent: string | undefined, lastFingerprint: string | undefined): Promise<AnonymousUserResponseOutput> {
Copy link
Contributor

Choose a reason for hiding this comment

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

let's pass it as an object with params? controller method would generally accept a payload like so

You can access request via execution context here https://github.com/akash-network/console/blob/main/apps/api/src/core/services/request-context-interceptor/request-context.interceptor.ts

@Redm4x Redm4x marked this pull request as ready for review November 26, 2024 13:02
@Redm4x Redm4x requested a review from a team as a code owner November 26, 2024 13:02
Copy link
Contributor

@baktun14 baktun14 left a comment

Choose a reason for hiding this comment

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

I think this is fine for now, we log this and we can do better analysis on the usage and decide if we want to apply it or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants