From ed477f32077196fe4563b603a9c2e42c83a3b31c Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Tue, 20 Jul 2021 12:16:58 +0200 Subject: [PATCH 01/48] Initial implementation --- package.json | 2 + .../modules/Stream/Auth/ConnectForm.tsx | 12 + src/shared/modules/auth/common.js | 233 ++++++++++++++++++ src/shared/modules/auth/constants.js | 13 + src/shared/modules/auth/helpers.js | 109 ++++++++ src/shared/modules/auth/helpers.test.js | 92 +++++++ src/shared/modules/auth/index.js | 233 ++++++++++++++++++ src/shared/modules/auth/settings.js | 32 +++ .../modules/connections/connectionsDuck.ts | 2 + src/shared/modules/discovery/discoveryDuck.ts | 111 +++++++-- tsconfig.json | 2 +- yarn.lock | 10 + 12 files changed, 836 insertions(+), 15 deletions(-) create mode 100644 src/shared/modules/auth/common.js create mode 100644 src/shared/modules/auth/constants.js create mode 100644 src/shared/modules/auth/helpers.js create mode 100644 src/shared/modules/auth/helpers.test.js create mode 100644 src/shared/modules/auth/index.js create mode 100644 src/shared/modules/auth/settings.js diff --git a/package.json b/package.json index 70881d2e50f..a76e329c145 100644 --- a/package.json +++ b/package.json @@ -181,10 +181,12 @@ "isomorphic-fetch": "^2.2.1", "jsonic": "^0.3.0", "jszip": "^3.2.2", + "jwt-decode": "^3.1.2", "lodash-es": "^4.17.15", "memoize-one": "^5.2.1", "monaco-editor": "0.23.0", "neo4j-driver": "^4.3.1", + "querystring": "^0.2.1", "react": "^16.9.0", "react-dnd": "^11.1.3", "react-dnd-html5-backend": "^11.1.3", diff --git a/src/browser/modules/Stream/Auth/ConnectForm.tsx b/src/browser/modules/Stream/Auth/ConnectForm.tsx index cb99555011c..5b2701f2b16 100644 --- a/src/browser/modules/Stream/Auth/ConnectForm.tsx +++ b/src/browser/modules/Stream/Auth/ConnectForm.tsx @@ -34,6 +34,8 @@ import { NATIVE, NO_AUTH } from 'services/bolt/boltHelpers' import { toKeyString } from 'services/utils' import { stripScheme, getScheme } from 'services/boltscheme.utils' import { AuthenticationMethod } from 'shared/modules/connections/connectionsDuck' +import { authRequestForSSO } from 'shared/modules/auth/index.js' +import { getSSOProvidersFromStorage } from 'shared/modules/auth/common.js' const readableauthenticationMethods: Record = { [NATIVE]: 'Username / Password', @@ -64,6 +66,8 @@ export default function ConnectForm(props: ConnectFormProps): JSX.Element { props.allowedSchemes ? `${getScheme(props.host)}://` : '' ) + const ssoProviders = getSSOProvidersFromStorage() + useEffect(() => { if (props.allowedSchemes) { return setScheme(`${getScheme(props.host)}://`) @@ -118,6 +122,14 @@ export default function ConnectForm(props: ConnectFormProps): JSX.Element { return ( + {ssoProviders.map((provider: any) => ( + + ))} Connect URL diff --git a/src/shared/modules/auth/common.js b/src/shared/modules/auth/common.js new file mode 100644 index 00000000000..a8d14a78a39 --- /dev/null +++ b/src/shared/modules/auth/common.js @@ -0,0 +1,233 @@ +import jwtDecode from 'jwt-decode' +import { isObject, pick } from 'lodash' +import { + AUTH_STORAGE_SSO_PROVIDERS, + AUTH_STORAGE_URL_SEARCH_PARAMS +} from './constants' +import { addSearchParamsInBrowserHistory, authLog } from './helpers' +import { + mandatoryKeysForSSOProviderParams, + mandatoryKeysForSSOProviders, + searchParamsToSaveForAfterAuthRedirect +} from './settings' +import { parse as parseQueryString } from 'querystring' + +export const getInitialisationParameters = ( + urlSearchParams = window.location.search, + urlHashParams = window.location.hash +) => { + let initParams = {} + try { + initParams = { + ...parseQueryString(urlSearchParams.replace(/^\?/, '')), + ...parseQueryString(urlHashParams.replace(/^#/, '')) + } + Object.keys(initParams).forEach(key => { + if (initParams[key].trim().length === 0) { + initParams[key] = null + } + }) + } catch (exc) { + console.warn('Exception occured while parsing Browser URL parameters', exc) + } + return initParams +} + +export const checkAndMergeSSOProviders = ( + discoveredSSOProviders, + isLocalhostOrigin +) => { + if (!discoveredSSOProviders || !discoveredSSOProviders.length) { + authLog('Invalid discovered SSO providers') + return + } + + let currentSSOProviders = + JSON.parse(window.sessionStorage.getItem(AUTH_STORAGE_SSO_PROVIDERS)) || [] + if (!Array.isArray(currentSSOProviders)) { + authLog( + 'Found SSO providers in storage are defect, consider clearing the SSO provider state and reload', + 'warn' + ) + currentSSOProviders = [] + } + + discoveredSSOProviders.forEach(provider => { + if (!provider) return + if ( + !mandatoryKeysForSSOProviders.every(key => provider.hasOwnProperty(key)) + ) { + authLog( + `Dropping invalid discovered SSO provider with id: "${provider.id}", missing key` + ) + return + } + if ( + !mandatoryKeysForSSOProviderParams.every(key => + provider?.params?.hasOwnProperty(key) + ) + ) { + authLog( + `Dropping invalid discovered SSO provider with id: "${provider.id}", missing params key` + ) + return + } + if ( + currentSSOProviders.find(crntProvider => crntProvider.id === provider.id) + ) { + if (isLocalhostOrigin) { + const idx = currentSSOProviders.findIndex( + crntProvider => crntProvider.id === provider.id + ) + currentSSOProviders.splice(idx, 1) + authLog(`Updating SSO provider with id: "${provider.id}"`) + } else { + authLog( + `Not accepting discovered SSO provider with id: "${provider.id}", id exists already` + ) + return + } + } + currentSSOProviders.push(provider) + }) + + window.sessionStorage.setItem( + AUTH_STORAGE_SSO_PROVIDERS, + JSON.stringify(currentSSOProviders) + ) + authLog('Checked and merged SSO providers') +} + +export const getSSOProvidersFromStorage = () => { + const ssoProviders = JSON.parse( + window.sessionStorage.getItem(AUTH_STORAGE_SSO_PROVIDERS) + ) + if (!ssoProviders || !ssoProviders.length) { + authLog('No SSO providers in (local) storage found') + return [] + } + return ssoProviders +} + +export const getSSOProviderByIdpId = idpId => { + const ssoProviders = getSSOProvidersFromStorage() + + const selectedSSOProvider = ssoProviders.find( + provider => provider.id === idpId + ) + if (!selectedSSOProvider) { + authLog(`No matching SSO provider to passed in argument: ${idpId}`, 'warn') + return null + } + return selectedSSOProvider +} + +export const getCredentialsFromAuthResult = (result, idpId) => { + const _parseJWTAndSetCredentials = toParseToken => { + const parsedJWT = jwtDecode(toParseToken) + + // TODO: remove this + console.log('Parsed JWT: ', parsedJWT) + + if (!parsedJWT) { + authLog(`Could not parse JWT for idp_id: ${idpId}, aborting`, 'warn') + return { username: '', password: '' } + } + + const email = parsedJWT[principal] || parsedJWT.email || parsedJWT.sub + authLog(`Credentials assembly, username: ${email}`) + + return { username: email, password: result.access_token } + } + + let credentials = {} + authLog(`Attempting to assemble credentials for idp_id: ${idpId}`) + + if (!result || !idpId) { + authLog('No result or idp_id passed in', 'warn') + return credentials + } + + const selectedSSOProvider = getSSOProviderByIdpId(idpId) + if (!selectedSSOProvider) return + + const principal = selectedSSOProvider.config?.principal || '' + authLog(`Credentials, principal: ${principal}`) + + switch (idpId) { + case 'google-oidc': + if (!result.id_token) { + authLog('No id_token in google-oidc result!', 'warn') + authLog( + `We do not support auth_flow: "${selectedSSOProvider.auth_flow}" for idp_id: "${idpId}" at the moment`, + 'warn' + ) + // INFO: Another HTTP call would be needed to get an id_token. + credentials = { username: '', password: '' } + break + } + credentials = _parseJWTAndSetCredentials(result.id_token) + break + default: + credentials = _parseJWTAndSetCredentials(result.access_token) + break + } + return credentials +} + +export const temporarlyStoreUrlSearchParams = () => { + const currentBrowserURLParams = getInitialisationParameters() + const toSaveSearchParams = pick( + currentBrowserURLParams, + searchParamsToSaveForAfterAuthRedirect + ) + + authLog( + `Temporarly storing the url search params. data: "${JSON.stringify( + toSaveSearchParams + )}"` + ) + window.sessionStorage.setItem( + AUTH_STORAGE_URL_SEARCH_PARAMS, + JSON.stringify(toSaveSearchParams) + ) +} + +export const addStoredUrlSearchParamsToBrowserHistory = ( + toRetrieveParams, + isClearStore = false +) => { + if (!toRetrieveParams || !toRetrieveParams.length) { + authLog( + 'Invalid arguments provided for retrieving temporarly stored url search params, aborting' + ) + return + } + + authLog( + `Retrieving temporarly stored url search params, params: "${toRetrieveParams}"` + ) + try { + const storedParams = JSON.parse( + window.sessionStorage.getItem(AUTH_STORAGE_URL_SEARCH_PARAMS) + ) + + if (isClearStore) { + window.sessionStorage.setItem(AUTH_STORAGE_URL_SEARCH_PARAMS, '') + } + + if (isObject(storedParams)) { + const retrievedParams = pick(storedParams, toRetrieveParams) + addSearchParamsInBrowserHistory(retrievedParams) + return retrievedParams + } else { + authLog('Invalid temporarly stored url search params') + return null + } + } catch (err) { + authLog( + `Error when parsing temporarly stored url search params, err: ${err}` + ) + return null + } +} diff --git a/src/shared/modules/auth/constants.js b/src/shared/modules/auth/constants.js new file mode 100644 index 00000000000..ddcf095ac39 --- /dev/null +++ b/src/shared/modules/auth/constants.js @@ -0,0 +1,13 @@ +export const REDIRECT_URI = 'redirect_uri' +export const SSO_REDIRECT = 'sso_redirect' +export const BEARER = 'bearer' +export const PKCE = 'pkce' +export const IMPLICIT = 'implicit' + +export const AUTH_LOGGING_PREFIX = 'OIDC/OAuth#' + +const AUTH_STORAGE_PREFIX = '/auth#' +export const AUTH_STORAGE_SSO_PROVIDERS = `${AUTH_STORAGE_PREFIX}sso_providers` +export const AUTH_STORAGE_STATE = `${AUTH_STORAGE_PREFIX}state` +export const AUTH_STORAGE_CODE_VERIFIER = `${AUTH_STORAGE_PREFIX}code_verifier` +export const AUTH_STORAGE_URL_SEARCH_PARAMS = `${AUTH_STORAGE_PREFIX}url_search_params` diff --git a/src/shared/modules/auth/helpers.js b/src/shared/modules/auth/helpers.js new file mode 100644 index 00000000000..01450d467d0 --- /dev/null +++ b/src/shared/modules/auth/helpers.js @@ -0,0 +1,109 @@ +import { isObject } from 'lodash' +import { AUTH_LOGGING_PREFIX } from './constants' +import { isAuthLoggingEnabled } from './settings' + +export const authLog = (msg, type = 'log') => { + if (!isAuthLoggingEnabled) return + if (!['log', 'error', 'warn'].includes(type)) return + console[type](`${AUTH_LOGGING_PREFIX} ${msg}`) +} + +export const createNonce = () => + Array.from(window.crypto.getRandomValues(new Uint32Array(4)), t => + t.toString(36) + ).join('-') + +export const createStateForRequest = () => { + const base = Array.from( + window.crypto.getRandomValues(new Uint32Array(4)), + t => t.toString(16) + ).join('-') + return `state-${base}` +} + +export const createCodeVerifier = method => { + const errorMessage = `${AUTH_LOGGING_PREFIX} code verifier, Invalid argument` + + switch (method) { + case 'plain': + case 'S256': + const randomString = Array.from( + window.crypto.getRandomValues(new Uint8Array(32)), + t => String.fromCharCode(t) + ).join('') + return _btoaUrlSafe(randomString) + case '': + case null: + return null + default: + throw errorMessage + } +} + +export const createCodeChallenge = async (method, codeVerifier) => { + const errorMessage = `${AUTH_LOGGING_PREFIX} code challenge, Invalid argument` + + switch (method) { + case 'plain': + if (codeVerifier === null) throw errorMessage + return codeVerifier + case 'S256': + if (codeVerifier === null) throw errorMessage + let bytes = Uint8Array.from(codeVerifier, t => t.charCodeAt(0)) + bytes = await window.crypto.subtle.digest('SHA-256', bytes) + const stringFromBytes = Array.from(new Uint8Array(bytes), t => + String.fromCharCode(t) + ).join('') + return _btoaUrlSafe(stringFromBytes) + case '': + case null: + return null + default: + throw errorMessage + } +} + +export const addSearchParamsInBrowserHistory = paramsToAddObj => { + if (!paramsToAddObj || !isObject(paramsToAddObj)) return + + const searchParams = new URLSearchParams(window.location.search) + Object.entries(paramsToAddObj).forEach(([key, value]) => { + if (key && value && value.length) { + searchParams.set(key, value) + } + }) + + const newUrl = `${window.location.origin}?${searchParams.toString()}` + window.history.replaceState({ path: newUrl }, '', newUrl) +} + +export const removeSearchParamsInBrowserHistory = paramsToRemove => { + if (!paramsToRemove || !paramsToRemove.length) return + + const currentUrlSearchParams = new URLSearchParams(window.location.search) + const cleansedSearchParams = {} + for (const [key, value] of currentUrlSearchParams.entries()) { + if (!paramsToRemove.includes(key)) { + cleansedSearchParams[key] = value + } + } + const newUrlSearchParams = new URLSearchParams(cleansedSearchParams) + + const newUrl = `${window.location.origin}?${newUrlSearchParams.toString()}` + window.history.replaceState({}, '', newUrl) +} + +/* + * Encode text safely to Base64 url + * https://tools.ietf.org/html/rfc7515#appendix-C + * https://tools.ietf.org/html/rfc4648#section-5 + */ +const _btoaUrlSafe = text => { + if (!text) return null + + return window + .btoa(text) + .replace(/\+/g, '-') + .replace(/\//g, '_') + .replace(/=+$/, '') // remove trailing padding characters +} diff --git a/src/shared/modules/auth/helpers.test.js b/src/shared/modules/auth/helpers.test.js new file mode 100644 index 00000000000..1590fec0af6 --- /dev/null +++ b/src/shared/modules/auth/helpers.test.js @@ -0,0 +1,92 @@ +import { + addSearchParamsInBrowserHistory, + removeSearchParamsInBrowserHistory +} from './helpers' + +describe('addSearchParamsInBrowserHistory', () => { + const originalWindowLocationHref = 'http://localhost/' + + afterEach(() => { + window.history.replaceState({}, '', originalWindowLocationHref) + }) + + test('no args passed in', () => { + expect(window.location.href).toEqual(originalWindowLocationHref) + addSearchParamsInBrowserHistory() + expect(window.location.href).toEqual(originalWindowLocationHref) + }) + + test('empty passed in args', () => { + expect(window.location.href).toEqual(originalWindowLocationHref) + addSearchParamsInBrowserHistory({}) + expect(window.location.href).toEqual(originalWindowLocationHref + '?') + }) + + test('simple search param', () => { + expect(window.location.href).toEqual(originalWindowLocationHref) + addSearchParamsInBrowserHistory({ rest: 'test' }) + expect(window.location.href).toEqual( + originalWindowLocationHref + '?rest=test' + ) + }) + + test('search param with special characters', () => { + expect(window.location.href).toEqual(originalWindowLocationHref) + addSearchParamsInBrowserHistory({ test_entry: '5%73bsod?892()€%&#"€' }) + expect(window.location.href).toEqual( + originalWindowLocationHref + + '?test_entry=5%2573bsod%3F892%28%29%E2%82%AC%25%26%23%22%E2%82%AC' + ) + }) + + test('multiple search params', () => { + expect(window.location.href).toEqual(originalWindowLocationHref) + addSearchParamsInBrowserHistory({ + test_entry: '5%73bsod?892()€%&#"€', + entryUrl: 'http://localhost:8043/test/?entry=boat' + }) + expect(window.location.href).toEqual( + originalWindowLocationHref + + '?test_entry=5%2573bsod%3F892%28%29%E2%82%AC%25%26%23%22%E2%82%AC&entryUrl=http%3A%2F%2Flocalhost%3A8043%2Ftest%2F%3Fentry%3Dboat' + ) + }) +}) + +describe('removeSearchParamsInBrowserHistory', () => { + const originalWindowLocationHref = + 'http://localhost/?test_param=house&code=843cvg' + + beforeEach(() => { + window.history.replaceState({}, '', originalWindowLocationHref) + }) + + test('no args passed in', () => { + expect(window.location.href).toEqual(originalWindowLocationHref) + removeSearchParamsInBrowserHistory() + expect(window.location.href).toEqual(originalWindowLocationHref) + }) + + test('empty passed args', () => { + expect(window.location.href).toEqual(originalWindowLocationHref) + removeSearchParamsInBrowserHistory([]) + expect(window.location.href).toEqual(originalWindowLocationHref) + }) + + test('non-existing param in browser history', () => { + expect(window.location.href).toEqual(originalWindowLocationHref) + removeSearchParamsInBrowserHistory(['kode']) + expect(window.location.href).toEqual(originalWindowLocationHref) + }) + + test('remove existing param from browser history', () => { + expect(window.location.href).toEqual(originalWindowLocationHref) + removeSearchParamsInBrowserHistory(['test_param']) + expect(window.location.href).toEqual('http://localhost/?code=843cvg') + }) + + test('remove all existing param from browser history', () => { + expect(window.location.href).toEqual(originalWindowLocationHref) + removeSearchParamsInBrowserHistory(['test_param', 'code']) + expect(window.location.href).toEqual('http://localhost/?') + }) +}) diff --git a/src/shared/modules/auth/index.js b/src/shared/modules/auth/index.js new file mode 100644 index 00000000000..44117cfd7f2 --- /dev/null +++ b/src/shared/modules/auth/index.js @@ -0,0 +1,233 @@ +import { + getCredentialsFromAuthResult, + getInitialisationParameters, + getSSOProviderByIdpId, + temporarlyStoreUrlSearchParams +} from './common' +import { + AUTH_STORAGE_CODE_VERIFIER, + AUTH_STORAGE_STATE, + BEARER, + IMPLICIT, + PKCE +} from './constants' +import { + createStateForRequest, + createNonce, + createCodeVerifier, + createCodeChallenge, + authLog, + removeSearchParamsInBrowserHistory +} from './helpers' +import { searchParamsToRemoveAfterAuthRedirect } from './settings' + +export const authRequestForSSO = idpId => { + const selectedSSOProvider = getSSOProviderByIdpId(idpId) + if (!selectedSSOProvider) return + + temporarlyStoreUrlSearchParams() + + const oauth2Endpoint = selectedSSOProvider.auth_endpoint + if (!oauth2Endpoint) { + authLog(`Invalid OAuth2 endpoint: "${oauth2Endpoint}"`) + return + } + authLog(`Using OAuth2 endpoint: "${oauth2Endpoint}" for idp_id: ${idpId}`) + + const form = document.createElement('form') + form.setAttribute('method', 'GET') + form.setAttribute('action', oauth2Endpoint) + + const ssoParams = selectedSSOProvider.params || {} + const state = createStateForRequest() + let params = { + ...ssoParams, + state + } + window.sessionStorage.setItem(AUTH_STORAGE_STATE, state) + + const ssoExtraAuthParams = selectedSSOProvider.auth_params || {} + if (ssoExtraAuthParams) { + params = { + ...params, + ...ssoExtraAuthParams + } + } + + authLog( + `Using the following authorization parameter: ${JSON.stringify(ssoParams)}` + ) + + const ssoConfig = selectedSSOProvider.config || {} + if (ssoConfig.implicit_flow_requires_nonce) { + params = { + ...params, + nonce: createNonce() + } + authLog(`Using nonce in authorization request`) + } + + const _submitForm = (form, params) => { + for (const param in params) { + const input = document.createElement('input') + input.setAttribute('type', 'hidden') + input.setAttribute('name', param) + input.setAttribute('value', params[param]) + form.appendChild(input) + } + + document.body.appendChild(form) + form.submit() + } + + if (selectedSSOProvider.auth_flow === PKCE) { + const codeChallengeMethod = ssoConfig.code_challenge_method || 'S256' + authLog( + `Auth flow "PKCE", using code_challenge_method: "${codeChallengeMethod}"` + ) + + const codeVerifier = createCodeVerifier(codeChallengeMethod) + window.sessionStorage.setItem(AUTH_STORAGE_CODE_VERIFIER, codeVerifier) + + createCodeChallenge(codeChallengeMethod, codeVerifier).then( + codeChallenge => { + params = { + ...params, + code_challenge_method: codeChallengeMethod, + code_challenge: codeChallenge + } + _submitForm(form, params) + } + ) + } else if (selectedSSOProvider.auth_flow === IMPLICIT) { + authLog('Auth flow "implicit"') + _submitForm(form, params) + } else { + authLog(`Auth flow "${selectedSSOProvider.auth_flow}" is not supported.`) + } +} + +export const handleAuthFromRedirect = sendSignal => + new Promise((resolve, reject) => { + const { + idp_id: idpId, + token_type: tokenType, + access_token: accessToken, + id_token: idToken, + error_description: errorDescription, + code, + state, + error + } = getInitialisationParameters() + + removeSearchParamsInBrowserHistory(searchParamsToRemoveAfterAuthRedirect) + + if (error) { + const errorMsg = `Error detected after auth redirect, aborting. Error: ${error}, Error description: ${errorDescription}` + authLog(errorMsg, 'warn') + reject(errorMsg) + return + } + + if (!idpId) { + const errorIdpMsg = 'Invalid idp_id parameter, aborting' + authLog(errorIdpMsg, 'warn') + reject(errorIdpMsg) + return + } + + const savedState = window.sessionStorage.getItem(AUTH_STORAGE_STATE) + if (state !== savedState) { + const errorStateMsg = 'Invalid state parameter, aborting' + authLog(errorStateMsg, 'warn') + reject(errorStateMsg) + return + } + window.sessionStorage.setItem(AUTH_STORAGE_STATE, '') + + if ((tokenType || '').toLowerCase() === BEARER && accessToken) { + authLog('Successfully aquired access_token in "implict flow"') + + // TODO: remove this + console.log('SSO:: IMPLICT SUCCESS RESULT', [accessToken, idToken]) + + const credentials = getCredentialsFromAuthResult( + { access_token: accessToken, id_token: idToken }, + idpId + ) + sendSignal({ type: 'RETRY', credentials }) + resolve({ credentials }) + } else { + authLog('Attempting to fetch token information in "PKCE flow"') + + authRequestForToken(idpId, code) + .then(data => { + data.json().then(result => { + if (result && result.error) { + const errorType = result?.error || 'unknown' + const errorDesc = result['error_description'] || 'unknown' + const errorMsg = `Error detected after auth token request, aborting. Error: ${errorType}, Error description: ${errorDesc}` + authLog(errorMsg, 'warn') + reject(errorMsg) + } else { + authLog('Successfully aquired token results') + + // TODO: remove this + console.log('SSO:: PKCE SUCCESS RESULT', result) + + const credentials = getCredentialsFromAuthResult(result, idpId) + sendSignal({ type: 'RETRY', credentials }) + resolve({ credentials }) + } + }) + }) + .catch(err => { + const errRequestMsg = `Aquiring token results for PKCE auth flow failed, err: ${err}` + authLog(errRequestMsg, 'warn') + reject(errRequestMsg) + }) + } + }) + +export const authRequestForToken = (idpId, code) => { + const selectedSSOProvider = getSSOProviderByIdpId(idpId) + if (!selectedSSOProvider) return + + const ssoParams = selectedSSOProvider.params || {} + let details = { + grant_type: 'authorization_code', + client_id: ssoParams.client_id, + redirect_uri: ssoParams.redirect_uri, + code_verifier: window.sessionStorage.getItem(AUTH_STORAGE_CODE_VERIFIER), + code + } + window.sessionStorage.setItem(AUTH_STORAGE_CODE_VERIFIER, '') + + const ssoExtraTokenParams = selectedSSOProvider.token_params || {} + if (ssoExtraTokenParams) { + details = { + ...details, + ...ssoExtraTokenParams + } + } + + const requestOptions = { + method: 'post', + headers: { + Accept: 'application/json', + 'Content-Type': 'application/x-www-form-urlencoded' + } + } + + let requestBody = [] + for (const property in details) { + const encodedKey = encodeURIComponent(property) + const encodedValue = encodeURIComponent(details[property]) + requestBody.push(encodedKey + '=' + encodedValue) + } + requestBody = requestBody.join('&') + + authLog(`Request for token in PKCE flow, idp_id: ${idpId}`) + const requestUrl = `${selectedSSOProvider.token_endpoint}?` + return window.fetch(requestUrl, { ...requestOptions, body: requestBody }) +} diff --git a/src/shared/modules/auth/settings.js b/src/shared/modules/auth/settings.js new file mode 100644 index 00000000000..794960bc034 --- /dev/null +++ b/src/shared/modules/auth/settings.js @@ -0,0 +1,32 @@ +export const mandatoryKeysForSSOProviders = [ + 'id', + 'name', + 'auth_flow', + 'params', + 'auth_endpoint', + 'well_known_discovery_uri' +] +export const mandatoryKeysForSSOProviderParams = [ + 'client_id', + 'redirect_uri', + 'response_type', + 'scope' +] + +export const searchParamsToRemoveAfterAutoRedirect = ['cmd', 'arg'] +export const searchParamsToRemoveAfterAuthRedirect = [ + 'idp_id', + 'auth_flow_step', + 'state', + 'session_state', + 'code' +] +export const searchParamsToSaveForAfterAuthRedirect = [ + 'connectURL', + 'discoveryURL', + 'search', + 'perspective', + 'run' +] + +export const isAuthLoggingEnabled = true // TODO: create a setting? or just check the NODE_ENV? always log warn and error? diff --git a/src/shared/modules/connections/connectionsDuck.ts b/src/shared/modules/connections/connectionsDuck.ts index f4c9d442fb3..f4f4268d666 100644 --- a/src/shared/modules/connections/connectionsDuck.ts +++ b/src/shared/modules/connections/connectionsDuck.ts @@ -437,6 +437,7 @@ type DiscoverDataAction = { type: typeof discovery.DONE discovered?: { username?: string + password?: string requestedUseDb?: string restApi?: string supportsMultiDb?: string @@ -489,6 +490,7 @@ export const startupConnectEpic = (action$: any, store: any) => { store.getState(), discovery.CONNECTION_ID ) + debugger if (shouldTryAutoconnecting(connUpdatedWithDiscovery)) { // Try connecting diff --git a/src/shared/modules/discovery/discoveryDuck.ts b/src/shared/modules/discovery/discoveryDuck.ts index cfe366f74b8..40b57637be2 100644 --- a/src/shared/modules/discovery/discoveryDuck.ts +++ b/src/shared/modules/discovery/discoveryDuck.ts @@ -36,6 +36,20 @@ import { getUrlInfo } from 'shared/services/utils' import { isConnectedAuraHost } from 'shared/modules/connections/connectionsDuck' import { isCloudHost } from 'shared/services/utils' import { NEO4J_CLOUD_DOMAINS } from 'shared/modules/settings/settingsDuck' +import { + authRequestForSSO, + handleAuthFromRedirect +} from 'shared/modules/auth/index' +import { + authLog, + removeSearchParamsInBrowserHistory +} from 'shared/modules/auth/helpers' +import { REDIRECT_URI, SSO_REDIRECT } from 'shared/modules/auth/constants' +import { + addStoredUrlSearchParamsToBrowserHistory, + checkAndMergeSSOProviders +} from 'shared/modules/auth/common' +import { searchParamsToRemoveAfterAutoRedirect } from 'shared/modules/auth/settings' export const NAME = 'discover-bolt-host' export const CONNECTION_ID = '$$discovery' @@ -142,6 +156,7 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { if (!passedURL || !passedURL.length) return action action.forceURL = decodeURIComponent(passedURL[0]) action.requestedUseDb = passedDb && passedDb[0] + return action }) .merge(some$.ofType(USER_CLEAR)) @@ -150,11 +165,19 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { if (!hasDiscoveryEndpoint(store.getState())) { return Promise.resolve({ type: 'NOOP' }) } - if (action.forceURL) { - const { username, protocol, host } = getUrlInfo(action.forceURL) + const searchParams = new URL(window.location.href).searchParams + const authFlowStep = + (searchParams.get('auth_flow_step') || '').toLowerCase() === + REDIRECT_URI + if (action.forceURL && !authFlowStep) { + const { username, password, protocol, host } = getUrlInfo( + action.forceURL + ) + console.log(username, password) const discovered = { username, + password, requestedUseDb: action.requestedUseDb, host: `${protocol ? `${protocol}//` : ''}${host}`, supportsMultiDb: !!action.requestedUseDb, @@ -172,18 +195,78 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { }) } return Rx.Observable.fromPromise( - remote - .getJSON(getDiscoveryEndpoint(getHostedUrl(store.getState()))) - // Uncomment below and comment out above when doing manual tests in dev mode to - // fake discovery response - //Promise.resolve({ - // bolt: 'bolt://localhost:7687', - //neo4j_version: '4.0.3' - //}) - .then(result => { - let host = - result && - (result.bolt_routing || result.bolt_direct || result.bolt) + //remote + // .getJSON(getDiscoveryEndpoint(getHostedUrl(store.getState()))) + + // Uncomment below and comment out above when doing manual tests in dev mode to + // fake discovery response + Promise.resolve({ + bolt: 'bolt://localhost:7687', + neo4j_version: '4', + sso_providers: [ + { + id: 'keycloak-oidc', + name: 'KeyCloak', + auth_flow: 'pkce', + auth_endpoint: + 'http://localhost:18080/auth/realms/myrealm/protocol/openid-connect/auth', + token_endpoint: + 'http://localhost:18080/auth/realms/myrealm/protocol/openid-connect/token', + well_known_discovery_uri: + 'http://localhost:18080/auth/realms/myrealm/.well-known/openid-configuration', + params: { + client_id: 'account', + redirect_uri: + 'http://localhost:8080?idp_id=keycloak-oidc&auth_flow_step=redirect_uri', + response_type: 'code', + scope: 'openid groups' + }, + config: { + principal: 'preferred_username', + code_challenge_method: 'S256' + } + } + ] + }) + .then(async result => { + const ssoProviders = result.sso_providers //|| result.ssoproviders || result.ssoProviders + + if (ssoProviders) { + authLog('SSO providers found on endpoint ') + checkAndMergeSSOProviders(ssoProviders, true) + } else { + const errMsg = 'No SSO providers found on endpoint' + authLog(errMsg) + } + + const searchParams = new URL(window.location.href).searchParams + + const cmd = (searchParams.get('cmd') || '').toLowerCase() + const arg = searchParams.get('arg') + const authFlowStep = ( + searchParams.get('auth_flow_step') || '' + ).toLowerCase() + if (cmd === SSO_REDIRECT && arg) { + authLog(`Initialised with cmd: "${cmd}" and arg: "${arg}"`) + + removeSearchParamsInBrowserHistory( + searchParamsToRemoveAfterAutoRedirect + ) + + authRequestForSSO(arg) + } else if (authFlowStep === REDIRECT_URI) { + authLog('Handling auth_flow_step redirect') + + addStoredUrlSearchParamsToBrowserHistory([ + 'connectURL', + 'discoveryURL' + ]) + const host = result && result.bolt + const creds = await handleAuthFromRedirect(() => undefined) + return { type: DONE, discovered: { host, ...creds.credentials } } + } + + let host = result && result.bolt // Try to get info from server if (!host) { throw new Error('No bolt address found') diff --git a/tsconfig.json b/tsconfig.json index 12beb351fb0..25a16319d7e 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -17,7 +17,7 @@ "noFallthroughCasesInSwitch": true, "jsx": "react", "outDir": "./build", - "allowJs": false, + "allowJs": true, "resolveJsonModule": true, "baseUrl": "./src", "paths": { diff --git a/yarn.lock b/yarn.lock index 981603296bc..f86dcf4ef09 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8326,6 +8326,11 @@ jws@^4.0.0: jwa "^2.0.0" safe-buffer "^5.0.1" +jwt-decode@^3.1.2: + version "3.1.2" + resolved "https://neo.jfrog.io/neo/api/npm/npm/jwt-decode/-/jwt-decode-3.1.2.tgz#3fb319f3675a2df0c2895c8f5e9fa4b67b04ed59" + integrity sha1-P7MZ82daLfDCiVyPXp+ktnsE7Vk= + keyboard-key@^1.0.4: version "1.1.0" resolved "https://neo.jfrog.io/neo/api/npm/npm/keyboard-key/-/keyboard-key-1.1.0.tgz#6f2e8e37fa11475bb1f1d65d5174f1b35653f5b7" @@ -10875,6 +10880,11 @@ querystring@0.2.0, querystring@^0.2.0: resolved "https://neo.jfrog.io/neo/api/npm/npm/querystring/-/querystring-0.2.0.tgz#b209849203bb25df820da756e747005878521620" integrity sha1-sgmEkgO7Jd+CDadW50cAWHhSFiA= +querystring@^0.2.1: + version "0.2.1" + resolved "https://neo.jfrog.io/neo/api/npm/npm/querystring/-/querystring-0.2.1.tgz#40d77615bb09d16902a85c3e38aa8b5ed761c2dd" + integrity sha1-QNd2FbsJ0WkCqFw+OKqLXtdhwt0= + querystringify@^2.1.1: version "2.2.0" resolved "https://neo.jfrog.io/neo/api/npm/npm/querystringify/-/querystringify-2.2.0.tgz#3345941b4153cb9d082d8eee4cda2016a9aef7f6" From 2d864d09f0d22f11e0a064e8b54f9da0cbd50777 Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Tue, 20 Jul 2021 12:33:16 +0200 Subject: [PATCH 02/48] Remove dep on querystring --- package.json | 1 - src/shared/modules/auth/common.js | 24 ++++++------------- src/shared/modules/discovery/discoveryDuck.ts | 1 - yarn.lock | 5 ---- 4 files changed, 7 insertions(+), 24 deletions(-) diff --git a/package.json b/package.json index a76e329c145..7b07f79c645 100644 --- a/package.json +++ b/package.json @@ -186,7 +186,6 @@ "memoize-one": "^5.2.1", "monaco-editor": "0.23.0", "neo4j-driver": "^4.3.1", - "querystring": "^0.2.1", "react": "^16.9.0", "react-dnd": "^11.1.3", "react-dnd-html5-backend": "^11.1.3", diff --git a/src/shared/modules/auth/common.js b/src/shared/modules/auth/common.js index a8d14a78a39..458562af45b 100644 --- a/src/shared/modules/auth/common.js +++ b/src/shared/modules/auth/common.js @@ -10,26 +10,16 @@ import { mandatoryKeysForSSOProviders, searchParamsToSaveForAfterAuthRedirect } from './settings' -import { parse as parseQueryString } from 'querystring' export const getInitialisationParameters = ( - urlSearchParams = window.location.search, - urlHashParams = window.location.hash + urlSearchParams = window.location.search ) => { - let initParams = {} - try { - initParams = { - ...parseQueryString(urlSearchParams.replace(/^\?/, '')), - ...parseQueryString(urlHashParams.replace(/^#/, '')) - } - Object.keys(initParams).forEach(key => { - if (initParams[key].trim().length === 0) { - initParams[key] = null - } - }) - } catch (exc) { - console.warn('Exception occured while parsing Browser URL parameters', exc) - } + const initParams = {} + + new URLSearchParams(urlSearchParams).forEach((value, key) => { + initParams[key] = value + }) + return initParams } diff --git a/src/shared/modules/discovery/discoveryDuck.ts b/src/shared/modules/discovery/discoveryDuck.ts index 40b57637be2..f539c6aea2e 100644 --- a/src/shared/modules/discovery/discoveryDuck.ts +++ b/src/shared/modules/discovery/discoveryDuck.ts @@ -156,7 +156,6 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { if (!passedURL || !passedURL.length) return action action.forceURL = decodeURIComponent(passedURL[0]) action.requestedUseDb = passedDb && passedDb[0] - return action }) .merge(some$.ofType(USER_CLEAR)) diff --git a/yarn.lock b/yarn.lock index f86dcf4ef09..0b5b434bfca 100644 --- a/yarn.lock +++ b/yarn.lock @@ -10880,11 +10880,6 @@ querystring@0.2.0, querystring@^0.2.0: resolved "https://neo.jfrog.io/neo/api/npm/npm/querystring/-/querystring-0.2.0.tgz#b209849203bb25df820da756e747005878521620" integrity sha1-sgmEkgO7Jd+CDadW50cAWHhSFiA= -querystring@^0.2.1: - version "0.2.1" - resolved "https://neo.jfrog.io/neo/api/npm/npm/querystring/-/querystring-0.2.1.tgz#40d77615bb09d16902a85c3e38aa8b5ed761c2dd" - integrity sha1-QNd2FbsJ0WkCqFw+OKqLXtdhwt0= - querystringify@^2.1.1: version "2.2.0" resolved "https://neo.jfrog.io/neo/api/npm/npm/querystringify/-/querystringify-2.2.0.tgz#3345941b4153cb9d082d8eee4cda2016a9aef7f6" From 2aff3dccae2f24c54ce420d611840b0d6757ed2b Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Tue, 20 Jul 2021 12:59:43 +0200 Subject: [PATCH 03/48] Restore search params properly --- src/browser/AppInit.tsx | 13 ++++- src/shared/modules/auth/common.js | 50 ++++++++----------- .../modules/connections/connectionsDuck.ts | 1 - src/shared/modules/discovery/discoveryDuck.ts | 14 +----- 4 files changed, 33 insertions(+), 45 deletions(-) diff --git a/src/browser/AppInit.tsx b/src/browser/AppInit.tsx index edbc80ece0e..e05f30f40e5 100644 --- a/src/browser/AppInit.tsx +++ b/src/browser/AppInit.tsx @@ -53,6 +53,10 @@ import { shouldAllowOutgoingConnections } from 'shared/modules/dbMeta/dbMetaDuck import { getUuid } from 'shared/modules/udc/udcDuck' import { DndProvider } from 'react-dnd' import { HTML5Backend } from 'react-dnd-html5-backend' +import { + restoreSearchParams, + redirectedBackFromSSOServer +} from 'shared/modules/auth/common' // Configure localstorage sync applyKeys( @@ -199,9 +203,16 @@ export function setupSentry(): void { // Introduce environment to be able to fork functionality const env = detectRuntimeEnv(window, NEO4J_CLOUD_DOMAINS) +// SSO requires a redirect that removes our search parameters +// To work around this they are stored in sessionStorage before +// we redirect to the server, and then restore them when we get +// redirected back +if (redirectedBackFromSSOServer()) { + restoreSearchParams() +} + // URL we're on const url = window.location.href - const searchParams = new URL(url).searchParams // Desktop/Relate params diff --git a/src/shared/modules/auth/common.js b/src/shared/modules/auth/common.js index 458562af45b..f462b9175ed 100644 --- a/src/shared/modules/auth/common.js +++ b/src/shared/modules/auth/common.js @@ -1,14 +1,14 @@ import jwtDecode from 'jwt-decode' -import { isObject, pick } from 'lodash' +import { isObject } from 'lodash' import { AUTH_STORAGE_SSO_PROVIDERS, - AUTH_STORAGE_URL_SEARCH_PARAMS + AUTH_STORAGE_URL_SEARCH_PARAMS, + REDIRECT_URI } from './constants' import { addSearchParamsInBrowserHistory, authLog } from './helpers' import { mandatoryKeysForSSOProviderParams, - mandatoryKeysForSSOProviders, - searchParamsToSaveForAfterAuthRedirect + mandatoryKeysForSSOProviders } from './settings' export const getInitialisationParameters = ( @@ -167,49 +167,39 @@ export const getCredentialsFromAuthResult = (result, idpId) => { export const temporarlyStoreUrlSearchParams = () => { const currentBrowserURLParams = getInitialisationParameters() - const toSaveSearchParams = pick( - currentBrowserURLParams, - searchParamsToSaveForAfterAuthRedirect - ) authLog( `Temporarly storing the url search params. data: "${JSON.stringify( - toSaveSearchParams + currentBrowserURLParams )}"` ) + window.sessionStorage.setItem( AUTH_STORAGE_URL_SEARCH_PARAMS, - JSON.stringify(toSaveSearchParams) + JSON.stringify(currentBrowserURLParams) ) } -export const addStoredUrlSearchParamsToBrowserHistory = ( - toRetrieveParams, - isClearStore = false -) => { - if (!toRetrieveParams || !toRetrieveParams.length) { - authLog( - 'Invalid arguments provided for retrieving temporarly stored url search params, aborting' - ) - return - } +export const redirectedBackFromSSOServer = () => { + const searchParams = new URL(window.location.href).searchParams + const authFlowStep = (searchParams.get('auth_flow_step') || '').toLowerCase() + + return authFlowStep === REDIRECT_URI +} + +export const restoreSearchParams = () => { + authLog(`Retrieving temporarly stored url search params`) - authLog( - `Retrieving temporarly stored url search params, params: "${toRetrieveParams}"` - ) try { const storedParams = JSON.parse( window.sessionStorage.getItem(AUTH_STORAGE_URL_SEARCH_PARAMS) ) - - if (isClearStore) { - window.sessionStorage.setItem(AUTH_STORAGE_URL_SEARCH_PARAMS, '') - } + window.sessionStorage.setItem(AUTH_STORAGE_URL_SEARCH_PARAMS, '') if (isObject(storedParams)) { - const retrievedParams = pick(storedParams, toRetrieveParams) - addSearchParamsInBrowserHistory(retrievedParams) - return retrievedParams + addSearchParamsInBrowserHistory(storedParams) + + return storedParams } else { authLog('Invalid temporarly stored url search params') return null diff --git a/src/shared/modules/connections/connectionsDuck.ts b/src/shared/modules/connections/connectionsDuck.ts index f4f4268d666..9bd02edb0ac 100644 --- a/src/shared/modules/connections/connectionsDuck.ts +++ b/src/shared/modules/connections/connectionsDuck.ts @@ -490,7 +490,6 @@ export const startupConnectEpic = (action$: any, store: any) => { store.getState(), discovery.CONNECTION_ID ) - debugger if (shouldTryAutoconnecting(connUpdatedWithDiscovery)) { // Try connecting diff --git a/src/shared/modules/discovery/discoveryDuck.ts b/src/shared/modules/discovery/discoveryDuck.ts index f539c6aea2e..ec36e0c2a9f 100644 --- a/src/shared/modules/discovery/discoveryDuck.ts +++ b/src/shared/modules/discovery/discoveryDuck.ts @@ -19,17 +19,14 @@ */ import Rx from 'rxjs/Rx' -import remote from 'services/remote' import { updateConnection } from 'shared/modules/connections/connectionsDuck' import { APP_START, USER_CLEAR, hasDiscoveryEndpoint, - getHostedUrl, getAllowedBoltSchemes, CLOUD_SCHEMES } from 'shared/modules/app/appDuck' -import { getDiscoveryEndpoint } from 'services/bolt/boltHelpers' import { getUrlParamValue } from 'services/utils' import { generateBoltUrl } from 'services/boltscheme.utils' import { getUrlInfo } from 'shared/services/utils' @@ -45,10 +42,7 @@ import { removeSearchParamsInBrowserHistory } from 'shared/modules/auth/helpers' import { REDIRECT_URI, SSO_REDIRECT } from 'shared/modules/auth/constants' -import { - addStoredUrlSearchParamsToBrowserHistory, - checkAndMergeSSOProviders -} from 'shared/modules/auth/common' +import { checkAndMergeSSOProviders } from 'shared/modules/auth/common' import { searchParamsToRemoveAfterAutoRedirect } from 'shared/modules/auth/settings' export const NAME = 'discover-bolt-host' @@ -238,8 +232,6 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { authLog(errMsg) } - const searchParams = new URL(window.location.href).searchParams - const cmd = (searchParams.get('cmd') || '').toLowerCase() const arg = searchParams.get('arg') const authFlowStep = ( @@ -256,10 +248,6 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { } else if (authFlowStep === REDIRECT_URI) { authLog('Handling auth_flow_step redirect') - addStoredUrlSearchParamsToBrowserHistory([ - 'connectURL', - 'discoveryURL' - ]) const host = result && result.bolt const creds = await handleAuthFromRedirect(() => undefined) return { type: DONE, discovered: { host, ...creds.credentials } } From e881ae74786190c729ab60a75cc659adc186bc3a Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Tue, 20 Jul 2021 13:07:04 +0200 Subject: [PATCH 04/48] Remove unneeded changes --- src/browser/modules/Stream/Auth/ConnectForm.tsx | 2 +- src/shared/modules/discovery/discoveryDuck.ts | 13 +++---------- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/src/browser/modules/Stream/Auth/ConnectForm.tsx b/src/browser/modules/Stream/Auth/ConnectForm.tsx index 5b2701f2b16..8ef5c6f7251 100644 --- a/src/browser/modules/Stream/Auth/ConnectForm.tsx +++ b/src/browser/modules/Stream/Auth/ConnectForm.tsx @@ -127,7 +127,7 @@ export default function ConnectForm(props: ConnectFormProps): JSX.Element { key={provider.id} onClick={() => authRequestForSSO(provider.id)} > - hej {provider.name} + {provider.name} ))} diff --git a/src/shared/modules/discovery/discoveryDuck.ts b/src/shared/modules/discovery/discoveryDuck.ts index ec36e0c2a9f..df39e42d498 100644 --- a/src/shared/modules/discovery/discoveryDuck.ts +++ b/src/shared/modules/discovery/discoveryDuck.ts @@ -158,19 +158,11 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { if (!hasDiscoveryEndpoint(store.getState())) { return Promise.resolve({ type: 'NOOP' }) } - const searchParams = new URL(window.location.href).searchParams - const authFlowStep = - (searchParams.get('auth_flow_step') || '').toLowerCase() === - REDIRECT_URI - if (action.forceURL && !authFlowStep) { - const { username, password, protocol, host } = getUrlInfo( - action.forceURL - ) - console.log(username, password) + if (action.forceURL) { + const { username, protocol, host } = getUrlInfo(action.forceURL) const discovered = { username, - password, requestedUseDb: action.requestedUseDb, host: `${protocol ? `${protocol}//` : ''}${host}`, supportsMultiDb: !!action.requestedUseDb, @@ -232,6 +224,7 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { authLog(errMsg) } + const searchParams = new URL(window.location.href).searchParams const cmd = (searchParams.get('cmd') || '').toLowerCase() const arg = searchParams.get('arg') const authFlowStep = ( From e6546aed8f9d577a54467097ab24001d90bad6c3 Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Tue, 20 Jul 2021 13:10:41 +0200 Subject: [PATCH 05/48] Touch up connect frame and update licenses --- LICENSES.txt | 26 +++++++++++++++++++ NOTICE.txt | 4 +++ .../modules/Stream/Auth/ConnectForm.tsx | 5 +++- 3 files changed, 34 insertions(+), 1 deletion(-) diff --git a/LICENSES.txt b/LICENSES.txt index 3dae7151dc4..f815e680aa1 100644 --- a/LICENSES.txt +++ b/LICENSES.txt @@ -5805,6 +5805,32 @@ ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEAL ----- +The following software may be included in this product: jwt-decode. A copy of the source code may be downloaded from git://github.com/auth0/jwt-decode. This software contains the following license and notice below: + +The MIT License (MIT) + +Copyright (c) 2015 Auth0, Inc. (http://auth0.com) + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. + +----- + The following software may be included in this product: keyboard-key. A copy of the source code may be downloaded from git+ssh://github.com/levithomason/keyboard-key.git. This software contains the following license and notice below: MIT License diff --git a/NOTICE.txt b/NOTICE.txt index e62ff30642f..a28b197c295 100644 --- a/NOTICE.txt +++ b/NOTICE.txt @@ -1275,6 +1275,10 @@ Third-party licenses │ ├─ jws@4.0.0 │ │ ├─ URL: git://github.com/brianloveswords/node-jws.git │ │ └─ VendorName: Brian J Brennan +│ ├─ jwt-decode@3.1.2 +│ │ ├─ URL: git://github.com/auth0/jwt-decode +│ │ ├─ VendorName: Jose F. Romaniello +│ │ └─ VendorUrl: https://github.com/auth0/jwt-decode#readme │ ├─ keyboard-key@1.1.0 │ │ ├─ URL: git+ssh://github.com/levithomason/keyboard-key.git │ │ └─ VendorName: Levi Thomason diff --git a/src/browser/modules/Stream/Auth/ConnectForm.tsx b/src/browser/modules/Stream/Auth/ConnectForm.tsx index 8ef5c6f7251..6fb8cb81d48 100644 --- a/src/browser/modules/Stream/Auth/ConnectForm.tsx +++ b/src/browser/modules/Stream/Auth/ConnectForm.tsx @@ -65,8 +65,11 @@ export default function ConnectForm(props: ConnectFormProps): JSX.Element { const [scheme, setScheme] = useState( props.allowedSchemes ? `${getScheme(props.host)}://` : '' ) + const [ssoProviders, setSsoProviders] = useState([]) - const ssoProviders = getSSOProvidersFromStorage() + useEffect(() => { + setSsoProviders(getSSOProvidersFromStorage()) + }, []) useEffect(() => { if (props.allowedSchemes) { From f7494a8e24db5449c62e8ec6a55fdc9082f643a5 Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Tue, 20 Jul 2021 13:20:39 +0200 Subject: [PATCH 06/48] Cleanup discovery duck --- src/shared/modules/auth/index.js | 4 +- src/shared/modules/discovery/discoveryDuck.ts | 48 ++++++++++--------- 2 files changed, 27 insertions(+), 25 deletions(-) diff --git a/src/shared/modules/auth/index.js b/src/shared/modules/auth/index.js index 44117cfd7f2..8249d07582f 100644 --- a/src/shared/modules/auth/index.js +++ b/src/shared/modules/auth/index.js @@ -156,7 +156,7 @@ export const handleAuthFromRedirect = sendSignal => idpId ) sendSignal({ type: 'RETRY', credentials }) - resolve({ credentials }) + resolve(credentials) } else { authLog('Attempting to fetch token information in "PKCE flow"') @@ -177,7 +177,7 @@ export const handleAuthFromRedirect = sendSignal => const credentials = getCredentialsFromAuthResult(result, idpId) sendSignal({ type: 'RETRY', credentials }) - resolve({ credentials }) + resolve(credentials) } }) }) diff --git a/src/shared/modules/discovery/discoveryDuck.ts b/src/shared/modules/discovery/discoveryDuck.ts index df39e42d498..732d7c9f74d 100644 --- a/src/shared/modules/discovery/discoveryDuck.ts +++ b/src/shared/modules/discovery/discoveryDuck.ts @@ -215,35 +215,37 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { }) .then(async result => { const ssoProviders = result.sso_providers //|| result.ssoproviders || result.ssoProviders + let creds: { username?: string; password?: string } = {} if (ssoProviders) { authLog('SSO providers found on endpoint ') checkAndMergeSSOProviders(ssoProviders, true) - } else { - const errMsg = 'No SSO providers found on endpoint' - authLog(errMsg) - } - const searchParams = new URL(window.location.href).searchParams - const cmd = (searchParams.get('cmd') || '').toLowerCase() - const arg = searchParams.get('arg') - const authFlowStep = ( - searchParams.get('auth_flow_step') || '' - ).toLowerCase() - if (cmd === SSO_REDIRECT && arg) { - authLog(`Initialised with cmd: "${cmd}" and arg: "${arg}"`) + // TODO function should redirect to sso, + // TODO function was redirected back + const searchParams = new URL(window.location.href).searchParams + const cmd = (searchParams.get('cmd') || '').toLowerCase() + const arg = searchParams.get('arg') + const authFlowStep = ( + searchParams.get('auth_flow_step') || '' + ).toLowerCase() + + if (cmd === SSO_REDIRECT && arg) { + authLog(`Initialised with cmd: "${cmd}" and arg: "${arg}"`) - removeSearchParamsInBrowserHistory( - searchParamsToRemoveAfterAutoRedirect - ) + removeSearchParamsInBrowserHistory( + searchParamsToRemoveAfterAutoRedirect + ) - authRequestForSSO(arg) - } else if (authFlowStep === REDIRECT_URI) { - authLog('Handling auth_flow_step redirect') + authRequestForSSO(arg) + } else if (authFlowStep === REDIRECT_URI) { + authLog('Handling auth_flow_step redirect') - const host = result && result.bolt - const creds = await handleAuthFromRedirect(() => undefined) - return { type: DONE, discovered: { host, ...creds.credentials } } + creds = await handleAuthFromRedirect(() => undefined) + } + } else { + const errMsg = 'No SSO providers found on endpoint' + authLog(errMsg) } let host = result && result.bolt @@ -260,8 +262,8 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { const supportsMultiDb = !isAura && parseInt((result.neo4j_version || '0').charAt(0)) >= 4 const discovered = supportsMultiDb - ? { supportsMultiDb, host } - : { host } + ? { supportsMultiDb, host, ...creds } + : { host, ...creds } return { type: DONE, discovered } }) From 0a9a9297c33306f41c2f19c5ec1da77131875034 Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Tue, 20 Jul 2021 13:30:43 +0200 Subject: [PATCH 07/48] Add helper fns to discovery duck --- src/browser/AppInit.tsx | 4 +-- src/shared/modules/auth/common.js | 15 +++++++-- src/shared/modules/discovery/discoveryDuck.ts | 31 ++++++++----------- 3 files changed, 28 insertions(+), 22 deletions(-) diff --git a/src/browser/AppInit.tsx b/src/browser/AppInit.tsx index e05f30f40e5..33b0de0a54e 100644 --- a/src/browser/AppInit.tsx +++ b/src/browser/AppInit.tsx @@ -55,7 +55,7 @@ import { DndProvider } from 'react-dnd' import { HTML5Backend } from 'react-dnd-html5-backend' import { restoreSearchParams, - redirectedBackFromSSOServer + wasRedirectedBackFromSSOServer } from 'shared/modules/auth/common' // Configure localstorage sync @@ -207,7 +207,7 @@ const env = detectRuntimeEnv(window, NEO4J_CLOUD_DOMAINS) // To work around this they are stored in sessionStorage before // we redirect to the server, and then restore them when we get // redirected back -if (redirectedBackFromSSOServer()) { +if (wasRedirectedBackFromSSOServer()) { restoreSearchParams() } diff --git a/src/shared/modules/auth/common.js b/src/shared/modules/auth/common.js index f462b9175ed..b6b5da9b86b 100644 --- a/src/shared/modules/auth/common.js +++ b/src/shared/modules/auth/common.js @@ -3,6 +3,7 @@ import { isObject } from 'lodash' import { AUTH_STORAGE_SSO_PROVIDERS, AUTH_STORAGE_URL_SEARCH_PARAMS, + SSO_REDIRECT, REDIRECT_URI } from './constants' import { addSearchParamsInBrowserHistory, authLog } from './helpers' @@ -180,8 +181,18 @@ export const temporarlyStoreUrlSearchParams = () => { ) } -export const redirectedBackFromSSOServer = () => { - const searchParams = new URL(window.location.href).searchParams +export const shouldRedirectToSSOServer = ( + searchParams = new URL(window.location.href).searchParams +) => { + const cmd = (searchParams.get('cmd') || '').toLowerCase() + const arg = searchParams.get('arg') + + return cmd === SSO_REDIRECT && arg +} + +export const wasRedirectedBackFromSSOServer = ( + searchParams = new URL(window.location.href).searchParams +) => { const authFlowStep = (searchParams.get('auth_flow_step') || '').toLowerCase() return authFlowStep === REDIRECT_URI diff --git a/src/shared/modules/discovery/discoveryDuck.ts b/src/shared/modules/discovery/discoveryDuck.ts index 732d7c9f74d..7edcdbcc874 100644 --- a/src/shared/modules/discovery/discoveryDuck.ts +++ b/src/shared/modules/discovery/discoveryDuck.ts @@ -42,7 +42,11 @@ import { removeSearchParamsInBrowserHistory } from 'shared/modules/auth/helpers' import { REDIRECT_URI, SSO_REDIRECT } from 'shared/modules/auth/constants' -import { checkAndMergeSSOProviders } from 'shared/modules/auth/common' +import { + checkAndMergeSSOProviders, + shouldRedirectToSSOServer, + wasRedirectedBackFromSSOServer +} from 'shared/modules/auth/common' import { searchParamsToRemoveAfterAutoRedirect } from 'shared/modules/auth/settings' export const NAME = 'discover-bolt-host' @@ -218,34 +222,25 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { let creds: { username?: string; password?: string } = {} if (ssoProviders) { - authLog('SSO providers found on endpoint ') + authLog('SSO providers found on endpoint') checkAndMergeSSOProviders(ssoProviders, true) + const { searchParams } = new URL(window.location.href) - // TODO function should redirect to sso, - // TODO function was redirected back - const searchParams = new URL(window.location.href).searchParams - const cmd = (searchParams.get('cmd') || '').toLowerCase() - const arg = searchParams.get('arg') - const authFlowStep = ( - searchParams.get('auth_flow_step') || '' - ).toLowerCase() - - if (cmd === SSO_REDIRECT && arg) { - authLog(`Initialised with cmd: "${cmd}" and arg: "${arg}"`) + if (shouldRedirectToSSOServer(searchParams)) { + const idpId = searchParams.get('arg') + authLog(`Initialised with idpId: "${idpId}"`) removeSearchParamsInBrowserHistory( searchParamsToRemoveAfterAutoRedirect ) - - authRequestForSSO(arg) - } else if (authFlowStep === REDIRECT_URI) { + authRequestForSSO(idpId) + } else if (wasRedirectedBackFromSSOServer(searchParams)) { authLog('Handling auth_flow_step redirect') creds = await handleAuthFromRedirect(() => undefined) } } else { - const errMsg = 'No SSO providers found on endpoint' - authLog(errMsg) + authLog('No SSO providers found on endpoint') } let host = result && result.bolt From 023cf7b2ef3e04f914f0bb784e78a676b757af01 Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Tue, 20 Jul 2021 13:35:16 +0200 Subject: [PATCH 08/48] cleanup --- src/browser/AppInit.tsx | 1 + src/shared/modules/auth/index.js | 4 +--- src/shared/modules/discovery/discoveryDuck.ts | 11 ++++++++--- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/browser/AppInit.tsx b/src/browser/AppInit.tsx index 33b0de0a54e..1a2a38e5420 100644 --- a/src/browser/AppInit.tsx +++ b/src/browser/AppInit.tsx @@ -213,6 +213,7 @@ if (wasRedirectedBackFromSSOServer()) { // URL we're on const url = window.location.href + const searchParams = new URL(url).searchParams // Desktop/Relate params diff --git a/src/shared/modules/auth/index.js b/src/shared/modules/auth/index.js index 8249d07582f..ed9cd4d78f8 100644 --- a/src/shared/modules/auth/index.js +++ b/src/shared/modules/auth/index.js @@ -107,7 +107,7 @@ export const authRequestForSSO = idpId => { } } -export const handleAuthFromRedirect = sendSignal => +export const handleAuthFromRedirect = () => new Promise((resolve, reject) => { const { idp_id: idpId, @@ -155,7 +155,6 @@ export const handleAuthFromRedirect = sendSignal => { access_token: accessToken, id_token: idToken }, idpId ) - sendSignal({ type: 'RETRY', credentials }) resolve(credentials) } else { authLog('Attempting to fetch token information in "PKCE flow"') @@ -176,7 +175,6 @@ export const handleAuthFromRedirect = sendSignal => console.log('SSO:: PKCE SUCCESS RESULT', result) const credentials = getCredentialsFromAuthResult(result, idpId) - sendSignal({ type: 'RETRY', credentials }) resolve(credentials) } }) diff --git a/src/shared/modules/discovery/discoveryDuck.ts b/src/shared/modules/discovery/discoveryDuck.ts index 7edcdbcc874..16fbf006b3d 100644 --- a/src/shared/modules/discovery/discoveryDuck.ts +++ b/src/shared/modules/discovery/discoveryDuck.ts @@ -218,7 +218,9 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { ] }) .then(async result => { - const ssoProviders = result.sso_providers //|| result.ssoproviders || result.ssoProviders + const ssoProviders = + // @ts-ignore + result.sso_providers || result.ssoproviders || result.ssoProviders let creds: { username?: string; password?: string } = {} if (ssoProviders) { @@ -237,13 +239,16 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { } else if (wasRedirectedBackFromSSOServer(searchParams)) { authLog('Handling auth_flow_step redirect') - creds = await handleAuthFromRedirect(() => undefined) + creds = await handleAuthFromRedirect() } } else { authLog('No SSO providers found on endpoint') } - let host = result && result.bolt + let host = + result && + // @ts-ignore + (result.bolt_routing || result.bolt_direct || result.bolt) // Try to get info from server if (!host) { throw new Error('No bolt address found') From 3bdccf87675047d8fc490ad3c6932c2866c95478 Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Tue, 20 Jul 2021 13:40:04 +0200 Subject: [PATCH 09/48] Touch up connect frame --- .../modules/Stream/Auth/ConnectForm.tsx | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/browser/modules/Stream/Auth/ConnectForm.tsx b/src/browser/modules/Stream/Auth/ConnectForm.tsx index 6fb8cb81d48..8a19155814d 100644 --- a/src/browser/modules/Stream/Auth/ConnectForm.tsx +++ b/src/browser/modules/Stream/Auth/ConnectForm.tsx @@ -125,14 +125,19 @@ export default function ConnectForm(props: ConnectFormProps): JSX.Element { return ( - {ssoProviders.map((provider: any) => ( - - ))} + {ssoProviders.length && ( +
+ SSO server{ssoProviders.length > 1 ? 's' : ''} detected: + {ssoProviders.map((provider: any) => ( + authRequestForSSO(provider.id)} + > + {provider.name} + + ))} +
+ )} Connect URL From f1e574ee438cfa93d14a2142afc8c9fb0d5c0197 Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Tue, 20 Jul 2021 14:00:47 +0200 Subject: [PATCH 10/48] Respect discoveryURL param --- .../modules/Stream/Auth/ConnectForm.tsx | 4 +- src/shared/modules/discovery/discoveryDuck.ts | 95 +++++++++++-------- 2 files changed, 56 insertions(+), 43 deletions(-) diff --git a/src/browser/modules/Stream/Auth/ConnectForm.tsx b/src/browser/modules/Stream/Auth/ConnectForm.tsx index 8a19155814d..af07077dd0f 100644 --- a/src/browser/modules/Stream/Auth/ConnectForm.tsx +++ b/src/browser/modules/Stream/Auth/ConnectForm.tsx @@ -125,7 +125,7 @@ export default function ConnectForm(props: ConnectFormProps): JSX.Element { return ( - {ssoProviders.length && ( + {ssoProviders.length ? (
SSO server{ssoProviders.length > 1 ? 's' : ''} detected: {ssoProviders.map((provider: any) => ( @@ -137,7 +137,7 @@ export default function ConnectForm(props: ConnectFormProps): JSX.Element { ))}
- )} + ) : null} Connect URL diff --git a/src/shared/modules/discovery/discoveryDuck.ts b/src/shared/modules/discovery/discoveryDuck.ts index 16fbf006b3d..9605ce522fb 100644 --- a/src/shared/modules/discovery/discoveryDuck.ts +++ b/src/shared/modules/discovery/discoveryDuck.ts @@ -19,14 +19,17 @@ */ import Rx from 'rxjs/Rx' +import remote from 'services/remote' import { updateConnection } from 'shared/modules/connections/connectionsDuck' import { APP_START, USER_CLEAR, hasDiscoveryEndpoint, + getHostedUrl, getAllowedBoltSchemes, CLOUD_SCHEMES } from 'shared/modules/app/appDuck' +import { getDiscoveryEndpoint } from 'services/bolt/boltHelpers' import { getUrlParamValue } from 'services/utils' import { generateBoltUrl } from 'services/boltscheme.utils' import { getUrlInfo } from 'shared/services/utils' @@ -41,7 +44,6 @@ import { authLog, removeSearchParamsInBrowserHistory } from 'shared/modules/auth/helpers' -import { REDIRECT_URI, SSO_REDIRECT } from 'shared/modules/auth/constants' import { checkAndMergeSSOProviders, shouldRedirectToSSOServer, @@ -145,21 +147,30 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { .ofType(APP_START) .map((action: any) => { if (!action.url) return action + const { searchParams } = new URL(action.url) + const passedURL = - getUrlParamValue('dbms', action.url) || - getUrlParamValue('connectURL', action.url) + searchParams.get('dbms') || searchParams.get('connectURL') + + const passedDb = searchParams.get('db') + + if (passedURL) { + action.forceURL = decodeURIComponent(passedURL) + action.requestedUseDb = passedDb + } + + const discoveryURL = searchParams.get('discoveryURL') - const passedDb = getUrlParamValue('db', action.url) + if (discoveryURL) { + action.discoveryURL = discoveryURL + } - if (!passedURL || !passedURL.length) return action - action.forceURL = decodeURIComponent(passedURL[0]) - action.requestedUseDb = passedDb && passedDb[0] return action }) .merge(some$.ofType(USER_CLEAR)) .mergeMap((action: any) => { // Only when in a environment were we can guess discovery endpoint - if (!hasDiscoveryEndpoint(store.getState())) { + if (!action.discoveryURL || !hasDiscoveryEndpoint(store.getState())) { return Promise.resolve({ type: 'NOOP' }) } if (action.forceURL) { @@ -184,39 +195,41 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { }) } return Rx.Observable.fromPromise( - //remote - // .getJSON(getDiscoveryEndpoint(getHostedUrl(store.getState()))) - - // Uncomment below and comment out above when doing manual tests in dev mode to - // fake discovery response - Promise.resolve({ - bolt: 'bolt://localhost:7687', - neo4j_version: '4', - sso_providers: [ - { - id: 'keycloak-oidc', - name: 'KeyCloak', - auth_flow: 'pkce', - auth_endpoint: - 'http://localhost:18080/auth/realms/myrealm/protocol/openid-connect/auth', - token_endpoint: - 'http://localhost:18080/auth/realms/myrealm/protocol/openid-connect/token', - well_known_discovery_uri: - 'http://localhost:18080/auth/realms/myrealm/.well-known/openid-configuration', - params: { - client_id: 'account', - redirect_uri: - 'http://localhost:8080?idp_id=keycloak-oidc&auth_flow_step=redirect_uri', - response_type: 'code', - scope: 'openid groups' - }, - config: { - principal: 'preferred_username', - code_challenge_method: 'S256' - } - } - ] - }) + remote + .getJSON( + action.discoveryURL || + getDiscoveryEndpoint(getHostedUrl(store.getState())) + ) + // Uncomment below and comment out above when doing manual tests in dev mode to + // fake discovery response + //Promise.resolve({ + //bolt: 'bolt://localhost:7687', + //neo4j_version: '4', + //sso_providers: [ + //{ + //id: 'keycloak-oidc', + //name: 'KeyCloak', + //auth_flow: 'pkce', + //auth_endpoint: + //'http://localhost:18080/auth/realms/myrealm/protocol/openid-connect/auth', + //token_endpoint: + //'http://localhost:18080/auth/realms/myrealm/protocol/openid-connect/token', + //well_known_discovery_uri: + //'http://localhost:18080/auth/realms/myrealm/.well-known/openid-configuration', + //params: { + //client_id: 'account', + //redirect_uri: + //'http://localhost:8080?idp_id=keycloak-oidc&auth_flow_step=redirect_uri', + //response_type: 'code', + //scope: 'openid groups' + //}, + //config: { + //principal: 'preferred_username', + //code_challenge_method: 'S256' + //} + //} + //] + //}) .then(async result => { const ssoProviders = // @ts-ignore From 007dd5c20cacab162dfd8d0e34a47eb7142b373e Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Tue, 20 Jul 2021 14:09:36 +0200 Subject: [PATCH 11/48] Borrow bloom auth debugger fn --- src/shared/modules/auth/common.js | 5 ++--- src/shared/modules/auth/helpers.js | 8 +++++++- src/shared/modules/auth/index.js | 9 ++++----- src/shared/modules/auth/settings.js | 1 + 4 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/shared/modules/auth/common.js b/src/shared/modules/auth/common.js index b6b5da9b86b..99395c7c306 100644 --- a/src/shared/modules/auth/common.js +++ b/src/shared/modules/auth/common.js @@ -6,7 +6,7 @@ import { SSO_REDIRECT, REDIRECT_URI } from './constants' -import { addSearchParamsInBrowserHistory, authLog } from './helpers' +import { addSearchParamsInBrowserHistory, authLog, authDebug } from './helpers' import { mandatoryKeysForSSOProviderParams, mandatoryKeysForSSOProviders @@ -117,8 +117,7 @@ export const getCredentialsFromAuthResult = (result, idpId) => { const _parseJWTAndSetCredentials = toParseToken => { const parsedJWT = jwtDecode(toParseToken) - // TODO: remove this - console.log('Parsed JWT: ', parsedJWT) + authDebug('Credentials, parsed JWT', parsedJWT) if (!parsedJWT) { authLog(`Could not parse JWT for idp_id: ${idpId}, aborting`, 'warn') diff --git a/src/shared/modules/auth/helpers.js b/src/shared/modules/auth/helpers.js index 01450d467d0..2decb348bb9 100644 --- a/src/shared/modules/auth/helpers.js +++ b/src/shared/modules/auth/helpers.js @@ -1,6 +1,6 @@ import { isObject } from 'lodash' import { AUTH_LOGGING_PREFIX } from './constants' -import { isAuthLoggingEnabled } from './settings' +import { isAuthLoggingEnabled, isAuthDebuggingEnabled } from './settings' export const authLog = (msg, type = 'log') => { if (!isAuthLoggingEnabled) return @@ -8,6 +8,12 @@ export const authLog = (msg, type = 'log') => { console[type](`${AUTH_LOGGING_PREFIX} ${msg}`) } +export const authDebug = (msg, content) => { + if (!isAuthDebuggingEnabled) return + console.log(`${AUTH_LOGGING_PREFIX} - DEBUG - ${msg}`) + console.dir(content) +} + export const createNonce = () => Array.from(window.crypto.getRandomValues(new Uint32Array(4)), t => t.toString(36) diff --git a/src/shared/modules/auth/index.js b/src/shared/modules/auth/index.js index ed9cd4d78f8..2da9a77756c 100644 --- a/src/shared/modules/auth/index.js +++ b/src/shared/modules/auth/index.js @@ -17,6 +17,7 @@ import { createCodeVerifier, createCodeChallenge, authLog, + authDebug, removeSearchParamsInBrowserHistory } from './helpers' import { searchParamsToRemoveAfterAuthRedirect } from './settings' @@ -148,8 +149,8 @@ export const handleAuthFromRedirect = () => if ((tokenType || '').toLowerCase() === BEARER && accessToken) { authLog('Successfully aquired access_token in "implict flow"') - // TODO: remove this - console.log('SSO:: IMPLICT SUCCESS RESULT', [accessToken, idToken]) + authDebug('Implicit flow id_token', idToken) + authDebug('Implicit flow access_token', accessToken) const credentials = getCredentialsFromAuthResult( { access_token: accessToken, id_token: idToken }, @@ -170,9 +171,7 @@ export const handleAuthFromRedirect = () => reject(errorMsg) } else { authLog('Successfully aquired token results') - - // TODO: remove this - console.log('SSO:: PKCE SUCCESS RESULT', result) + authDebug('PKCE flow result', result) const credentials = getCredentialsFromAuthResult(result, idpId) resolve(credentials) diff --git a/src/shared/modules/auth/settings.js b/src/shared/modules/auth/settings.js index 794960bc034..23b363d413f 100644 --- a/src/shared/modules/auth/settings.js +++ b/src/shared/modules/auth/settings.js @@ -30,3 +30,4 @@ export const searchParamsToSaveForAfterAuthRedirect = [ ] export const isAuthLoggingEnabled = true // TODO: create a setting? or just check the NODE_ENV? always log warn and error? +export const isAuthDebuggingEnabled = true // TODO: check the NODE_ENV, shall only work in development From 972c5bedda65dedc2b5eaf383b411e44a1bb05b5 Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Sat, 24 Jul 2021 12:01:42 +0200 Subject: [PATCH 12/48] Updated auth code from bloom --- src/shared/modules/auth/common.js | 109 +++++++++--------- src/shared/modules/auth/helpers.js | 10 +- src/shared/modules/auth/helpers.test.js | 44 ++++++- src/shared/modules/auth/index.js | 15 ++- src/shared/modules/auth/settings.js | 9 +- src/shared/modules/discovery/discoveryDuck.ts | 4 +- 6 files changed, 117 insertions(+), 74 deletions(-) diff --git a/src/shared/modules/auth/common.js b/src/shared/modules/auth/common.js index 99395c7c306..6f0ab64778c 100644 --- a/src/shared/modules/auth/common.js +++ b/src/shared/modules/auth/common.js @@ -3,11 +3,13 @@ import { isObject } from 'lodash' import { AUTH_STORAGE_SSO_PROVIDERS, AUTH_STORAGE_URL_SEARCH_PARAMS, - SSO_REDIRECT, - REDIRECT_URI + REDIRECT_URI, + SSO_REDIRECT } from './constants' import { addSearchParamsInBrowserHistory, authLog, authDebug } from './helpers' import { + defaultTokenTypeAuthentication, + defaultTokenTypePrincipal, mandatoryKeysForSSOProviderParams, mandatoryKeysForSSOProviders } from './settings' @@ -97,6 +99,13 @@ export const getSSOProvidersFromStorage = () => { authLog('No SSO providers in (local) storage found') return [] } + if (!window.isSecureContext) { + authLog( + 'This application is NOT executed in a secure context. SSO support is therefore disabled. Load the application in a secure context to proceed with SSO.', + 'warn' + ) + return [] + } return ssoProviders } @@ -114,55 +123,50 @@ export const getSSOProviderByIdpId = idpId => { } export const getCredentialsFromAuthResult = (result, idpId) => { - const _parseJWTAndSetCredentials = toParseToken => { - const parsedJWT = jwtDecode(toParseToken) - - authDebug('Credentials, parsed JWT', parsedJWT) - - if (!parsedJWT) { - authLog(`Could not parse JWT for idp_id: ${idpId}, aborting`, 'warn') - return { username: '', password: '' } - } - - const email = parsedJWT[principal] || parsedJWT.email || parsedJWT.sub - authLog(`Credentials assembly, username: ${email}`) - - return { username: email, password: result.access_token } - } - - let credentials = {} + const emptyCredentials = { username: '', password: '' } authLog(`Attempting to assemble credentials for idp_id: ${idpId}`) if (!result || !idpId) { authLog('No result or idp_id passed in', 'warn') - return credentials + return emptyCredentials } const selectedSSOProvider = getSSOProviderByIdpId(idpId) - if (!selectedSSOProvider) return + if (!selectedSSOProvider) return emptyCredentials - const principal = selectedSSOProvider.config?.principal || '' - authLog(`Credentials, principal: ${principal}`) + const tokenTypePrincipal = + selectedSSOProvider.config?.['token_type_principal'] || + defaultTokenTypePrincipal + authLog( + `Credentials, using token type "${tokenTypePrincipal}" to retrieve principal` + ) - switch (idpId) { - case 'google-oidc': - if (!result.id_token) { - authLog('No id_token in google-oidc result!', 'warn') - authLog( - `We do not support auth_flow: "${selectedSSOProvider.auth_flow}" for idp_id: "${idpId}" at the moment`, - 'warn' - ) - // INFO: Another HTTP call would be needed to get an id_token. - credentials = { username: '', password: '' } - break - } - credentials = _parseJWTAndSetCredentials(result.id_token) - break - default: - credentials = _parseJWTAndSetCredentials(result.access_token) - break + const parsedJWT = jwtDecode(result[tokenTypePrincipal]) + authDebug('Credentials, parsed JWT', parsedJWT) + + if (!parsedJWT) { + authLog( + `Could not parse JWT of type "${tokenTypePrincipal}" for idp_id "${idpId}", aborting`, + 'warn' + ) + return emptyCredentials } - return credentials + + const principal = selectedSSOProvider.config?.principal || '' + authLog(`Credentials, provided principal in config: ${principal}`) + + const credsPrincipal = + parsedJWT[principal] || parsedJWT.email || parsedJWT.sub + authLog(`Credentials assembly with username: ${credsPrincipal}`) + + const tokenTypeAuthentication = + selectedSSOProvider.config?.['token_type_authentication'] || + defaultTokenTypeAuthentication + authLog( + `Credentials assembly with token type "${tokenTypeAuthentication}" as password` + ) + + return { username: credsPrincipal, password: result[tokenTypeAuthentication] } } export const temporarlyStoreUrlSearchParams = () => { @@ -173,42 +177,33 @@ export const temporarlyStoreUrlSearchParams = () => { currentBrowserURLParams )}"` ) - window.sessionStorage.setItem( AUTH_STORAGE_URL_SEARCH_PARAMS, JSON.stringify(currentBrowserURLParams) ) } -export const shouldRedirectToSSOServer = ( - searchParams = new URL(window.location.href).searchParams -) => { - const cmd = (searchParams.get('cmd') || '').toLowerCase() - const arg = searchParams.get('arg') - - return cmd === SSO_REDIRECT && arg +export const shouldRedirectToSSOServer = () => { + const { cmd, arg } = getInitialisationParameters() + return (cmd || '').toLowerCase() === SSO_REDIRECT && arg } -export const wasRedirectedBackFromSSOServer = ( - searchParams = new URL(window.location.href).searchParams -) => { - const authFlowStep = (searchParams.get('auth_flow_step') || '').toLowerCase() - - return authFlowStep === REDIRECT_URI +export const wasRedirectedBackFromSSOServer = () => { + const { auth_flow_step: authFlowStep } = getInitialisationParameters() + return (authFlowStep || '').toLowerCase() === REDIRECT_URI } export const restoreSearchParams = () => { authLog(`Retrieving temporarly stored url search params`) - try { const storedParams = JSON.parse( window.sessionStorage.getItem(AUTH_STORAGE_URL_SEARCH_PARAMS) ) + window.sessionStorage.setItem(AUTH_STORAGE_URL_SEARCH_PARAMS, '') if (isObject(storedParams)) { addSearchParamsInBrowserHistory(storedParams) - return storedParams } else { authLog('Invalid temporarly stored url search params') diff --git a/src/shared/modules/auth/helpers.js b/src/shared/modules/auth/helpers.js index 2decb348bb9..0b9fb1afb07 100644 --- a/src/shared/modules/auth/helpers.js +++ b/src/shared/modules/auth/helpers.js @@ -72,6 +72,7 @@ export const createCodeChallenge = async (method, codeVerifier) => { export const addSearchParamsInBrowserHistory = paramsToAddObj => { if (!paramsToAddObj || !isObject(paramsToAddObj)) return + const crntHashParams = window.location.hash || undefined const searchParams = new URLSearchParams(window.location.search) Object.entries(paramsToAddObj).forEach(([key, value]) => { if (key && value && value.length) { @@ -79,13 +80,16 @@ export const addSearchParamsInBrowserHistory = paramsToAddObj => { } }) - const newUrl = `${window.location.origin}?${searchParams.toString()}` + const newUrl = `${ + window.location.origin + }?${searchParams.toString()}${crntHashParams || ''}` window.history.replaceState({ path: newUrl }, '', newUrl) } export const removeSearchParamsInBrowserHistory = paramsToRemove => { if (!paramsToRemove || !paramsToRemove.length) return + const crntHashParams = window.location.hash || undefined const currentUrlSearchParams = new URLSearchParams(window.location.search) const cleansedSearchParams = {} for (const [key, value] of currentUrlSearchParams.entries()) { @@ -95,7 +99,9 @@ export const removeSearchParamsInBrowserHistory = paramsToRemove => { } const newUrlSearchParams = new URLSearchParams(cleansedSearchParams) - const newUrl = `${window.location.origin}?${newUrlSearchParams.toString()}` + const newUrl = `${ + window.location.origin + }?${newUrlSearchParams.toString()}${crntHashParams || ''}` window.history.replaceState({}, '', newUrl) } diff --git a/src/shared/modules/auth/helpers.test.js b/src/shared/modules/auth/helpers.test.js index 1590fec0af6..3b46a8b6495 100644 --- a/src/shared/modules/auth/helpers.test.js +++ b/src/shared/modules/auth/helpers.test.js @@ -22,7 +22,7 @@ describe('addSearchParamsInBrowserHistory', () => { expect(window.location.href).toEqual(originalWindowLocationHref + '?') }) - test('simple search param', () => { + test('simple search parameter', () => { expect(window.location.href).toEqual(originalWindowLocationHref) addSearchParamsInBrowserHistory({ rest: 'test' }) expect(window.location.href).toEqual( @@ -30,7 +30,7 @@ describe('addSearchParamsInBrowserHistory', () => { ) }) - test('search param with special characters', () => { + test('search parameter with special characters', () => { expect(window.location.href).toEqual(originalWindowLocationHref) addSearchParamsInBrowserHistory({ test_entry: '5%73bsod?892()€%&#"€' }) expect(window.location.href).toEqual( @@ -39,7 +39,7 @@ describe('addSearchParamsInBrowserHistory', () => { ) }) - test('multiple search params', () => { + test('multiple search parameters', () => { expect(window.location.href).toEqual(originalWindowLocationHref) addSearchParamsInBrowserHistory({ test_entry: '5%73bsod?892()€%&#"€', @@ -50,6 +50,22 @@ describe('addSearchParamsInBrowserHistory', () => { '?test_entry=5%2573bsod%3F892%28%29%E2%82%AC%25%26%23%22%E2%82%AC&entryUrl=http%3A%2F%2Flocalhost%3A8043%2Ftest%2F%3Fentry%3Dboat' ) }) + + test('keeps URL hash parameters', () => { + const hashUrlParams = '#code=df56&code_verifier=rt43' + window.history.replaceState( + {}, + '', + originalWindowLocationHref + hashUrlParams + ) + expect(window.location.href).toEqual( + originalWindowLocationHref + hashUrlParams + ) + addSearchParamsInBrowserHistory({ rest: 'test' }) + expect(window.location.href).toEqual( + originalWindowLocationHref + '?rest=test' + hashUrlParams + ) + }) }) describe('removeSearchParamsInBrowserHistory', () => { @@ -72,21 +88,37 @@ describe('removeSearchParamsInBrowserHistory', () => { expect(window.location.href).toEqual(originalWindowLocationHref) }) - test('non-existing param in browser history', () => { + test('non-existing parameter in browser history', () => { expect(window.location.href).toEqual(originalWindowLocationHref) removeSearchParamsInBrowserHistory(['kode']) expect(window.location.href).toEqual(originalWindowLocationHref) }) - test('remove existing param from browser history', () => { + test('remove existing parameter from browser history', () => { expect(window.location.href).toEqual(originalWindowLocationHref) removeSearchParamsInBrowserHistory(['test_param']) expect(window.location.href).toEqual('http://localhost/?code=843cvg') }) - test('remove all existing param from browser history', () => { + test('remove all existing parameters from browser history', () => { expect(window.location.href).toEqual(originalWindowLocationHref) removeSearchParamsInBrowserHistory(['test_param', 'code']) expect(window.location.href).toEqual('http://localhost/?') }) + + test('keeps URL hash parameters', () => { + const hashUrlParams = '#box=/&%#/=Q4535&state_test=ljhf873' + window.history.replaceState( + {}, + '', + originalWindowLocationHref + hashUrlParams + ) + expect(window.location.href).toEqual( + originalWindowLocationHref + hashUrlParams + ) + removeSearchParamsInBrowserHistory(['test_param']) + expect(window.location.href).toEqual( + 'http://localhost/?code=843cvg' + hashUrlParams + ) + }) }) diff --git a/src/shared/modules/auth/index.js b/src/shared/modules/auth/index.js index 2da9a77756c..690fe5e8596 100644 --- a/src/shared/modules/auth/index.js +++ b/src/shared/modules/auth/index.js @@ -20,7 +20,11 @@ import { authDebug, removeSearchParamsInBrowserHistory } from './helpers' -import { searchParamsToRemoveAfterAuthRedirect } from './settings' +import { + defaultCodeChallengeMethod, + defaultGrantType, + searchParamsToRemoveAfterAuthRedirect +} from './settings' export const authRequestForSSO = idpId => { const selectedSSOProvider = getSSOProviderByIdpId(idpId) @@ -82,7 +86,8 @@ export const authRequestForSSO = idpId => { } if (selectedSSOProvider.auth_flow === PKCE) { - const codeChallengeMethod = ssoConfig.code_challenge_method || 'S256' + const codeChallengeMethod = + ssoConfig.code_challenge_method || defaultCodeChallengeMethod authLog( `Auth flow "PKCE", using code_challenge_method: "${codeChallengeMethod}"` ) @@ -101,7 +106,7 @@ export const authRequestForSSO = idpId => { } ) } else if (selectedSSOProvider.auth_flow === IMPLICIT) { - authLog('Auth flow "implicit"') + authLog('Auth flow "implicit flow"') _submitForm(form, params) } else { authLog(`Auth flow "${selectedSSOProvider.auth_flow}" is not supported.`) @@ -147,7 +152,7 @@ export const handleAuthFromRedirect = () => window.sessionStorage.setItem(AUTH_STORAGE_STATE, '') if ((tokenType || '').toLowerCase() === BEARER && accessToken) { - authLog('Successfully aquired access_token in "implict flow"') + authLog('Successfully aquired access_token in "implicit flow"') authDebug('Implicit flow id_token', idToken) authDebug('Implicit flow access_token', accessToken) @@ -192,7 +197,7 @@ export const authRequestForToken = (idpId, code) => { const ssoParams = selectedSSOProvider.params || {} let details = { - grant_type: 'authorization_code', + grant_type: defaultGrantType, client_id: ssoParams.client_id, redirect_uri: ssoParams.redirect_uri, code_verifier: window.sessionStorage.getItem(AUTH_STORAGE_CODE_VERIFIER), diff --git a/src/shared/modules/auth/settings.js b/src/shared/modules/auth/settings.js index 23b363d413f..89d550c6fca 100644 --- a/src/shared/modules/auth/settings.js +++ b/src/shared/modules/auth/settings.js @@ -29,5 +29,10 @@ export const searchParamsToSaveForAfterAuthRedirect = [ 'run' ] -export const isAuthLoggingEnabled = true // TODO: create a setting? or just check the NODE_ENV? always log warn and error? -export const isAuthDebuggingEnabled = true // TODO: check the NODE_ENV, shall only work in development +export const defaultTokenTypePrincipal = 'access_token' +export const defaultTokenTypeAuthentication = 'access_token' +export const defaultGrantType = 'authorization_code' +export const defaultCodeChallengeMethod = 'S256' + +export const isAuthLoggingEnabled = true +export const isAuthDebuggingEnabled = process.env.NODE_ENV !== 'production' diff --git a/src/shared/modules/discovery/discoveryDuck.ts b/src/shared/modules/discovery/discoveryDuck.ts index 9605ce522fb..dfb6da21af5 100644 --- a/src/shared/modules/discovery/discoveryDuck.ts +++ b/src/shared/modules/discovery/discoveryDuck.ts @@ -241,7 +241,7 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { checkAndMergeSSOProviders(ssoProviders, true) const { searchParams } = new URL(window.location.href) - if (shouldRedirectToSSOServer(searchParams)) { + if (shouldRedirectToSSOServer()) { const idpId = searchParams.get('arg') authLog(`Initialised with idpId: "${idpId}"`) @@ -249,7 +249,7 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { searchParamsToRemoveAfterAutoRedirect ) authRequestForSSO(idpId) - } else if (wasRedirectedBackFromSSOServer(searchParams)) { + } else if (wasRedirectedBackFromSSOServer()) { authLog('Handling auth_flow_step redirect') creds = await handleAuthFromRedirect() From 60b1e7934297f01c188b5e817cf8cb36daa0cc7f Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Sat, 24 Jul 2021 12:36:57 +0200 Subject: [PATCH 13/48] Fix keycloak implicit flow --- src/shared/modules/auth/common.js | 11 +++++++---- src/shared/modules/discovery/discoveryDuck.ts | 7 +++++-- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/shared/modules/auth/common.js b/src/shared/modules/auth/common.js index 6f0ab64778c..679aa2f1398 100644 --- a/src/shared/modules/auth/common.js +++ b/src/shared/modules/auth/common.js @@ -14,14 +14,17 @@ import { mandatoryKeysForSSOProviders } from './settings' -export const getInitialisationParameters = ( - urlSearchParams = window.location.search -) => { - const initParams = {} +export const getInitialisationParameters = () => { + const urlSearchParams = window.location.search + const urlHashParamsAsSearchParams = '?' + window.location.hash.substring(1) + const initParams = {} new URLSearchParams(urlSearchParams).forEach((value, key) => { initParams[key] = value }) + new URLSearchParams(urlHashParamsAsSearchParams).forEach((value, key) => { + initParams[key] = value + }) return initParams } diff --git a/src/shared/modules/discovery/discoveryDuck.ts b/src/shared/modules/discovery/discoveryDuck.ts index dfb6da21af5..60943ca18e7 100644 --- a/src/shared/modules/discovery/discoveryDuck.ts +++ b/src/shared/modules/discovery/discoveryDuck.ts @@ -30,7 +30,6 @@ import { CLOUD_SCHEMES } from 'shared/modules/app/appDuck' import { getDiscoveryEndpoint } from 'services/bolt/boltHelpers' -import { getUrlParamValue } from 'services/utils' import { generateBoltUrl } from 'services/boltscheme.utils' import { getUrlInfo } from 'shared/services/utils' import { isConnectedAuraHost } from 'shared/modules/connections/connectionsDuck' @@ -252,7 +251,11 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { } else if (wasRedirectedBackFromSSOServer()) { authLog('Handling auth_flow_step redirect') - creds = await handleAuthFromRedirect() + try { + creds = await handleAuthFromRedirect() + } catch (e) { + authLog(e) + } } } else { authLog('No SSO providers found on endpoint') From b35996cf9c1c467c6814ab385b1a29b0c2aa7e52 Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Mon, 26 Jul 2021 09:33:11 +0200 Subject: [PATCH 14/48] cleanup --- src/shared/modules/auth/index.js | 3 ++ src/shared/modules/discovery/discoveryDuck.ts | 30 ++----------------- 2 files changed, 6 insertions(+), 27 deletions(-) diff --git a/src/shared/modules/auth/index.js b/src/shared/modules/auth/index.js index 690fe5e8596..1463b9c10ba 100644 --- a/src/shared/modules/auth/index.js +++ b/src/shared/modules/auth/index.js @@ -112,6 +112,9 @@ export const authRequestForSSO = idpId => { authLog(`Auth flow "${selectedSSOProvider.auth_flow}" is not supported.`) } } +const removeHashParamsFromSessionStorage = () => { + searchParamsToRemoveAfterAuthRedirect.forEach(param) +} export const handleAuthFromRedirect = () => new Promise((resolve, reject) => { diff --git a/src/shared/modules/discovery/discoveryDuck.ts b/src/shared/modules/discovery/discoveryDuck.ts index 60943ca18e7..f217f613a20 100644 --- a/src/shared/modules/discovery/discoveryDuck.ts +++ b/src/shared/modules/discovery/discoveryDuck.ts @@ -203,35 +203,10 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { // fake discovery response //Promise.resolve({ //bolt: 'bolt://localhost:7687', - //neo4j_version: '4', - //sso_providers: [ - //{ - //id: 'keycloak-oidc', - //name: 'KeyCloak', - //auth_flow: 'pkce', - //auth_endpoint: - //'http://localhost:18080/auth/realms/myrealm/protocol/openid-connect/auth', - //token_endpoint: - //'http://localhost:18080/auth/realms/myrealm/protocol/openid-connect/token', - //well_known_discovery_uri: - //'http://localhost:18080/auth/realms/myrealm/.well-known/openid-configuration', - //params: { - //client_id: 'account', - //redirect_uri: - //'http://localhost:8080?idp_id=keycloak-oidc&auth_flow_step=redirect_uri', - //response_type: 'code', - //scope: 'openid groups' - //}, - //config: { - //principal: 'preferred_username', - //code_challenge_method: 'S256' - //} - //} - //] + //neo4j_version: '4' //}) .then(async result => { const ssoProviders = - // @ts-ignore result.sso_providers || result.ssoproviders || result.ssoProviders let creds: { username?: string; password?: string } = {} @@ -247,6 +222,8 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { removeSearchParamsInBrowserHistory( searchParamsToRemoveAfterAutoRedirect ) + // TODO check that stuff that shouldn't be in URL is not in url + // TODO UI -> errors authRequestForSSO(idpId) } else if (wasRedirectedBackFromSSOServer()) { authLog('Handling auth_flow_step redirect') @@ -263,7 +240,6 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { let host = result && - // @ts-ignore (result.bolt_routing || result.bolt_direct || result.bolt) // Try to get info from server if (!host) { From 2fb55901dbfbe8afb41244b7e54490b424eb5531 Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Mon, 26 Jul 2021 14:01:39 +0200 Subject: [PATCH 15/48] Add bloom updates --- src/shared/modules/auth/common.js | 1 - src/shared/modules/auth/helpers.js | 9 ++++----- src/shared/modules/auth/helpers.test.js | 6 ++---- src/shared/modules/auth/settings.js | 7 ------- 4 files changed, 6 insertions(+), 17 deletions(-) diff --git a/src/shared/modules/auth/common.js b/src/shared/modules/auth/common.js index 679aa2f1398..d84b70af50b 100644 --- a/src/shared/modules/auth/common.js +++ b/src/shared/modules/auth/common.js @@ -174,7 +174,6 @@ export const getCredentialsFromAuthResult = (result, idpId) => { export const temporarlyStoreUrlSearchParams = () => { const currentBrowserURLParams = getInitialisationParameters() - authLog( `Temporarly storing the url search params. data: "${JSON.stringify( currentBrowserURLParams diff --git a/src/shared/modules/auth/helpers.js b/src/shared/modules/auth/helpers.js index 0b9fb1afb07..27430acba50 100644 --- a/src/shared/modules/auth/helpers.js +++ b/src/shared/modules/auth/helpers.js @@ -72,7 +72,6 @@ export const createCodeChallenge = async (method, codeVerifier) => { export const addSearchParamsInBrowserHistory = paramsToAddObj => { if (!paramsToAddObj || !isObject(paramsToAddObj)) return - const crntHashParams = window.location.hash || undefined const searchParams = new URLSearchParams(window.location.search) Object.entries(paramsToAddObj).forEach(([key, value]) => { if (key && value && value.length) { @@ -80,10 +79,10 @@ export const addSearchParamsInBrowserHistory = paramsToAddObj => { } }) - const newUrl = `${ - window.location.origin - }?${searchParams.toString()}${crntHashParams || ''}` - window.history.replaceState({ path: newUrl }, '', newUrl) + const newUrlSearchParams = new URLSearchParams(cleansedSearchParams) + + const newUrl = `${window.location.origin}?${newUrlSearchParams.toString()}` + window.history.replaceState({}, '', newUrl) } export const removeSearchParamsInBrowserHistory = paramsToRemove => { diff --git a/src/shared/modules/auth/helpers.test.js b/src/shared/modules/auth/helpers.test.js index 3b46a8b6495..050f2b01bc0 100644 --- a/src/shared/modules/auth/helpers.test.js +++ b/src/shared/modules/auth/helpers.test.js @@ -51,7 +51,7 @@ describe('addSearchParamsInBrowserHistory', () => { ) }) - test('keeps URL hash parameters', () => { + test('does not keep URL hash parameters', () => { const hashUrlParams = '#code=df56&code_verifier=rt43' window.history.replaceState( {}, @@ -62,9 +62,7 @@ describe('addSearchParamsInBrowserHistory', () => { originalWindowLocationHref + hashUrlParams ) addSearchParamsInBrowserHistory({ rest: 'test' }) - expect(window.location.href).toEqual( - originalWindowLocationHref + '?rest=test' + hashUrlParams - ) + expect(window.location.href).toEqual('http://localhost/?code=843cvg') }) }) diff --git a/src/shared/modules/auth/settings.js b/src/shared/modules/auth/settings.js index 89d550c6fca..767439d3987 100644 --- a/src/shared/modules/auth/settings.js +++ b/src/shared/modules/auth/settings.js @@ -21,13 +21,6 @@ export const searchParamsToRemoveAfterAuthRedirect = [ 'session_state', 'code' ] -export const searchParamsToSaveForAfterAuthRedirect = [ - 'connectURL', - 'discoveryURL', - 'search', - 'perspective', - 'run' -] export const defaultTokenTypePrincipal = 'access_token' export const defaultTokenTypeAuthentication = 'access_token' From 673b96e1d30eff3546b419e00237ed07f804e36f Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Mon, 26 Jul 2021 14:26:56 +0200 Subject: [PATCH 16/48] Fix crashes --- src/shared/modules/auth/helpers.js | 2 +- src/shared/modules/auth/index.js | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/shared/modules/auth/helpers.js b/src/shared/modules/auth/helpers.js index 27430acba50..13df8d4621c 100644 --- a/src/shared/modules/auth/helpers.js +++ b/src/shared/modules/auth/helpers.js @@ -79,7 +79,7 @@ export const addSearchParamsInBrowserHistory = paramsToAddObj => { } }) - const newUrlSearchParams = new URLSearchParams(cleansedSearchParams) + const newUrlSearchParams = new URLSearchParams(searchParams) const newUrl = `${window.location.origin}?${newUrlSearchParams.toString()}` window.history.replaceState({}, '', newUrl) diff --git a/src/shared/modules/auth/index.js b/src/shared/modules/auth/index.js index 1463b9c10ba..7652addb32f 100644 --- a/src/shared/modules/auth/index.js +++ b/src/shared/modules/auth/index.js @@ -95,6 +95,7 @@ export const authRequestForSSO = idpId => { const codeVerifier = createCodeVerifier(codeChallengeMethod) window.sessionStorage.setItem(AUTH_STORAGE_CODE_VERIFIER, codeVerifier) + debugger createCodeChallenge(codeChallengeMethod, codeVerifier).then( codeChallenge => { params = { From e80fd11c18dc518e324ee2d77fbc3d6b5a8093e8 Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Mon, 26 Jul 2021 15:10:08 +0200 Subject: [PATCH 17/48] mellan --- src/browser/AppInit.tsx | 4 ++-- src/shared/modules/auth/common.js | 6 +++++- src/shared/modules/auth/helpers.js | 14 ++++++-------- src/shared/modules/auth/index.js | 4 ---- src/shared/modules/discovery/discoveryDuck.ts | 2 -- 5 files changed, 13 insertions(+), 17 deletions(-) diff --git a/src/browser/AppInit.tsx b/src/browser/AppInit.tsx index 1a2a38e5420..d5d565f4ff9 100644 --- a/src/browser/AppInit.tsx +++ b/src/browser/AppInit.tsx @@ -54,7 +54,7 @@ import { getUuid } from 'shared/modules/udc/udcDuck' import { DndProvider } from 'react-dnd' import { HTML5Backend } from 'react-dnd-html5-backend' import { - restoreSearchParams, + restoreSearchAndHashParamsParams, wasRedirectedBackFromSSOServer } from 'shared/modules/auth/common' @@ -208,7 +208,7 @@ const env = detectRuntimeEnv(window, NEO4J_CLOUD_DOMAINS) // we redirect to the server, and then restore them when we get // redirected back if (wasRedirectedBackFromSSOServer()) { - restoreSearchParams() + restoreSearchAndHashParamsParams() } // URL we're on diff --git a/src/shared/modules/auth/common.js b/src/shared/modules/auth/common.js index d84b70af50b..a8771b4a17c 100644 --- a/src/shared/modules/auth/common.js +++ b/src/shared/modules/auth/common.js @@ -195,7 +195,7 @@ export const wasRedirectedBackFromSSOServer = () => { return (authFlowStep || '').toLowerCase() === REDIRECT_URI } -export const restoreSearchParams = () => { +export const restoreSearchAndHashParamsParams = () => { authLog(`Retrieving temporarly stored url search params`) try { const storedParams = JSON.parse( @@ -204,8 +204,12 @@ export const restoreSearchParams = () => { window.sessionStorage.setItem(AUTH_STORAGE_URL_SEARCH_PARAMS, '') + debugger if (isObject(storedParams)) { + const crntHashParams = window.location.hash || undefined addSearchParamsInBrowserHistory(storedParams) + const newUrl = `${window.location.href}${crntHashParams || ''}` + window.history.replaceState({}, '', newUrl) return storedParams } else { authLog('Invalid temporarly stored url search params') diff --git a/src/shared/modules/auth/helpers.js b/src/shared/modules/auth/helpers.js index 13df8d4621c..de198b73bf3 100644 --- a/src/shared/modules/auth/helpers.js +++ b/src/shared/modules/auth/helpers.js @@ -72,6 +72,7 @@ export const createCodeChallenge = async (method, codeVerifier) => { export const addSearchParamsInBrowserHistory = paramsToAddObj => { if (!paramsToAddObj || !isObject(paramsToAddObj)) return + const crntHashParams = window.location.hash || undefined const searchParams = new URLSearchParams(window.location.search) Object.entries(paramsToAddObj).forEach(([key, value]) => { if (key && value && value.length) { @@ -79,16 +80,15 @@ export const addSearchParamsInBrowserHistory = paramsToAddObj => { } }) - const newUrlSearchParams = new URLSearchParams(searchParams) - - const newUrl = `${window.location.origin}?${newUrlSearchParams.toString()}` - window.history.replaceState({}, '', newUrl) + const newUrl = `${ + window.location.origin + }?${searchParams.toString()}${crntHashParams || ''}` + window.history.replaceState({ path: newUrl }, '', newUrl) } export const removeSearchParamsInBrowserHistory = paramsToRemove => { if (!paramsToRemove || !paramsToRemove.length) return - const crntHashParams = window.location.hash || undefined const currentUrlSearchParams = new URLSearchParams(window.location.search) const cleansedSearchParams = {} for (const [key, value] of currentUrlSearchParams.entries()) { @@ -98,9 +98,7 @@ export const removeSearchParamsInBrowserHistory = paramsToRemove => { } const newUrlSearchParams = new URLSearchParams(cleansedSearchParams) - const newUrl = `${ - window.location.origin - }?${newUrlSearchParams.toString()}${crntHashParams || ''}` + const newUrl = `${window.location.origin}?${newUrlSearchParams.toString()}` window.history.replaceState({}, '', newUrl) } diff --git a/src/shared/modules/auth/index.js b/src/shared/modules/auth/index.js index 7652addb32f..690fe5e8596 100644 --- a/src/shared/modules/auth/index.js +++ b/src/shared/modules/auth/index.js @@ -95,7 +95,6 @@ export const authRequestForSSO = idpId => { const codeVerifier = createCodeVerifier(codeChallengeMethod) window.sessionStorage.setItem(AUTH_STORAGE_CODE_VERIFIER, codeVerifier) - debugger createCodeChallenge(codeChallengeMethod, codeVerifier).then( codeChallenge => { params = { @@ -113,9 +112,6 @@ export const authRequestForSSO = idpId => { authLog(`Auth flow "${selectedSSOProvider.auth_flow}" is not supported.`) } } -const removeHashParamsFromSessionStorage = () => { - searchParamsToRemoveAfterAuthRedirect.forEach(param) -} export const handleAuthFromRedirect = () => new Promise((resolve, reject) => { diff --git a/src/shared/modules/discovery/discoveryDuck.ts b/src/shared/modules/discovery/discoveryDuck.ts index f217f613a20..4a893517f3a 100644 --- a/src/shared/modules/discovery/discoveryDuck.ts +++ b/src/shared/modules/discovery/discoveryDuck.ts @@ -222,8 +222,6 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { removeSearchParamsInBrowserHistory( searchParamsToRemoveAfterAutoRedirect ) - // TODO check that stuff that shouldn't be in URL is not in url - // TODO UI -> errors authRequestForSSO(idpId) } else if (wasRedirectedBackFromSSOServer()) { authLog('Handling auth_flow_step redirect') From 123d4ceb0291a2d32ed367a782bc871579598b03 Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Mon, 26 Jul 2021 19:48:10 +0200 Subject: [PATCH 18/48] Update tests --- src/shared/modules/auth/helpers.test.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/shared/modules/auth/helpers.test.js b/src/shared/modules/auth/helpers.test.js index 050f2b01bc0..96710f98ef0 100644 --- a/src/shared/modules/auth/helpers.test.js +++ b/src/shared/modules/auth/helpers.test.js @@ -51,7 +51,7 @@ describe('addSearchParamsInBrowserHistory', () => { ) }) - test('does not keep URL hash parameters', () => { + test('keeps URL hash parameters', () => { const hashUrlParams = '#code=df56&code_verifier=rt43' window.history.replaceState( {}, @@ -62,7 +62,9 @@ describe('addSearchParamsInBrowserHistory', () => { originalWindowLocationHref + hashUrlParams ) addSearchParamsInBrowserHistory({ rest: 'test' }) - expect(window.location.href).toEqual('http://localhost/?code=843cvg') + expect(window.location.href).toEqual( + originalWindowLocationHref + '?rest=test' + hashUrlParams + ) }) }) @@ -104,7 +106,7 @@ describe('removeSearchParamsInBrowserHistory', () => { expect(window.location.href).toEqual('http://localhost/?') }) - test('keeps URL hash parameters', () => { + test('does not keep URL hash parameters', () => { const hashUrlParams = '#box=/&%#/=Q4535&state_test=ljhf873' window.history.replaceState( {}, @@ -115,8 +117,6 @@ describe('removeSearchParamsInBrowserHistory', () => { originalWindowLocationHref + hashUrlParams ) removeSearchParamsInBrowserHistory(['test_param']) - expect(window.location.href).toEqual( - 'http://localhost/?code=843cvg' + hashUrlParams - ) + expect(window.location.href).toEqual('http://localhost/?code=843cvg') }) }) From d2911148576257438171f2bb72e24959e7f94f63 Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Mon, 26 Jul 2021 20:31:05 +0200 Subject: [PATCH 19/48] fix styling --- __reactpreview__/Wrapper.tsx | 8 + .../modules/Stream/Auth/ConnectForm.tsx | 247 ++++++++++-------- src/browser/modules/Stream/Auth/styled.tsx | 21 ++ 3 files changed, 160 insertions(+), 116 deletions(-) create mode 100644 __reactpreview__/Wrapper.tsx diff --git a/__reactpreview__/Wrapper.tsx b/__reactpreview__/Wrapper.tsx new file mode 100644 index 00000000000..02fa9d6e18e --- /dev/null +++ b/__reactpreview__/Wrapper.tsx @@ -0,0 +1,8 @@ +import React from 'react' + +// You can import global CSS files here. + +// No-op wrapper. +export const Wrapper: React.FC = ({ children }) => { + return <>{children} +} diff --git a/src/browser/modules/Stream/Auth/ConnectForm.tsx b/src/browser/modules/Stream/Auth/ConnectForm.tsx index af07077dd0f..2e7bc5003d8 100644 --- a/src/browser/modules/Stream/Auth/ConnectForm.tsx +++ b/src/browser/modules/Stream/Auth/ConnectForm.tsx @@ -28,7 +28,11 @@ import { StyledConnectionLabel, StyledConnectionFormEntry, StyledSegment, - StyledBoltUrlHintText + StyledBoltUrlHintText, + FormContainer, + SsoOptions, + SsoButtonContainer, + SsoError } from './styled' import { NATIVE, NO_AUTH } from 'services/bolt/boltHelpers' import { toKeyString } from 'services/utils' @@ -36,6 +40,8 @@ import { stripScheme, getScheme } from 'services/boltscheme.utils' import { AuthenticationMethod } from 'shared/modules/connections/connectionsDuck' import { authRequestForSSO } from 'shared/modules/auth/index.js' import { getSSOProvidersFromStorage } from 'shared/modules/auth/common.js' +import { H3 } from 'browser-components/headers' +import { StyledCypherErrorMessage } from '../styled' const readableauthenticationMethods: Record = { [NATIVE]: 'Username / Password', @@ -58,6 +64,7 @@ interface ConnectFormProps { username: string used: boolean supportsMultiDb: boolean + ssoError?: string } export default function ConnectForm(props: ConnectFormProps): JSX.Element { @@ -123,131 +130,139 @@ export default function ConnectForm(props: ConnectFormProps): JSX.Element { }:// for a direct connection to a single instance.` : '' + const { ssoError } = props return ( - - {ssoProviders.length ? ( -
- SSO server{ssoProviders.length > 1 ? 's' : ''} detected: + + {ssoProviders.length > 0 && ( + +

Single sign-on

{ssoProviders.map((provider: any) => ( - authRequestForSSO(provider.id)} - > - {provider.name} - + + authRequestForSSO(provider.id)}> + {provider.name} + + ))} -
- ) : null} - - - Connect URL - - {schemeRestriction ? ( - <> - - - {props.allowedSchemes.map(s => { - const schemeString = `${s}://` - return ( - - ) - })} - - - - - {hoverText} - - - ) : ( - - )} - - {props.supportsMultiDb && ( - - - Database - leave empty for default - - - + {ssoError && ( + + ERROR +
{ssoError}
+ + )} + )} - - {props.allowedAuthMethods.length > 1 && ( + - - Authentication type - - {props.allowedAuthMethods.map(auth => ( - - ))} - + + Connect URL - - )} - - {props.authenticationMethod === NATIVE && ( - - - Username + {schemeRestriction ? ( + <> + + + {props.allowedSchemes.map(s => { + const schemeString = `${s}://` + return ( + + ) + })} + + + + + {hoverText} + + + ) : ( - + )} - )} + {props.supportsMultiDb && ( + + + Database - leave empty for default + + + + )} - {props.authenticationMethod === NATIVE && ( - - - Password - - - - )} + {props.allowedAuthMethods.length > 1 && ( + + + Authentication type + + {props.allowedAuthMethods.map(auth => ( + + ))} + + + + )} + + {props.authenticationMethod === NATIVE && ( + + + Username + + + + )} + + {props.authenticationMethod === NATIVE && ( + + + Password + + + + )} - - - Connect - - - Connecting... - + + + Connect + + + Connecting... + + ) } diff --git a/src/browser/modules/Stream/Auth/styled.tsx b/src/browser/modules/Stream/Auth/styled.tsx index df7f99f3bb3..338c81e1c36 100644 --- a/src/browser/modules/Stream/Auth/styled.tsx +++ b/src/browser/modules/Stream/Auth/styled.tsx @@ -25,6 +25,7 @@ import { StyledFrameAside } from '../../Frame/styled' export const StyledConnectionForm = styled.form` padding: 0 15px; + flex: 1; &.isLoading { opacity: 0.5; } @@ -149,3 +150,23 @@ export const StyledCode = styled.code` ` export const StyledDbsRow = styled.li`` + +export const FormContainer = styled.div` + display: flex; +` + +export const SsoOptions = styled.div` + border-right: 1px solid rgb(77, 74, 87, 0.3); + padding-right: 30px; + margin-right: 30px; + max-width: 200px; +` +export const SsoButtonContainer = styled.div` + margin-bottom: 5px; +` +export const SsoError = styled.div` + margin-top: 30px; + background-color: rgba(253, 118, 110, 0.3); + border-radius: 2px; + padding: 3px; +` From 984cac68a0aa823e7bf19fd4ba3e538b5ebea26c Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Tue, 27 Jul 2021 09:28:44 +0200 Subject: [PATCH 20/48] Add better logging --- .../__snapshots__/SchemaFrame.test.tsx.snap | 72 +++++++++---------- src/shared/modules/discovery/discoveryDuck.ts | 19 +++-- 2 files changed, 50 insertions(+), 41 deletions(-) diff --git a/src/browser/modules/Stream/__snapshots__/SchemaFrame.test.tsx.snap b/src/browser/modules/Stream/__snapshots__/SchemaFrame.test.tsx.snap index 49f28da9660..603d2326a1d 100644 --- a/src/browser/modules/Stream/__snapshots__/SchemaFrame.test.tsx.snap +++ b/src/browser/modules/Stream/__snapshots__/SchemaFrame.test.tsx.snap @@ -10,13 +10,13 @@ exports[`SchemaFrame renders empty 1`] = ` class="sc-dqBHgY eVMXHH" > @@ -24,10 +24,10 @@ exports[`SchemaFrame renders empty 1`] = ` @@ -35,13 +35,13 @@ exports[`SchemaFrame renders empty 1`] = `
Indexes
None
@@ -49,10 +49,10 @@ exports[`SchemaFrame renders empty 1`] = ` @@ -92,43 +92,43 @@ exports[`SchemaFrame renders empty for Neo4j >= 4.0 1`] = ` class="sc-dqBHgY eVMXHH" >
Constraints
None
@@ -136,42 +136,42 @@ exports[`SchemaFrame renders empty for Neo4j >= 4.0 1`] = `
Index Name Type Uniqueness EntityType LabelsOrTypes Properties State
None
@@ -179,10 +179,10 @@ exports[`SchemaFrame renders empty for Neo4j >= 4.0 1`] = ` @@ -222,13 +222,13 @@ exports[`SchemaFrame renders results for Neo4j < 4.0 1`] = ` class="sc-dqBHgY eVMXHH" >
Constraints
None
@@ -236,10 +236,10 @@ exports[`SchemaFrame renders results for Neo4j < 4.0 1`] = ` @@ -247,13 +247,13 @@ exports[`SchemaFrame renders results for Neo4j < 4.0 1`] = `
Indexes
ON :Movie(released) ONLINE
@@ -261,10 +261,10 @@ exports[`SchemaFrame renders results for Neo4j < 4.0 1`] = ` diff --git a/src/shared/modules/discovery/discoveryDuck.ts b/src/shared/modules/discovery/discoveryDuck.ts index 4a893517f3a..9a27ec2a2f2 100644 --- a/src/shared/modules/discovery/discoveryDuck.ts +++ b/src/shared/modules/discovery/discoveryDuck.ts @@ -170,6 +170,7 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { .mergeMap((action: any) => { // Only when in a environment were we can guess discovery endpoint if (!action.discoveryURL || !hasDiscoveryEndpoint(store.getState())) { + authLog('No ') return Promise.resolve({ type: 'NOOP' }) } if (action.forceURL) { @@ -193,12 +194,12 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { discovered: onlyTruthy }) } + const discoveryEndpoint = getDiscoveryEndpoint( + getHostedUrl(store.getState()) + ) return Rx.Observable.fromPromise( remote - .getJSON( - action.discoveryURL || - getDiscoveryEndpoint(getHostedUrl(store.getState())) - ) + .getJSON(action.discoveryURL || discoveryEndpoint) // Uncomment below and comment out above when doing manual tests in dev mode to // fake discovery response //Promise.resolve({ @@ -233,7 +234,10 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { } } } else { - authLog('No SSO providers found on endpoint') + authLog( + `No SSO providers found in data at endpoint ${action.discoveryURL || + discoveryEndpoint}` + ) } let host = @@ -241,6 +245,7 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { (result.bolt_routing || result.bolt_direct || result.bolt) // Try to get info from server if (!host) { + authLog('No host found in discovery data, aborting') throw new Error('No bolt address found') } host = generateBoltUrl( @@ -258,6 +263,10 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { return { type: DONE, discovered } }) .catch(() => { + authLog( + `No discovery json data found at ${action.discoveryURL || + discoveryEndpoint}` + ) throw new Error('No info from endpoint') }) ).catch(() => { From 015a73ad3a745d1c45b639db328359350424f6da Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Tue, 27 Jul 2021 13:06:20 +0200 Subject: [PATCH 21/48] mel --- .../modules/Stream/Auth/ConnectionForm.tsx | 1 + src/shared/modules/auth/index.js | 15 +++++-- .../modules/connections/connectionsDuck.ts | 2 + src/shared/modules/discovery/discoveryDuck.ts | 39 +++++++++++++++---- 4 files changed, 45 insertions(+), 12 deletions(-) diff --git a/src/browser/modules/Stream/Auth/ConnectionForm.tsx b/src/browser/modules/Stream/Auth/ConnectionForm.tsx index 8defca8deb8..f381d028c93 100644 --- a/src/browser/modules/Stream/Auth/ConnectionForm.tsx +++ b/src/browser/modules/Stream/Auth/ConnectionForm.tsx @@ -345,6 +345,7 @@ export class ConnectionForm extends Component { this )} host={host} + ssoError={this.state.ssoError} username={this.state.username} password={this.state.password} database={this.state.requestedUseDb} diff --git a/src/shared/modules/auth/index.js b/src/shared/modules/auth/index.js index 690fe5e8596..369266b7e2e 100644 --- a/src/shared/modules/auth/index.js +++ b/src/shared/modules/auth/index.js @@ -28,14 +28,19 @@ import { export const authRequestForSSO = idpId => { const selectedSSOProvider = getSSOProviderByIdpId(idpId) - if (!selectedSSOProvider) return + if (!selectedSSOProvider) { + const error = `Invalid OAuth2 endpoint: "${oauth2Endpoint}"` + authLog(error) + return error + } temporarlyStoreUrlSearchParams() const oauth2Endpoint = selectedSSOProvider.auth_endpoint if (!oauth2Endpoint) { - authLog(`Invalid OAuth2 endpoint: "${oauth2Endpoint}"`) - return + const error = `Invalid OAuth2 endpoint: "${oauth2Endpoint}"` + authLog(error) + return error } authLog(`Using OAuth2 endpoint: "${oauth2Endpoint}" for idp_id: ${idpId}`) @@ -109,7 +114,9 @@ export const authRequestForSSO = idpId => { authLog('Auth flow "implicit flow"') _submitForm(form, params) } else { - authLog(`Auth flow "${selectedSSOProvider.auth_flow}" is not supported.`) + const error = `Auth flow "${selectedSSOProvider.auth_flow}" is not supported.` + authLog(error) + return error } } diff --git a/src/shared/modules/connections/connectionsDuck.ts b/src/shared/modules/connections/connectionsDuck.ts index 9bd02edb0ac..cd80cc2f745 100644 --- a/src/shared/modules/connections/connectionsDuck.ts +++ b/src/shared/modules/connections/connectionsDuck.ts @@ -93,6 +93,7 @@ export type Connection = { authenticationMethod: AuthenticationMethod requestedUseDb?: string restApi?: string + ssoError?: string } const initialState: ConnectionReduxState = { @@ -445,6 +446,7 @@ type DiscoverDataAction = { encrypted?: string hasForceUrl?: boolean } + ssoError?: string } function shouldTryAutoconnecting(conn: Connection | null): boolean { diff --git a/src/shared/modules/discovery/discoveryDuck.ts b/src/shared/modules/discovery/discoveryDuck.ts index 9a27ec2a2f2..3570bc6740d 100644 --- a/src/shared/modules/discovery/discoveryDuck.ts +++ b/src/shared/modules/discovery/discoveryDuck.ts @@ -210,6 +210,7 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { const ssoProviders = result.sso_providers || result.ssoproviders || result.ssoProviders let creds: { username?: string; password?: string } = {} + let ssoError: string | undefined if (ssoProviders) { authLog('SSO providers found on endpoint') @@ -223,13 +224,17 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { removeSearchParamsInBrowserHistory( searchParamsToRemoveAfterAutoRedirect ) - authRequestForSSO(idpId) + const err = authRequestForSSO(idpId) + if (err) { + ssoError = err + } } else if (wasRedirectedBackFromSSOServer()) { authLog('Handling auth_flow_step redirect') try { creds = await handleAuthFromRedirect() } catch (e) { + ssoError = e authLog(e) } } @@ -238,6 +243,10 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { `No SSO providers found in data at endpoint ${action.discoveryURL || discoveryEndpoint}` ) + // We assume if discoveryURL is set that they intended to use SSO + if (action.discoveryURL) { + ssoError = `No SSO providers found at ${action.discoveryURL}` + } } let host = @@ -257,16 +266,30 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { const supportsMultiDb = !isAura && parseInt((result.neo4j_version || '0').charAt(0)) >= 4 const discovered = supportsMultiDb - ? { supportsMultiDb, host, ...creds } - : { host, ...creds } + ? { supportsMultiDb, host, ...creds, ssoError } + : { host, ...creds, ssoError } - return { type: DONE, discovered } + return { type: DONE, discovered, ssoError } }) .catch(() => { - authLog( - `No discovery json data found at ${action.discoveryURL || - discoveryEndpoint}` - ) + const noDataFoundMessage = `No discovery json data found at ${action.discoveryURL || + discoveryEndpoint}` + const noHttpPrefixMessage = action?.discoveryURL.startsWith('http') + ? 'Double check that the url is a valid url (including HTTP(S)).' + : '' + const noJsonSuffixMessage = action?.discoveryURL.endsWith('.json') + ? 'Double check that the discovery url returns a valid JSON file.' + : '' + const ssoError = [ + noDataFoundMessage, + noHttpPrefixMessage, + noJsonSuffixMessage + ].join('\n') + authLog(ssoError) + + if (action.discoveryURL) { + return { type: DONE, ssoError } + } throw new Error('No info from endpoint') }) ).catch(() => { From 3ebc1c757b0be51c0921586a716142fa4b34a041 Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Tue, 27 Jul 2021 13:41:42 +0200 Subject: [PATCH 22/48] Show error messages from SSO --- .../modules/Stream/Auth/ConnectForm.tsx | 3 +- src/browser/modules/Stream/Auth/styled.tsx | 1 + src/shared/modules/auth/common.js | 4 +-- src/shared/modules/auth/helpers.js | 2 +- .../modules/connections/connectionsDuck.ts | 9 ++++-- src/shared/modules/discovery/discoveryDuck.ts | 31 ++++++++++++------- 6 files changed, 32 insertions(+), 18 deletions(-) diff --git a/src/browser/modules/Stream/Auth/ConnectForm.tsx b/src/browser/modules/Stream/Auth/ConnectForm.tsx index 2e7bc5003d8..f4b7b73a6e9 100644 --- a/src/browser/modules/Stream/Auth/ConnectForm.tsx +++ b/src/browser/modules/Stream/Auth/ConnectForm.tsx @@ -131,9 +131,10 @@ export default function ConnectForm(props: ConnectFormProps): JSX.Element { : '' const { ssoError } = props + const showSso = ssoProviders.length > 0 || ssoError return ( - {ssoProviders.length > 0 && ( + {showSso && (

Single sign-on

{ssoProviders.map((provider: any) => ( diff --git a/src/browser/modules/Stream/Auth/styled.tsx b/src/browser/modules/Stream/Auth/styled.tsx index 338c81e1c36..a7dc70a7be1 100644 --- a/src/browser/modules/Stream/Auth/styled.tsx +++ b/src/browser/modules/Stream/Auth/styled.tsx @@ -169,4 +169,5 @@ export const SsoError = styled.div` background-color: rgba(253, 118, 110, 0.3); border-radius: 2px; padding: 3px; + white-space: pre-line; ` diff --git a/src/shared/modules/auth/common.js b/src/shared/modules/auth/common.js index a8771b4a17c..d057b2dfb66 100644 --- a/src/shared/modules/auth/common.js +++ b/src/shared/modules/auth/common.js @@ -166,7 +166,8 @@ export const getCredentialsFromAuthResult = (result, idpId) => { selectedSSOProvider.config?.['token_type_authentication'] || defaultTokenTypeAuthentication authLog( - `Credentials assembly with token type "${tokenTypeAuthentication}" as password` + `Credentials assembled with token type "${tokenTypeAuthentication}" as password. +If connection still does not succeed, make sure neo4j.conf is set up correctly` ) return { username: credsPrincipal, password: result[tokenTypeAuthentication] } @@ -204,7 +205,6 @@ export const restoreSearchAndHashParamsParams = () => { window.sessionStorage.setItem(AUTH_STORAGE_URL_SEARCH_PARAMS, '') - debugger if (isObject(storedParams)) { const crntHashParams = window.location.hash || undefined addSearchParamsInBrowserHistory(storedParams) diff --git a/src/shared/modules/auth/helpers.js b/src/shared/modules/auth/helpers.js index de198b73bf3..535ae7d5967 100644 --- a/src/shared/modules/auth/helpers.js +++ b/src/shared/modules/auth/helpers.js @@ -5,7 +5,7 @@ import { isAuthLoggingEnabled, isAuthDebuggingEnabled } from './settings' export const authLog = (msg, type = 'log') => { if (!isAuthLoggingEnabled) return if (!['log', 'error', 'warn'].includes(type)) return - console[type](`${AUTH_LOGGING_PREFIX} ${msg}`) + console[type](`${AUTH_LOGGING_PREFIX} [${new Date().toISOString()}] ${msg}`) } export const authDebug = (msg, content) => { diff --git a/src/shared/modules/connections/connectionsDuck.ts b/src/shared/modules/connections/connectionsDuck.ts index cd80cc2f745..e418313d108 100644 --- a/src/shared/modules/connections/connectionsDuck.ts +++ b/src/shared/modules/connections/connectionsDuck.ts @@ -445,8 +445,8 @@ type DiscoverDataAction = { host?: string encrypted?: string hasForceUrl?: boolean + ssoError?: string } - ssoError?: string } function shouldTryAutoconnecting(conn: Connection | null): boolean { @@ -523,7 +523,12 @@ export const startupConnectEpic = (action$: any, store: any) => { // Otherwise fail autoconnect store.dispatch(setActiveConnection(null)) - store.dispatch(discovery.updateDiscoveryConnection({ password: '' })) + store.dispatch( + discovery.updateDiscoveryConnection({ + password: '', + ssoError: discovered?.ssoError + }) + ) return Promise.resolve({ type: STARTUP_CONNECTION_FAILED }) }) } diff --git a/src/shared/modules/discovery/discoveryDuck.ts b/src/shared/modules/discovery/discoveryDuck.ts index 3570bc6740d..67c62368568 100644 --- a/src/shared/modules/discovery/discoveryDuck.ts +++ b/src/shared/modules/discovery/discoveryDuck.ts @@ -170,7 +170,7 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { .mergeMap((action: any) => { // Only when in a environment were we can guess discovery endpoint if (!action.discoveryURL || !hasDiscoveryEndpoint(store.getState())) { - authLog('No ') + authLog('No discovery endpoint found or passed') return Promise.resolve({ type: 'NOOP' }) } if (action.forceURL) { @@ -266,29 +266,36 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { const supportsMultiDb = !isAura && parseInt((result.neo4j_version || '0').charAt(0)) >= 4 const discovered = supportsMultiDb - ? { supportsMultiDb, host, ...creds, ssoError } - : { host, ...creds, ssoError } + ? { supportsMultiDb, host, ssoError, ...creds } + : { host, ssoError, ...creds } - return { type: DONE, discovered, ssoError } + return { type: DONE, discovered } }) .catch(() => { const noDataFoundMessage = `No discovery json data found at ${action.discoveryURL || discoveryEndpoint}` - const noHttpPrefixMessage = action?.discoveryURL.startsWith('http') - ? 'Double check that the url is a valid url (including HTTP(S)).' - : '' - const noJsonSuffixMessage = action?.discoveryURL.endsWith('.json') - ? 'Double check that the discovery url returns a valid JSON file.' - : '' + const noHttpPrefixMessage = action?.discoveryURL + .toLowerCase() + .startsWith('http') + ? '' + : 'Double check that the url is a valid url (including HTTP(S)).' + const noJsonSuffixMessage = action?.discoveryURL + .toLowerCase() + .endsWith('.json') + ? '' + : 'Double check that the discovery url returns a valid JSON file.' + const ssoError = [ noDataFoundMessage, noHttpPrefixMessage, noJsonSuffixMessage - ].join('\n') + ] + .join('\n') + .trim() authLog(ssoError) if (action.discoveryURL) { - return { type: DONE, ssoError } + return { type: DONE, discovered: { ssoError } } } throw new Error('No info from endpoint') }) From 7c8fcc86925d1a16e4bb773f708fa50a467fff60 Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Tue, 27 Jul 2021 13:50:34 +0200 Subject: [PATCH 23/48] Self review --- __reactpreview__/Wrapper.tsx | 8 -------- src/shared/modules/discovery/discoveryDuck.ts | 4 ++-- 2 files changed, 2 insertions(+), 10 deletions(-) delete mode 100644 __reactpreview__/Wrapper.tsx diff --git a/__reactpreview__/Wrapper.tsx b/__reactpreview__/Wrapper.tsx deleted file mode 100644 index 02fa9d6e18e..00000000000 --- a/__reactpreview__/Wrapper.tsx +++ /dev/null @@ -1,8 +0,0 @@ -import React from 'react' - -// You can import global CSS files here. - -// No-op wrapper. -export const Wrapper: React.FC = ({ children }) => { - return <>{children} -} diff --git a/src/shared/modules/discovery/discoveryDuck.ts b/src/shared/modules/discovery/discoveryDuck.ts index 67c62368568..50cc7b87af5 100644 --- a/src/shared/modules/discovery/discoveryDuck.ts +++ b/src/shared/modules/discovery/discoveryDuck.ts @@ -203,8 +203,8 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { // Uncomment below and comment out above when doing manual tests in dev mode to // fake discovery response //Promise.resolve({ - //bolt: 'bolt://localhost:7687', - //neo4j_version: '4' + // bolt: 'bolt://localhost:7687', + // neo4j_version: '4.0.3' //}) .then(async result => { const ssoProviders = From 413bcfcafeb4a41d3ba07f0b69e8aaad0a4ea02a Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Tue, 27 Jul 2021 16:00:13 +0200 Subject: [PATCH 24/48] Fix normal discovery --- src/shared/modules/discovery/discoveryDuck.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/modules/discovery/discoveryDuck.ts b/src/shared/modules/discovery/discoveryDuck.ts index 50cc7b87af5..aae66a89922 100644 --- a/src/shared/modules/discovery/discoveryDuck.ts +++ b/src/shared/modules/discovery/discoveryDuck.ts @@ -169,7 +169,7 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { .merge(some$.ofType(USER_CLEAR)) .mergeMap((action: any) => { // Only when in a environment were we can guess discovery endpoint - if (!action.discoveryURL || !hasDiscoveryEndpoint(store.getState())) { + if (!action.discoveryURL && !hasDiscoveryEndpoint(store.getState())) { authLog('No discovery endpoint found or passed') return Promise.resolve({ type: 'NOOP' }) } From 25eb2c610d0ac2c616a0bfdb970029053501fc61 Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Wed, 28 Jul 2021 17:59:16 +0200 Subject: [PATCH 25/48] Downloadable logs --- .../modules/Stream/Auth/ConnectForm.tsx | 2 + src/browser/modules/Stream/Auth/styled.tsx | 2 +- src/shared/modules/auth/common.js | 11 +---- src/shared/modules/auth/constants.js | 2 +- src/shared/modules/auth/helpers.js | 19 +++++++- .../modules/connections/connectionsDuck.ts | 6 ++- src/shared/modules/discovery/discoveryDuck.ts | 43 +++++++++++++------ 7 files changed, 58 insertions(+), 27 deletions(-) diff --git a/src/browser/modules/Stream/Auth/ConnectForm.tsx b/src/browser/modules/Stream/Auth/ConnectForm.tsx index f4b7b73a6e9..be36e637701 100644 --- a/src/browser/modules/Stream/Auth/ConnectForm.tsx +++ b/src/browser/modules/Stream/Auth/ConnectForm.tsx @@ -42,6 +42,7 @@ import { authRequestForSSO } from 'shared/modules/auth/index.js' import { getSSOProvidersFromStorage } from 'shared/modules/auth/common.js' import { H3 } from 'browser-components/headers' import { StyledCypherErrorMessage } from '../styled' +import { downloadAuthLogs } from 'shared/modules/auth/helpers' const readableauthenticationMethods: Record = { [NATIVE]: 'Username / Password', @@ -148,6 +149,7 @@ export default function ConnectForm(props: ConnectFormProps): JSX.Element { ERROR
{ssoError}
+ )} diff --git a/src/browser/modules/Stream/Auth/styled.tsx b/src/browser/modules/Stream/Auth/styled.tsx index a7dc70a7be1..116354fc424 100644 --- a/src/browser/modules/Stream/Auth/styled.tsx +++ b/src/browser/modules/Stream/Auth/styled.tsx @@ -159,7 +159,7 @@ export const SsoOptions = styled.div` border-right: 1px solid rgb(77, 74, 87, 0.3); padding-right: 30px; margin-right: 30px; - max-width: 200px; + max-width: 300px; ` export const SsoButtonContainer = styled.div` margin-bottom: 5px; diff --git a/src/shared/modules/auth/common.js b/src/shared/modules/auth/common.js index d057b2dfb66..9f016628bcb 100644 --- a/src/shared/modules/auth/common.js +++ b/src/shared/modules/auth/common.js @@ -3,8 +3,7 @@ import { isObject } from 'lodash' import { AUTH_STORAGE_SSO_PROVIDERS, AUTH_STORAGE_URL_SEARCH_PARAMS, - REDIRECT_URI, - SSO_REDIRECT + REDIRECT_URI } from './constants' import { addSearchParamsInBrowserHistory, authLog, authDebug } from './helpers' import { @@ -166,8 +165,7 @@ export const getCredentialsFromAuthResult = (result, idpId) => { selectedSSOProvider.config?.['token_type_authentication'] || defaultTokenTypeAuthentication authLog( - `Credentials assembled with token type "${tokenTypeAuthentication}" as password. -If connection still does not succeed, make sure neo4j.conf is set up correctly` + `Credentials assembled with token type "${tokenTypeAuthentication}" as password. If connection still does not succeed, make sure neo4j.conf is set up correctly` ) return { username: credsPrincipal, password: result[tokenTypeAuthentication] } @@ -186,11 +184,6 @@ export const temporarlyStoreUrlSearchParams = () => { ) } -export const shouldRedirectToSSOServer = () => { - const { cmd, arg } = getInitialisationParameters() - return (cmd || '').toLowerCase() === SSO_REDIRECT && arg -} - export const wasRedirectedBackFromSSOServer = () => { const { auth_flow_step: authFlowStep } = getInitialisationParameters() return (authFlowStep || '').toLowerCase() === REDIRECT_URI diff --git a/src/shared/modules/auth/constants.js b/src/shared/modules/auth/constants.js index ddcf095ac39..e8df8026a13 100644 --- a/src/shared/modules/auth/constants.js +++ b/src/shared/modules/auth/constants.js @@ -1,5 +1,4 @@ export const REDIRECT_URI = 'redirect_uri' -export const SSO_REDIRECT = 'sso_redirect' export const BEARER = 'bearer' export const PKCE = 'pkce' export const IMPLICIT = 'implicit' @@ -11,3 +10,4 @@ export const AUTH_STORAGE_SSO_PROVIDERS = `${AUTH_STORAGE_PREFIX}sso_providers` export const AUTH_STORAGE_STATE = `${AUTH_STORAGE_PREFIX}state` export const AUTH_STORAGE_CODE_VERIFIER = `${AUTH_STORAGE_PREFIX}code_verifier` export const AUTH_STORAGE_URL_SEARCH_PARAMS = `${AUTH_STORAGE_PREFIX}url_search_params` +export const AUTH_STORAGE_LOGS = `${AUTH_STORAGE_PREFIX}logs` diff --git a/src/shared/modules/auth/helpers.js b/src/shared/modules/auth/helpers.js index 535ae7d5967..a69e32ae428 100644 --- a/src/shared/modules/auth/helpers.js +++ b/src/shared/modules/auth/helpers.js @@ -1,11 +1,26 @@ import { isObject } from 'lodash' -import { AUTH_LOGGING_PREFIX } from './constants' +import { AUTH_LOGGING_PREFIX, AUTH_STORAGE_LOGS } from './constants' import { isAuthLoggingEnabled, isAuthDebuggingEnabled } from './settings' +import { saveAs } from 'file-saver' export const authLog = (msg, type = 'log') => { if (!isAuthLoggingEnabled) return if (!['log', 'error', 'warn'].includes(type)) return - console[type](`${AUTH_LOGGING_PREFIX} [${new Date().toISOString()}] ${msg}`) + const log = `${AUTH_LOGGING_PREFIX} [${new Date().toISOString()}] ${msg}` + const logs = sessionStorage.getItem(AUTH_STORAGE_LOGS) || '' + const logsLines = logs.split('\n') + + const truncatedOldLogs = + logsLines.length > 200 ? logsLines.slice(-199).join('\n') : logs + + sessionStorage.setItem(AUTH_STORAGE_LOGS, `${truncatedOldLogs}${log}\n`) +} + +export const downloadAuthLogs = () => { + const blob = new Blob([sessionStorage.getItem(AUTH_STORAGE_LOGS)], { + type: 'text/plain;charset=utf-8' + }) + saveAs(blob, 'sso.log') } export const authDebug = (msg, content) => { diff --git a/src/shared/modules/connections/connectionsDuck.ts b/src/shared/modules/connections/connectionsDuck.ts index e418313d108..9843ab352e2 100644 --- a/src/shared/modules/connections/connectionsDuck.ts +++ b/src/shared/modules/connections/connectionsDuck.ts @@ -446,6 +446,7 @@ type DiscoverDataAction = { encrypted?: string hasForceUrl?: boolean ssoError?: string + attemptSsoLogin?: boolean } } @@ -512,7 +513,10 @@ export const startupConnectEpic = (action$: any, store: any) => { store.dispatch( discovery.updateDiscoveryConnection({ username: '', - password: '' + password: '', + ssoError: discovered.attemptSsoLogin + ? 'SSO token was not accepted by neo4j' + : undefined }) ) resolve({ type: STARTUP_CONNECTION_FAILED }) diff --git a/src/shared/modules/discovery/discoveryDuck.ts b/src/shared/modules/discovery/discoveryDuck.ts index aae66a89922..688e100d3ae 100644 --- a/src/shared/modules/discovery/discoveryDuck.ts +++ b/src/shared/modules/discovery/discoveryDuck.ts @@ -45,7 +45,6 @@ import { } from 'shared/modules/auth/helpers' import { checkAndMergeSSOProviders, - shouldRedirectToSSOServer, wasRedirectedBackFromSSOServer } from 'shared/modules/auth/common' import { searchParamsToRemoveAfterAutoRedirect } from 'shared/modules/auth/settings' @@ -158,7 +157,10 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { action.requestedUseDb = passedDb } - const discoveryURL = searchParams.get('discoveryURL') + const discoveryURL = + searchParams.get('discoveryURL') || + searchParams.get('discoveryurl') || + searchParams.get('discoveryUrl') if (discoveryURL) { action.discoveryURL = discoveryURL @@ -216,15 +218,15 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { authLog('SSO providers found on endpoint') checkAndMergeSSOProviders(ssoProviders, true) const { searchParams } = new URL(window.location.href) + const sso_redirect = searchParams.get('sso_redirect') - if (shouldRedirectToSSOServer()) { - const idpId = searchParams.get('arg') - authLog(`Initialised with idpId: "${idpId}"`) + if (sso_redirect) { + authLog(`Initialised with idpId: "${sso_redirect}"`) removeSearchParamsInBrowserHistory( searchParamsToRemoveAfterAutoRedirect ) - const err = authRequestForSSO(idpId) + const err = authRequestForSSO(sso_redirect) if (err) { ssoError = err } @@ -254,9 +256,16 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { (result.bolt_routing || result.bolt_direct || result.bolt) // Try to get info from server if (!host) { - authLog('No host found in discovery data, aborting') - throw new Error('No bolt address found') + if (ssoProviders) { + const ssoError = 'No host found in discovery data' + authLog(ssoError) + + return { type: DONE, discovered: { ssoError } } + } else { + throw new Error('No bolt address found') + } } + host = generateBoltUrl( getAllowedBoltSchemesForHost(store.getState(), host), host @@ -265,13 +274,17 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { const isAura = isConnectedAuraHost(store.getState()) const supportsMultiDb = !isAura && parseInt((result.neo4j_version || '0').charAt(0)) >= 4 - const discovered = supportsMultiDb - ? { supportsMultiDb, host, ssoError, ...creds } - : { host, ssoError, ...creds } + const discovered = { + supportsMultiDb, + host, + ssoError, + ...creds, + attemptSsoLogin: !!creds.password + } return { type: DONE, discovered } }) - .catch(() => { + .catch(e => { const noDataFoundMessage = `No discovery json data found at ${action.discoveryURL || discoveryEndpoint}` const noHttpPrefixMessage = action?.discoveryURL @@ -286,13 +299,17 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { : 'Double check that the discovery url returns a valid JSON file.' const ssoError = [ + e.message, noDataFoundMessage, noHttpPrefixMessage, noJsonSuffixMessage ] + .map(error => { + authLog(error) + return error + }) .join('\n') .trim() - authLog(ssoError) if (action.discoveryURL) { return { type: DONE, discovered: { ssoError } } From b81a87b78ec2d2d9e99dc1eedb25c35dcb3d73a3 Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Fri, 30 Jul 2021 12:41:50 +0200 Subject: [PATCH 26/48] Update naming as per review comments --- src/browser/AppInit.tsx | 4 +- .../modules/Stream/Auth/ConnectForm.tsx | 42 +++++++++---------- .../modules/Stream/Auth/ConnectionForm.tsx | 2 +- src/browser/modules/Stream/Auth/styled.tsx | 8 ++-- src/shared/modules/auth/common.js | 20 ++++----- src/shared/modules/auth/helpers.js | 2 +- src/shared/modules/auth/index.js | 34 +++++++-------- .../modules/connections/connectionsDuck.ts | 10 ++--- src/shared/modules/discovery/discoveryDuck.ts | 38 ++++++++--------- 9 files changed, 80 insertions(+), 80 deletions(-) diff --git a/src/browser/AppInit.tsx b/src/browser/AppInit.tsx index d5d565f4ff9..75593dbb9b9 100644 --- a/src/browser/AppInit.tsx +++ b/src/browser/AppInit.tsx @@ -54,7 +54,7 @@ import { getUuid } from 'shared/modules/udc/udcDuck' import { DndProvider } from 'react-dnd' import { HTML5Backend } from 'react-dnd-html5-backend' import { - restoreSearchAndHashParamsParams, + restoreSearchAndHashParams, wasRedirectedBackFromSSOServer } from 'shared/modules/auth/common' @@ -208,7 +208,7 @@ const env = detectRuntimeEnv(window, NEO4J_CLOUD_DOMAINS) // we redirect to the server, and then restore them when we get // redirected back if (wasRedirectedBackFromSSOServer()) { - restoreSearchAndHashParamsParams() + restoreSearchAndHashParams() } // URL we're on diff --git a/src/browser/modules/Stream/Auth/ConnectForm.tsx b/src/browser/modules/Stream/Auth/ConnectForm.tsx index be36e637701..818923c823f 100644 --- a/src/browser/modules/Stream/Auth/ConnectForm.tsx +++ b/src/browser/modules/Stream/Auth/ConnectForm.tsx @@ -29,10 +29,10 @@ import { StyledConnectionFormEntry, StyledSegment, StyledBoltUrlHintText, - FormContainer, - SsoOptions, - SsoButtonContainer, - SsoError + StyledFormContainer, + StyledSSOOptions, + StyledSSOButtonContainer, + StyledSSOError } from './styled' import { NATIVE, NO_AUTH } from 'services/bolt/boltHelpers' import { toKeyString } from 'services/utils' @@ -65,7 +65,7 @@ interface ConnectFormProps { username: string used: boolean supportsMultiDb: boolean - ssoError?: string + SSOError?: string } export default function ConnectForm(props: ConnectFormProps): JSX.Element { @@ -73,10 +73,10 @@ export default function ConnectForm(props: ConnectFormProps): JSX.Element { const [scheme, setScheme] = useState( props.allowedSchemes ? `${getScheme(props.host)}://` : '' ) - const [ssoProviders, setSsoProviders] = useState([]) + const [SSOProviders, setSSOProviders] = useState([]) useEffect(() => { - setSsoProviders(getSSOProvidersFromStorage()) + setSSOProviders(getSSOProvidersFromStorage()) }, []) useEffect(() => { @@ -131,28 +131,28 @@ export default function ConnectForm(props: ConnectFormProps): JSX.Element { }:// for a direct connection to a single instance.` : '' - const { ssoError } = props - const showSso = ssoProviders.length > 0 || ssoError + const { SSOError } = props + const showSSO = SSOProviders.length > 0 || SSOError return ( - - {showSso && ( - + + {showSSO && ( +

Single sign-on

- {ssoProviders.map((provider: any) => ( - + {SSOProviders.map((provider: any) => ( + authRequestForSSO(provider.id)}> {provider.name} - + ))} - {ssoError && ( - + {SSOError && ( + ERROR -
{ssoError}
+
{SSOError}
- + )} - + )} @@ -266,6 +266,6 @@ export default function ConnectForm(props: ConnectFormProps): JSX.Element { Connecting... - + ) } diff --git a/src/browser/modules/Stream/Auth/ConnectionForm.tsx b/src/browser/modules/Stream/Auth/ConnectionForm.tsx index f381d028c93..a50f544f02e 100644 --- a/src/browser/modules/Stream/Auth/ConnectionForm.tsx +++ b/src/browser/modules/Stream/Auth/ConnectionForm.tsx @@ -345,7 +345,7 @@ export class ConnectionForm extends Component { this )} host={host} - ssoError={this.state.ssoError} + SSOError={this.state.SSOError} username={this.state.username} password={this.state.password} database={this.state.requestedUseDb} diff --git a/src/browser/modules/Stream/Auth/styled.tsx b/src/browser/modules/Stream/Auth/styled.tsx index 116354fc424..428a8115649 100644 --- a/src/browser/modules/Stream/Auth/styled.tsx +++ b/src/browser/modules/Stream/Auth/styled.tsx @@ -151,20 +151,20 @@ export const StyledCode = styled.code` export const StyledDbsRow = styled.li`` -export const FormContainer = styled.div` +export const StyledFormContainer = styled.div` display: flex; ` -export const SsoOptions = styled.div` +export const StyledSSOOptions = styled.div` border-right: 1px solid rgb(77, 74, 87, 0.3); padding-right: 30px; margin-right: 30px; max-width: 300px; ` -export const SsoButtonContainer = styled.div` +export const StyledSSOButtonContainer = styled.div` margin-bottom: 5px; ` -export const SsoError = styled.div` +export const StyledSSOError = styled.div` margin-top: 30px; background-color: rgba(253, 118, 110, 0.3); border-radius: 2px; diff --git a/src/shared/modules/auth/common.js b/src/shared/modules/auth/common.js index 9f016628bcb..ee36b7ffe9d 100644 --- a/src/shared/modules/auth/common.js +++ b/src/shared/modules/auth/common.js @@ -94,10 +94,10 @@ export const checkAndMergeSSOProviders = ( } export const getSSOProvidersFromStorage = () => { - const ssoProviders = JSON.parse( + const SSOProviders = JSON.parse( window.sessionStorage.getItem(AUTH_STORAGE_SSO_PROVIDERS) ) - if (!ssoProviders || !ssoProviders.length) { + if (!SSOProviders || !SSOProviders.length) { authLog('No SSO providers in (local) storage found') return [] } @@ -108,13 +108,13 @@ export const getSSOProvidersFromStorage = () => { ) return [] } - return ssoProviders + return SSOProviders } export const getSSOProviderByIdpId = idpId => { - const ssoProviders = getSSOProvidersFromStorage() + const SSOProviders = getSSOProvidersFromStorage() - const selectedSSOProvider = ssoProviders.find( + const selectedSSOProvider = SSOProviders.find( provider => provider.id === idpId ) if (!selectedSSOProvider) { @@ -171,7 +171,7 @@ export const getCredentialsFromAuthResult = (result, idpId) => { return { username: credsPrincipal, password: result[tokenTypeAuthentication] } } -export const temporarlyStoreUrlSearchParams = () => { +export const temporarilyStoreUrlSearchParams = () => { const currentBrowserURLParams = getInitialisationParameters() authLog( `Temporarly storing the url search params. data: "${JSON.stringify( @@ -189,8 +189,8 @@ export const wasRedirectedBackFromSSOServer = () => { return (authFlowStep || '').toLowerCase() === REDIRECT_URI } -export const restoreSearchAndHashParamsParams = () => { - authLog(`Retrieving temporarly stored url search params`) +export const restoreSearchAndHashParams = () => { + authLog(`Retrieving temporarily stored url search params`) try { const storedParams = JSON.parse( window.sessionStorage.getItem(AUTH_STORAGE_URL_SEARCH_PARAMS) @@ -205,12 +205,12 @@ export const restoreSearchAndHashParamsParams = () => { window.history.replaceState({}, '', newUrl) return storedParams } else { - authLog('Invalid temporarly stored url search params') + authLog('Invalid temporarily stored url search params') return null } } catch (err) { authLog( - `Error when parsing temporarly stored url search params, err: ${err}` + `Error when parsing temporarily stored url search params, err: ${err}` ) return null } diff --git a/src/shared/modules/auth/helpers.js b/src/shared/modules/auth/helpers.js index a69e32ae428..469e4961a50 100644 --- a/src/shared/modules/auth/helpers.js +++ b/src/shared/modules/auth/helpers.js @@ -20,7 +20,7 @@ export const downloadAuthLogs = () => { const blob = new Blob([sessionStorage.getItem(AUTH_STORAGE_LOGS)], { type: 'text/plain;charset=utf-8' }) - saveAs(blob, 'sso.log') + saveAs(blob, 'neo4j-browser-sso.log') } export const authDebug = (msg, content) => { diff --git a/src/shared/modules/auth/index.js b/src/shared/modules/auth/index.js index 369266b7e2e..6dd9bd5e0d4 100644 --- a/src/shared/modules/auth/index.js +++ b/src/shared/modules/auth/index.js @@ -2,7 +2,7 @@ import { getCredentialsFromAuthResult, getInitialisationParameters, getSSOProviderByIdpId, - temporarlyStoreUrlSearchParams + temporarilyStoreUrlSearchParams } from './common' import { AUTH_STORAGE_CODE_VERIFIER, @@ -34,7 +34,7 @@ export const authRequestForSSO = idpId => { return error } - temporarlyStoreUrlSearchParams() + temporarilyStoreUrlSearchParams() const oauth2Endpoint = selectedSSOProvider.auth_endpoint if (!oauth2Endpoint) { @@ -48,28 +48,28 @@ export const authRequestForSSO = idpId => { form.setAttribute('method', 'GET') form.setAttribute('action', oauth2Endpoint) - const ssoParams = selectedSSOProvider.params || {} + const SSOParams = selectedSSOProvider.params || {} const state = createStateForRequest() let params = { - ...ssoParams, + ...SSOParams, state } window.sessionStorage.setItem(AUTH_STORAGE_STATE, state) - const ssoExtraAuthParams = selectedSSOProvider.auth_params || {} - if (ssoExtraAuthParams) { + const SSOExtraAuthParams = selectedSSOProvider.auth_params || {} + if (SSOExtraAuthParams) { params = { ...params, - ...ssoExtraAuthParams + ...SSOExtraAuthParams } } authLog( - `Using the following authorization parameter: ${JSON.stringify(ssoParams)}` + `Using the following authorization parameter: ${JSON.stringify(SSOParams)}` ) - const ssoConfig = selectedSSOProvider.config || {} - if (ssoConfig.implicit_flow_requires_nonce) { + const SSOConfig = selectedSSOProvider.config || {} + if (SSOConfig.implicit_flow_requires_nonce) { params = { ...params, nonce: createNonce() @@ -92,7 +92,7 @@ export const authRequestForSSO = idpId => { if (selectedSSOProvider.auth_flow === PKCE) { const codeChallengeMethod = - ssoConfig.code_challenge_method || defaultCodeChallengeMethod + SSOConfig.code_challenge_method || defaultCodeChallengeMethod authLog( `Auth flow "PKCE", using code_challenge_method: "${codeChallengeMethod}"` ) @@ -202,21 +202,21 @@ export const authRequestForToken = (idpId, code) => { const selectedSSOProvider = getSSOProviderByIdpId(idpId) if (!selectedSSOProvider) return - const ssoParams = selectedSSOProvider.params || {} + const SSOParams = selectedSSOProvider.params || {} let details = { grant_type: defaultGrantType, - client_id: ssoParams.client_id, - redirect_uri: ssoParams.redirect_uri, + client_id: SSOParams.client_id, + redirect_uri: SSOParams.redirect_uri, code_verifier: window.sessionStorage.getItem(AUTH_STORAGE_CODE_VERIFIER), code } window.sessionStorage.setItem(AUTH_STORAGE_CODE_VERIFIER, '') - const ssoExtraTokenParams = selectedSSOProvider.token_params || {} - if (ssoExtraTokenParams) { + const SSOExtraTokenParams = selectedSSOProvider.token_params || {} + if (SSOExtraTokenParams) { details = { ...details, - ...ssoExtraTokenParams + ...SSOExtraTokenParams } } diff --git a/src/shared/modules/connections/connectionsDuck.ts b/src/shared/modules/connections/connectionsDuck.ts index 9843ab352e2..a290dd0b590 100644 --- a/src/shared/modules/connections/connectionsDuck.ts +++ b/src/shared/modules/connections/connectionsDuck.ts @@ -93,7 +93,7 @@ export type Connection = { authenticationMethod: AuthenticationMethod requestedUseDb?: string restApi?: string - ssoError?: string + SSOError?: string } const initialState: ConnectionReduxState = { @@ -445,8 +445,8 @@ type DiscoverDataAction = { host?: string encrypted?: string hasForceUrl?: boolean - ssoError?: string - attemptSsoLogin?: boolean + SSOError?: string + attemptSSOLogin?: boolean } } @@ -514,7 +514,7 @@ export const startupConnectEpic = (action$: any, store: any) => { discovery.updateDiscoveryConnection({ username: '', password: '', - ssoError: discovered.attemptSsoLogin + SSOError: discovered.attemptSSOLogin ? 'SSO token was not accepted by neo4j' : undefined }) @@ -530,7 +530,7 @@ export const startupConnectEpic = (action$: any, store: any) => { store.dispatch( discovery.updateDiscoveryConnection({ password: '', - ssoError: discovered?.ssoError + SSOError: discovered?.SSOError }) ) return Promise.resolve({ type: STARTUP_CONNECTION_FAILED }) diff --git a/src/shared/modules/discovery/discoveryDuck.ts b/src/shared/modules/discovery/discoveryDuck.ts index 688e100d3ae..932aa8cfc94 100644 --- a/src/shared/modules/discovery/discoveryDuck.ts +++ b/src/shared/modules/discovery/discoveryDuck.ts @@ -209,26 +209,26 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { // neo4j_version: '4.0.3' //}) .then(async result => { - const ssoProviders = + const SSOProviders = result.sso_providers || result.ssoproviders || result.ssoProviders let creds: { username?: string; password?: string } = {} - let ssoError: string | undefined + let SSOError: string | undefined - if (ssoProviders) { + if (SSOProviders) { authLog('SSO providers found on endpoint') - checkAndMergeSSOProviders(ssoProviders, true) + checkAndMergeSSOProviders(SSOProviders, true) const { searchParams } = new URL(window.location.href) - const sso_redirect = searchParams.get('sso_redirect') + const SSORedirect = searchParams.get('sso_redirect') - if (sso_redirect) { - authLog(`Initialised with idpId: "${sso_redirect}"`) + if (SSORedirect) { + authLog(`Initialised with idpId: "${SSORedirect}"`) removeSearchParamsInBrowserHistory( searchParamsToRemoveAfterAutoRedirect ) - const err = authRequestForSSO(sso_redirect) + const err = authRequestForSSO(SSORedirect) if (err) { - ssoError = err + SSOError = err } } else if (wasRedirectedBackFromSSOServer()) { authLog('Handling auth_flow_step redirect') @@ -236,7 +236,7 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { try { creds = await handleAuthFromRedirect() } catch (e) { - ssoError = e + SSOError = e authLog(e) } } @@ -247,7 +247,7 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { ) // We assume if discoveryURL is set that they intended to use SSO if (action.discoveryURL) { - ssoError = `No SSO providers found at ${action.discoveryURL}` + SSOError = `No SSO providers found at ${action.discoveryURL}` } } @@ -256,11 +256,11 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { (result.bolt_routing || result.bolt_direct || result.bolt) // Try to get info from server if (!host) { - if (ssoProviders) { - const ssoError = 'No host found in discovery data' - authLog(ssoError) + if (SSOProviders) { + const SSOError = 'No host found in discovery data' + authLog(SSOError) - return { type: DONE, discovered: { ssoError } } + return { type: DONE, discovered: { SSOError } } } else { throw new Error('No bolt address found') } @@ -277,9 +277,9 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { const discovered = { supportsMultiDb, host, - ssoError, + SSOError, ...creds, - attemptSsoLogin: !!creds.password + attemptSSOLogin: !!creds.password } return { type: DONE, discovered } @@ -298,7 +298,7 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { ? '' : 'Double check that the discovery url returns a valid JSON file.' - const ssoError = [ + const SSOError = [ e.message, noDataFoundMessage, noHttpPrefixMessage, @@ -312,7 +312,7 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { .trim() if (action.discoveryURL) { - return { type: DONE, discovered: { ssoError } } + return { type: DONE, discovered: { SSOError } } } throw new Error('No info from endpoint') }) From 41fe35b97e09905073aa6bd2ad6d4dfc91b92c1e Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Sun, 1 Aug 2021 15:08:52 +0200 Subject: [PATCH 27/48] comments --- src/shared/modules/auth/common.js | 5 ++++ src/shared/modules/auth/constants.js | 1 + src/shared/modules/discovery/discoveryDuck.ts | 24 ++++++++++++++----- 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/src/shared/modules/auth/common.js b/src/shared/modules/auth/common.js index ee36b7ffe9d..b5dcb57a3e6 100644 --- a/src/shared/modules/auth/common.js +++ b/src/shared/modules/auth/common.js @@ -184,6 +184,11 @@ export const temporarilyStoreUrlSearchParams = () => { ) } +export const getSSOServerIdIfShouldRedirect = () => { + const { searchParams } = new URL(window.location.href) + return searchParams.get(SSO_REDIRECT) +} + export const wasRedirectedBackFromSSOServer = () => { const { auth_flow_step: authFlowStep } = getInitialisationParameters() return (authFlowStep || '').toLowerCase() === REDIRECT_URI diff --git a/src/shared/modules/auth/constants.js b/src/shared/modules/auth/constants.js index e8df8026a13..a1fd3b178bf 100644 --- a/src/shared/modules/auth/constants.js +++ b/src/shared/modules/auth/constants.js @@ -1,3 +1,4 @@ +export const SSO_REDIRECT = 'sso_redirect' export const REDIRECT_URI = 'redirect_uri' export const BEARER = 'bearer' export const PKCE = 'pkce' diff --git a/src/shared/modules/discovery/discoveryDuck.ts b/src/shared/modules/discovery/discoveryDuck.ts index 932aa8cfc94..d718077f7af 100644 --- a/src/shared/modules/discovery/discoveryDuck.ts +++ b/src/shared/modules/discovery/discoveryDuck.ts @@ -170,11 +170,8 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { }) .merge(some$.ofType(USER_CLEAR)) .mergeMap((action: any) => { - // Only when in a environment were we can guess discovery endpoint - if (!action.discoveryURL && !hasDiscoveryEndpoint(store.getState())) { - authLog('No discovery endpoint found or passed') - return Promise.resolve({ type: 'NOOP' }) - } + // Note: We currently prefer forceURL over discovery/SSO + // with SSO we should change this to be the other way around if (action.forceURL) { const { username, protocol, host } = getUrlInfo(action.forceURL) @@ -196,6 +193,13 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { discovered: onlyTruthy }) } + + // Only do network call when we can guess discovery endpoint + if (!action.discoveryURL && !hasDiscoveryEndpoint(store.getState())) { + authLog('No discovery endpoint found or passed') + return Promise.resolve({ type: 'NOOP' }) + } + const discoveryEndpoint = getDiscoveryEndpoint( getHostedUrl(store.getState()) ) @@ -216,7 +220,15 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { if (SSOProviders) { authLog('SSO providers found on endpoint') - checkAndMergeSSOProviders(SSOProviders, true) + // we're meant to ping both the discoveryEndpoint and + // the discoveryURL and then merge them, preferring the + // discoveryEndpoint. but that's not implemeted yet. + + const preferNewDataOnCollision = true + checkAndMergeSSOProviders(SSOProviders, preferNewDataOnCollision) + + // We should move the SSO redirect handling to be outside of discovery duck. As it + // is much closer related to the auto connection code in connectionsDuck const { searchParams } = new URL(window.location.href) const SSORedirect = searchParams.get('sso_redirect') From d03a8879316b7c961d3d989c41c7edd9863dd691 Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Sun, 1 Aug 2021 17:46:48 +0200 Subject: [PATCH 28/48] Fix comment around logging --- src/shared/modules/auth/common.js | 27 +++++++++++++++++++-------- src/shared/modules/auth/helpers.js | 6 ++++-- src/shared/modules/auth/index.js | 2 +- 3 files changed, 24 insertions(+), 11 deletions(-) diff --git a/src/shared/modules/auth/common.js b/src/shared/modules/auth/common.js index b5dcb57a3e6..1d1339ee1ec 100644 --- a/src/shared/modules/auth/common.js +++ b/src/shared/modules/auth/common.js @@ -30,10 +30,10 @@ export const getInitialisationParameters = () => { export const checkAndMergeSSOProviders = ( discoveredSSOProviders, - isLocalhostOrigin + updateExistingProviders ) => { if (!discoveredSSOProviders || !discoveredSSOProviders.length) { - authLog('Invalid discovered SSO providers') + authLog('Invalid discovered SSO providers', 'warn') return } @@ -48,12 +48,15 @@ export const checkAndMergeSSOProviders = ( } discoveredSSOProviders.forEach(provider => { - if (!provider) return + if (!provider) { + authlog(`Found invalid discoved sso provider`) + return + } if ( !mandatoryKeysForSSOProviders.every(key => provider.hasOwnProperty(key)) ) { - authLog( - `Dropping invalid discovered SSO provider with id: "${provider.id}", missing key` + authlog( + `dropping invalid discovered sso provider with id: "${provider.id}", missing key` ) return } @@ -70,7 +73,7 @@ export const checkAndMergeSSOProviders = ( if ( currentSSOProviders.find(crntProvider => crntProvider.id === provider.id) ) { - if (isLocalhostOrigin) { + if (updateExistingProviders) { const idx = currentSSOProviders.findIndex( crntProvider => crntProvider.id === provider.id ) @@ -161,9 +164,17 @@ export const getCredentialsFromAuthResult = (result, idpId) => { parsedJWT[principal] || parsedJWT.email || parsedJWT.sub authLog(`Credentials assembly with username: ${credsPrincipal}`) + const configuredTokenType = + selectedSSOProvider.config?.['token_type_authentication'] const tokenTypeAuthentication = - selectedSSOProvider.config?.['token_type_authentication'] || - defaultTokenTypeAuthentication + configuredTokenType || defaultTokenTypeAuthentication + + if (!configuredTokenType) { + authLog( + `token_type_authentication not configured, using default token type "${defaultTokenTypeAuthentication}".` + ) + } + authLog( `Credentials assembled with token type "${tokenTypeAuthentication}" as password. If connection still does not succeed, make sure neo4j.conf is set up correctly` ) diff --git a/src/shared/modules/auth/helpers.js b/src/shared/modules/auth/helpers.js index 469e4961a50..7ca9c86a6f6 100644 --- a/src/shared/modules/auth/helpers.js +++ b/src/shared/modules/auth/helpers.js @@ -3,15 +3,17 @@ import { AUTH_LOGGING_PREFIX, AUTH_STORAGE_LOGS } from './constants' import { isAuthLoggingEnabled, isAuthDebuggingEnabled } from './settings' import { saveAs } from 'file-saver' +const MAX_LOG_LINES = 200 export const authLog = (msg, type = 'log') => { if (!isAuthLoggingEnabled) return if (!['log', 'error', 'warn'].includes(type)) return - const log = `${AUTH_LOGGING_PREFIX} [${new Date().toISOString()}] ${msg}` + const messageNoNewlines = msg.replace('\n', ' ') + const log = `${AUTH_LOGGING_PREFIX} [${new Date().toISOString()}] ${messageNoNewlines}` const logs = sessionStorage.getItem(AUTH_STORAGE_LOGS) || '' const logsLines = logs.split('\n') const truncatedOldLogs = - logsLines.length > 200 ? logsLines.slice(-199).join('\n') : logs + logsLines.length > MAX_LOG_LINES ? logsLines.slice(-199).join('\n') : logs sessionStorage.setItem(AUTH_STORAGE_LOGS, `${truncatedOldLogs}${log}\n`) } diff --git a/src/shared/modules/auth/index.js b/src/shared/modules/auth/index.js index 6dd9bd5e0d4..8d134d79cea 100644 --- a/src/shared/modules/auth/index.js +++ b/src/shared/modules/auth/index.js @@ -29,7 +29,7 @@ import { export const authRequestForSSO = idpId => { const selectedSSOProvider = getSSOProviderByIdpId(idpId) if (!selectedSSOProvider) { - const error = `Invalid OAuth2 endpoint: "${oauth2Endpoint}"` + const error = `Could not find any SSO provider with idpId: "${idpId}"` authLog(error) return error } From ac2abd031f3744c1f050d45c86c60f7fb6427594 Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Sun, 1 Aug 2021 18:00:45 +0200 Subject: [PATCH 29/48] Add rejections per review comments --- src/shared/modules/auth/index.js | 32 ++++++++++++------- src/shared/modules/discovery/discoveryDuck.ts | 14 ++++---- 2 files changed, 29 insertions(+), 17 deletions(-) diff --git a/src/shared/modules/auth/index.js b/src/shared/modules/auth/index.js index 8d134d79cea..d55ae0d3a0a 100644 --- a/src/shared/modules/auth/index.js +++ b/src/shared/modules/auth/index.js @@ -31,7 +31,7 @@ export const authRequestForSSO = idpId => { if (!selectedSSOProvider) { const error = `Could not find any SSO provider with idpId: "${idpId}"` authLog(error) - return error + throw new Error(error) } temporarilyStoreUrlSearchParams() @@ -40,7 +40,7 @@ export const authRequestForSSO = idpId => { if (!oauth2Endpoint) { const error = `Invalid OAuth2 endpoint: "${oauth2Endpoint}"` authLog(error) - return error + throw new Error(error) } authLog(`Using OAuth2 endpoint: "${oauth2Endpoint}" for idp_id: ${idpId}`) @@ -116,7 +116,7 @@ export const authRequestForSSO = idpId => { } else { const error = `Auth flow "${selectedSSOProvider.auth_flow}" is not supported.` authLog(error) - return error + throw new Error(error) } } @@ -138,14 +138,14 @@ export const handleAuthFromRedirect = () => if (error) { const errorMsg = `Error detected after auth redirect, aborting. Error: ${error}, Error description: ${errorDescription}` authLog(errorMsg, 'warn') - reject(errorMsg) + reject(new Error(errorMsg)) return } if (!idpId) { const errorIdpMsg = 'Invalid idp_id parameter, aborting' authLog(errorIdpMsg, 'warn') - reject(errorIdpMsg) + reject(new Error(errorIdpMsg)) return } @@ -153,7 +153,7 @@ export const handleAuthFromRedirect = () => if (state !== savedState) { const errorStateMsg = 'Invalid state parameter, aborting' authLog(errorStateMsg, 'warn') - reject(errorStateMsg) + reject(new Error(errorStateMsg)) return } window.sessionStorage.setItem(AUTH_STORAGE_STATE, '') @@ -168,7 +168,11 @@ export const handleAuthFromRedirect = () => { access_token: accessToken, id_token: idToken }, idpId ) - resolve(credentials) + if (credentials.password && credentials.username) { + resolve(credentials) + } else { + reject(new Error("Couldn't get credientals from accesstoken")) + } } else { authLog('Attempting to fetch token information in "PKCE flow"') @@ -180,27 +184,33 @@ export const handleAuthFromRedirect = () => const errorDesc = result['error_description'] || 'unknown' const errorMsg = `Error detected after auth token request, aborting. Error: ${errorType}, Error description: ${errorDesc}` authLog(errorMsg, 'warn') - reject(errorMsg) + reject(new Error(errorMsg)) } else { authLog('Successfully aquired token results') authDebug('PKCE flow result', result) const credentials = getCredentialsFromAuthResult(result, idpId) - resolve(credentials) + if (credentials.password && credentials.username) { + resolve(credentials) + } else { + reject(new Error("Couldn't get credientals from accesstoken")) + } } }) }) .catch(err => { const errRequestMsg = `Aquiring token results for PKCE auth flow failed, err: ${err}` authLog(errRequestMsg, 'warn') - reject(errRequestMsg) + reject(new Error(errRequestMsg)) }) } }) export const authRequestForToken = (idpId, code) => { const selectedSSOProvider = getSSOProviderByIdpId(idpId) - if (!selectedSSOProvider) return + if (!selectedSSOProvider) { + throw new Error(`Missing SSO Provider for idpId: ${idpId}`) + } const SSOParams = selectedSSOProvider.params || {} let details = { diff --git a/src/shared/modules/discovery/discoveryDuck.ts b/src/shared/modules/discovery/discoveryDuck.ts index d718077f7af..990ca8895a6 100644 --- a/src/shared/modules/discovery/discoveryDuck.ts +++ b/src/shared/modules/discovery/discoveryDuck.ts @@ -238,18 +238,20 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { removeSearchParamsInBrowserHistory( searchParamsToRemoveAfterAutoRedirect ) - const err = authRequestForSSO(SSORedirect) - if (err) { - SSOError = err + try { + authRequestForSSO(SSORedirect) + } catch (err) { + SSOError = err.message + authLog(err) } } else if (wasRedirectedBackFromSSOServer()) { authLog('Handling auth_flow_step redirect') try { creds = await handleAuthFromRedirect() - } catch (e) { - SSOError = e - authLog(e) + } catch (err) { + SSOError = err.message + authLog(err) } } } else { From 25aca12bae5dd8162dca91e05efd19eae20e125e Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Sun, 1 Aug 2021 18:06:15 +0200 Subject: [PATCH 30/48] Fix comment about history.replace --- src/shared/modules/auth/helpers.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/modules/auth/helpers.js b/src/shared/modules/auth/helpers.js index 7ca9c86a6f6..ef9455c0afc 100644 --- a/src/shared/modules/auth/helpers.js +++ b/src/shared/modules/auth/helpers.js @@ -100,7 +100,7 @@ export const addSearchParamsInBrowserHistory = paramsToAddObj => { const newUrl = `${ window.location.origin }?${searchParams.toString()}${crntHashParams || ''}` - window.history.replaceState({ path: newUrl }, '', newUrl) + window.history.replaceState({}, '', newUrl) } export const removeSearchParamsInBrowserHistory = paramsToRemove => { From 29f1bcafc117c56397ebc8a0878abc08dae6517f Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Sun, 1 Aug 2021 18:12:01 +0200 Subject: [PATCH 31/48] More throws --- src/shared/modules/auth/helpers.js | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/shared/modules/auth/helpers.js b/src/shared/modules/auth/helpers.js index ef9455c0afc..83a7deda864 100644 --- a/src/shared/modules/auth/helpers.js +++ b/src/shared/modules/auth/helpers.js @@ -45,8 +45,6 @@ export const createStateForRequest = () => { } export const createCodeVerifier = method => { - const errorMessage = `${AUTH_LOGGING_PREFIX} code verifier, Invalid argument` - switch (method) { case 'plain': case 'S256': @@ -57,21 +55,25 @@ export const createCodeVerifier = method => { return _btoaUrlSafe(randomString) case '': case null: - return null default: - throw errorMessage + const errMsg = `Unsupported or missing code verification method: ${method}` + authLog(errMsg) + throw new Error(errMsg) } } export const createCodeChallenge = async (method, codeVerifier) => { - const errorMessage = `${AUTH_LOGGING_PREFIX} code challenge, Invalid argument` + if (!codeVerifier) { + const errMsg = + 'Unable to create code challenger: Missing code verifier argument to createCodeChallenge' + authLog(errMsg) + throw new Error(errMsg) + } switch (method) { case 'plain': - if (codeVerifier === null) throw errorMessage return codeVerifier case 'S256': - if (codeVerifier === null) throw errorMessage let bytes = Uint8Array.from(codeVerifier, t => t.charCodeAt(0)) bytes = await window.crypto.subtle.digest('SHA-256', bytes) const stringFromBytes = Array.from(new Uint8Array(bytes), t => @@ -80,9 +82,10 @@ export const createCodeChallenge = async (method, codeVerifier) => { return _btoaUrlSafe(stringFromBytes) case '': case null: - return null default: - throw errorMessage + const errMsg = `Unsupported or missing code challenge method: ${method}` + authLog(errMsg) + throw new Error(errMsg) } } From a928eea16916cbb606e7edc414430b82293bd209 Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Sun, 1 Aug 2021 18:15:36 +0200 Subject: [PATCH 32/48] List missing keys on invalid sso provider --- src/shared/modules/auth/common.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/shared/modules/auth/common.js b/src/shared/modules/auth/common.js index 1d1339ee1ec..58386a124c7 100644 --- a/src/shared/modules/auth/common.js +++ b/src/shared/modules/auth/common.js @@ -62,11 +62,16 @@ export const checkAndMergeSSOProviders = ( } if ( !mandatoryKeysForSSOProviderParams.every(key => - provider?.params?.hasOwnProperty(key) + provider.params.hasOwnProperty(key) ) ) { + const missingKeys = mandatoryKeysForSSOProviderParams.filter( + key => !provider.params.hasOwnProperty(key) + ) authLog( - `Dropping invalid discovered SSO provider with id: "${provider.id}", missing params key` + `Dropping invalid discovered SSO provider with id: "${ + provider.id + }", missing params key(s) ${missingKeys.join(', ')}` ) return } From 130f20e05e0b4e8ae1ae10cdd96f617aa28c5b7f Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Sun, 1 Aug 2021 18:24:57 +0200 Subject: [PATCH 33/48] Update log message about principal and secure context --- src/shared/modules/auth/common.js | 17 ++++++++--------- src/shared/modules/auth/index.js | 7 +++++++ 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/shared/modules/auth/common.js b/src/shared/modules/auth/common.js index 58386a124c7..32c55f1ff7f 100644 --- a/src/shared/modules/auth/common.js +++ b/src/shared/modules/auth/common.js @@ -109,13 +109,6 @@ export const getSSOProvidersFromStorage = () => { authLog('No SSO providers in (local) storage found') return [] } - if (!window.isSecureContext) { - authLog( - 'This application is NOT executed in a secure context. SSO support is therefore disabled. Load the application in a secure context to proceed with SSO.', - 'warn' - ) - return [] - } return SSOProviders } @@ -162,8 +155,14 @@ export const getCredentialsFromAuthResult = (result, idpId) => { return emptyCredentials } - const principal = selectedSSOProvider.config?.principal || '' - authLog(`Credentials, provided principal in config: ${principal}`) + const principal = selectedSSOProvider.config?.principal + if (principal) { + authLog(`Credentials, provided principal in config: ${principal}`) + } else { + authLog( + `Credentials, no principal provided in config, falling back to 'username' then 'sub'` + ) + } const credsPrincipal = parsedJWT[principal] || parsedJWT.email || parsedJWT.sub diff --git a/src/shared/modules/auth/index.js b/src/shared/modules/auth/index.js index d55ae0d3a0a..b59e72881f2 100644 --- a/src/shared/modules/auth/index.js +++ b/src/shared/modules/auth/index.js @@ -27,6 +27,13 @@ import { } from './settings' export const authRequestForSSO = idpId => { + if (!window.isSecureContext) { + const error = + 'This application is NOT executed in a secure context. SSO support is therefore disabled. Load the application in a secure context to proceed with SSO.' + + authLog(error) + throw new Error(error) + } const selectedSSOProvider = getSSOProviderByIdpId(idpId) if (!selectedSSOProvider) { const error = `Could not find any SSO provider with idpId: "${idpId}"` From 7d07f8aa05345498074047cb23508ba4b70ce38c Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Sun, 1 Aug 2021 18:26:54 +0200 Subject: [PATCH 34/48] Use constant for url param --- src/shared/modules/discovery/discoveryDuck.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/shared/modules/discovery/discoveryDuck.ts b/src/shared/modules/discovery/discoveryDuck.ts index 990ca8895a6..46b8fdd8ae0 100644 --- a/src/shared/modules/discovery/discoveryDuck.ts +++ b/src/shared/modules/discovery/discoveryDuck.ts @@ -48,6 +48,7 @@ import { wasRedirectedBackFromSSOServer } from 'shared/modules/auth/common' import { searchParamsToRemoveAfterAutoRedirect } from 'shared/modules/auth/settings' +import { SSO_REDIRECT } from '../auth/constants' export const NAME = 'discover-bolt-host' export const CONNECTION_ID = '$$discovery' @@ -230,7 +231,7 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { // We should move the SSO redirect handling to be outside of discovery duck. As it // is much closer related to the auto connection code in connectionsDuck const { searchParams } = new URL(window.location.href) - const SSORedirect = searchParams.get('sso_redirect') + const SSORedirect = searchParams.get(SSO_REDIRECT) if (SSORedirect) { authLog(`Initialised with idpId: "${SSORedirect}"`) From c9183d7ba7c645ebeafec9d2a32002c9b4bec26d Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Sun, 1 Aug 2021 18:38:41 +0200 Subject: [PATCH 35/48] Stop log and throwing --- src/shared/modules/auth/common.js | 19 +++++-------- src/shared/modules/auth/helpers.js | 15 ++++------ src/shared/modules/auth/index.js | 45 +++++++++++++----------------- 3 files changed, 32 insertions(+), 47 deletions(-) diff --git a/src/shared/modules/auth/common.js b/src/shared/modules/auth/common.js index 32c55f1ff7f..49ed9af14b2 100644 --- a/src/shared/modules/auth/common.js +++ b/src/shared/modules/auth/common.js @@ -1,5 +1,4 @@ import jwtDecode from 'jwt-decode' -import { isObject } from 'lodash' import { AUTH_STORAGE_SSO_PROVIDERS, AUTH_STORAGE_URL_SEARCH_PARAMS, @@ -218,20 +217,16 @@ export const restoreSearchAndHashParams = () => { window.sessionStorage.setItem(AUTH_STORAGE_URL_SEARCH_PARAMS, '') - if (isObject(storedParams)) { - const crntHashParams = window.location.hash || undefined - addSearchParamsInBrowserHistory(storedParams) - const newUrl = `${window.location.href}${crntHashParams || ''}` - window.history.replaceState({}, '', newUrl) - return storedParams - } else { - authLog('Invalid temporarily stored url search params') - return null - } + const crntHashParams = window.location.hash || undefined + addSearchParamsInBrowserHistory(storedParams) + const newUrl = `${window.location.href}${crntHashParams || ''}` + window.history.replaceState({}, '', newUrl) + return storedParams } catch (err) { authLog( - `Error when parsing temporarily stored url search params, err: ${err}` + `Error when parsing temporarily stored url search params, err: ${err}. Clearing.` ) + window.sessionStorage.setItem(AUTH_STORAGE_URL_SEARCH_PARAMS, '') return null } } diff --git a/src/shared/modules/auth/helpers.js b/src/shared/modules/auth/helpers.js index 83a7deda864..94b2d73fc0e 100644 --- a/src/shared/modules/auth/helpers.js +++ b/src/shared/modules/auth/helpers.js @@ -56,18 +56,17 @@ export const createCodeVerifier = method => { case '': case null: default: - const errMsg = `Unsupported or missing code verification method: ${method}` - authLog(errMsg) - throw new Error(errMsg) + throw new Error( + `Unsupported or missing code verification method: ${method}` + ) } } export const createCodeChallenge = async (method, codeVerifier) => { if (!codeVerifier) { - const errMsg = + throw new Error( 'Unable to create code challenger: Missing code verifier argument to createCodeChallenge' - authLog(errMsg) - throw new Error(errMsg) + ) } switch (method) { @@ -83,9 +82,7 @@ export const createCodeChallenge = async (method, codeVerifier) => { case '': case null: default: - const errMsg = `Unsupported or missing code challenge method: ${method}` - authLog(errMsg) - throw new Error(errMsg) + throw new Error(`Unsupported or missing code challenge method: ${method}`) } } diff --git a/src/shared/modules/auth/index.js b/src/shared/modules/auth/index.js index b59e72881f2..4eaa98f3dda 100644 --- a/src/shared/modules/auth/index.js +++ b/src/shared/modules/auth/index.js @@ -28,26 +28,20 @@ import { export const authRequestForSSO = idpId => { if (!window.isSecureContext) { - const error = + throw new Error( 'This application is NOT executed in a secure context. SSO support is therefore disabled. Load the application in a secure context to proceed with SSO.' - - authLog(error) - throw new Error(error) + ) } const selectedSSOProvider = getSSOProviderByIdpId(idpId) if (!selectedSSOProvider) { - const error = `Could not find any SSO provider with idpId: "${idpId}"` - authLog(error) - throw new Error(error) + throw new Error(`Could not find any SSO provider with idpId: "${idpId}"`) } temporarilyStoreUrlSearchParams() const oauth2Endpoint = selectedSSOProvider.auth_endpoint if (!oauth2Endpoint) { - const error = `Invalid OAuth2 endpoint: "${oauth2Endpoint}"` - authLog(error) - throw new Error(error) + throw new Error(`Invalid OAuth2 endpoint: "${oauth2Endpoint}"`) } authLog(`Using OAuth2 endpoint: "${oauth2Endpoint}" for idp_id: ${idpId}`) @@ -121,9 +115,9 @@ export const authRequestForSSO = idpId => { authLog('Auth flow "implicit flow"') _submitForm(form, params) } else { - const error = `Auth flow "${selectedSSOProvider.auth_flow}" is not supported.` - authLog(error) - throw new Error(error) + throw new Error( + `Auth flow "${selectedSSOProvider.auth_flow}" is not supported.` + ) } } @@ -143,24 +137,22 @@ export const handleAuthFromRedirect = () => removeSearchParamsInBrowserHistory(searchParamsToRemoveAfterAuthRedirect) if (error) { - const errorMsg = `Error detected after auth redirect, aborting. Error: ${error}, Error description: ${errorDescription}` - authLog(errorMsg, 'warn') - reject(new Error(errorMsg)) + reject( + new Error( + `Error detected after auth redirect, aborting. Error: ${error}, Error description: ${errorDescription}` + ) + ) return } if (!idpId) { - const errorIdpMsg = 'Invalid idp_id parameter, aborting' - authLog(errorIdpMsg, 'warn') - reject(new Error(errorIdpMsg)) + reject(new Error('Invalid idp_id parameter, aborting')) return } const savedState = window.sessionStorage.getItem(AUTH_STORAGE_STATE) if (state !== savedState) { - const errorStateMsg = 'Invalid state parameter, aborting' - authLog(errorStateMsg, 'warn') - reject(new Error(errorStateMsg)) + reject(new Error('Invalid state parameter, aborting')) return } window.sessionStorage.setItem(AUTH_STORAGE_STATE, '') @@ -190,7 +182,6 @@ export const handleAuthFromRedirect = () => const errorType = result?.error || 'unknown' const errorDesc = result['error_description'] || 'unknown' const errorMsg = `Error detected after auth token request, aborting. Error: ${errorType}, Error description: ${errorDesc}` - authLog(errorMsg, 'warn') reject(new Error(errorMsg)) } else { authLog('Successfully aquired token results') @@ -206,9 +197,11 @@ export const handleAuthFromRedirect = () => }) }) .catch(err => { - const errRequestMsg = `Aquiring token results for PKCE auth flow failed, err: ${err}` - authLog(errRequestMsg, 'warn') - reject(new Error(errRequestMsg)) + reject( + new Error( + `Aquiring token results for PKCE auth flow failed, err: ${err}` + ) + ) }) } }) From 3070bc3b8bcd93e2c21c4f5a0de55e453664f2c8 Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Sun, 1 Aug 2021 22:02:29 +0200 Subject: [PATCH 36/48] Improve error handling --- .../modules/Stream/Auth/ConnectForm.tsx | 6 +- src/shared/modules/auth/common.js | 54 ++++++------ src/shared/modules/auth/helpers.js | 16 ++-- src/shared/modules/auth/index.js | 84 ++++++++++--------- src/shared/modules/discovery/discoveryDuck.ts | 7 +- 5 files changed, 91 insertions(+), 76 deletions(-) diff --git a/src/browser/modules/Stream/Auth/ConnectForm.tsx b/src/browser/modules/Stream/Auth/ConnectForm.tsx index 818923c823f..0b2e35b33a1 100644 --- a/src/browser/modules/Stream/Auth/ConnectForm.tsx +++ b/src/browser/modules/Stream/Auth/ConnectForm.tsx @@ -42,7 +42,7 @@ import { authRequestForSSO } from 'shared/modules/auth/index.js' import { getSSOProvidersFromStorage } from 'shared/modules/auth/common.js' import { H3 } from 'browser-components/headers' import { StyledCypherErrorMessage } from '../styled' -import { downloadAuthLogs } from 'shared/modules/auth/helpers' +import { authLog, downloadAuthLogs } from 'shared/modules/auth/helpers' const readableauthenticationMethods: Record = { [NATIVE]: 'Username / Password', @@ -140,7 +140,9 @@ export default function ConnectForm(props: ConnectFormProps): JSX.Element {

Single sign-on

{SSOProviders.map((provider: any) => ( - authRequestForSSO(provider.id)}> + authRequestForSSO(provider.id).catch(authLog)} + > {provider.name} diff --git a/src/shared/modules/auth/common.js b/src/shared/modules/auth/common.js index 49ed9af14b2..5831734ef79 100644 --- a/src/shared/modules/auth/common.js +++ b/src/shared/modules/auth/common.js @@ -36,25 +36,17 @@ export const checkAndMergeSSOProviders = ( return } - let currentSSOProviders = - JSON.parse(window.sessionStorage.getItem(AUTH_STORAGE_SSO_PROVIDERS)) || [] - if (!Array.isArray(currentSSOProviders)) { - authLog( - 'Found SSO providers in storage are defect, consider clearing the SSO provider state and reload', - 'warn' - ) - currentSSOProviders = [] - } + const currentSSOProviders = getSSOProvidersFromStorage() discoveredSSOProviders.forEach(provider => { if (!provider) { - authlog(`Found invalid discoved sso provider`) + authLog(`Found invalid discoved sso provider`) return } if ( !mandatoryKeysForSSOProviders.every(key => provider.hasOwnProperty(key)) ) { - authlog( + authLog( `dropping invalid discovered sso provider with id: "${provider.id}", missing key` ) return @@ -101,13 +93,23 @@ export const checkAndMergeSSOProviders = ( } export const getSSOProvidersFromStorage = () => { - const SSOProviders = JSON.parse( - window.sessionStorage.getItem(AUTH_STORAGE_SSO_PROVIDERS) - ) - if (!SSOProviders || !SSOProviders.length) { + let SSOProviders + try { + SSOProviders = JSON.parse( + window.sessionStorage.getItem(AUTH_STORAGE_SSO_PROVIDERS) + ) + } catch (e) {} + if (!Array.isArray(SSOProviders)) { + authLog('Found sso defect providers in storage, clearing..', 'warn') + window.sessionStorage.setItem(AUTH_STORAGE_SSO_PROVIDERS, []) + return [] + } + + if (SSOProviders.length === 0) { authLog('No SSO providers in (local) storage found') return [] } + return SSOProviders } @@ -125,12 +127,13 @@ export const getSSOProviderByIdpId = idpId => { } export const getCredentialsFromAuthResult = (result, idpId) => { - const emptyCredentials = { username: '', password: '' } authLog(`Attempting to assemble credentials for idp_id: ${idpId}`) - if (!result || !idpId) { - authLog('No result or idp_id passed in', 'warn') - return emptyCredentials + if (!result) { + throw new Error('Missing result in auth result handler') + } + if (!idpId) { + throw new Error('Missing idp_id in auth result handler') } const selectedSSOProvider = getSSOProviderByIdpId(idpId) @@ -143,16 +146,17 @@ export const getCredentialsFromAuthResult = (result, idpId) => { `Credentials, using token type "${tokenTypePrincipal}" to retrieve principal` ) - const parsedJWT = jwtDecode(result[tokenTypePrincipal]) - authDebug('Credentials, parsed JWT', parsedJWT) + let parsedJWT + try { + parsedJWT = jwtDecode(result[tokenTypePrincipal]) + } catch (e) {} if (!parsedJWT) { - authLog( - `Could not parse JWT of type "${tokenTypePrincipal}" for idp_id "${idpId}", aborting`, - 'warn' + throw new Error( + `Could not parse JWT of type "${tokenTypePrincipal}" for idp_id "${idpId}", aborting` ) - return emptyCredentials } + authDebug('Credentials, parsed JWT', parsedJWT) const principal = selectedSSOProvider.config?.principal if (principal) { diff --git a/src/shared/modules/auth/helpers.js b/src/shared/modules/auth/helpers.js index 94b2d73fc0e..afc745b4002 100644 --- a/src/shared/modules/auth/helpers.js +++ b/src/shared/modules/auth/helpers.js @@ -73,12 +73,16 @@ export const createCodeChallenge = async (method, codeVerifier) => { case 'plain': return codeVerifier case 'S256': - let bytes = Uint8Array.from(codeVerifier, t => t.charCodeAt(0)) - bytes = await window.crypto.subtle.digest('SHA-256', bytes) - const stringFromBytes = Array.from(new Uint8Array(bytes), t => - String.fromCharCode(t) - ).join('') - return _btoaUrlSafe(stringFromBytes) + try { + let bytes = Uint8Array.from(codeVerifier, t => t.charCodeAt(0)) + bytes = await window.crypto.subtle.digest('SHA-256', bytes) + const stringFromBytes = Array.from(new Uint8Array(bytes), t => + String.fromCharCode(t) + ).join('') + return _btoaUrlSafe(stringFromBytes) + } catch (e) { + throw new Error(`Failed to create code challenge with error ${e}`) + } case '': case null: default: diff --git a/src/shared/modules/auth/index.js b/src/shared/modules/auth/index.js index 4eaa98f3dda..acf80b76a9e 100644 --- a/src/shared/modules/auth/index.js +++ b/src/shared/modules/auth/index.js @@ -26,7 +26,7 @@ import { searchParamsToRemoveAfterAuthRedirect } from './settings' -export const authRequestForSSO = idpId => { +export const authRequestForSSO = async idpId => { if (!window.isSecureContext) { throw new Error( 'This application is NOT executed in a secure context. SSO support is therefore disabled. Load the application in a secure context to proceed with SSO.' @@ -98,19 +98,25 @@ export const authRequestForSSO = idpId => { `Auth flow "PKCE", using code_challenge_method: "${codeChallengeMethod}"` ) - const codeVerifier = createCodeVerifier(codeChallengeMethod) - window.sessionStorage.setItem(AUTH_STORAGE_CODE_VERIFIER, codeVerifier) - - createCodeChallenge(codeChallengeMethod, codeVerifier).then( - codeChallenge => { - params = { - ...params, - code_challenge_method: codeChallengeMethod, - code_challenge: codeChallenge - } - _submitForm(form, params) + try { + const codeVerifier = createCodeVerifier(codeChallengeMethod) + window.sessionStorage.setItem(AUTH_STORAGE_CODE_VERIFIER, codeVerifier) + + const codeChallenge = await createCodeChallenge( + codeChallengeMethod, + codeVerifier + ) + params = { + ...params, + code_challenge_method: codeChallengeMethod, + code_challenge: codeChallenge } - ) + _submitForm(form, params) + } catch (e) { + // caller handles the catching, adding rethrowing to make + // it clear we expect `createCodeVerifier` could throw + throw e + } } else if (selectedSSOProvider.auth_flow === IMPLICIT) { authLog('Auth flow "implicit flow"') _submitForm(form, params) @@ -163,38 +169,40 @@ export const handleAuthFromRedirect = () => authDebug('Implicit flow id_token', idToken) authDebug('Implicit flow access_token', accessToken) - const credentials = getCredentialsFromAuthResult( - { access_token: accessToken, id_token: idToken }, - idpId - ) - if (credentials.password && credentials.username) { + try { + const credentials = getCredentialsFromAuthResult( + { access_token: accessToken, id_token: idToken }, + idpId + ) resolve(credentials) - } else { - reject(new Error("Couldn't get credientals from accesstoken")) + } catch (e) { + reject(new Error(`Failed to get credentials: ${e.message}`)) } } else { authLog('Attempting to fetch token information in "PKCE flow"') authRequestForToken(idpId, code) - .then(data => { - data.json().then(result => { - if (result && result.error) { - const errorType = result?.error || 'unknown' - const errorDesc = result['error_description'] || 'unknown' - const errorMsg = `Error detected after auth token request, aborting. Error: ${errorType}, Error description: ${errorDesc}` - reject(new Error(errorMsg)) - } else { - authLog('Successfully aquired token results') - authDebug('PKCE flow result', result) - - const credentials = getCredentialsFromAuthResult(result, idpId) - if (credentials.password && credentials.username) { - resolve(credentials) - } else { - reject(new Error("Couldn't get credientals from accesstoken")) - } + .then(res => res.json()) + .then(body => { + if (body && body.error) { + const errorType = body?.error || 'unknown' + const errorDesc = body['error_description'] || 'unknown' + const errorMsg = `Error detected after auth token request, aborting. Error: ${errorType}, Error description: ${errorDesc}` + reject(new Error(errorMsg)) + } else { + authLog('Successfully aquired token results') + authDebug('PKCE flow result', body) + + try { + const credentials = getCredentialsFromAuthResult( + { access_token: accessToken, id_token: idToken }, + idpId + ) + resolve(credentials) + } catch (e) { + reject(new Error(`Failed to get credentials: ${e.message}`)) } - }) + } }) .catch(err => { reject( diff --git a/src/shared/modules/discovery/discoveryDuck.ts b/src/shared/modules/discovery/discoveryDuck.ts index 46b8fdd8ae0..317e19bcb6b 100644 --- a/src/shared/modules/discovery/discoveryDuck.ts +++ b/src/shared/modules/discovery/discoveryDuck.ts @@ -158,10 +158,7 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { action.requestedUseDb = passedDb } - const discoveryURL = - searchParams.get('discoveryURL') || - searchParams.get('discoveryurl') || - searchParams.get('discoveryUrl') + const discoveryURL = searchParams.get('discoveryURL') if (discoveryURL) { action.discoveryURL = discoveryURL @@ -240,7 +237,7 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { searchParamsToRemoveAfterAutoRedirect ) try { - authRequestForSSO(SSORedirect) + await authRequestForSSO(SSORedirect) } catch (err) { SSOError = err.message authLog(err) From 87d107c09cbde598cea574799179c03ae6f96f90 Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Mon, 2 Aug 2021 09:28:12 +0200 Subject: [PATCH 37/48] Add better types --- .../modules/connections/connectionsDuck.ts | 43 +++++++++++++------ 1 file changed, 30 insertions(+), 13 deletions(-) diff --git a/src/shared/modules/connections/connectionsDuck.ts b/src/shared/modules/connections/connectionsDuck.ts index a290dd0b590..f1246cf8efe 100644 --- a/src/shared/modules/connections/connectionsDuck.ts +++ b/src/shared/modules/connections/connectionsDuck.ts @@ -434,20 +434,37 @@ export const verifyConnectionCredentialsEpic = (action$: any) => { }) } -type DiscoverDataAction = { - type: typeof discovery.DONE - discovered?: { - username?: string - password?: string - requestedUseDb?: string - restApi?: string - supportsMultiDb?: string - host?: string - encrypted?: string - hasForceUrl?: boolean - SSOError?: string - attemptSSOLogin?: boolean +export type DiscoverableData = { + username?: string + password?: string + requestedUseDb?: string + restApi?: string + supportsMultiDb?: boolean + host?: string + encrypted?: string + hasForceUrl?: boolean + SSOError?: string + attemptSSOLogin?: boolean + SSOProviders?: SSOProvider[] + neo4jVersion?: string +} +export type SSOProvider = { + id: string + name: string + auth_flow: string + params: { + client_id: string + redirect_uri: string + response_type: string + scope: string } + auth_endpoint: string + well_known_discovery_uri: string +} + +export type DiscoverDataAction = { + type: typeof discovery.DONE + discovered?: DiscoverableData } function shouldTryAutoconnecting(conn: Connection | null): boolean { From 7555dc13bcf65702da3fc60531bc17b1fd719915 Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Mon, 2 Aug 2021 09:29:44 +0200 Subject: [PATCH 38/48] Helper fn for getting valid sso providers --- src/shared/modules/auth/common.js | 48 +++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/src/shared/modules/auth/common.js b/src/shared/modules/auth/common.js index 5831734ef79..1e6bcfe16a4 100644 --- a/src/shared/modules/auth/common.js +++ b/src/shared/modules/auth/common.js @@ -27,6 +27,54 @@ export const getInitialisationParameters = () => { return initParams } +export const getValidSSOProviders = discoveredSSOProviders => { + if (!discoveredSSOProviders) { + return [] + } + + if (!Array.isArray(discoveredSSOProviders)) { + authLog( + `Discovered SSO providers should be a list, got ${discoveredSSOProviders}`, + 'warn' + ) + } + if (discoveredSSOProviders.length === 0) { + authLog('List of discovered SSO providers was empty', 'warn') + return [] + } + + const validSSOProviders = discoveredSSOProviders.filter(provider => { + const missingKeys = mandatoryKeysForSSOProviders.filter( + key => !provider.hasOwnProperty(key) + ) + if (missingKeys.length !== 0) { + authLog( + `dropping invalid discovered sso provider with id: "${ + provider.id + }", missing key(s) ${missingKeys.join(', ')} ` + ) + return false + } + + const missingParamKeys = mandatoryKeysForSSOProviderParams.filter( + key => !provider.params.hasOwnProperty(key) + ) + if (missingParamKeys !== 0) { + authLog( + `Dropping invalid discovered SSO provider with id: "${ + provider.id + }", missing params key(s) ${missingKeys.join(', ')}` + ) + return false + } + + return true + }) + + authLog('Checked SSO providers') + return validSSOProviders +} + export const checkAndMergeSSOProviders = ( discoveredSSOProviders, updateExistingProviders From f5483aa8184eb43bba6c96403165fb32f4821ad2 Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Mon, 2 Aug 2021 12:44:21 +0200 Subject: [PATCH 39/48] Handle connectURL and multiple discovery endpoints properly --- .../modules/Stream/Auth/ConnectForm.tsx | 22 +- .../modules/Stream/Auth/ConnectionForm.tsx | 1 + src/shared/modules/auth/common.js | 118 +------ src/shared/modules/auth/constants.js | 1 - src/shared/modules/auth/index.js | 48 +-- src/shared/modules/discovery/discoveryDuck.ts | 301 ++++++++++-------- 6 files changed, 216 insertions(+), 275 deletions(-) diff --git a/src/browser/modules/Stream/Auth/ConnectForm.tsx b/src/browser/modules/Stream/Auth/ConnectForm.tsx index 0b2e35b33a1..1c836beb2c8 100644 --- a/src/browser/modules/Stream/Auth/ConnectForm.tsx +++ b/src/browser/modules/Stream/Auth/ConnectForm.tsx @@ -37,9 +37,11 @@ import { import { NATIVE, NO_AUTH } from 'services/bolt/boltHelpers' import { toKeyString } from 'services/utils' import { stripScheme, getScheme } from 'services/boltscheme.utils' -import { AuthenticationMethod } from 'shared/modules/connections/connectionsDuck' +import { + AuthenticationMethod, + SSOProvider +} from 'shared/modules/connections/connectionsDuck' import { authRequestForSSO } from 'shared/modules/auth/index.js' -import { getSSOProvidersFromStorage } from 'shared/modules/auth/common.js' import { H3 } from 'browser-components/headers' import { StyledCypherErrorMessage } from '../styled' import { authLog, downloadAuthLogs } from 'shared/modules/auth/helpers' @@ -66,6 +68,7 @@ interface ConnectFormProps { used: boolean supportsMultiDb: boolean SSOError?: string + SSOProviders: SSOProvider[] } export default function ConnectForm(props: ConnectFormProps): JSX.Element { @@ -73,11 +76,6 @@ export default function ConnectForm(props: ConnectFormProps): JSX.Element { const [scheme, setScheme] = useState( props.allowedSchemes ? `${getScheme(props.host)}://` : '' ) - const [SSOProviders, setSSOProviders] = useState([]) - - useEffect(() => { - setSSOProviders(getSSOProvidersFromStorage()) - }, []) useEffect(() => { if (props.allowedSchemes) { @@ -131,17 +129,21 @@ export default function ConnectForm(props: ConnectFormProps): JSX.Element { }:// for a direct connection to a single instance.` : '' - const { SSOError } = props + const { SSOError, SSOProviders } = props const showSSO = SSOProviders.length > 0 || SSOError return ( {showSSO && (

Single sign-on

- {SSOProviders.map((provider: any) => ( + {SSOProviders.map((provider: SSOProvider) => ( authRequestForSSO(provider.id).catch(authLog)} + onClick={() => + authRequestForSSO(provider).catch((e: any) => { + authLog(e.message) + }) + } > {provider.name} diff --git a/src/browser/modules/Stream/Auth/ConnectionForm.tsx b/src/browser/modules/Stream/Auth/ConnectionForm.tsx index a50f544f02e..4e814d43bc0 100644 --- a/src/browser/modules/Stream/Auth/ConnectionForm.tsx +++ b/src/browser/modules/Stream/Auth/ConnectionForm.tsx @@ -346,6 +346,7 @@ export class ConnectionForm extends Component { )} host={host} SSOError={this.state.SSOError} + SSOProviders={this.state.SSOProviders} username={this.state.username} password={this.state.password} database={this.state.requestedUseDb} diff --git a/src/shared/modules/auth/common.js b/src/shared/modules/auth/common.js index 1e6bcfe16a4..9f57f21b638 100644 --- a/src/shared/modules/auth/common.js +++ b/src/shared/modules/auth/common.js @@ -1,8 +1,8 @@ import jwtDecode from 'jwt-decode' import { - AUTH_STORAGE_SSO_PROVIDERS, AUTH_STORAGE_URL_SEARCH_PARAMS, - REDIRECT_URI + REDIRECT_URI, + SSO_REDIRECT } from './constants' import { addSearchParamsInBrowserHistory, authLog, authDebug } from './helpers' import { @@ -38,6 +38,7 @@ export const getValidSSOProviders = discoveredSSOProviders => { 'warn' ) } + if (discoveredSSOProviders.length === 0) { authLog('List of discovered SSO providers was empty', 'warn') return [] @@ -59,7 +60,7 @@ export const getValidSSOProviders = discoveredSSOProviders => { const missingParamKeys = mandatoryKeysForSSOProviderParams.filter( key => !provider.params.hasOwnProperty(key) ) - if (missingParamKeys !== 0) { + if (missingParamKeys.length !== 0) { authLog( `Dropping invalid discovered SSO provider with id: "${ provider.id @@ -75,121 +76,22 @@ export const getValidSSOProviders = discoveredSSOProviders => { return validSSOProviders } -export const checkAndMergeSSOProviders = ( - discoveredSSOProviders, - updateExistingProviders -) => { - if (!discoveredSSOProviders || !discoveredSSOProviders.length) { - authLog('Invalid discovered SSO providers', 'warn') - return - } - - const currentSSOProviders = getSSOProvidersFromStorage() - - discoveredSSOProviders.forEach(provider => { - if (!provider) { - authLog(`Found invalid discoved sso provider`) - return - } - if ( - !mandatoryKeysForSSOProviders.every(key => provider.hasOwnProperty(key)) - ) { - authLog( - `dropping invalid discovered sso provider with id: "${provider.id}", missing key` - ) - return - } - if ( - !mandatoryKeysForSSOProviderParams.every(key => - provider.params.hasOwnProperty(key) - ) - ) { - const missingKeys = mandatoryKeysForSSOProviderParams.filter( - key => !provider.params.hasOwnProperty(key) - ) - authLog( - `Dropping invalid discovered SSO provider with id: "${ - provider.id - }", missing params key(s) ${missingKeys.join(', ')}` - ) - return - } - if ( - currentSSOProviders.find(crntProvider => crntProvider.id === provider.id) - ) { - if (updateExistingProviders) { - const idx = currentSSOProviders.findIndex( - crntProvider => crntProvider.id === provider.id - ) - currentSSOProviders.splice(idx, 1) - authLog(`Updating SSO provider with id: "${provider.id}"`) - } else { - authLog( - `Not accepting discovered SSO provider with id: "${provider.id}", id exists already` - ) - return - } - } - currentSSOProviders.push(provider) - }) - - window.sessionStorage.setItem( - AUTH_STORAGE_SSO_PROVIDERS, - JSON.stringify(currentSSOProviders) - ) - authLog('Checked and merged SSO providers') -} - -export const getSSOProvidersFromStorage = () => { - let SSOProviders - try { - SSOProviders = JSON.parse( - window.sessionStorage.getItem(AUTH_STORAGE_SSO_PROVIDERS) - ) - } catch (e) {} - if (!Array.isArray(SSOProviders)) { - authLog('Found sso defect providers in storage, clearing..', 'warn') - window.sessionStorage.setItem(AUTH_STORAGE_SSO_PROVIDERS, []) - return [] - } - - if (SSOProviders.length === 0) { - authLog('No SSO providers in (local) storage found') - return [] - } - - return SSOProviders -} - -export const getSSOProviderByIdpId = idpId => { - const SSOProviders = getSSOProvidersFromStorage() - - const selectedSSOProvider = SSOProviders.find( - provider => provider.id === idpId +export const getCredentialsFromAuthResult = (result, selectedSSOProvider) => { + authLog( + `Attempting to assemble credentials for idp_id: ${selectedSSOProvider.id}` ) if (!selectedSSOProvider) { - authLog(`No matching SSO provider to passed in argument: ${idpId}`, 'warn') - return null + throw new Error('No SSO provider passed') } - return selectedSSOProvider -} - -export const getCredentialsFromAuthResult = (result, idpId) => { - authLog(`Attempting to assemble credentials for idp_id: ${idpId}`) if (!result) { throw new Error('Missing result in auth result handler') } - if (!idpId) { - throw new Error('Missing idp_id in auth result handler') - } - - const selectedSSOProvider = getSSOProviderByIdpId(idpId) - if (!selectedSSOProvider) return emptyCredentials const tokenTypePrincipal = selectedSSOProvider.config?.['token_type_principal'] || defaultTokenTypePrincipal + authLog( `Credentials, using token type "${tokenTypePrincipal}" to retrieve principal` ) @@ -201,7 +103,7 @@ export const getCredentialsFromAuthResult = (result, idpId) => { if (!parsedJWT) { throw new Error( - `Could not parse JWT of type "${tokenTypePrincipal}" for idp_id "${idpId}", aborting` + `Could not parse JWT of type "${tokenTypePrincipal}" for idp_id "${selectedSSOProvider.id}", aborting` ) } authDebug('Credentials, parsed JWT', parsedJWT) diff --git a/src/shared/modules/auth/constants.js b/src/shared/modules/auth/constants.js index a1fd3b178bf..49a2e29f010 100644 --- a/src/shared/modules/auth/constants.js +++ b/src/shared/modules/auth/constants.js @@ -7,7 +7,6 @@ export const IMPLICIT = 'implicit' export const AUTH_LOGGING_PREFIX = 'OIDC/OAuth#' const AUTH_STORAGE_PREFIX = '/auth#' -export const AUTH_STORAGE_SSO_PROVIDERS = `${AUTH_STORAGE_PREFIX}sso_providers` export const AUTH_STORAGE_STATE = `${AUTH_STORAGE_PREFIX}state` export const AUTH_STORAGE_CODE_VERIFIER = `${AUTH_STORAGE_PREFIX}code_verifier` export const AUTH_STORAGE_URL_SEARCH_PARAMS = `${AUTH_STORAGE_PREFIX}url_search_params` diff --git a/src/shared/modules/auth/index.js b/src/shared/modules/auth/index.js index acf80b76a9e..75270269b64 100644 --- a/src/shared/modules/auth/index.js +++ b/src/shared/modules/auth/index.js @@ -1,7 +1,6 @@ import { getCredentialsFromAuthResult, getInitialisationParameters, - getSSOProviderByIdpId, temporarilyStoreUrlSearchParams } from './common' import { @@ -26,16 +25,17 @@ import { searchParamsToRemoveAfterAuthRedirect } from './settings' -export const authRequestForSSO = async idpId => { +export const authRequestForSSO = async selectedSSOProvider => { + authLog('Initializing auth redirect request for SSO') + if (!selectedSSOProvider) { + throw new Error('Could not find SSO provider') + } + if (!window.isSecureContext) { throw new Error( 'This application is NOT executed in a secure context. SSO support is therefore disabled. Load the application in a secure context to proceed with SSO.' ) } - const selectedSSOProvider = getSSOProviderByIdpId(idpId) - if (!selectedSSOProvider) { - throw new Error(`Could not find any SSO provider with idpId: "${idpId}"`) - } temporarilyStoreUrlSearchParams() @@ -43,7 +43,9 @@ export const authRequestForSSO = async idpId => { if (!oauth2Endpoint) { throw new Error(`Invalid OAuth2 endpoint: "${oauth2Endpoint}"`) } - authLog(`Using OAuth2 endpoint: "${oauth2Endpoint}" for idp_id: ${idpId}`) + authLog( + `Using OAuth2 endpoint: "${oauth2Endpoint}" for idp_id: ${selectedSSOProvider.id}` + ) const form = document.createElement('form') form.setAttribute('method', 'GET') @@ -127,7 +129,7 @@ export const authRequestForSSO = async idpId => { } } -export const handleAuthFromRedirect = () => +export const handleAuthFromRedirect = SSOProviders => new Promise((resolve, reject) => { const { idp_id: idpId, @@ -140,6 +142,8 @@ export const handleAuthFromRedirect = () => error } = getInitialisationParameters() + authLog('Handling auth rediret from SSO server') + removeSearchParamsInBrowserHistory(searchParamsToRemoveAfterAuthRedirect) if (error) { @@ -163,6 +167,17 @@ export const handleAuthFromRedirect = () => } window.sessionStorage.setItem(AUTH_STORAGE_STATE, '') + const selectedSSOProvider = SSOProviders.find(({ id }) => id === idpId) + if (!selectedSSOProvider) { + reject( + new Error( + `Couldn't find identity provider with id ${idpId}, only found ${SSOProviders.map( + provider => provider.id + ).join(', ')}` + ) + ) + } + if ((tokenType || '').toLowerCase() === BEARER && accessToken) { authLog('Successfully aquired access_token in "implicit flow"') @@ -172,7 +187,7 @@ export const handleAuthFromRedirect = () => try { const credentials = getCredentialsFromAuthResult( { access_token: accessToken, id_token: idToken }, - idpId + selectedSSOProvider ) resolve(credentials) } catch (e) { @@ -181,7 +196,7 @@ export const handleAuthFromRedirect = () => } else { authLog('Attempting to fetch token information in "PKCE flow"') - authRequestForToken(idpId, code) + authRequestForToken(selectedSSOProvider, code) .then(res => res.json()) .then(body => { if (body && body.error) { @@ -195,8 +210,8 @@ export const handleAuthFromRedirect = () => try { const credentials = getCredentialsFromAuthResult( - { access_token: accessToken, id_token: idToken }, - idpId + body, + selectedSSOProvider ) resolve(credentials) } catch (e) { @@ -214,12 +229,7 @@ export const handleAuthFromRedirect = () => } }) -export const authRequestForToken = (idpId, code) => { - const selectedSSOProvider = getSSOProviderByIdpId(idpId) - if (!selectedSSOProvider) { - throw new Error(`Missing SSO Provider for idpId: ${idpId}`) - } - +export const authRequestForToken = (selectedSSOProvider, code) => { const SSOParams = selectedSSOProvider.params || {} let details = { grant_type: defaultGrantType, @@ -254,7 +264,7 @@ export const authRequestForToken = (idpId, code) => { } requestBody = requestBody.join('&') - authLog(`Request for token in PKCE flow, idp_id: ${idpId}`) + authLog(`Request for token in PKCE flow, idp_id: ${selectedSSOProvider.id}`) const requestUrl = `${selectedSSOProvider.token_endpoint}?` return window.fetch(requestUrl, { ...requestOptions, body: requestBody }) } diff --git a/src/shared/modules/discovery/discoveryDuck.ts b/src/shared/modules/discovery/discoveryDuck.ts index 317e19bcb6b..6cecc68470d 100644 --- a/src/shared/modules/discovery/discoveryDuck.ts +++ b/src/shared/modules/discovery/discoveryDuck.ts @@ -18,9 +18,12 @@ * along with this program. If not, see . */ -import Rx from 'rxjs/Rx' import remote from 'services/remote' -import { updateConnection } from 'shared/modules/connections/connectionsDuck' +import { + DiscoverableData, + SSOProvider, + updateConnection +} from 'shared/modules/connections/connectionsDuck' import { APP_START, USER_CLEAR, @@ -44,11 +47,11 @@ import { removeSearchParamsInBrowserHistory } from 'shared/modules/auth/helpers' import { - checkAndMergeSSOProviders, + getSSOServerIdIfShouldRedirect, + getValidSSOProviders, wasRedirectedBackFromSSOServer } from 'shared/modules/auth/common' import { searchParamsToRemoveAfterAutoRedirect } from 'shared/modules/auth/settings' -import { SSO_REDIRECT } from '../auth/constants' export const NAME = 'discover-bolt-host' export const CONNECTION_ID = '$$discovery' @@ -167,9 +170,16 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { return action }) .merge(some$.ofType(USER_CLEAR)) - .mergeMap((action: any) => { - // Note: We currently prefer forceURL over discovery/SSO - // with SSO we should change this to be the other way around + .mergeMap(async (action: any) => { + // we can get data about which host different mechanisms, they are ranked in + // the following order + // 1. Url param - dbms + // 2. Url param - connectURL + // 3. database in discovery endpoint + // 3. Url param - discoveryURL + + let dataFromForceUrl: DiscoverableData = {} + if (action.forceURL) { const { username, protocol, host } = getUrlInfo(action.forceURL) @@ -186,151 +196,168 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { .filter(item => item[1] /* truthy check on value */) .reduce((acc, [key, value]) => ({ ...acc, [key]: value }), {}) - return Promise.resolve({ - type: DONE, - discovered: onlyTruthy - }) + dataFromForceUrl = onlyTruthy } // Only do network call when we can guess discovery endpoint if (!action.discoveryURL && !hasDiscoveryEndpoint(store.getState())) { authLog('No discovery endpoint found or passed') - return Promise.resolve({ type: 'NOOP' }) + if (action.forceURL) { + return Promise.resolve({ type: DONE, discovered: dataFromForceUrl }) + } else { + return Promise.resolve({ type: 'NOOP' }) + } } const discoveryEndpoint = getDiscoveryEndpoint( getHostedUrl(store.getState()) ) - return Rx.Observable.fromPromise( - remote - .getJSON(action.discoveryURL || discoveryEndpoint) - // Uncomment below and comment out above when doing manual tests in dev mode to - // fake discovery response - //Promise.resolve({ - // bolt: 'bolt://localhost:7687', - // neo4j_version: '4.0.3' - //}) - .then(async result => { - const SSOProviders = - result.sso_providers || result.ssoproviders || result.ssoProviders - let creds: { username?: string; password?: string } = {} - let SSOError: string | undefined - - if (SSOProviders) { - authLog('SSO providers found on endpoint') - // we're meant to ping both the discoveryEndpoint and - // the discoveryURL and then merge them, preferring the - // discoveryEndpoint. but that's not implemeted yet. - - const preferNewDataOnCollision = true - checkAndMergeSSOProviders(SSOProviders, preferNewDataOnCollision) - - // We should move the SSO redirect handling to be outside of discovery duck. As it - // is much closer related to the auto connection code in connectionsDuck - const { searchParams } = new URL(window.location.href) - const SSORedirect = searchParams.get(SSO_REDIRECT) - - if (SSORedirect) { - authLog(`Initialised with idpId: "${SSORedirect}"`) - - removeSearchParamsInBrowserHistory( - searchParamsToRemoveAfterAutoRedirect - ) - try { - await authRequestForSSO(SSORedirect) - } catch (err) { - SSOError = err.message - authLog(err) - } - } else if (wasRedirectedBackFromSSOServer()) { - authLog('Handling auth_flow_step redirect') - - try { - creds = await handleAuthFromRedirect() - } catch (err) { - SSOError = err.message - authLog(err) - } - } - } else { - authLog( - `No SSO providers found in data at endpoint ${action.discoveryURL || - discoveryEndpoint}` - ) - // We assume if discoveryURL is set that they intended to use SSO - if (action.discoveryURL) { - SSOError = `No SSO providers found at ${action.discoveryURL}` - } - } + const discoveryEndpointData = await fetchDataFromDiscoveryUrl( + discoveryEndpoint + ) + const discoveryURLData = await (action.discoveryURL + ? fetchDataFromDiscoveryUrl(action.discoveryURL) + : Promise.resolve({ SSOProviders: [] })) + + const newProvidersFromDiscoveryURL = discoveryURLData.SSOProviders.filter( + providerFromDiscUrl => + !discoveryEndpointData.SSOProviders.find( + provider => providerFromDiscUrl.id === provider.id + ) + ) - let host = - result && - (result.bolt_routing || result.bolt_direct || result.bolt) - // Try to get info from server - if (!host) { - if (SSOProviders) { - const SSOError = 'No host found in discovery data' - authLog(SSOError) - - return { type: DONE, discovered: { SSOError } } - } else { - throw new Error('No bolt address found') - } - } + const mergedSSOProviders = discoveryEndpointData.SSOProviders.concat( + newProvidersFromDiscoveryURL + ) + + const mergedDiscoveryData = { + ...discoveryURLData, + ...discoveryEndpointData, + ...dataFromForceUrl, + SSOProviders: mergedSSOProviders + } + + const isAura = isConnectedAuraHost(store.getState()) + mergedDiscoveryData.supportsMultiDb = + !isAura && + parseInt((mergedDiscoveryData.neo4j_version || '0').charAt(0)) >= 4 - host = generateBoltUrl( - getAllowedBoltSchemesForHost(store.getState(), host), - host - ) - - const isAura = isConnectedAuraHost(store.getState()) - const supportsMultiDb = - !isAura && parseInt((result.neo4j_version || '0').charAt(0)) >= 4 - const discovered = { - supportsMultiDb, - host, - SSOError, + if (!mergedDiscoveryData.host) { + authLog('No host found in discovery data, aborting.') + return { type: DONE } + } + + mergedDiscoveryData.host = generateBoltUrl( + getAllowedBoltSchemesForHost( + store.getState(), + mergedDiscoveryData.host + ), + mergedDiscoveryData.host + ) + + let SSOError + const SSORedirect = getSSOServerIdIfShouldRedirect() + if (SSORedirect) { + authLog(`Initialised with idpId: "${SSORedirect}"`) + + removeSearchParamsInBrowserHistory( + searchParamsToRemoveAfterAutoRedirect + ) + const selectedSSOProvider = mergedDiscoveryData.SSOProviders.find( + ({ id }) => id === SSORedirect + ) + try { + await authRequestForSSO(selectedSSOProvider) + } catch (err) { + SSOError = err.message + authLog(err.message) + } + } else if (wasRedirectedBackFromSSOServer()) { + authLog('Handling auth_flow_step redirect') + + try { + const creds = (await handleAuthFromRedirect( + mergedDiscoveryData.SSOProviders + )) as { + username: string + password: string + } + + return { + type: DONE, + discovered: { + ...mergedDiscoveryData, ...creds, - attemptSSOLogin: !!creds.password + attemptSSOLogin: true } + } + } catch (err) { + SSOError = err.message + authLog(err.message) + } + } - return { type: DONE, discovered } - }) - .catch(e => { - const noDataFoundMessage = `No discovery json data found at ${action.discoveryURL || - discoveryEndpoint}` - const noHttpPrefixMessage = action?.discoveryURL - .toLowerCase() - .startsWith('http') - ? '' - : 'Double check that the url is a valid url (including HTTP(S)).' - const noJsonSuffixMessage = action?.discoveryURL - .toLowerCase() - .endsWith('.json') - ? '' - : 'Double check that the discovery url returns a valid JSON file.' - - const SSOError = [ - e.message, - noDataFoundMessage, - noHttpPrefixMessage, - noJsonSuffixMessage - ] - .map(error => { - authLog(error) - return error - }) - .join('\n') - .trim() - - if (action.discoveryURL) { - return { type: DONE, discovered: { SSOError } } - } - throw new Error('No info from endpoint') - }) - ).catch(() => { - return Promise.resolve({ type: DONE }) - }) + return { type: DONE, discovered: { ...mergedDiscoveryData, SSOError } } }) .map((a: any) => a) } + +async function fetchDataFromDiscoveryUrl( + url: string +): Promise<{ + host?: string + neo4j_version?: string + SSOProviders: SSOProvider[] +}> { + try { + const result = await remote.getJSON(url) + // Uncomment below and comment out above when doing manual tests in dev mode to + // fake discovery response + //Promise.resolve({ + // bolt: 'bolt://localhost:7687', + // neo4j_version: '4.0.3' + //}) + const host = + result && (result.bolt_routing || result.bolt_direct || result.bolt) + + const ssoProviderField = + result.sso_providers || result.ssoproviders || result.ssoProviders + + if (!ssoProviderField) { + authLog(`No sso provider field found on json at ${url}`) + } + + const SSOProviders: SSOProvider[] = getValidSSOProviders(ssoProviderField) + if (SSOProviders.length === 0) { + authLog(`None of the sso providers found at ${url} were valid`) + } else { + authLog( + `Found SSO providers with ids:${SSOProviders.map(p => p.id).join( + ', ' + )} on ${url}` + ) + } + + return { SSOProviders, host } + } catch (e) { + const noDataFoundMessage = authLog(`No discovery json data found at ${url}`) + const noHttpPrefixMessage = url.toLowerCase().startsWith('http') + ? '' + : 'Double check that the url is a valid url (including HTTP(S)).' + const noJsonSuffixMessage = url.toLowerCase().endsWith('.json') + ? '' + : 'Double check that the discovery url returns a valid JSON file.' + ;[ + `Request to ${url} failed with message: ${e.message}`, + noDataFoundMessage, + noHttpPrefixMessage, + noJsonSuffixMessage + ] + .filter(a => a) + .forEach(err => { + authLog(err) + return err + }) + return { SSOProviders: [] } + } +} From 49afb599579cbeaec1615a9e14442aa2238c6bd3 Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Mon, 2 Aug 2021 12:53:35 +0200 Subject: [PATCH 40/48] Update from jons review comments --- src/browser/modules/Stream/Auth/ConnectForm.tsx | 12 ++++++++---- src/shared/modules/auth/common.js | 6 ++++++ src/shared/modules/auth/helpers.js | 4 +++- 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/browser/modules/Stream/Auth/ConnectForm.tsx b/src/browser/modules/Stream/Auth/ConnectForm.tsx index 1c836beb2c8..163977c06cb 100644 --- a/src/browser/modules/Stream/Auth/ConnectForm.tsx +++ b/src/browser/modules/Stream/Auth/ConnectForm.tsx @@ -131,6 +131,7 @@ export default function ConnectForm(props: ConnectFormProps): JSX.Element { const { SSOError, SSOProviders } = props const showSSO = SSOProviders.length > 0 || SSOError + const [SSORedirectError, setRedirectError] = useState('') return ( {showSSO && ( @@ -140,8 +141,9 @@ export default function ConnectForm(props: ConnectFormProps): JSX.Element { - authRequestForSSO(provider).catch((e: any) => { + authRequestForSSO(provider).catch(e => { authLog(e.message) + setRedirectError(e.message) }) } > @@ -149,11 +151,13 @@ export default function ConnectForm(props: ConnectFormProps): JSX.Element { ))} - {SSOError && ( + {(SSOError || SSORedirectError) && ( ERROR -
{SSOError}
- +
{SSOError || SSORedirectError}
+ )} diff --git a/src/shared/modules/auth/common.js b/src/shared/modules/auth/common.js index 9f57f21b638..6ce0b313c15 100644 --- a/src/shared/modules/auth/common.js +++ b/src/shared/modules/auth/common.js @@ -1,4 +1,5 @@ import jwtDecode from 'jwt-decode' +import { isObject } from 'lodash-es' import { AUTH_STORAGE_URL_SEARCH_PARAMS, REDIRECT_URI, @@ -171,6 +172,11 @@ export const restoreSearchAndHashParams = () => { window.sessionStorage.setItem(AUTH_STORAGE_URL_SEARCH_PARAMS, '') + if (!isObject(storedParams)) { + throw new Error( + `Stored search params were ${storedParams}, expected an object` + ) + } const crntHashParams = window.location.hash || undefined addSearchParamsInBrowserHistory(storedParams) const newUrl = `${window.location.href}${crntHashParams || ''}` diff --git a/src/shared/modules/auth/helpers.js b/src/shared/modules/auth/helpers.js index afc745b4002..1db8cc9cd2a 100644 --- a/src/shared/modules/auth/helpers.js +++ b/src/shared/modules/auth/helpers.js @@ -13,7 +13,9 @@ export const authLog = (msg, type = 'log') => { const logsLines = logs.split('\n') const truncatedOldLogs = - logsLines.length > MAX_LOG_LINES ? logsLines.slice(-199).join('\n') : logs + logsLines.length > MAX_LOG_LINES + ? logsLines.slice(1 - MAX_LOG_LINES).join('\n') + : logs sessionStorage.setItem(AUTH_STORAGE_LOGS, `${truncatedOldLogs}${log}\n`) } From 106686ac9534cb2e64871779ec95c9dfc83be594 Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Mon, 2 Aug 2021 13:00:11 +0200 Subject: [PATCH 41/48] Add discovery logs to :debug frame --- src/shared/services/commandInterpreterHelper.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/shared/services/commandInterpreterHelper.ts b/src/shared/services/commandInterpreterHelper.ts index 5a05ae379eb..e16e9e8e091 100644 --- a/src/shared/services/commandInterpreterHelper.ts +++ b/src/shared/services/commandInterpreterHelper.ts @@ -103,6 +103,7 @@ import { import { unescapeCypherIdentifier } from './utils' import { getLatestFromFrameStack } from 'browser/modules/Stream/stream.utils' import { resolveGuide } from './guideResolverHelper' +import { AUTH_STORAGE_LOGS } from 'shared/modules/auth/constants' const PLAY_FRAME_TYPES = ['play', 'play-remote'] @@ -338,7 +339,8 @@ const availableCommands = [ const out = { userCapabilities: getUserCapabilities(store.getState()), serverConfig: getAvailableSettings(store.getState()), - browserSettings: getSettings(store.getState()) + browserSettings: getSettings(store.getState()), + ssoLogs: sessionStorage.getItem(AUTH_STORAGE_LOGS)?.split('\n') } put( frames.add({ From 62977eafe29d89b624bdc86b6517847a7d9957e3 Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Mon, 2 Aug 2021 15:31:19 +0200 Subject: [PATCH 42/48] Update styling as per lizas comments --- src/browser/components/buttons/index.tsx | 8 ++++---- src/browser/components/headers/Headers.tsx | 7 +++++++ .../modules/Stream/Auth/ConnectForm.tsx | 20 +++++++++++++------ src/browser/modules/Stream/Auth/styled.tsx | 13 +++++++----- src/browser/styles/themes.ts | 4 ++++ .../modules/connections/connectionsDuck.ts | 7 +++++++ src/shared/modules/discovery/discoveryDuck.ts | 1 + 7 files changed, 45 insertions(+), 15 deletions(-) diff --git a/src/browser/components/buttons/index.tsx b/src/browser/components/buttons/index.tsx index fd3d7ab351d..2c84a899497 100644 --- a/src/browser/components/buttons/index.tsx +++ b/src/browser/components/buttons/index.tsx @@ -115,8 +115,8 @@ export const NavigationButtonContainer = styled.li<{ isOpen: boolean }>` const StyledFormButton = styled.button` color: ${props => props.theme.primaryButtonText}; - background-color: ${props => props.theme.primaryButtonBackground}; - border: 1px solid ${props => props.theme.primaryButtonBackground}; + background-color: ${props => props.theme.primary}; + border: 1px solid ${props => props.theme.primary}; font-family: ${props => props.theme.primaryFontFamily}; padding: 6px 18px; margin-right: 10px; @@ -129,9 +129,9 @@ const StyledFormButton = styled.button` border-radius: 4px; line-height: 20px; &:hover { - background-color: ${props => props.theme.secondaryButtonBackgroundHover}; + background-color: ${props => props.theme.primary50}; color: ${props => props.theme.secondaryButtonTextHover}; - border: 1px solid ${props => props.theme.secondaryButtonBackgroundHover}; + border: 1px solid ${props => props.theme.primary50}; } ` diff --git a/src/browser/components/headers/Headers.tsx b/src/browser/components/headers/Headers.tsx index 4ef9952b873..06bda2b9628 100644 --- a/src/browser/components/headers/Headers.tsx +++ b/src/browser/components/headers/Headers.tsx @@ -26,3 +26,10 @@ export const H3 = styled.h3` font-family: ${props => props.theme.primaryFontFamily}; color: ${props => props.theme.headerText}; ` +export const H4 = styled.h3` + font-weight: 500; + font-size: 18px; + font-family: ${props => props.theme.primaryFontFamily}; + color: ${props => props.theme.headerText}; + margin-bottom: 32px; +` diff --git a/src/browser/modules/Stream/Auth/ConnectForm.tsx b/src/browser/modules/Stream/Auth/ConnectForm.tsx index 163977c06cb..2baee0b22b8 100644 --- a/src/browser/modules/Stream/Auth/ConnectForm.tsx +++ b/src/browser/modules/Stream/Auth/ConnectForm.tsx @@ -32,7 +32,8 @@ import { StyledFormContainer, StyledSSOOptions, StyledSSOButtonContainer, - StyledSSOError + StyledSSOError, + StyledSSOLogDownload } from './styled' import { NATIVE, NO_AUTH } from 'services/bolt/boltHelpers' import { toKeyString } from 'services/utils' @@ -42,9 +43,9 @@ import { SSOProvider } from 'shared/modules/connections/connectionsDuck' import { authRequestForSSO } from 'shared/modules/auth/index.js' -import { H3 } from 'browser-components/headers' import { StyledCypherErrorMessage } from '../styled' import { authLog, downloadAuthLogs } from 'shared/modules/auth/helpers' +import { H4 } from 'browser-components/headers/Headers' const readableauthenticationMethods: Record = { [NATIVE]: 'Username / Password', @@ -130,13 +131,15 @@ export default function ConnectForm(props: ConnectFormProps): JSX.Element { : '' const { SSOError, SSOProviders } = props + const showSSO = SSOProviders.length > 0 || SSOError const [SSORedirectError, setRedirectError] = useState('') + return ( {showSSO && ( -

Single sign-on

+

Single sign-on

{SSOProviders.map((provider: SSOProvider) => ( {provider.name} @@ -155,15 +159,19 @@ export default function ConnectForm(props: ConnectFormProps): JSX.Element { ERROR
{SSOError || SSORedirectError}
- + + Download logs + )} )} + {showSSO &&

Login with Password

} Connect URL diff --git a/src/browser/modules/Stream/Auth/styled.tsx b/src/browser/modules/Stream/Auth/styled.tsx index 428a8115649..d22ffa087c5 100644 --- a/src/browser/modules/Stream/Auth/styled.tsx +++ b/src/browser/modules/Stream/Auth/styled.tsx @@ -157,17 +157,20 @@ export const StyledFormContainer = styled.div` export const StyledSSOOptions = styled.div` border-right: 1px solid rgb(77, 74, 87, 0.3); - padding-right: 30px; - margin-right: 30px; + padding-right: 60px; + margin-right: 60px; + margin-left: 30px; max-width: 300px; ` + +export const StyledSSOLogDownload = styled.a` + margin-bottom: 12px; +` export const StyledSSOButtonContainer = styled.div` - margin-bottom: 5px; + margin-bottom: 12px; ` export const StyledSSOError = styled.div` margin-top: 30px; - background-color: rgba(253, 118, 110, 0.3); - border-radius: 2px; padding: 3px; white-space: pre-line; ` diff --git a/src/browser/styles/themes.ts b/src/browser/styles/themes.ts index cb275069ae7..39030787f1c 100644 --- a/src/browser/styles/themes.ts +++ b/src/browser/styles/themes.ts @@ -40,6 +40,10 @@ export const base = { neo4jBlue: '#018BFF', darkBlue: '#0056B3', + // Design system colors + primary: '#018BFF', + primary50: '#0070d9', + // Backgrounds primaryBackground: '#D2D5DA', secondaryBackground: '#fff', diff --git a/src/shared/modules/connections/connectionsDuck.ts b/src/shared/modules/connections/connectionsDuck.ts index f1246cf8efe..993b21753f9 100644 --- a/src/shared/modules/connections/connectionsDuck.ts +++ b/src/shared/modules/connections/connectionsDuck.ts @@ -487,6 +487,13 @@ export const startupConnectEpic = (action$: any, store: any) => { store.getState(), discovery.CONNECTION_ID ) + // always update SSO state providers + store.dispatch( + discovery.updateDiscoveryConnection({ + SSOProviders: discovered?.SSOProviders || [], + SSOError: discovered?.SSOError + }) + ) if ( !(discovered && discovered.hasForceUrl) && // If we have force url, don't try old connection data diff --git a/src/shared/modules/discovery/discoveryDuck.ts b/src/shared/modules/discovery/discoveryDuck.ts index 6cecc68470d..49ac361c686 100644 --- a/src/shared/modules/discovery/discoveryDuck.ts +++ b/src/shared/modules/discovery/discoveryDuck.ts @@ -310,6 +310,7 @@ async function fetchDataFromDiscoveryUrl( SSOProviders: SSOProvider[] }> { try { + authLog(`Fetching ${url} to discover SSO providers`) const result = await remote.getJSON(url) // Uncomment below and comment out above when doing manual tests in dev mode to // fake discovery response From 7c46548cb449f94d4e97d8895604cb760ad1285c Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Mon, 2 Aug 2021 17:00:25 +0200 Subject: [PATCH 43/48] Fix typo --- src/shared/modules/auth/common.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/modules/auth/common.js b/src/shared/modules/auth/common.js index 6ce0b313c15..002214e28b0 100644 --- a/src/shared/modules/auth/common.js +++ b/src/shared/modules/auth/common.js @@ -143,7 +143,7 @@ export const getCredentialsFromAuthResult = (result, selectedSSOProvider) => { export const temporarilyStoreUrlSearchParams = () => { const currentBrowserURLParams = getInitialisationParameters() authLog( - `Temporarly storing the url search params. data: "${JSON.stringify( + `Temporarily storing the url search params. data: "${JSON.stringify( currentBrowserURLParams )}"` ) From e2e4fbacf2331816e460379e5ae22b8c570d00a9 Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Mon, 2 Aug 2021 17:07:29 +0200 Subject: [PATCH 44/48] Rename provider id --- src/shared/modules/discovery/discoveryDuck.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/shared/modules/discovery/discoveryDuck.ts b/src/shared/modules/discovery/discoveryDuck.ts index 49ac361c686..bfa5da616eb 100644 --- a/src/shared/modules/discovery/discoveryDuck.ts +++ b/src/shared/modules/discovery/discoveryDuck.ts @@ -256,15 +256,15 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { ) let SSOError - const SSORedirect = getSSOServerIdIfShouldRedirect() - if (SSORedirect) { - authLog(`Initialised with idpId: "${SSORedirect}"`) + const SSORedirectId = getSSOServerIdIfShouldRedirect() + if (SSORedirectId) { + authLog(`Initialised with idpId: "${SSORedirectId}"`) removeSearchParamsInBrowserHistory( searchParamsToRemoveAfterAutoRedirect ) const selectedSSOProvider = mergedDiscoveryData.SSOProviders.find( - ({ id }) => id === SSORedirect + ({ id }) => id === SSORedirectId ) try { await authRequestForSSO(selectedSSOProvider) From 16f0e401353fcf9b25cf8ce7a78ada96e0a210cf Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Mon, 2 Aug 2021 17:16:07 +0200 Subject: [PATCH 45/48] remove extra newline in :debug logs --- src/shared/services/commandInterpreterHelper.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/shared/services/commandInterpreterHelper.ts b/src/shared/services/commandInterpreterHelper.ts index e16e9e8e091..9932450bf1e 100644 --- a/src/shared/services/commandInterpreterHelper.ts +++ b/src/shared/services/commandInterpreterHelper.ts @@ -340,7 +340,10 @@ const availableCommands = [ userCapabilities: getUserCapabilities(store.getState()), serverConfig: getAvailableSettings(store.getState()), browserSettings: getSettings(store.getState()), - ssoLogs: sessionStorage.getItem(AUTH_STORAGE_LOGS)?.split('\n') + ssoLogs: sessionStorage + .getItem(AUTH_STORAGE_LOGS) + ?.trim() + .split('\n') } put( frames.add({ From 7d9ebae39125346f9047530e190db95e274f4e44 Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Mon, 2 Aug 2021 17:20:28 +0200 Subject: [PATCH 46/48] Move code to styled comp --- src/browser/modules/Stream/Auth/ConnectForm.tsx | 5 +---- src/browser/modules/Stream/Auth/styled.tsx | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/browser/modules/Stream/Auth/ConnectForm.tsx b/src/browser/modules/Stream/Auth/ConnectForm.tsx index 2baee0b22b8..4c4aaa8d524 100644 --- a/src/browser/modules/Stream/Auth/ConnectForm.tsx +++ b/src/browser/modules/Stream/Auth/ConnectForm.tsx @@ -159,10 +159,7 @@ export default function ConnectForm(props: ConnectFormProps): JSX.Element { ERROR
{SSOError || SSORedirectError}
- + Download logs diff --git a/src/browser/modules/Stream/Auth/styled.tsx b/src/browser/modules/Stream/Auth/styled.tsx index d22ffa087c5..37efccec242 100644 --- a/src/browser/modules/Stream/Auth/styled.tsx +++ b/src/browser/modules/Stream/Auth/styled.tsx @@ -164,7 +164,7 @@ export const StyledSSOOptions = styled.div` ` export const StyledSSOLogDownload = styled.a` - margin-bottom: 12px; + cursor: pointer; ` export const StyledSSOButtonContainer = styled.div` margin-bottom: 12px; From 261556dbd67834c2379face1c4d1a61f23d47976 Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Mon, 2 Aug 2021 19:28:44 +0200 Subject: [PATCH 47/48] Cleanup comment --- src/shared/modules/discovery/discoveryDuck.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/shared/modules/discovery/discoveryDuck.ts b/src/shared/modules/discovery/discoveryDuck.ts index bfa5da616eb..f32a44d4355 100644 --- a/src/shared/modules/discovery/discoveryDuck.ts +++ b/src/shared/modules/discovery/discoveryDuck.ts @@ -172,11 +172,11 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { .merge(some$.ofType(USER_CLEAR)) .mergeMap(async (action: any) => { // we can get data about which host different mechanisms, they are ranked in - // the following order + // the following prioritization order // 1. Url param - dbms // 2. Url param - connectURL // 3. database in discovery endpoint - // 3. Url param - discoveryURL + // 4. Url param - discoveryURL let dataFromForceUrl: DiscoverableData = {} From e9261edac4456367c2e4ba52f2310d0ce075241c Mon Sep 17 00:00:00 2001 From: Oskar Damkjaer Date: Tue, 10 Aug 2021 09:27:32 +0200 Subject: [PATCH 48/48] Fix unit tests --- .../modules/Stream/Auth/ConnectionForm.tsx | 2 +- .../__snapshots__/SchemaFrame.test.tsx.snap | 72 +++++++-------- .../connections/connectionsDuck.test.ts | 12 ++- .../modules/discovery/discoveryDuck.test.ts | 91 +++++++++++++++---- src/shared/modules/discovery/discoveryDuck.ts | 5 +- 5 files changed, 121 insertions(+), 61 deletions(-) diff --git a/src/browser/modules/Stream/Auth/ConnectionForm.tsx b/src/browser/modules/Stream/Auth/ConnectionForm.tsx index 4e814d43bc0..ce3d9296183 100644 --- a/src/browser/modules/Stream/Auth/ConnectionForm.tsx +++ b/src/browser/modules/Stream/Auth/ConnectionForm.tsx @@ -346,7 +346,7 @@ export class ConnectionForm extends Component { )} host={host} SSOError={this.state.SSOError} - SSOProviders={this.state.SSOProviders} + SSOProviders={this.state.SSOProviders || []} username={this.state.username} password={this.state.password} database={this.state.requestedUseDb} diff --git a/src/browser/modules/Stream/__snapshots__/SchemaFrame.test.tsx.snap b/src/browser/modules/Stream/__snapshots__/SchemaFrame.test.tsx.snap index 603d2326a1d..3b46ab3c5c5 100644 --- a/src/browser/modules/Stream/__snapshots__/SchemaFrame.test.tsx.snap +++ b/src/browser/modules/Stream/__snapshots__/SchemaFrame.test.tsx.snap @@ -10,13 +10,13 @@ exports[`SchemaFrame renders empty 1`] = ` class="sc-dqBHgY eVMXHH" >
Constraints
ON ( book:Book ) ASSERT book.isbn IS UNIQUE
@@ -24,10 +24,10 @@ exports[`SchemaFrame renders empty 1`] = ` @@ -35,13 +35,13 @@ exports[`SchemaFrame renders empty 1`] = `
Indexes
None
@@ -49,10 +49,10 @@ exports[`SchemaFrame renders empty 1`] = ` @@ -92,43 +92,43 @@ exports[`SchemaFrame renders empty for Neo4j >= 4.0 1`] = ` class="sc-dqBHgY eVMXHH" >
Constraints
None
@@ -136,42 +136,42 @@ exports[`SchemaFrame renders empty for Neo4j >= 4.0 1`] = `
Index Name Type Uniqueness EntityType LabelsOrTypes Properties State
None
@@ -179,10 +179,10 @@ exports[`SchemaFrame renders empty for Neo4j >= 4.0 1`] = ` @@ -222,13 +222,13 @@ exports[`SchemaFrame renders results for Neo4j < 4.0 1`] = ` class="sc-dqBHgY eVMXHH" >
Constraints
None
@@ -236,10 +236,10 @@ exports[`SchemaFrame renders results for Neo4j < 4.0 1`] = ` @@ -247,13 +247,13 @@ exports[`SchemaFrame renders results for Neo4j < 4.0 1`] = `
Indexes
ON :Movie(released) ONLINE
@@ -261,10 +261,10 @@ exports[`SchemaFrame renders results for Neo4j < 4.0 1`] = ` diff --git a/src/shared/modules/connections/connectionsDuck.test.ts b/src/shared/modules/connections/connectionsDuck.test.ts index 1832b37083f..33f441004d6 100644 --- a/src/shared/modules/connections/connectionsDuck.test.ts +++ b/src/shared/modules/connections/connectionsDuck.test.ts @@ -178,8 +178,12 @@ describe('connectionsDucks Epics', () => { expect(store.getActions()).toEqual([ action, connections.useDb(null), + updateDiscoveryConnection({ + SSOProviders: [], + SSOError: undefined + }), connections.setActiveConnection(null), - updateDiscoveryConnection({ password: '' }), + updateDiscoveryConnection({ password: '', SSOError: undefined }), currentAction ]) expect(bolt.openConnection).toHaveBeenCalledTimes(0) @@ -264,8 +268,12 @@ describe('startupConnectEpic', () => { expect(actions).toEqual([ action, connections.useDb(null), + updateDiscoveryConnection({ + SSOProviders: [], + SSOError: undefined + }), connections.setActiveConnection(null), - updateDiscoveryConnection({ password: '' }), + updateDiscoveryConnection({ password: '', SSOError: undefined }), currentAction ]) expect(bolt.openConnection).toHaveBeenCalledTimes(1) diff --git a/src/shared/modules/discovery/discoveryDuck.test.ts b/src/shared/modules/discovery/discoveryDuck.test.ts index 55c89fe5543..9e7721c0bbb 100644 --- a/src/shared/modules/discovery/discoveryDuck.test.ts +++ b/src/shared/modules/discovery/discoveryDuck.test.ts @@ -97,7 +97,12 @@ describe('discoveryOnStartupEpic', () => { action, { type: discovery.DONE, - discovered: { host: expectedHost } + discovered: { + host: expectedHost, + SSOError: undefined, + SSOProviders: [], + supportsMultiDb: false + } } ]) done() @@ -118,7 +123,15 @@ describe('discoveryOnStartupEpic', () => { // Then expect(store.getActions()).toEqual([ action, - { type: discovery.DONE, discovered: { host: expectedHost } } + { + type: discovery.DONE, + discovered: { + host: expectedHost, + SSOError: undefined, + SSOProviders: [], + supportsMultiDb: false + } + } ]) done() }) @@ -140,7 +153,12 @@ describe('discoveryOnStartupEpic', () => { action, { type: discovery.DONE, - discovered: { host: expectedHost } + discovered: { + host: expectedHost, + SSOError: undefined, + SSOProviders: [], + supportsMultiDb: false + } } ]) done() @@ -166,7 +184,12 @@ describe('discoveryOnStartupEpic', () => { action, { type: discovery.DONE, - discovered: { host: expectedHost } + discovered: { + host: expectedHost, + SSOError: undefined, + SSOProviders: [], + supportsMultiDb: false + } } ]) done() @@ -180,16 +203,22 @@ describe('discoveryOnStartupEpic', () => { // Given const action = { type: APP_START, - url: 'http://localhost/?connectURL=myhost:8888' + url: 'http://localhost/?connectURL=neo4j://myhost:8888' } - const expectedHost = 'myhost:8888' + const expectedHost = 'neo4j://myhost:8888' bus.take(discovery.DONE, () => { // Then expect(store.getActions()).toEqual([ action, { type: discovery.DONE, - discovered: { host: expectedHost, hasForceURL: true } + discovered: { + host: expectedHost, + SSOError: undefined, + SSOProviders: [], + supportsMultiDb: false, + hasForceURL: true + } } ]) done() @@ -203,16 +232,22 @@ describe('discoveryOnStartupEpic', () => { // Given const action = { type: APP_START, - url: 'http://localhost/?dbms=myhost:8888' + url: 'http://localhost/?dbms=neo4j://myhost:8888' } - const expectedHost = 'myhost:8888' + const expectedHost = 'neo4j://myhost:8888' bus.take(discovery.DONE, () => { // Then expect(store.getActions()).toEqual([ action, { type: discovery.DONE, - discovered: { host: expectedHost, hasForceURL: true } + discovered: { + host: expectedHost, + SSOError: undefined, + SSOProviders: [], + supportsMultiDb: false, + hasForceURL: true + } } ]) done() @@ -226,9 +261,9 @@ describe('discoveryOnStartupEpic', () => { // Given const action = { type: APP_START, - url: 'http://localhost/?dbms=myhost:8888&db=test' + url: 'http://localhost/?dbms=neo4j://myhost:8888&db=test' } - const expectedHost = 'myhost:8888' + const expectedHost = 'neo4j://myhost:8888' bus.take(discovery.DONE, () => { // Then expect(store.getActions()).toEqual([ @@ -239,7 +274,9 @@ describe('discoveryOnStartupEpic', () => { host: expectedHost, requestedUseDb: 'test', hasForceURL: true, - supportsMultiDb: true + supportsMultiDb: true, + SSOError: undefined, + SSOProviders: [] } } ]) @@ -256,7 +293,7 @@ describe('discoveryOnStartupEpic', () => { type: APP_START, url: 'http://localhost/?connectURL=bolt%2Brouting%3A%2F%2Fmyhost%3A8889' } - const expectedHost = 'bolt+routing://myhost:8889' + const expectedHost = 'neo4j://myhost:8889' bus.take(discovery.DONE, () => { // Then expect(store.getActions()).toEqual([ @@ -264,8 +301,11 @@ describe('discoveryOnStartupEpic', () => { { type: discovery.DONE, discovered: { - hasForceURL: true, - host: expectedHost + host: expectedHost, + SSOError: undefined, + SSOProviders: [], + supportsMultiDb: false, + hasForceURL: true } } ]) @@ -283,7 +323,7 @@ describe('discoveryOnStartupEpic', () => { url: 'http://localhost/?connectURL=bolt%2Brouting%3A%2F%2Fneo4j%3Aneo4j%40myhost%3A8889' } - const expectedHost = 'bolt+routing://myhost:8889' + const expectedHost = 'neo4j://myhost:8889' bus.take(discovery.DONE, () => { // Then expect(store.getActions()).toEqual([ @@ -291,9 +331,12 @@ describe('discoveryOnStartupEpic', () => { { type: discovery.DONE, discovered: { - hasForceURL: true, + username: 'neo4j', host: expectedHost, - username: 'neo4j' + SSOError: undefined, + SSOProviders: [], + supportsMultiDb: false, + hasForceURL: true } } ]) @@ -338,7 +381,15 @@ describe('discoveryOnStartupEpic cloud env', () => { // Then expect(store.getActions()).toEqual([ action, - { type: discovery.DONE, discovered: { host: expectedHost } } + { + type: discovery.DONE, + discovered: { + host: expectedHost, + SSOError: undefined, + SSOProviders: [], + supportsMultiDb: false + } + } ]) done() }) diff --git a/src/shared/modules/discovery/discoveryDuck.ts b/src/shared/modules/discovery/discoveryDuck.ts index f32a44d4355..ad6460e2561 100644 --- a/src/shared/modules/discovery/discoveryDuck.ts +++ b/src/shared/modules/discovery/discoveryDuck.ts @@ -239,8 +239,9 @@ export const discoveryOnStartupEpic = (some$: any, store: any) => { const isAura = isConnectedAuraHost(store.getState()) mergedDiscoveryData.supportsMultiDb = - !isAura && - parseInt((mergedDiscoveryData.neo4j_version || '0').charAt(0)) >= 4 + !!action.requestedUseDb || + (!isAura && + parseInt((mergedDiscoveryData.neo4j_version || '0').charAt(0)) >= 4) if (!mergedDiscoveryData.host) { authLog('No host found in discovery data, aborting.')
Constraints
ON ( book:Book ) ASSERT book.isbn IS UNIQUE