feat: make user's accessToken available for jobv4#2761
Conversation
…cessToken in handlebars
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- Storing the raw JWT in the Job document is sensitive; consider either not persisting it (e.g., keep it only in memory/context) or at least marking the field
select: false/encrypting it so it cannot be read back from the database by default. - The
toJSON.transformhook will not protectaccessTokenforlean()queries or other serialization paths (e.g.,toObject()), so if the token must never be exposed externally you may want to add additional safeguards (e.g., schema-levelselect: false, explicit projections, or a dedicated DTO layer) to guarantee it never leaks.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Storing the raw JWT in the Job document is sensitive; consider either not persisting it (e.g., keep it only in memory/context) or at least marking the field `select: false`/encrypting it so it cannot be read back from the database by default.
- The `toJSON.transform` hook will not protect `accessToken` for `lean()` queries or other serialization paths (e.g., `toObject()`), so if the token must never be exposed externally you may want to add additional safeguards (e.g., schema-level `select: false`, explicit projections, or a dedicated DTO layer) to guarantee it never leaks.
## Individual Comments
### Comment 1
<location path="src/jobs/schemas/job.schema.ts" line_range="15-17" />
<code_context>
timestamps: true,
toJSON: {
getters: true,
+ transform: (_doc: Document, ret: Record<string, unknown>) => {
+ delete ret.accessToken;
+ return ret;
+ },
},
</code_context>
<issue_to_address>
**🚨 suggestion (security):** Current hiding of `accessToken` only covers `toJSON`; consider also protecting it at query/serialization level.
This only protects API responses using `toJSON`; the field is still included by default in queries and in `toObject()` results. If `accessToken` is sensitive, also mark it as non-selectable (e.g. `select: false`) or use an equivalent mechanism so it can’t be exposed via other serialization paths that bypass this transform.
Suggested implementation:
```typescript
timestamps: true,
toJSON: {
getters: true,
transform: (_doc: Document, ret: Record<string, unknown>) => {
delete ret.accessToken;
return ret;
},
},
toObject: {
getters: true,
transform: (_doc: Document, ret: Record<string, unknown>) => {
delete ret.accessToken;
return ret;
},
},
})
export class JobClass extends OwnableClass {
```
To fully implement the suggestion and protect `accessToken` at the query level, you should also mark the `accessToken` field as non-selectable in its `@Prop` definition. For example, if the field currently looks like:
```ts
@Prop()
accessToken: string;
```
you should change it to:
```ts
@Prop({ select: false })
accessToken: string;
```
or, if there are already options:
```ts
@Prop({ type: String, select: false })
accessToken: string;
```
This ensures `accessToken` is excluded by default from query results and all serialization paths, while still allowing explicit inclusion via `.select('+accessToken')` when needed.
</issue_to_address>
### Comment 2
<location path="src/jobs/schemas/job.schema.ts" line_range="119-123" />
<code_context>
+ * Stored for reuse by actions performed within the job.
+ * Not exposed in API responses for security reasons.
+ */
+ @Prop({
+ type: String,
+ required: false,
+ })
+ accessToken?: string;
}
export const JobSchema = SchemaFactory.createForClass(JobClass);
</code_context>
<issue_to_address>
**🚨 issue (security):** Storing raw JWTs in the database may not be necessary and increases the blast radius of a DB compromise.
Persisting the full access token means a DB leak exposes reusable credentials until expiry. If you only need it to call downstream services, consider storing a less-sensitive representation (e.g. minimal claims or a reference/ID) or shortening its lifetime. If the full token must be stored, consider additional at-rest protection for this field (e.g. encryption).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| transform: (_doc: Document, ret: Record<string, unknown>) => { | ||
| delete ret.accessToken; | ||
| return ret; |
There was a problem hiding this comment.
🚨 suggestion (security): Current hiding of accessToken only covers toJSON; consider also protecting it at query/serialization level.
This only protects API responses using toJSON; the field is still included by default in queries and in toObject() results. If accessToken is sensitive, also mark it as non-selectable (e.g. select: false) or use an equivalent mechanism so it can’t be exposed via other serialization paths that bypass this transform.
Suggested implementation:
timestamps: true,
toJSON: {
getters: true,
transform: (_doc: Document, ret: Record<string, unknown>) => {
delete ret.accessToken;
return ret;
},
},
toObject: {
getters: true,
transform: (_doc: Document, ret: Record<string, unknown>) => {
delete ret.accessToken;
return ret;
},
},
})
export class JobClass extends OwnableClass {To fully implement the suggestion and protect accessToken at the query level, you should also mark the accessToken field as non-selectable in its @Prop definition. For example, if the field currently looks like:
@Prop()
accessToken: string;you should change it to:
@Prop({ select: false })
accessToken: string;or, if there are already options:
@Prop({ type: String, select: false })
accessToken: string;This ensures accessToken is excluded by default from query results and all serialization paths, while still allowing explicit inclusion via .select('+accessToken') when needed.
| @Prop({ | ||
| type: String, | ||
| required: false, | ||
| }) | ||
| accessToken?: string; |
There was a problem hiding this comment.
🚨 issue (security): Storing raw JWTs in the database may not be necessary and increases the blast radius of a DB compromise.
Persisting the full access token means a DB leak exposes reusable credentials until expiry. If you only need it to call downstream services, consider storing a less-sensitive representation (e.g. minimal claims or a reference/ID) or shortening its lifetime. If the full token must be stored, consider additional at-rest protection for this field (e.g. encryption).
Add accessToken to jobClass so it can be reused in actions via job.accessToken in handlebars
Description
the
accessTokenwith which the Jobv4 was submitted is added to the jobClass so that in a jobConfigyou can use
authorization: "Bearer {{job.accessToken}}"for authorization.This is especially useful if the job action performs a Scicat API call as the user which is not possible otherwise.
Motivation
configurable actions and jobs using urlactoin make it possible to perform arbitrary API calls, and with the proposed changes these calls can be done as the user that is currently logged in a submits a job to scicat.
Summary by Sourcery
Store the submitting user's access token on Job v4 instances for reuse by actions while ensuring it is not exposed in API responses.
New Features:
Enhancements: