-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Properly validate current user password during password change. #43447
Properly validate current user password during password change. #43447
Conversation
Pinging @elastic/kibana-security |
💚 Build Succeeded |
.send({ password: wrongPassword, newPassword }) | ||
.expect(401); | ||
|
||
// Let's check that we can't login with wrong password (bug happen :shrug:). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: typo in a silly comment, hilarious 🙈 (will remove when we'll be addressing review comments).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very meta :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great! Only one optional comment
/** | ||
* Utility class that knows how to decorate request with proper Basic authentication headers. | ||
*/ | ||
export class BasicCredentials { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good riddance!!!
let sessionCookie: Cookie; | ||
beforeEach(async () => { | ||
// Create mock user to change password for. | ||
await getService('supertest') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
optional: we added a security service which potentially makes this a bit easier
kibana/x-pack/test/api_integration/apis/apm/feature_controls.ts
Lines 190 to 194 in d66b3c7
await security.user.create(username, { | |
password, | |
roles: [roleName], | |
full_name: 'a kibana user', | |
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wow, didn't know about that, thanks!
.send({ password: wrongPassword, newPassword }) | ||
.expect(401); | ||
|
||
// Let's check that we can't login with wrong password (bug happen :shrug:). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very meta :)
💚 Build Succeeded |
7.x/7.4.0: ce716ae |
…_update_json_spec * 'master' of github.com:elastic/kibana: (35 commits) fix: 🐛 pass whole action context to isCompatible() method (elastic#43457) Deleted old kbn-top-nav directive (elastic#43168) [ML] Fixing cloning of single metric distinct count job (elastic#43435) Update @elastic/charts version 8.1.6 > 9.1.1 (elastic#43516) [Inspector Views] [Request View] - Migrate inspector_views to new platform (elastic#43191) [ML] Adding loading indicators to all wizard charts (elastic#43382) disable flaky test (elastic#43492) feature(code/frontend): cancel file blob and directory commits request if outdated (elastic#43348) fix(code/frontend): button group url should have previous query string (elastic#43428) [SIEM] Fixes index substring incorrectly matching configured indices and failing to install ML job (elastic#43409) [SIEM] Adds performance enhancements such by removing wasted renderers and adding incremental DOM rendering (elastic#43157) disable flaky test (elastic#37859) Added sass lint to Canvas (elastic#43410) [Maps] add indicator when layer is filtered by search bar (elastic#43283) Properly validate current user password during password change. (elastic#43447) Spaces - allow for hex color codes that include uppercase characters (elastic#43470) [Reporting] Add a bit more logging and a few more logging level promotions (elastic#43415) Partially convert index pattern server to typescript (elastic#43291) [Infra UI] Use sum for aggregating AWS metrics. (elastic#43293) [SIEM] Format bytes columns in timeline (elastic#43147) ...
In this PR we properly validate current user password before we try to change it and finally get rid of
BasicCredentials
class. The issue was introduced in #39446.Fixes: #42807