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 28, 2023
1 parent 628ac0d commit 30e5bf1
Show file tree
Hide file tree
Showing 66 changed files with 5,738 additions and 626 deletions.
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.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-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.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: 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"
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.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
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"
}
}
}

5 changes: 4 additions & 1 deletion lib/bin/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@ const email = () => program.opts().email;
program.requiredOption('-u, --email <email-address>');

program.command('user-create')
.action(() => withPassword((password) => run(createUser(email(), password))));
.option('--null-password', 'Create a user without a password.')
.action(({ nullPassword }) => (nullPassword ?
run(createUser(email(), null)) :
withPassword((password) => run(createUser(email(), password)))));

program.command('user-promote')
.action(() => run(promoteUser(email())));
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 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 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
8 changes: 5 additions & 3 deletions lib/http/preprocessors.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const { isTrue } = require('../util/http');
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 +68,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 +106,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
14 changes: 14 additions & 0 deletions lib/http/service.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,16 @@
// defined elsewhere into an actual Express service. The only thing it needs in
// order to do this is a valid dependency injection context container.

// REVIEW: per CONTRIBUTING.md, perhaps this should be rewritten? Specifically:
//
// > We do something a little bit odd: we only use actual Express middleware
// > sparingly. Besides the usual assortment of stock/packaged parsers and
// > loggers, only middleware that rewrites the incoming request URL are
// > actually implemented as Express middleware; everything else we run as what
// > we call a preprocessor.
//
const cookieParser = require('cookie-parser');

module.exports = (container) => {
const service = require('express')();

Expand Down Expand Up @@ -41,6 +51,8 @@ module.exports = (container) => {
service.use(versionParser);
service.use(fieldKeyParser);

service.use(cookieParser());


////////////////////////////////////////////////////////////////////////////////
// PREPROCESSORS
Expand Down Expand Up @@ -74,6 +86,8 @@ module.exports = (container) => {
require('../resources/analytics')(service, endpoint);
require('../resources/datasets')(service, endpoint);
require('../resources/entities')(service, endpoint);
// REVIEW: should these definitely be at /v1/ URLs and using standard preprocessor?
require('../resources/oidc')(service, endpoint);

////////////////////////////////////////////////////////////////////////////////
// POSTRESOURCE HANDLERS
Expand Down
Loading

0 comments on commit 30e5bf1

Please sign in to comment.