Bug 1363722 - Allow login to work in all development workflows (#3534)

Previously it was not possible to test features that required an
authenticated user when:
* using `yarn start` with Vagrant (bug 1363722), which meant slower
  watch builds
* pointing the UI at the prod/stage API (bug 1317752), which was
  extremely limiting

Now login works in all environments, since the frontend no longer uses a
URL prefix, but instead webpack-dev-server proxies non-webpack URLs to
the chosen `BACKEND_DOMAIN` - avoiding cross-domain issues. Cookies are
rewritten to remove any `secure` directive (which is set on production),
so that they can still be read from HTTP localhost. The `Referer` has to
also be changed to stop Django's CSRF checks from rejecting request.

The slower "build into `dist` and watch" mode is therefore no longer
necessary, so `yarn start:local` instead invokes webpack-dev-server just
like `yarn start` - and the `local-watch.js` workaround has been
removed.

Support for the "publish to GitHub with hardcoded `SERVICE_DOMAIN`"
workflow has been dropped, since it was already rarely used and there is
no way to make it support login.

The API domain environment variable was renamed to `BACKEND_DOMAIN` to
avoid potential confusion given it no longer behaves the same as
`SERVICE_DOMAIN` used to.

NB: For full stack Vagrant workflows users must now connect to port
*5000* on localhost, not 8000.
This commit is contained in:
Ed Morley 2018-05-10 17:58:04 +01:00 коммит произвёл GitHub
Родитель ba908d82f7
Коммит 55654a1195
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: 4AEE18F83AFDEB23
15 изменённых файлов: 63 добавлений и 163 удалений

Просмотреть файл

@ -106,32 +106,6 @@ Building the docs locally
* Source changes will result in automatic rebuilds and browser page reload.
Sharing UI-only changes with others using GitHub Pages
------------------------------------------------------
It's possible to share UI-only changes with others (for prototyping/testing) using
GitHub Pages. This is recommended over pushing a custom branch to stage, unless the
feature requires that you be logged into Treeherder (which won't work
cross-domain).
To do this:
* Fork the Treeherder repository to your own GitHub account.
* Create a gh-pages branch locally based on the feature branch you wish to test, that is configured to point at production's API. eg:
.. code-block:: bash
git checkout (your feature branch)
git checkout -b gh-pages
SERVICE_DOMAIN=https://treeherder.mozilla.org yarn build
git add -f dist/
git commit -m "Add dist directory containing built UI"
* Push the ``gh-pages`` branch to your Treeherder fork.
* Tell people to visit: ``https://<your-username>.github.io/treeherder/dist/``
Updating package.json
---------------------

Просмотреть файл

@ -66,7 +66,7 @@ Starting a local Treeherder instance
this is more convenient because it automatically refreshes every time there's a change in the code.
* You must also build the UI. Open a new terminal window and ``vagrant ssh`` to
* You must also start the UI dev server. Open a new terminal window and ``vagrant ssh`` to
the VM again, then run the following:
.. code-block:: bash
@ -76,7 +76,7 @@ Starting a local Treeherder instance
This will build the UI code in the ``dist/`` folder and keep watching for
new changes (See the :doc:`UI installation section <ui/installation>` for more ways to work with the UI code).
* Visit http://localhost:8000 in your browser. Note: There will be no data to display until the ingestion tasks are run.
* Visit http://localhost:5000 in your browser (NB: port has changed). Note: There will be no data to display until the ingestion tasks are run.
Running the ingestion tasks
---------------------------

Просмотреть файл

@ -57,10 +57,10 @@ After you set up Treeherder, ssh (3 different tabs) into the provisioned VM and
Try out the various APIs
------------------------
* http://localhost:8000/api/project/mozilla-inbound/seta/v1/job-priorities/?build_system_type=buildbot
* http://localhost:8000/api/project/mozilla-inbound/seta/v1/job-priorities/?build_system_type=taskcluster
* http://localhost:8000/api/project/mozilla-inbound/seta/v1/job-types/
* http://localhost:8000/api/seta/v1/failures-fixed-by-commit/
* http://localhost:5000/api/project/mozilla-inbound/seta/v1/job-priorities/?build_system_type=buildbot
* http://localhost:5000/api/project/mozilla-inbound/seta/v1/job-priorities/?build_system_type=taskcluster
* http://localhost:5000/api/project/mozilla-inbound/seta/v1/job-types/
* http://localhost:5000/api/seta/v1/failures-fixed-by-commit/
* This one won't work until https://bugzilla.mozilla.org/show_bug.cgi?id=1389123 is fixed

Просмотреть файл

@ -1,10 +1,7 @@
Installation
============
It's possible to work on the UI without setting up the Vagrant VM. There are a
few limitations, such as login not being available, but it works well enough for
quick testing. For instructions on how to serve the UI with working URL rewriting,
see the Vagrant instructions.
It's possible to work on the UI without setting up the Vagrant VM.
To get started:
@ -16,7 +13,7 @@ Running the standalone development server
-----------------------------------------
The default development server runs the unminified UI and fetches data from the
production site. You do not need to set up the Vagrant VM, but login will be unavailable.
production site. You do not need to set up the Vagrant VM unless making backend changes.
* Start the development server by running:
@ -36,7 +33,7 @@ production site. You do not need to set up the Vagrant VM, but login will be una
.. code-block:: bash
$ SERVICE_DOMAIN=<url> yarn start
$ BACKEND_DOMAIN=<url> yarn start
This will run the unminified UI using ``<url>`` as the service domain.
@ -62,9 +59,7 @@ installation instructions, then follow these steps:
vagrant ~/treeherder$ yarn start:local
This will watch UI files for changes and build an unminified version in the ``dist/`` directory.
Note that this process is a little slower than using the regular development server, so you may
wish to use it only for development that requires a frontend login.
* The server will perform an initial build and then watch for new changes. Once the server is running, you can navigate to: `<http://localhost:5000>`_ to see the UI.
Building the minified UI with Vagrant
-------------------------------------
@ -82,7 +77,7 @@ If you would like to view the minified production version of the UI with Vagrant
$ yarn build
Once the build is complete, the minified version of the UI will now be accessible at http://localhost:8000.
Once the build is complete, the minified version of the UI will now be accessible at http://localhost:8000 (NB: port 8000, unlike above).
Validating JavaScript
=====================

Просмотреть файл

@ -5,6 +5,7 @@ const lintPreset = require('./lint');
const reactPreset = require('neutrino-preset-react');
const HtmlPlugin = require('html-webpack-plugin');
const htmlTemplate = require('html-webpack-template');
const CopyPlugin = require('copy-webpack-plugin');
const CWD = process.cwd();
const SRC = path.join(CWD, 'src'); // neutrino's default source directory
@ -279,6 +280,15 @@ module.exports = neutrino => {
});
neutrino.config.devtool('source-map');
// Overwriting the existing copy rule rather than modifying, since it's only
// enabled in production, whereas it really should be used in development too.
neutrino.config.plugin('copy')
.use(CopyPlugin, [
{ context: UI, from: 'contribute.json' },
{ context: UI, from: 'revision.txt' },
{ context: UI, from: 'robots.txt' },
]);
};
module.exports.CWD = CWD;

Просмотреть файл

@ -1,30 +1,38 @@
'use strict';
const webpack = require('webpack');
const basePreset = require('./base');
const UI = require('./base').UI;
// Set the service domain to production if no environment value was provided, since
// webpack-dev-server doesn't serve data from the vagrant machine.
const SERVICE_DOMAIN = (typeof process.env.SERVICE_DOMAIN !== 'undefined')
? process.env.SERVICE_DOMAIN : 'https://treeherder.mozilla.org';
// Default to the production backend (is overridden to localhost when using start:local)
const BACKEND_DOMAIN = process.env.BACKEND_DOMAIN || 'https://treeherder.mozilla.org';
module.exports = neutrino => {
basePreset(neutrino);
// Set service domain so that ui/js/config can use it:
neutrino.config
.plugin('define')
.use(webpack.DefinePlugin, {
SERVICE_DOMAIN: JSON.stringify(SERVICE_DOMAIN)
});
// Set up the dev server with an api proxy to the service domain:
// Make the dev server proxy any paths not recognised by webpack to the specified backend.
neutrino.config.devServer
.contentBase(UI)
.set('proxy', {
'/api/*': {
target: SERVICE_DOMAIN,
changeOrigin: true
'*': {
target: BACKEND_DOMAIN,
changeOrigin: true,
onProxyReq: (proxyReq) => {
// Adjust the referrer to keep Django's CSRF protection happy, whilst
// still making it clear that the requests were from local development.
proxyReq.setHeader('referer', `${BACKEND_DOMAIN}/webpack-dev-server`);
},
onProxyRes: (proxyRes) => {
// Strip the cookie `secure` attribute, otherwise prod cookies
// will be rejected by the browser when using non-HTTPS localhost:
// https://github.com/nodejitsu/node-http-proxy/pull/1166
const removeSecure = str => str.replace(/; secure/i, '');
const setCookie = proxyRes.headers['set-cookie'];
if (setCookie) {
const result = Array.isArray(setCookie)
? setCookie.map(removeSecure)
: removeSecure(setCookie);
proxyRes.headers['set-cookie'] = result;
}
}
}
});
};

Просмотреть файл

@ -59,9 +59,6 @@ module.exports = neutrino => {
'space-infix-ops': 'off',
'spaced-comment': 'off',
},
globals: [
'SERVICE_DOMAIN',
]
}
}));
};

Просмотреть файл

@ -1,43 +0,0 @@
'use strict';
const productionPreset = require('./production');
// Taskcluster login POSTs in treeherder require that the UI is served from the same
// origin as the backend. To allow for logins in watched, unminified mode, here we
// build a webpack watcher that recompiles sources to dist/ on change, so that the
// Vagrant box may serve them. This approach is slower than using the dev server, but
// it may become unnecessary if Bug 1317752 (Enable logging in with Taskcluster Auth
// cross-domain) is completed.
module.exports = neutrino => {
// This configuration is based on the production config, but is not minified
productionPreset(neutrino);
// Removing the dev server configuration puts neutrino into watch mode
neutrino.config.devServer.clear();
// Don't minify:
neutrino.config.plugins.delete('minify');
// Now we must remove some hot-reload settings triggered by the 'development'
// NODE_ENV variable set by `neutrino start`:
// Remove hot loader plugin:
neutrino.config.plugins.delete('hot');
// Remove hot loader patch from index:
const protocol = process.env.HTTPS ? 'https' : 'http';
const host = process.env.HOST || 'localhost';
const port = parseInt(process.env.PORT) || 5000;
neutrino.config.entry('index')
.delete(`webpack-dev-server/client?${protocol}://${host}:${port}/`)
.delete('webpack/hot/dev-server')
.delete(require.resolve('react-hot-loader/patch'));
// Remove hot loader react plugin:
neutrino.config.module
.rule('compile')
.loader('babel', ({ options }) => {
options.env.development.plugins.shift();
return { options };
});
};

Просмотреть файл

@ -1,10 +1,7 @@
'use strict';
const webpack = require('webpack');
const basePreset = require('./base');
const CopyPlugin = require('copy-webpack-plugin');
const CleanPlugin = require('clean-webpack-plugin');
const CWD = require('./base').CWD;
const UI = require('./base').UI;
const DIST = require('./base').DIST;
module.exports = neutrino => {
@ -23,25 +20,6 @@ module.exports = neutrino => {
}
));
// Define the service domain globally:
neutrino.config.plugin('define')
.use(webpack.DefinePlugin, {
SERVICE_DOMAIN: JSON.stringify(
typeof process.env.SERVICE_DOMAIN !== 'undefined' ? process.env.SERVICE_DOMAIN : '')
});
// The copy plugin is overwritten and not injected so that when this preset is
// imported in ./local-watch.js and run via `neutrino start`, it is still included
// in the config (it is only applied in !development by default):
neutrino.config.plugin('copy')
.use(CopyPlugin, [
{context: UI, from: 'contribute.json'},
{context: UI, from: 'revision.txt'},
{context: UI, from: 'robots.txt'},
]);
// Likewise for this clean plugin:
neutrino.config.plugin('clean')
.use(CleanPlugin, [DIST], { root: CWD } );

Просмотреть файл

@ -82,8 +82,8 @@
"build": "node ./node_modules/neutrino/bin/neutrino build --presets ./neutrino-custom/production.js",
"lint": "node ./node_modules/eslint/bin/eslint.js --ext .js,.jsx ui/",
"start": "node ./node_modules/neutrino/bin/neutrino start --presets ./neutrino-custom/development.js",
"start:local": "node ./node_modules/neutrino/bin/neutrino start --presets ./neutrino-custom/local-watch.js",
"start:stage": "SERVICE_DOMAIN=https://treeherder.allizom.org node ./node_modules/neutrino/bin/neutrino start --presets ./neutrino-custom/development.js",
"start:local": "BACKEND_DOMAIN=http://localhost:8000 node ./node_modules/neutrino/bin/neutrino start --presets ./neutrino-custom/development.js",
"start:stage": "BACKEND_DOMAIN=https://treeherder.allizom.org node ./node_modules/neutrino/bin/neutrino start --presets ./neutrino-custom/development.js",
"test": "node ./node_modules/neutrino/bin/neutrino test --presets ./neutrino-custom/test.js",
"test:watch": "node ./node_modules/neutrino/bin/neutrino test --watch --presets ./neutrino-custom/test.js"
}

Просмотреть файл

@ -9,13 +9,13 @@ import { configure } from 'enzyme';
import Adapter from 'enzyme-adapter-react-16';
// Global variables are set here instead of with webpack.ProvidePlugin
// because neutrino removes plugin definitions for karma runs
// because neutrino removes plugin definitions for karma runs:
// https://github.com/mozilla-neutrino/neutrino-dev/issues/617
window.jQuery = jQuery;
configure({ adapter: new Adapter() });
const jsContext = require.context('../../../ui/js', true, /^\.\/.*\.jsx?$/);
window.SERVICE_DOMAIN = process.env.SERVICE_DOMAIN || '';
jsContext('./filters.js');
const controllerContext = require.context('../../../ui/js/controllers', true, /^\.\/.*\.jsx?$/);

Просмотреть файл

@ -122,12 +122,6 @@ label.dropdown-item {
padding-right: 14px;
}
.nav-login-btn-unavail {
font-size: 12px;
color: grey;
cursor: not-allowed;
}
.th-context-navbar {
background-color: #354048;
overflow: visible;

Просмотреть файл

@ -9,8 +9,10 @@ export const createQueryParams = function createQueryParams(params) {
return `?${query.toString()}`;
};
// Leaving this here since even though SERVICE_DOMAIN no longer exists (proxying
// is used instead), it provides a single place to modify if needed in the future.
export const getServiceUrl = function getServiceUrl(uri) {
return `${SERVICE_DOMAIN}${uri}`;
return uri;
};
export const getApiUrl = function getApiUrl(uri) {
@ -129,9 +131,10 @@ export const parseQueryParams = function parseQueryParams(search) {
return obj;
};
// TODO: Combine this with getApiUrl().
export const createApiUrl = function createApiUrl(api, params) {
const query = createQueryParams(params);
return `${SERVICE_DOMAIN}/api/${api}${query}`;
return `/api/${api}${query}`;
};
//bugs can be one bug or a comma separated (no spaces) string of bugs

Просмотреть файл

@ -36,11 +36,8 @@ treeherder.component("login", {
</span>
<span class="btn nav-login-btn"
ng-if="!$ctrl.user.loggedin && $ctrl.userCanLogin"
ng-if="!$ctrl.user.loggedin"
ng-click="$ctrl.login()">Login/Register</span>
<span ng-if="!$ctrl.userCanLogin"
class="nav-login-btn nav-login-btn-unavail"
title="SERVICE_DOMAIN does not match host domain">Login not available</span>
`,
bindings: {
// calls to the HTML which sets the user value in the $rootScope.
@ -55,18 +52,6 @@ treeherder.component("login", {
ctrl.user = {};
// check if the user can login. SERVICE_DOMAIN must match
// host domain. Remove this if we fix
// Bug 1317752 - Enable logging in with Taskcluster Auth cross-domain
if (!SERVICE_DOMAIN) {
// SERVICE_DOMAIN isn't being used, so no mismatch possible.
this.userCanLogin = true;
} else {
const a = document.createElement('a');
a.href = SERVICE_DOMAIN;
this.userCanLogin = (a.hostname === $location.host() && a.port === $location.port);
}
/**
* Using a window listener here because I wasn't getting reliable
* results from the events from angular-local-storage.
@ -91,15 +76,13 @@ treeherder.component("login", {
});
// Ask the back-end if a user is logged in on page load
if (ctrl.userCanLogin) {
ThUserModel.get().then(async function (currentUser) {
if (currentUser.email && localStorage.getItem('userSession')) {
ctrl.setLoggedIn(currentUser);
} else {
ctrl.setLoggedOut();
}
});
}
ThUserModel.get().then(async function (currentUser) {
if (currentUser.email && localStorage.getItem('userSession')) {
ctrl.setLoggedIn(currentUser);
} else {
ctrl.setLoggedOut();
}
});
/**
* Opens a new tab to handle authentication, which will get closed

Просмотреть файл

@ -259,6 +259,7 @@ function toggleHideClassified(store, fetchParams) {
});
}
// TODO: Remove this now that SERVICE_DOMAIN no longer exists.
// Remove the host from the URL. This is because sometimes the URL
// will include the SERVICE_DOMAIN and sometimes SERVICE_DOMAIN will
// be blank and therefore the browser will fill in the domain. So