From e7a6e22593eba5598f8755922776784857f63d1c Mon Sep 17 00:00:00 2001 From: Sven Slootweg Date: Sun, 5 May 2019 23:27:50 +0200 Subject: [PATCH] WIP --- notes.txt | 5 + src/authenticators/dummy.js | 11 ++ src/errors.js | 4 +- src/index.js | 7 + src/middlewares/error-handler.js | 13 +- .../user-interactive-authentication.js | 79 +++++++++ src/middlewares/validate-payload.js | 7 +- .../client-api/authentication/register.js | 19 ++- src/uia-tracker.js | 160 ++++++++++++++++++ 9 files changed, 293 insertions(+), 12 deletions(-) create mode 100644 src/authenticators/dummy.js create mode 100644 src/middlewares/user-interactive-authentication.js create mode 100644 src/uia-tracker.js diff --git a/notes.txt b/notes.txt index 7f6a298..a0abe93 100644 --- a/notes.txt +++ b/notes.txt @@ -60,3 +60,8 @@ The following API endpoints are allowed to be accessed by guest accounts for end POST /keys/upload POST /keys/query POST /keys/claim + + +---------- + +FIXME: Review where the homeserver *address* should be returned and where the *name* should be returned (which can differ, ref. https://brendan.abolivier.bzh/enter-the-matrix/) -- in particular the `homeserver` property in login/register routes \ No newline at end of file diff --git a/src/authenticators/dummy.js b/src/authenticators/dummy.js new file mode 100644 index 0000000..9125581 --- /dev/null +++ b/src/authenticators/dummy.js @@ -0,0 +1,11 @@ +"use strict"; + +module.exports = function (_state) { + return function createDummyAuthenticator(_sessionId) { + return { + handler: () => { + return { result: "completed" }; + } + }; + }; +}; \ No newline at end of file diff --git a/src/errors.js b/src/errors.js index 09b67a5..3429013 100644 --- a/src/errors.js +++ b/src/errors.js @@ -15,6 +15,8 @@ let BadJson = BadRequest.extend("BadJson", { errorCode: "M_BAD_JSON" }); let Unauthorized = HttpError.extend("Unauthorized", { statusCode: 401, errorCode: "M_UNAUTHORIZED" }); let MissingAccessToken = Unauthorized.extend("MissingAccessToken", { errorCode: "M_MISSING_TOKEN" }); let MissingCaptcha = Unauthorized.extend("MissingCaptcha", { errorCode: "M_CAPTCHA_NEEDED" }); +let UIARequired = HttpError.extend("UIARequired", { statusCode: 401 }); +let AuthenticationError = Unauthorized.extend("AuthenticationError", { errorCode: "M_FORBIDDEN" }); /*** HTTP 403: Forbidden ***/ @@ -67,5 +69,5 @@ let UnknownError = InternalServerError.extend("UnknownError", { errorCode: "M_UN let Unreachable = InternalServerError.extend("Unreachable"); /* FIXME: How to represent this with an error code? This should crash the process anyway. */ module.exports = { - HttpError, Unauthorized, MissingAccessToken, InvalidAccessToken, NotJson, BadJson, NotFound, TooManyRequests, MissingCaptcha, InvalidCaptcha, ServerNotTrusted, NoGuestAccess, UserExists, RoomExists, RequestTooLarge, InvalidUsername, InvalidRoomVersion, InvalidRoomState, InvalidStateChange, InvalidParameter, MissingParameter, IncompatibleRoomVersion, InternalServerError, UnknownError, ExclusiveResource, ResourceExists, Forbidden, BadRequest, ExternalIdentifierNotFound, ExternalIdentifierExists, ExternalIdentifierAuthenticationFailed, InvalidExternalIdentifier, WeakPassword, Unreachable + HttpError, Unauthorized, MissingAccessToken, InvalidAccessToken, NotJson, BadJson, NotFound, TooManyRequests, MissingCaptcha, InvalidCaptcha, ServerNotTrusted, NoGuestAccess, UserExists, RoomExists, RequestTooLarge, InvalidUsername, InvalidRoomVersion, InvalidRoomState, InvalidStateChange, InvalidParameter, MissingParameter, IncompatibleRoomVersion, InternalServerError, UnknownError, ExclusiveResource, ResourceExists, Forbidden, BadRequest, ExternalIdentifierNotFound, ExternalIdentifierExists, ExternalIdentifierAuthenticationFailed, InvalidExternalIdentifier, WeakPassword, Unreachable, UIARequired, AuthenticationError }; \ No newline at end of file diff --git a/src/index.js b/src/index.js index ffefaee..21dea5d 100644 --- a/src/index.js +++ b/src/index.js @@ -9,6 +9,7 @@ const url = require("url"); const debugMiddleware = require("./debug-middleware"); const accessTokenMiddleware = require("./middlewares/access-token"); const errorHandlerMiddleware = require("./middlewares/error-handler"); +const createUIATracker = require("./uia-tracker"); const clientApiRouter = require("./routers/client-api"); @@ -37,6 +38,12 @@ state.db = { accounts: require("./db/accounts")(state) }; +state.uiaTracker = createUIATracker({ + stages: { + "m.login.dummy": require("./authenticators/dummy")(state) + } +}); + let baseUrl = url.format({ protocol: (configuration.tls) ? "https" : "http", host: configuration.hostname diff --git a/src/middlewares/error-handler.js b/src/middlewares/error-handler.js index 76f01bd..c32858e 100644 --- a/src/middlewares/error-handler.js +++ b/src/middlewares/error-handler.js @@ -23,8 +23,8 @@ module.exports = function errorHandler(error, _req, res, _next) { ? errorContext.errorCode : "M_UNKNOWN"; - let errorMessage = (errorContext.errorCode !== 500) - ? defaultValue(errorContext.message, http.STATUS_CODES[statusCode]) + let errorMessage = (statusCode !== 500) + ? defaultValue(error.message, http.STATUS_CODES[statusCode]) : "An internal server error occurred. Please contact the server administrator for more information." /* TODO: Add contact details? */ @@ -32,8 +32,9 @@ module.exports = function errorHandler(error, _req, res, _next) { ? errorContext.errorMeta : {}; - res.status(statusCode).json(Object.assign({}, errorMeta, { - errcode: errorCode, - error: errorMessage - })); + let errorCodeData = (!errorContext.noErrorCode) + ? { errcode: errorCode, error: errorMessage } + : {}; + + res.status(statusCode).json(Object.assign({}, errorMeta, errorCodeData)); }; \ No newline at end of file diff --git a/src/middlewares/user-interactive-authentication.js b/src/middlewares/user-interactive-authentication.js new file mode 100644 index 0000000..a3070f1 --- /dev/null +++ b/src/middlewares/user-interactive-authentication.js @@ -0,0 +1,79 @@ +"use strict"; + +const Promise = require("bluebird"); +const defaultValue = require("default-value"); + +const errors = require("../errors"); + +module.exports = function ({ uiaTracker }) { + return function (options) { + if (options.flows == null || options.flows.length === 0) { + throw new Error("At least one authentication flow must be defined"); + } else { + let required = defaultValue(options.required, true); + + return function userInteractiveAuthenticationMiddleware(req, res, next) { + if (req.body.auth != null && req.body.auth.session != null) { + let authenticationData = req.body.auth; + /* FIXME: Riot currently doesn't pass back the session ID it was handed: https://github.com/vector-im/riot-web/issues/8458 - need to work around this*/ + let sessionId = authenticationData.session; + let authenticationType = authenticationData.type; + + return Promise.try(() => { + if (authenticationType != null) { + return uiaTracker.performAuthentication({ + sessionId: sessionId, + type: authenticationType, + data: authenticationData + }); + } else { + /* Client re-requested authentication session state without providing credentials; they most likely completed an out-of-band stage. */ + return uiaTracker.getStatus({ sessionId }); + } + }).then((status) => { + if (status.completed) { + req.uiaSessionId = sessionId; + return next(); + } else { + throw new errors.UIARequired("More authentication steps are required", { + noErrorCode: true, + errorMeta: status.sessionData + }); + } + }); + } else { + /* Workaround for Riot violating spec: https://github.com/vector-im/riot-web/issues/8458#issuecomment-488839052 */ + let riotAttempt = (req.body.auth != null); + + if (required || riotAttempt) { + return Promise.try(() => { + return uiaTracker.createSession({ + flows: options.flows, + }); + }).then((sessionData) => { + throw new errors.UIARequired("User-Interactive Authentication is required for this endpoint", { + noErrorCode: true, + errorMeta: sessionData + }); + }); + } else { + return next(); + } + } + }; + } + }; +}; + +/* MARKER: +If successful: either let through request (if all stages completed), or send back a 401 with updated "completed" list +Authentication method handlers are defined on the uiaTracker itself +Also need to remove the `tx = knex` from db modules + +errorMeta: { + flows: [], + params: {}, + session: sessionId + } +*/ +/* MARKER: Store flows/params in UIA tracker, send flows/params to user, write logic for subsequent UIA requests that interacts with the UIA tracker... also add code for UIA initialization where it is *optional* to do so */ \ No newline at end of file diff --git a/src/middlewares/validate-payload.js b/src/middlewares/validate-payload.js index ee2a9b5..e03b446 100644 --- a/src/middlewares/validate-payload.js +++ b/src/middlewares/validate-payload.js @@ -13,7 +13,12 @@ module.exports = function createPayloadValidator(validator) { } catch (error) { if (error instanceof ValidationError) { console.log(require("util").inspect(error, {colors: true, depth: null})); - throw new errors.MissingParameter("Something in the validation was wrong"); + + let errorMessage = (error.path != null) + ? `(property: ${error.path.join(".")}) ${error.message}` + : error.message; + + throw new errors.MissingParameter(`Validation error: ${errorMessage}`); } else { throw error; } diff --git a/src/routers/client-api/authentication/register.js b/src/routers/client-api/authentication/register.js index 742c6f0..0350abf 100644 --- a/src/routers/client-api/authentication/register.js +++ b/src/routers/client-api/authentication/register.js @@ -2,13 +2,17 @@ const Promise = require("bluebird"); const defaultValue = require("default-value"); +const expressPromiseRouter = require("express-promise-router"); const errors = require("../../../errors"); const validate = require("../../../middlewares/validate-payload"); const processDeviceID = require("../../../process-device-id"); -module.exports = function({ db, knex, configuration, fullyQualifyUser }) { - let router = require("express-promise-router")(); +module.exports = function(state) { + const userInteractiveAuthentication = require("../../../middlewares/user-interactive-authentication")(state); + + let { db, knex, configuration, fullyQualifyUser } = state; + let router = expressPromiseRouter(); function validateRegisterPayload(payload) { let {assertProperties, isPresent, isString, isOneOf, isBoolean} = require("../../../validator-lib"); @@ -30,11 +34,18 @@ module.exports = function({ db, knex, configuration, fullyQualifyUser }) { }); } - router.post("/r0/register", validate(validateRegisterPayload), (req, res) => { + let registerUIAMiddleware = userInteractiveAuthentication({ + /* FIXME: Modify settings based on eg. CAPTCHAs being enabled */ + required: false, + flows: [ + { stages: [ "m.login.dummy" ] } + ] + }); + + router.post("/r0/register", registerUIAMiddleware, validate(validateRegisterPayload), (req, res) => { // https://matrix.org/docs/spec/client_server/r0.4.0.html#post-matrix-client-r0-register // Register for an account on this homeserver. This API endpoint uses the User-Interactive Authentication API. - /* FIXME: Support User-Interactive Authentication (eg. for registration CAPTCHAs) */ /* FIXME: Rate-limit */ if (req.body.kind === "guest") { diff --git a/src/uia-tracker.js b/src/uia-tracker.js new file mode 100644 index 0000000..70685c4 --- /dev/null +++ b/src/uia-tracker.js @@ -0,0 +1,160 @@ +"use strict"; + +const Promise = require("bluebird"); +const nanoid = require("nanoid"); +const mapObj = require("map-obj"); +const defaultValue = require("default-value"); + +const errors = require("./errors"); + +function createResponse(session) { + return { + session: session.id, + completed: session.completedStageKeys, + params: mapObj(session.stages, ([stageKey, stageData]) => { + return [stageKey, stageData.parameters]; + }), + flows: session.flows.map((flow) => { + return { stages: flow.stages }; + }) + }; +} + +function createResult(session) { + return Promise.try(() => { + return createResponse(session); + }).then((response) => { + return { + completed: session.flows.some((flow) => { + return flow.stages.every((stage) => session.completedStageKeys.includes(stage)); + }), + sessionData: response + }; + }); +} + +// let stages = { +// "m.login.dummy": (_sessionId) => { +// return { +// parameters: { +// foo: "bar" +// }, +// handler: () => { +// return { result: "completed" }; +// } +// }; +// } +// }; + +/* FIXME: +- Restrict to valid stages / stage combinations (flows) +*/ + +function mapObjAsync(object, handler) { + return Promise.map(Object.entries(object), ([key, value]) => { + return handler(key, value); + }).reduce((mappedObject, [key, value]) => { + mappedObject[key] = value; + return mappedObject; + }, {}); +} + +module.exports = function createUIATracker({ stages }) { + let sessions = new Map(); + + function getSession(sessionId) { + if (sessions.has(sessionId)) { + return sessions.get(sessionId); + } else { + throw new errors.BadRequest("Authentication session does not exist"); + } + } + + return { + createSession: function ({ flows }) { + return Promise.try(() => { + let sessionId = nanoid(); + + return mapObjAsync(stages, (stageKey, stageInitializer) => { + return Promise.try(() => { + return stageInitializer(); + }).then((data) => { + return [stageKey, { + handler: data.handler, + parameters: defaultValue(data.parameters, {}) + }]; + }); + }).then((initializedStages) => { + let sessionObject = { + id: sessionId, + completedStageKeys: [], + stages: initializedStages, + flows: flows + }; + + sessions.set(sessionId, sessionObject); + + return createResponse(sessionObject); + }); + }); + }, + performAuthentication: function ({ sessionId, type, data }) { + return Promise.try(() => { + let session = getSession(sessionId); + let stage = session.stages[type]; + + return Promise.try(() => { + if (stage != null) { + if (session.completedStageKeys.includes(type)) { + /* Stage was already completed */ + /* FIXME: For now, we throw a 400 error; we may need to reevaluate this later: https://github.com/matrix-org/matrix-doc/issues/1987 */ + throw new errors.BadRequest("Attempted to complete an already-completed authentication stage"); + } else { + return Promise.try(() => { + let handler = stage.handler; + + if (handler != null) { + return Promise.try(() => { + return handler(data); + }).catch(errors.AuthenticationError, (error) => { + /* This attaches information about the current state of the authentication process, to any authentication error that occurs, so that it ends up in the response - as is required by the specification, for retryable authentication errors. */ + return Promise.try(() => { + return createResponse(session); + }).then((response) => { + error.errorMeta = response; + throw error; + }); + }); + } else { + throw new Error("Authentication stage is missing a handler; this is a bug"); + } + }).then((handlerResult) => { + if (handlerResult.newParameters != null) { + Object.assign(stage.parameters, handlerResult.newParameters); + } + + if (handlerResult.result === "completed") { + session.completedStageKeys.push(type); + } else if (handlerResult.result === "nextStepRequired") { + /* Do nothing, in this case new parameters will usually have been assigned */ + } else if (handlerResult.result === "failed") { + /* FIXME: Send along metadata? */ + throw new errors.Unauthorized(handlerResult.reason); + } else { + throw new Error(`Unrecognized authentication handler result '${handlerResult.result}'; this is a bug`); + } + + return createResult(session); + }); + } + } else { + throw new errors.BadRequest("Attempted to complete a non-existent authentication stage"); + } + }); + }); + }, + getStatus: function ({ sessionId }) { + return createResult(getSession(sessionId)); + } + }; +}; \ No newline at end of file