Skip to content

Commit

Permalink
Support Single Sign-on with OpenID Connect
Browse files Browse the repository at this point in the history
  • Loading branch information
alxndrsn committed Aug 25, 2023
1 parent b9d5e4c commit cc4b222
Show file tree
Hide file tree
Showing 67 changed files with 5,789 additions and 644 deletions.
2 changes: 1 addition & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ version: 2.1
jobs:
build:
docker:
- image: cimg/node:16.19.1
- image: cimg/node:18.17.0
- image: cimg/postgres:14.6
environment:
POSTGRES_PASSWORD: odktest
Expand Down
22 changes: 22 additions & 0 deletions .dockerignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Ignore everything
*

# Explicitly whitelist _necessary_ **source files**
!/package.json
!/package-lock.json
!/Makefile
!/lib/
!/config/
!/test/

!/oidc-tester/odk-central-backend-config.json
!/oidc-tester/certs/*.pem
!/oidc-tester/fake-oidc-server/accounts.json
!/oidc-tester/fake-oidc-server/index.js
!/oidc-tester/fake-oidc-server/package.json
!/oidc-tester/fake-oidc-server/package-lock.json
!/oidc-tester/playwright-tests/package.json
!/oidc-tester/playwright-tests/package-lock.json
!/oidc-tester/playwright-tests/playwright.config.js
!/oidc-tester/playwright-tests/src/**/*.js
!/oidc-tester/scripts/*.sh
1 change: 1 addition & 0 deletions .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
"array-bracket-spacing": "off",
"arrow-parens": "off",
"class-methods-use-this": "off",
"camelcase": [ "error", { "ignoreDestructuring": true, "properties": "never" } ],
"comma-dangle": "off",
"consistent-return": "off",
"curly": "off",
Expand Down
22 changes: 22 additions & 0 deletions .github/workflows/oidc-e2e.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
name: OIDC e2e tests

on: push

jobs:
oidc-e2e-test:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- name: Use Node.js 18
uses: actions/setup-node@v3
with:
node-version: 18
cache: 'npm'
- run: sudo apt-get install -y curl
- run: make test-oidc-e2e
- name: Archive playwright screenshots
if: failure()
uses: actions/upload-artifact@v3
with:
name: Playwright Screenshots
path: oidc-tester/playwright-results/**/*.png
36 changes: 36 additions & 0 deletions .github/workflows/oidc-integration.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
name: OIDC integration tests

on: push

jobs:
oidc-integration-test:
# TODO should we use the same container as circle & central?
runs-on: ubuntu-latest
services:
# see: https://docs.github.com/en/[email protected]/actions/using-containerized-services/creating-postgresql-service-containers
postgres:
image: postgres:14.6
env:
POSTGRES_PASSWORD: odktest
ports:
- 5432:5432
# Set health checks to wait until postgres has started
options: >-
--health-cmd pg_isready
--health-interval 10s
--health-timeout 5s
--health-retries 5
steps:
- uses: actions/checkout@v3
- name: Use Node.js 18
uses: actions/setup-node@v3
with:
node-version: 18
cache: 'npm'
- run: npm ci --legacy-peer-deps
- run: make fake-oidc-server-ci > fake-oidc-server.log &
- run: node lib/bin/create-docker-databases.js
- run: TEST_AUTH=oidc NODE_CONFIG_ENV=oidc-integration-test make test-integration
- name: Fake OIDC Server Logs
if: always()
run: "! [[ -f ./fake-oidc-server.log ]] || cat ./fake-oidc-server.log"
4 changes: 2 additions & 2 deletions .github/workflows/soak-test.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
name: Node.js CI
name: Soak Test

on: push

Expand All @@ -8,7 +8,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
node-version: [16.19.1]
node-version: [18]
services:
# see: https://docs.github.com/en/[email protected]/actions/using-containerized-services/creating-postgresql-service-containers
postgres:
Expand Down
32 changes: 32 additions & 0 deletions .github/workflows/standard-suite.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
name: Full Standard Test Suite

on: push

jobs:
standard-tests:
# TODO should we use the same container as circle & central?
runs-on: ubuntu-latest
services:
# see: https://docs.github.com/en/[email protected]/actions/using-containerized-services/creating-postgresql-service-containers
postgres:
image: postgres:14.6
env:
POSTGRES_PASSWORD: odktest
ports:
- 5432:5432
# Set health checks to wait until postgres has started
options: >-
--health-cmd pg_isready
--health-interval 10s
--health-timeout 5s
--health-retries 5
steps:
- uses: actions/checkout@v3
- name: Use Node.js 18
uses: actions/setup-node@v3
with:
node-version: 18
cache: 'npm'
- run: npm ci --legacy-peer-deps
- run: node lib/bin/create-docker-databases.js
- run: make test-full
6 changes: 6 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ If you're looking for help or discussion on _how_ ODK Central Backend works inte

Please see the [project README](https://github.com/getodk/central-backend#setting-up-a-development-environment) for instructions on how to set up your development environment.

### OpenID Connect

If you want to use OpenID Connect instead of username/password for authentication in development:

Instead of `make dev`, run both `make dev-oidc` and `make fake-oidc-server`.

## Guidelines

If you're starting work on an issue ticket, please leave a comment saying so. If you run into trouble or have to stop working on a ticket, please leave a comment as well. As you write code, the usual guidelines apply; please ensure you are following existing conventions:
Expand Down
25 changes: 24 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
@@ -1,9 +1,32 @@
default: base

node_modules: package.json
npm install --legacy-peer-deps
npm clean-install --legacy-peer-deps
touch node_modules

.PHONY: test-oidc-e2e
test-oidc-e2e: node_modules
cd oidc-tester && \
docker compose down && \
docker compose build && \
docker compose up --exit-code-from odk-central-oidc-tester

.PHONY: dev-oidc
dev-oidc: base
NODE_CONFIG_ENV=oidc-development npx nodemon --watch lib --watch config lib/bin/run-server.js

.PHONY: fake-oidc-server
fake-oidc-server:
cd oidc-tester/fake-oidc-server && \
npm clean-install && \
FAKE_OIDC_ROOT_URL=http://localhost:9898 npx nodemon index.js

.PHONY: fake-oidc-server-ci
fake-oidc-server-ci:
cd oidc-tester/fake-oidc-server && \
npm clean-install && \
FAKE_OIDC_ROOT_URL=http://localhost:9898 node index.js

.PHONY: node_version
node_version: node_modules
node lib/bin/enforce-node-version.js
Expand Down
14 changes: 14 additions & 0 deletions config/oidc-development.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"default": {
"env": {
"oidcProviderName": "OpenID Connect"
},
"oidc": {
"_description": "local test server: from https://www.npmjs.com/package/oidc-provider",
"issuerUrl": "http://localhost:9898",
"clientId": "odk-central-backend-dev",
"clientSecret": "super-top-secret",
"enabled": true
}
}
}
14 changes: 14 additions & 0 deletions config/oidc-example-auth0.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"default": {
"env": {
"oidcProviderName": "Auth0"
},
"oidc": {
"_description": "auth0: https://manage.auth0.com/dashboard/us/odk-oidc-dev/",
"issuerUrl": "https://odk-oidc-dev.us.auth0.com",
"clientId": "ZKKpcW8TpKymVLbD1dbDVExj7SU4Zxbn",
"clientSecret": "7tuVT7OsjRHfmUiwYYyWNT8YArMNlmvvv70tqlChkjtVHW0Xsp0mvVAyKIfCgUn5",
"enabled": true
}
}
}
14 changes: 14 additions & 0 deletions config/oidc-example-broken.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"default": {
"env": {
"oidcProviderName": "Broken provider"
},
"oidc": {
"_description": "broken config: fiddle with this config to test out different init failure modes",
"issuerUrl": "http://example.com",
"clientId": "this is required; should be reported during client init if this line commented out",
"clientSecret": "this is required; should be reported during client init if this line commented out",
"enabled": true
}
}
}
14 changes: 14 additions & 0 deletions config/oidc-example-google.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"default": {
"env": {
"oidcProviderName": "Google"
},
"oidc": {
"_description": "google: from https://console.cloud.google.com/apis/credentials",
"issuerUrl": "https://accounts.google.com",
"clientId": "564021877275-o5q3i8j44190d93d9mldd3rti1fncn3u.apps.googleusercontent.com",
"clientSecret": "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",
"enabled": true
}
}
}
14 changes: 14 additions & 0 deletions config/oidc-integration-test.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"default": {
"env": {
"oidcProviderName": "OpenID Connect"
},
"oidc": {
"enabled": true,
"issuerUrl": "http://localhost:9898",
"clientId": "odk-central-backend-dev",
"clientSecret": "super-top-secret"
}
}
}

20 changes: 15 additions & 5 deletions lib/bin/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,23 +16,33 @@
const { run } = require('../task/task');
const { createUser, promoteUser, setUserPassword } = require('../task/account');

const crypto = require('node:crypto');

// gets a password interactively if not supplied in cli args.
const prompt = require('prompt');
const withPassword = (f) => {
prompt.start();
prompt.get([{ name: 'password', hidden: true, replace: '*' }], (_, { password }) => f(password));
};

// command line nonsense (i'm not a huge fan of this library).
const cli = require('cli');
const cliArgs = {
email: [ 'u', 'For user create and set password commands, supplies the email.', 'email' ]
email: [ 'u', 'For user create and set password commands, supplies the email.', 'email' ],
'generate-password': [ 'g', 'For user create and set password commands, generates a random password and prints it to stdout.', 'boolean' ],
};
const cliCommands = [ 'user-create', 'user-promote', 'user-set-password' ];
cli.parse(cliArgs, cliCommands);

// map commands to tasks.
cli.main((args, options) => {
const withPassword = (f) => {
if (options['generate-password']) {
const password = crypto.randomBytes(32).toString('base64');
console.error('Generated password:', password);
return f(password);
}

prompt.start();
prompt.get([{ name: 'password', hidden: true, replace: '*' }], (_, { password }) => f(password));
};

if (cli.command === 'user-create')
withPassword((password) => run(createUser(options.email, password)));
else if (cli.command === 'user-promote')
Expand Down
3 changes: 3 additions & 0 deletions lib/formats/mail.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ const messages = {
// Notifies a user that an account has been created with a predetermined password.
accountCreatedWithPassword: message('ODK Central account created', '<html>Hello!<p>An account has been provisioned for you on an ODK Central data collection server.</p><p>If this message is unexpected, simply ignore it. Your account was created with an assigned password. Please use that password to <a href="{{{domain}}}">sign in</a>.</p><p>If you have not been given the password, or you cannot remember it, you can reset it at any time at this link:</p><p><a href="{{{domain}}}/#/reset-password">{{{domain}}}/#/reset-password</a></p></html>'),

// Notifies a user that an account has been created for login exclusively with OIDC.
accountCreatedForOidc: message('ODK Central account created', '<html>Hello!<p>An account has been provisioned for you on an ODK Central data collection server.</p><p>If this message is unexpected, simply ignore it. Your account was created for login with {{{oidcProviderName}}}. Please go to <a href="{{{domain}}}">{{{domain}}}</a> to sign in.</p></html>'),

// Notifies a user that their account's email has been changed
accountEmailChanged: message('ODK Central account email changed', '<html>Hello!<p><p>We are emailing because you have an ODK Central data collection account, and somebody has just changed the email address associated with the account from this one you are reading right now ({{oldEmail}}) to a new address ({{newEmail}}).</p><p>If this was you, please feel free to ignore this email. Otherwise, please contact your local ODK system administrator immediately.</p></html>'),

Expand Down
8 changes: 8 additions & 0 deletions lib/http/endpoint.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ const getRequestContext = (request) => ({
params: request.params,
query: request.query,
files: request.files,
cookies: request.cookies,

apiVersion: request.apiVersion,
fieldKey: request.fieldKey
Expand Down Expand Up @@ -235,6 +236,12 @@ const defaultEndpoint = endpointBase({
errorWriter: defaultErrorWriter
});

// TODO convert this to htmlEndpoint
const manualEndpoint = endpointBase({
resultWriter: () => {},
errorWriter: () => {},
});


////////////////////////////////////////
// OPENROSA
Expand Down Expand Up @@ -355,6 +362,7 @@ const odataXmlEndpoint = endpointBase({

const builder = (container, preprocessors) => {
const result = defaultEndpoint(container, preprocessors);
result.manual = manualEndpoint(container, preprocessors);
result.openRosa = openRosaEndpoint(container, preprocessors);
result.odata = {
json: odataJsonEndpoint(container, preprocessors),
Expand Down
12 changes: 10 additions & 2 deletions lib/http/preprocessors.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,18 @@
// including this file, may be copied, modified, propagated, or distributed
// except according to the terms contained in the LICENSE file.

const config = require('config');
const { isBlank, isPresent, noop, without } = require('../util/util');
const { isTrue } = require('../util/http');
const Problem = require('../util/problem');
const { QueryOptions } = require('../util/db');
const { reject, getOrReject } = require('../util/promise');

// TODO use req.protocol?
const HTTPS_ENABLED = config.get('default.env.domain').startsWith('https://');

// TODO add some thoughts about why __Host is important in prod but impossible in dev
const SESSION_COOKIE = HTTPS_ENABLED ? '__Host-session' : 'session';

// injects an empty/anonymous auth object into the request context.
const emptyAuthInjector = ({ Auth }, context) => context.with({ auth: Auth.by(null) });
Expand Down Expand Up @@ -68,6 +74,8 @@ const authHandler = ({ Sessions, Users, Auth, bcrypt }, context) => {

// Basic Auth, which is allowed over HTTPS only:
} else if (isPresent(authHeader) && authHeader.startsWith('Basic ')) {
// REVIEW: do web users still have legitimate reasons for accessing using BasicAuth?

// fail the request unless we are under HTTPS.
// this logic does mean that if we are not under nginx it is possible to fool the server.
// but it is the user's prerogative to undertake this bypass, so their security is in their hands.
Expand Down Expand Up @@ -104,13 +112,13 @@ const authHandler = ({ Sessions, Users, Auth, bcrypt }, context) => {
return;

// otherwise get the cookie contents.
const token = /session=([^;]+)(?:;|$)/.exec(context.headers.cookie);
const token = context.cookies[SESSION_COOKIE];
if (token == null)
return;

// actually try to authenticate with it. no Problem on failure. short circuit
// out if we have a GET or HEAD request.
const maybeSession = authBySessionToken(decodeURIComponent(token[1]));
const maybeSession = authBySessionToken(decodeURIComponent(token));
if ((context.method === 'GET') || (context.method === 'HEAD')) return maybeSession;

// if non-GET run authentication as usual but we'll have to check CSRF afterwards.
Expand Down
Loading

0 comments on commit cc4b222

Please sign in to comment.