fix(auth): encrypt OAuth tokens using encryption utility directly#144
Open
Ridanshi wants to merge 1 commit into
Open
fix(auth): encrypt OAuth tokens using encryption utility directly#144Ridanshi wants to merge 1 commit into
Ridanshi wants to merge 1 commit into
Conversation
Harxhit
added
the
gssoc:approved
Contributor
|
@Ridanshi Please fix the merge conflicts. |
Author
|
Thanks for the review. I’m currently resolving the merge conflicts in |
auth.ts silently stored GitHub OAuth access tokens as plaintext because the encryption check relied on a non-existent `app.encryption` Fastify decorator - the condition always evaluated false, falling back to the raw token. connect.ts called `app.encryption.encrypt()` directly, throwing a TypeError at runtime and breaking the GitHub connect flow entirely. Both routes now import `encrypt()` directly from utils/encryption.ts, consistent with how follow.ts already imports `decrypt()` from the same module.
Ridanshi
force-pushed
the
fix/oauth-token-encryption
branch
from
83aae1c to
f2c3273
Compare
Author
|
Resolved the merge conflicts by rebasing onto the latest upstream
The PR should now be mergeable. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
auth.tswas silently storing GitHub OAuth access tokens as plaintext.The encryption check tested
(app as any).encryption, which is neverregistered as a Fastify decorator, so the condition always evaluated false
and the raw token was written to the database.
connect.tscalledapp.encryption.encrypt()directly. Since thatproperty does not exist, it caused a runtime
TypeErrorand broke theGitHub platform connection flow.
Both routes now import and use
encrypt()directly fromutils/encryption.ts, consistent with the existing pattern already usedin
follow.ts.Closes #126
Changes Made
apps/backend/src/routes/auth.tsencryptdirectly fromutils/encryption.tsapps/backend/src/routes/connect.tsencryptdirectly fromutils/encryption.tsapp.encryption.encrypt()usage with direct utility-based encryptionImpact
Security
Ensures GitHub OAuth access tokens are encrypted before database persistence.
Reliability
Fixes runtime crashes in the GitHub account connection flow.
Consistency
Aligns route behavior with the existing encryption/decryption utility pattern already used in
follow.ts.Test Plan
OAuthToken.accessTokenTypeErrorENCRYPTION_KEYis configured correctly in environment variablesNotes
This PR intentionally keeps the implementation:
No architectural refactors or unrelated changes were introduced.