diff --git a/CHANGELOG.md b/CHANGELOG.md index 12c5ea66..4a797552 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Develop -- TODO Change to 1.8.x. ### Compatibility-breaking changes +* **IMPORTANT:** It is no longer possible to protect a group pad with a + password. All API calls to `setPassword` or `isPasswordProtected` will fail. + Existing group pads that were previously password protected will no longer be + password protected. If you need fine-grained access control, you can restrict + API session creation in your frontend service, or you can use plugins. * Authorization failures now return 403 by default instead of 401 * The `authorize` hook is now only called after successful authentication. Use the new `preAuthorize` hook if you need to bypass diff --git a/doc/api/hooks_server-side.md b/doc/api/hooks_server-side.md index 9c8630b4..10d73863 100644 --- a/doc/api/hooks_server-side.md +++ b/doc/api/hooks_server-side.md @@ -139,9 +139,8 @@ Called from: src/node/db/SecurityManager.js Things in context: 1. padID - the pad the user wants to access -2. password - the password the user has given to access the pad -3. token - the token of the author -4. sessionCookie - the session the use has +2. token - the token of the author +3. sessionCookie - the session the use has This hook gets called when the access to the concrete pad is being checked. Return `false` to deny access. diff --git a/doc/api/http_api.md b/doc/api/http_api.md index 7cdadefc..fb570a39 100644 --- a/doc/api/http_api.md +++ b/doc/api/http_api.md @@ -581,24 +581,6 @@ return true of false * `{code: 0, message:"ok", data: {publicStatus: true}}` * `{code: 1, message:"padID does not exist", data: null}` -#### setPassword(padID, password) - * API >= 1 - -returns ok or an error message - -*Example returns:* - * `{code: 0, message:"ok", data: null}` - * `{code: 1, message:"padID does not exist", data: null}` - -#### isPasswordProtected(padID) - * API >= 1 - -returns true or false - -*Example returns:* - * `{code: 0, message:"ok", data: {passwordProtection: true}}` - * `{code: 1, message:"padID does not exist", data: null}` - #### listAuthorsOfPad(padID) * API >= 1 diff --git a/doc/docker.md b/doc/docker.md index 5eb0a16a..e12a8fb1 100644 --- a/doc/docker.md +++ b/doc/docker.md @@ -171,7 +171,6 @@ For the editor container, you can also make it full width by adding `full-width- | `SUPPRESS_ERRORS_IN_PAD_TEXT` | Should we suppress errors from being visible in the default Pad Text? | `false` | | `REQUIRE_SESSION` | If this option is enabled, a user must have a session to access pads. This effectively allows only group pads to be accessed. | `false` | | `EDIT_ONLY` | Users may edit pads but not create new ones. Pad creation is only via the API. This applies both to group pads and regular pads. | `false` | -| `SESSION_NO_PASSWORD` | If set to true, those users who have a valid session will automatically be granted access to password protected pads. | `false` | | `MINIFY` | If true, all css & js will be minified before sending to the client. This will improve the loading performance massively, but makes it difficult to debug the javascript/css | `true` | | `MAX_AGE` | How long may clients use served javascript code (in seconds)? Not setting this may cause problems during deployment. Set to 0 to disable caching. | `21600` (6 hours) | | `ABIWORD` | Absolute path to the Abiword executable. Abiword is needed to get advanced import/export features of pads. Setting it to null disables Abiword and will only allow plain text and HTML import/exports. | `null` | diff --git a/settings.json.docker b/settings.json.docker index 5e5d806f..081af38b 100644 --- a/settings.json.docker +++ b/settings.json.docker @@ -260,12 +260,6 @@ */ "editOnly": "${EDIT_ONLY:false}", - /* - * If set to true, those users who have a valid session will automatically be - * granted access to password protected pads. - */ - "sessionNoPassword": "${SESSION_NO_PASSWORD:false}", - /* * If true, all css & js will be minified before sending to the client. * diff --git a/settings.json.template b/settings.json.template index e4b2b3d3..f4cfdea6 100644 --- a/settings.json.template +++ b/settings.json.template @@ -263,12 +263,6 @@ */ "editOnly": false, - /* - * If set to true, those users who have a valid session will automatically be - * granted access to password protected pads. - */ - "sessionNoPassword": false, - /* * If true, all css & js will be minified before sending to the client. * diff --git a/src/locales/en.json b/src/locales/en.json index a65e6288..319746e7 100644 --- a/src/locales/en.json +++ b/src/locales/en.json @@ -60,9 +60,7 @@ "pad.loading": "Loading...", "pad.noCookie": "Cookie could not be found. Please allow cookies in your browser! Your session and settings will not be saved between visits. This may be due to Etherpad being included in an iFrame in some Browsers. Please ensure Etherpad is on the same subdomain/domain as the parent iFrame", - "pad.passwordRequired": "You need a password to access this pad", "pad.permissionDenied": "You do not have permission to access this pad", - "pad.wrongPassword": "Your password was wrong", "pad.settings.padSettings": "Pad Settings", "pad.settings.myView": "My View", diff --git a/src/node/db/API.js b/src/node/db/API.js index 9f5b786f..1b5136b6 100644 --- a/src/node/db/API.js +++ b/src/node/db/API.js @@ -684,7 +684,6 @@ exports.setPublicStatus = async function(padID, publicStatus) publicStatus = (publicStatus.toLowerCase() === "true"); } - // set the password await pad.setPublicStatus(publicStatus); } @@ -706,44 +705,6 @@ exports.getPublicStatus = async function(padID) return { publicStatus: pad.getPublicStatus() }; } -/** -setPassword(padID, password) returns ok or a error message - -Example returns: - -{code: 0, message:"ok", data: null} -{code: 1, message:"padID does not exist", data: null} -*/ -exports.setPassword = async function(padID, password) -{ - // ensure this is a group pad - checkGroupPad(padID, "password"); - - // get the pad - let pad = await getPadSafe(padID, true); - - // set the password - await pad.setPassword(password === '' ? null : password); -} - -/** -isPasswordProtected(padID) returns true or false - -Example returns: - -{code: 0, message:"ok", data: {passwordProtection: true}} -{code: 1, message:"padID does not exist", data: null} -*/ -exports.isPasswordProtected = async function(padID) -{ - // ensure this is a group pad - checkGroupPad(padID, "password"); - - // get the pad - let pad = await getPadSafe(padID, true); - return { isPasswordProtected: pad.isPasswordProtected() }; -} - /** listAuthorsOfPad(padID) returns an array of authors who contributed to this pad diff --git a/src/node/db/Pad.js b/src/node/db/Pad.js index 85188d2b..f41e64a4 100644 --- a/src/node/db/Pad.js +++ b/src/node/db/Pad.js @@ -37,7 +37,6 @@ let Pad = function Pad(id) { this.head = -1; this.chatHead = -1; this.publicStatus = false; - this.passwordHash = null; this.id = id; this.savedRevisions = []; }; @@ -595,19 +594,6 @@ Pad.prototype.setPublicStatus = async function setPublicStatus(publicStatus) { await this.saveToDatabase(); }; -Pad.prototype.setPassword = async function setPassword(password) { - this.passwordHash = password == null ? null : hash(password, generateSalt()); - await this.saveToDatabase(); -}; - -Pad.prototype.isCorrectPassword = function isCorrectPassword(password) { - return compare(this.passwordHash, password); -}; - -Pad.prototype.isPasswordProtected = function isPasswordProtected() { - return this.passwordHash != null; -}; - Pad.prototype.addSavedRevision = async function addSavedRevision(revNum, savedById, label) { // if this revision is already saved, return silently for (var i in this.savedRevisions) { @@ -633,19 +619,3 @@ Pad.prototype.getSavedRevisions = function getSavedRevisions() { return this.savedRevisions; }; -/* Crypto helper methods */ - -function hash(password, salt) { - var shasum = crypto.createHash('sha512'); - shasum.update(password + salt); - - return shasum.digest("hex") + "$" + salt; -} - -function generateSalt() { - return randomString(86); -} - -function compare(hashStr, password) { - return hash(password, hashStr.split("$")[1]) === hashStr; -} diff --git a/src/node/db/SecurityManager.js b/src/node/db/SecurityManager.js index 818f04d9..073dcdf8 100644 --- a/src/node/db/SecurityManager.js +++ b/src/node/db/SecurityManager.js @@ -28,8 +28,6 @@ var log4js = require('log4js'); var authLogger = log4js.getLogger("auth"); const DENY = Object.freeze({accessStatus: 'deny'}); -const WRONG_PASSWORD = Object.freeze({accessStatus: 'wrongPassword'}); -const NEED_PASSWORD = Object.freeze({accessStatus: 'needPassword'}); /** * Determines whether the user can access a pad. @@ -42,16 +40,14 @@ const NEED_PASSWORD = Object.freeze({accessStatus: 'needPassword'}); * is false and the user is accessing a public pad. If there is not an author already associated * with this token then a new author object is created (including generating an author ID) and * associated with this token. - * @param password is the password the user has given to access this pad. It can be null. * @param userSettings is the settings.users[username] object (or equivalent from an authn plugin). - * @return {accessStatus: grant|deny|wrongPassword|needPassword, authorID: a.xxxxxx}. The caller - * must use the author ID returned in this object when making any changes associated with the - * author. + * @return {accessStatus: grant|deny, authorID: a.xxxxxx}. The caller must use the author ID + * returned in this object when making any changes associated with the author. * * WARNING: Tokens and session IDs MUST be kept secret, otherwise users will be able to impersonate * each other (which might allow them to gain privileges). */ -exports.checkAccess = async function(padID, sessionCookie, token, password, userSettings) +exports.checkAccess = async function(padID, sessionCookie, token, userSettings) { if (!padID) { authLogger.debug('access denied: missing padID'); @@ -84,7 +80,7 @@ exports.checkAccess = async function(padID, sessionCookie, token, password, user // allow plugins to deny access const isFalse = (x) => x === false; - if (hooks.callAll('onAccessCheck', {padID, password, token, sessionCookie}).some(isFalse)) { + if (hooks.callAll('onAccessCheck', {padID, token, sessionCookie}).some(isFalse)) { authLogger.debug('access denied: an onAccessCheck hook function returned false'); return DENY; } @@ -112,8 +108,7 @@ exports.checkAccess = async function(padID, sessionCookie, token, password, user }; if (!padID.includes('$')) { - // Only group pads can be private or have passwords, so there is nothing more to check for this - // non-group pad. + // Only group pads can be private, so there is nothing more to check for this non-group pad. return grant; } @@ -122,7 +117,7 @@ exports.checkAccess = async function(padID, sessionCookie, token, password, user authLogger.debug('access denied: must have an HTTP API session to create a group pad'); return DENY; } - // Creating a group pad, so there is no password or public status to check. + // Creating a group pad, so there is no public status to check. return grant; } @@ -133,18 +128,5 @@ exports.checkAccess = async function(padID, sessionCookie, token, password, user return DENY; } - const passwordExempt = settings.sessionNoPassword && sessionAuthorID != null; - const requirePassword = pad.isPasswordProtected() && !passwordExempt; - if (requirePassword) { - if (password == null) { - authLogger.debug('access denied: password required'); - return NEED_PASSWORD; - } - if (!password || !pad.isCorrectPassword(password)) { - authLogger.debug('access denied: wrong password'); - return WRONG_PASSWORD; - } - } - return grant; }; diff --git a/src/node/handler/APIHandler.js b/src/node/handler/APIHandler.js index cb714460..f4e0be38 100644 --- a/src/node/handler/APIHandler.js +++ b/src/node/handler/APIHandler.js @@ -70,8 +70,6 @@ version["1"] = Object.assign({}, , "getReadOnlyID" : ["padID"] , "setPublicStatus" : ["padID", "publicStatus"] , "getPublicStatus" : ["padID"] - , "setPassword" : ["padID", "password"] - , "isPasswordProtected" : ["padID"] , "listAuthorsOfPad" : ["padID"] , "padUsersCount" : ["padID"] } diff --git a/src/node/handler/PadMessageHandler.js b/src/node/handler/PadMessageHandler.js index 78a8b985..e068df98 100644 --- a/src/node/handler/PadMessageHandler.js +++ b/src/node/handler/PadMessageHandler.js @@ -219,7 +219,7 @@ exports.handleMessage = async function(client, message) const {session: {user} = {}} = client.client.request; const {accessStatus, authorID} = - await securityManager.checkAccess(padId, auth.sessionID, auth.token, auth.password, user); + await securityManager.checkAccess(padId, auth.sessionID, auth.token, user); if (accessStatus !== 'grant') { // Access denied. Send the reason to the user. client.json.send({accessStatus}); @@ -826,7 +826,7 @@ async function handleSwitchToPad(client, message, _authorID) const newPadIds = await readOnlyManager.getIds(message.padId); const {session: {user} = {}} = client.client.request; const {accessStatus, authorID} = await securityManager.checkAccess( - newPadIds.padId, message.sessionID, message.token, message.password, user); + newPadIds.padId, message.sessionID, message.token, user); if (accessStatus !== 'grant') { // Access denied. Send the reason to the user. client.json.send({accessStatus}); @@ -868,7 +868,6 @@ function createSessionInfoAuth(sessionInfo, message) sessionID: message.sessionID, padID: message.padId, token: message.token, - password: message.password, }; } diff --git a/src/node/handler/SocketIORouter.js b/src/node/handler/SocketIORouter.js index d7712b8e..4eb8b1cc 100644 --- a/src/node/handler/SocketIORouter.js +++ b/src/node/handler/SocketIORouter.js @@ -58,7 +58,7 @@ exports.setSocketIO = function(_socket) { // wrap the original send function to log the messages client._send = client.send; client.send = function(message) { - messageLogger.debug("to " + client.id + ": " + stringifyWithoutPassword(message)); + messageLogger.debug(`to ${client.id}: ${JSON.stringify(message)}`); client._send(message); } @@ -69,14 +69,14 @@ exports.setSocketIO = function(_socket) { client.on('message', async function(message) { if (message.protocolVersion && message.protocolVersion != 2) { - messageLogger.warn("Protocolversion header is not correct:" + stringifyWithoutPassword(message)); + messageLogger.warn(`Protocolversion header is not correct: ${JSON.stringify(message)}`); return; } if (!message.component || !components[message.component]) { - messageLogger.error("Can't route the message:" + stringifyWithoutPassword(message)); + messageLogger.error(`Can't route the message: ${JSON.stringify(message)}`); return; } - messageLogger.debug("from " + client.id + ": " + stringifyWithoutPassword(message)); + messageLogger.debug(`from ${client.id}: ${JSON.stringify(message)}`); await components[message.component].handleMessage(client, message); }); @@ -88,16 +88,3 @@ exports.setSocketIO = function(_socket) { }); }); } - -// returns a stringified representation of a message, removes the password -// this ensures there are no passwords in the log -function stringifyWithoutPassword(message) -{ - let newMessage = Object.assign({}, message); - - if (newMessage.password != null) { - newMessage.password = "xxx"; - } - - return JSON.stringify(newMessage); -} diff --git a/src/node/hooks/express/importexport.js b/src/node/hooks/express/importexport.js index d5e6c3bb..6b376c0e 100644 --- a/src/node/hooks/express/importexport.js +++ b/src/node/hooks/express/importexport.js @@ -65,7 +65,7 @@ exports.expressCreateServer = function (hook_name, args, cb) { args.app.post('/p/:pad/import', async function(req, res, next) { const {session: {user} = {}} = req; const {accessStatus} = await securityManager.checkAccess( - req.params.pad, req.cookies.sessionID, req.cookies.token, req.cookies.password, user); + req.params.pad, req.cookies.sessionID, req.cookies.token, user); if (accessStatus !== 'grant' || !webaccess.userCanModify(req.params.pad, req)) { return res.status(403).send('Forbidden'); } diff --git a/src/node/hooks/express/openapi.js b/src/node/hooks/express/openapi.js index 76ed6693..bc7473b9 100644 --- a/src/node/hooks/express/openapi.js +++ b/src/node/hooks/express/openapi.js @@ -201,15 +201,6 @@ const resources = { summary: 'return true of false', responseSchema: { publicStatus: { type: 'boolean' } }, }, - setPassword: { - operationId: 'setPassword', - summary: 'returns ok or a error message', - }, - isPasswordProtected: { - operationId: 'isPasswordProtected', - summary: 'returns true or false', - responseSchema: { passwordProtection: { type: 'boolean' } }, - }, authors: { operationId: 'listAuthorsOfPad', summary: 'returns an array of authors who contributed to this pad', diff --git a/src/node/padaccess.js b/src/node/padaccess.js index 6e294403..b30c43b4 100644 --- a/src/node/padaccess.js +++ b/src/node/padaccess.js @@ -5,7 +5,7 @@ module.exports = async function (req, res) { try { const {session: {user} = {}} = req; const accessObj = await securityManager.checkAccess( - req.params.pad, req.cookies.sessionID, req.cookies.token, req.cookies.password, user); + req.params.pad, req.cookies.sessionID, req.cookies.token, user); if (accessObj.accessStatus === "grant") { // there is access, continue diff --git a/src/node/utils/Settings.js b/src/node/utils/Settings.js index baf4d539..ee84f4b8 100644 --- a/src/node/utils/Settings.js +++ b/src/node/utils/Settings.js @@ -193,11 +193,6 @@ exports.requireSession = false; */ exports.editOnly = false; -/** - * A flag that bypasses password prompts for users with valid sessions - */ -exports.sessionNoPassword = false; - /** * Max age that responses will have (affects caching layer). */ diff --git a/src/static/css/pad/loadingbox.css b/src/static/css/pad/loadingbox.css index 5e990cd7..dcd00f46 100644 --- a/src/static/css/pad/loadingbox.css +++ b/src/static/css/pad/loadingbox.css @@ -10,10 +10,6 @@ z-index: 100; } -#editorloadingbox .passForm{ - padding:10px; -} - #editorloadingbox input{ padding:10px; } @@ -22,6 +18,6 @@ padding:10px; } -#passwordRequired, #permissionDenied, #wrongPassword, #noCookie { +#permissionDenied, #noCookie { display:none; -} \ No newline at end of file +} diff --git a/src/static/js/pad.js b/src/static/js/pad.js index 416d7897..9040cc1d 100644 --- a/src/static/js/pad.js +++ b/src/static/js/pad.js @@ -126,15 +126,6 @@ function getUrlVars() return vars; } -function savePassword() -{ - //set the password cookie - Cookies.set('password', $('#passwordinput').val(), {path: document.location.pathname}); - //reload - document.location=document.location; - return false; -} - function sendClientReady(isReconnect, messageType) { messageType = typeof messageType !== 'undefined' ? messageType : 'CLIENT_READY'; @@ -160,7 +151,6 @@ function sendClientReady(isReconnect, messageType) type: messageType, padId: padId, sessionID: Cookies.get('sessionID'), - password: Cookies.get('password'), token: token, protocolVersion: 2 }; @@ -225,10 +215,6 @@ function handshake() //the access was not granted, give the user a message if(obj.accessStatus) { - if(!receivedClientVars){ - $('.passForm').submit(require(module.id).savePassword); - } - if(obj.accessStatus == "deny") { $('#loading').hide(); @@ -241,19 +227,6 @@ function handshake() $("#editorloadingbox").show(); } } - else if(obj.accessStatus == "needPassword") - { - $('#loading').hide(); - $('#passwordRequired').show(); - $("#passwordinput").focus(); - } - else if(obj.accessStatus == "wrongPassword") - { - $('#loading').hide(); - $('#wrongPassword').show(); - $('#passwordRequired').show(); - $("#passwordinput").focus(); - } } //if we haven't recieved the clientVars yet, then this message should it be @@ -954,7 +927,6 @@ exports.settings = settings; exports.randomString = randomString; exports.getParams = getParams; exports.getUrlVars = getUrlVars; -exports.savePassword = savePassword; exports.handshake = handshake; exports.pad = pad; exports.init = init; diff --git a/src/static/js/timeslider.js b/src/static/js/timeslider.js index 2721bd0e..86f5f613 100644 --- a/src/static/js/timeslider.js +++ b/src/static/js/timeslider.js @@ -113,7 +113,6 @@ function sendSocketMsg(type, data) padId, token, sessionID: Cookies.get('sessionID'), - password: Cookies.get('password'), protocolVersion: 2, }); } diff --git a/src/static/tests.html b/src/static/tests.html index cbcd90af..3c1f71c1 100644 --- a/src/static/tests.html +++ b/src/static/tests.html @@ -161,7 +161,5 @@
getReadOnlyID(padID)
setPublicStatus(padID,publicStatus)
getPublicStatus(padID)
-
setPassword(padID,password)
-
isPasswordProtected(padID)
diff --git a/src/templates/pad.html b/src/templates/pad.html index 0ba81109..9624975a 100644 --- a/src/templates/pad.html +++ b/src/templates/pad.html @@ -91,18 +91,9 @@
-
-

You need a password to access this pad

-
- -
-

You do not have permission to access this pad

-
-

Your password was wrong

-
<% e.begin_block("loading"); %>

Loading...

<% e.end_block(); %> diff --git a/tests/backend/specs/api/sessionsAndGroups.js b/tests/backend/specs/api/sessionsAndGroups.js index 1e11ff0a..61aa3ceb 100644 --- a/tests/backend/specs/api/sessionsAndGroups.js +++ b/tests/backend/specs/api/sessionsAndGroups.js @@ -51,9 +51,6 @@ describe('API Versioning', function() { -> getPublicStatus(padId) -> setPublicStatus(padId, status) -> getPublicStatus(padId) - -> isPasswordProtected(padID) -- should be false - -> setPassword(padID, password) - -> isPasswordProtected(padID) -- should be true -> listPadsOfAuthor(authorID) */ @@ -269,35 +266,6 @@ describe('API: Pad security', function() { assert.equal(res.body.data.publicStatus, true); }); }); - - it('isPasswordProtected', async function() { - await api.get(endPoint('isPasswordProtected') + `&padID=${padID}`) - .expect(200) - .expect('Content-Type', /json/) - .expect((res) => { - assert.equal(res.body.code, 0); - assert.equal(res.body.data.isPasswordProtected, false); - }); - }); - - it('setPassword', async function() { - await api.get(endPoint('setPassword') + `&padID=${padID}&password=test`) - .expect(200) - .expect('Content-Type', /json/) - .expect((res) => { - assert.equal(res.body.code, 0); - }); - }); - - it('isPasswordProtected after setting password', async function() { - await api.get(endPoint('isPasswordProtected') + `&padID=${padID}`) - .expect(200) - .expect('Content-Type', /json/) - .expect((res) => { - assert.equal(res.body.code, 0); - assert.equal(res.body.data.isPasswordProtected, true); - }); - }); }); // NOT SURE HOW TO POPULAT THIS /-_-\ diff --git a/tests/backend/specs/socketio.js b/tests/backend/specs/socketio.js index e17af09f..ca3ddc4d 100644 --- a/tests/backend/specs/socketio.js +++ b/tests/backend/specs/socketio.js @@ -83,7 +83,6 @@ const handshake = async (socket, padID) => { type: 'CLIENT_READY', padId: padID, sessionID: null, - password: null, token: 't.12345', protocolVersion: 2, });