Skip to content
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
160 changes: 77 additions & 83 deletions apps/backend/src/services/auth/auth.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,52 @@ export class AuthService {
}
}

private async syncUserOrganizations(
userId: string,
organizations?: Array<{
id: string;
name: string;
role?: 'OWNER' | 'ADMIN' | 'MEMBER' | 'SUPERADMIN';
}>
) {
if (!organizations || !organizations.length) {
return;
}

const userOrgs = await this._organizationService.getOrgsByUserId(userId);
const existingOrgMap = new Map(userOrgs.map((o) => [o.id, o]));

for (const orgInfo of organizations) {
const isOwner = orgInfo.role === 'OWNER' || orgInfo.role === 'SUPERADMIN';
const role = isOwner ? 'SUPERADMIN' : orgInfo.role === 'ADMIN' ? 'ADMIN' : 'USER';
const existing = existingOrgMap.get(orgInfo.id);

if (existing) {
if (orgInfo.name && existing.name !== orgInfo.name) {
await this._organizationService
.updateOrganizationName(orgInfo.id, orgInfo.name)
.catch(() => {});
}
} else {
const orgExistsInDb = await this._organizationService.getOrgById(orgInfo.id);
if (orgExistsInDb) {
if (orgInfo.name && orgExistsInDb.name !== orgInfo.name) {
await this._organizationService
.updateOrganizationName(orgInfo.id, orgInfo.name)
.catch(() => {});
}
await this._organizationService
.addUserToOrg(userId, makeId(5), orgInfo.id, role === 'SUPERADMIN' ? 'ADMIN' : role)
.catch(() => {});
Comment on lines +158 to +174

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Updating the organization name should be restricted to users with administrative privileges (e.g., OWNER, SUPERADMIN, or ADMIN). Currently, any user logging in who has a matching organization in their OAuth claims will trigger an organization name update, regardless of their role. This could allow a regular member to inadvertently rename the organization for all users.

Consider checking the user's role before calling updateOrganizationName.

Suggested change
if (existing) {
if (orgInfo.name && existing.name !== orgInfo.name) {
await this._organizationService
.updateOrganizationName(orgInfo.id, orgInfo.name)
.catch(() => {});
}
} else {
const orgExistsInDb = await this._organizationService.getOrgById(orgInfo.id);
if (orgExistsInDb) {
if (orgInfo.name && orgExistsInDb.name !== orgInfo.name) {
await this._organizationService
.updateOrganizationName(orgInfo.id, orgInfo.name)
.catch(() => {});
}
await this._organizationService
.addUserToOrg(userId, makeId(5), orgInfo.id, role === 'SUPERADMIN' ? 'ADMIN' : role)
.catch(() => {});
if (existing) {
if (orgInfo.name && existing.name !== orgInfo.name && (role === 'SUPERADMIN' || role === 'ADMIN')) {
await this._organizationService
.updateOrganizationName(orgInfo.id, orgInfo.name)
.catch(() => {});
}
} else {
const orgExistsInDb = await this._organizationService.getOrgById(orgInfo.id);
if (orgExistsInDb) {
if (orgInfo.name && orgExistsInDb.name !== orgInfo.name && (role === 'SUPERADMIN' || role === 'ADMIN')) {
await this._organizationService
.updateOrganizationName(orgInfo.id, orgInfo.name)
.catch(() => {});
}
await this._organizationService
.addUserToOrg(userId, makeId(5), orgInfo.id, role === 'SUPERADMIN' ? 'ADMIN' : role)
.catch(() => {});

} else {
await this._organizationService
.createOrgForExistingUser(userId, orgInfo.name, role, orgInfo.id)
.catch(() => {});
}
}
}
}

private async loginOrRegisterProvider(
provider: Provider,
body: CreateOrgUserDto,
Expand All @@ -161,24 +207,7 @@ export class AuthService {
}

if (providerUser.organizations && providerUser.organizations.length > 0) {
const userOrgs = await this._organizationService.getOrgsByUserId(user.id);
const existingOrgIds = new Set(userOrgs.map((o) => o.id));

for (const orgInfo of providerUser.organizations) {
if (!existingOrgIds.has(orgInfo.id)) {
const role = orgInfo.role === 'MEMBER' ? 'USER' : 'ADMIN';
const orgExists = await this._organizationService.getOrgById(orgInfo.id);
if (orgExists) {
await this._organizationService
.addUserToOrg(user.id, makeId(5), orgInfo.id, role)
.catch(() => {});
} else {
await this._organizationService
.createOrgForExistingUser(user.id, orgInfo.name, role === 'ADMIN' ? 'ADMIN' : 'USER', orgInfo.id)
.catch(() => {});
}
}
}
await this.syncUserOrganizations(user.id, providerUser.organizations);
}

return user;
Expand All @@ -194,50 +223,19 @@ export class AuthService {
body.company ||
(providerUser.name ? `${providerUser.name}'s Organization` : providerUser.email.split('@')[0]);

let create: any;
if (firstOrg?.id) {
const orgExists = await this._organizationService.getOrgById(firstOrg.id);
if (!orgExists) {
create = await this._organizationService.createOrgAndUser(
{
company: companyName,
email: providerUser.email,
password: '',
provider,
providerId: providerUser.id,
datafast_visitor_id: body.datafast_visitor_id || '',
},
ip,
userAgent
);
} else {
create = await this._organizationService.createOrgAndUser(
{
company: companyName,
email: providerUser.email,
password: '',
provider,
providerId: providerUser.id,
datafast_visitor_id: body.datafast_visitor_id || '',
},
ip,
userAgent
);
}
} else {
create = await this._organizationService.createOrgAndUser(
{
company: companyName,
email: providerUser.email,
password: '',
provider,
providerId: providerUser.id,
datafast_visitor_id: body.datafast_visitor_id || '',
},
ip,
userAgent
);
}
const create = await this._organizationService.createOrgAndUser(
{
company: companyName,
email: providerUser.email,
password: '',
provider,
providerId: providerUser.id,
datafast_visitor_id: body.datafast_visitor_id || '',
orgId: firstOrg?.id,
},
ip,
userAgent
);
Comment on lines +226 to +238

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

If firstOrg?.id already exists in the database (e.g., when a second user from the same organization registers), calling createOrgAndUser with orgId: firstOrg.id will throw a database unique constraint violation error on the organization.id primary key, causing the registration to fail completely.

Before calling createOrgAndUser, you must check if the organization already exists. If it does, you should create the user and associate them with the existing organization instead of attempting to recreate the organization.


if (providerUser.name) {
await this._userService.changePersonal(create.users[0].user.id, {
Expand All @@ -246,26 +244,8 @@ export class AuthService {
});
}

if (providerUser.organizations && providerUser.organizations.length > 1) {
for (let i = 1; i < providerUser.organizations.length; i++) {
const orgInfo = providerUser.organizations[i];
const role = orgInfo.role === 'MEMBER' ? 'USER' : 'ADMIN';
const orgExists = await this._organizationService.getOrgById(orgInfo.id);
if (orgExists) {
await this._organizationService
.addUserToOrg(create.users[0].user.id, makeId(5), orgInfo.id, role)
.catch(() => {});
} else {
await this._organizationService
.createOrgForExistingUser(
create.users[0].user.id,
orgInfo.name,
role === 'ADMIN' ? 'ADMIN' : 'USER',
orgInfo.id
)
.catch(() => {});
}
}
if (providerUser.organizations && providerUser.organizations.length > 0) {
await this.syncUserOrganizations(create.users[0].user.id, providerUser.organizations);
}

this._track('register', providerUser.email, body.datafast_visitor_id).catch(
Expand Down Expand Up @@ -412,11 +392,25 @@ export class AuthService {
if (!user) {
throw new Error('Invalid user');
}
const checkExists = await this._userService.getUserByProvider(
let checkExists = await this._userService.getUserByProvider(
user.id,
provider as Provider
);
if (!checkExists && user.email) {
checkExists = await this._userService.getUserByEmail(user.email);
}
Comment on lines +399 to +401

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

security-high high

Logging in a user solely by matching their email address from an external OAuth provider without verifying that the email is verified by the provider can lead to Account Takeover (ATO) vulnerabilities. If a malicious actor registers an account on the OAuth provider using a victim's email address (without verification) and then logs in via OAuth, they will gain access to the victim's account on this platform.

Please ensure that:

  1. The OAuth provider's payload indicates that the email is verified (e.g., email_verified: true or equivalent claim).
  2. You link the provider to the user's account (updating providerName and providerId) so that subsequent logins are securely mapped via getUserByProvider rather than repeatedly falling back to email matching.

if (checkExists) {
if (user.name && checkExists.name !== user.name) {
await this._userService.changePersonal(checkExists.id, {
fullname: user.name,
bio: checkExists.bio || '',
});
}

if (user.organizations && user.organizations.length > 0) {
await this.syncUserOrganizations(checkExists.id, user.organizations);
}

return { jwt: await this.jwt(checkExists) };
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -350,13 +350,14 @@ export class OrganizationRepository {
}

async createOrgAndUser(
body: Omit<CreateOrgUserDto, 'providerToken'> & { providerId?: string },
body: Omit<CreateOrgUserDto, 'providerToken'> & { providerId?: string; orgId?: string },
hasEmail: boolean,
ip: string,
userAgent: string
) {
return this._organization.model.organization.create({
data: {
...(body.orgId ? { id: body.orgId } : {}),
name: body.company,
apiKey: AuthService.fixedEncryption(makeId(20)),
allowTrial: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export class OrganizationService {
private _notificationsService: NotificationService
) {}
async createOrgAndUser(
body: Omit<CreateOrgUserDto, 'providerToken'> & { providerId?: string },
body: Omit<CreateOrgUserDto, 'providerToken'> & { providerId?: string; orgId?: string },
ip: string,
userAgent: string
) {
Expand Down
Loading