Issueid #226738 fix: Score not updating for speak with me section if … #160
Conversation
…we load the assets from cloudfront
WalkthroughThe changes involve updates to the Changes
Sequence Diagram(s)sequenceDiagram
participant A as Component
participant B as Window
A->>B: postMessage(data, "*")
B-->>A: Acknowledgment
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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
Documentation and Community
|
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @@ -344,7 +344,7 @@ | |||
| const handleProfileBack = () => { | |||
| try { | |||
| if (process.env.REACT_APP_IS_APP_IFRAME === "true") { | |||
| window.parent.postMessage({ type: "restore-iframe-content" }); | |||
| window.parent.postMessage({ type: "restore-iframe-content" }, "*"); | |||
Check failure
Code scanning / SonarCloud
Origins should be verified during cross-origin communications High
| message: "all-test-rig-score", | ||
| }); | ||
| if (process.env.REACT_APP_IS_APP_IFRAME === "true") { | ||
| window.parent.postMessage( |
Check failure
Code scanning / SonarCloud
Origins should be verified during cross-origin communications High
| score: score, | ||
| message: "all-test-rig-score", | ||
| }); | ||
| window.parent.postMessage( |
Check failure
Code scanning / SonarCloud
Origins should be verified during cross-origin communications High
| @@ -683,7 +686,7 @@ | |||
| questions[currentQuestion]?.contentSourceData || []; | |||
| const stringLengths = contentSourceData.map((item) => item.text.length); | |||
| const length = stringLengths[0]; | |||
| window.parent.postMessage({ type: "stringLengths", length }); | |||
| window.parent.postMessage({ type: "stringLengths", length }, "*"); | |||
Check failure
Code scanning / SonarCloud
Origins should be verified during cross-origin communications High
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Outside diff range and nitpick comments (2)
src/components/DiscoverSentance/DiscoverSentance.jsx (2)
170-180: Add error handling for the new API call.The new API call to create learner progress looks good and is correctly gated behind the
REACT_APP_POST_LEARNER_PROGRESSenvironment variable.Consider adding error handling for this API call. If the API call fails, it should be caught and handled gracefully to avoid any disruption to the user flow. For example:
try { await axios.post( `${process.env.REACT_APP_LEARNER_AI_ORCHESTRATION_HOST}/${config.URLS.CREATE_LEARNER_PROGRESS}`, { userId: localStorage.getItem("virtualId"), sessionId: localStorage.getItem("sessionId"), subSessionId: sub_session_id, milestoneLevel: getSetData?.data?.currentLevel, language: localStorage.getItem("lang"), } ); } catch (error) { console.error("Failed to create learner progress:", error); // Handle the error, show a user-friendly message, etc. }
296-297: Remove the old commented out code.The updated conditional navigation in the
handleBackfunction looks good. It provides flexibility to navigate to different routes based on theREACT_APP_IS_APP_IFRAMEenvironment variable.Consider removing the old commented out code for the conditional navigation to keep the codebase clean:
// if (process.env.REACT_APP_IS_APP_IFRAME === 'true') { // navigate("/"); // } else { // navigate("/discover-start") // }
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (3)
- src/components/Assesment/Assesment.jsx (1 hunks)
- src/components/DiscoverSentance/DiscoverSentance.jsx (6 hunks)
- src/views/Practice/Practice.jsx (2 hunks)
Additional context used
GitHub Check: SonarCloud
src/components/DiscoverSentance/DiscoverSentance.jsx
[failure] 103-103: Origins should be verified during cross-origin communications
Specify a target origin for this message.See more on SonarCloud
src/components/Assesment/Assesment.jsx
[failure] 347-347: Origins should be verified during cross-origin communications
Specify a target origin for this message.See more on SonarCloud
src/views/Practice/Practice.jsx
[failure] 122-122: Origins should be verified during cross-origin communications
Specify a target origin for this message.See more on SonarCloud
[failure] 689-689: Origins should be verified during cross-origin communications
Specify a target origin for this message.See more on SonarCloud
Additional comments not posted (3)
src/components/DiscoverSentance/DiscoverSentance.jsx (1)
114-115: Provide more context on the new state variables.Could you please provide more information on the purpose and usage of the
isNextButtonCalledandenableNextstate variables? It's unclear from the provided code context why these states are being updated in thehandleNextfunction and how they are being used elsewhere in the component.src/views/Practice/Practice.jsx (2)
Line range hint
1-11: LGTM!The import statements look good. No changes needed.
Tools
GitHub Check: SonarCloud
[failure] 122-122: Origins should be verified during cross-origin communications
Specify a target origin for this message.See more on SonarCloud
Line range hint
32-38: LGTM!The state variable definition using
useStatelooks good. No changes needed.Tools
GitHub Check: SonarCloud
[failure] 122-122: Origins should be verified during cross-origin communications
Specify a target origin for this message.See more on SonarCloud
| if (process.env.REACT_APP_IS_APP_IFRAME === "true") { | ||
| window.parent.postMessage( | ||
| { | ||
| score: score, | ||
| message: "all-test-rig-score", | ||
| }, | ||
| "*" | ||
| ); |
There was a problem hiding this comment.
Restrict the target origin to trusted domains.
The formatting changes to the postMessage call improve readability. However, using "*" as the target origin allows the message to be sent to any origin, which could be a security risk if the message contains sensitive data.
Restrict the target origin to trusted domains instead of using "*". For example:
window.parent.postMessage(
{
score: score,
message: "all-test-rig-score",
},
"https://trusted-domain.com"
);Tools
GitHub Check: SonarCloud
[failure] 103-103: Origins should be verified during cross-origin communications
Specify a target origin for this message.See more on SonarCloud
| @@ -344,7 +344,7 @@ | |||
| const handleProfileBack = () => { | |||
| try { | |||
| if (process.env.REACT_APP_IS_APP_IFRAME === "true") { | |||
| window.parent.postMessage({ type: "restore-iframe-content" }); | |||
| window.parent.postMessage({ type: "restore-iframe-content" }, "*"); | |||
There was a problem hiding this comment.
Security Issue: Specify the exact target origin instead of using "*".
Using "*" as the target origin in the postMessage call allows the message to be sent to any window, which can lead to security vulnerabilities. It is recommended to specify the exact origin of the intended recipient instead.
To address this, replace "*" with the specific origin URL:
- window.parent.postMessage({ type: "restore-iframe-content" }, "*");
+ window.parent.postMessage({ type: "restore-iframe-content" }, "https://example.com");Make sure to replace "https://example.com" with the actual origin URL of the intended recipient.
Committable suggestion was skipped due to low confidence.
Tools
GitHub Check: SonarCloud
[failure] 347-347: Origins should be verified during cross-origin communications
Specify a target origin for this message.See more on SonarCloud
| window.parent.postMessage( | ||
| { | ||
| score: score, | ||
| message: "all-test-rig-score", | ||
| }, | ||
| "*" | ||
| ); |
There was a problem hiding this comment.
Security Issue: Specify trusted domain instead of "*" for postMessage target origin.
Using "*" as the target origin in postMessage is a security risk because it allows sending messages to any origin. This can potentially leak sensitive data to untrusted sources.
Fix this by replacing "*" with the specific trusted domain that is allowed to receive the message. For example:
- window.parent.postMessage({score: score, message: "all-test-rig-score"}, "*");
+ window.parent.postMessage({score: score, message: "all-test-rig-score"}, "https://trusted-domain.com");Committable suggestion was skipped due to low confidence.
Tools
GitHub Check: SonarCloud
[failure] 122-122: Origins should be verified during cross-origin communications
Specify a target origin for this message.See more on SonarCloud
| @@ -683,7 +686,7 @@ | |||
| questions[currentQuestion]?.contentSourceData || []; | |||
| const stringLengths = contentSourceData.map((item) => item.text.length); | |||
| const length = stringLengths[0]; | |||
| window.parent.postMessage({ type: "stringLengths", length }); | |||
| window.parent.postMessage({ type: "stringLengths", length }, "*"); | |||
There was a problem hiding this comment.
Security Issue: Specify trusted domain instead of "*" for postMessage target origin.
Using "*" as the target origin in postMessage is a security risk because it allows sending messages to any origin. This can potentially leak sensitive data to untrusted sources.
Fix this by replacing "*" with the specific trusted domain that is allowed to receive the message. For example:
- window.parent.postMessage({ type: "stringLengths", length }, "*");
+ window.parent.postMessage({ type: "stringLengths", length }, "https://trusted-domain.com");Committable suggestion was skipped due to low confidence.
Tools
GitHub Check: SonarCloud
[failure] 689-689: Origins should be verified during cross-origin communications
Specify a target origin for this message.See more on SonarCloud
|
Merged the PR for now - later we need to add whitelist urls as config to make this work |
Issueid #226738 fix: Score not updating for speak with me section if …
Issueid #226738 fix: Score not updating for speak with me section if …
…we load the assets from cloudfront
Summary by CodeRabbit