feat: move api key fetching to use api key service

[elibosley]

Summary by CodeRabbit

• Refactor

  • Simplified and streamlined the management of the local Connect API key, renaming it to "ConnectInternal" and removing legacy keys.
  • Updated internal logic to directly retrieve or create the Connect API key without storing or emitting it in configuration or events.
  • Replaced custom WebSocket handling with default implementation and improved asynchronous API key retrieval for client connections.
  • Enhanced asynchronous handling for subscription and query execution to ensure proper client initialization.

• Chores

  • Removed obsolete methods and test suites related to previous Connect API key management logic.

[coderabbitai]

Walkthrough

The changes remove legacy logic for automatically creating or managing a "Connect" local API key in the core service, shifting this responsibility to the Connect plugin. The Connect plugin now uses a new key name, simplifies key management, and updates dependencies accordingly. Related tests and obsolete helper methods are also removed.

Changes

File(s)	Change Summary
api/src/unraid-api/auth/api-key.service.ts, api-key.service.spec.ts	Remove logic and tests for auto-creating "Connect" local API key; delete related methods and initialization.
packages/unraid-api-plugin-connect/src/service/connect-api-key.service.ts	Rename Connect API key, simplify retrieval/creation logic, remove legacy key handling, update descriptions.
packages/unraid-api-plugin-connect/src/service/connect-settings.service.ts	Remove private helper for local key creation; update sign-in flow to use new service method directly.
packages/unraid-api-plugin-connect/src/service/internal.client.ts	Replace dependency, update client creation to use async key retrieval, simplify connection params.
packages/unraid-api-plugin-connect/src/service/mothership-subscription.handler.ts	Make subscription methods async; await async client retrieval before proceeding.
packages/unraid-api-plugin-connect/src/module/mothership.module.ts	Add ConnectApiKeyService to providers.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant ConnectSettingsService
    participant ConnectApiKeyService

    User->>ConnectSettingsService: signIn()
    ConnectSettingsService->>ConnectApiKeyService: getOrCreateLocalApiKey()
    ConnectApiKeyService->>ConnectApiKeyService: Remove legacy keys (if any)
    ConnectApiKeyService->>ConnectApiKeyService: Find/Create 'ConnectInternal' key
    ConnectApiKeyService-->>ConnectSettingsService: Return API key
    ConnectSettingsService-->>User: Complete sign-in

Loading

sequenceDiagram
    participant InternalClientService
    participant ConnectApiKeyService

    InternalClientService->>ConnectApiKeyService: getOrCreateLocalApiKey()
    ConnectApiKeyService-->>InternalClientService: Return API key
    InternalClientService->>InternalClientService: Create GraphQL client using API key

Loading

Possibly related PRs

• feat: generate key one time #992: Introduced initial local Connect API key creation and configuration management, which is reversed or removed by this PR.

Suggested reviewers

• pujitm
• mdatelle
• zackspear

Poem

Out with the old, in with the new,
"Connect_Internal" now shines through!
Legacy keys are swept away,
Simpler logic leads the way.
Async calls and cleaner code,
Onward, Connect plugin—let's hit the road! 🚀✨

---

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c14cd4c and b928902.

📒 Files selected for processing (1)

• packages/unraid-api-plugin-connect/src/service/connect-api-key.service.ts (3 hunks)

🧰 Additional context used

🧠 Learnings (2)

📓 Common learnings

Learnt from: elibosley
PR: unraid/api#1425
File: api/dev/configs/connect.json:5-13
Timestamp: 2025-06-20T20:50:14.050Z
Learning: Connect configuration files like api/dev/configs/connect.json use their own naming conventions (e.g., apikey, accesstoken, idtoken, refreshtoken) as implementation details, and these should not be changed to follow general camelCase patterns.

packages/unraid-api-plugin-connect/src/service/connect-api-key.service.ts (12)

Learnt from: elibosley
PR: unraid/api#1425
File: api/dev/configs/connect.json:5-13
Timestamp: 2025-06-20T20:50:14.050Z
Learning: Connect configuration files like api/dev/configs/connect.json use their own naming conventions (e.g., apikey, accesstoken, idtoken, refreshtoken) as implementation details, and these should not be changed to follow general camelCase patterns.

Learnt from: elibosley
PR: unraid/api#942
File: api/src/unraid-api/auth/api-key.service.ts:176-188
Timestamp: 2024-11-15T16:22:03.485Z
Learning: Atomic writes are not required for the `saveApiKey` method in `ApiKeyService` (`api/src/unraid-api/auth/api-key.service.ts`) unless specifically needed.

Learnt from: elibosley
PR: unraid/api#942
File: api/src/unraid-api/auth/api-key.service.ts:62-70
Timestamp: 2024-11-05T14:49:07.308Z
Learning: In `api/src/unraid-api/auth/api-key.service.ts`, when handling read errors in the `findById` method, throw a `GraphQLError` instead of an `InternalServerErrorException`.

Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/auth/auth.service.ts:0-0
Timestamp: 2024-11-04T20:44:46.432Z
Learning: When modifying `apiKey.roles` in `removeRoleFromApiKey` and `addRoleToApiKey` within `api/src/unraid-api/auth/auth.service.ts`, concurrency issues are not a concern because the keys are stored in the file system.

Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/auth/auth.service.ts:0-0
Timestamp: 2024-11-04T20:41:22.303Z
Learning: In `api/src/unraid-api/auth/auth.service.ts`, the `addRoleToApiKey` function operates on API keys stored as JSON files in a directory, not a database, so concurrency is not a concern for modifying `apiKey.roles`.

Learnt from: elibosley
PR: unraid/api#992
File: api/src/mothership/utils/create-local-connect-api-key.ts:23-29
Timestamp: 2024-12-20T14:56:29.464Z
Learning: The user prefers not to add error handling for the Redux dispatch in create-local-connect-api-key.ts.

Learnt from: pujitm
PR: unraid/api#1211
File: web/codegen.ts:14-14
Timestamp: 2025-03-12T13:35:43.900Z
Learning: The JSON scalar type in web/codegen.ts was temporarily changed from 'string' to 'any' for compatibility with JsonForms integration. This change facilitates the implementation of the Connect settings web component.

Learnt from: pujitm
PR: unraid/api#1211
File: api/src/graphql/schema/types/connect/connect.graphql:142-146
Timestamp: 2025-03-14T16:10:38.562Z
Learning: The updateApiSettings mutation in api/src/unraid-api/graph/connect/connect.resolver.ts is protected with the @UsePermissions decorator that requires UPDATE permission on the CONFIG resource.

Learnt from: pujitm
PR: unraid/api#1367
File: packages/unraid-api-plugin-connect/src/pubsub/user.service.ts:44-52
Timestamp: 2025-04-23T20:19:42.542Z
Learning: The project uses a custom or extended implementation of NestJS ConfigService that includes a `set()` method for runtime configuration mutation, unlike the standard @nestjs/config package which only provides getter methods.

Learnt from: pujitm
PR: unraid/api#1367
File: packages/unraid-api-plugin-connect/src/pubsub/user.service.ts:44-52
Timestamp: 2025-04-23T20:19:42.542Z
Learning: The project uses a custom ConfigService implementation that includes a `set()` method for runtime configuration mutation, unlike the standard @nestjs/config package which only provides getter methods.

Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts:111-113
Timestamp: 2024-11-06T20:59:25.809Z
Learning: In the Unraid API project, error handling for mutations is handled at the service level rather than in the GraphQL resolvers. Specifically, in `api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts`, methods like `removeRoleFromApiKey` rely on service-level error handling.

Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/auth/auth.service.ts:42-105
Timestamp: 2024-11-04T22:00:41.946Z
Learning: In the 'AuthService' within `api/src/unraid-api/auth/auth.service.ts`, rate limiting is not required because the API is not public facing.

⏰ Context from checks skipped due to timeout of 90000ms (4)

• GitHub Check: Build API
• GitHub Check: Test API
• GitHub Check: Build Web App
• GitHub Check: Analyze (javascript-typescript)

🔇 Additional comments (3)

packages/unraid-api-plugin-connect/src/service/connect-api-key.service.ts (3)

11-13: Well-structured constant extraction and naming update.

The renaming from 'Connect' to 'ConnectInternal' clearly distinguishes the new key from legacy ones, and extracting the description as a static property improves maintainability.

---

89-93: Effective legacy key cleanup implementation.

The logic correctly identifies and removes legacy 'Connect' keys before proceeding with the new key management. The logging provides good visibility into the migration process.

---

86-107: Simplified and robust key management logic.

The refactored method follows a clear pattern: cleanup legacy keys, check for existing new key, create if needed. The error handling is appropriate and the overall flow is much cleaner than the previous multi-step approach mentioned in the summary.

✨ Finishing Touches

• 📝 Generate Docstrings

---

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Explain this complex logic.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai explain this code block.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and explain its main purpose.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai generate docstrings to generate docstrings for this PR.
• @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

packages/unraid-api-plugin-connect/src/service/connect-api-key.service.ts (1)

86-107: Consider optimizing the migration strategy.

The refactored method handles the migration from legacy 'Connect' keys to 'Connect_Internal' keys well. However, there are performance considerations:

1. Fetching all API keys on every call could be inefficient with a large number of keys
2. The legacy key cleanup runs on every invocation rather than being a one-time migration

Consider adding a flag or timestamp to track when migration is complete, or implement a more targeted query approach.

public async getOrCreateLocalApiKey(): Promise<string> {
-   const allKeys = await this.findAll();
-   
-   const legacyConnectKeys = allKeys.filter((key) => key.name === 'Connect');
-   if (legacyConnectKeys.length > 0) {
-       await this.deleteApiKeys(legacyConnectKeys.map((key) => key.id));
-       this.logger.log(`Deleted legacy Connect API keys`);
-   }
+   // Check for existing Connect_Internal key first
+   const connectKey = this.findByField('name', ConnectApiKeyService.CONNECT_API_KEY_NAME);
+   if (connectKey) {
+       return connectKey.key;
+   }
+   
+   // Handle legacy migration only when needed
+   const legacyKey = this.findByField('name', 'Connect');
+   if (legacyKey) {
+       await this.deleteApiKeys([legacyKey.id]);
+       this.logger.log(`Deleted legacy Connect API key during migration`);
+   }

-   const connectKey = this.findByField('name', ConnectApiKeyService.CONNECT_API_KEY_NAME);
-   if (connectKey) {
-       return connectKey.key;
-   }

    const localApiKey = await this.createLocalConnectApiKey();

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1a33bfe and 895ac22.

📒 Files selected for processing (2)

• packages/unraid-api-plugin-connect/src/module/mothership.module.ts (2 hunks)
• packages/unraid-api-plugin-connect/src/service/connect-api-key.service.ts (3 hunks)

✅ Files skipped from review due to trivial changes (1)

• packages/unraid-api-plugin-connect/src/module/mothership.module.ts

🔇 Additional comments (3)

packages/unraid-api-plugin-connect/src/service/connect-api-key.service.ts (3)

3-3: LGTM!

The additional imports are necessary for the enhanced type safety in the method signatures.

---

11-13: Good refactoring for maintainability.

The rename from 'Connect' to 'Connect_Internal' provides better clarity, and extracting the description into a constant improves maintainability and consistency.

---

73-73: LGTM!

Using the extracted constant maintains consistency with the other changes.

[github-actions]

This plugin has been deployed to Cloudflare R2 and is available for testing.
Download it at this URL:

https://preview.dl.unraid.net/unraid-api/tag/PR1439/dynamix.unraid.net.plg