Issue S3 web identity token refresh call with sufficient permissions

[pxsalehi]

Closes #119747

[elasticsearchmachine]

Hi @pxsalehi, I've created a changelog YAML for you.

[pxsalehi]

The fix is simple. I'm working on a test. As the issue is pretty clear, we could also merge this and follow up with a test. I'm gonna leave this on draft until I figure out an IT (probably based on RepositoryS3StsCredentialsRestIT).

[pxsalehi]

I tried to reproduce this with an IT in 7339307, based on RepositoryS3StsCredentialsRestIT but I can't make it fail. I don't know why. It is pretty clear that the refresh call needs to be in a SocketAccess.doPrivilegedVoid since making the same call outside the onFileChanged would lead to the AccessControlException, and other refresh calls are also done as a privileged operation. AFAICT, the onFileChanged is not coming from a privileged call itself, and it does successfully lead to a call to the fixture!

In any case, I think the fix on its own is valid. The rest test is also not ideal even if it works, since it relies on periodic (60s) intervals to pick up the file change.

[elasticsearchmachine]

Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination)

[mhl-b]

LGTM

[ywangd]

I tried to reproduce this with an IT in 7339307, based on RepositoryS3StsCredentialsRestIT but I can't make it fail. I don't know why

I think you may need test with real s3 to make it fail or a fixture that is not running on localhost. The permission check is for "connect,resolve". If a connection is already available in the pool, it may not trigger either. I had similar experience, see also #108280 (comment)

[ywangd]

LGTM

It would be great if you can confirm this does fix the issue with some manual test if automated test is not feasible.

[pxsalehi]

Thanks, both!

If a connection is already available in the pool

Yeah, you're right. That is the "why". As i mentioned, if I add an early refresh call, I can see the permission issue.

It would be great if you can confirm this does fix the issue with some manual test if automated test is not feasible.

A slightly different ordering of the dirty IT I had, does reproduce the exact exception and shows that the fix gets rid of it. See here. Considering this is about the permission issue not the STS stuff per se, I'm gonna leave it at that. An automated test would also not work in this specific setup due to how we check for the file change. I also don't think it is worth the hassle to setup a whole ECK with STS scenario.

[elasticsearchmachine]

💚 Backport successful

Status	Branch	Result
✅	8.x