-
Notifications
You must be signed in to change notification settings - Fork 70
fix(cc-widgets): remediate P1 security scan findings #726
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: next
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,9 +28,10 @@ import './App.scss'; | |
| import {observer} from 'mobx-react-lite'; | ||
| import EngageWidget from './EngageWidget'; | ||
|
|
||
| // This is not to be included to a production app. | ||
| // Have added here for debugging purposes | ||
| window['store'] = store; | ||
| if (process.env.NODE_ENV !== 'production') { | ||
| // Expose store for debugging in non-production builds only | ||
| window['store'] = store; | ||
| } | ||
|
|
||
| const defaultWidgets = { | ||
| stationLogin: true, | ||
|
|
@@ -76,10 +77,9 @@ function App() { | |
| const [collapsedTasks, setCollapsedTasks] = React.useState([]); | ||
| const [showLoader, setShowLoader] = useState(false); | ||
| const [toast, setToast] = useState<{type: 'success' | 'error'} | null>(null); | ||
| const [integrationEnv, setintegrationEnv] = useState(() => { | ||
| const savedintegrationEnv = window.localStorage.getItem('integrationEnv'); | ||
| return savedintegrationEnv === 'true'; | ||
| }); | ||
| const [integrationEnv, setintegrationEnv] = useState( | ||
| process.env.REACT_APP_INTEGRATION_ENV === 'true' | ||
| ); | ||
|
Comment on lines
+80
to
+82
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a user enables Integration Env in the React sample and starts OAuth, the full-page redirect remounts the app; this new initializer no longer reads the previous localStorage value and relies on Useful? React with 👍 / 👎. |
||
| const [conferenceEnabled, setConferenceEnabled] = useState(() => { | ||
| const savedMultiPartyConferenceEnabled = window.localStorage.getItem('conferenceEnabled'); | ||
| return savedMultiPartyConferenceEnabled !== null ? savedMultiPartyConferenceEnabled === 'true' : true; | ||
|
|
@@ -132,6 +132,26 @@ function App() { | |
| const accessToken = urlParams.get('access_token'); | ||
|
|
||
| if (accessToken) { | ||
| // Validate CSRF state token before accepting the OAuth redirect. | ||
| // The SDK encodes state as base64(JSON({app_state, csrf_token})) in the hash. | ||
| const rawState = urlParams.get('state'); | ||
| const storedState = window.sessionStorage.getItem('oauth2-state-token'); | ||
| window.sessionStorage.removeItem('oauth2-state-token'); | ||
|
|
||
| let stateValid = false; | ||
| try { | ||
| const parsedState = rawState ? JSON.parse(atob(rawState)) : null; | ||
| stateValid = !!(parsedState && storedState && parsedState.app_state === storedState); | ||
| } catch { | ||
| stateValid = false; | ||
| } | ||
|
|
||
| if (!stateValid) { | ||
| console.error('OAuth state mismatch — possible CSRF attack; token rejected.'); | ||
| window.history.replaceState({}, document.title, window.location.pathname + window.location.search); | ||
| return; | ||
| } | ||
|
|
||
| window.localStorage.setItem('accessToken', accessToken); | ||
| setAccessToken(accessToken); | ||
| // Clear the hash from the URL to remove the token from browser history | ||
|
|
@@ -334,6 +354,12 @@ function App() { | |
| redirectUri += window.location.pathname; | ||
| } | ||
|
|
||
| // Generate cryptographically-random CSRF state token and persist in sessionStorage | ||
| const stateArray = new Uint8Array(32); | ||
| crypto.getRandomValues(stateArray); | ||
| const stateToken = Array.from(stateArray, (b) => b.toString(16).padStart(2, '0')).join(''); | ||
| window.sessionStorage.setItem('oauth2-state-token', stateToken); | ||
|
|
||
| // Reference: https://developer.webex-cx.com/documentation/integrations | ||
| const ccMandatoryScopes = ['cjp:config_read', 'cjp:config_write', 'cjp:config', 'cjp:user']; | ||
|
|
||
|
|
@@ -372,7 +398,9 @@ function App() { | |
| const webex = Webex.init(webexConfig); | ||
|
|
||
| webex.once('ready', () => { | ||
| webex.authorization.initiateLogin(); | ||
| // Pass our state token so the SDK embeds it in the authorize URL; | ||
| // the redirect handler validates the returned state against sessionStorage. | ||
| webex.authorization.initiateLogin({state: {app_state: stateToken}}); | ||
| }); | ||
| }; | ||
|
|
||
|
|
@@ -394,9 +422,6 @@ function App() { | |
| useEffect(() => { | ||
| window.localStorage.setItem('currentTheme', currentTheme); | ||
| }, [currentTheme]); | ||
| useEffect(() => { | ||
| window.localStorage.setItem('integrationEnv', JSON.stringify(integrationEnv)); | ||
| }, [integrationEnv]); | ||
| useEffect(() => { | ||
| window.localStorage.setItem('conferenceEnabled', JSON.stringify(conferenceEnabled)); | ||
| }, [conferenceEnabled]); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,6 @@ | ||
| import {store} from '@webex/cc-widgets/wc'; | ||
| // @ts-expect-error: for web components this is required | ||
| window.store = store; | ||
|
|
||
| if (process.env.NODE_ENV !== 'production') { | ||
| // @ts-expect-error: expose store for debugging in non-production builds only | ||
| window.store = store; | ||
|
Comment on lines
+3
to
+5
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When Useful? React with 👍 / 👎. |
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If
initializeApp(dataCenter, jwtToken)rejects once due to a transient token/network/datacenter issue, this new in-flight flag staystruebecause the catch block only logs the error whileisDigitalChannelsInitializedremains false. Any later effect run for a refreshed token or task will return at the guard and the digital-channels widget will keep renderingnulluntil the hook is unmounted; reset the ref on failure or in afinallypath that distinguishes success from failure.Useful? React with 👍 / 👎.