Coder Social home page Coder Social logo

fullstackacademy / boilermaker Goto Github PK

View Code? Open in Web Editor NEW
216.0 15.0 709.0 3.49 MB

Code scaffold for projects

Home Page: https://www.youtube.com/watch?v=7bLSuTHH4Ag&list=PLx0iOsdUOUmn7D5XL4mRUftn8hvAJGs8H

License: MIT License

JavaScript 93.87% CSS 0.64% HTML 1.01% Shell 4.48%

boilermaker's Introduction

Boilermaker

Good things come in pairs

Looking to mix up a backend with express/sequelize and a frontend with react/redux? That's boilermaker!

Follow along with the boilerplate workshop to make your own! This canonical version can serve as a reference, or a starting point. For an in depth discussion into the code that makes up this repository, see the Boilermaker Guided Tour

Setup

To use this as boilerplate, you'll need to take the following steps:

  • Don't fork or clone this repo! Instead, create a new, empty directory on your machine and git init (or create an empty repo on Github and clone it to your local machine)
  • Run the following commands:
git remote add boilermaker https://github.com/FullstackAcademy/boilermaker.git
git fetch boilermaker
git merge boilermaker/master

Why did we do that? Because every once in a while, boilermaker may be updated with additional features or bug fixes, and you can easily get those changes from now on by entering:

git fetch boilermaker
git merge boilermaker/master

Customize

Now that you've got the code, follow these steps to get acclimated:

  • Update project name and description in package.json and .travis.yml files
  • npm install
  • Create two postgres databases (MY_APP_NAME should match the name parameter in package.json):
export MY_APP_NAME=boilermaker
createdb $MY_APP_NAME
createdb $MY_APP_NAME-test
  • By default, running npm test will use boilermaker-test, while regular development uses boilermaker
  • Create a file called secrets.js in the project root
    • This file is listed in .gitignore, and will only be required in your development environment
    • Its purpose is to attach the secret environment variables that you will use while developing
    • However, it's very important that you not push it to Github! Otherwise, prying eyes will find your secret API keys!
    • It might look like this:
process.env.GOOGLE_CLIENT_ID = 'hush hush'
process.env.GOOGLE_CLIENT_SECRET = 'pretty secret'
process.env.GOOGLE_CALLBACK = '/auth/google/callback'

OAuth

  • To use OAuth with Google, complete the steps above with a real client ID and client secret supplied from Google

Linting

Linters are fundamental to any project. They ensure that your code has a consistent style, which is critical to writing readable code.

Boilermaker comes with a working linter (ESLint, with eslint-config-fullstack) "out of the box." However, everyone has their own style, so we recommend that you and your team work out yours and stick to it. Any linter rule that you object to can be "turned off" in .eslintrc.json. You may also choose an entirely different config if you don't like ours:

Start

Running npm run start-dev will make great things happen!

If you want to run the server and/or webpack separately, you can also npm run start-server and npm run build-client.

From there, just follow your bliss.

Deployment

Ready to go world wide? Here's a guide to deployment! There are two supported ways to deploy in Boilermaker:

  • automatically, via continuous deployment with Travis.
  • "manually", from your local machine via the deploy script.

Either way, you'll need to set up your deployment server to start. The steps below are also covered in the CI/CD workshop.

Heroku

  1. Set up the Heroku command line tools
  2. heroku login
  3. Add a git remote for heroku:
  • If you are creating a new app...

    1. heroku create or heroku create your-app-name if you have a name in mind.
    2. heroku addons:create heroku-postgresql:hobby-dev to add ("provision") a postgres database to your heroku dyno
  • If you already have a Heroku app...

    1. heroku git:remote your-app-name You'll need to be a collaborator on the app.

Travis

NOTE that this step assumes that Travis-CI is already testing your code. Continuous Integration is not about testing per se – it's about continuously integrating your changes into the live application, instead of periodically releasing new versions. CI tools can not only test your code, but then automatically deploy your app. This is known as Continuous Deployment. Boilermaker comes with a .travis.yml configuration almost ready for continuous deployment; follow these steps to the job.

  1. Run the following commands to create a new branch:
git checkout master
git pull
git checkout -b f/travis-deploy
  1. Run the following script to finish configuring travis.yml : npm run heroku-token This will use your heroku CLI (that you configured previously, if not then see above) to generate an authentication token. It will then use openssl to encrypt this token using a public key that Travis has generated for you. It will then update your .travis.yml file with the encrypted value to be sent with the secure key under the api_key.
  2. Run the following commands to commit these changes
git add .travis.yml
git commit -m 'travis: activate deployment'
git push -u origin f/travis-deploy
  1. Make a Pull Request for the new branch, get it approved, and merge it into the master branch.

NOTE that this script depends on your local origin Git remote matching your GitHub URL, and your local heroku remote matching the name of your Heroku app. This is only an issue if you rename your GitHub organization, repository name or Heroku app name. You can update these values using git remote and its related commands.

Travis CLI

There is a procedure to complete the above steps by installing the official Travis CLI tools. This requires a recent Ruby, but this step should not be, strictly speaking, necessary. Only explore this option when the above has failed.

That's it! From now on, whenever master is updated on GitHub, Travis will automatically push the app to Heroku for you.

Cody's own deploy script

Your local copy of the application can be pushed up to Heroku at will, using Boilermaker's handy deployment script:

  1. Make sure that all your work is fully committed and merged into your master branch on Github.
  2. If you currently have an existing branch called "deploy", delete it now (git branch -d deploy). We will use a dummy branch with the name deploy (see below), so and the script below will error if a branch with that name already exists.
  3. npm run deploy _ this will cause the following commands to happen in order: _ git checkout -b deploy: checks out a new branch called deploy. Note that the name deploy here is not magical, but it needs to match the name of the branch we specify when we push to our heroku remote. _ webpack -p: webpack will run in "production mode" _ git add -f public/bundle.js public/bundle.js.map: "force" add these files which are listed in .gitignore. _ git commit --allow-empty -m 'Deploying': create a commit, even if nothing changed _ git push --force heroku deploy:master: push your local deploy branch to the master branch on heroku _ git checkout master: return to your master branch _ git branch -D deploy: remove the deploy branch

Now, you should be deployed!

Why do all of these steps? The big reason is because we don't want our production server to be cluttered up with dev dependencies like webpack, but at the same time we don't want our development git-tracking to be cluttered with production build files like bundle.js! By doing these steps, we make sure our development and production environments both stay nice and clean!

boilermaker's People

Contributors

avillr avatar b17z avatar collin avatar connieelee avatar dadewoyin avatar dakotablair avatar dependabot[bot] avatar dpatlut avatar fterdal avatar geoffbass avatar glebec avatar khumphrey avatar knxyzkn avatar luigilegion avatar nlane avatar omribernstein avatar queerviolet avatar rinaldo avatar sethfork avatar short-matthew-f avatar tmkelly28 avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

boilermaker's Issues

New syntax needed with "history"

I get these two browser errors:

Warning: Please use require("history").createBrowserHistory instead of require("history/createBrowserHistory"). Support for the latter will be removed in the next major release.

warnAboutDeprecatedCJSRequire.js:17 Warning: Please use require("history").createMemoryHistory instead of require("history/createMemoryHistory"). Support for the latter will be removed in the next major release.

Add spec for seed file

I am seeing a lot of students breaking their app by making db/index export something other than db — e.g. an object containing db — and not updating the db.sync calls elsewhere. A good way of killing two birds with one stone would be to have a sanity test / sniff test for the seed file, which basically just confirms that the seed file runs without returning an exit code. Can be executed as a child process, with errors printed out.

Enable HMR

Hot Module Replacement would be pretty awesome to get working, although the overhead in terms of config complexity / noise for our students to sift through when using Boilermaker might not be worth it.

Use Volleyball instead of Morgan?

This is maybe self-serving, but I wrote volleyball to provide a more pleasant and accurate development experience for Express projects. While it isn't a drop in the bucket compared to Morgan, it does log requests and responses as the temporally distinct events that they are.

Thoughts?

Unable to deploy Google OAuth on herokuapp.com

PROBLEM

Google OAuth requires authentication with an authorized domain list to run outside of localhost. herokuapp.com is not an authorized domain

Potential soltutions:

  • FSA runs internal OAuth server using docker and Ory Hydra
  • FSA grants ability to deploy on fullstackacademy.com

Restore missing error handling for user login

router.post('/login', async (req, res, next) => {
const user = await User.findOne({where: {email: req.body.email}})
if (!user) {
console.log('No such user found:', req.body.email)
res.status(401).send('Wrong username and/or password')
} else if (!user.correctPassword(req.body.password)) {
console.log('Incorrect password for user:', req.body.email)
res.status(401).send('Wrong username and/or password')
} else {
req.login(user, err => (err ? next(err) : res.json(user)))
}
})

During #20 we accidentally forgot to include error handling for failed login attempts. We should put that back.

bitmoji

Travis CI testing fails because of missing 'secrets.js' require in server/index.js

When travis tries to test and deploy from the tracked repository, testing fails because we can't require secrets.js in server/index.js.

Workaround to check if not in testing environment in server/index.js:

/*
    config keys for development
*/
if (process.env.NODE_ENV !== 'production' && process.env.NODE_ENV !== 'test')
    require('../secrets')

Make the travis.yml read the db name from the package.json?

In theory we could add a script which outputs the db name, then use that from within travis.yml in the before_script field.

  • Pros
    • students wouldn't have to remember to update the project name in as many places
  • Cons
    • adding more cleverness to what is supposed to be a student code scaffold
    • removing an opportunity for them to interact with and understand travis.yml

I'm leaning towards not doing this, but figured it wouldn't hurt to make it an open discussion.

Upgrade Webpack

As of this issue, Boilermaker's webpack is v2, but current is v4.

Add License

  • "As a developer, I want to know the terms of any applicable licenses before using a particular boilerplate."

Change Engines field to use a bounded range

Heroku now disallows overly-permissive version ranges like "Node": ">7.0.0". We should change the range to either explicitly list an upper bound (e.g. greater than 7, less than 9) or use the SEMVER compatibility flag ^, e.g. ^8.0.0.

Don't nest `Switch` components

React-Router explicitly states that only Router and Route components should be children of Switch.

We have nested Switches in our current codebase, in order to make some routes exist only if the user is logged in. However, if a user tries to go to a non-recognized route, we currently render a fallback route until the user is asynchronously logged in (at which point one would hope the route becomes recognized).

Unfortunately, this fallback route does not trigger if placed below the nested Switch and the user is logged in, but the route does not match anything in the sub-switch. This is because Switch components are misinterpreted by the parent Switch as actually being an unconditional (pathless) route – so no routes below them will ever trigger.

A student had this issue. The solution we came up with was to duplicate the fallback route inside the nested switch, which wasn't very DRY. The student subsequently created an issue here. A user responded with the following alternative:

If it were me, I would probably write a custom <Switch> component that is better suited for this use case. It is really just a loop over props.children, so implementing your own isn't very difficult.

Lack of linting

Linting is currently not part of boilermaker (as far as I can tell). I think we should include something for linting—and we should include something in the workshop as well.

make Prettier opt-out

Boilermaker's Prettier support is currently opt-in following this PR discussion.

The venerable instructors who objected to making Prettier automatic / opt-out have moved on. While we all miss them dearly and salute their valiant contributions to the curriculum, this does make me want to re-open the issue and see how our current instructional team views this feature.

My take on this is that it is not only good for students collaborating to have PRs be exclusively about content rather than formatting, but it is also good for us as staff members. It requires no special effort if it is opt-out, but it requires periodic reminders to staff & students if it is opt-in.

So what say you? Should automatic Prettier formatting on commit be opt-in or opt-out?

Consider using an ignored `build` directory instead of mixing ignored/included `public` files

I sometimes see students use tools which dynamically generate assets meant to be served up (e.g. CSS files, svgs, etc.). Students use public as the build target but fail to gitignore those assets, meaning they end up in version control.

Arguably a better idea is to have separate public and build directories, the former being included and the latter being ignored, both served up statically. Webpack would build to build, and other tools could as well; conversely, students would be taught that everything in public is supposed to be tracked in version control.

Fix: restarting server on client change

When running npm run start-dev, saving a client-side file causes nodemon to restart the server, even though at first glance, the script is set up to only restart when server changes occur. This should be debugged and fixed.

Use package name or config file for db connections?

The database name is hard-coded in a couple different locations – the scripts in package.json, the db file, etc. This is nice and simple, but it does mean that students can miss a location if they want to change the database name from the default. It might be good to centralize this to a single place, e.g. a config file or just the package.json app name (à la Bones).

React or other unrelated errors can log out user unexpectedly

Because this line is used as an error handler during login, if an error occurs earlier up the chain, the user will be logged out. However, earlier in the chain includes a vanilla dispatch which triggers React rendering and other unrelated code. Therefore, a mistake in an unrelated portion of the app (e.g. trying to display user && user.name.error.oops can cause the user to be immediately logged out, unnecessarily (and confusingly).

The fix is fairly simple, and is a nice example of when you want an intermediary non-catch error handler.

Breaking Babel if rebuild npm packages

I deleted my package-lock.json file and the node_modules file and reran npm install in an effort to address numerous vulnerabilities that were brought up from running npm audit. After the install, when trying to run my tests, I ran into this problem:

On update to version 7.00-beta.46 Get Decorators Support Error

I solved it by removing the ^ wherever ^7.0.0-beta.40 appears in the package.json file and then ran npm install again:

    "@babel/core": "7.0.0-beta.40",
    "@babel/polyfill": "7.0.0-beta.40",
    "@babel/preset-env": "7.0.0-beta.40",
    "@babel/preset-react": "7.0.0-beta.40",
    "@babel/preset-stage-2": "7.0.0-beta.40",
    "@babel/register": "7.0.0-beta.40",

Perhaps the @babel version should be changed to not use the beta version which is susceptible to breaking changes from babel.

Babel stage-X plugins deprecated

Yee Brandon J of 1806-FS-CH posted in Slack:

Just a heads up for anyone using boilermaker. I just tried to pull some of the dependencies over to a new project and ran into an error regarding babel deprecating the use of preset-stage-X plugins:

As of v7.0.0-beta.55, we've removed Babel's Stage presets. Please consider reading our blog post on this decision at https://babeljs.io/blog/2018/07/27/removing-babels-stage-presets for more details. TL;DR is that it's more beneficial in the long run to explicitly add which proposals to use.

For a more automatic migration, we have updated babel-upgrade, https://github.com/babel/babel-upgrade to do this for you with npx babel-upgrade.

I "fixed" the issue by removing the caret from the beginning of the babel libs to keep them at a lower version.

Provide example "gatekeeper" middleware?

Using Express middleware to block certain routes if not admin / logged in might be a nice example for students to work off of.

PR #16 addressed this issue but is currently out of date.

Tests fail on git clone

Boilermaker tests do not run until:

  1. Students manually create dbs with the right name
  2. Students manually make a secrets.js file
  3. Students manually add some Google OAuth keys

This is more than enough stuff in the way of even running tests. Will be hard to get students practicing TDD if they first have to fix the testing environment.

Prefer capability sniffing to environment check?

As per #160, it is technically possible to run code with the environment set to test even if the code isn't actually being run by Mocha. This causes a reference error for code which attempts to use after.

While that isn't the way the codebase is intended to be used, one possible way of making the code more robust is to use capability sniffing:

-if (process.env.NODE_ENV === 'test') {
+if (typeof global.after === 'function') {
   after(...)
}

I'm not 100% sure I prefer this – it is safer, but perhaps less clear.

npm-merge-driver breaks heroku deployment

Steps to reproduce:

git clone [email protected]:FullstackAcademy/boilermaker.git break-deploy
cd break-deploy
heroku create
npm run deploy

The deploy breaks when it runs npm-merge-driver install

remote:        > [email protected] prepare /tmp/build_6d7886aa5046b6f71d822b48cca31e2d
remote:        > npm-merge-driver install

Is there some way we can skip this on deploy?

Full error:

remote:        > [email protected] prepare /tmp/build_6d7886aa5046b6f71d822b48cca31e2d
remote:        > npm-merge-driver install
remote:        
remote:        fatal: Not a git repository (or any parent up to mount point /tmp)
remote:        Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
remote:        /tmp/build_6d7886aa5046b6f71d822b48cca31e2d/node_modules/npm-merge-driver/node_modules/yargs/yargs.js:1100
remote:        else throw err
remote:        ^
remote:        
remote:        Error: Command failed: git rev-parse --git-dir
remote:        fatal: Not a git repository (or any parent up to mount point /tmp)
remote:        Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
remote:        
remote:        at checkExecSyncError (child_process.js:603:11)
remote:        at Object.execSync (child_process.js:640:13)
remote:        at findAttributes (/tmp/build_6d7886aa5046b6f71d822b48cca31e2d/node_modules/npm-merge-driver/index.js:184:8)
remote:        at Object.install [as handler] (/tmp/build_6d7886aa5046b6f71d822b48cca31e2d/node_modules/npm-merge-driver/index.js:97:20)
remote:        at Object.runCommand (/tmp/build_6d7886aa5046b6f71d822b48cca31e2d/node_modules/npm-merge-driver/node_modules/yargs/lib/command.js:228:22)
remote:        at Object.parseArgs [as _parseArgs] (/tmp/build_6d7886aa5046b6f71d822b48cca31e2d/node_modules/npm-merge-driver/node_modules/yargs/yargs.js:1013:30)
remote:        at Object.get [as argv] (/tmp/build_6d7886aa5046b6f71d822b48cca31e2d/node_modules/npm-merge-driver/node_modules/yargs/yargs.js:957:21)
remote:        at parseArgs (/tmp/build_6d7886aa5046b6f71d822b48cca31e2d/node_modules/npm-merge-driver/index.js:93:21)
remote:        at Object.<anonymous> (/tmp/build_6d7886aa5046b6f71d822b48cca31e2d/node_modules/npm-merge-driver/index.js:11:3)
remote:        at Module._compile (internal/modules/cjs/loader.js:689:30)
remote:        npm ERR! code ELIFECYCLE
remote:        npm ERR! errno 1
remote:        npm ERR! [email protected] prepare: `npm-merge-driver install`
remote:        npm ERR! Exit status 1
remote:        npm ERR!
remote:        npm ERR! Failed at the [email protected] prepare script.
remote:        npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
remote:        
remote:        npm ERR! A complete log of this run can be found in:
remote:        npm ERR!     /tmp/npmcache.757YJ/_logs/2018-07-25T15_16_56_199Z-debug.log

dev mode webpack

Problem

Our current start script runs like this:

npm run build-client-watch & npm run start-server

Webpack (build-client-watch) runs as a background job (that's what the single ampersand does). This creates a couple problems:

1: The background job can turn into a zombie process causing confusion while debugging
2: It is possible to reload a browser tab before the webpack process finishes. express will happily serve the stale bundle.js

Proposal

Use webpack-dev-middleware. Webpack ships a middleware that can be added with app.use.

Webpack dev middleware bundles the javascirpt just-in-time when it is requested. It's impossible(probably) to get a stale bundle. And there will be only one process for boilermaker projects.

Problems created

dev/deployed configuration

This will create a little more disconnect between the dev and deployed environments. It makes sense to build a bundle.js on disk for deployed environments, so we'll need to write our webpack configuration such that it can be used both by the middleware and by webpack command line. (this is not difficult)

inconsistent with other projects

We follow this pattern in some other projects, it would be best to find them and update them to follow this pattern as well. (senior enrichment, some later jr phase projects)

Error upon install

I've been struggling with my own boilermaker, so I cloned, npm installed and npm started this boilermaker.

This is the response I got:

➜ boilermaker git:(master) npm start

[email protected] start /Users/yateswalker/github/boilermakerFromFullstack/boilermaker
node server

sequelize deprecated String based operators are now deprecated. Please use Symbol based operators for better security, read more at http://docs.sequelizejs.com/manual/tutorial/querying.html#operators node_modules/sequelize/lib/sequelize.js:236:13
Google client ID / secret not found. Skipping Google OAuth.
Mixing it up on port 8080
GET / 200 10.743 ms - 320
{ Error: Not found
at app.use.use (/Users/yateswalker/github/boilermakerFromFullstack/boilermaker/server/index.js:64:19)
at Layer.handle [as handle_request] (/Users/yateswalker/github/boilermakerFromFullstack/boilermaker/node_modules/express/lib/router/layer.js:95:5)
at trim_prefix (/Users/yateswalker/github/boilermakerFromFullstack/boilermaker/node_modules/express/lib/router/index.js:317:13)
at /Users/yateswalker/github/boilermakerFromFullstack/boilermaker/node_modules/express/lib/router/index.js:284:7
at Function.process_params (/Users/yateswalker/github/boilermakerFromFullstack/boilermaker/node_modules/express/lib/router/index.js:335:12)
at next (/Users/yateswalker/github/boilermakerFromFullstack/boilermaker/node_modules/express/lib/router/index.js:275:10)
at SendStream.error (/Users/yateswalker/github/boilermakerFromFullstack/boilermaker/node_modules/serve-static/index.js:121:7)
at emitOne (events.js:96:13)
at SendStream.emit (events.js:188:7)
at SendStream.error (/Users/yateswalker/github/boilermakerFromFullstack/boilermaker/node_modules/send/index.js:270:17)
at SendStream.onStatError (/Users/yateswalker/github/boilermakerFromFullstack/boilermaker/node_modules/send/index.js:421:12)
at onstat (/Users/yateswalker/github/boilermakerFromFullstack/boilermaker/node_modules/send/index.js:727:26)
at FSReqWrap.oncomplete (fs.js:123:15) status: 404 }
Error: Not found
at app.use.use (/Users/yateswalker/github/boilermakerFromFullstack/boilermaker/server/index.js:64:19)
at Layer.handle [as handle_request] (/Users/yateswalker/github/boilermakerFromFullstack/boilermaker/node_modules/express/lib/router/layer.js:95:5)
at trim_prefix (/Users/yateswalker/github/boilermakerFromFullstack/boilermaker/node_modules/express/lib/router/index.js:317:13)
at /Users/yateswalker/github/boilermakerFromFullstack/boilermaker/node_modules/express/lib/router/index.js:284:7
at Function.process_params (/Users/yateswalker/github/boilermakerFromFullstack/boilermaker/node_modules/express/lib/router/index.js:335:12)
at next (/Users/yateswalker/github/boilermakerFromFullstack/boilermaker/node_modules/express/lib/router/index.js:275:10)
at SendStream.error (/Users/yateswalker/github/boilermakerFromFullstack/boilermaker/node_modules/serve-static/index.js:121:7)
at emitOne (events.js:96:13)
at SendStream.emit (events.js:188:7)
at SendStream.error (/Users/yateswalker/github/boilermakerFromFullstack/boilermaker/node_modules/send/index.js:270:17)
at SendStream.onStatError (/Users/yateswalker/github/boilermakerFromFullstack/boilermaker/node_modules/send/index.js:421:12)
at onstat (/Users/yateswalker/github/boilermakerFromFullstack/boilermaker/node_modules/send/index.js:727:26)
at FSReqWrap.oncomplete (fs.js:123:15)
GET /bundle.js 404 8.457 ms - 9
GET /style.css 200 12.671 ms - 202

Strip password and salt from user object

We ought to demo how to sanitize Sequelize objects, e.g. stripping the password and salt fields from user objects. Could be an instance method (easy to implement but dangerously easy to forget), some sort of toJSON override (automatic, but I've run into weird edge case bugs with this approach), or some other solution.

Run the build as part of travis testing?

Build errors are not currently prevented by Travis – if the code can install and test, that is considered passing. By creating the build as part of testing (not just deployment) we can prevent a lot of errors with bad module paths, missing modules, etc.

switch to `findOrCreate`

User.find({where: {googleId}})
.then(foundUser => (foundUser
? done(null, foundUser)
: User.create({name, email, googleId})
.then(createdUser => done(null, createdUser))
))
.catch(done)

This could probably be a findOrCreate with search query & default params (in case of create). From Fellows who taught 1804!

bitmoji

Change `get ()` password/salt obfuscation to use `defaultScope`

password: {
type: Sequelize.STRING,
// Making `.password` act like a func hides it when serializing to JSON.
// This is a hack to get around Sequelize's lack of a "private" option.
get() {
return () => this.getDataValue('password')
}

The password and salt fields define getters. Those getters return functions instead of values. This was done to demonstrate a (hacky) way to prevent certain fields from being leaked to the front-end. This works because res.json-ing an instance JSON stringifies the object, and functions are skipped when serializing to JSON.

A much more semantic solution is to define a defaultScope which uses either an attributes whitelist, or alternatively a blacklist (attributes: { exclude: ['password', 'salt'] }. The downside to this solution is that defaultScope is disabled when opting into custom scopes; the solution is to manually compose the custom scope with defaultScope. This requires the developer to know about the issue and remember to do the composition.

The more semantic solution is thus easier to omit by accident. A mitigating measure would be to add a comment explicitly warning students to always include defaultScope when opting into using custom scopes on a query.

Even though the get trick works in more circumstances, it is still ultimately a trick, and difficult to predict all the possible edge cases. It makes assumptions about what will be done with instances in order to send them to the front end. Conversely, the use of a defaultScope and attributes indicates precisely what is intended, and is therefore less confusing. It also stops the fields from being included closer to the source, that is, at the db query level – potentially preventing leakage of private fields in currently-unforeseen circumstances.


EDIT: ah, now I remember why I did it this way. You still need to fetch the password and salt fields in order to verify user credentials. So omitting by default won't work – unless the login route does a fetch for the user with an explicit scope that includes the password and salt. Given that, I am not sure if I want to bother, as this feels like it would add complexity.

Enable `async`-`await`

Students are getting used to being able to use async-await code. Even if Boilermaker doesn't feature any example async functions, it shouldn't break due to Babel transpilation. Issue #20 is stale but did some work towards this goal.

Move some test setup/teardown to a central location.

I've had a number of groups who end up with quite a bit more test set-up and tear down needs.

To these students I have recommended they add a central ./test-setup.jsand instruct mocha to load this file explicitly and first.

Things that move into this file are setting up globally used mock adapters, the enzyme adapter, and database setup/teardown tasks.

"NODE_ENV='test' mocha ./test-setup \"./server/**/*.spec.js\" \"./client/**/*.spec.js\" \"./script/**/*.spec.js\" --require @babel/polyfill --require @babel/register
// test-setup.js
// with these and other hooks at the top-level.
before(() => db.sync({ force: true });
afterEach(() => db.sync({ force: true });

Use `--org` in travis encrypt instructions

Opaque and difficult to debug issue with only one student during deployment. They had to run the travis encrypt line using --org flag for unclear reasons. We could probably just add that to proactively avoid similar problems in the future.

This would just be a docs fix (bottom of the README).

Do not distinguish between user not found vs. wrong password

These lines:

router.post('/login', (req, res, next) => {
  User.findOne({where: {email: req.body.email}})
    .then(user => {
      if (!user) {
        res.status(401).send('User not found')
      } else if (!user.correctPassword(req.body.password)) {
        res.status(401).send('Incorrect password')
      } else {
        req.login(user, err => (err ? next(err) : res.json(user)))
      }
    })
    .catch(next)
})

…are not good practice. An attacker can learn:

  • that someone with a given email has an account at this site (privacy breach), and
  • that from now on they can try passwords knowing the email is correct (security vulnerability).

We should just send back a 401 in both cases, with "username and/or password incorrect" message. If we like, we can include a server log that distinguishes between them, for dev purposes. I would not bother though.

Auth is not RESTful

I think it'd be cool if we structured our auth to be more RESTful by having a /me route: POST means signup, PUT means login, DELETE means logout, GET means retrieve data of current user.

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.