Skip to content

Commit

Permalink
Support Single Sign-on with OpenID Connect (#910)
Browse files Browse the repository at this point in the history
  • Loading branch information
alxndrsn authored Sep 12, 2023
1 parent ee9211c commit dff2a52
Show file tree
Hide file tree
Showing 61 changed files with 5,426 additions and 640 deletions.
21 changes: 21 additions & 0 deletions .dockerignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# Ignore everything
*

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

!/oidc-dev/certs/*.pem
!/oidc-dev/fake-oidc-server/accounts.json
!/oidc-dev/fake-oidc-server/index.js
!/oidc-dev/fake-oidc-server/package.json
!/oidc-dev/fake-oidc-server/package-lock.json
!/oidc-dev/playwright-tests/package.json
!/oidc-dev/playwright-tests/package-lock.json
!/oidc-dev/playwright-tests/playwright.config.js
!/oidc-dev/playwright-tests/src/**/*.js
!/oidc-dev/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.17.0
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-dev/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/enterprise-server@3.5/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.17.0
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: make test-oidc-integration
- name: Fake OIDC Server Logs
if: always()
run: "! [[ -f ./fake-oidc-server.log ]] || cat ./fake-oidc-server.log"
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/enterprise-server@3.5/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.17.0
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
27 changes: 27 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,33 @@ node_modules: package.json
npm install --legacy-peer-deps
touch node_modules

.PHONY: test-oidc-integration
test-oidc-integration: node_modules
TEST_AUTH=oidc NODE_CONFIG_ENV=oidc-integration-test make test-integration

.PHONY: test-oidc-e2e
test-oidc-e2e: node_modules
cd oidc-dev && \
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-dev/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-dev/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
11 changes: 11 additions & 0 deletions config/oidc-development.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"default": {
"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
}
}
}
11 changes: 11 additions & 0 deletions config/oidc-example-auth0.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"default": {
"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
}
}
}
11 changes: 11 additions & 0 deletions config/oidc-example-broken.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"default": {
"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
}
}
}
11 changes: 11 additions & 0 deletions config/oidc-example-google.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"default": {
"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
}
}
}
11 changes: 11 additions & 0 deletions config/oidc-integration-test.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"default": {
"oidc": {
"enabled": true,
"issuerUrl": "http://localhost:9898",
"clientId": "odk-central-backend-dev",
"clientSecret": "super-top-secret"
}
}
}

19 changes: 19 additions & 0 deletions config/oidc-tester-docker.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"default": {
"database": {
"host": "odk-central-oidc-tester-postgres",
"database": "oidc-tester",
"user": "odk-central-backend",
"password": "supertopsecret3000"
},
"env": {
"domain": "https://odk-central.example.org:8989"
},
"oidc": {
"issuerUrl": "https://fake-oidc-server.example.net:9898",
"clientId": "odk-central-backend-dev",
"clientSecret": "super-top-secret",
"enabled": true
}
}
}
35 changes: 23 additions & 12 deletions lib/bin/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,7 @@

const { run } = require('../task/task');
const { createUser, promoteUser, setUserPassword } = require('../task/account');

// 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));
};
const oidc = require('../util/oidc');

const { Command } = require('commander');
const program = new Command('node lib/bin/cli.js');
Expand All @@ -30,13 +24,30 @@ const email = () => program.opts().email;

program.requiredOption('-u, --email <email-address>');

program.command('user-create')
.action(() => withPassword((password) => run(createUser(email(), password))));
if (oidc.isEnabled()) {
program.command('user-create')
.action(() => run(createUser(email(), null)));

program.command('user-set-password')
.action(() => {
throw new Error(`You cannot set a user's password when OpenID Connect (OIDC) is enabled.`); // eslint-disable-line quotes
});
} else {
// 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));
};

program.command('user-create')
.action(() => (withPassword((password) => run(createUser(email(), password)))));

program.command('user-set-password')
.action(() => withPassword((password) => run(setUserPassword(email(), password))));
}

program.command('user-promote')
.action(() => run(promoteUser(email())));

program.command('user-set-password')
.action(() => withPassword((password) => run(setUserPassword(email(), password))));

program.parse();
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 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. 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 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
22 changes: 22 additions & 0 deletions lib/http/endpoint.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ const { reduce } = require('ramda');
const { openRosaError } = require('../formats/openrosa');
const { odataXmlError } = require('../formats/odata');
const { noop, isPresent } = require('../util/util');
const { frontendPage, html } = require('../util/html');
const { serialize, redirect } = require('../util/http');
const { resolve, reject } = require('../util/promise');
const { PartialPipe } = require('../util/stream');
Expand Down Expand Up @@ -89,6 +90,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 +237,25 @@ const defaultEndpoint = endpointBase({
errorWriter: defaultErrorWriter
});

// Render html content in the style of frontend
const htmlEndpoint = endpointBase({
resultWriter: (result, request, response) => {
response.type('text/html');
response.send(frontendPage(result));
},
errorWriter: (error, request, response) => {
response.type('text/html');
response.status(500);
response.send(frontendPage({
body: html`
<h1>Error!</h1>
<div id="error-message"><pre>An unknown error occurred on the server.</pre></div>
<div><a href="/">Go home</a></div>
`,
}));
},
});


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

const builder = (container, preprocessors) => {
const result = defaultEndpoint(container, preprocessors);
result.html = htmlEndpoint(container, preprocessors);
result.openRosa = openRosaEndpoint(container, preprocessors);
result.odata = {
json: odataJsonEndpoint(container, preprocessors),
Expand Down
9 changes: 6 additions & 3 deletions lib/http/preprocessors.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,11 @@

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

const { SESSION_COOKIE } = require('../util/sessions');

// 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 +69,8 @@ const authHandler = ({ Sessions, Users, Auth, bcrypt }, context) => {

// Basic Auth, which is allowed over HTTPS only:
} else if (isPresent(authHeader) && authHeader.startsWith('Basic ')) {
if (oidc.isEnabled()) return reject(Problem.user.basicAuthNotSupportedWhenOidcEnabled());

// 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 +107,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 dff2a52

Please sign in to comment.