Users can reset their own password without specifying their current password

[kobelb]

Currently, users can reset their own password in Kibana without specifying their current password.

I believe this is occurring because when we try to set the credentials to be used

kibana/x-pack/legacy/plugins/security/server/routes/api/v1/users.js

Lines 94 to 96 in 04a9547

	await server.plugins.security.getUser(
	BasicCredentials.decorateRequest(request, username, password)
	);

they're ignored, and we end up relying upon the credentials that were associated with the request in the security's plugins "auth handler".

@azasypkin should we consider augmenting the getUser function exposed by the security plugin

kibana/x-pack/plugins/security/server/authentication/index.ts

Lines 53 to 61 in 04a9547

	const getCurrentUser = async (request: KibanaRequest) => {
	if (isSecurityFeatureDisabled()) {
	return null;
	}
	return (await clusterClient
	.asScoped(request)
	.callAsCurrentUser('shield.authenticate')) as AuthenticatedUser;
	};

to essentially what we do in the BaseAuthenticationProvider:

kibana/x-pack/plugins/security/server/authentication/providers/base.ts

Lines 87 to 91 in 04a9547

	protected async getUser(request: KibanaRequest, authHeaders: Headers = {}) {
	return (await this.options.client
	.asScoped({ headers: { ...request.headers, ...authHeaders } })
	.callAsCurrentUser('shield.authenticate')) as AuthenticatedUser;
	}

[elasticmachine]

Pinging @elastic/kibana-security

[azasypkin]

Amazing, thanks for discovering that @kobelb! Let me debug it a bit first.

[azasypkin]

they're ignored, and we end up relying upon the credentials that were associated with the request in the security's plugins "auth handler".

You're right, we have a request <---> auth headers cache in the core and headers from this cache take precedence over headers that request has.

And it seems to be correct behavior (e.g. in Kerberos case request will contain Authorization: Negotiate xxx, but authentication headers will provide Authorization: Bearer yyy).

I'm leaning towards using your proposed solution (.asScoped({ headers: { ...request.headers, ...authHeaders } }) ) as an immediate fix.

But as a long term fix I was planning to get rid of BasicCredentials and use authenticator.login to cover this case eventually:

authenticator.login(request, { 
  provider: 'basic',
  sessionMode: SessionMode.NotAllowed,
  value: { username, password }
});

That solution requires a new sessionMode property (or something like this) in ProviderLoginAttempt that is Allowed by default (login attempt can modify existing session or establish a new one) and NotAllowed (login attempt doesn't touch session at all) for this use case (at least that was an initial idea). This way getUser will always return user associated with the request as it does currently and should do in the future I believe. I didn't give it much thought yet though...

What do you think?

[kobelb]

That solution requires a new sessionMode property (or something like this) in ProviderLoginAttempt that is Allowed by default (login attempt can modify existing session or establish a new one) and NotAllowed (login attempt doesn't touch session at all) for this use case (at least that was an initial idea). This way getUser will always return user associated with the request as it does currently and should do in the future I believe. I didn't give it much thought yet though...

I like it even better :)

[azasypkin]

I like it even better :)

Good, I'll try go this path then!