diff --git a/config/static.ts b/config/static.ts index be15b9da..280c97e1 100644 --- a/config/static.ts +++ b/config/static.ts @@ -14,9 +14,6 @@ const client: Config = {}; const server: Config = { defaultServerConfig: { configValue: { - authentication: { - type: 'none', - }, client: { configOverrides: {}, transport: {}, @@ -35,6 +32,9 @@ const server: Config = { contextRoot: '/', port: 9080, transport: {}, + authentication: { + type: 'none', + }, }, session: { name: 'strimzi-ui', diff --git a/package.json b/package.json index 14d7bac4..2f629f9e 100644 --- a/package.json +++ b/package.json @@ -51,11 +51,13 @@ "apollo-link-http": "^1.5.17", "apollo-server-express": "^2.18.2", "axios": "^0.21.0", + "body-parser": "^1.19.0", "compression-webpack-plugin": "^4.0.0", "express": "^4.17.1", "express-session": "^1.17.1", "express-static-gzip": "^2.1.0", "graphql": "^15.4.0", + "graphql-tag": "^2.11.0", "graphql-ws": "^1.14.0", "helmet": "^4.2.0", "html-webpack-plugin": "^4.5.0", diff --git a/server/README.md b/server/README.md index 24837192..7ecc266b 100644 --- a/server/README.md +++ b/server/README.md @@ -20,22 +20,22 @@ This directory contains all server code for the Strimzi UI - ie code which is re As described in [the configuration approach](../docs/Architecture.md#configuration-and-feature-flagging), the UI server's configuration is provided via a file, which is then watched at runtime for modification. This configuration file is expected to be called `server.config.json` (available in the same directory as the `node` executable is run from), but this can be configured at runtime via environment variable `configPath`, dictating a different path and file name. The file must be either valid JSON or JS. The server also hosts configuration for discovery by the client via the `config` module. The configuration options for the server provided in the previously mentioned configuration file are as follows: -| Configuration | Required | Default | Purpose | -| ---------------------------- | -------- | ---------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| authentication.strategy | No | `none` | What authentication strategy to use to authenticate users. See [the security section](#security) for details of the available options. | -| authentication.configuration | No | `{}` | Any additional configuration required for the provided authentication strategy `authentication.strategy` . See [the security section](#security) for details of the available options. | -| client.configOverrides | No | `{}` | Overrides to send to the client. See [client configuration for further details](#client-configuration). These values will take precedence over any others provided. | -| client.publicDir | No | `/dist/client` | The location of the built client to serve. | -| client.transport.cert | No | N/A - if one of `client.transport.cert` or `client.transport.key` are not provided, server will be HTTP | PEM certificate presented to browsers on connecting to the UI server. | -| client.transport.key | No | N/A - if one of `client.transport.cert` or `client.transport.key` are not provided, server will be HTTP | PEM certificate private key for the certificate provided in `client.transport.cert`. | -| client.transport.ciphers | No | default set from [node's tls module](https://nodejs.org/api/tls.html#tls_modifying_the_default_tls_cipher_suite) | TLS ciphers used/supported by the HTTPS server for client negotiation. Only applies if starting an HTTPS server. | -| client.transport.minTLS | No | `TLSv1.2` | Minimum TLS version supported by the server. Only applies if starting an HTTPS server. Set to `TLSv1.2` for browser compatibility. | -| featureFlags | No | `{}` | Feature flag overrides to set. The configuration is as per the format specified [here](#feature-flags). These values will take precedence over any others provided. | -| hostname | No | '0.0.0.0' | The hostname the UI server will be bound to. | -| logging | No | TBD | Logging configuration settings. Format to be defined in https://github.com/strimzi/strimzi-ui/issues/24 | -| modules | No | Object - [enabled modules and configuration can be found here](../docs/Architecture.md#router-controller-data-pattern) | The modules which are either enabled or disabled. | -| port | No | 3000 | The port the UI server will be bound to. | -| proxy.transport.cert | No | If not provided, SSL certificate validation of the upstream admin server is disabled | CA certificate in PEM format of the backend admin server api requests are to be sent to. | -| proxy.hostname | Yes | N/A | The hostname of the admin server to send api requests to. | -| proxy.port | Yes | N/A | The port of the admin server to send api requests to. | -| session.name | no | `strimzi-ui` | The name used to identify the session cookie | +| Configuration | Required | Default | Purpose | +| ---------------------------------- | -------- | ---------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| client.configOverrides | No | `{}` | Overrides to send to the client. See [client configuration for further details](#client-configuration). These values will take precedence over any others provided. | +| client.publicDir | No | `/dist/client` | The location of the built client to serve. | +| client.transport.cert | No | N/A - if one of `client.transport.cert` or `client.transport.key` are not provided, server will be HTTP | PEM certificate presented to browsers on connecting to the UI server. | +| client.transport.key | No | N/A - if one of `client.transport.cert` or `client.transport.key` are not provided, server will be HTTP | PEM certificate private key for the certificate provided in `client.transport.cert`. | +| client.transport.ciphers | No | default set from [node's tls module](https://nodejs.org/api/tls.html#tls_modifying_the_default_tls_cipher_suite) | TLS ciphers used/supported by the HTTPS server for client negotiation. Only applies if starting an HTTPS server. | +| client.transport.minTLS | No | `TLSv1.2` | Minimum TLS version supported by the server. Only applies if starting an HTTPS server. Set to `TLSv1.2` for browser compatibility. | +| featureFlags | No | `{}` | Feature flag overrides to set. The configuration is as per the format specified [here](#feature-flags). These values will take precedence over any others provided. | +| hostname | No | '0.0.0.0' | The hostname the UI server will be bound to. | +| logging | No | TBD | Logging configuration settings. Format to be defined in https://github.com/strimzi/strimzi-ui/issues/24 | +| modules | No | Object - [enabled modules and configuration can be found here](../docs/Architecture.md#router-controller-data-pattern) | The modules which are either enabled or disabled. | +| port | No | 3000 | The port the UI server will be bound to. | +| proxy.transport.cert | No | If not provided, SSL certificate validation of the upstream admin server is disabled | CA certificate in PEM format of the backend admin server api requests are to be sent to. | +| proxy.hostname | Yes | N/A | The hostname of the admin server to send api requests to. | +| proxy.port | Yes | N/A | The port of the admin server to send api requests to. | +| proxy.authentication.type | No | `none` | What authentication strategy to use to authenticate users. See [the security section](#security) for details of the available options. | +| proxy.authentication.configuration | No | `{}` | Any additional configuration required for the provided authentication strategy `authentication.strategy` . See [the security section](#security) for details of the available options. | +| session.name | no | `strimzi-ui` | The name used to identify the session cookie | diff --git a/server/client/client.feature b/server/client/client.feature index 4b6ee3b1..dad36057 100644 --- a/server/client/client.feature +++ b/server/client/client.feature @@ -46,9 +46,9 @@ Feature: client module # if the route (not file) is not matched, we redirect to index.html. Hence / and someroute response Examples: | Asset | StatusCode | - | /index.html | 200 | + | /index.html | 302 | | /images/picture.svg | 200 | | /doesnotexist.html | 404 | | /someroute | 302 | | /protected.html | 302 | - | / | 200 | + | / | 302 | diff --git a/server/client/client.steps.ts b/server/client/client.steps.ts index d47ade1c..11e861b6 100644 --- a/server/client/client.steps.ts +++ b/server/client/client.steps.ts @@ -3,11 +3,8 @@ * License: Apache License 2.0 (see the file LICENSE or http://apache.org/licenses/LICENSE-2.0.html). */ import merge from 'lodash.merge'; -import { And, Then, Fusion } from 'jest-cucumber-fusion'; -import { - stepWithWorld, - stepWhichUpdatesWorld, -} from 'test_common/commonServerSteps'; +import { And, Fusion } from 'jest-cucumber-fusion'; +import { stepWhichUpdatesWorld } from 'test_common/commonServerSteps'; And( 'There are no files to serve', @@ -27,13 +24,4 @@ And('There are files to serve', () => { // NO_OP - the `client_only` configuration is already configured to serve fixture files }); -Then( - /I get the expected status code '(.+)' response/, - stepWithWorld((world, statusCode) => { - const expectedStatus = parseInt(statusCode as string); - const { request } = world; - return request.expect(expectedStatus); - }) -); - Fusion('client.feature'); diff --git a/server/client/controller.ts b/server/client/controller.ts index a11c5ca6..10d43e94 100644 --- a/server/client/controller.ts +++ b/server/client/controller.ts @@ -3,30 +3,29 @@ * License: Apache License 2.0 (see the file LICENSE or http://apache.org/licenses/LICENSE-2.0.html). */ -import { resolve, sep } from 'path'; -import { existsSync, readdirSync } from 'fs'; +import { resolve, sep } from "path"; +import { existsSync, readdirSync } from "fs"; // function to recursively get all files from a directory const getFilesInDirectory: (directory: string) => Array = (directory) => existsSync(directory) ? readdirSync(directory, { withFileTypes: true }).reduce((acc, fileObj) => { - return fileObj.isFile() - ? acc.concat([`${directory}${sep}${fileObj.name}`]) - : acc.concat( - getFilesInDirectory(`${directory}${sep}${fileObj.name}`) - ); - }, [] as string[]) + return fileObj.isFile() + ? acc.concat([`${directory}${sep}${fileObj.name}`]) + : acc.concat( + getFilesInDirectory(`${directory}${sep}${fileObj.name}`) + ); + }, [] as string[]) : []; // mark a subset of files as public - this means any user can access them. These entries will be used in a regex - if the test passes, it will be considered public const publicFiles = [ - 'images/*', - 'fonts/*', - 'favicon.ico', - 'index.html', - 'main.css', - 'main.bundle.js', - 'main.bundle.js.gz', + "images/*", + "fonts/*", + "favicon.ico", + "main.css", + "main.bundle.js", + "main.bundle.js.gz", ]; export const getFiles: ( @@ -57,7 +56,7 @@ export const getFiles: ( return { totalNumberOfFiles: allFilesInClientDirectory.length, - hasIndexFile: allFilesInClientDirectory.includes('/index.html'), + hasIndexFile: allFilesInClientDirectory.includes("/index.html"), protectedFiles: protectedFiles, builtClientDir, }; diff --git a/server/client/router.ts b/server/client/router.ts index 17cb9c1b..f9b9bd69 100644 --- a/server/client/router.ts +++ b/server/client/router.ts @@ -30,6 +30,7 @@ export const ClientModule: UIServerModule = { // add the auth middleware to all non public files protectedFiles.forEach((file) => routerForModule.get(`${file}`, checkAuth)); + routerForModule.get('/', checkAuth); // host all files from the client dir routerForModule.get( @@ -40,9 +41,10 @@ export const ClientModule: UIServerModule = { // if no match, not a file (path contains '.'), and we have an index.html file, redirect to it (ie return index so client navigation logic kicks in). Else do nothing (404 unless another module handles it) hasIndexFile && - routerForModule.get(/^((?!\.).)+$/, (req, res) => - res.redirect(`/index.html`) - ); + routerForModule.get(/^((?!\.).)+$/, (req, res) => { + logger.info('Redirecting to index'); + res.redirect(`/`); + }); return exit({ mountPoint: '/', routerForModule }); }, diff --git a/server/core/app.ts b/server/core/app.ts index 5b381d2e..6c0abc80 100644 --- a/server/core/app.ts +++ b/server/core/app.ts @@ -7,6 +7,7 @@ import helmet from 'helmet'; import * as availableModules from './modules'; import { serverConfig, UIServerModule } from 'types'; import expressSession, { SessionOptions } from 'express-session'; +import bodyParser from 'body-parser'; import { generateLogger, generateHttpLogger, @@ -20,7 +21,7 @@ export const returnExpress: ( const logger = generateLogger('core'); const app = express(); - const { session: sessionConfig, authentication: authConfig } = getConfig(); + const { session: sessionConfig, proxy: proxyConfig } = getConfig(); // add helmet middleware app.use(helmet()); @@ -37,8 +38,9 @@ export const returnExpress: ( }, }; app.use(expressSession(sessionOpts)); + app.use(bodyParser.json()); - const authentication = bootstrapAuthentication(app, authConfig); + const authentication = bootstrapAuthentication(app, proxyConfig); // for each module, call the function to add it to the routing table const routingTable = Object.values(availableModules).reduce( diff --git a/server/core/modules.ts b/server/core/modules.ts index bdf7b514..6d4dd63b 100644 --- a/server/core/modules.ts +++ b/server/core/modules.ts @@ -3,7 +3,9 @@ * License: Apache License 2.0 (see the file LICENSE or http://apache.org/licenses/LICENSE-2.0.html). */ export * from 'api/index'; -export * from 'client/index'; export * from 'config/index'; export * from 'log/index'; export * from 'mockapi/index'; +export { SecurityModule } from 'security/index'; +// WARNING - client must be exported last (it will register catch-all routes) +export * from 'client/index'; diff --git a/server/mockapi/data.ts b/server/mockapi/data.ts index 034afa0a..5c63d0df 100644 --- a/server/mockapi/data.ts +++ b/server/mockapi/data.ts @@ -2,7 +2,17 @@ * Copyright Strimzi authors. * License: Apache License 2.0 (see the file LICENSE or http://apache.org/licenses/LICENSE-2.0.html). */ - +import { gql } from 'apollo-server-express'; // placeholder GQL schema for a topic/topic list - ideally to come from file -export const schema = - 'type Topic {name: String partitions: Int replicas: Int } type Query { topic(name: String): Topic topics: [Topic] } '; +export const schema = gql` + type Topic { + name: String + partitions: Int + replicas: Int + } + type Query { + topic(name: String): Topic + topics: [Topic] + clusterInfo: String + } +`; diff --git a/server/security/Bootstrap.ts b/server/security/Bootstrap.ts index 3938f6e6..4f9ef01a 100644 --- a/server/security/Bootstrap.ts +++ b/server/security/Bootstrap.ts @@ -2,12 +2,14 @@ * Copyright Strimzi authors. * License: Apache License 2.0 (see the file LICENSE or http://apache.org/licenses/LICENSE-2.0.html). */ -import { getStrategy } from './strategy/StrategyFactory'; -import passport from 'passport'; -import { RequestHandler } from 'express'; -import { authenticationConfig, authenticationStrategies } from 'types'; -import { Application } from 'express'; -import { Authentication } from './types'; +import { getStrategy } from "./strategy/StrategyFactory"; +import passport from "passport"; +import { RequestHandler } from "express"; +import { proxyConfigType, authenticationStrategies } from "types"; +import { Application } from "express"; +import { Authentication } from "./types"; +import { apiRoot, scram } from "./routeConfig"; +import { join } from "path"; const noOp: RequestHandler = (_req, _res, next) => next(); const noAuth = { @@ -18,9 +20,11 @@ const noAuth = { export const bootstrapPassport = ( app: Application, - config: authenticationConfig + config: proxyConfigType ): Authentication => { - if (config.type === authenticationStrategies.NONE) { + const authenticationConfig = config.authentication; + + if (authenticationConfig.type === authenticationStrategies.NONE) { return noAuth; } @@ -30,22 +34,25 @@ export const bootstrapPassport = ( const authStrategy = getStrategy(config); passport.use(authStrategy.name, authStrategy.strategy); - const authenticate = passport.authenticate(authStrategy.name, config.options); - switch (config.type) { - case authenticationStrategies.SCRAM: { - return { - authenticate, - checkAuth: (req, res, next) => { - return req.isAuthenticated() ? next() : res.redirect('/login'); - }, - logout: (req, res) => { - req.logout(); - res.redirect('/logout'); - }, - }; - } - default: - return noAuth; + switch (authenticationConfig.type) { + case authenticationStrategies.SCRAM: { + passport.serializeUser((user, done) => done(null, user)); + passport.deserializeUser((user, done) => done(null, user)); + return { + authenticate: passport.authenticate(authStrategy.name), + checkAuth: (req, res, next) => { + return req.isAuthenticated() + ? next() + : res.redirect(join(apiRoot, scram.login)); + }, + logout: (req, res) => { + req.logout(); + res.redirect(join(apiRoot, scram.logout)); + }, + }; + } + default: + throw new Error(`Unsupported type "${authenticationConfig.type}"`); } }; diff --git a/server/security/bootstrap.feature b/server/security/bootstrap.feature new file mode 100644 index 00000000..087005f6 --- /dev/null +++ b/server/security/bootstrap.feature @@ -0,0 +1,34 @@ +# Copyright Strimzi authors. +# License: Apache License 2.0 (see the file LICENSE or http://apache.org/licenses/LICENSE-2.0.html). +Feature: Bootstrapping passport + + Scenario: Check auth function when not authenticated + Given an Application + When I bootstrap passport with authentication type 'scram' + Then check auth redirects to '/auth/login' + + Scenario: Check auth function when authenticated - Scram + Given an Application + When I bootstrap passport with authentication type 'scram' + And the request is authenticated + Then check auth returns '200' + + Scenario: Check auth function - No auth + Given an Application + When I bootstrap passport with authentication type 'none' + Then check auth returns '200' + + Scenario: Logout function - Scram + Given an Application + When I bootstrap passport with authentication type 'scram' + Then logout redirects to '/auth/logout' + + Scenario: Logout function - No auth + Given an Application + When I bootstrap passport with authentication type 'none' + Then logout returns '200' + + Scenario: Unsupported auth type + Given an Application + When I bootstrap passport with authentication type 'unsupported' + Then an error is thrown diff --git a/server/security/bootstrap.steps.ts b/server/security/bootstrap.steps.ts new file mode 100644 index 00000000..c1abe8ed --- /dev/null +++ b/server/security/bootstrap.steps.ts @@ -0,0 +1,137 @@ +/* + * Copyright Strimzi authors. + * License: Apache License 2.0 (see the file LICENSE or http://apache.org/licenses/LICENSE-2.0.html). + */ +import { bootstrapPassport } from './bootstrap'; +import express from 'express'; +import { authenticationStrategies } from 'types'; +import { + stepWhichUpdatesWorld, + stepWithWorld, +} from 'test_common/commonServerSteps'; +import { When, Then, Fusion, And } from 'jest-cucumber-fusion'; +import request from 'supertest'; +import { Authentication } from 'security'; + +const proxyDefaults = { + hostname: '', + port: 0, + contextRoot: '/', + transport: {}, +}; + +When( + /^I bootstrap passport with authentication type '(\w+)'$/, + stepWhichUpdatesWorld((world, authType) => { + try { + const auth = bootstrapPassport(world.context.app as express.Application, { + authentication: { type: authType as authenticationStrategies }, + ...proxyDefaults, + }); + world.context.auth = auth; + } catch (e) { + world.context.error = e; + } + + return world; + }) +); + +And( + 'the request is authenticated', + stepWithWorld((world) => { + const app = world.context.app as express.Application; + app.get('*', (req, _res, next) => { + req.isAuthenticated = () => true; + return next(); + }); + }) +); + +Then( + /^check auth redirects to '(.+)'$/, + stepWithWorld((world, redirectUrl) => { + const app = world.context.app as express.Application; + const auth = world.context.auth as Authentication; + app.get('/test', auth.checkAuth, (_req, res) => { + res.send('success'); + }); + + return request(app) + .get('/test') + .expect(302) + .expect('Location', redirectUrl as string); + }) +); + +Then( + /^check auth returns '(\d+)'$/, + stepWithWorld((world) => { + const app = world.context.app as express.Application; + const auth = world.context.auth as Authentication; + app.get('/test', auth.checkAuth, (_req, res) => { + res.send('success'); + }); + return request(app).get('/test').expect(200); + }) +); + +Then( + /^logout returns '(\d+)'$/, + stepWithWorld((world) => { + const app = world.context.app as express.Application; + const auth = world.context.auth as Authentication; + app.get('/test', auth.logout, (_req, res) => { + res.send('success'); + }); + return request(app).get('/test').expect(200); + }) +); + +Then( + /^logout redirects to '(.+)'$/, + stepWithWorld((world, redirectUrl) => { + const app = world.context.app as express.Application; + const auth = world.context.auth as Authentication; + const logout = jest.fn(); + app.use('*', (req, _res, next) => { + req.logout = logout; + return next(); + }); + app.post('/logout', auth.logout); + + return request(app) + .post('/logout') + .expect(302) + .expect('Location', redirectUrl as string) + .then(() => { + expect(logout).toHaveBeenCalled(); + }); + }) +); + +Then( + /^I bootstrap passport with authentication type '(\w+)'$/, + stepWhichUpdatesWorld((world, authType) => { + const auth = bootstrapPassport(world.context.app as express.Application, { + authentication: { type: authType as authenticationStrategies }, + ...proxyDefaults, + }); + + world.context.auth = auth; + return world; + }) +); + +Then( + 'an error is thrown', + stepWithWorld((world) => { + const { + context: { error }, + } = world; + + expect(error).toBeTruthy(); + }) +); + +Fusion('bootstrap.feature'); diff --git a/server/security/index.ts b/server/security/index.ts index e1821c1d..c69bb1ff 100644 --- a/server/security/index.ts +++ b/server/security/index.ts @@ -4,3 +4,4 @@ */ export { bootstrapPassport as bootstrapAuthentication } from './bootstrap'; export * from './types'; +export { SecurityModule } from './router'; diff --git a/server/security/routeConfig.ts b/server/security/routeConfig.ts new file mode 100644 index 00000000..fa808e75 --- /dev/null +++ b/server/security/routeConfig.ts @@ -0,0 +1,10 @@ +/* + * Copyright Strimzi authors. + * License: Apache License 2.0 (see the file LICENSE or http://apache.org/licenses/LICENSE-2.0.html). + */ +export const scram = { + login: '/login', + logout: '/logout', +}; + +export const apiRoot = '/auth'; diff --git a/server/security/router.ts b/server/security/router.ts index 71b97ebb..09c0da67 100644 --- a/server/security/router.ts +++ b/server/security/router.ts @@ -4,19 +4,27 @@ */ import express from 'express'; import { authenticationStrategies, UIServerModule } from '../types'; +import { apiRoot, scram } from './routeConfig'; const moduleName = 'security'; -export const ClientModule: UIServerModule = { +export const SecurityModule: UIServerModule = { moduleName, - addModule: (logger, auth, { authentication }) => { + addModule: (logger, auth, { proxy }) => { + const authentication = proxy.authentication; const { exit } = logger.entry('addModule', authentication); const routerForModule = express.Router(); switch (authentication.type) { case authenticationStrategies.SCRAM: { - routerForModule.post('/login', auth.authenticate); - routerForModule.post('/logout', auth.logout); + logger.info('Mouting SCRAM security routes'); + routerForModule.post(scram.login, auth.authenticate, (_req, res) => + res.send(200) + ); + routerForModule.get(scram.login, (_req, res) => + res.send('This will later be the login page') + ); + routerForModule.post(scram.logout, auth.logout); break; } case authenticationStrategies.NONE: @@ -26,6 +34,6 @@ export const ClientModule: UIServerModule = { } } - return exit({ mountPoint: '/', routerForModule }); + return exit({ mountPoint: apiRoot, routerForModule }); }, }; diff --git a/server/security/security.feature b/server/security/security.feature new file mode 100644 index 00000000..70d237cc --- /dev/null +++ b/server/security/security.feature @@ -0,0 +1,29 @@ +# Copyright Strimzi authors. +# License: Apache License 2.0 (see the file LICENSE or http://apache.org/licenses/LICENSE-2.0.html). +Feature: Security module + + Scenario: Auth - valid SCRAM + + Given a 'security_only' server configuration + And 'scram' authentication is required + And I run an instance of the Strimzi-UI server + And a user is valid + When I send credentials to endpoint '/auth/login' + Then I get the expected status code '200' response + + Scenario: Auth - invalid SCRAM + + Given a 'security_only' server configuration + And 'scram' authentication is required + And I run an instance of the Strimzi-UI server + And a user is invalid + When I send credentials to endpoint '/auth/login' + Then I get the expected status code '401' response + + Scenario: Auth + + Given a 'security_only' server configuration + And 'none' authentication is required + And I run an instance of the Strimzi-UI server + When I send credentials to endpoint '/auth/login' + Then I get the expected status code '404' response \ No newline at end of file diff --git a/server/security/security.steps.ts b/server/security/security.steps.ts new file mode 100644 index 00000000..9fbf7de2 --- /dev/null +++ b/server/security/security.steps.ts @@ -0,0 +1,44 @@ +/* + * Copyright Strimzi authors. + * License: Apache License 2.0 (see the file LICENSE or http://apache.org/licenses/LICENSE-2.0.html). + */ +import { And } from 'jest-cucumber-fusion'; +import nock from 'nock'; +import { + stepWithWorld, + stepWhichUpdatesWorld, +} from 'test_common/commonServerSteps'; +import { When, Fusion } from 'jest-cucumber-fusion'; + +And( + 'a user is valid', + stepWithWorld((world) => { + const { hostname, contextRoot, port } = world.configuration.proxy; + nock(`http://${hostname}:${port}`).post(contextRoot).reply(200, {}); + }) +); +And( + 'a user is invalid', + stepWithWorld((world) => { + const { hostname, contextRoot, port } = world.configuration.proxy; + nock(`http://${hostname}:${port}`) + .post(contextRoot) + .reply(200, { errors: ['unauth'] }); + }) +); + +When( + /^I send credentials to endpoint '(.+)'$/, + stepWhichUpdatesWorld((world, endpoint) => { + const { server } = world; + return { + ...world, + request: server.post(endpoint as string).send({ + username: 'user', + password: 'password', + }), + }; + }) +); + +Fusion('security.feature'); diff --git a/server/security/strategy/StrategyFactory.ts b/server/security/strategy/StrategyFactory.ts index d8e19ea8..c182b798 100644 --- a/server/security/strategy/StrategyFactory.ts +++ b/server/security/strategy/StrategyFactory.ts @@ -3,13 +3,9 @@ * License: Apache License 2.0 (see the file LICENSE or http://apache.org/licenses/LICENSE-2.0.html). */ import { Strategy } from 'passport'; -import { - authenticationConfig, - authenticationStrategies, - scramConfig, -} from 'types'; +import { authenticationStrategies, proxyConfigType } from 'types'; import { Strategy as LocalStrategy } from 'passport-local'; -import { createVerifyCallback as createSaslCallback } from './sasl/SaslAuthenticator'; +import { createVerifyCallback as createSaslCallback } from './scram/ScramAuthenticator'; export interface AuthenticationStrategy { name: string; @@ -17,14 +13,15 @@ export interface AuthenticationStrategy { } export const getStrategy = ( - config: authenticationConfig + config: proxyConfigType ): AuthenticationStrategy => { - switch (config.type) { + const authConfig = config.authentication; + switch (authConfig.type) { case authenticationStrategies.SCRAM: { - const scramConfig = config as scramConfig; + const endpoint = `http://${config.hostname}:${config.port}${config.contextRoot}`; //TODO https support return { - name: config.type.toString(), - strategy: new LocalStrategy(createSaslCallback(scramConfig)), + name: authConfig.type.toString(), + strategy: new LocalStrategy(createSaslCallback(endpoint)), }; } case authenticationStrategies.NONE: diff --git a/server/security/strategy/sasl/SaslAuthenticator.ts b/server/security/strategy/sasl/SaslAuthenticator.ts deleted file mode 100644 index c8a3f612..00000000 --- a/server/security/strategy/sasl/SaslAuthenticator.ts +++ /dev/null @@ -1,31 +0,0 @@ -/* - * Copyright Strimzi authors. - * License: Apache License 2.0 (see the file LICENSE or http://apache.org/licenses/LICENSE-2.0.html). - */ -import axios from 'axios'; -import { VerifyFunction } from 'passport-local'; -import { scramConfig } from 'types'; -import { generateLogger } from 'logging'; - -const logger = generateLogger('ScramAuthenticator'); - -const createVerifyCallback = (config: scramConfig): VerifyFunction => { - const { exit } = logger.entry('createVerifyCallback'); - - const verify = async (username, password, done) => { - const { exit } = logger.entry('createVerifyCallback - callback'); - try { - const { data } = await axios.post(config.endpoint.toString(), { - username, - password, - }); - - return exit(done(null, { username, token: data.token })); - } catch (err) { - return exit(done(err, null)); - } - }; - return exit(verify); -}; - -export { createVerifyCallback }; diff --git a/server/security/strategy/sasl/SaslAuthenticator.feature b/server/security/strategy/scram/ScramAuthenticator.feature similarity index 68% rename from server/security/strategy/sasl/SaslAuthenticator.feature rename to server/security/strategy/scram/ScramAuthenticator.feature index 1e0d6793..84dab5eb 100644 --- a/server/security/strategy/sasl/SaslAuthenticator.feature +++ b/server/security/strategy/scram/ScramAuthenticator.feature @@ -1,28 +1,27 @@ # Copyright Strimzi authors. # License: Apache License 2.0 (see the file LICENSE or http://apache.org/licenses/LICENSE-2.0.html). -Feature: Sasl Authenticator +Feature: Scram Authenticator Scenario Outline: Accepts a valid username and password - Given an authentication endpoint of 'http://strimzi-admin' + Given an authentication endpoint of 'https://strimzi-admin' And the authentication endpoint accepts username '' and password '' - And the authentication endpoint returns an auth token '' When a verify callback is generated And I call verify with username '' and password '' - Then the callback should return a user with username '' and an auth token '' + Then the callback should return a user with username '' and a token Examples: - | username | password | token | - | test-user | test-pw | test-token | + | username | password | + | test-user | test-pw | Scenario: Rejects an invalid username and password - Given an authentication endpoint of 'http://strimzi-admin' + Given an authentication endpoint of 'https://strimzi-admin' And the authentication endpoint refuses any credentials When a verify callback is generated And I call verify with username 'username' and password 'password' - Then the callback should return an error + Then the callback should return 'false' Scenario: Rejects when unable to authenticate - Given an authentication endpoint of 'http://strimzi-admin' + Given an authentication endpoint of 'https://strimzi-admin' And the authentication endpoint returns a server error When a verify callback is generated And I call verify with username 'username' and password 'password' diff --git a/server/security/strategy/sasl/SaslAuthenticator.steps.ts b/server/security/strategy/scram/ScramAuthenticator.steps.ts similarity index 73% rename from server/security/strategy/sasl/SaslAuthenticator.steps.ts rename to server/security/strategy/scram/ScramAuthenticator.steps.ts index 2807c23a..dfcf3c7d 100644 --- a/server/security/strategy/sasl/SaslAuthenticator.steps.ts +++ b/server/security/strategy/scram/ScramAuthenticator.steps.ts @@ -8,10 +8,9 @@ import { stepWithWorld, } from 'test_common/commonServerSteps'; -import { createVerifyCallback } from './SaslAuthenticator'; +import { createVerifyCallback } from './ScramAuthenticator'; import { VerifyFunction } from 'passport-local'; import nock from 'nock'; -import { authenticationStrategies } from 'types'; let verify: VerifyFunction; @@ -31,13 +30,15 @@ Given( And( /^the authentication endpoint accepts username '(\S+)' and password '(\S+)'$/, stepWhichUpdatesWorld((world, username, password) => { - const requestBody = { - username: username as string, - password: password as string, - }; + const token = `Basic ${Buffer.from(`${username}:${password}`).toString( + 'base64' + )}`; const { context } = world; - - context.nock = (context.nock as nock.Scope).post('/', requestBody); + context.token = token; + context.nock = (context.nock as nock.Scope) + .matchHeader('authentication', token) + .post('/') + .reply(200, {}); return { ...world, context, @@ -50,7 +51,13 @@ And( stepWhichUpdatesWorld((world) => { const { context } = world; - context.nock = (context.nock as nock.Scope).post('/').reply(403); + context.nock = (context.nock as nock.Scope).post('/').reply(200, { + errors: [ + { + message: 'auth error', + }, + ], + }); return { ...world, context, @@ -71,33 +78,13 @@ And( }) ); -And( - /^the authentication endpoint returns an auth token '(\S+)'$/, - stepWhichUpdatesWorld((world, token) => { - const { context } = world; - - context.nock = (context.nock as nock.Interceptor).reply(200, { - token, - }); - context['authToken'] = token; - return { - ...world, - context, - }; - }) -); - When( 'a verify callback is generated', stepWithWorld((world) => { const { context: { endpoint }, } = world; - verify = createVerifyCallback({ - type: authenticationStrategies.SCRAM, - endpoint: new URL(endpoint as string), - options: {}, - }); + verify = createVerifyCallback(endpoint as string); }) ); @@ -117,16 +104,27 @@ And( ); Then( - /^the callback should return a user with username '(\S+)' and an auth token '(\S+)'$/, - stepWithWorld((world, username, token) => { + /^the callback should return a user with username '(\S+)' and a token$/, + stepWithWorld((world, username) => { const { - context: { error, user }, + context: { error, user, token }, } = world; expect(error).toBeNull(); expect(user).toEqual({ username, token }); }) ); +Then( + /^the callback should return '(\S+)'$/, + stepWithWorld((world, result) => { + const { + context: { error, user }, + } = world; + expect(error).toBeNull(); + expect(JSON.stringify(user)).toEqual(result); + }) +); + Then( 'the callback should return an error', stepWithWorld((world) => { @@ -138,4 +136,4 @@ Then( }) ); -Fusion('SaslAuthenticator.feature'); +Fusion('ScramAuthenticator.feature'); diff --git a/server/security/strategy/scram/ScramAuthenticator.ts b/server/security/strategy/scram/ScramAuthenticator.ts new file mode 100644 index 00000000..0c81d61d --- /dev/null +++ b/server/security/strategy/scram/ScramAuthenticator.ts @@ -0,0 +1,43 @@ +/* + * Copyright Strimzi authors. + * License: Apache License 2.0 (see the file LICENSE or http://apache.org/licenses/LICENSE-2.0.html). + */ +import axios from 'axios'; +import { VerifyFunction } from 'passport-local'; +import { generateLogger } from 'logging'; + +const logger = generateLogger('ScramAuthenticator'); + +const createVerifyCallback = (endpoint: string): VerifyFunction => { + const { exit } = logger.entry('createVerifyCallback'); + + const verify = async (username, password, done) => { + const { exit } = logger.entry('createVerifyCallback - callback'); + const query = { query: 'query {clusterInfo}' }; + try { + const token = Buffer.from(`${username}:${password}`).toString('base64'); + const basicAuth = `Basic ${token}`; + const { data } = await axios.post(endpoint, query, { + //TODO HTTPS support + headers: { + Authentication: basicAuth, + 'Content-Type': 'application/json', + Accept: 'application/json', + }, + }); + + if (data.errors) { + logger.error('Error in Admin server response', data.errors); + return exit(done(null, false)); + } + + return exit(done(null, { username, token: basicAuth })); + } catch (err) { + logger.error('Error with admin server', err); + return exit(done(err, null)); + } + }; + return exit(verify); +}; + +export { createVerifyCallback }; diff --git a/server/security/types.ts b/server/security/types.ts index 5dbe3dfa..64bbb605 100644 --- a/server/security/types.ts +++ b/server/security/types.ts @@ -9,3 +9,8 @@ export interface Authentication { checkAuth: RequestHandler; logout: RequestHandler; } + +export interface User { + name: string; + accessToken: string; +} diff --git a/server/test_common/commonServerSteps.ts b/server/test_common/commonServerSteps.ts index 5b8bcad3..8e1b143e 100644 --- a/server/test_common/commonServerSteps.ts +++ b/server/test_common/commonServerSteps.ts @@ -4,7 +4,7 @@ */ import request from 'supertest'; import { returnExpress } from 'core'; -import { Given, When, And } from 'jest-cucumber-fusion'; +import { Given, When, Then, And } from 'jest-cucumber-fusion'; import { authenticationStrategies, serverConfig } from 'types'; import { getConfigForName } from './testConfigs'; import express from 'express'; @@ -50,11 +50,21 @@ Given( }) ); +Given( + 'an Application', + stepWhichUpdatesWorld((world) => { + world.context.app = express(); + return world; + }) +); + And( /^'(\w+)' authentication is required$/, stepWhichUpdatesWorld((world, type) => { const configuration = merge(world.configuration, { - authentication: { type: type as authenticationStrategies }, + proxy: { + authentication: { type: type as authenticationStrategies }, + }, }); return { ...world, @@ -98,4 +108,13 @@ When( }) ); +Then( + /I get the expected status code '(.+)' response/, + stepWithWorld((world, statusCode) => { + const expectedStatus = parseInt(statusCode as string); + const { request } = world; + return request.expect(expectedStatus); + }) +); + export { stepWhichUpdatesWorld, stepWithWorld }; diff --git a/server/test_common/testConfigs.ts b/server/test_common/testConfigs.ts index e4149676..16c42b46 100644 --- a/server/test_common/testConfigs.ts +++ b/server/test_common/testConfigs.ts @@ -18,49 +18,40 @@ const modules = { config: false, log: false, mockapi: false, + security: false, }; -const mockapiModuleConfig: () => serverConfig = () => - merge(merge({}, defaultTestConfig()), { - modules: { ...modules, mockapi: true }, - }); -const logModuleConfig: () => serverConfig = () => - merge(merge({}, defaultTestConfig()), { - modules: { ...modules, log: true }, - }); -const configModuleConfig: () => serverConfig = () => - merge(merge({}, defaultTestConfig()), { - modules: { ...modules, config: true }, +const enableModule: (module: string) => serverConfig = (module) => + merge({}, defaultTestConfig(), { + modules: { ...modules, ...{ [module]: true } }, }); + const clientModuleConfig: () => serverConfig = () => - merge(merge({}, defaultTestConfig()), { + merge(enableModule('client'), { client: { publicDir: resolve(__dirname, './__test_fixtures__/client'), }, - modules: { ...modules, client: true }, }); const apiModuleConfig: () => serverConfig = () => - merge(merge({}, defaultTestConfig()), { + merge(enableModule('api'), { proxy: { hostname: 'test-backend', port: 3434, }, - modules: { ...modules, api: true }, }); const apiModuleConfigWithCustomContextRoot: () => serverConfig = () => - merge(merge({}, defaultTestConfig()), { + merge(enableModule('api'), { proxy: { hostname: 'test-backend', port: 3434, contextRoot: '/myCustomContextRoot', }, - modules: { ...modules, api: true }, }); const securedApiModuleConfig: () => serverConfig = () => - merge(merge(apiModuleConfig()), { + merge(enableModule('api'), { proxy: { transport: { cert: 'mock certificate', @@ -75,13 +66,15 @@ export const getConfigForName: (name: string) => serverConfig = (name) => { case 'production': return defaultTestConfig(); case 'mockapi_only': - return mockapiModuleConfig(); + return enableModule('mockapi'); case 'log_only': - return logModuleConfig(); + return enableModule('log'); case 'config_only': - return configModuleConfig(); + return enableModule('config'); case 'client_only': return clientModuleConfig(); + case 'security_only': + return enableModule('security'); case 'api_only': return apiModuleConfig(); case 'api_secured_only': diff --git a/server/types.ts b/server/types.ts index defe888d..6645cd71 100644 --- a/server/types.ts +++ b/server/types.ts @@ -5,7 +5,6 @@ import express from 'express'; import { SecureVersion } from 'tls'; import { Logger, LoggerOptions } from 'pino'; -import passport from 'passport'; import { Authentication } from 'security'; export enum authenticationStrategies { @@ -17,12 +16,6 @@ export enum authenticationStrategies { export interface authenticationConfig { /** What authentication strategy to use to authenticate users */ type: authenticationStrategies; - options: passport.AuthenticateOptions; -} - -export interface scramConfig extends authenticationConfig { - type: authenticationStrategies.SCRAM; - endpoint: URL; } type sslCertificateType = { @@ -61,7 +54,7 @@ type moduleConfigType = { mockapi: boolean; }; -type proxyConfigType = { +export type proxyConfigType = { /** The Hostname of the backend server to send API requests to */ hostname: string; /** The port number of the backend server to send API requests to */ @@ -70,6 +63,8 @@ type proxyConfigType = { contextRoot: string; /** SSL transport configuration */ transport: sslCertificateType; + /** authentication configuration */ + authentication: authenticationConfig; }; type sessionConfigType = { @@ -78,8 +73,6 @@ type sessionConfigType = { }; export type serverConfig = { - /** authentication configuration */ - authentication: authenticationConfig; /** client (browser) facing configuration */ client: clientConfigType; /** feature flag configuration overrides (for both client and server) */ diff --git a/test_common/jest_cucumber_support/index.ts b/test_common/jest_cucumber_support/index.ts index 41c84c14..bed6cd93 100644 --- a/test_common/jest_cucumber_support/index.ts +++ b/test_common/jest_cucumber_support/index.ts @@ -3,5 +3,5 @@ * License: Apache License 2.0 (see the file LICENSE or http://apache.org/licenses/LICENSE-2.0.html). */ /* eslint-disable */ -import { common_stepdefs } from './commonStepdefs'; +import './commonStepdefs'; import * as commonServerStepDefinitions from '../../server/test_common/commonServerSteps'; diff --git a/utils/dev_config/mockadmin.config.js b/utils/dev_config/mockadmin.config.js index f5116cf8..d62f9333 100644 --- a/utils/dev_config/mockadmin.config.js +++ b/utils/dev_config/mockadmin.config.js @@ -9,9 +9,6 @@ const { const { mockadminServer } = devEnvValues; module.exports = { - authentication: { - type: 'none', - }, client: { transport: { ...mockAdminCertificates, diff --git a/utils/dev_config/server.dev.config.js b/utils/dev_config/server.dev.config.js index c1205fba..ff7adac0 100644 --- a/utils/dev_config/server.dev.config.js +++ b/utils/dev_config/server.dev.config.js @@ -23,16 +23,20 @@ module.exports = { }, modules: { api: true, - client: false, + client: true, config: true, log: true, mockapi: false, + security: true, }, proxy: { ...mockadminServer, transport: { ...mockAdminCertificates, }, + authentication: { + type: 'none', + }, }, ...devServer, }; diff --git a/utils/dev_config/server.e2e.config.js b/utils/dev_config/server.e2e.config.js index c17b39e8..d5372741 100644 --- a/utils/dev_config/server.e2e.config.js +++ b/utils/dev_config/server.e2e.config.js @@ -21,6 +21,9 @@ module.exports = { transport: { ...mockAdminCertificates, }, + authentication: { + type: 'none', + }, }, ...devServer, }; diff --git a/utils/tooling/runtimeDevUtils.js b/utils/tooling/runtimeDevUtils.js index 34b9e866..fd6cc23a 100644 --- a/utils/tooling/runtimeDevUtils.js +++ b/utils/tooling/runtimeDevUtils.js @@ -23,9 +23,6 @@ const devEnvValues = { hostname: process.env.SD_HOSTNAME || 'localhost', port: process.env.SD_PORT || 3000, }, - authentication: { - type: 'none', - }, }; const getIfExists = (file) =>