Skip to content

bug: remove refresh endpoint and require auth#6256

Draft
chloebyun-wd wants to merge 1 commit intomainfrom
bug/524/unauthenticated-oauth-refresh-token
Draft

bug: remove refresh endpoint and require auth#6256
chloebyun-wd wants to merge 1 commit intomainfrom
bug/524/unauthenticated-oauth-refresh-token

Conversation

@chloebyun-wd
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request secures the OAuth2 token refresh endpoint by adding an internal header authentication check and removing sensitive token data from the response. However, the implementation should use the getEncryptionKey utility for the authentication header to ensure compatibility with various secret storage backends. Additionally, the endpoint must remain in the whitelist to prevent global authentication middleware from blocking internal requests before the header check is performed.

headers: {
'Content-Type': 'application/json'
'Content-Type': 'application/json',
'x-flowise-internal-key': process.env.FLOWISE_SECRETKEY_OVERWRITE || ''
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.

high

The internal authentication header should use the actual encryption key retrieved via getEncryptionKey(). Relying solely on process.env.FLOWISE_SECRETKEY_OVERWRITE will cause authentication failures (401 Unauthorized) if the key is stored in a file or AWS Secrets Manager, as getEncryptionKey() handles these fallbacks while the current implementation defaults to an empty string when the environment variable is unset.

                            'x-flowise-internal-key': await getEncryptionKey()
References
  1. When a feature requires a specific configuration like an API key or secret, it is preferable to use the established retrieval mechanism that handles environment fallbacks or throws errors if missing, rather than relying on a single environment variable.

'/api/v1/pricing',
'/api/v1/user/test',
'/api/v1/oauth2-credential/callback',
'/api/v1/oauth2-credential/refresh',
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.

high

Removing /api/v1/oauth2-credential/refresh from WHITELIST_URLS will likely break the internal token refresh mechanism. The global authentication middleware typically intercepts requests to non-whitelisted endpoints. Since the server-side call in refreshOAuth2Token does not include user session cookies or an API key, it will be rejected with a 401 error before reaching the route-level internal key check. To allow internal authentication while preventing public access, this endpoint should remain in the whitelist.

References
  1. Bypassing user-based authorization is acceptable for API endpoints that are intentionally public or whitelisted for internal flows, provided they do not expose sensitive data or allow privilege escalation.
  2. Internal endpoints might have different authentication flows compared to external API endpoints; ensure the correct middleware or whitelist status allows these internal calls to succeed.

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.

1 participant