feat: add Discord service verification and token generation #2419
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Summary by CodeRabbit
WalkthroughA new constant for the Discord service was introduced and integrated into the bot constants. The bot authorization middleware was updated to check for a specific service header and use a new Discord service token verification function when appropriate. This function was implemented to validate JWTs using a dedicated public key. Test configuration was extended to include Discord service RSA keys, and a utility was added to generate Discord service tokens. The middleware's test suite was expanded with cases for Discord service authorization, ensuring correct handling of valid and invalid tokens. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Middleware
participant VerificationService
Client->>Middleware: Request with Authorization token and x-service-name
alt x-service-name == DISCORD_SERVICE
Middleware->>VerificationService: verifyDiscordService(token)
VerificationService-->>Middleware: Verification result
alt name == DISCORD_SERVICE
Middleware->>Client: Proceed (next middleware)
else name mismatch
Middleware->>Client: Unauthorized response
end
else
Middleware->>VerificationService: verifyToken(token, CLOUDFLARE_WORKER)
VerificationService-->>Middleware: Verification result
alt valid
Middleware->>Client: Proceed (next middleware)
else invalid
Middleware->>Client: Unauthorized response
end
end
Possibly related issues
Poem
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 (
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (2)
services/botVerificationService.js (1)
1-1: 💡 Verification agent🧩 Analysis chain
Verify config module import is present.
The
configobject is used in the verification functions, but I don't see an import statement for it in this file.Please verify that the config module is properly imported or globally available. You can check this with:
🏁 Script executed:
#!/bin/bash # Check if config is properly imported in the file grep -n "const config" services/botVerificationService.js || echo "config import missing" # Check if it might be imported with a different name grep -n "require.*config" services/botVerificationService.jsLength of output: 175
Add missing config import in services/botVerificationService.js
The
configobject is referenced in your verification functions but isn’t imported in this file. Please add the appropriate import at the top ofservices/botVerificationService.js:const config = require('config');• Location: services/botVerificationService.js — insert as the first line after other
requirestatements.test/utils/generateBotToken.js (1)
1-1: 💡 Verification agent🧩 Analysis chain
Verify config module import is present.
Similar to the verification service, the
configobject is used but I don't see an import statement for it.Please verify that the config module is properly imported. You can check this with:
🏁 Script executed:
#!/bin/bash # Check if config is properly imported in the file grep -n "const config" test/utils/generateBotToken.js || echo "config import missing" # Check if it might be imported with a different name grep -n "require.*config" test/utils/generateBotToken.jsLength of output: 167
We still need to find how other modules import the
configobject to suggest the correct path. Let’s search across the repo for existingconfigimports:
🏁 Script executed:
#!/bin/bash # Search for all import/require statements that bring in the config module grep -R "require.*config" -n .Length of output: 69708
Add missing config import in generateBotToken.js
The
configobject is used intest/utils/generateBotToken.jsbut never imported. Please add the standard import at the top of the file.• File: test/utils/generateBotToken.js
• Location: immediately after the JWT importSuggested diff:
const jwt = require("jsonwebtoken"); +const config = require("config");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
constants/bot.ts(1 hunks)middlewares/authorizeBot.js(2 hunks)services/botVerificationService.js(2 hunks)test/config/test.js(1 hunks)test/unit/middlewares/authorizeBot.test.js(2 hunks)test/utils/generateBotToken.js(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: build (22.10.0)
🔇 Additional comments (14)
constants/bot.ts (2)
4-4: LGTM: New constant for Discord service added.The addition of this constant aligns well with existing naming conventions and will serve as the identifier for the Discord service authorization flow.
11-11: LGTM: Export updated to include new constant.The module exports have been correctly updated to include the new
DISCORD_SERVICEconstant.test/config/test.js (1)
156-195:✅ Verification successful
Security consideration: Discord service is using identical keys to bot tokens.
I notice that the RSA keys for the Discord service appear to be identical to those used for
botToken. For proper security isolation between services, each service should have its own unique keys in a production environment.If this is only for testing purposes, that's acceptable, but please ensure production uses unique keys. You can verify the key differences by checking:
🏁 Script executed:
#!/bin/bash # Compare bot token keys and Discord service keys to ensure they're different in production echo "Checking if bot and Discord service keys are different..." # Extract keys from environment variables or config files in production # This is a simplified example - adjust to your actual production config retrieval method BOT_PUBLIC_KEY=$(grep -A 10 "botToken" /path/to/production/config.js | grep "botPublicKey" | head -1) DISCORD_PUBLIC_KEY=$(grep -A 10 "discordServiceToken" /path/to/production/config.js | grep "discordServicePublicKey" | head -1) if [ "$BOT_PUBLIC_KEY" == "$DISCORD_PUBLIC_KEY" ]; then echo "WARNING: Bot and Discord service are using identical keys in production" else echo "Keys are different as expected" fiLength of output: 654
🏁 Script executed:
#!/bin/bash echo "Comparing test botToken and discordServiceToken keys in test/config/test.js..." # Extract botToken public key line BOT_PUB=$(grep -A5 "botToken" test/config/test.js | grep "botPublicKey" | sed 's/.*"\(.*\)".*/\1/') # Extract discordServiceToken public key line DISCORD_PUB=$(grep -A5 "discordServiceToken" test/config/test.js | grep "discordServicePublicKey" | sed 's/.*"\(.*\)".*/\1/') echo "BOT_PUBLIC_KEY snippet: ${BOT_PUB:0:40}…" echo "DISCORD_PUBLIC_KEY snippet: ${DISCORD_PUB:0:40}…" if [ "$BOT_PUB" == "$DISCORD_PUB" ]; then echo "❗ Public keys are identical in test config" else echo "✅ Public keys differ in test config" fi echo # Extract botToken private key line BOT_PRIV=$(grep -A5 "botToken" test/config/test.js | grep "botPrivateKey" | sed 's/.*"\(.*\)".*/\1/') # Extract discordServiceToken private key line DISCORD_PRIV=$(grep -A5 "discordServiceToken" test/config/test.js | grep "discordServicePrivateKey" | sed 's/.*"\(.*\)".*/\1/') echo "BOT_PRIVATE_KEY snippet: ${BOT_PRIV:0:40}…" echo "DISCORD_PRIVATE_KEY snippet: ${DISCORD_PRIV:0:40}…" if [ "$BOT_PRIV" == "$DISCORD_PRIV" ]; then echo "❗ Private keys are identical in test config" else echo "✅ Private keys differ in test config" fiLength of output: 1298
🏁 Script executed:
#!/bin/bash echo "Extracting and comparing full key blocks for botToken vs discordServiceToken..." # Extract bot public key block sed -n -e '/botPublicKey:/,/END PUBLIC KEY-----/p' test/config/test.js > /tmp/bot_pub.pem # Extract discord public key block sed -n -e '/discordServicePublicKey:/,/END PUBLIC KEY-----/p' test/config/test.js > /tmp/discord_pub.pem echo "Public key differences:" diff -u /tmp/bot_pub.pem /tmp/discord_pub.pem || echo "❌ Public keys differ" echo # Extract bot private key block sed -n -e '/botPrivateKey:/,/END RSA PRIVATE KEY-----/p' test/config/test.js > /tmp/bot_priv.pem # Extract discord private key block sed -n -e '/discordServicePrivateKey:/,/END RSA PRIVATE KEY-----/p' test/config/test.js > /tmp/discord_priv.pem echo "Private key differences:" diff -u /tmp/bot_priv.pem /tmp/discord_priv.pem || echo "❌ Private keys differ"Length of output: 1587
Security isolation (production only)
✅ I confirmed that in test/config/test.js the RSA key blocks for botToken and discordServiceToken are in fact identical, meaning they share the same key pair in your test setup. This is acceptable for a testing environment, but please ensure that in your production configuration you generate and assign a unique public/private key pair for each service (botToken vs. discordServiceToken).
services/botVerificationService.js (2)
13-21: LGTM: Well-structured Discord service token verification function.The implementation follows the same pattern as existing verification functions and properly uses the Discord service public key from config.
33-33: LGTM: Export updated correctly.The module exports have been updated to include the new verification function.
test/utils/generateBotToken.js (2)
16-27: LGTM: Well-implemented token generation function for Discord service.The implementation follows the same pattern as existing token generation functions, with consistent naming, algorithm, and expiration time.
37-37: LGTM: Export updated correctly.The module exports have been updated to include the new token generation function.
middlewares/authorizeBot.js (1)
2-2: Good addition of the DISCORD_SERVICE constant importThe import of the new constant aligns with the PR objective of adding Discord service verification support.
test/unit/middlewares/authorizeBot.test.js (6)
5-6: Good addition of required importsThe addition of the jwt library import and DISCORD_SERVICE constant is appropriate for the new test cases.
120-145: Well-structured test for expired/malformed Discord service tokensThis test case effectively verifies that the middleware properly handles expired or malformed tokens for the Discord service path. Good use of sinon stubs to simulate the jwt.verify error and proper cleanup with restore().
147-165: Good coverage of invalid token scenarioThis test properly verifies that the middleware rejects invalid tokens coming from the Discord service and responds with a bad request.
167-181: Complete verification of the happy path scenarioThis test correctly verifies that valid Discord service tokens are properly authorized by the middleware.
199-213: Good negative test case for Discord service name validationThis test effectively validates that the middleware checks the token's name claim matches DISCORD_SERVICE, rejecting tokens with incorrect name values.
215-229: Comprehensive coverage of Cloudflare worker pathThis test ensures that the original Cloudflare worker authorization path still validates the name field correctly, maintaining backward compatibility while adding the new Discord service feature.
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
Can you confirm what additional value integration tests add here, which is not covered by unit tests? We would be able to verify the api behaviour with discord-service token.
The header name itself acts as a feature flag. |
pankajjs
force-pushed
the
feat/discord-service-auth
branch
from
f5d6040 to
b8a4db4
Compare
|
@pankajjs do we need to make any changes in the readme to add these keys or not ? |
|
LGTM! |
pankajjs
force-pushed
the
feat/discord-service-auth
branch
from
8af484e to
88246d6
Compare
pankajjs
force-pushed
the
feat/discord-service-auth
branch
from
88246d6 to
d0e5da0
Compare
pankajjs
force-pushed
the
feat/discord-service-auth
branch
from
754226f to
b447abd
Compare
Date: 3 May 2025
Developer Name: Joy Gupta, @pankajjs
Issue Ticket Number
Description
This PR contains an update to the logic of
verifyDiscordBot. With these changes, the same middleware could authorize the requests from both the Discord service and the Discord slash.Documentation Updated?
Under Feature Flag
Database Changes
Breaking Changes
Development Tested?
Screenshots
Screenshot 1
screen-capture.1.webm
Test Coverage
Description by Korbit AI
What change is being made?
Add Discord service verification and token generation functionality, including updates to authorization middleware, new test cases, and configuration enhancements for handling Discord Service JWTs.
Why are these changes being made?
To implement authentication for Discord service interactions by validating incoming requests through JWTs, ensuring only authorized Discord services can access protected resources. This approach enhances security by expanding the authorization mechanism to support Discord service communications.