Add authentication for all the api requests #170
Closed
Conversation
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
… for flow-editor requests
WalkthroughRelocates Axios default configuration from src/external/index.ts to a new initializer src/services/axios.ts. Adds conditional auth header injection for Flow Editor requests via Axios interceptor and a fetch override. Ensures initialization by importing the new module in src/index.js. Exports getAuthHeaders for reuse. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as UI
participant Axios as Axios (interceptor)
participant Svc as services/axios.getAuthHeaders
participant API as Flow Editor API
Note over UI,API: Flow Editor request via Axios
UI->>Axios: axios.request(url: /flow-editor/..., config)
alt URL contains "/flow-editor"
Axios->>Svc: getAuthHeaders()
Svc-->>Axios: { Authorization: "Bearer ..." } or {}
Axios->>API: request with merged headers
API-->>Axios: response
else Other URLs
Axios->>API: request unchanged
API-->>Axios: response
end
Axios-->>UI: response
sequenceDiagram
autonumber
participant UI as UI
participant Fetch as window.fetch (overridden)
participant Svc as services/axios.getAuthHeaders
participant API as Flow Editor API
Note over UI,API: Flow Editor request via fetch
UI->>Fetch: fetch(url, init)
alt URL contains "/flow-editor"
Fetch->>Svc: getAuthHeaders()
Svc-->>Fetch: { Authorization: "Bearer ..." } or {}
Fetch->>API: request with merged headers
API-->>Fetch: response
else Other URLs
Fetch->>API: request unchanged
API-->>Fetch: response
end
Fetch-->>UI: response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/external/index.ts(0 hunks)src/index.js(1 hunks)src/services/axios.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/external/index.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/services/axios.ts (1)
src/__mocks__/axios.ts (1)
axios(3-3)
🔇 Additional comments (4)
src/services/axios.ts (3)
17-21: LGTM!The helper function correctly handles both string and URL types and performs a simple substring check for flow-editor requests.
28-44: LGTM!The Axios request interceptor correctly:
- Checks for flow-editor URLs before adding auth headers
- Merges headers without overwriting existing ones
- Handles errors appropriately
23-26: Scope and correct the global POST Content-Type header. You’re settingContent-Type: application/javascriptfor all POSTs, which is highly unusual—most APIs expectapplication/json. Either change it toapplication/jsonor limitapplication/javascriptto only the endpoints that actually require it. Confirm the FlowEditor API’s requirements.src/index.js (1)
6-6: LGTM!The side-effect import correctly initializes the Axios configuration and fetch override when the module loads, ensuring authentication headers are applied to all flow-editor requests.
Comment on lines
+3
to
+15
| export const getAuthHeaders = () => { | ||
| const glific_session = localStorage.getItem('glific_session'); | ||
| if (glific_session) { | ||
| try { | ||
| const token = JSON.parse(glific_session).access_token; | ||
| return { Authorization: token }; | ||
| } catch (error) { | ||
| console.error('Failed to parse glific_session:', error); | ||
| return {}; | ||
| } | ||
| } | ||
| return {}; | ||
| }; |
There was a problem hiding this comment.
Add "Bearer " prefix to the Authorization header.
The Authorization header should typically follow the format Bearer <token> for JWT-based authentication. The current implementation returns just the token value without the "Bearer " prefix, which may cause authentication failures.
Apply this diff to add the Bearer prefix:
export const getAuthHeaders = () => {
const glific_session = localStorage.getItem('glific_session');
if (glific_session) {
try {
const token = JSON.parse(glific_session).access_token;
- return { Authorization: token };
+ return { Authorization: `Bearer ${token}` };
} catch (error) {
console.error('Failed to parse glific_session:', error);
return {};
}
}
return {};
};🤖 Prompt for AI Agents
In src/services/axios.ts around lines 3 to 15, the getAuthHeaders function
returns the raw token instead of a Bearer token; update the returned
Authorization header to include the "Bearer " prefix (e.g., Authorization:
`Bearer ${token}`) while preserving existing JSON parsing and error handling so
that when a valid glific_session exists the header is returned as {
Authorization: `Bearer ${token}` } and on parse failure or missing session it
still returns {}.
Comment on lines
+46
to
+69
| // Intercept fetch requests globally (for requests that might use fetch) | ||
| const originalFetch = window.fetch; | ||
| window.fetch = function(input: RequestInfo | URL, init?: RequestInit): Promise<Response> { | ||
| // Only add auth headers if this is a flow-editor request | ||
| if (typeof input == 'string' && isFlowEditorRequest(input)) { | ||
| const authHeaders = getAuthHeaders(); | ||
|
|
||
| // Merge headers | ||
| const headers = { | ||
| ...authHeaders, | ||
| ...(init?.headers || {}) | ||
| }; | ||
|
|
||
| const modifiedInit = { | ||
| ...init, | ||
| headers | ||
| }; | ||
|
|
||
| return originalFetch(input, modifiedInit); | ||
| } | ||
|
|
||
| // For all other requests, use original fetch without modification | ||
| return originalFetch(input, init); | ||
| }; |
There was a problem hiding this comment.
Fix fetch override to handle all input types and Headers objects.
The current implementation has several critical issues:
- Line 50: Only checks if
inputis a string, missing URL objects that would also match the flow-editor pattern - Lines 54-57: Header merging assumes
init?.headersis a plain object, but it can be aHeadersinstance or an array of[key, value]pairs - Missing Request handling: The function doesn't handle cases where
inputis aRequestobject (which can contain its own headers and URL)
These gaps could result in auth headers not being applied or runtime errors.
Apply this diff to fix the issues:
// Intercept fetch requests globally (for requests that might use fetch)
const originalFetch = window.fetch;
window.fetch = function(input: RequestInfo | URL, init?: RequestInit): Promise<Response> {
- // Only add auth headers if this is a flow-editor request
- if (typeof input == 'string' && isFlowEditorRequest(input)) {
+ // Determine the URL for checking
+ let url: string;
+ if (typeof input === 'string') {
+ url = input;
+ } else if (input instanceof URL) {
+ url = input.toString();
+ } else if (input instanceof Request) {
+ url = input.url;
+ } else {
+ // Fallback: pass through unmodified
+ return originalFetch(input, init);
+ }
+
+ // Only add auth headers if this is a flow-editor request
+ if (isFlowEditorRequest(url)) {
const authHeaders = getAuthHeaders();
- // Merge headers
- const headers = {
- ...authHeaders,
- ...(init?.headers || {})
- };
-
- const modifiedInit = {
- ...init,
- headers
- };
+ // Merge headers - handle different header types
+ const existingHeaders = init?.headers || {};
+ const mergedHeaders = new Headers(existingHeaders);
+
+ // Add auth headers if not already present
+ Object.entries(authHeaders).forEach(([key, value]) => {
+ if (!mergedHeaders.has(key)) {
+ mergedHeaders.set(key, value as string);
+ }
+ });
+
+ const modifiedInit = {
+ ...init,
+ headers: mergedHeaders
+ };
return originalFetch(input, modifiedInit);
}
// For all other requests, use original fetch without modification
return originalFetch(input, init);
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Intercept fetch requests globally (for requests that might use fetch) | |
| const originalFetch = window.fetch; | |
| window.fetch = function(input: RequestInfo | URL, init?: RequestInit): Promise<Response> { | |
| // Only add auth headers if this is a flow-editor request | |
| if (typeof input == 'string' && isFlowEditorRequest(input)) { | |
| const authHeaders = getAuthHeaders(); | |
| // Merge headers | |
| const headers = { | |
| ...authHeaders, | |
| ...(init?.headers || {}) | |
| }; | |
| const modifiedInit = { | |
| ...init, | |
| headers | |
| }; | |
| return originalFetch(input, modifiedInit); | |
| } | |
| // For all other requests, use original fetch without modification | |
| return originalFetch(input, init); | |
| }; | |
| // Intercept fetch requests globally (for requests that might use fetch) | |
| const originalFetch = window.fetch; | |
| window.fetch = function(input: RequestInfo | URL, init?: RequestInit): Promise<Response> { | |
| // Determine the URL for checking | |
| let url: string; | |
| if (typeof input === 'string') { | |
| url = input; | |
| } else if (input instanceof URL) { | |
| url = input.toString(); | |
| } else if (input instanceof Request) { | |
| url = input.url; | |
| } else { | |
| // Fallback: pass through unmodified | |
| return originalFetch(input, init); | |
| } | |
| // Only add auth headers if this is a flow-editor request | |
| if (isFlowEditorRequest(url)) { | |
| const authHeaders = getAuthHeaders(); | |
| // Merge headers - handle different header types | |
| const existingHeaders = init?.headers || {}; | |
| const mergedHeaders = new Headers(existingHeaders); | |
| // Add auth headers if not already present | |
| Object.entries(authHeaders).forEach(([key, value]) => { | |
| if (!mergedHeaders.has(key)) { | |
| mergedHeaders.set(key, value as string); | |
| } | |
| }); | |
| const modifiedInit = { | |
| ...init, | |
| headers: mergedHeaders | |
| }; | |
| return originalFetch(input, modifiedInit); | |
| } | |
| // For all other requests, use original fetch without modification | |
| return originalFetch(input, init); | |
| }; |
shijithkjayan
approved these changes
Oct 10, 2025
|
Without activity, this PR will be closed in 14 days. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
closes glific/glific-frontend#3547
Summary by CodeRabbit