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

Feat upgrade api keys #601

Merged
merged 8 commits into from
Aug 3, 2024
Merged
Show file tree
Hide file tree
Changes from 4 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
1 change: 1 addition & 0 deletions docker-compose.dev.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ services:
context: ./
dockerfile: ./packages/api/Dockerfile.dev
environment:
ENV: ${ENV}
DATABASE_URL: postgresql://${POSTGRES_USER}:${POSTGRES_PASSWORD}@${POSTGRES_HOST}:5432/${POSTGRES_DB}?ssl=false
DISTRIBUTION: ${DISTRIBUTION}
WEBHOOK_INGRESS: ${WEBHOOK_INGRESS}
Expand Down
1 change: 1 addition & 0 deletions docker-compose.source.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ services:
context: ./
dockerfile: ./packages/api/Dockerfile
environment:
ENV: ${ENV}
DOPPLER_TOKEN: ${DOPPLER_TOKEN_API}
DATABASE_URL: postgresql://${POSTGRES_USER}:${POSTGRES_PASSWORD}@${POSTGRES_HOST}:5432/${POSTGRES_DB}?ssl=false
DISTRIBUTION: ${DISTRIBUTION}
Expand Down
1 change: 1 addition & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ services:
api:
image: panora.docker.scarf.sh/panoradotdev/backend-api:selfhosted
environment:
ENV: ${ENV}
DATABASE_URL: postgresql://${POSTGRES_USER}:${POSTGRES_PASSWORD}@${POSTGRES_HOST}:5432/${POSTGRES_DB}?ssl=false
DISTRIBUTION: ${DISTRIBUTION}
WEBHOOK_INGRESS: ${WEBHOOK_INGRESS}
Expand Down
2 changes: 1 addition & 1 deletion packages/api/src/@core/auth/auth.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ export class AuthController {
@ApiBody({ type: ApiKeyDto })
@ApiResponse({ status: 201 })
@UseGuards(JwtAuthGuard)
@Post()
@Post('api_keys')
async generateApiKey(@Body() data: ApiKeyDto): Promise<{ api_key: string }> {
return this.authService.generateApiKeyForUser(
data.userId,
Expand Down
74 changes: 42 additions & 32 deletions packages/api/src/@core/auth/auth.service.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { LoggerService } from '@@core/@core-services/logger/logger.service';
import { ProjectsService } from '@@core/projects/projects.service';
import { AuthError } from '@@core/utils/errors';

Check warning on line 3 in packages/api/src/@core/auth/auth.service.ts

View workflow job for this annotation

GitHub Actions / Build and Test (18.x)

'AuthError' is defined but never used
import { Injectable, BadRequestException } from '@nestjs/common';
import { JwtService } from '@nestjs/jwt';
import * as bcrypt from 'bcrypt';
Expand Down Expand Up @@ -120,7 +120,7 @@
to: email,
subject: 'Panora | Password Reset Request',
text: `You requested a password reset. Click the following link within one hour from now to reset your password: ${resetLink}`,
html: `<p>You requested a password reset. Click the link to reset your password:</p><a href="${resetLink}">${resetLink}</a> <p>The link will expire after one hour</p>`,

Check warning on line 123 in packages/api/src/@core/auth/auth.service.ts

View workflow job for this annotation

GitHub Actions / Build and Test (18.x)

'info' is assigned a value but never used
});

this.logger.log(`Send reset email to ${email} with token ${resetToken}`);
Expand Down Expand Up @@ -324,30 +324,36 @@
keyName: string,
): Promise<{ api_key: string }> {
try {

// Check project & User exist
const foundProject = await this.prisma.projects.findUnique({
where: { id_project: projectId },
});
if (!foundProject) {
throw new ReferenceError('project undefined');
throw new ReferenceError('Project not found');
}
const foundUser = await this.prisma.users.findUnique({
where: { id_user: userId },
});
if (!foundUser) {
throw new ReferenceError('user undefined');
throw new ReferenceError('User Not Found');
}

/*if (foundProject.id_organization !== foundUser.id_organization) {
throw new ReferenceError('User is not inside the project');
}*/
// Generate a new API key (use a secure method for generation)
const { access_token } = await this.generateApiKey(projectId, userId);
//const { access_token } = await this.generateApiKey(projectId, userId);
// Store the API key in the database associated with the user
//const hashed_token = this.hashApiKey(access_token);
//const hashed_token = this.hashApiKey(access_token);"

const base_key = `sk_${process.env.ENV}_${uuidv4()}`;
const hashed_key = crypto.createHash('sha256').update(base_key).digest('hex');

const new_api_key = await this.prisma.api_keys.create({
data: {
id_api_key: uuidv4(),
api_key_hash: access_token,
api_key_hash: hashed_key,
name: keyName,
id_project: projectId as string,
id_user: userId as string,
Expand All @@ -357,7 +363,7 @@
throw new ReferenceError('api key undefined');
}

return { api_key: access_token, ...new_api_key };
return { api_key: base_key, ...new_api_key };
} catch (error) {
throw error;
}
Expand All @@ -377,17 +383,11 @@



async getProjectIdForApiKey(apiKey: string) {
async getProjectIdForApiKey(hashed_apiKey: string) {
try {
// Decode the JWT to verify if it's valid and get the payload
const decoded = this.jwtService.verify(apiKey, {
secret: process.env.JWT_SECRET,
});

//const hashed_api_key = this.hashApiKey(apiKey);
const saved_api_key = await this.prisma.api_keys.findUnique({
where: {
api_key_hash: apiKey,
api_key_hash: hashed_apiKey,
},
});

Comment on lines +391 to 398
Copy link
Contributor

Choose a reason for hiding this comment

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

Add logging and improve error handling.

Consider adding logging for successful retrieval and handling cases where the API key is not found.

+ this.logger.log(`Retrieving project ID for API key`);

const saved_api_key = await this.prisma.api_keys.findUnique({
  where: {
    api_key_hash: hashed_apiKey,
  },
});

if (!saved_api_key) {
  this.logger.error(`API key not found: ${hashed_apiKey}`);
  throw new ReferenceError('API key not found');
}

this.logger.log(`Project ID retrieved successfully for API key`);
return saved_api_key.id_project;
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async getProjectIdForApiKey(hashed_apiKey: string) {
try {
// Decode the JWT to verify if it's valid and get the payload
const decoded = this.jwtService.verify(apiKey, {
secret: process.env.JWT_SECRET,
});
//const hashed_api_key = this.hashApiKey(apiKey);
const saved_api_key = await this.prisma.api_keys.findUnique({
where: {
api_key_hash: apiKey,
api_key_hash: hashed_apiKey,
},
});
async getProjectIdForApiKey(hashed_apiKey: string) {
this.logger.log(`Retrieving project ID for API key`);
const saved_api_key = await this.prisma.api_keys.findUnique({
where: {
api_key_hash: hashed_apiKey,
},
});
if (!saved_api_key) {
this.logger.error(`API key not found: ${hashed_apiKey}`);
throw new ReferenceError('API key not found');
}
this.logger.log(`Project ID retrieved successfully for API key`);
return saved_api_key.id_project;

Expand All @@ -399,33 +399,43 @@

async validateApiKey(apiKey: string): Promise<boolean> {
try {

// TO DO : add Expiration in part 3

// Decode the JWT to verify if it's valid and get the payload
const decoded = this.jwtService.verify(apiKey, {
secret: process.env.JWT_SECRET,
});
// const decoded = this.jwtService.verify(apiKey, {
// secret: process.env.JWT_SECRET,
// });


//const hashed_api_key = this.hashApiKey(apiKey);
// pseudo-code:
// 1 - SHA256 the API key from the header
const hashed_key = crypto.createHash('sha256').update(apiKey).digest('hex');


// 2- check against DB
// if not found, return false
const saved_api_key = await this.prisma.api_keys.findUnique({
where: {
api_key_hash: apiKey,
api_key_hash: hashed_key,
},
});

if (!saved_api_key) {
throw new ReferenceError('Api Key undefined');
}
if (String(decoded.project_id) !== String(saved_api_key.id_project)) {
throw new ReferenceError(
'Failed to validate API key: projectId mismatch.',
);
}

// Validate that the JWT payload matches the provided userId and projectId
if (String(decoded.sub) !== String(saved_api_key.id_user)) {
throw new ReferenceError(
'Failed to validate API key: userId mismatch.',
);
throw new ReferenceError('API Key not found.');
}
// if (String(decoded.project_id) !== String(saved_api_key.id_project)) {
// throw new ReferenceError(
// 'Failed to validate API key: projectId mismatch.',
// );
// }

// // Validate that the JWT payload matches the provided userId and projectId
// if (String(decoded.sub) !== String(saved_api_key.id_user)) {
// throw new ReferenceError(
// 'Failed to validate API key: userId mismatch.',
// );
// }
Copy link
Contributor

Choose a reason for hiding this comment

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

Improve logging, error handling, and remove commented-out code.

Add logging for validation steps, improve error messages, and remove commented-out code for clarity.

+ this.logger.log(`Validating API key`);

const hashed_key = crypto.createHash('sha256').update(apiKey).digest('hex');

const saved_api_key = await this.prisma.api_keys.findUnique({
  where: {
    api_key_hash: hashed_key,
  },
});

if (!saved_api_key) {
  this.logger.error(`API key not found: ${hashed_key}`);
  throw new ReferenceError('API key not found');
}

this.logger.log(`API key validated successfully`);
return true;

Committable suggestion was skipped due to low confidence.

return true;
} catch (error) {
throw error;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { HeaderAPIKeyStrategy } from 'passport-headerapikey';
import { PassportStrategy } from '@nestjs/passport';
import { Injectable, UnauthorizedException } from '@nestjs/common';
import { AuthService } from '../auth.service';
import * as crypto from 'crypto';
Copy link
Contributor

Choose a reason for hiding this comment

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

Use the node: protocol for Node.js built-in modules.

Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.

- import * as crypto from 'crypto';
+ import * as crypto from 'node:crypto';
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import * as crypto from 'crypto';
import * as crypto from 'node:crypto';
Tools
Biome

[error] 5-5: A Node.js builtin module should be imported with the node: protocol.

Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.

(lint/style/useNodejsImportProtocol)


@Injectable()
export class ApiKeyStrategy extends PassportStrategy(
Expand All @@ -10,16 +11,17 @@ export class ApiKeyStrategy extends PassportStrategy(
) {
constructor(private authService: AuthService) {
super(
{ header: 'Authorization', prefix: 'Bearer ' },
{ header: 'x-api-key', prefix: '' },
true,
async (apikey: string, done, req) => {
try {
const isValid = await this.authService.validateApiKey(apikey);
if (!isValid) {
return done(new UnauthorizedException('Invalid API Key'), null);
}
const hashed_api_key = crypto.createHash('sha256').update(apikey).digest('hex');
const projectId = await this.authService.getProjectIdForApiKey(
apikey,
hashed_api_key,
);
//console.log('validating api request... : ' + req.user);
// If the API key is valid, attach the user to the request object
Expand Down
4 changes: 4 additions & 0 deletions packages/api/src/@core/connections/@utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ export class ConnectionUtils {
token: string,
): Promise<ConnectionMetadata> {
try {
console.log('**********')
console.log(token);
console.log('**********')
Comment on lines +19 to +21
Copy link
Contributor

Choose a reason for hiding this comment

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

Avoid logging sensitive information.

Logging sensitive information such as tokens can pose a security risk. Consider removing or obfuscating the token in the logs.

- console.log('**********')
- console.log(token);
- console.log('**********')
+ console.log('**********')
+ console.log('Token received');
+ console.log('**********')
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
console.log('**********')
console.log(token);
console.log('**********')
console.log('**********')
console.log('Token received');
console.log('**********')


const res = await this.prisma.connections.findFirst({
where: {
connection_token: token,
Expand Down
Loading