Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 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 .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ workflows:
branches:
only:
- develop
- pm-2539

# Production builds are exectuted only on tagged commits to the
# master branch.
Expand Down
11 changes: 8 additions & 3 deletions .env.sample
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,11 @@ SERVICEACC02_CID="devadmin1"
SERVICEACC02_SECRET="devadmin1"
SERVICEACC02_UID="100000027"

# Note: Registration default password is no longer configurable; for social/SSO
# registrations without a provided password, a unique 16-character random
# password is generated at registration time.
# Note: Registration default password is no longer configurable; for social/SSO
# registrations without a provided password, a unique 16-character random
# password is generated at registration time.


# Prisma configuration

IDENTITY_SERVICE_PRISMA_TIMEOUT=10000

Choose a reason for hiding this comment

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

[💡 style]
Consider adding a newline at the end of the file to adhere to POSIX standards, which can help prevent issues with certain tools and version control systems.

34 changes: 34 additions & 0 deletions .github/workflows/trivy.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
name: Trivy Scanner

permissions:
contents: read
security-events: write
on:
push:
branches:
- main
- dev
pull_request:
jobs:
trivy-scan:
name: Use Trivy
runs-on: ubuntu-24.04

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Consider using a stable version of ubuntu-latest instead of ubuntu-24.04 to ensure compatibility and reduce maintenance overhead when new Ubuntu versions are released.

steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Run Trivy scanner in repo mode
uses: aquasecurity/trivy-action@0.33.1

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Ensure that the version 0.33.1 of aquasecurity/trivy-action is the intended version and not a placeholder. Locking to a specific version can help avoid unexpected changes, but it also requires regular updates to benefit from new features and security patches.

with:
scan-type: "fs"
ignore-unfixed: true
format: "sarif"
output: "trivy-results.sarif"
severity: "CRITICAL,HIGH,UNKNOWN"
scanners: vuln,secret,misconfig,license
github-pat: ${{ secrets.GITHUB_TOKEN }}

- name: Upload Trivy scan results to GitHub Security tab
uses: github/codeql-action/upload-sarif@v3

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Verify that github/codeql-action/upload-sarif@v3 is the correct and intended version. Using a specific major version can provide stability, but ensure it aligns with your update and security policies.

with:
sarif_file: "trivy-results.sarif"
7 changes: 5 additions & 2 deletions src/api/role/role.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -390,13 +390,16 @@ describe('RoleService', () => {
).rejects.toThrow(NotFoundException);
});

it('should throw ConflictException if assignment already exists', async () => {
it('should ignore duplicate assignment if already exists', async () => {
mockPrisma.role.count.mockResolvedValue(1);
mockPrisma.roleAssignment.create.mockRejectedValue({ code: 'P2002' });

Choose a reason for hiding this comment

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

[❗❗ correctness]
The change from rejecting with a ConflictException to resolving with undefined when a duplicate assignment is detected alters the method's contract. Ensure that all consumers of assignRoleToSubject are aware of this change in behavior, as they may be expecting an exception to handle duplicates.


await expect(
service.assignRoleToSubject(roleId, subjectId, operatorId),
).rejects.toThrow(ConflictException);
).resolves.toBeUndefined();
expect(mockLogger.warn).toHaveBeenCalledWith(

Choose a reason for hiding this comment

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

[💡 maintainability]
The use of mockLogger.warn to log duplicate assignments is a good practice for observability. However, ensure that the logging level and message format align with the overall logging strategy of the application to maintain consistency.

`Attempt to assign role ${roleId} to subject ${subjectId} which already exists. Ignoring duplicate.`,
);
});
});

Expand Down
17 changes: 7 additions & 10 deletions src/api/role/role.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -376,18 +376,15 @@ export class RoleService {
} catch (error) {
if (error.code === Constants.prismaUniqueConflictcode) {
this.logger.warn(
`Attempt to assign role ${roleId} to subject ${subjectId} which already exists.`,
`Attempt to assign role ${roleId} to subject ${subjectId} which already exists. Ignoring duplicate.`,

Choose a reason for hiding this comment

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

[⚠️ correctness]
The change from throwing a ConflictException to returning silently when a duplicate role assignment is detected may lead to silent failures. Consider whether the caller needs to be informed of this situation to handle it appropriately.

);
throw new ConflictException(
`Role ${roleId} is already assigned to subject ${subjectId}.`,
);
} else {
this.logger.error(
`Failed to assign role ${roleId} to subject ${subjectId}: ${error.message}`,
error.stack,
);
throw error;
return;
}
this.logger.error(
`Failed to assign role ${roleId} to subject ${subjectId}: ${error.message}`,
error.stack,
);
throw error;
}
}

Expand Down
10 changes: 10 additions & 0 deletions src/shared/member-prisma/member-prisma.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,16 @@ export class MemberPrismaService
extends MemberPrismaClient
implements OnModuleInit, OnModuleDestroy
{
constructor() {
super({
transactionOptions: {
timeout: process.env.IDENTITY_SERVICE_PRISMA_TIMEOUT

Choose a reason for hiding this comment

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

[❗❗ correctness]
Consider validating that process.env.IDENTITY_SERVICE_PRISMA_TIMEOUT is a valid number before using parseInt. If the environment variable is not a valid number, parseInt will return NaN, which could lead to unexpected behavior.

? parseInt(process.env.IDENTITY_SERVICE_PRISMA_TIMEOUT, 10)
: 10000,
},
});
}

async onModuleInit() {
await this.$connect();
}
Expand Down