Refactoring for multiple logins and institutions: Part 2 (#288)

* Make user-facing UID name configurable

* Update schema and create migration for rename to uid

* Transition to UID instead of NetID

* Fix all tests

* Add autocomplete to question email

* Fix broken tests

* Fix type error

* Allow creating new users in new question panel

* Capitalize uid name on profile page

* Filter existing staff from staff autocompletion

* Fix linter errors

* Fix ts-node options

* Append @illinois.edu to the end of all uids

* Upgrade MySQL APT Config

* Specify MySQL collation

* Use utf8mb4

* Specify charset in node

* Switch back to utf8mb4

* Switch to utf8

* Add unique constraint

* Remove unique constraint before renaming column

* Remove constraint under MySQL

* Update CHANGELOG.md

* Source environment variables using dotenv

* Specify dialect when creating migration/verification databases

* Fix uid in tests

* Rename columns

* Move env variables outside of build environment

* Add documentation for .env

* Add broader Shibboleth compatibility for the eppn attribute

* Expose eppn suffix check as an environment variable

* Add EPPN_SUFFIX to now for consistency

* Get rid of @ in EPPN_SUFFIX for now

Now interprets environment variables prefixed
with an "@" to be a secret

Co-authored-by: Nathan Walters <[email protected]>
Co-authored-by: Wade Fagen-Ulmschneider <[email protected]>

CHANGELOG.md

@@ -6,6 +6,7 @@ When a new version is tagged, the changes since the last deploy should be labele
 with the current semantic version and the next changes should go under a **[Next]** header.
 
 ## [Next]
+* Refactor authentication to support multiple logins and institutions. ([@nwalters512](https://github.com/nwalters512) in [#283](https://github.com/illinois/queue/pull/283))
 
 * Update API doc ([@jackieo5023](https://github.com/jackieo5023) in [#303](https://github.com/illinois/queue/pull/303))
 

README.md

@@ -40,7 +40,7 @@ If you wish to report a bug, feature request, etc., please open a new issue (fir
 
 Run `npm run build` to build assets for production; read more about the build process [in the docs](docs/Build.md). Run `npm run start` to start the application.
 
-Several configuration options are exposed via environment variables:
+Several configuration options are exposed via environment variables and `.env`:
 
 - `PORT`: controls which port the app will be served from.
 - `BASE_URL`: allows the app to be served from somewhere other than the server
@@ -51,3 +51,28 @@ Several configuration options are exposed via environment variables:
   `/my/path/`, then you should run with `BASE_URL=/my/path` (note the lack of
   trailing slash), and a request for queue 1 should be received as `/my/path/queue/1`.
 - `JWT_SECRET`: a secret key used to sign JSON Web Tokens for our users.
+- `UID_NAME`: official name for user ids (default: `email`)
+- `UID_ARTICLE`: article used in reference to `UID_NAME` (default: `an`)
+- `EPPN_SUFFIX`: the expected suffix for all valid Shibboleth eppn attributes. If this variable is not present, then all Shibboleth responses will be accepted.
+
+Database information is also stored through environment variables, and can be configured for multiple environments depending on the value of `NODE_ENV`.
+```
+DB_USERNAME_<ENV>="username"
+DB_PASSWORD_<ENV>="password"
+DB_DATABASE_<ENV>="database_name"
+DB_HOST_<ENV>="localhost"
+DB_DIALECT_<ENV>="sqlite"
+DB_LOGGING_<ENV>="false"
+DB_STORAGE_<ENV>="./dev.sqlite"  # Sqlite only
+
+# Example for development:
+DB_USERNAME_DEVELOPMENT="username"
+DB_PASSWORD_DEVELOPMENT="password"
+DB_DATABASE_DEVELOPMENT="queue"
+DB_HOST_DEVELOPMENT="localhost"
+DB_DIALECT_DEVELOPMENT="sqlite"
+DB_LOGGING_DEVELOPMENT="false"
+DB_STORAGE_DEVELOPMENT="./dev.sqlite"  # Sqlite only
+```
+
+Note that for SQLite databases, only `DB_STORAGE_<ENV>` is required.

next.config.js

@@ -2,11 +2,17 @@ const withCSS = require('@zeit/next-css')
 const withSass = require('@zeit/next-sass')
 const withTypescript = require('@zeit/next-typescript')
 
+require('dotenv').config()
+
 module.exports = withTypescript(
   withSass(
     withCSS({
       useFileSystemPublicRoutes: false,
       assetPrefix: process.env.BASE_URL || '',
+      publicRuntimeConfig: {
+        uidName: process.env.UID_NAME || 'email',
+        uidArticle: process.env.UID_ARTICLE || 'an',
+      },
     })
   )
 )

nodemon.json

@@ -1,6 +1,6 @@
 {
   "watch": ["src/**/*"],
   "execMap": {
-    "ts": "ts-node --typeCheck --compilerOptions '{\"module\":\"commonjs\"}'"
+    "ts": "ts-node --type-check --compiler-options '{\"module\":\"commonjs\"}'"
   }
 }

now.json

@@ -5,5 +5,11 @@
       "max": 1
     }
   },
-  "public": true
+  "public": true,
+  "env": {
+    "DB_DIALECT_NOW": "sqlite",
+    "DB_STORAGE_NOW": "/tmp/now.sqlite",
+    "DB_LOGGING_NOW": "false",
+    "EPPN_SUFFIX": "illinois.edu"
+  }
 }

package.json

@@ -10,7 +10,7 @@
     "start": "NODE_ENV=production node build/server.js",
     "now-start": "NODE_ENV=now node build/server.js",
     "test": "NODE_ENV=test jest",
-    "test-sequential": "jest --runInBand",
+    "test-sequential": "NODE_ENV=test jest --runInBand",
     "lint": "eslint --ext .js,.jsx,.ts,.tsx ./src",
     "fix-lint": "npm run lint -- --fix",
     "typecheck": "npx tsc --noEmit",
@@ -142,6 +142,9 @@
     "transform": {
       "^.+\\.tsx?$": "ts-jest",
       "^.+\\.jsx?$": "babel-jest"
+    },
+    "moduleNameMapper": {
+      "\\.(css|less|sass|scss)$": "<rootDir>/src/test/styleMock.js"
     }
   },
   "husky": {

src/actions/course.js

@@ -127,8 +127,7 @@ export function fetchCourse(courseId) {
 const addCourseStaffRequest = makeActionCreator(
   types.ADD_COURSE_STAFF.REQUEST,
   'courseId',
-  'netid',
-  'name'
+  'userId'
 )
 const addCourseStaffSuccess = makeActionCreator(
   types.ADD_COURSE_STAFF.SUCCESS,
@@ -140,22 +139,17 @@ const addCourseStaffFailure = makeActionCreator(
   'data'
 )
 
-export function addCourseStaff(courseId, netid, name) {
+export function addCourseStaff(courseId, userId) {
   return dispatch => {
-    dispatch(addCourseStaffRequest(courseId, netid, name))
-
-    return axios
-      .post(`/api/courses/${courseId}/staff`, {
-        netid,
-        name,
-      })
-      .then(
-        res => dispatch(addCourseStaffSuccess(courseId, res.data)),
-        err => {
-          console.error(err)
-          dispatch(addCourseStaffFailure(err))
-        }
-      )
+    dispatch(addCourseStaffRequest(courseId, userId))
+
+    return axios.put(`/api/courses/${courseId}/staff/${userId}`).then(
+      res => dispatch(addCourseStaffSuccess(courseId, res.data)),
+      err => {
+        console.error(err)
+        dispatch(addCourseStaffFailure(err))
+      }
+    )
   }
 }
 

src/api/autocomplete.js

@@ -18,7 +18,7 @@ router.get(
     const users = await User.findAll({
       where: {
         [Sequelize.Op.or]: {
-          netid: {
+          uid: {
             [Sequelize.Op.like]: `${q}%`,
           },
         },

src/api/autocomplete.test.js

@@ -11,35 +11,35 @@ afterEach(() => testutil.destroyTestDb())
 
 describe('Autocomplete API', () => {
   it('returns all users for empty query', async () => {
-    const request = await requestAsUser(app, 'admin')
+    const request = await requestAsUser(app, 'admin@illinois.edu')
     const res = await request.get('/api/autocomplete/users?q=')
     expect(res.statusCode).toBe(200)
     expect(res.body).toHaveLength(7)
     expect(res.body[0]).toMatchObject({
-      netid: 'dev',
+      uid: 'dev@illinois.edu',
       isAdmin: true,
     })
   })
 
-  it('returns specific user for partial netid', async () => {
-    const request = await requestAsUser(app, 'admin')
+  it('returns specific user for partial uid', async () => {
+    const request = await requestAsUser(app, 'admin@illinois.edu')
     const res = await request.get('/api/autocomplete/users?q=225')
     expect(res.statusCode).toBe(200)
     expect(res.body).toHaveLength(1)
     expect(res.body[0]).toMatchObject({
-      netid: '225staff',
+      uid: '225staff@illinois.edu',
       name: '225 Staff',
     })
   })
 
   it('disallows requests from course staff', async () => {
-    const request = await requestAsUser(app, '225staff')
+    const request = await requestAsUser(app, '225staff@illinois.edu')
     const res = await request.get('/api/autocomplete/users?q=225')
     expect(res.statusCode).toBe(403)
   })
 
   it('disallows requests from student', async () => {
-    const request = await requestAsUser(app, 'student')
+    const request = await requestAsUser(app, 'student@illinois.edu')
     const res = await request.get('/api/autocomplete/users?q=225')
     expect(res.statusCode).toBe(403)
   })

src/api/courses.js

@@ -23,9 +23,9 @@ const getCsv = questions => {
     'comments',
     'preparedness',
     'UserLocation',
-    'answeredBy.AnsweredBy_netid',
+    'answeredBy.AnsweredBy_uid',
     'answeredBy.AnsweredBy_UniversityName',
-    'askedBy.AskedBy_netid',
+    'askedBy.AskedBy_uid',
     'askedBy.AskedBy_UniversityName',
     'queue.queueId',
     'queue.courseId',
@@ -113,7 +113,7 @@ router.get(
       includes.push({
         model: User,
         as: 'staff',
-        attributes: ['id', 'netid', 'name'],
+        attributes: ['id', 'uid', 'name'],
         through: {
           attributes: [],
         },
@@ -179,7 +179,7 @@ router.get(
           model: User,
           as: 'askedBy',
           attributes: [
-            ['netid', 'AskedBy_netid'],
+            ['uid', 'AskedBy_uid'],
             ['universityName', 'AskedBy_UniversityName'],
           ],
           required: true,
@@ -189,7 +189,7 @@ router.get(
           model: User,
           as: 'answeredBy',
           attributes: [
-            ['netid', 'AnsweredBy_netid'],
+            ['uid', 'AnsweredBy_uid'],
             ['universityName', 'AnsweredBy_UniversityName'],
           ],
           required: false,
@@ -302,20 +302,11 @@ router.patch(
 )
 
 // Add someone to course staff
-router.post(
-  '/:courseId/staff',
-  [
-    requireCourseStaff,
-    requireCourse,
-    check('netid', 'netid must be specified')
-      .exists()
-      .trim(),
-    failIfErrors,
-  ],
+router.put(
+  '/:courseId/staff/:userId',
+  [requireCourseStaff, requireCourse, requireUser, failIfErrors],
   safeAsync(async (req, res, _next) => {
-    const { netid: originalNetid } = matchedData(req)
-    const [netid] = originalNetid.split('@')
-    const [user] = await User.findOrCreate({ where: { netid } })
+    const { user } = res.locals
     await user.addStaffAssignment(res.locals.course.id)
     res.status(201).send(user)
   })
@@ -327,10 +318,7 @@ router.delete(
   [requireCourseStaff, requireCourse, requireUser, failIfErrors],
   safeAsync(async (req, res, _next) => {
     const { user, course } = res.locals
-    const oldStaffAssignments = await user.getStaffAssignments()
-    await user.setStaffAssignments(
-      oldStaffAssignments.filter(c => c.id !== course.id)
-    )
+    await user.removeStaffAssignment(course)
     res.status(202).send()
   })
 )