Skip to content

Security: Remove all plain text password logic and ui#4178

Merged
JohnMcLear merged 5 commits intoether:developfrom
JohnMcLear:strip-out-password
Oct 7, 2020
Merged

Security: Remove all plain text password logic and ui#4178
JohnMcLear merged 5 commits intoether:developfrom
JohnMcLear:strip-out-password

Conversation

@JohnMcLear
Copy link
Copy Markdown
Member

@JohnMcLear JohnMcLear commented Jul 17, 2020

Fixes #230

This doesn't deprecate.
It doesn't gracefully remove the API methods from being available
It doesn't publish an API version.

It is a pure cut off. If you use these API endpoints, it will screw you over.

@JohnMcLear JohnMcLear changed the title remove all password logic and ui Security: Remove all plain text password logic and ui Sep 5, 2020
Copy link
Copy Markdown
Member

@rhansen rhansen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the idea to force users to use plugins instead? (authorize in particular)

Also need to remove password stuff from:

  • src/node/db/SecurityManager.js
  • src/static/js/pad.js

Comment thread src/templates/pad.html
Comment thread src/templates/pad.html
@rhansen
Copy link
Copy Markdown
Member

rhansen commented Sep 28, 2020

Please rebase onto current develop.

@rhansen
Copy link
Copy Markdown
Member

rhansen commented Sep 29, 2020

Please rebase onto current develop.

I rebased your branch onto develop and force-pushed to your fork.

@JohnMcLear
Copy link
Copy Markdown
Member Author

Ready for further review.

@rhansen rhansen force-pushed the strip-out-password branch from 0bb8c47 to ae33209 Compare October 7, 2020 01:57
@rhansen rhansen force-pushed the strip-out-password branch from ae33209 to 71cab37 Compare October 7, 2020 02:40
@rhansen
Copy link
Copy Markdown
Member

rhansen commented Oct 7, 2020

@JohnMcLear I rebased your branch and pushed some additional changes. Please take a look. If everything looks good to you, I think it's ready to squash and merge.

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Oct 7, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
1.1% 1.1% Duplication

@JohnMcLear JohnMcLear merged commit 66df0a5 into ether:develop Oct 7, 2020
JohnMcLear added a commit that referenced this pull request Dec 23, 2020
#4178)

This will be a breaking change for some people.  

We removed all internal password control logic.  If this affects you, you have two options:

1. Use a plugin for authentication and use session based pad access (recommended).
1. Use a plugin for password setting.

The reasoning for removing this feature is to reduce the overall security footprint of Etherpad.  It is unnecessary and cumbersome to keep this feature and with the thousands of available authentication methods available in the world our focus should be on supporting those and allowing more granual access based on their implementations (instead of half assed baking our own).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Etherpad password only secured pads stores the specific pad password in plain-text cookies

2 participants