Skip to content

Add user to task #124584

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Add user to task #124584

wants to merge 1 commit into from

Conversation

nik9000
Copy link
Member

@nik9000 nik9000 commented Mar 11, 2025

Add the user to the tasks output.

Closes #124252

Add the user to the tasks output.
@nik9000 nik9000 added :Security/Security Security issues without another label v9.1.0 labels Mar 11, 2025
@nik9000
Copy link
Member Author

nik9000 commented Mar 11, 2025

I'm pretty sure we don't want this in the headers, but that was the easiest place to add it in the draft.

String user = threadContext.getTransient(Task.USER_KEY);
if (user != null) {
headers.put(Task.USER_KEY, user);
}
Task task = request.createTask(taskIdGenerator.incrementAndGet(), type, action, request.getParentTask(), headers);
Copy link
Member Author

Choose a reason for hiding this comment

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

I think because there are so many implementations of this we might should make an object CreateTaskParams and stuff everything in there.

@tvernum tvernum self-requested a review April 1, 2025 02:41
@tvernum
Copy link
Contributor

tvernum commented Apr 1, 2025

My concern here (and we've run into this every time we've wanted to put user information into useful places) is that "username" isn't as meaningful or as simple as people expect.

Some challenges:

  • API Keys have a username, but it's the username of the person that created the API key. That's accurate, but rarely sufficient.
  • Usernames are not globally unique. The same username may exist in multiple realms and reflect different identities
  • When run-as is in use, there are 2 username (the authenticating user and the effective user)
  • Not everything that authenticates has a username that is meaningful.

As you can see in how we do this for the slowlog, representing that information well requires more than a single field.

  • public Map<String, String> getAuthContextForSlowLog() {
    if (this.securityContext.get() != null && this.securityContext.get().getAuthentication() != null) {
    Authentication authentication = this.securityContext.get().getAuthentication();
    Subject authenticatingSubject = authentication.getAuthenticatingSubject();
    Subject effetctiveSubject = authentication.getEffectiveSubject();
    Map<String, String> authContext = new HashMap<>();
    if (authenticatingSubject.getUser() != null) {
    authContext.put("user.name", authenticatingSubject.getUser().principal());
    authContext.put("user.realm", authenticatingSubject.getRealm().getName());
    if (authenticatingSubject.getUser().fullName() != null) {
    authContext.put("user.full_name", authenticatingSubject.getUser().fullName());
    }
    }
    // Only include effective user if different from authenticating user (run-as)
    if (effetctiveSubject.getUser() != null && effetctiveSubject.equals(authenticatingSubject) == false) {
    authContext.put("user.effective.name", effetctiveSubject.getUser().principal());
    authContext.put("user.effective.realm", effetctiveSubject.getRealm().getName());
    if (effetctiveSubject.getUser().fullName() != null) {
    authContext.put("user.effective.full_name", effetctiveSubject.getUser().fullName());
    }
    }
    authContext.put("auth.type", authentication.getAuthenticationType().name());
    if (authentication.isApiKey()) {
    authContext.put("apikey.id", authenticatingSubject.getMetadata().get(AuthenticationField.API_KEY_ID_KEY).toString());
    authContext.put("apikey.name", authenticatingSubject.getMetadata().get(AuthenticationField.API_KEY_NAME_KEY).toString());
    }
    return authContext;
    }
    return Map.of();
    }

It questionable as to whether we need all of that information for the tasks API, but I think it we add a username we'll immediately get asked to do it "right" and provide the user's name and/or the API key id (and name) as well. And probably realm.
Each of those really is useful and necessary for some real world use cases, and admins do ask us to make it all available.

@tvernum
Copy link
Contributor

tvernum commented Apr 1, 2025

A few years (!?!) ago we experimented with putting a user concept into core elasticsearch and even tried to plumb it into Task (as a real concept, so it touched a lot of files)

One of the big problems was deciding how complicated to make it.
We ended up with something simple and opaque, but it required a few debates, and I don't think anyone was perfectly sure we landed in the right spot.

@nik9000
Copy link
Member Author

nik9000 commented Apr 8, 2025

Oh fun! I think it's pretty important that we start getting this stuff in - for, well, some definition of this stuff. I'm not surprised that user is a complex concept. It really would be nice if I, personally, didn't have to understand it and could just "add the user stuff".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Security/Security Security issues without another label v9.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add user to task description
2 participants