Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

move basic auth state to appStore #3390

Merged
merged 1 commit into from
Aug 27, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 61 additions & 0 deletions app/browser/basicAuth.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
const electron = require('electron')
const app = electron.app
const appActions = require('../../js/actions/appActions')
const appConstants = require('../../js/constants/appConstants')
const appDispatcher = require('../../js/dispatcher/appDispatcher')
const appStore = require('../../js/stores/appStore')

// URLs to callback for auth.
let authCallbacks = {}

const cleanupAuthCallback = (tabId) => {
delete authCallbacks[tabId]
}

const runAuthCallback = (tabId, detail) => {
let cb = authCallbacks[tabId]
if (cb) {
delete authCallbacks[tabId]
if (detail) {
let username = detail.get('username')
let password = detail.get('password')
cb(username, password)
} else {
cb()
}
}
}

const doAction = (action) => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

not sure how I feel about this. I wanted to separate the electron code from the appState to make it more unit testable, but not sure if this is the best way to handle it or not. I also considered some kind of change listener, but I think that might need a larger change to make sure the notifications are efficient

switch (action.actionType) {
case appConstants.APP_SET_LOGIN_RESPONSE_DETAIL:
appDispatcher.waitFor([appStore.dispatchToken], () => {
runAuthCallback(action.tabId, action.detail)
})
break
default:
}
}

const basicAuth = {
init: () => {
appDispatcher.register(doAction)
app.on('login', (e, webContents, request, authInfo, cb) => {
e.preventDefault()
let tabId = webContents.getId()
authCallbacks[tabId] = cb
webContents.on('destroyed', () => {
cleanupAuthCallback(tabId)
})
webContents.on('crashed', () => {
cleanupAuthCallback(tabId)
})
appActions.setLoginRequiredDetail(tabId, {
request,
authInfo
})
})
}
}

module.exports = basicAuth
41 changes: 41 additions & 0 deletions app/common/state/basicAuthState.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

const tabState = require('./tabState')
const { makeImmutable } = require('./immutableUtil')

const loginRequiredDetail = 'loginRequiredDetail'
tabState.addTransientFields([loginRequiredDetail])

const basicAuthState = {
setLoginRequiredDetail: (appState, tabId, detail) => {
appState = makeImmutable(appState)
detail = makeImmutable(detail)
let tab = tabState.getOrCreateByTabId(appState, tabId)
if (!detail || detail.size === 0) {
tab = tab.delete(loginRequiredDetail)
} else {
tab = tab.set(loginRequiredDetail, detail)
}
return tabState.updateTab(appState, tabId, tab)
},

getLoginRequiredDetail: (appState, tabId) => {
appState = makeImmutable(appState)
let tab = tabState.getByTabId(appState, tabId)
return tab && tab.get(loginRequiredDetail)
},

setLoginResponseDetail: (appState, tabId, detail) => {
appState = makeImmutable(appState)
let tab = tabState.getByTabId(appState, tabId)
if (!tab) {
return appState
}
tab = tab.delete(loginRequiredDetail)
return tabState.updateTab(appState, tabId, tab)
}
}

module.exports = basicAuthState
16 changes: 16 additions & 0 deletions app/common/state/immutableUtil.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

const Immutable = require('immutable')

const immutableUtils = {
makeImmutable: (obj) => {
Copy link
Member

Choose a reason for hiding this comment

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

Definitely like this 👍

if (!obj) {
return null
}
return obj.toJS ? obj : Immutable.fromJS(obj)
}
}

module.exports = immutableUtils
69 changes: 69 additions & 0 deletions app/common/state/tabState.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

const { makeImmutable } = require('./immutableUtil')

let transientFields = ['tabId', 'windowId']

const tabState = {
defaultTabState: makeImmutable({
windowId: -1,
frameKey: -1,
tabId: -1
}),

getTabIndexByTabId: (state, tabId) => {
return state.get('tabs').findIndex((tab) => tab.get('tabId') === tabId)
},

createTab: (props) => {
props = makeImmutable(props)
return tabState.defaultTabState.merge(props)
},

getOrCreateByTabId: (state, tabId) => {
let tab = tabState.getByTabId(state, tabId)
return tab || tabState.createTab({tabId})
},

getByTabId: (state, tabId) => {
return state.get('tabs').find((tab) => tab.get('tabId') === tabId)
},

closeTab: (state, tabId) => {
let index = tabState.getTabIndexByTabId(state, tabId)
if (index === -1) {
return state
}

let tabs = state.get('tabs').delete(index)
state = state.set('tabs', tabs)
return state
},

updateTab: (state, tabId, tab) => {
let tabs = state.get('tabs')
let index = tabState.getTabIndexByTabId(state, tabId)
tabs = tabs.delete(index).insert(index, tab)
return state.set('tabs', tabs)
},

addTransientFields: (fields) => {
transientFields = transientFields.concat(fields)
},

getTransientFields: () => {
return transientFields
},

getPersistentTabState: (tab) => {
tab = makeImmutable(tab)
tabState.getTransientFields().forEach((field) => {
tab = tab.delete(field)
})
return tab
}
}

module.exports = tabState
29 changes: 2 additions & 27 deletions app/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ const ledger = require('./ledger')
const flash = require('../js/flash')
const contentSettings = require('../js/state/contentSettings')
const privacy = require('../js/state/privacy')
const basicAuth = require('./browser/basicAuth')

// Used to collect the per window state when shutting down the application
let perWindowState = []
Expand All @@ -73,8 +74,6 @@ let lastWindowClosed = false
// Domains to accept bad certs for. TODO: Save the accepted cert fingerprints.
let acceptCertDomains = {}
let errorCerts = {}
// URLs to callback for auth.
let authCallbacks = {}
// Don't show the keytar prompt more than once per 24 hours
let throttleKeytar = false

Expand Down Expand Up @@ -252,16 +251,6 @@ app.on('ready', () => {
})
})

app.on('login', (e, webContents, request, authInfo, cb) => {
e.preventDefault()
authCallbacks[request.url] = cb
let sender = webContents.hostWebContents || webContents
sender.send(messages.LOGIN_REQUIRED, {
url: request.url,
tabId: webContents.getId(),
authInfo
})
})
app.on('window-all-closed', () => {
// On macOS it is common for applications and their menu bar
// to stay active until the user quits explicitly with Cmd + Q
Expand Down Expand Up @@ -354,21 +343,6 @@ app.on('ready', () => {
return event.returnValue
})

ipcMain.on(messages.LOGIN_RESPONSE, (e, url, username, password) => {
if (username || password) {
// Having 2 of the same tab URLs open right now, where both require auth
// can cause an error / alert here. Ignore it for now.
try {
if (authCallbacks[url]) {
authCallbacks[url](username, password)
}
} catch (e) {
console.error(e)
}
}
delete authCallbacks[url]
})

process.on(messages.UNDO_CLOSED_WINDOW, () => {
if (lastWindowState) {
appActions.newWindow(undefined, undefined, lastWindowState)
Expand All @@ -392,6 +366,7 @@ app.on('ready', () => {
appActions.setState(Immutable.fromJS(initialState))
return loadedPerWindowState
}).then((loadedPerWindowState) => {
basicAuth.init()
contentSettings.init()
privacy.init()
Extensions.init()
Expand Down
7 changes: 7 additions & 0 deletions app/sessionStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const downloadStates = require('../js/constants/downloadStates')
const {tabFromFrame} = require('../js/state/frameStateUtil')
const sessionStorageVersion = 1
const filtering = require('./filtering')
// const tabState = require('./common/state/tabState')

let suffix = ''
if (process.env.NODE_ENV === 'development') {
Expand Down Expand Up @@ -274,6 +275,11 @@ module.exports.cleanAppData = (data, isShutdown) => {
})
}
}
// all data in tabs is transient while we make the transition from window to app state
delete data.tabs
// if (data.tabs) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I left this in here so it was clear how it would work. I wanted to keep all the state management for tabs in on place

// data.tabs = data.tabs.map((tab) => tabState.getPersistentTabState(tab).toJS())
// }
}

/**
Expand Down Expand Up @@ -356,6 +362,7 @@ module.exports.loadAppState = () => {
module.exports.defaultAppState = () => {
return {
sites: [],
tabs: [],
visits: [],
settings: {},
siteSettings: {},
Expand Down
12 changes: 12 additions & 0 deletions docs/appActions.md
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,18 @@ Adds a word to the dictionary



### setLoginRequiredDetail(tabId, detail)

Adds information about pending basic auth login requests

**Parameters**

**tabId**: `number`, The tabId that generated the request

**detail**: `string`, login request info



### clearAppData(clearDataDetail)

Clears the data specified in dataDetail
Expand Down
12 changes: 0 additions & 12 deletions docs/windowActions.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,18 +88,6 @@ Dispatches a message to set the frame error state



### setLoginRequiredDetail(frameProps, detail)

Dispatches a message to set the login required detail.

**Parameters**

**frameProps**: `Object`, The frame where the login required prompt should be shown.

**detail**: `Object`, Details of the login required operation.



### setNavBarUserInput(location)

Dispatches a message to the store to set the user entered text for the URL bar.
Expand Down
21 changes: 21 additions & 0 deletions js/actions/appActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,27 @@ const appActions = {
})
},

/**
* Adds information about pending basic auth login requests
* @param {number} tabId - The tabId that generated the request
* @param {string} detail - login request info
*/
setLoginRequiredDetail: function (tabId, detail) {
AppDispatcher.dispatch({
actionType: AppConstants.APP_SET_LOGIN_REQUIRED_DETAIL,
tabId,
detail
})
},

setLoginResponseDetail: function (tabId, detail) {
AppDispatcher.dispatch({
actionType: AppConstants.APP_SET_LOGIN_RESPONSE_DETAIL,
tabId,
detail
})
},

/**
* Clears the data specified in dataDetail
* @param {object} clearDataDetail - the app data to clear as per doc/state.md's clearBrowsingDataDetail
Expand Down
17 changes: 0 additions & 17 deletions js/actions/windowActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,19 +137,6 @@ const windowActions = {
})
},

/**
* Dispatches a message to set the login required detail.
* @param {Object} frameProps - The frame where the login required prompt should be shown.
* @param {Object} detail - Details of the login required operation.
*/
setLoginRequiredDetail: function (frameProps, detail) {
dispatch({
actionType: WindowConstants.WINDOW_SET_LOGIN_REQUIRED_DETAIL,
frameProps,
detail
})
},

/**
* Dispatches a message to the store to set the user entered text for the URL bar.
* Unlike setLocation and loadUrl, this does not modify the state of src and location.
Expand Down Expand Up @@ -324,10 +311,6 @@ const windowActions = {
webviewActions.setFullScreen(false)
this.setFullScreen(frameProps, false)
}
// Flush out any pending login required prompts
if (frameProps && frameProps.getIn(['security', 'loginRequiredDetail'])) {
ipc.send(messages.LOGIN_RESPONSE, frameProps.get('location'))
}
// Unless a caller explicitly specifies to close a pinned frame, then
// ignore the call.
const nonPinnedFrames = frames.filter((frame) => !frame.get('pinnedLocation'))
Expand Down
Loading