diff --git a/CHANGELOG.md b/CHANGELOG.md index 12c5ea66e4d..4a797552c17 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 57ae1c117c9..c011fe1d1c0 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 7cdadefcc67..fb570a393f6 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 5eb0a16adf6..e12a8fb1283 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 5e5d806fa06..081af38b828 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 e4b2b3d3c1d..f4cfdea6212 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 a65e62885f4..319746e749d 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 9f5b786f90b..1b5136b6137 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 85188d2b6d1..f41e64a4c17 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 818f04d9d42..073dcdf8cb5 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 cb714460fc8..f4e0be38e75 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 0794a7b1da0..9c300587fec 100644 --- a/src/node/handler/PadMessageHandler.js +++ b/src/node/handler/PadMessageHandler.js @@ -228,7 +228,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}); @@ -835,7 +835,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}); @@ -877,7 +877,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 a52393634fa..668e20a895d 100644 --- a/src/node/handler/SocketIORouter.js +++ b/src/node/handler/SocketIORouter.js @@ -68,7 +68,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); } @@ -79,14 +79,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); }); @@ -98,16 +98,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 d5e6c3bbde4..6b376c0e231 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 76ed6693247..bc7473b9c9e 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 6e294403ef3..b30c43b40c3 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 baf4d539eac..ee84f4b8b5c 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 5e990cd7255..dcd00f46966 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 416d7897e3d..9040cc1dd78 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 2721bd0e5fc..86f5f613774 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 cbcd90af4fd..3c1f71c1015 100644 --- a/src/static/tests.html +++ b/src/static/tests.html @@ -161,7 +161,5 @@

createGroup()

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 0ba81109561..9624975abb8 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 1e11ff0a18b..61aa3ceb7ba 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 e17af09fca2..ca3ddc4de64 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, });