Skip to content

feat: make user's accessToken available for jobv4 - #2761

Open
belfhi wants to merge 8 commits into
SciCatProject:masterfrom
belfhi:accessToken-jobsv4
Open

feat: make user's accessToken available for jobv4#2761
belfhi wants to merge 8 commits into
SciCatProject:masterfrom
belfhi:accessToken-jobsv4

Conversation

@belfhi

@belfhi belfhi commented May 27, 2026

Copy link
Copy Markdown

Add accessToken to jobClass so it can be reused in actions via job.accessToken in handlebars

Description

the accessToken with which the Jobv4 was submitted is added to the jobClass so that in a jobConfig
you 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

Generate short-lived user JWTs for job executions and expose them to job templates to enable authenticated user-scoped actions.

New Features:

  • Provide a derived user JWT (userToken) in the job execution context so jobv4 templates can perform authenticated actions on behalf of the job owner.
  • Allow job configurations to use a dedicated JOB_TOKEN_EXPIRES_IN setting for controlling job-specific JWT lifetimes.

Bug Fixes:

  • Fix UsersService to store the correct user identifier from user identities and align JWT payload structure with JWTUser.

Enhancements:

  • Relax the JobClass results field type to SchemaTypes.Mixed for greater flexibility in stored results.
  • Add utility and tests for generating job user tokens and for using userToken within Handlebars-based job templates.

Documentation:

  • Document the JOB_TOKEN_EXPIRES_IN environment variable and its fallback behavior in the README.

Tests:

  • Add unit tests for job Handlebars utilities to ensure userToken is available in templates and behaves as expected.
  • Add unit tests for job token utility functions to cover success and failure scenarios when generating user JWTs.

@belfhi
belfhi requested a review from a team as a code owner May 27, 2026 12:40
@belfhi belfhi changed the title feat: make user's accessToken availabel for jobv4 feat: make user's accessToken available for jobv4 May 27, 2026

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread src/jobs/schemas/job.schema.ts Outdated
Comment thread src/jobs/schemas/job.schema.ts Outdated
@bpedersen2

Copy link
Copy Markdown
Contributor

Hmm, while I see the idea, maybe a different implementation would be better:

  • just store the user id
  • generate a shortlived scicat JWT token for this user on job execution and inject that instead

If we need the token to access non-scicat resources, it will get a bit more tricky as either a token exchange is needed anyway (the token audience field would not match) or maybe the new keycloak Identity Assertion JWT Grant ( see https://www.keycloak.org/2026/07/keycloak-2670-released) may later be used.

@belfhi

belfhi commented Jul 15, 2026

Copy link
Copy Markdown
Author

I like the idea of a short-lived token that is generated, that could also work. I'll investigate.
Regarding non-scicat resources, that's a very different field but also very intriguing, I hadn't even considered that yet.

@belfhi
belfhi force-pushed the accessToken-jobsv4 branch 2 times, most recently from a09498b to 1f57b82 Compare July 16, 2026 17:00
@belfhi

belfhi commented Jul 16, 2026

Copy link
Copy Markdown
Author

@bpedersen2 I changed the approach and at least in a first test it seems to work as expected. Does that look better from a safety perspective?

@bpedersen2 bpedersen2 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks better to me

@bpedersen2

Copy link
Copy Markdown
Contributor

@sourcery-ai review

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've found 1 issue, and left some high level feedback:

  • The JWT payload structure in UsersService.createUserJWT changed (using currentGroups and signing the entire JWTUser instead of a { username, groups } shape), which may break any consumers expecting the old groups field or a narrower payload; consider keeping backward-compatible field names or mapping to a minimal DTO for the token.
  • JOB_TOKEN_EXPIRES_IN is documented as required in the README but the configuration treats it as optional and falls back to JWT_EXPIRES_IN; align the env var documentation with the actual behavior so operators understand when they must set it.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The JWT payload structure in `UsersService.createUserJWT` changed (using `currentGroups` and signing the entire `JWTUser` instead of a `{ username, groups }` shape), which may break any consumers expecting the old `groups` field or a narrower payload; consider keeping backward-compatible field names or mapping to a minimal DTO for the token.
- `JOB_TOKEN_EXPIRES_IN` is documented as required in the README but the configuration treats it as optional and falls back to `JWT_EXPIRES_IN`; align the env var documentation with the actual behavior so operators understand when they must set it.

## Individual Comments

### Comment 1
<location path="src/users/users.service.ts" line_range="377-383" />
<code_context>

   async createUserJWT(
     accessToken: JWTUser | undefined,
+    expiresIn?: string,
   ): Promise<CreateUserJWT | null> {
     const expiresInOption =
-      this.configService.get<string>("jwt.expiresIn") || "1h";
+      expiresIn ||
+      this.configService.get<string>("jwt.jobTokenExpiresIn") ||
+      this.configService.get<string>("jwt.expiresIn") ||
+      "1h";
</code_context>
<issue_to_address>
**issue (bug_risk):** Using `jwt.jobTokenExpiresIn` in `createUserJWT` affects all JWTs, not just job tokens

Because `createUserJWT` now prefers `jwt.jobTokenExpiresIn`, all callers that don’t pass `expiresIn` will use the job-specific expiry instead of the general `jwt.expiresIn`. That can unintentionally shorten the lifetime of non-job JWTs that reuse this helper. To keep job-token expiry scoped to job execution only, consider:

- Leaving `createUserJWT` defaulted to `jwt.expiresIn` / `jwt.neverExpires`, and
- Having `generateJobUserToken` pass an explicit `expiresIn` for job tokens, or
- Extracting job-token creation into a separate method.

Otherwise, this change risks altering the expiration behavior of all JWTs system-wide.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread src/users/users.service.ts Outdated
@belfhi

belfhi commented Jul 17, 2026

Copy link
Copy Markdown
Author

concering the change of the payload from groups to currentGroups:

The only callers of createUserJWT are:

  • token.utils.ts (our job token generation)
  • users.controller.ts (admin endpoint /users/createUserJWT)

The users.controller.ts caller passes the request.user which is already a JWTUser from JwtStrategy.validate() — so signing the full JWTUser is fine there too.

For the payload change: the old format { username, groups } was unique to createUserJWT (the login JWT was always the full User object via auth.service.ts). Since both callers are internal and the payload is consumed by JwtStrategy.validate() which expects the full User-like shape, the new format is correct.

@belfhi

belfhi commented Jul 21, 2026

Copy link
Copy Markdown
Author

Any more comments @bpedersen2 ? Maybe you also want to take a look @sbliven ?

@belfhi
belfhi force-pushed the accessToken-jobsv4 branch from b317dcd to cb592c8 Compare July 21, 2026 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants