-
Notifications
You must be signed in to change notification settings - Fork 64
feat: SSO support and clientcredentials based switchig for single sign on feature #1183
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
Conversation
Signed-off-by: Sai Ranjit Tummalapalli <[email protected]>
Signed-off-by: bhavanakarwade <[email protected]>
* fix: added api param validations Signed-off-by: bhavanakarwade <[email protected]> * fix: removed unnecessary validations Signed-off-by: bhavanakarwade <[email protected]> * fix:resolved validations issue Signed-off-by: bhavanakarwade <[email protected]> * added comment on function for understanding Signed-off-by: bhavanakarwade <[email protected]> * fix: resolve orgid validations Signed-off-by: bhavanakarwade <[email protected]> * fix: added response message Signed-off-by: bhavanakarwade <[email protected]> * fix: added space in response messages Signed-off-by: bhavanakarwade <[email protected]> --------- Signed-off-by: bhavanakarwade <[email protected]>
Signed-off-by: pallavighule <[email protected]>
* fix: removed prisma commands in build stage Signed-off-by: [email protected] <[email protected]> * feat: create yml file for all services Signed-off-by: [email protected] <[email protected]> * Updated Dockerfiles Signed-off-by: [email protected] <[email protected]> * Updated yml to push docker images Signed-off-by: [email protected] <[email protected]> * updated yml files Signed-off-by: [email protected] <[email protected]> --------- Signed-off-by: [email protected] <[email protected]>
* fix: used single yml file using matrix instead of multiple files Signed-off-by: [email protected] <[email protected]> * fix: renamed yml file to cicd.yml Signed-off-by: [email protected] <[email protected]> * fix: renamed yml file Signed-off-by: [email protected] <[email protected]> * fix: renamed yml file Signed-off-by: [email protected] <[email protected]> * chore: removed id-token and attestations Signed-off-by: [email protected] <[email protected]> --------- Signed-off-by: [email protected] <[email protected]>
* chore: add aws into monorepo Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> * chore: update tsconfig Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> * refactor:tsconfig.build.json file Signed-off-by: Tipu_Singh <[email protected]> * refactor: remove redundant .nvmrc file Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> --------- Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> Signed-off-by: Tipu_Singh <[email protected]> Co-authored-by: Tipu_Singh <[email protected]>
Signed-off-by: Sai Ranjit Tummalapalli <[email protected]>
* refactor: merge nats-interceptor into common Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> * refactor: merge repsonses lib into common Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> * refactor: create common function to handle errors Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> * chore: add todo Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> * fix: remove missed image service Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> * fix: create separate function to handle common error Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> * chore: add comment for the purpose of the functions Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> * chore: update function comments Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> --------- Signed-off-by: Sai Ranjit Tummalapalli <[email protected]>
* refactor: merge nats-interceptor into common Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> * refactor: merge repsonses lib into common Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> * refactor: create common function to handle errors Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> * chore: add todo Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> * refactor: create common package in libs Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> * fix: remove missed image service Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> * refactor: common service Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> --------- Signed-off-by: Sai Ranjit Tummalapalli <[email protected]>
* wip: support nested attributes while creating schema Signed-off-by: bhavanakarwade <[email protected]> * wip: aligned issuance functionality with nested attributes structure Signed-off-by: bhavanakarwade <[email protected]> * refactor: modify csv to json function Signed-off-by: bhavanakarwade <[email protected]> * fix: formatting changes Signed-off-by: bhavanakarwade <[email protected]> * fix: resolved sonar cloud issue Signed-off-by: bhavanakarwade <[email protected]> * fix: security hotspot issue Signed-off-by: bhavanakarwade <[email protected]> * feat: added schema builder function Signed-off-by: bhavanakarwade <[email protected]> * fix: resolved issue Signed-off-by: bhavanakarwade <[email protected]> * refactor: modify extract attributes function Signed-off-by: bhavanakarwade <[email protected]> * fix: destructured objects Signed-off-by: bhavanakarwade <[email protected]> * feat: added description property Signed-off-by: bhavanakarwade <[email protected]> --------- Signed-off-by: bhavanakarwade <[email protected]>
* wip: support nested attributes while creating schema Signed-off-by: bhavanakarwade <[email protected]> * wip: aligned issuance functionality with nested attributes structure Signed-off-by: bhavanakarwade <[email protected]> * refactor: modify csv to json function Signed-off-by: bhavanakarwade <[email protected]> * fix: formatting changes Signed-off-by: bhavanakarwade <[email protected]> * fix: resolved sonar cloud issue Signed-off-by: bhavanakarwade <[email protected]> * fix: security hotspot issue Signed-off-by: bhavanakarwade <[email protected]> * feat: added schema builder function Signed-off-by: bhavanakarwade <[email protected]> * fix: resolved issue Signed-off-by: bhavanakarwade <[email protected]> * refactor: modify extract attributes function Signed-off-by: bhavanakarwade <[email protected]> * fix: destructured objects Signed-off-by: bhavanakarwade <[email protected]> * feat: added description property Signed-off-by: bhavanakarwade <[email protected]> * fix: added validations for schema type Signed-off-by: bhavanakarwade <[email protected]> * formatted enum file Signed-off-by: bhavanakarwade <[email protected]> * chore: added enum for indy schema data type Signed-off-by: bhavanakarwade <[email protected]> --------- Signed-off-by: bhavanakarwade <[email protected]>
…te and city (#1180) * refactor: update organization API to support updation of country, state and city Signed-off-by: pranalidhanavade <[email protected]> * resolved sonarlint issues Signed-off-by: pranalidhanavade <[email protected]> * resolved sonarlint issues Signed-off-by: pranalidhanavade <[email protected]> --------- Signed-off-by: pranalidhanavade <[email protected]>
Signed-off-by: pranalidhanavade <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
const encryptedToken = CryptoJS.AES.encrypt(JSON.stringify(clientCredential), process.env.CRYPTO_PRIVATE_KEY).toString(); | ||
const command = `${process.cwd()}/${scriptPath} ${dbUrl}`; | ||
|
||
const { stdout, stderr } = await execPromise(command); |
Check warning
Code scanning / CodeQL
Shell command built from environment values Medium
absolute path
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
To fix the issue, we will replace the use of execPromise
with execFilePromise
, which allows us to pass the command and its arguments separately. This approach avoids shell interpretation of the arguments, mitigating the risk of shell injection. Specifically:
- Extract the script path and arguments into separate variables.
- Use
execFilePromise
to execute the script, passing the script path as the command and the arguments as an array. - Ensure that all inputs (e.g.,
scriptPath
,dbUrl
) are validated or sanitized before use.
The changes will be applied to both the importGeoLocationMasterData
and updateClientCredential
functions.
-
Copy modified lines R404-R405 -
Copy modified line R407 -
Copy modified lines R437-R438 -
Copy modified line R440
@@ -403,5 +403,6 @@ | ||
|
||
const command = `${process.cwd()}/${scriptPath} ${dbUrl}`; | ||
const scriptFullPath = `${process.cwd()}/${scriptPath}`; | ||
const args = [dbUrl]; | ||
|
||
const { stdout, stderr } = await execPromise(command); | ||
const { stdout, stderr } = await execPromise(scriptFullPath, args); | ||
|
||
@@ -435,5 +436,6 @@ | ||
|
||
const command = `${process.cwd()}/${scriptPath} ${dbUrl} ${encryptedClientId} ${encryptedClientSecret}`; | ||
const scriptFullPath = `${process.cwd()}/${scriptPath}`; | ||
const args = [dbUrl, encryptedClientId, encryptedClientSecret]; | ||
|
||
const { stdout, stderr } = await execPromise(command); | ||
const { stdout, stderr } = await execPromise(scriptFullPath, args); | ||
|
async function main(): Promise<void> { | ||
const command = `${process.cwd()}/${scriptPath} ${dbUrl} ${encryptedClientId} ${encryptedClientSecret}`; | ||
|
||
const { stdout, stderr } = await execPromise(command); |
Check warning
Code scanning / CodeQL
Shell command built from environment values Medium
absolute path
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
To fix the issue, we will replace the use of execPromise
with execFilePromise
, which allows us to pass the command and its arguments separately. This approach avoids shell interpretation of the command string and mitigates the risk of shell injection. Specifically:
- Extract the script path and arguments into separate variables.
- Use
execFilePromise
to execute the script with the arguments passed as an array. - Ensure that all inputs (e.g.,
scriptPath
,dbUrl
,encryptedClientId
,encryptedClientSecret
) are validated or sanitized before use.
Changes will be made to the updateClientCredential
function in libs/prisma-service/prisma/seed.ts
.
-
Copy modified line R6 -
Copy modified line R9 -
Copy modified lines R436-R437 -
Copy modified line R439
@@ -5,6 +5,6 @@ | ||
import { CommonConstants } from '../../common/src/common.constant'; | ||
import { exec } from 'child_process'; | ||
import { execFile } from 'child_process'; | ||
import * as util from 'util'; | ||
import { encryptClientCredential } from '@credebl/common/cast.helper'; | ||
const execPromise = util.promisify(exec); | ||
const execPromise = util.promisify(execFile); | ||
|
||
@@ -435,5 +435,6 @@ | ||
|
||
const command = `${process.cwd()}/${scriptPath} ${dbUrl} ${encryptedClientId} ${encryptedClientSecret}`; | ||
const scriptFullPath = `${process.cwd()}/${scriptPath}`; | ||
const args = [dbUrl, encryptedClientId, encryptedClientSecret]; | ||
|
||
const { stdout, stderr } = await execPromise(command); | ||
const { stdout, stderr } = await execPromise(scriptFullPath, args); | ||
|
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
Signed-off-by: Krishna Waske <[email protected]>
|
Signed-off-by: Sahil Kamble <[email protected]>
refactor: update dockerfile to reduce dockerimages
* fix: qa conflict fixes (#1214) * chore: Platform version upgrade (#1100) * chore: platform version upgrade Signed-off-by: pranalidhanavade <[email protected]> * fix: docker file for cloud wallet Signed-off-by: pranalidhanavade <[email protected]> --------- Signed-off-by: pranalidhanavade <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * Add OpenSSL installation support for Docker image Signed-off-by: [email protected] <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * Add OpenSSL installation support for Docker image Signed-off-by: [email protected] <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * Add OpenSSL installation support for Docker image Signed-off-by: [email protected] <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * fix: Changes in NATS interceptor for error handling (#1103) * fix: changes in nats intercepor for error handling Signed-off-by: pranalidhanavade <[email protected]> * fix: Error handling for send proof request with identical attributes when one attribute uses predicates and the other does not Signed-off-by: pranalidhanavade <[email protected]> * fix: Resolve comments on PR Signed-off-by: pranalidhanavade <[email protected]> * fix: added comments in service Signed-off-by: pranalidhanavade <[email protected]> * fix: added comments in service Signed-off-by: pranalidhanavade <[email protected]> --------- Signed-off-by: pranalidhanavade <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * error handling in NATS interceptor (#1106) Signed-off-by: pranalidhanavade <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * fix: exception handling for common handler (#1108) * fix: exception handling Signed-off-by: pranalidhanavade <[email protected]> * refactor: NATS interceptor Signed-off-by: pranalidhanavade <[email protected]> * refactor: exception handler component changes Signed-off-by: pranalidhanavade <[email protected]> --------- Signed-off-by: pranalidhanavade <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * feat: add array data type in schema payload (#1110) * fix:added array data type in schema payload Signed-off-by: pallavicoder <[email protected]> * fix:added nested attributes in schema paylaod Signed-off-by: pallavicoder <[email protected]> * feat: add array data type while creating schema Signed-off-by: bhavanakarwade <[email protected]> * replace hardcoded value with dynamic Signed-off-by: bhavanakarwade <[email protected]> * fix: removed commented code Signed-off-by: bhavanakarwade <[email protected]> --------- Signed-off-by: pallavicoder <[email protected]> Signed-off-by: bhavanakarwade <[email protected]> Co-authored-by: bhavanakarwade <[email protected]> Signed-off-by: bhavanakarwade <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * fix: added search on schema name for credentials list (#1111) * fix:added array data type in schema payload Signed-off-by: pallavicoder <[email protected]> * fix:added nested attributes in schema paylaod Signed-off-by: pallavicoder <[email protected]> * feat: add array data type while creating schema Signed-off-by: bhavanakarwade <[email protected]> * replace hardcoded value with dynamic Signed-off-by: bhavanakarwade <[email protected]> * fix: removed commented code Signed-off-by: bhavanakarwade <[email protected]> * fix: added search on schema name Signed-off-by: bhavanakarwade <[email protected]> * fix: made api property optional Signed-off-by: bhavanakarwade <[email protected]> * handled conditions for empty array Signed-off-by: bhavanakarwade <[email protected]> --------- Signed-off-by: pallavicoder <[email protected]> Signed-off-by: bhavanakarwade <[email protected]> Co-authored-by: pallavicoder <[email protected]> Signed-off-by: bhavanakarwade <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * chore: added parentThreadId in webhook dto (#1112) * fix:added array data type in schema payload Signed-off-by: pallavicoder <[email protected]> * fix:added nested attributes in schema paylaod Signed-off-by: pallavicoder <[email protected]> * feat: add array data type while creating schema Signed-off-by: bhavanakarwade <[email protected]> * replace hardcoded value with dynamic Signed-off-by: bhavanakarwade <[email protected]> * fix: removed commented code Signed-off-by: bhavanakarwade <[email protected]> * fix: added search on schema name Signed-off-by: bhavanakarwade <[email protected]> * fix: made api property optional Signed-off-by: bhavanakarwade <[email protected]> * handled conditions for empty array Signed-off-by: bhavanakarwade <[email protected]> * chore: added loggers Signed-off-by: bhavanakarwade <[email protected]> * chore: added loggers for issuance and verification Signed-off-by: bhavanakarwade <[email protected]> * chore: added parentthreadid in webhook dto Signed-off-by: bhavanakarwade <[email protected]> * fix: remove unnecessary loggers Signed-off-by: bhavanakarwade <[email protected]> * fix: remove unnecessary loggers Signed-off-by: bhavanakarwade <[email protected]> --------- Signed-off-by: pallavicoder <[email protected]> Signed-off-by: bhavanakarwade <[email protected]> Co-authored-by: pallavicoder <[email protected]> Signed-off-by: bhavanakarwade <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * fix: changes schema endpoint (#1114) Signed-off-by: Tipu_Singh <[email protected]> Signed-off-by: bhavanakarwade <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * feat: added purpose property in send proof request (#1113) Signed-off-by: bhavanakarwade <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * refactor:modification on API summary and description (#1116) Signed-off-by: Tipu_Singh <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * chore: added orgdid in get all orgs response Signed-off-by: bhavanakarwade <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * refactor: improve response for oob verification via email (#1118) Signed-off-by: bhavanakarwade <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * refactor: swagger documentation changes for auth, schema, ledger, credential-definition, agent and user controllers. (#1117) * swagger documentation changes for auth, schema and user controllers Signed-off-by: pranalidhanavade <[email protected]> * swagger documentation changes for auth, schema and user controller Signed-off-by: pranalidhanavade <[email protected]> * refactor: API descrption Signed-off-by: pranalidhanavade <[email protected]> * refactor: swagger API documentation for agent controller Signed-off-by: pranalidhanavade <[email protected]> * fix: API trim validation Signed-off-by: pranalidhanavade <[email protected]> * fix: Removed extra description related to pagination from API documentation Signed-off-by: pranalidhanavade <[email protected]> * fix: changes in email example Signed-off-by: pranalidhanavade <[email protected]> * fix: changes in API documentation Signed-off-by: pranalidhanavade <[email protected]> --------- Signed-off-by: pranalidhanavade <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * fix: removed unnecessary code Signed-off-by: bhavanakarwade <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * chore: added orgdid in get all orgs response (#1119) Signed-off-by: bhavanakarwade <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * fix: added API params validations (#1124) * fix: added API params validations Signed-off-by: pranalidhanavade <[email protected]> * fix: removed unneccessary roles from role gaurd Signed-off-by: pranalidhanavade <[email protected]> --------- Signed-off-by: pranalidhanavade <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * fix: added api param validations (#1121) * fix: added api param validations Signed-off-by: bhavanakarwade <[email protected]> * fix: removed unnecessary validations Signed-off-by: bhavanakarwade <[email protected]> --------- Signed-off-by: bhavanakarwade <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * refactor:Conditions to get issued credential list for OOB issuance (#1125) Signed-off-by: pranalidhanavade <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * fix: parameter validations issues (#1126) * fix: added api param validations Signed-off-by: bhavanakarwade <[email protected]> * fix: removed unnecessary validations Signed-off-by: bhavanakarwade <[email protected]> * fix:resolved validations issue Signed-off-by: bhavanakarwade <[email protected]> * added comment on function for understanding Signed-off-by: bhavanakarwade <[email protected]> --------- Signed-off-by: bhavanakarwade <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * added compass.yml file Signed-off-by: pranalidhanavade <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * feat: update schema details and add alias in schema table (#1129) * API for update schema details Signed-off-by: pallavighule <[email protected]> * refactored query Signed-off-by: pallavighule <[email protected]> * chore: added alias in response Signed-off-by: bhavanakarwade <[email protected]> --------- Signed-off-by: pallavighule <[email protected]> Signed-off-by: bhavanakarwade <[email protected]> Co-authored-by: bhavanakarwade <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * chore: remove yarn.lock and package-lock.json files (#1133) Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> Signed-off-by: Sahil Kamble <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * fix: platform agent set up issue (#1136) Signed-off-by: bhavanakarwade <[email protected]> Signed-off-by: Sahil Kamble <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * fix: Parameter validations (#1138) * fix: added api param validations Signed-off-by: bhavanakarwade <[email protected]> * fix: removed unnecessary validations Signed-off-by: bhavanakarwade <[email protected]> * fix:resolved validations issue Signed-off-by: bhavanakarwade <[email protected]> * added comment on function for understanding Signed-off-by: bhavanakarwade <[email protected]> * fix: resolve orgid validations Signed-off-by: bhavanakarwade <[email protected]> * fix: added response message Signed-off-by: bhavanakarwade <[email protected]> * fix: added space in response messages Signed-off-by: bhavanakarwade <[email protected]> --------- Signed-off-by: bhavanakarwade <[email protected]> Signed-off-by: Sahil Kamble <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * added tenant id in get org info (#1139) Signed-off-by: pallavighule <[email protected]> Signed-off-by: Sahil Kamble <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * fix/added-yml-file-to-push-docker-images (#1142) * fix: removed prisma commands in build stage Signed-off-by: [email protected] <[email protected]> * feat: create yml file for all services Signed-off-by: [email protected] <[email protected]> * Updated Dockerfiles Signed-off-by: [email protected] <[email protected]> * Updated yml to push docker images Signed-off-by: [email protected] <[email protected]> * updated yml files Signed-off-by: [email protected] <[email protected]> --------- Signed-off-by: [email protected] <[email protected]> Signed-off-by: Sahil Kamble <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * fix/push docker image (#1154) * fix: used single yml file using matrix instead of multiple files Signed-off-by: [email protected] <[email protected]> * fix: renamed yml file to cicd.yml Signed-off-by: [email protected] <[email protected]> * fix: renamed yml file Signed-off-by: [email protected] <[email protected]> * fix: renamed yml file Signed-off-by: [email protected] <[email protected]> * chore: removed id-token and attestations Signed-off-by: [email protected] <[email protected]> --------- Signed-off-by: [email protected] <[email protected]> Signed-off-by: Sahil Kamble <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * chore: add aws module into monorepo (#1135) * chore: add aws into monorepo Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> * chore: update tsconfig Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> * refactor:tsconfig.build.json file Signed-off-by: Tipu_Singh <[email protected]> * refactor: remove redundant .nvmrc file Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> --------- Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> Signed-off-by: Tipu_Singh <[email protected]> Co-authored-by: Tipu_Singh <[email protected]> Signed-off-by: Sahil Kamble <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * chore: remove unused modules (#1163) Signed-off-by: Sahil Kamble <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * refactor: remove image-service from libs (#1164) Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> Signed-off-by: Sahil Kamble <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * refactor: merge nats-interceptor and responses lib into common (#1165) * refactor: merge nats-interceptor into common Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> * refactor: merge repsonses lib into common Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> * refactor: create common function to handle errors Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> * chore: add todo Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> * fix: remove missed image service Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> * fix: create separate function to handle common error Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> * chore: add comment for the purpose of the functions Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> * chore: update function comments Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> --------- Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> Signed-off-by: Sahil Kamble <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * refactor: create common package in libs (#1167) * refactor: merge nats-interceptor into common Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> * refactor: merge repsonses lib into common Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> * refactor: create common function to handle errors Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> * chore: add todo Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> * refactor: create common package in libs Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> * fix: remove missed image service Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> * refactor: common service Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> --------- Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> Signed-off-by: Sahil Kamble <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * feat: Support nested attributes while creating schema (#1166) * wip: support nested attributes while creating schema Signed-off-by: bhavanakarwade <[email protected]> * wip: aligned issuance functionality with nested attributes structure Signed-off-by: bhavanakarwade <[email protected]> * refactor: modify csv to json function Signed-off-by: bhavanakarwade <[email protected]> * fix: formatting changes Signed-off-by: bhavanakarwade <[email protected]> * fix: resolved sonar cloud issue Signed-off-by: bhavanakarwade <[email protected]> * fix: security hotspot issue Signed-off-by: bhavanakarwade <[email protected]> * feat: added schema builder function Signed-off-by: bhavanakarwade <[email protected]> * fix: resolved issue Signed-off-by: bhavanakarwade <[email protected]> * refactor: modify extract attributes function Signed-off-by: bhavanakarwade <[email protected]> * fix: destructured objects Signed-off-by: bhavanakarwade <[email protected]> * feat: added description property Signed-off-by: bhavanakarwade <[email protected]> --------- Signed-off-by: bhavanakarwade <[email protected]> Signed-off-by: Sahil Kamble <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * refactor: schema data type validations (#1174) * wip: support nested attributes while creating schema Signed-off-by: bhavanakarwade <[email protected]> * wip: aligned issuance functionality with nested attributes structure Signed-off-by: bhavanakarwade <[email protected]> * refactor: modify csv to json function Signed-off-by: bhavanakarwade <[email protected]> * fix: formatting changes Signed-off-by: bhavanakarwade <[email protected]> * fix: resolved sonar cloud issue Signed-off-by: bhavanakarwade <[email protected]> * fix: security hotspot issue Signed-off-by: bhavanakarwade <[email protected]> * feat: added schema builder function Signed-off-by: bhavanakarwade <[email protected]> * fix: resolved issue Signed-off-by: bhavanakarwade <[email protected]> * refactor: modify extract attributes function Signed-off-by: bhavanakarwade <[email protected]> * fix: destructured objects Signed-off-by: bhavanakarwade <[email protected]> * feat: added description property Signed-off-by: bhavanakarwade <[email protected]> * fix: added validations for schema type Signed-off-by: bhavanakarwade <[email protected]> * formatted enum file Signed-off-by: bhavanakarwade <[email protected]> * chore: added enum for indy schema data type Signed-off-by: bhavanakarwade <[email protected]> --------- Signed-off-by: bhavanakarwade <[email protected]> Signed-off-by: Sahil Kamble <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * refactor: update organization API to support updation of country, state and city (#1180) * refactor: update organization API to support updation of country, state and city Signed-off-by: pranalidhanavade <[email protected]> * resolved sonarlint issues Signed-off-by: pranalidhanavade <[email protected]> * resolved sonarlint issues Signed-off-by: pranalidhanavade <[email protected]> --------- Signed-off-by: pranalidhanavade <[email protected]> Signed-off-by: Sahil Kamble <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * refactor: error handling for download csv file (#1182) * wip: support nested attributes while creating schema Signed-off-by: bhavanakarwade <[email protected]> * wip: aligned issuance functionality with nested attributes structure Signed-off-by: bhavanakarwade <[email protected]> * refactor: modify csv to json function Signed-off-by: bhavanakarwade <[email protected]> * fix: formatting changes Signed-off-by: bhavanakarwade <[email protected]> * fix: resolved sonar cloud issue Signed-off-by: bhavanakarwade <[email protected]> * fix: security hotspot issue Signed-off-by: bhavanakarwade <[email protected]> * feat: added schema builder function Signed-off-by: bhavanakarwade <[email protected]> * fix: resolved issue Signed-off-by: bhavanakarwade <[email protected]> * refactor: modify extract attributes function Signed-off-by: bhavanakarwade <[email protected]> * fix: destructured objects Signed-off-by: bhavanakarwade <[email protected]> * feat: added description property Signed-off-by: bhavanakarwade <[email protected]> * fix: added validations for schema type Signed-off-by: bhavanakarwade <[email protected]> * formatted enum file Signed-off-by: bhavanakarwade <[email protected]> * chore: added enum for indy schema data type Signed-off-by: bhavanakarwade <[email protected]> * chore: refactor validations Signed-off-by: bhavanakarwade <[email protected]> --------- Signed-off-by: bhavanakarwade <[email protected]> Signed-off-by: Sahil Kamble <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * feat: implemented get verified presentation counts by issuer id (#1184) Signed-off-by: bhavanakarwade <[email protected]> Signed-off-by: Sahil Kamble <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * fix: local setup issues (#1155) * Update start_agent.sh Signed-off-by: Krishna Waske <[email protected]> * Update .env.demo fix: correct script for local build Signed-off-by: Krishna Waske <[email protected]> --------- Signed-off-by: Krishna Waske <[email protected]> Signed-off-by: Sahil Kamble <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * fix: issues related nested attributes in issuance process (#1194) * fix: issues related nested attributes in issuance process Signed-off-by: bhavanakarwade <[email protected]> * fix: added statuscode Signed-off-by: bhavanakarwade <[email protected]> * fix: required field validation for request id field Signed-off-by: bhavanakarwade <[email protected]> --------- Signed-off-by: bhavanakarwade <[email protected]> Signed-off-by: Sahil Kamble <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * chore: update .env.demo file (#1198) Signed-off-by: Sahil Kamble <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * feat: Validations for organization and template ledgerId in all issuance methods (#1200) * fix: ledgerId validations in issuance process Signed-off-by: bhavanakarwade <[email protected]> * fix: added validations for not found exception Signed-off-by: bhavanakarwade <[email protected]> * chore: added comment Signed-off-by: bhavanakarwade <[email protected]> --------- Signed-off-by: bhavanakarwade <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * feat: added seed dockerfile (#1203) * feat: added seed dockerfile Signed-off-by: Sahil Kamble <[email protected]> * feat: updated seed dockerfile Signed-off-by: Sahil Kamble <[email protected]> --------- Signed-off-by: Sahil Kamble <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * Update nats server and redis configurations (#1206) * Update nats-server.conf Signed-off-by: Krishna Waske <[email protected]> * Create docker-compose.nats.yml Signed-off-by: Krishna Waske <[email protected]> * Create docker-compose.redis.yml Signed-off-by: Krishna Waske <[email protected]> --------- Signed-off-by: Krishna Waske <[email protected]> * Update .env.demo (#1205) * Update .env.demo Signed-off-by: Krishna Waske <[email protected]> * Update .env.demo Signed-off-by: Krishna Waske <[email protected]> --------- Signed-off-by: Krishna Waske <[email protected]> * feat/update-docker-compose (#1209) * feat: add schema and seed service to docker-compose Signed-off-by: Sahil Kamble <[email protected]> * feat: add docker-compose-dev.yml Signed-off-by: Sahil Kamble <[email protected]> --------- Signed-off-by: Sahil Kamble <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * chore: removed unwanted values (#1212) Signed-off-by: Sahil Kamble <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * feat: created yml file for tag v2.0.0 Signed-off-by: [email protected] <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * fix: added organization service in yml file Signed-off-by: [email protected] <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * feat: create yml file to push docker images Signed-off-by: [email protected] <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * Create FEATURE-REQUEST.md (#1181) * Create FEATURE-REQUEST.md Create FEATURE-REQUEST.md Signed-off-by: Krishna Waske <[email protected]> * Update bug_report.md Update bug_report.md Signed-off-by: Krishna Waske <[email protected]> * Update FEATURE-REQUEST.md change links Signed-off-by: Krishna Waske <[email protected]> * Update bug_report.md Signed-off-by: Krishna Waske <[email protected]> --------- Signed-off-by: Krishna Waske <[email protected]> * fix readme typo (#1170) fix readme typo Signed-off-by: NucleonGodX <[email protected]> Signed-off-by: Krishna Waske <[email protected]> * fix: Update GET proof record endpoint to return 404 for non-existing records (#1190) Signed-off-by: Amr Mubarak <[email protected]> Signed-off-by: Krishna Waske <[email protected]> --------- Signed-off-by: pranalidhanavade <[email protected]> Signed-off-by: Krishna Waske <[email protected]> Signed-off-by: [email protected] <[email protected]> Signed-off-by: pallavicoder <[email protected]> Signed-off-by: bhavanakarwade <[email protected]> Signed-off-by: Tipu_Singh <[email protected]> Signed-off-by: pallavighule <[email protected]> Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> Signed-off-by: Sahil Kamble <[email protected]> Signed-off-by: NucleonGodX <[email protected]> Signed-off-by: Amr Mubarak <[email protected]> Co-authored-by: pranalidhanavade <[email protected]> Co-authored-by: [email protected] <[email protected]> Co-authored-by: pallavighule <[email protected]> Co-authored-by: bhavanakarwade <[email protected]> Co-authored-by: pallavicoder <[email protected]> Co-authored-by: Tipu_Singh <[email protected]> Co-authored-by: Sai Ranjit Tummalapalli <[email protected]> Co-authored-by: Manit Singh <[email protected]> Co-authored-by: Amr Mubarak <[email protected]> * feat: created yml file for tag v2.0.0 Signed-off-by: [email protected] <[email protected]> * fix: added organization service in yml file Signed-off-by: [email protected] <[email protected]> * feat: create yml file to push docker images Signed-off-by: [email protected] <[email protected]> * Update and rename FEATURE-REQUEST.md to FEATURE-REQUEST.yml update template from .md to .yml Signed-off-by: Krishna Waske <[email protected]> * Update FEATURE-REQUEST.yml Signed-off-by: Krishna Waske <[email protected]> * Update and rename bug_report.md to bug_report.yml Signed-off-by: Krishna Waske <[email protected]> * Update FEATURE-REQUEST.yml Signed-off-by: Krishna Waske <[email protected]> * Update bug_report.yml Signed-off-by: Krishna Waske <[email protected]> * Update bug_report.yml Signed-off-by: Krishna Waske <[email protected]> * Update bug_report.yml Signed-off-by: Krishna Waske <[email protected]> * Update bug_report.yml Signed-off-by: Krishna Waske <[email protected]> * Update FEATURE-REQUEST.yml Signed-off-by: Krishna Waske <[email protected]> --------- Signed-off-by: pranalidhanavade <[email protected]> Signed-off-by: Krishna Waske <[email protected]> Signed-off-by: [email protected] <[email protected]> Signed-off-by: pallavicoder <[email protected]> Signed-off-by: bhavanakarwade <[email protected]> Signed-off-by: Tipu_Singh <[email protected]> Signed-off-by: pallavighule <[email protected]> Signed-off-by: Sai Ranjit Tummalapalli <[email protected]> Signed-off-by: Sahil Kamble <[email protected]> Signed-off-by: NucleonGodX <[email protected]> Signed-off-by: Amr Mubarak <[email protected]> Co-authored-by: pranalidhanavade <[email protected]> Co-authored-by: [email protected] <[email protected]> Co-authored-by: pallavighule <[email protected]> Co-authored-by: bhavanakarwade <[email protected]> Co-authored-by: pallavicoder <[email protected]> Co-authored-by: Tipu_Singh <[email protected]> Co-authored-by: Sai Ranjit Tummalapalli <[email protected]> Co-authored-by: Manit Singh <[email protected]> Co-authored-by: Amr Mubarak <[email protected]>
* chore: remove unwanted values from .env.demo Signed-off-by: Sahil Kamble <[email protected]> * chore: remove unwanted values from agent.env Signed-off-by: Sahil Kamble <[email protected]> --------- Signed-off-by: Sahil Kamble <[email protected]>
Signed-off-by: RinkalBhojani <[email protected]>
fix: removed unwanted log which was displaying large data
update: modify get organization query
Signed-off-by: bhavanakarwade <[email protected]>
Signed-off-by: bhavanakarwade <[email protected]>
…at/single-sign-on-feature
WalkthroughThis update introduces Single Sign-On (SSO) client alias support across the system. It adds new environment variable templates and database schema for managing client aliases and URLs, implements backend endpoints for retrieving and validating client aliases, and refactors the user verification email flow to use client aliases instead of direct credentials. Additional formatting, import organization, and error handling improvements are included. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API-Gateway
participant AuthzService
participant UserService
participant Utils
participant DB
Client->>API-Gateway: POST /authz/verification-email?clientAlias=ALIAS
API-Gateway->>AuthzService: sendVerificationMail(userEmailVerification, clientAlias)
AuthzService->>UserService: sendVerificationMail(userEmailVerification, clientAlias)
UserService->>Utils: getCredentialsByAlias(clientAlias)
Utils->>DB: Fetch client credentials for alias
DB-->>Utils: Return clientId, clientSecret, domain
Utils-->>UserService: Return encrypted credentials
UserService->>UserService: Compose verification email with clientAlias
UserService->>DB: Store verification email request
UserService-->>AuthzService: Return user entity
AuthzService-->>API-Gateway: Return user entity
API-Gateway-->>Client: Respond with status/message
sequenceDiagram
participant Client
participant API-Gateway
participant AuthzService
participant DB
Client->>API-Gateway: GET /authz/clientAliases
API-Gateway->>AuthzService: getClientAlias()
AuthzService->>DB: Fetch all client aliases and URLs
DB-->>AuthzService: Return alias list
AuthzService-->>API-Gateway: Return alias list
API-Gateway-->>Client: Respond with aliases and URLs
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Signed-off-by: bhavanakarwade <[email protected]>
Signed-off-by: bhavanakarwade <[email protected]>
|
@CodeRabbit full review |
✅ Actions performedReview triggered.
|
Docstrings generation was requested by @GHkrishna. * #1183 (comment) The following files were modified: * `apps/api-gateway/src/main.ts` * `apps/api-gateway/src/user/utils/index.ts` * `libs/common/src/cast.helper.ts` * `libs/prisma-service/prisma/seed.ts`
Note Generated docstrings for this pull request at #1304 |
✅ Actions performedFull review triggered. |
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
♻️ Duplicate comments (1)
libs/prisma-service/prisma/seed.ts (1)
404-406
: Address shell command security vulnerabilities.The shell commands constructed from environment variables pose security risks as flagged by static analysis and previous reviews. Consider using more secure approaches.
Based on the learned preference for utility functions in this codebase, consider creating a utility function to validate and sanitize environment variables before shell execution:
+ import { validateEnvironmentPath } from '@credebl/common/env-validator'; - const command = `${process.cwd()}/${scriptPath} ${dbUrl}`; + const validatedScriptPath = validateEnvironmentPath(scriptPath); + const command = `${process.cwd()}/${validatedScriptPath} ${dbUrl}`;And similarly for the
updateClientCredential
function.Also applies to: 436-438
🧹 Nitpick comments (12)
apps/agent-service/src/repositories/agent-service.repository.ts (1)
1-1
: Avoid a file-widecamelcase
opt-out – scope the rule instead
Disabling the rule for the whole file makes it easy for unrelated violations to creep in. If the underscore-based Prisma model names are the only offenders, prefer a single-line directive on the relevant import or rename the symbols locally:-/* eslint-disable camelcase */ +// eslint-disable-next-line camelcase +import { + Prisma, + ledgerConfig as LedgerConfigModel, + ... +} from '@prisma/client';That keeps the remainder of the file protected by the linter.
.github/ISSUE_TEMPLATE/FEATURE-REQUEST.yml (1)
73-73
: Add missing newline character at end of file.YAML files should end with a newline character according to POSIX standards and YAMLlint rules.
Apply this diff to fix the formatting issue:
- required: false + required: false +docker-compose.yml (1)
169-169
: Remove trailing space.There's a trailing space after
api-gateway
that should be removed for consistency.Apply this diff to fix the formatting issue:
- - api-gateway + - api-gateway.env.sample (2)
162-162
: Fix typos in domain URL examples.There are typos in the domain URL examples - "htts" should be "https".
Apply this diff to fix the typos:
-# CLIENT-NAME_DOMAIN=htts://myapp.com +# CLIENT-NAME_DOMAIN=https://myapp.com-# VERIFIER_DOMAIN=htts://VERIFIER-domain.com +# VERIFIER_DOMAIN=https://VERIFIER-domain.comAlso applies to: 169-169
165-165
: Fix incomplete environment variable name.The environment variable name is incomplete - missing the suffix.
Apply this diff to fix the variable name:
-# CLIENT-NAME_KEYCLOAK_MANAGEMENT_CLIENT_SECRET +# CLIENT-NAME_KEYCLOAK_MANAGEMENT_CLIENT_SECRET=.env.demo (1)
106-127
: LGTM with minor improvements needed.The SSO client configuration structure is well-designed and supports the multi-client feature effectively. However, there are a couple of issues to address:
- Fix typo in sample domain URL: Line 117 and 124 have "htts" instead of "https"
- Clarify encryption method: The comments mention "encrypted client credentials using the
CRYPTO_PRIVATE_KEY
" but don't specify the encryption algorithm or formatApply this diff to fix the typo:
-# CLIENT-NAME_DOMAIN=htts://VERIFIER-domain.com +# CLIENT-NAME_DOMAIN=https://VERIFIER-domain.com-# VERIFIER_DOMAIN=htts://VERIFIER-domain.com +# VERIFIER_DOMAIN=https://VERIFIER-domain.comConsider adding a comment explaining the encryption method used for client credentials to help with setup and troubleshooting.
.github/ISSUE_TEMPLATE/bug_report.yml (1)
90-90
: Add missing newline at end of file.The static analysis tool correctly identified that this file is missing a newline character at the end, which violates YAML formatting conventions.
Apply this diff to add the missing newline:
validations: - required: false + required: false +apps/api-gateway/src/authz/decorators/user-auth-client.ts (1)
1-26
: Well-implemented validation pipe with a minor enhancement suggestion.The pipe correctly implements client alias validation with proper error handling and case-insensitive comparison. The logic for allowing optional empty values is appropriate.
Consider adding validation for the environment variable configuration to provide better error messaging:
constructor() { + if (!process.env.SUPPORTED_SSO_CLIENTS) { + throw new Error('SUPPORTED_SSO_CLIENTS environment variable is not configured'); + } this.allowedAliases = (process.env.SUPPORTED_SSO_CLIENTS || '') .split(',') .map((alias) => alias.trim()) .filter(Boolean); + + if (this.allowedAliases.length === 0) { + throw new Error('SUPPORTED_SSO_CLIENTS contains no valid aliases'); + } }This would help catch configuration issues early during application startup rather than at runtime validation.
apps/api-gateway/src/authz/authz.service.ts (1)
4-4
: Question: WebSocket imports seem unrelated to SSO feature.The WebSocket imports appear unrelated to the client alias functionality being introduced. Are these imports necessary for this feature, or are they part of a different change that was accidentally included?
apps/api-gateway/src/user/utils/index.ts (1)
11-31
: Consider adding validation for required environment variables.The function constructs client details from environment variables but doesn't validate their existence. Consider adding validation to ensure required environment variables are set before returning the client details.
function getClientDetails(alias: string): IClientDetailsSSO { const clientIdKey = `${alias}_KEYCLOAK_MANAGEMENT_CLIENT_ID`; const clientSecretKey = `${alias}_KEYCLOAK_MANAGEMENT_CLIENT_SECRET`; const domainKey = `${alias}_DOMAIN`; const aliasNameKey = `${alias}_ALIAS`; const clientId = process.env[clientIdKey]; const clientSecret = process.env[clientSecretKey]; const domain = process.env[domainKey]; const aliasName = process.env[aliasNameKey] || alias; + // Early validation could be added here + if (!clientId || !clientSecret || !domain) { + throw new Error(`Missing environment variables for SSO client: ${alias}`); + } const clientDetails: IClientDetailsSSO = { clientId, clientSecret, domain, alias: aliasName }; return clientDetails; }apps/api-gateway/src/authz/authz.controller.ts (1)
46-47
: Fix typos in JSDoc comments.There are grammatical errors in the documentation:
- Line 46: "aliase" should be "aliases"
- Line 54: "itr" should be "its"
- * Fetch client aliase. + * Fetch client aliases.- description: 'Fetch client aliases and itr url' + description: 'Fetch client aliases and its url'libs/prisma-service/prisma/seed.ts (1)
377-382
: Fix ESLint linebreak error.The static analysis flagged an implicit-arrow-linebreak error on line 378.
- const updatePromises = emptyTypeSchemaList.map((schema) => - prisma.schema.update({ + const updatePromises = emptyTypeSchemaList.map((schema) => prisma.schema.update({
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (29)
.env.demo
(1 hunks).env.sample
(1 hunks).github/ISSUE_TEMPLATE/FEATURE-REQUEST.yml
(1 hunks).github/ISSUE_TEMPLATE/bug_report.yml
(1 hunks)apps/agent-service/src/repositories/agent-service.repository.ts
(2 hunks)apps/api-gateway/src/authz/authz.controller.ts
(6 hunks)apps/api-gateway/src/authz/authz.service.ts
(4 hunks)apps/api-gateway/src/authz/decorators/user-auth-client.ts
(1 hunks)apps/api-gateway/src/main.ts
(1 hunks)apps/api-gateway/src/user/dto/create-user.dto.ts
(1 hunks)apps/api-gateway/src/user/utils/index.ts
(1 hunks)apps/issuance/src/issuance.repository.ts
(1 hunks)apps/issuance/src/issuance.service.ts
(8 hunks)apps/ledger/src/schema/schema.service.ts
(0 hunks)apps/organization/repositories/organization.repository.ts
(1 hunks)apps/user/repositories/user.repository.ts
(6 hunks)apps/user/src/user.controller.ts
(6 hunks)apps/user/src/user.service.ts
(29 hunks)apps/user/templates/user-email-template.ts
(2 hunks)apps/verification/src/verification.service.ts
(7 hunks)docker-compose.yml
(2 hunks)libs/common/src/cast.helper.ts
(3 hunks)libs/common/src/common.module.ts
(1 hunks)libs/common/src/interfaces/user.interface.ts
(1 hunks)libs/common/src/response-messages/index.ts
(1 hunks)libs/logger/src/logger.service.ts
(1 hunks)libs/prisma-service/prisma/migrations/20250603101813_added_client_alias/migration.sql
(1 hunks)libs/prisma-service/prisma/schema.prisma
(1 hunks)libs/prisma-service/prisma/seed.ts
(1 hunks)
💤 Files with no reviewable changes (1)
- apps/ledger/src/schema/schema.service.ts
🧰 Additional context used
🧠 Learnings (2)
.github/ISSUE_TEMPLATE/bug_report.yml (1)
Learnt from: tipusinghaw
PR: credebl/platform#1297
File: apps/api-gateway/src/tracer.ts:24-28
Timestamp: 2025-06-24T11:15:29.468Z
Learning: In the credebl/platform repository, the team prefers to create utilities for validating required environment variables rather than implementing inline validation. This approach promotes reusability across different parts of the application.
libs/prisma-service/prisma/seed.ts (1)
Learnt from: tipusinghaw
PR: credebl/platform#1297
File: apps/api-gateway/src/tracer.ts:24-28
Timestamp: 2025-06-24T11:15:29.468Z
Learning: In the credebl/platform repository, the team prefers to create utilities for validating required environment variables rather than implementing inline validation. This approach promotes reusability across different parts of the application.
🧬 Code Graph Analysis (8)
apps/api-gateway/src/user/dto/create-user.dto.ts (2)
libs/common/src/cast.helper.ts (2)
trim
(40-44)toLowerCase
(36-38)libs/validations/maxLength.ts (1)
MaxLength
(3-28)
apps/user/repositories/user.repository.ts (1)
apps/user/interfaces/user.interface.ts (1)
IUserDeletedActivity
(210-218)
libs/common/src/common.module.ts (1)
libs/logger/src/logger.interface.ts (1)
Logger
(6-20)
apps/api-gateway/src/user/utils/index.ts (2)
libs/common/src/interfaces/user.interface.ts (1)
IClientDetailsSSO
(24-29)libs/common/src/cast.helper.ts (1)
encryptClientCredential
(443-457)
apps/api-gateway/src/authz/authz.service.ts (3)
libs/common/src/interfaces/user.interface.ts (2)
IClientAliases
(64-70)IResetPasswordResponse
(55-58)apps/api-gateway/src/user/dto/create-user.dto.ts (1)
UserEmailVerificationDto
(7-37)apps/api-gateway/src/authz/dtos/reset-password.dto.ts (1)
ResetPasswordDto
(7-25)
libs/common/src/interfaces/user.interface.ts (1)
apps/user/interfaces/user.interface.ts (4)
IVerifyUserEmail
(173-176)ISendVerificationEmail
(51-56)IOrgRole
(22-26)IOrganisation
(27-38)
apps/issuance/src/issuance.service.ts (1)
apps/issuance/interfaces/issuance.interfaces.ts (1)
SchemaDetails
(190-196)
apps/api-gateway/src/authz/authz.controller.ts (4)
libs/common/src/interfaces/response.interface.ts (1)
IResponseType
(1-7)libs/common/src/response-messages/index.ts (1)
ResponseMessages
(1-495)apps/api-gateway/src/user/dto/create-user.dto.ts (1)
UserEmailVerificationDto
(7-37)apps/api-gateway/src/user/utils/index.ts (1)
getDefaultClient
(4-9)
🪛 YAMLlint (1.37.1)
.github/ISSUE_TEMPLATE/bug_report.yml
[error] 90-90: no new line character at the end of file
(new-line-at-end-of-file)
docker-compose.yml
[error] 169-169: trailing spaces
(trailing-spaces)
.github/ISSUE_TEMPLATE/FEATURE-REQUEST.yml
[error] 73-73: no new line character at the end of file
(new-line-at-end-of-file)
🪛 GitHub Check: CodeQL
libs/prisma-service/prisma/seed.ts
[warning] 406-406: Shell command built from environment values
This shell command depends on an uncontrolled absolute path.
[warning] 438-438: Shell command built from environment values
This shell command depends on an uncontrolled absolute path.
🪛 ESLint
libs/prisma-service/prisma/seed.ts
[error] 378-378: Expected no linebreak before this expression.
(implicit-arrow-linebreak)
🔇 Additional comments (39)
apps/agent-service/src/repositories/agent-service.repository.ts (1)
28-28
: 👍 Re-ordering imports improves readability
MovingPrismaService
below the grouped external/type imports follows a consistent top-down dependency order and is a welcome cleanup.docker-compose.yml (1)
64-64
: Good addition of Redis dependency for issuance service.Adding Redis as a dependency for the issuance service aligns well with SSO support, as it can be used for caching authentication tokens and session data.
libs/logger/src/logger.service.ts (1)
92-99
: LGTM! Improved formatting of nested ternary operator.The reformatted ternary conditional expression is more readable while maintaining the same logic for error handling.
libs/common/src/cast.helper.ts (2)
1-3
: Good addition of crypto imports and logger instance.The imports for crypto-js and Logger are appropriate for the new encryption functionality.
Also applies to: 28-28
443-457
: Confirmed:CRYPTO_PRIVATE_KEY
is documented and decryption uses the same keyEverything checks out:
- Documentation
.env.sample
(line 87) and.env.demo
(line 39) includeCRYPTO_PRIVATE_KEY
.- Encryption/Decryption consistency
encryptClientCredential
inlibs/common/src/cast.helper.ts
usesprocess.env.CRYPTO_PRIVATE_KEY
.- AES decryption in
libs/common/src/common.service.ts
(CryptoJS.AES.decrypt(..., process.env.CRYPTO_PRIVATE_KEY)
) matches the same key.- Other encrypt calls in
common.service.ts
andapps/agent-service/src/agent-service.service.ts
also use the identical env var.All functions share the same key, and it’s properly surfaced in the sample env files. No further action required.
.env.sample (1)
151-158
: Excellent SSO configuration structure.The new SSO configuration section is well-designed with clear documentation, consistent naming conventions, and a scalable template approach for multiple clients.
apps/organization/repositories/organization.repository.ts (1)
685-687
: LGTM! Good addition for multi-tenant SSO support.Adding
tenantId
to the org_agents selection enhances the data model to support tenant-aware client alias functionality, which aligns perfectly with the SSO feature requirements.libs/common/src/common.module.ts (2)
2-3
: LGTM! Proper logging integration.The import statements correctly bring in the necessary logging dependencies from both NestJS core and the custom logger module.
8-9
: LGTM! Correct module configuration.The LoggerModule is properly added to imports and Logger is correctly included in the providers array, following NestJS best practices for dependency injection.
apps/issuance/src/issuance.repository.ts (1)
684-686
: LGTM - Good formatting improvement.The multiline template literal improves readability of the error message construction without changing the functionality.
apps/api-gateway/src/main.ts (1)
7-7
: LGTM - Import organization improvement.The reordering of imports improves code organization and readability without affecting functionality.
Also applies to: 10-11, 14-14, 16-18
libs/prisma-service/prisma/schema.prisma (2)
522-523
: LGTM - Proper enum closure.The CloudWalletType enum is now properly closed with the closing brace.
524-530
: LGTM - Well-structured client aliases model.The new
client_aliases
model follows established patterns with:
- Standard UUID primary key
- Proper timestamp fields with defaults
- Appropriate nullable/required field constraints
- Clear naming conventions
This properly supports the SSO client alias functionality.
libs/prisma-service/prisma/migrations/20250603101813_added_client_alias/migration.sql (1)
1-10
: LGTM - Correct migration implementation.The migration script properly creates the
client_aliases
table with:
- Appropriate column types and constraints
- Proper timestamp defaults
- Correct nullability settings
- Explicit primary key constraint
This correctly implements the schema changes defined in the Prisma model.
apps/issuance/src/issuance.service.ts (3)
153-155
: LGTM! Clean formatting improvement.The reformatting of multi-line
await
calls to single-line with parentheses improves code readability while maintaining the same functionality.Also applies to: 335-337, 796-798
484-487
: Good readability improvement for long error messages.Breaking the logger error message into multiple lines enhances readability for debugging purposes.
1159-1161
: Excellent formatting of ternary expressions.The consistent indentation alignment of the ternary operators improves code readability while preserving the conditional logic.
Also applies to: 1169-1171, 1179-1181, 1189-1192
apps/user/repositories/user.repository.ts (2)
6-16
: Good import organization improvement.The reorganization and grouping of import statements enhances code readability and maintainability.
Also applies to: 19-19, 21-21
719-726
: Clean formatting improvements.The parameter formatting and method structure improvements enhance code readability without changing functionality.
Also applies to: 780-782, 785-785, 827-841
apps/verification/src/verification.service.ts (1)
246-248
: LGTM: Formatting improvements enhance readability.The indentation and formatting improvements make the code more readable without affecting functionality.
Also applies to: 810-871
apps/user/templates/user-email-template.ts (4)
2-11
: LGTM: Well-designed parameter additions maintain backward compatibility.The addition of optional
redirectTo
andclientAlias
parameters is well-structured and maintains backward compatibility with existing callers.
17-23
: LGTM: Clean query parameter handling.The conditional appending of
redirectTo
andclientAlias
query parameters is implemented correctly and follows good practices.
80-82
: Improved error handling with descriptive message.The error handling improvement provides a more descriptive error message, which is better for debugging and user experience.
12-12
: Confirm the/verify-email-success
route is implemented and correct
Please ensure that simplifying the URL construction to always use/verify-email-success
whenclientId
is truthy matches your routing setup and doesn’t break existing email verification links.• Verify in your routing config or pages directory that a handler/page for
/verify-email-success
exists
• Test the generated link (with and without aclientId
) to confirm it resolves correctly
• Check any redirects or middleware that might alter this path after template dispatchapps/api-gateway/src/authz/authz.service.ts (3)
8-14
: LGTM: Well-organized import consolidation.The import consolidation and addition of
IClientAliases
and Prismauser
types properly support the new SSO client alias functionality.Also applies to: 19-19
33-35
: LGTM: Clean implementation of client alias retrieval.The
getClientAlias()
method is well-implemented using the existing NATS pattern for service communication.
42-42
: No breaking changes detected for sendVerificationMail
All usages ofsendVerificationMail
either ignore the returneduser
or already expectPromise<user>
:
- apps/api-gateway/src/authz/authz.controller.ts – the controller calls and discards the result
- apps/user/src/user.controller.ts & user.service.ts – both are typed to return
Promise<user>
No callers need updates.
apps/api-gateway/src/user/utils/index.ts (2)
4-9
: LGTM: Well-structured default client retrieval with encryption.The function properly encrypts client credentials using the established encryption helper. The use of environment variables for configuration is appropriate.
33-46
: LGTM: Good validation and error handling.The function properly validates required fields and provides clear error messages. The logic for handling the default client case is well-implemented.
libs/common/src/interfaces/user.interface.ts (3)
24-29
: LGTM: Well-designed SSO client details interface.The
IClientDetailsSSO
interface is well-structured and provides all necessary fields for SSO client management with clear typing.
64-70
: LGTM: Clean client aliases interface design.The
IClientAliases
interface properly models client alias records with appropriate fields including timestamps and identifiers.
13-22
: clientAlias field verified across local consumers – safe to merge• apps/user/src/user.service.ts: destructures and uses
clientAlias
• apps/user/src/user.controller.ts: acceptsISendVerificationEmail
payload withclientAlias
• apps/user/repositories/user.repository.ts: imports and forwardsISendVerificationEmail
No other missing occurrences of this interface were found in the repo.
Note: External producers of thesend-verification-mail
message must include the newclientAlias
property to avoid runtime errors.apps/api-gateway/src/authz/authz.controller.ts (1)
101-105
: Add documentation and validation for SUPPORTED_SSO_CLIENTSI didn’t find any references to
SUPPORTED_SSO_CLIENTS
in your environment sample files or docs. Please ensure that:
- You include
SUPPORTED_SSO_CLIENTS
in your.env.example
(or equivalent) with a clear format (comma-separated, uppercase aliases).- Your deployment guide or README (e.g.
docs/DEPLOYMENT.md
,README.md
) explains:
- What this variable controls
- The permissible values
- Any defaults or fallbacks
- At startup, missing or malformed values trigger a clear validation error rather than silently resulting in an empty enum.
apps/user/src/user.controller.ts (3)
1-25
: Good: Import organization improved.The imports are now better organized with clear separation between local interfaces, common interfaces, and Prisma types. This improves code readability and maintainability.
43-47
: LGTM: Client aliases endpoint implementation.The new
getClientAliases()
method follows the established pattern for message-based communication and properly delegates to the service layer.
55-55
: sendVerificationMail return type consistency verifiedI’ve checked all relevant layers:
- apps/user/src/user.controller.ts → Promise
- apps/user/src/user.service.ts → Promise
- apps/api-gateway/src/authz/authz.service.ts → Promise
- apps/api-gateway/src/authz/authz.controller.ts consumes the returned
user
and wraps it in anIResponseType
No mismatches found—everything is aligned with the new
Promise<user>
signature.libs/prisma-service/prisma/seed.ts (1)
8-8
: LGTM: Encryption helper import.Good refactoring to use the centralized
encryptClientCredential
helper instead of local encryption logic. This promotes code reuse and consistency.apps/user/src/user.service.ts (2)
93-101
: LGTM: Client aliases retrieval method.The implementation follows established patterns with proper error handling and logging. The method correctly delegates to the repository layer.
148-157
: Good: Enhanced email verification with client context.The updated
sendEmailForVerification
method now properly passes client domain and alias information to the email template, enabling better client-specific email customization.
import { | ||
Prisma, | ||
ledgerConfig, | ||
ledgers, | ||
org_agents, | ||
org_agents_type, | ||
org_dids, | ||
organisation, | ||
platform_config, | ||
user | ||
} from '@prisma/client'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Mark Prisma model imports as type-only to prevent accidental runtime usage
ledgerConfig
, organisation
, etc. are compile-time types – they are not needed at runtime and importing them as values slightly increases the bundle size and can mislead readers.
-import {
- Prisma,
- ledgerConfig,
- ledgers,
- org_agents,
- org_agents_type,
- org_dids,
- organisation,
- platform_config,
- user
-} from '@prisma/client';
+import type {
+ Prisma,
+ ledgerConfig,
+ ledgers,
+ org_agents,
+ org_agents_type,
+ org_dids,
+ organisation,
+ platform_config,
+ user
+} from '@prisma/client';
Using import type
(TS 3.8+) keeps the import purely in the type space and communicates intent more clearly.
🤖 Prompt for AI Agents
In apps/agent-service/src/repositories/agent-service.repository.ts around lines
16 to 26, the Prisma model imports like ledgerConfig and organisation are used
only as types and should be imported with 'import type' to avoid including them
in the runtime bundle. Change the import statement to use 'import type' for
these Prisma models to ensure they are treated purely as types, reducing bundle
size and clarifying intent.
create: 'User registered successfully', | ||
update: 'User details updated successfully', | ||
emaiVerified: 'Email verified successfully', | ||
fetchClientAlises: 'Client aliases fetched successfully', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix typo in response message key.
The response message key has a typo: "fetchClientAlises" should be "fetchClientAliases".
Apply this diff to fix the typo:
- fetchClientAlises: 'Client aliases fetched successfully',
+ fetchClientAliases: 'Client aliases fetched successfully',
This typo could cause runtime errors if the key is referenced elsewhere in the codebase.
📝 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.
fetchClientAlises: 'Client aliases fetched successfully', | |
fetchClientAliases: 'Client aliases fetched successfully', |
🤖 Prompt for AI Agents
In libs/common/src/response-messages/index.ts at line 7, there is a typo in the
response message key "fetchClientAlises". Rename this key to
"fetchClientAliases" to correct the spelling and prevent potential runtime
errors when this key is accessed elsewhere in the codebase.
@IsString({ message: 'platformName should be string' }) | ||
platformName?: string; | ||
|
||
clientAlias?: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add validation and documentation decorators for consistency.
The clientAlias
property lacks the validation and documentation decorators that other properties have. This creates inconsistency and potential security issues.
Apply this diff to add proper decorators:
+ @ApiPropertyOptional({ example: 'clientAlias1' })
+ @Transform(({ value }) => trim(value))
+ @IsOptional()
+ @IsString({ message: 'clientAlias should be string' })
clientAlias?: string;
This ensures:
- Proper API documentation
- Input validation and sanitization
- Consistency with other DTO properties
📝 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.
clientAlias?: string; | |
@ApiPropertyOptional({ example: 'clientAlias1' }) | |
@Transform(({ value }) => trim(value)) | |
@IsOptional() | |
@IsString({ message: 'clientAlias should be string' }) | |
clientAlias?: string; |
🤖 Prompt for AI Agents
In apps/api-gateway/src/user/dto/create-user.dto.ts at line 36, the clientAlias
property is missing validation and documentation decorators. Add appropriate
decorators such as @ApiProperty or @ApiPropertyOptional for API documentation
and class-validator decorators like @IsString and @IsOptional to ensure input
validation and consistency with other DTO properties.
async fetchClientAliases(): Promise<client_aliases[]> { | ||
try { | ||
return this.prisma.client_aliases.findMany(); | ||
} catch (error) { | ||
this.logger.error(`checkUserExist: ${JSON.stringify(error)}`); | ||
throw new error(); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix error handling in fetchClientAliases method.
The method has two issues that need to be addressed:
- Incorrect error message references the wrong method name
- Invalid error throwing syntax
Apply this diff to fix the error handling:
async fetchClientAliases(): Promise<client_aliases[]> {
try {
return this.prisma.client_aliases.findMany();
} catch (error) {
- this.logger.error(`checkUserExist: ${JSON.stringify(error)}`);
- throw new error();
+ this.logger.error(`fetchClientAliases: ${JSON.stringify(error)}`);
+ throw error;
}
}
🤖 Prompt for AI Agents
In apps/user/repositories/user.repository.ts lines 44 to 51, the
fetchClientAliases method has incorrect error handling: the error log message
references the wrong method name and the syntax for throwing the error is
invalid. Update the logger.error call to reference fetchClientAliases instead of
checkUserExist, and change the throw statement to rethrow the caught error
correctly by using 'throw error;' instead of 'throw new error();'.
export async function getCredentialsByAlias(alias: string): Promise<IClientDetailsSSO> { | ||
const defaultClient = await getDefaultClient(); | ||
if (alias.toUpperCase() === defaultClient.alias) { | ||
return defaultClient; | ||
} | ||
|
||
const clientDetails = getClientDetails(alias); | ||
|
||
if (!clientDetails.clientId || !clientDetails.clientSecret || !clientDetails.domain) { | ||
throw new Error(`Missing configuration for SSO client: ${alias}`); | ||
} | ||
|
||
return clientDetails; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider encrypting credentials in getCredentialsByAlias.
The getDefaultClient()
function encrypts credentials, but getCredentialsByAlias()
returns raw credentials from getClientDetails()
. For consistency and security, consider encrypting these credentials as well.
export async function getCredentialsByAlias(alias: string): Promise<IClientDetailsSSO> {
const defaultClient = await getDefaultClient();
if (alias.toUpperCase() === defaultClient.alias) {
return defaultClient;
}
const clientDetails = getClientDetails(alias);
if (!clientDetails.clientId || !clientDetails.clientSecret || !clientDetails.domain) {
throw new Error(`Missing configuration for SSO client: ${alias}`);
}
+ // Encrypt credentials for consistency with default client
+ clientDetails.clientId = await encryptClientCredential(clientDetails.clientId);
+ clientDetails.clientSecret = await encryptClientCredential(clientDetails.clientSecret);
return clientDetails;
}
📝 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.
export async function getCredentialsByAlias(alias: string): Promise<IClientDetailsSSO> { | |
const defaultClient = await getDefaultClient(); | |
if (alias.toUpperCase() === defaultClient.alias) { | |
return defaultClient; | |
} | |
const clientDetails = getClientDetails(alias); | |
if (!clientDetails.clientId || !clientDetails.clientSecret || !clientDetails.domain) { | |
throw new Error(`Missing configuration for SSO client: ${alias}`); | |
} | |
return clientDetails; | |
} | |
export async function getCredentialsByAlias(alias: string): Promise<IClientDetailsSSO> { | |
const defaultClient = await getDefaultClient(); | |
if (alias.toUpperCase() === defaultClient.alias) { | |
return defaultClient; | |
} | |
const clientDetails = getClientDetails(alias); | |
if (!clientDetails.clientId || !clientDetails.clientSecret || !clientDetails.domain) { | |
throw new Error(`Missing configuration for SSO client: ${alias}`); | |
} | |
// Encrypt credentials for consistency with default client | |
clientDetails.clientId = await encryptClientCredential(clientDetails.clientId); | |
clientDetails.clientSecret = await encryptClientCredential(clientDetails.clientSecret); | |
return clientDetails; | |
} |
🤖 Prompt for AI Agents
In apps/api-gateway/src/user/utils/index.ts around lines 33 to 46, the function
getCredentialsByAlias returns raw credentials from getClientDetails without
encryption, unlike getDefaultClient which returns encrypted credentials. To fix
this, modify getCredentialsByAlias to encrypt the clientId, clientSecret, and
domain values before returning them, ensuring consistency and security in
credential handling.
@Body() userEmailVerification: UserEmailVerificationDto, | ||
@Res() res: Response | ||
): Promise<Response> { | ||
userEmailVerification.clientAlias = clientAlias ?? (await getDefaultClient()).alias; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Ensure robust fallback for client alias assignment.
The fallback to getDefaultClient().alias
uses async operation but there's potential for this to fail silently. Consider adding error handling for when the default client cannot be retrieved.
- userEmailVerification.clientAlias = clientAlias ?? (await getDefaultClient()).alias;
+ try {
+ userEmailVerification.clientAlias = clientAlias ?? (await getDefaultClient()).alias;
+ } catch (error) {
+ this.logger.error('Failed to get default client alias:', error);
+ throw new InternalServerErrorException('Unable to determine client configuration');
+ }
🤖 Prompt for AI Agents
In apps/api-gateway/src/authz/authz.controller.ts at line 112, the assignment of
clientAlias falls back to an async call getDefaultClient().alias without error
handling, which may fail silently. Modify the code to wrap the await
getDefaultClient() call in a try-catch block, and handle errors appropriately,
such as logging the error and providing a sensible default or rethrowing the
error to prevent silent failures.
let keycloakDetails = null; | ||
|
||
const token = await this.clientRegistrationService.getManagementToken( | ||
checkUserDetails.clientId, | ||
checkUserDetails.clientSecret | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Ensure consistent client credential usage.
The service uses checkUserDetails.clientId
and checkUserDetails.clientSecret
in multiple places. Verify these are consistently encrypted/decrypted and that the client alias system properly populates these fields.
Also applies to: 324-329, 339-343
🏁 Script executed:
#!/bin/bash
# Description: Check how clientId and clientSecret are populated from client aliases
# Expected: Find the relationship between client aliases and stored credentials
rg "clientId.*clientSecret" -A 3 -B 3
Length of output: 16877
Action Required: Verify Client Credential Handling
– Client credentials (clientId
/clientSecret
) are seeded into the user
table directly from env vars (see libs/prisma-service/prisma/seed.ts
and update_client_credential_data.sh
).
– They are stored in cleartext and passed straight to getManagementToken
(no encryption/decryption hooks found).
– Alias-based lookups in apps/api-gateway/src/user/utils/index.ts
similarly surface raw secrets from config.
Please confirm:
- That storing these credentials unencrypted in the database meets your security requirements.
- If encryption/decryption is needed, implement a CryptoService (or similar) around repository reads/writes.
- That client aliases (in config or DB) correctly map to the seeded credentials.
🤖 Prompt for AI Agents
In apps/user/src/user.service.ts around lines 308 to 313, client credentials are
retrieved from the database and passed in cleartext to getManagementToken
without any encryption or decryption. Review your security requirements to
confirm if storing and using these credentials unencrypted is acceptable. If
encryption is required, implement a CryptoService to encrypt credentials before
saving to the database and decrypt them when reading, then update this code to
use the decrypted values. Also verify that client aliases correctly map to the
seeded credentials in both the database and configuration.
const verifyCode = uuidv4(); | ||
let sendVerificationMail: boolean; | ||
|
||
const clientDetails = await getCredentialsByAlias(clientAlias); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify client credential retrieval security.
The getCredentialsByAlias
function retrieves encrypted credentials. Ensure the function properly validates the client alias and handles errors gracefully.
🏁 Script executed:
#!/bin/bash
# Description: Review the getCredentialsByAlias implementation for security
# Expected: Find proper validation and error handling
ast-grep --pattern 'export const getCredentialsByAlias = $$$'
Length of output: 63
To inspect the implementation of getCredentialsByAlias
, let’s locate its definition and surrounding logic:
🏁 Script executed:
#!/bin/bash
# Search for the getCredentialsByAlias definition and context
rg -n "getCredentialsByAlias" -C10
Length of output: 4664
Let’s inspect how getClientDetails and getDefaultClient use the alias and handle errors:
#!/bin/bash
# Show getClientDetails implementation
rg -n "getClientDetails" -C10 apps/api-gateway/src/user/utils/index.ts
# Show getDefaultClient implementation
rg -n "getDefaultClient" -C10 apps/api-gateway/src/user/utils/index.ts
Let’s inspect how aliases are validated in getClientDetails
and getDefaultClient
:
#!/bin/bash
# Show getClientDetails implementation
rg -n "function getClientDetails" -C5 apps/api-gateway/src/user/utils/index.ts
# Show getDefaultClient implementation
rg -n "function getDefaultClient" -C5 apps/api-gateway/src/user/utils/index.ts
To verify proper alias validation and error handling in the SSO utilities, let’s inspect both implementations:
🏁 Script executed:
#!/bin/bash
# Show full getClientDetails implementation
rg -n "getClientDetails" -C10 apps/api-gateway/src/user/utils/index.ts
# Show full getDefaultClient implementation
rg -n "getDefaultClient" -C10 apps/api-gateway/src/user/utils/index.ts
Length of output: 3060
Validate clientAlias and improve error handling in getCredentialsByAlias
To prevent unauthorized alias values and surface clear errors:
• Whitelist acceptable aliases before lookup
– e.g. maintain an ALLOWED_SSO_ALIASES
array (from config or env) and verify alias.toUpperCase()
is included.
• Normalize and sanitize the incoming alias (e.g. .trim().toUpperCase()
) in UserService
before calling getCredentialsByAlias
.
• In getCredentialsByAlias
(apps/api-gateway/src/user/utils/index.ts lines 39–44), catch the generic Error
for missing config and rethrow a framework-specific exception (e.g. BadRequestException
) with a clear message.
• Wrap the await getCredentialsByAlias(clientAlias)
call in UserService (apps/user/src/user.service.ts around line 134) in a try/catch
to handle and map errors to appropriate HTTP responses.
By enforcing a known set of aliases and mapping configuration misses to explicit errors, we avoid unintentional env-var exposure and improve debuggability.
🤖 Prompt for AI Agents
In apps/user/src/user.service.ts at line 134, validate and sanitize clientAlias
by trimming and converting it to uppercase before use, and check it against a
predefined ALLOWED_SSO_ALIASES whitelist to reject unauthorized aliases. In
apps/api-gateway/src/user/utils/index.ts lines 39–44, modify
getCredentialsByAlias to catch generic errors for missing config and rethrow
them as BadRequestException with clear messages. Also, wrap the await
getCredentialsByAlias(clientAlias) call in a try/catch block in user.service.ts
to handle these exceptions and map them to appropriate HTTP responses, improving
error clarity and security.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🔭 Outside diff range comments (3)
libs/logger/src/logger.service.ts (1)
88-100
:JSON.parse(JSON.stringify(...))
can break logging & drop the event
JSON.stringify
will throw on circular structures, BigInt values, Symbols, etc.
If that happens inside this block, the catch at L107 – 110 will swallow the error and nothing gets emitted to Otel, silently losing the log entry.attributes.error = 'string' === typeof data.error ? data.error : data.error instanceof Error ? { name: data.error.name, message: data.error.message, stack: data.error.stack } - : 'object' === typeof data.error - ? JSON.parse(JSON.stringify(data.error)) + : 'object' === typeof data.error + ? safeStringify(data.error) // avoid runtime TypeErrors : String(data.error);Suggested direction:
- Use a safe-stringify helper (e.g.
@ungap/structured-clone/json
orsafe-stable-stringify
) orutil.inspect
to serialize arbitrary objects without throwing.- Alternatively, fall back to
String(data.error)
for non-Error
objects; OTel attributes aren’t meant to carry nested structures anyway.- Extract this logic into a small
serializeError
utility for readability instead of the deeply-nested ternary.Failing to address this risks silent loss of production logs.
apps/verification/src/verification.service.ts (2)
804-816
: Refactor the deeply-nested ternaries ingetAgentUrl
for maintainability.Six separate
case
blocks now contain 4-level nested ternaries. While this was only re-formatted in the PR, the pattern is:url = cond1 && cond2 ? val1 : cond3 && cond2 ? val2 : cond1 ? val3 : cond3 ? val4 : null;Problems introduced / reinforced:
- Readability & reasoning cost – a small change becomes error-prone.
- Missing‐value bugs – if
tenantId
orthreadId
isundefined
,.replace()
will literally inject the string “undefined”.- Static-analysis blind spots – cyclomatic complexity ≈ 30 in a single expression.
- Future SSO client-alias logic will make this branch explode further.
Consider extracting a helper that builds the path with explicit
if/else
blocks and guards for required params:- url = - orgAgentType === OrgAgentType.DEDICATED && threadId - ? `${agentEndPoint}${CommonConstants.URL_GET_PROOF_PRESENTATIONS}?threadId=${threadId}` - : … + url = this.buildProofPresentationUrl({ + mode: 'list', + agentType: orgAgentType, + endPoint: agentEndPoint, + tenantId, + threadId, + });and inside the helper:
private buildProofPresentationUrl(opts: { mode: 'list' | 'single' | 'request' | 'accept' | 'oob' | 'verified'; agentType: OrgAgentType; endPoint: string; tenantId?: string; threadId?: string; presentationId?: string; }): string { const { mode, agentType, endPoint, tenantId, threadId, presentationId } = opts; const isShared = agentType === OrgAgentType.SHARED; const isDedicated = agentType === OrgAgentType.DEDICATED; if (isShared && !tenantId) { throw new NotFoundException('tenantId is required for shared agents'); } switch (mode) { case 'list': return `${endPoint}${ isDedicated ? CommonConstants.URL_GET_PROOF_PRESENTATIONS : CommonConstants.URL_SHAGENT_GET_PROOFS.replace('#', tenantId!) }${threadId ? `?threadId=${encodeURIComponent(threadId)}` : ''}`; … } }Benefits: linear control flow, explicit param checks, URL-encoding, and easier extension for SSO alias support.
Also applies to: 820-828, 832-838, 842-850, 854-860, 864-872
805-815
: URL parameters should be URI-encoded to prevent injection / breakage.
threadId
,proofPresentationId
, andtenantId
are interpolated directly into query-strings / paths. If any contains&
,/
,%
, or spaces the agent may mis-route the request or expose an injection vector.Wrap every dynamic segment with
encodeURIComponent()
(query params) orencodeURIComponent()
/encodeURI()
(path params):- ?threadId=${threadId} + ?threadId=${encodeURIComponent(threadId)} … - .replace('#', proofPresentationId) + .replace('#', encodeURIComponent(proofPresentationId))Same applies to
tenantId
.
[security]Also applies to: 821-827, 833-837, 843-849, 855-859, 869-871
♻️ Duplicate comments (10)
libs/common/src/response-messages/index.ts (1)
7-7
: Fix the typo in the response message key.The response message key has a typo: "fetchClientAlises" should be "fetchClientAliases". This could cause runtime errors when the key is referenced elsewhere in the codebase.
Apply this diff to fix the typo:
- fetchClientAlises: 'Client aliases fetched successfully', + fetchClientAliases: 'Client aliases fetched successfully',apps/agent-service/src/repositories/agent-service.repository.ts (1)
16-26
: Mark Prisma model imports as type-only to prevent accidental runtime usage.The Prisma model imports are used only as types and should be imported with
import type
to avoid including them in the runtime bundle and clarify intent.Apply this diff to use type-only imports:
-import { - Prisma, - ledgerConfig, - ledgers, - org_agents, - org_agents_type, - org_dids, - organisation, - platform_config, - user -} from '@prisma/client'; +import type { + Prisma, + ledgerConfig, + ledgers, + org_agents, + org_agents_type, + org_dids, + organisation, + platform_config, + user +} from '@prisma/client';apps/api-gateway/src/user/dto/create-user.dto.ts (1)
36-36
: Add validation and documentation decorators for consistency.The
clientAlias
property lacks the validation and documentation decorators that other properties have, creating inconsistency and potential security issues.Apply this diff to add proper decorators:
+ @ApiPropertyOptional({ example: 'clientAlias1' }) + @Transform(({ value }) => trim(value)) + @IsOptional() + @IsString({ message: 'clientAlias should be string' }) clientAlias?: string;.env.demo (1)
117-126
: Fix URL scheme typo in domain examples.Same typo as in .env.sample -
htts://
should behttps://
.-# CLIENT-NAME_DOMAIN=htts://VERIFIER-domain.com +# CLIENT-NAME_DOMAIN=https://VERIFIER-domain.com-# VERIFIER_DOMAIN=htts://VERIFIER-domain.com +# VERIFIER_DOMAIN=https://VERIFIER-domain.comapps/api-gateway/src/user/utils/index.ts (1)
33-46
: Address encryption inconsistency identified in past review.The past review comment correctly identified that
getCredentialsByAlias()
returns raw credentials fromgetClientDetails()
whilegetDefaultClient()
returns encrypted credentials, creating an inconsistency.The past review comment is still valid - this function should encrypt credentials for consistency and security. Please apply the suggested fix to encrypt
clientId
andclientSecret
before returning them.apps/user/repositories/user.repository.ts (1)
44-51
: Fix error handling issues identified in past review.The past review comment correctly identified two critical issues in the
fetchClientAliases()
method that need to be addressed.The error handling has incorrect method name in the log message and invalid error throwing syntax. Please apply the suggested fix from the past review to correct these issues.
apps/api-gateway/src/authz/authz.controller.ts (1)
107-112
: Address error handling concern from past review.The past review comment identified a valid concern about the fallback to
getDefaultClient().alias
lacking error handling.The async operation
(await getDefaultClient()).alias
could fail silently without proper error handling. Consider implementing the suggested try-catch block to handle potential failures gracefully.libs/prisma-service/prisma/seed.ts (2)
404-406
: Address shell command security vulnerability.The shell command is built from environment variables that could be manipulated, creating a potential security risk.
Consider these security improvements:
- Validate and sanitize
scriptPath
to ensure it's within expected boundaries- Use allowlisted script paths instead of arbitrary environment variables
- Implement path traversal protection
- Consider using Node.js alternatives instead of shell execution where possible
-const command = `${process.cwd()}/${scriptPath} ${dbUrl}`; +// Validate script path is within allowed directory +const allowedScripts = ['scripts/geo-import.sh', 'scripts/client-update.sh']; +if (!allowedScripts.includes(scriptPath)) { + throw new Error(`Script path not allowed: ${scriptPath}`); +} +const command = `${process.cwd()}/${scriptPath} ${dbUrl}`;
436-438
: Address shell command security vulnerability.Same security concern as the previous shell command - the script path from environment variables needs validation and sanitization.
Apply the same security improvements as suggested for the
importGeoLocationMasterData
function.apps/user/src/user.service.ts (1)
134-140
: Verify secure client credential handling.The
getCredentialsByAlias
function retrieves client credentials that are then passed directly to the registration service. Based on past review comments, ensure that:
- Client credentials are properly encrypted/decrypted
- The
clientAlias
parameter is validated and sanitized- Error handling properly masks sensitive information
The previous review noted that client credentials may be stored and used without proper encryption. Please verify that the security requirements are met in the credential retrieval and usage flow.
🧹 Nitpick comments (6)
apps/issuance/src/issuance.repository.ts (1)
684-686
: Minor readability nit – pull the joined list out of the template literal.Inlining the
referencedTables.join(', ')
call inside the template literal generates a tiny cognitive bump each time the string is read. Assigning it to a const first slightly clarifies the intent and keeps the template literal clean:- let errorMessage = `Organization ID ${orgId} is referenced in the following table(s): ${referencedTables.join( - ', ' - )}`; + const tableList = referencedTables.join(', '); + let errorMessage = `Organization ID ${orgId} is referenced in the following table(s): ${tableList}`;docker-compose.yml (1)
169-169
: Fix trailing spaces formatting issue.There's a trailing space after
api-gateway
that should be removed for consistency.Apply this diff to fix the formatting:
- - api-gateway + - api-gatewaylibs/prisma-service/prisma/migrations/20250603101813_added_client_alias/migration.sql (1)
1-11
: Consider adding indexes for query performance.The migration creates a well-structured table for client aliases. Consider adding indexes on frequently queried columns to improve performance:
-- Add indexes for common query patterns CREATE INDEX "client_aliases_clientAlias_idx" ON "client_aliases"("clientAlias"); CREATE INDEX "client_aliases_clientUrl_idx" ON "client_aliases"("clientUrl");This will optimize lookups by client alias and URL, which are likely to be common query patterns based on the SSO functionality being implemented.
libs/common/src/cast.helper.ts (1)
443-457
: Remove unnecessary async declaration.The function is marked as
async
but doesn't perform any asynchronous operations. CryptoJS operations are synchronous, making thePromise<string>
return type unnecessary.-export const encryptClientCredential = async (clientCredential: string): Promise<string> => { +export const encryptClientCredential = (clientCredential: string): string => { try { const encryptedToken = CryptoJS.AES.encrypt( JSON.stringify(clientCredential), process.env.CRYPTO_PRIVATE_KEY ).toString(); logger.debug('Client credentials encrypted successfully'); return encryptedToken; } catch (error) { logger.error('An error occurred during encryptClientCredential:', error); throw error; } };.env.sample (1)
162-171
: Fix URL scheme typo in domain examples.There's a typo in the URL scheme -
htts://
should behttps://
.-# CLIENT-NAME_DOMAIN=htts://myapp.com +# CLIENT-NAME_DOMAIN=https://myapp.com-# VERIFIER_DOMAIN=htts://VERIFIER-domain.com +# VERIFIER_DOMAIN=https://VERIFIER-domain.comlibs/prisma-service/prisma/seed.ts (1)
377-382
: Fix ESLint formatting error.There's a linebreak issue that violates the implicit-arrow-linebreak rule.
- const updatePromises = emptyTypeSchemaList.map((schema) => - prisma.schema.update({ + const updatePromises = emptyTypeSchemaList.map((schema) => prisma.schema.update({ where: { id: schema.id }, data: { type: 'indy' } - }) - ); + }));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (29)
.env.demo
(1 hunks).env.sample
(1 hunks).github/ISSUE_TEMPLATE/FEATURE-REQUEST.yml
(1 hunks).github/ISSUE_TEMPLATE/bug_report.yml
(1 hunks)apps/agent-service/src/repositories/agent-service.repository.ts
(2 hunks)apps/api-gateway/src/authz/authz.controller.ts
(6 hunks)apps/api-gateway/src/authz/authz.service.ts
(4 hunks)apps/api-gateway/src/authz/decorators/user-auth-client.ts
(1 hunks)apps/api-gateway/src/main.ts
(1 hunks)apps/api-gateway/src/user/dto/create-user.dto.ts
(1 hunks)apps/api-gateway/src/user/utils/index.ts
(1 hunks)apps/issuance/src/issuance.repository.ts
(1 hunks)apps/issuance/src/issuance.service.ts
(8 hunks)apps/ledger/src/schema/schema.service.ts
(0 hunks)apps/organization/repositories/organization.repository.ts
(1 hunks)apps/user/repositories/user.repository.ts
(6 hunks)apps/user/src/user.controller.ts
(6 hunks)apps/user/src/user.service.ts
(29 hunks)apps/user/templates/user-email-template.ts
(2 hunks)apps/verification/src/verification.service.ts
(7 hunks)docker-compose.yml
(2 hunks)libs/common/src/cast.helper.ts
(3 hunks)libs/common/src/common.module.ts
(1 hunks)libs/common/src/interfaces/user.interface.ts
(1 hunks)libs/common/src/response-messages/index.ts
(1 hunks)libs/logger/src/logger.service.ts
(1 hunks)libs/prisma-service/prisma/migrations/20250603101813_added_client_alias/migration.sql
(1 hunks)libs/prisma-service/prisma/schema.prisma
(1 hunks)libs/prisma-service/prisma/seed.ts
(1 hunks)
💤 Files with no reviewable changes (1)
- apps/ledger/src/schema/schema.service.ts
🧰 Additional context used
🧠 Learnings (2)
.github/ISSUE_TEMPLATE/bug_report.yml (1)
Learnt from: tipusinghaw
PR: credebl/platform#1297
File: apps/api-gateway/src/tracer.ts:24-28
Timestamp: 2025-06-24T11:15:29.468Z
Learning: In the credebl/platform repository, the team prefers to create utilities for validating required environment variables rather than implementing inline validation. This approach promotes reusability across different parts of the application.
libs/prisma-service/prisma/seed.ts (1)
Learnt from: tipusinghaw
PR: credebl/platform#1297
File: apps/api-gateway/src/tracer.ts:24-28
Timestamp: 2025-06-24T11:15:29.468Z
Learning: In the credebl/platform repository, the team prefers to create utilities for validating required environment variables rather than implementing inline validation. This approach promotes reusability across different parts of the application.
🧬 Code Graph Analysis (7)
apps/api-gateway/src/user/dto/create-user.dto.ts (1)
libs/common/src/cast.helper.ts (2)
trim
(40-44)toLowerCase
(36-38)
apps/api-gateway/src/user/utils/index.ts (2)
libs/common/src/interfaces/user.interface.ts (1)
IClientDetailsSSO
(24-29)libs/common/src/cast.helper.ts (1)
encryptClientCredential
(443-457)
libs/common/src/interfaces/user.interface.ts (1)
apps/user/interfaces/user.interface.ts (4)
IVerifyUserEmail
(173-176)ISendVerificationEmail
(51-56)IOrgRole
(22-26)IOrganisation
(27-38)
libs/common/src/common.module.ts (1)
libs/logger/src/logger.interface.ts (1)
Logger
(6-20)
apps/issuance/src/issuance.service.ts (1)
apps/issuance/interfaces/issuance.interfaces.ts (1)
SchemaDetails
(190-196)
apps/api-gateway/src/authz/authz.service.ts (3)
libs/common/src/interfaces/user.interface.ts (1)
IClientAliases
(64-70)apps/api-gateway/src/user/dto/create-user.dto.ts (1)
UserEmailVerificationDto
(7-37)apps/api-gateway/src/authz/dtos/reset-password.dto.ts (1)
ResetPasswordDto
(7-25)
apps/user/repositories/user.repository.ts (1)
apps/user/interfaces/user.interface.ts (1)
IUserDeletedActivity
(210-218)
🪛 YAMLlint (1.37.1)
.github/ISSUE_TEMPLATE/FEATURE-REQUEST.yml
[error] 73-73: no new line character at the end of file
(new-line-at-end-of-file)
.github/ISSUE_TEMPLATE/bug_report.yml
[error] 90-90: no new line character at the end of file
(new-line-at-end-of-file)
docker-compose.yml
[error] 169-169: trailing spaces
(trailing-spaces)
🪛 GitHub Check: CodeQL
libs/prisma-service/prisma/seed.ts
[warning] 406-406: Shell command built from environment values
This shell command depends on an uncontrolled absolute path.
[warning] 438-438: Shell command built from environment values
This shell command depends on an uncontrolled absolute path.
🪛 ESLint
libs/prisma-service/prisma/seed.ts
[error] 378-378: Expected no linebreak before this expression.
(implicit-arrow-linebreak)
🔇 Additional comments (36)
apps/verification/src/verification.service.ts (1)
246-248
: LGTM – clearer multi-line error logging.Splitting the log message over two lines improves readability without altering behaviour.
libs/prisma-service/prisma/schema.prisma (1)
522-530
: LGTM! Schema changes support SSO client alias functionality.The addition of the
client_aliases
model provides the necessary database foundation for Single Sign-On client alias management. The model follows proper Prisma conventions with UUID primary key, timestamps, and appropriate field types. The fix to close theCloudWalletType
enum also resolves a syntax issue.apps/organization/repositories/organization.repository.ts (1)
685-687
: Good addition of tenantId for multi-tenant support.Adding
tenantId
to theorg_agents
selection enhances the organization data retrieval to support multi-tenant SSO functionality. This change aligns well with the client alias features introduced in this PR.docker-compose.yml (1)
64-64
: Good addition of redis dependency for issuance service.Adding
redis
as a dependency for theissuance
service makes sense for supporting caching and session management requirements that likely come with the new SSO client alias functionality.libs/common/src/common.module.ts (1)
2-3
: LGTM! Proper integration of centralized logging.The addition of
LoggerModule
andLogger
to theCommonModule
provides centralized logging capabilities that will be valuable for the new SSO client alias features. The imports and providers are correctly configured following NestJS patterns.Also applies to: 8-9
apps/api-gateway/src/main.ts (1)
7-18
: LGTM: Clean import organization.The import reordering improves code organization by grouping related NestJS core imports, application modules, and utilities together. No functional changes.
apps/agent-service/src/repositories/agent-service.repository.ts (1)
1-2
: Import organization improvements look good.The ESLint directive and import reordering improve code organization and consistency.
Also applies to: 28-28
apps/issuance/src/issuance.service.ts (1)
153-155
: LGTM: Formatting improvements enhance readability.The multi-line formatting of await calls, logger statements, and ternary expressions improves code readability without changing functionality. These changes make the code easier to maintain and review.
Also applies to: 335-337, 484-487, 796-798, 1159-1160, 1169-1170, 1179-1180, 1189-1192
apps/api-gateway/src/authz/decorators/user-auth-client.ts (1)
1-26
: LGTM! Well-implemented validation pipe.The implementation correctly:
- Parses the environment variable with proper filtering and trimming
- Handles optional empty values gracefully
- Provides clear error messages with allowed values
- Uses consistent case transformation (uppercase)
- Follows NestJS pipe patterns properly
.env.sample (1)
151-172
: Well-structured SSO configuration template.The SSO configuration section provides clear documentation and examples for multi-client setup. The variable naming convention and organization are consistent and easy to follow.
apps/user/templates/user-email-template.ts (3)
2-11
: LGTM! Good parameter extension for client alias support.The addition of optional
redirectTo
andclientAlias
parameters properly extends the email template functionality while maintaining backward compatibility.
17-23
: Clean query parameter handling.The conditional appending of query parameters is implemented correctly with proper checks for undefined values.
80-82
: Improved error handling.The descriptive error message is much better than silent failure, making debugging easier when template creation fails.
apps/api-gateway/src/authz/authz.service.ts (3)
4-19
: Import organization and new dependencies look good.The consolidation of multi-line imports and addition of new interfaces (
IClientAliases
) and Prisma types (user
) properly supports the SSO functionality. The import organization is clean and follows consistent patterns.
33-35
: Simple and effective client alias retrieval method.The
getClientAlias()
method properly delegates to NATS messaging for retrieving client aliases. The implementation is clean and follows the established pattern used by other methods in this service.
42-45
: Return type change aligns with interface updates.The change from
ISendVerificationEmail
interface to Prismauser
type is consistent with the broader refactoring to support client aliases. This change aligns with the database-first approach and provides more comprehensive user data.apps/api-gateway/src/user/utils/index.ts (2)
4-9
: Secure default client credential handling.The
getDefaultClient()
function properly encrypts sensitive credentials (clientId and clientSecret) before returning them, following good security practices. The use of environment variables for configuration is appropriate.
12-31
: Environment variable pattern is clean and consistent.The
getClientDetails()
function uses a clear naming convention for environment variables and provides sensible defaults. The pattern of using alias-prefixed environment variables supports multi-client configuration well.apps/user/repositories/user.repository.ts (2)
3-22
: Import reorganization improves readability.The multi-line import formatting and addition of the
client_aliases
type properly supports the new functionality. The alphabetical organization within import groups follows good practices.
719-726
: Method signature formatting improves readability.The multi-line parameter formatting for
updateOrgDeletedActivity()
makes the method signature more readable and follows consistent formatting patterns used elsewhere in the codebase.libs/common/src/interfaces/user.interface.ts (3)
13-22
: Interface enhancement supports flexible client management.The addition of
redirectTo
(optional) andclientAlias
(required) properties toISendVerificationEmail
properly supports the enhanced email verification flow with client-specific routing. The type definitions are consistent and well-structured.
24-29
: Well-defined SSO client details interface.The
IClientDetailsSSO
interface clearly defines the structure for SSO client credentials with appropriate string types for all properties. This interface aligns well with the utility functions in the codebase.
64-70
: Client aliases interface matches database schema.The
IClientAliases
interface properly represents the database structure for client aliases with appropriate string types for timestamps and identifiers. This supports the new database table and repository methods.apps/api-gateway/src/authz/authz.controller.ts (3)
31-32
: Appropriate imports for new functionality.The addition of
ClientAliasValidationPipe
andgetDefaultClient
imports properly supports the enhanced client alias validation and utility functionality.
50-65
: Well-implemented client aliases endpoint.The new
/clientAliases
endpoint follows established patterns with proper HTTP status codes, response structure, and API documentation. The implementation is clean and consistent with other endpoints in the controller.
98-105
: Dynamic API documentation enhances developer experience.The
@ApiQuery
decorator with dynamic enum values derived from environment variables provides accurate API documentation that reflects the actual supported client aliases. This is a good practice for maintaining documentation consistency.apps/user/src/user.controller.ts (4)
1-25
: LGTM! Import organization and formatting improvements.The import reorganization and semicolon-separated parameter typings improve code readability and follow TypeScript best practices.
38-47
: LGTM! Well-structured client aliases endpoint.The method properly delegates to the service layer and maintains consistent typing. The ESLint disable for camelcase is justified due to Prisma generated types.
55-55
: LGTM! Improved return type specificity.Changing the return type from
ISendVerificationEmail
touser
provides more specific and useful information about the created user.
221-237
: LGTM! Enhanced parameter readability.The expanded parameter destructuring improves code readability and maintainability without affecting functionality.
libs/prisma-service/prisma/seed.ts (2)
8-8
: LGTM! Centralized encryption logic.Using the shared
encryptClientCredential
helper from@credebl/common/cast.helper
follows the DRY principle and centralizes encryption logic.
17-34
: LGTM! Consistent error handling pattern.The refactored error handling with try-catch blocks, specific error logging, and proper error propagation improves reliability and debugging capabilities.
apps/user/src/user.service.ts (4)
87-101
: LGTM! Well-structured service method.The
getClientAliases
method follows the established service pattern with proper delegation to the repository layer and consistent error handling.
206-215
: LGTM! Improved parameter formatting.The expanded parameter list with proper line breaks and optional parameter indication enhances code readability and maintainability.
467-483
: LGTM! Consistent error handling pattern.The error handling in
refreshTokenDetails
and throughout the service maintains a consistent pattern with proper logging and appropriate exception types.
1152-1168
: LGTM! Enhanced method signature readability.The improved parameter destructuring and formatting in
updateOrgDeletedActivity
and similar methods throughout the file enhances code readability and maintainability.
- Tests written and passing | ||
validations: | ||
required: false | ||
required: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-add missing trailing newline to satisfy YAMLlint & POSIX text-file convention
Removing the final newline triggers the YAMLlint new-line-at-end-of-file
error and can break CI pipelines that enforce linting. All text files should terminate with a single \n
.
validations:
required: false
+
📝 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.
required: false | |
validations: | |
required: false | |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 73-73: no new line character at the end of file
(new-line-at-end-of-file)
🤖 Prompt for AI Agents
In .github/ISSUE_TEMPLATE/FEATURE-REQUEST.yml at line 73, the file is missing a
trailing newline at the end. Add a single newline character after the last line
to satisfy YAMLlint requirements and POSIX text-file conventions, ensuring the
file ends with a newline to prevent linting errors and CI pipeline failures.
- Stack traces | ||
validations: | ||
required: false | ||
required: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Restore trailing newline to avoid YAMLlint failure
Same issue as in the feature-request template—lack of a terminal newline violates lint rules and standard tooling expectations.
validations:
required: false
+
📝 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.
required: false | |
required: false | |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 90-90: no new line character at the end of file
(new-line-at-end-of-file)
🤖 Prompt for AI Agents
In .github/ISSUE_TEMPLATE/bug_report.yml at line 90, the file is missing a
trailing newline which causes YAMLlint to fail. Add a newline character at the
end of the file to comply with linting rules and standard file formatting
conventions.
? `${agentEndPoint}${CommonConstants.URL_SHAGENT_GET_PROOFS_BY_PRESENTATION_ID}` | ||
.replace('#', proofPresentationId) | ||
.replace('@', tenantId) | ||
: null; | ||
break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guard against undefined
replacements in shared-agent URLs.
All shared-agent branches use:
.replace('#', tenantId)
.replace('@', tenantId)
If tenantId
is falsy (undefined
, ''
, etc.), the resulting URL contains the literal placeholders undefined
or leaves the marker intact – both cause 404s at runtime.
Add an explicit validation block before the switch:
if (orgAgentType === OrgAgentType.SHARED && !tenantId) {
throw new BadRequestException('tenantId is mandatory for shared-agent URLs');
}
or, inside each case, short-circuit early.
Also applies to: 846-850, 868-872
🤖 Prompt for AI Agents
In apps/verification/src/verification.service.ts around lines 824 to 828, the
code replaces placeholders in shared-agent URLs with tenantId without checking
if tenantId is defined, causing invalid URLs and 404 errors. Add a validation
before the switch statement to check if orgAgentType is SHARED and tenantId is
falsy, and throw a BadRequestException with a clear message if so. Apply the
same validation or early short-circuit in the similar code blocks at lines
846-850 and 868-872 to prevent undefined replacements in all shared-agent URL
constructions.
try { | ||
const encryptedToken = CryptoJS.AES.encrypt( | ||
JSON.stringify(clientCredential), | ||
process.env.CRYPTO_PRIVATE_KEY |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify environment variable availability.
Ensure that CRYPTO_PRIVATE_KEY
is always available when this function is called. Consider adding validation for the environment variable.
🏁 Script executed:
#!/bin/bash
# Check if CRYPTO_PRIVATE_KEY is used elsewhere and how it's validated
rg -A 3 -B 3 "CRYPTO_PRIVATE_KEY" --type ts
Length of output: 2088
🏁 Script executed:
#!/bin/bash
# Locate environment variable validation or config utilities in libs/common
fd -t f --glob "*env*.ts" libs/common/src
fd -t f --glob "*config*.ts" libs/common/src
Length of output: 121
🏁 Script executed:
#!/bin/bash
# Display NATS configuration file for environment variable handling
sed -n '1,200p' libs/common/src/nats.config.ts
Length of output: 782
Add centralized validation for CRYPTO_PRIVATE_KEY
Instead of calling process.env.CRYPTO_PRIVATE_KEY
directly, introduce—or reuse—a utility (for example, getRequiredEnv('CRYPTO_PRIVATE_KEY')
) that throws if the variable is missing. This ensures the app fails fast on misconfiguration. Update all usages accordingly:
- libs/common/src/cast.helper.ts
- libs/common/src/common.service.ts (in
decryptPassword
anddataEncryption
) - apps/agent-service/src/agent-service.service.ts
🤖 Prompt for AI Agents
In libs/common/src/cast.helper.ts at line 447, replace direct access to
process.env.CRYPTO_PRIVATE_KEY with a centralized utility function like
getRequiredEnv('CRYPTO_PRIVATE_KEY') that throws an error if the environment
variable is missing. This change ensures the application fails fast on
misconfiguration. Apply similar updates to all other occurrences in
libs/common/src/common.service.ts (in decryptPassword and dataEncryption) and
apps/agent-service/src/agent-service.service.ts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🔭 Outside diff range comments (2)
apps/api-gateway/src/main.ts (2)
76-97
: DuplicateenableVersioning()
– keep only the final call
app.enableVersioning()
is invoked twice (76-79 and 94-97).
Calling it the first time is redundant and potentially confusing because the second call overrides the settings anyway. Drop the earlier call to reduce noise and avoid accidental divergence in the future.- app.enableVersioning({ - type: VersioningType.URI, - defaultVersion: ['1'] - });
39-43
: Micro-service never startedAfter
app.connectMicroservice(...)
, the Nest docs requireawait app.startAllMicroservices()
(or the observable variant).
Without it, the NATS transport won’t consume messages, giving the illusion the gateway is “up” while the async layer is silent.app.connectMicroservice<MicroserviceOptions>({ transport: Transport.NATS, options: getNatsOptions(CommonConstants.API_GATEWAY_SERVICE, process.env.API_GATEWAY_NKEY_SEED) }); + + // Start the NATS micro-service before HTTP server starts + await app.startAllMicroservices();
♻️ Duplicate comments (12)
.github/ISSUE_TEMPLATE/FEATURE-REQUEST.yml (1)
73-73
: Restore missing trailing newline to pass YAMLlintSame issue flagged previously—absence of a final
\n
breaksnew-line-at-end-of-file
check and can fail CI.
Fix by adding a single blank line at EOF.validations: required: false +
.github/ISSUE_TEMPLATE/bug_report.yml (1)
90-90
: Re-add trailing newline to satisfy linter & toolingIdentical formatting defect as noted earlier; file must terminate with a newline to comply with YAMLlint and POSIX text-file conventions.
validations: required: false +
apps/verification/src/verification.service.ts (1)
810-815
: LGTM! Improved ternary expression formatting.The reformatted ternary expressions with better indentation and line breaks significantly improve code readability while preserving the original logic.
However, the past review comment about
tenantId
validation for shared-agent URLs is still applicable - these formatting changes don't address the underlying issue where undefinedtenantId
could cause 404 errors in shared-agent URL construction.Also applies to: 824-828, 836-837, 846-850, 858-859, 868-872
libs/common/src/response-messages/index.ts (1)
7-7
: The typo identified in the previous review is still present - "fetchClientAlises" should be "fetchClientAliases".libs/common/src/cast.helper.ts (1)
447-447
: Use centralized environment variable validationThis change directly accesses
process.env.CRYPTO_PRIVATE_KEY
without validation, which contradicts the established pattern mentioned in previous reviews. The team prefers creating utilities for validating required environment variables rather than implementing inline validation.apps/api-gateway/src/user/dto/create-user.dto.ts (1)
36-36
: Add validation and documentation decorators for consistency.The
clientAlias
property still lacks the validation and documentation decorators that other properties have, creating inconsistency and potential security issues.apps/api-gateway/src/user/utils/index.ts (1)
33-46
: Encryption inconsistency needs to be addressed.The
getCredentialsByAlias
function returns raw credentials fromgetClientDetails
without encryption, unlikegetDefaultClient
which encrypts credentials. This creates a security inconsistency.apps/user/repositories/user.repository.ts (1)
44-51
: Fix error handling in fetchClientAliases method.The method has the same issues previously identified: incorrect error message references the wrong method name and invalid error throwing syntax.
apps/api-gateway/src/authz/authz.controller.ts (1)
112-112
: Ensure robust fallback for client alias assignment.The fallback to
getDefaultClient().alias
uses async operation but there's potential for this to fail silently.apps/user/src/user.service.ts (3)
134-134
: Validate clientAlias and improve error handling in getCredentialsByAliasThe previous review comment about validating clientAlias before calling
getCredentialsByAlias
still applies. This line needs proper alias validation and error handling to prevent unauthorized access and provide clear error messages.
308-313
: Ensure consistent client credential usage.The previous review comment about client credential security still applies. These credentials are retrieved from the database and used without encryption/decryption. Please verify this meets your security requirements or implement proper credential encryption.
324-329
: Client credential security applies here too.The client credential security concerns from the previous review comments also apply to this usage of
userData.clientId
anduserData.clientSecret
.
🧹 Nitpick comments (5)
docker-compose.yml (1)
169-169
: Fix trailing space formatting issue.Static analysis detected a trailing space that should be removed for clean formatting.
Apply this diff to remove the trailing space:
- - api-gateway + - api-gatewayapps/issuance/src/issuance.repository.ts (1)
684-686
: Multiline template literal inserts unwanted new-line/indentation charactersBreaking the template literal across lines means the resulting string will contain
\n
plus leading spaces before,
.
For log / API responses this usually isn’t desirable and makes the message harder to parse or compare in tests.- let errorMessage = `Organization ID ${orgId} is referenced in the following table(s): ${referencedTables.join( - ', ' - )}`; + const errorMessage = `Organization ID ${orgId} is referenced in the following table(s): ${referencedTables.join(', ')}`;apps/api-gateway/src/authz/decorators/user-auth-client.ts (1)
3-26
: LGTM! Well-implemented validation pipe with good design decisionsThe
ClientAliasValidationPipe
correctly implements client alias validation with:
- Proper dependency injection pattern
- Appropriate normalization to uppercase
- Sensible handling of empty values for optional parameters
- Clear error messaging
Consider caching the environment variable parsing to avoid repeated string operations:
@Injectable() export class ClientAliasValidationPipe implements PipeTransform { private readonly allowedAliases: string[]; + private readonly allowedAliasesSet: Set<string>; constructor() { this.allowedAliases = (process.env.SUPPORTED_SSO_CLIENTS || '') .split(',') .map((alias) => alias.trim()) .filter(Boolean); + this.allowedAliasesSet = new Set(this.allowedAliases); } transform(value: string): string { if (!value) { return value; } const upperValue = value.toUpperCase(); - if (!this.allowedAliases.includes(upperValue)) { + if (!this.allowedAliasesSet.has(upperValue)) { throw new BadRequestException(`Invalid clientAlias. Allowed values are: ${this.allowedAliases.join(', ')}`); } return upperValue; } }.env.demo (1)
117-117
: Fix typo in sample domain URL.There's a typo in the sample domain URL - "htts" should be "https".
-# CLIENT-NAME_DOMAIN=htts://VERIFIER-domain.com +# CLIENT-NAME_DOMAIN=https://VERIFIER-domain.com-# VERIFIER_DOMAIN=htts://VERIFIER-domain.com +# VERIFIER_DOMAIN=https://VERIFIER-domain.comAlso applies to: 124-124
apps/user/src/user.service.ts (1)
87-101
: Fix misleading error message and improve documentation.The error message references "Create User" which is incorrect for this method. Also, the JSDoc comment is incomplete.
Apply this diff to fix the issues:
/** - * - * @returns client alias and its url + * Retrieves all available client aliases and their associated URLs + * @returns Promise<client_aliases[]> Array of client aliases with their configuration */ // eslint-disable-next-line camelcase async getClientAliases(): Promise<client_aliases[]> { try { const clientAliases = await this.userRepository.fetchClientAliases(); return clientAliases; } catch (error) { - this.logger.error(`In Create User : ${JSON.stringify(error)}`); + this.logger.error(`In getClientAliases: ${JSON.stringify(error)}`); throw new RpcException(error.response ? error.response : error); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (29)
.env.demo
(1 hunks).env.sample
(1 hunks).github/ISSUE_TEMPLATE/FEATURE-REQUEST.yml
(1 hunks).github/ISSUE_TEMPLATE/bug_report.yml
(1 hunks)apps/agent-service/src/repositories/agent-service.repository.ts
(2 hunks)apps/api-gateway/src/authz/authz.controller.ts
(6 hunks)apps/api-gateway/src/authz/authz.service.ts
(4 hunks)apps/api-gateway/src/authz/decorators/user-auth-client.ts
(1 hunks)apps/api-gateway/src/main.ts
(1 hunks)apps/api-gateway/src/user/dto/create-user.dto.ts
(1 hunks)apps/api-gateway/src/user/utils/index.ts
(1 hunks)apps/issuance/src/issuance.repository.ts
(1 hunks)apps/issuance/src/issuance.service.ts
(8 hunks)apps/ledger/src/schema/schema.service.ts
(0 hunks)apps/organization/repositories/organization.repository.ts
(1 hunks)apps/user/repositories/user.repository.ts
(6 hunks)apps/user/src/user.controller.ts
(6 hunks)apps/user/src/user.service.ts
(29 hunks)apps/user/templates/user-email-template.ts
(2 hunks)apps/verification/src/verification.service.ts
(7 hunks)docker-compose.yml
(2 hunks)libs/common/src/cast.helper.ts
(3 hunks)libs/common/src/common.module.ts
(1 hunks)libs/common/src/interfaces/user.interface.ts
(1 hunks)libs/common/src/response-messages/index.ts
(1 hunks)libs/logger/src/logger.service.ts
(1 hunks)libs/prisma-service/prisma/migrations/20250603101813_added_client_alias/migration.sql
(1 hunks)libs/prisma-service/prisma/schema.prisma
(1 hunks)libs/prisma-service/prisma/seed.ts
(1 hunks)
💤 Files with no reviewable changes (1)
- apps/ledger/src/schema/schema.service.ts
🧰 Additional context used
🧠 Learnings (3)
.github/ISSUE_TEMPLATE/bug_report.yml (1)
Learnt from: tipusinghaw
PR: credebl/platform#1297
File: apps/api-gateway/src/tracer.ts:24-28
Timestamp: 2025-06-24T11:15:29.468Z
Learning: In the credebl/platform repository, the team prefers to create utilities for validating required environment variables rather than implementing inline validation. This approach promotes reusability across different parts of the application.
libs/common/src/cast.helper.ts (1)
Learnt from: tipusinghaw
PR: credebl/platform#1297
File: apps/api-gateway/src/tracer.ts:24-28
Timestamp: 2025-06-24T11:15:29.468Z
Learning: In the credebl/platform repository, the team prefers to create utilities for validating required environment variables rather than implementing inline validation. This approach promotes reusability across different parts of the application.
libs/prisma-service/prisma/seed.ts (1)
Learnt from: tipusinghaw
PR: credebl/platform#1297
File: apps/api-gateway/src/tracer.ts:24-28
Timestamp: 2025-06-24T11:15:29.468Z
Learning: In the credebl/platform repository, the team prefers to create utilities for validating required environment variables rather than implementing inline validation. This approach promotes reusability across different parts of the application.
🧬 Code Graph Analysis (7)
libs/common/src/common.module.ts (1)
libs/logger/src/logger.interface.ts (1)
Logger
(6-20)
apps/api-gateway/src/user/dto/create-user.dto.ts (2)
libs/common/src/cast.helper.ts (2)
trim
(40-44)toLowerCase
(36-38)libs/validations/maxLength.ts (1)
MaxLength
(3-28)
apps/issuance/src/issuance.service.ts (1)
apps/issuance/interfaces/issuance.interfaces.ts (1)
SchemaDetails
(190-196)
apps/user/repositories/user.repository.ts (1)
apps/user/interfaces/user.interface.ts (1)
IUserDeletedActivity
(210-218)
apps/api-gateway/src/authz/authz.service.ts (3)
libs/common/src/interfaces/user.interface.ts (2)
IClientAliases
(64-70)IResetPasswordResponse
(55-58)apps/api-gateway/src/user/dto/create-user.dto.ts (1)
UserEmailVerificationDto
(7-37)apps/api-gateway/src/authz/dtos/reset-password.dto.ts (1)
ResetPasswordDto
(7-25)
apps/api-gateway/src/user/utils/index.ts (2)
libs/common/src/interfaces/user.interface.ts (1)
IClientDetailsSSO
(24-29)libs/common/src/cast.helper.ts (1)
encryptClientCredential
(443-457)
libs/common/src/interfaces/user.interface.ts (1)
apps/user/interfaces/user.interface.ts (4)
IVerifyUserEmail
(173-176)ISendVerificationEmail
(51-56)IOrgRole
(22-26)IOrganisation
(27-38)
🪛 YAMLlint (1.37.1)
.github/ISSUE_TEMPLATE/FEATURE-REQUEST.yml
[error] 73-73: no new line character at the end of file
(new-line-at-end-of-file)
.github/ISSUE_TEMPLATE/bug_report.yml
[error] 90-90: no new line character at the end of file
(new-line-at-end-of-file)
docker-compose.yml
[error] 169-169: trailing spaces
(trailing-spaces)
🪛 GitHub Check: CodeQL
libs/prisma-service/prisma/seed.ts
[warning] 406-406: Shell command built from environment values
This shell command depends on an uncontrolled absolute path.
[warning] 438-438: Shell command built from environment values
This shell command depends on an uncontrolled absolute path.
🪛 ESLint
libs/prisma-service/prisma/seed.ts
[error] 378-378: Expected no linebreak before this expression.
(implicit-arrow-linebreak)
🔇 Additional comments (41)
apps/api-gateway/src/main.ts (1)
10-11
: Path alias looks correct – LGTM
AllExceptionsFilter
&AppModule
are imported from their dedicated barrels. No issues spotted here.apps/verification/src/verification.service.ts (1)
246-248
: LGTM! Improved error logging readability.The multi-line error logging format enhances readability while maintaining the same functionality.
apps/organization/repositories/organization.repository.ts (1)
685-687
: LGTM! Added tenantId to support SSO client alias functionality.Including the
tenantId
field in theorg_agents
selection appropriately expands the data retrieved to support the new client alias and multi-tenant SSO features being introduced.docker-compose.yml (1)
64-64
: LGTM! Added redis dependency for issuance service.Adding
redis
as a dependency for theissuance
service appropriately ensures proper service orchestration, likely supporting caching or session management for the new SSO features.libs/common/src/common.module.ts (1)
2-3
: Avoid providingLogger
manually – it is already registered globally by Nest
Logger
from@nestjs/common
is a global provider; putting it inproviders
creates an extra instance and adds noise to DI graphs.
Unless you need a scoped / customised logger, you can remove it and still injectLogger
anywhere.-import { Logger, Module } from '@nestjs/common'; +import { Module } from '@nestjs/common'; … -imports: [HttpModule, LoggerModule], -providers: [CommonService, Logger], +imports: [HttpModule, LoggerModule], +providers: [CommonService],[ suggest_nitpick ]
Also applies to: 8-9
libs/logger/src/logger.service.ts (1)
92-99
: Minor: serialising arbitrary objects may throw on circular structures
JSON.stringify
will throw ifdata.error
contains circular refs.
Consider a safe-stringify helper (e.g.util.inspect
) or wrap this block in a try-catch to avoid losing the entire log event.[ suggest_optional_refactor ]
libs/prisma-service/prisma/schema.prisma (1)
524-530
:client_aliases.clientAlias
probably needs a uniqueness constraintWithout
@unique
, two rows could use the same alias, which would break “alias → URL” look-ups and SSO routing.- clientAlias String? + clientAlias String? @uniqueYou may also want an index on
clientUrl
if it’s queried frequently.
[ suggest_essential_refactor ]apps/agent-service/src/repositories/agent-service.repository.ts (1)
16-26
: Import Prisma models as types only (duplicate of previous feedback)These Prisma models are used purely for typing; importing them as values adds dead code to the bundle.
-import { - Prisma, - ledgerConfig, - ledgers, - org_agents, - org_agents_type, - org_dids, - organisation, - platform_config, - user -} from '@prisma/client'; +import type { + Prisma, + ledgerConfig, + ledgers, + org_agents, + org_agents_type, + org_dids, + organisation, + platform_config, + user +} from '@prisma/client';[ duplicate_comment ]
libs/common/src/cast.helper.ts (1)
443-457
: LGTM! Well-implemented encryption helper with proper error handlingThe function correctly implements AES encryption for client credentials with appropriate error handling and logging. The async/await pattern and JSON stringification are properly implemented.
libs/prisma-service/prisma/migrations/20250603101813_added_client_alias/migration.sql (1)
1-11
: LGTM! Well-structured database migrationThe migration correctly creates the
client_aliases
table with appropriate data types and constraints. The UUID primary key and timestamp fields follow standard conventions, and the nullableclientAlias
with non-nullableclientUrl
makes sense for the SSO client management use case.apps/issuance/src/issuance.service.ts (5)
153-155
: LGTM! Formatting improvementGood consolidation of the multi-line
await
call into a cleaner single-line assignment.
335-337
: LGTM! Consistent formatting improvementSimilar formatting improvement for better readability.
484-487
: LGTM! Better string template formattingThe reformatted logger error call improves readability by properly splitting the long template expression.
796-798
: LGTM! Consistent formatting patternAnother good formatting improvement maintaining consistency with the other similar changes.
1159-1192
: LGTM! Improved ternary expression formattingThe reformatted ternary expressions in the
getAgentUrl
method improve readability with better indentation and line breaks..env.sample (1)
151-172
: SSO environment variables are correctly integratedOnly the new
SUPPORTED_SSO_CLIENTS
variable is actually consumed in the code, and it’s consistently referenced in:
- apps/api-gateway/src/authz/authz.controller.ts
- apps/api-gateway/src/authz/decorators/user-auth-client.ts
The
CLIENT-NAME_*
entries in.env.sample
are illustrative placeholders for future per-client settings. No inconsistencies detected..env.demo (1)
106-127
: Well-structured SSO configuration template.The SSO configuration section is comprehensive and well-documented with clear examples and comments explaining the purpose of each variable.
apps/user/templates/user-email-template.ts (3)
2-11
: Good implementation of optional SSO parameters.The method signature properly handles the new optional parameters for SSO support. The conditional parameter appending prevents unnecessary query parameters when values are not provided.
17-23
: Clean conditional query parameter handling.The conditional appending of
redirectTo
andclientAlias
parameters is well-implemented, ensuring only defined values are added to the URL.
80-82
: Appropriate error handling for template creation.The try-catch block with a descriptive error message improves error handling compared to silent failures.
apps/api-gateway/src/authz/authz.service.ts (3)
8-14
: Clean import consolidation.The import statements are well-organized, properly importing the new interfaces needed for SSO client alias support.
33-35
: Consistent NATS message pattern.The new
getClientAlias
method follows the established pattern for NATS communication, using the same message sending approach as other methods in the service.
42-42
: Appropriate return type change.Changing the return type to Prisma's
user
type aligns with the database integration and provides better type safety.apps/api-gateway/src/user/utils/index.ts (2)
4-9
: Well-implemented default client credential encryption.The
getDefaultClient
function properly encrypts client credentials using the shared encryption helper, ensuring secure handling of sensitive data.
41-43
: Good validation for missing configuration.The validation check ensures all required client configuration is present before proceeding, providing clear error messages for missing SSO client setup.
apps/user/repositories/user.repository.ts (2)
19-19
: LGTM!The addition of
client_aliases
import properly supports the new SSO client alias functionality.
719-726
: LGTM!The multiline method signature improves readability for methods with multiple parameters.
libs/common/src/interfaces/user.interface.ts (3)
13-22
: LGTM!The interface updates properly support SSO client alias functionality. Making
clientAlias
required ensures proper client identification for email verification flows.
24-29
: LGTM!The new
IClientDetailsSSO
interface properly structures SSO client configuration data.
64-70
: LGTM!The new
IClientAliases
interface aligns with the client_aliases database model for exposing client alias metadata.apps/api-gateway/src/authz/authz.controller.ts (2)
45-65
: LGTM!The new client aliases endpoint is well-structured with proper API documentation and response handling.
98-105
: LGTM!The dynamic enum generation from
SUPPORTED_SSO_CLIENTS
environment variable provides good API documentation for supported client aliases.apps/user/src/user.controller.ts (2)
38-47
: LGTM!The new
getClientAliases
method properly implements the microservice pattern for exposing client alias data.
55-55
: LGTM!The return type change from
ISendVerificationEmail
touser
correctly reflects the actual Prisma model being returned.libs/prisma-service/prisma/seed.ts (2)
8-8
: LGTM!Using the shared
encryptClientCredential
helper promotes code reusability and maintains consistent encryption logic across the platform.
433-434
: LGTM!The encryption of client credentials using the shared helper ensures secure storage of sensitive data.
apps/user/src/user.service.ts (5)
206-215
: LGTM: Method signature updated appropriately for SSO support.The addition of optional
redirectTo
andclientAlias
parameters maintains backward compatibility while enabling the new SSO functionality.
227-236
: LGTM: Email template call properly updated.The email template call correctly passes all new parameters for SSO support including
redirectTo
andclientAlias
.
467-478
: Improved error handling structure.The restructuring with proper try-catch blocks improves error handling for token refresh operations. However, the client credential security concerns from previous reviews still apply to lines 472-473.
29-29
: LGTM: Import addition for client aliases support.The addition of
client_aliases
import supports the new SSO client alias functionality.
67-67
: LGTM: Utility function import for credential lookup.The import of
getCredentialsByAlias
utility function enables the alias-based credential retrieval functionality.
import helmet from 'helmet'; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
xssFilter
option is removed in Helmet ≥5
helmet({ xssFilter: true })
no longer does anything from v5 onward and triggers a runtime warning.
Either pin Helmet to v4 or replace with the dedicated xssClean
/ content-security-policy protection you actually need.
🤖 Prompt for AI Agents
In apps/api-gateway/src/main.ts around lines 17 to 18, the use of helmet with
the xssFilter option is deprecated and triggers a runtime warning in Helmet
version 5 and above. Remove the xssFilter option from the helmet configuration
and instead implement XSS protection using a dedicated middleware like xss-clean
or configure a content-security-policy header as needed. Alternatively, if you
want to keep using xssFilter, downgrade Helmet to version 4, but updating to the
new recommended approach is preferred.
const command = `${process.cwd()}/${scriptPath} ${dbUrl} ${encryptedClientId} ${encryptedClientSecret}`; | ||
|
||
const { stdout, stderr } = await execPromise(command); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Address security risk with shell command execution.
Similar to the above, this shell command construction also uses uncontrolled environment variables which could pose a security risk.
Apply similar validation as suggested for the previous shell command execution.
🧰 Tools
🪛 GitHub Check: CodeQL
[warning] 438-438: Shell command built from environment values
This shell command depends on an uncontrolled absolute path.
🤖 Prompt for AI Agents
In libs/prisma-service/prisma/seed.ts around lines 436 to 438, the shell command
is constructed using environment variables without validation, posing a security
risk. To fix this, validate and sanitize the environment variables (dbUrl,
encryptedClientId, encryptedClientSecret) before including them in the command
string to prevent injection attacks. Alternatively, consider using safer methods
to pass arguments to the command, such as using an array of arguments with a
child process spawn function that avoids shell interpretation.
const command = `${process.cwd()}/${scriptPath} ${dbUrl}`; | ||
|
||
const { stdout, stderr } = await execPromise(command); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Address security risk with shell command execution.
The shell command construction uses uncontrolled environment variables which could pose a security risk. Consider validating the script path and database URL before execution.
Add validation for the environment variables before constructing the shell command:
const importGeoLocationMasterData = async (): Promise<void> => {
try {
const scriptPath = process.env.GEO_LOCATION_MASTER_DATA_IMPORT_SCRIPT;
const dbUrl = process.env.DATABASE_URL;
if (!scriptPath || !dbUrl) {
throw new Error('Environment variables GEO_LOCATION_MASTER_DATA_IMPORT_SCRIPT or DATABASE_URL are not set.');
}
+ // Validate script path to prevent path traversal attacks
+ if (scriptPath.includes('..') || !scriptPath.startsWith('./')) {
+ throw new Error('Invalid script path detected');
+ }
const command = `${process.cwd()}/${scriptPath} ${dbUrl}`;
🏁 Script executed:
#!/bin/bash
# Inspect how execPromise is imported/defined
rg -n "execPromise" -C 5 libs/prisma-service/prisma/seed.ts
# Inspect context of importGeoLocationMasterData function around lines 380–430
rg -n "const importGeoLocationMasterData" -C 50 libs/prisma-service/prisma/seed.ts
Length of output: 5061
Use execFile/spawn to Prevent Shell Injection
Passing unchecked environment variables into a shell command still leaves an injection risk. Instead of exec
, switch to execFile
(or spawn
) and pass the script path and its args as an array. This bypasses the shell and eliminates concatenation-based attacks.
Suggested changes:
-
Replace the
exec
import and usage:- import { exec } from 'child_process'; + import { execFile } from 'child_process'; import * as util from 'util'; const execPromise = util.promisify(exec);
-
In importGeoLocationMasterData (around lines 404–406):
// validate presence if (!scriptPath || !dbUrl) { throw new Error('Environment variables GEO_LOCATION_MASTER_DATA_IMPORT_SCRIPT or DATABASE_URL are not set.'); } - const command = `${process.cwd()}/${scriptPath} ${dbUrl}`; - const { stdout, stderr } = await execPromise(command); + // resolve and lock down script location + const scriptFullPath = path.resolve(process.cwd(), scriptPath); + const { stdout, stderr } = await util + .promisify(execFile)(scriptFullPath, [dbUrl]);
-
Apply the same pattern in updateClientCredential (around lines 436–438), passing
[dbUrl, encryptedClientId, encryptedClientSecret]
as the args array.
Also ensure scriptPath
is validated (e.g. resides under an expected subdirectory) to guard against path traversal.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Check: CodeQL
[warning] 406-406: Shell command built from environment values
This shell command depends on an uncontrolled absolute path.
🤖 Prompt for AI Agents
In libs/prisma-service/prisma/seed.ts around lines 404 to 406, replace the use
of execPromise with execFile or spawn to avoid shell injection risks by passing
the script path and arguments as an array instead of a concatenated string.
Update the import accordingly and ensure scriptPath is validated to prevent path
traversal. Apply the same change in updateClientCredential around lines 436 to
438, passing the arguments as an array. This eliminates shell command
concatenation and improves security.
What
Summary by CodeRabbit
New Features
Enhancements
Database
Refactor & Style
Documentation
Chores