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
2 changes: 2 additions & 0 deletions apps/backend/src/api/api.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import {
import { AnnouncementsController } from '@gitroom/backend/api/routes/announcements.controller';
import { AdminController } from '@gitroom/backend/api/routes/admin.controller';
import { DosOrgSyncWebhookController } from '@gitroom/backend/api/routes/dos-org-sync.controller';
import { ProvisionController } from '@gitroom/backend/api/routes/provision.controller';
import { AuthProviderManager } from '@gitroom/backend/services/auth/providers/providers.manager';
import { GithubProvider } from '@gitroom/backend/services/auth/providers/github.provider';
import { GoogleProvider } from '@gitroom/backend/services/auth/providers/google.provider';
Expand Down Expand Up @@ -84,6 +85,7 @@ const authenticatedController = [
NoAuthIntegrationsController,
OAuthController,
DosOrgSyncWebhookController,
ProvisionController,
...authenticatedController,
],
providers: [
Expand Down
233 changes: 233 additions & 0 deletions apps/backend/src/api/routes/provision.controller.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,233 @@
import {
Body,
Controller,
Headers,
HttpCode,
HttpException,
HttpStatus,
Post,
Req,
Res,
} from '@nestjs/common';
import { ApiTags } from '@nestjs/swagger';
import { Request, Response } from 'express';
import { OrganizationService } from '@gitroom/nestjs-libraries/database/prisma/organizations/organization.service';
import { UsersService } from '@gitroom/nestjs-libraries/database/prisma/users/users.service';
import { AuthService } from '@gitroom/backend/services/auth/auth.service';
import { AuthService as AuthChecker } from '@gitroom/helpers/auth/auth.service';
import { getCookieUrlFromDomain } from '@gitroom/helpers/subdomain/subdomain.management';
import { ProvisionUserDto } from '@gitroom/nestjs-libraries/dtos/provision/provision-user.dto';
import { ConsumeTicketDto } from '@gitroom/nestjs-libraries/dtos/provision/consume-ticket.dto';
import { makeId } from '@gitroom/nestjs-libraries/services/make.is';
import { Provider } from '@prisma/client';

@ApiTags('Provisioning')
@Controller('/v1')
export class ProvisionController {
constructor(
private _orgService: OrganizationService,
private _userService: UsersService,
private _authService: AuthService
) {}

private verifyAuth(authHeader?: string): boolean {
const secret =
process.env.PROVISIONING_SECRET_KEY ||
process.env.DOS_PROVISIONING_SECRET ||
process.env.DOS_SYNC_WEBHOOK_SECRET ||
process.env.INTERNAL_API_KEY ||
process.env.JWT_SECRET;

if (!secret) {
return true;
}
Comment on lines +41 to +43

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-critical critical

Security Vulnerability: Fail-Open Authentication

If none of the environment variables (PROVISIONING_SECRET_KEY, DOS_PROVISIONING_SECRET, etc.) are configured, verifyAuth returns true. This creates a critical security vulnerability where the /provision endpoint becomes completely public and unauthenticated in environments where these variables are missing or misconfigured.

Recommendation:
Fail secure by returning false if no secret is configured.

Suggested change
if (!secret) {
return true;
}
if (!secret) {
return false;
}


if (!authHeader) {
return false;
}

const token = authHeader.replace(/^Bearer\s+/i, '').trim();
return token === secret;
}

@Post('/provision')
@HttpCode(200)
async provision(
@Body() body: ProvisionUserDto,
@Headers('authorization') authHeader?: string
) {
if (!this.verifyAuth(authHeader)) {
throw new HttpException('Unauthorized', HttpStatus.UNAUTHORIZED);
}

const { userId, email, name, orgId, orgName, role } = body;

// 1. Find or create user
let user = await this._userService.getUserByProvider(userId, Provider.GENERIC);
if (!user && email) {
user = await this._userService.getUserByEmail(email);
}

let targetOrg: any = null;
if (orgId) {
targetOrg = await this._orgService.getOrgById(orgId);
}

const effectiveOrgName =
orgName || (name ? `${name}'s Workspace` : `${email.split('@')[0]} Workspace`);

if (!user) {
// Create user and initial organization
const created = await this._orgService.createOrgAndUser(
{
company: effectiveOrgName,
email,
password: '',
provider: 'GENERIC',
providerId: userId,
datafast_visitor_id: '',
},
'127.0.0.1',
'headless-provisioning'
);
user = created.users[0].user;
targetOrg = created;
Comment on lines +93 to +94

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

Potential Bug: Provisioned User is Inactive

When a user is created via createOrgAndUser, they are typically created with activated: false (requiring email verification). However, in consumeTicket (line 188), there is a strict check that throws an error if the user is not activated. Since this is a headless provisioning flow for trusted first-party integrations, the user should be activated automatically upon creation so they can consume the ticket immediately.

Recommendation:
Explicitly activate the user immediately after creation.

Suggested change
user = created.users[0].user;
targetOrg = created;
user = created.users[0].user;
await this._userService.activateUser(user.id);
targetOrg = created;


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

// 2. Ensure organization exists and user is assigned
if (orgId) {
if (!targetOrg) {
targetOrg = await this._orgService.createOrgForExistingUser(
user.id,
effectiveOrgName,
role || 'SUPERADMIN',
orgId
);
} else {
const userOrgs = await this._orgService.getOrgsByUserId(user.id);
const inOrg = userOrgs.some((o) => o.id === targetOrg.id);
if (!inOrg) {
const appRole = role === 'USER' ? 'USER' : 'ADMIN';
await this._orgService.addUserToOrg(
user.id,
makeId(5),
targetOrg.id,
appRole
);
}
}
}

if (!targetOrg) {
const userOrgs = await this._orgService.getOrgsByUserId(user.id);
targetOrg = userOrgs[0] || { id: orgId || makeId(10), name: effectiveOrgName };
}

// 3. Issue one-time login ticket (valid for 5 minutes)
const ticket = AuthChecker.signJWT({
userId: user.id,
orgId: targetOrg.id,
type: 'one_time_ticket',
exp: Math.floor(Date.now() / 1000) + 300,
});

const loginUrl = `${process.env.FRONTEND_URL}/auth/ticket?ticket=${ticket}`;

return {
success: true,
ticket,
loginUrl,
user: {
id: user.id,
email: user.email,
name: user.name,
},
organization: {
id: targetOrg.id,
name: targetOrg.name,
},
};
}

@Post('/ticket/consume')
@HttpCode(200)
async consumeTicket(
@Body() body: ConsumeTicketDto,
@Res({ passthrough: false }) response: Response
) {
if (!body?.ticket) {
throw new HttpException('Ticket is required', HttpStatus.BAD_REQUEST);
}

let payload: any;
try {
payload = AuthChecker.verifyJWT(body.ticket);
} catch (e) {
throw new HttpException('Invalid or expired ticket', HttpStatus.BAD_REQUEST);
}
Comment on lines +176 to +181

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

Security Issue: Replay Attack on "One-Time" Ticket

The ticket issued is a stateless JWT. Although it has a short expiration (5 minutes), there is no mechanism to track whether a ticket has already been consumed. An attacker who intercepts the ticket can reuse the same ticket multiple times within its 5-minute validity window.

Recommendation:
To make the ticket truly "one-time", implement a blocklist or consumption tracker (e.g., using Redis with a 5-minute TTL or a database table) to record consumed ticket IDs (using a jti claim in the JWT) and reject any ticket that has already been consumed.


if (payload?.type !== 'one_time_ticket' || !payload?.userId) {
throw new HttpException('Invalid ticket type', HttpStatus.BAD_REQUEST);
}

const user = await this._userService.getUserById(payload.userId);
if (!user || !user.activated) {
throw new HttpException('User not found or inactive', HttpStatus.NOT_FOUND);
}

const jwt = await this._authService.jwt(user);

response.cookie('auth', jwt, {
domain: getCookieUrlFromDomain(process.env.FRONTEND_URL!),
...(!process.env.NOT_SECURED
? {
secure: true,
httpOnly: true,
sameSite: 'none',
}
: {}),
Comment on lines +196 to +202

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-medium medium

Security Recommendation: Use sameSite: 'lax' instead of 'none'

Setting sameSite: 'none' allows the authentication cookie to be sent on cross-site requests, which exposes the application to CSRF (Cross-Site Request Forgery) attacks. Since the cookie is scoped to the domain/subdomain of FRONTEND_URL (using getCookieUrlFromDomain), sameSite: 'lax' is a much safer default and is sufficient for subdomain-based cookie sharing.

Suggested change
...(!process.env.NOT_SECURED
? {
secure: true,
httpOnly: true,
sameSite: 'none',
}
: {}),
...(!process.env.NOT_SECURED
? {
secure: true,
httpOnly: true,
sameSite: 'lax',
}
: {}),

expires: new Date(Date.now() + 1000 * 60 * 60 * 24 * 365),
});

if (payload.orgId) {
response.cookie('showorg', payload.orgId, {
domain: getCookieUrlFromDomain(process.env.FRONTEND_URL!),
...(!process.env.NOT_SECURED
? {
secure: true,
httpOnly: true,
sameSite: 'none',
}
: {}),
expires: new Date(Date.now() + 1000 * 60 * 60 * 24 * 365),
});
}

if (process.env.NOT_SECURED) {
response.header('auth', jwt);
if (payload.orgId) response.header('showorg', payload.orgId);
}

return response.json({
success: true,
jwt,
userId: user.id,
orgId: payload.orgId,
redirect_to: body.redirect_to || '/',
});
}
}
2 changes: 1 addition & 1 deletion apps/backend/src/services/auth/auth.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ export class AuthService {
return { token };
}

private async jwt(user: User) {
async jwt(user: User) {
if (user.password) {
delete user.password;
}
Expand Down
60 changes: 60 additions & 0 deletions apps/frontend/src/app/(app)/auth/ticket/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
'use client';

import { useEffect, useState } from 'react';
import { useSearchParams } from 'next/navigation';
import { useFetch } from '@gitroom/helpers/utils/custom.fetch';
import { LoadingComponent } from '@gitroom/frontend/components/layout/loading';

export default function TicketAuthPage() {
const searchParams = useSearchParams();
const fetch = useFetch();
const [error, setError] = useState('');

const ticket = searchParams.get('ticket');
const redirectTo = searchParams.get('redirect_to') || '/';

useEffect(() => {
if (!ticket) {
setError('Missing ticket parameter');
return;
}

fetch('/v1/ticket/consume', {
method: 'POST',
body: JSON.stringify({
ticket,
redirect_to: redirectTo,
}),
})
.then((r) => r.json())
.then((data) => {
if (data.success) {
window.location.href = data.redirect_to || redirectTo;
} else {
setError(data.message || 'Invalid or expired ticket');
}
})
.catch((err) => {
setError('Failed to consume authentication ticket');
});
}, [ticket, redirectTo]);

if (error) {
return (
<div className="flex flex-1 items-center justify-center text-white">
<div className="bg-third p-8 rounded-xl border border-tableBorder text-center max-w-md">
<h2 className="text-xl font-bold text-red-500 mb-2">Authentication Error</h2>
<p className="text-gray-300 text-sm mb-4">{error}</p>
<a
href="/auth/login"
className="inline-block bg-btnPrimary px-4 py-2 rounded-lg text-sm font-medium"
>
Go to Login
</a>
</div>
</div>
);
}

return <LoadingComponent />;
}
Comment on lines +1 to +60

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

Next.js Best Practice: Missing Suspense Boundary for useSearchParams

In Next.js (App Router), calling useSearchParams() in a client component during static rendering will cause the entire page to deoptimize to client-side rendering or throw an error during the production build unless it is wrapped in a <Suspense> boundary.

Recommendation:
Wrap the component or the hook usage in a <Suspense> boundary to ensure successful static builds and proper loading states.

'use client';

import { useEffect, useState, Suspense } from 'react';
import { useSearchParams } from 'next/navigation';
import { useFetch } from '@gitroom/helpers/utils/custom.fetch';
import { LoadingComponent } from '@gitroom/frontend/components/layout/loading';

function TicketAuthContent() {
  const searchParams = useSearchParams();
  const fetch = useFetch();
  const [error, setError] = useState('');

  const ticket = searchParams.get('ticket');
  const redirectTo = searchParams.get('redirect_to') || '/';

  useEffect(() => {
    if (!ticket) {
      setError('Missing ticket parameter');
      return;
    }

    fetch('/v1/ticket/consume', {
      method: 'POST',
      body: JSON.stringify({
        ticket,
        redirect_to: redirectTo,
      }),
    })
      .then((r) => r.json())
      .then((data) => {
        if (data.success) {
          window.location.href = data.redirect_to || redirectTo;
        } else {
          setError(data.message || 'Invalid or expired ticket');
        }
      })
      .catch((err) => {
        setError('Failed to consume authentication ticket');
      });
  }, [ticket, redirectTo]);

  if (error) {
    return (
      <div className="flex flex-1 items-center justify-center text-white">
        <div className="bg-third p-8 rounded-xl border border-tableBorder text-center max-w-md">
          <h2 className="text-xl font-bold text-red-500 mb-2">Authentication Error</h2>
          <p className="text-gray-300 text-sm mb-4">{error}</p>
          <a
            href="/auth/login"
            className="inline-block bg-btnPrimary px-4 py-2 rounded-lg text-sm font-medium"
          >
            Go to Login
          </a>
        </div>
      </div>
    );
  }

  return <LoadingComponent />;
}

export default function TicketAuthPage() {
  return (
    <Suspense fallback={<LoadingComponent />}>
      <TicketAuthContent />
    </Suspense>
  );
}

4 changes: 2 additions & 2 deletions apps/frontend/src/proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,8 @@ export async function proxy(request: NextRequest) {
);
}

// If the url is /auth and the cookie exists, redirect to /
if (nextUrl.pathname.startsWith('/auth') && authCookie) {
// If the url is /auth (except ticket consumption) and the cookie exists, redirect to /
if (nextUrl.pathname.startsWith('/auth') && !nextUrl.pathname.startsWith('/auth/ticket') && authCookie) {
return NextResponse.redirect(new URL(`/${url}`, nextUrl.href));
}
if (nextUrl.pathname.startsWith('/auth') && !authCookie) {
Expand Down
Loading
Loading