New UserTaskRun Metadata fields

[jhosep-marin]

When it comes to a productive environment in which you integrate an Identity Provider (IdP) into the UserTasks equation it's kind of hard to track assignments. Given the fact that you can assign a UserTaskRun to a userId, or a userGroup or both, it gets tricky when you do assignments based on IdPs' user IDs and/or user group IDs, because when you query the UserTaskRun you will see that it is assigned (or was assigned in the past) to userId "dffe5b6ac7a94c60951d8fdf8cf41d52" but you cannot tell if that id is Jessie from IT, or Mary the CTO, or anyone else, and the same applies for userGroups.

You may think something like "Okay, easy, just do assignments based on user emails/username and user group names. In the end, userId and userGroup take any string value!", and that is true, you can actually do that, but now, what happens if at some point, for any given reason, Jessie's username changes from "jessiewilliams" to "jwilliams"? I think that IdPs' user Ids and user group Ids are the way to go in order to address that issue, ensure longevity and traceability of UserTaskRuns when a userName or user group changes. And, the way to solve the initial predicament about translating userId "dffe5b6ac7a94c60951d8fdf8cf41d52" to "Jessie" or "Mary" could be solved by just adding metadata to the UserTaskRun.

This new metadata object should be OPTIONAL so that all existing features do not change, and you let the LH Server's clients decide whether they set it or not.

That way, not only will you be solving the IdPs id-to-actual-user + id-to-actual-user-group translation, but also improving UX when checking UserTaskRuns through CLI or LH built-in Dashboard.

[coltmcnealy-lh]

This is good, thanks @jhosep-marin! We can also add Tags on it so that you can search by these fields.

I have a feeling that @eduwercamacaro is going to say "I TOLD YOU SO" because in the past he proposed making the the assignment an object rather than just a string id. I guess he can say "I told you so" 😃

Changing the user_id into an object at this point would be a breaking change. I'd like to avoid that if possible. What we can do is make the metadata an object so that we keep the top-level fields in the UserTaskRun clean. Maybe something like this:

message UserTaskRunMetadata {
  optional string username = 1;
  optional string group_name = 2;
  optional string email = 3;
  optional string firstName = 4;
  // ...
}

Something like this in the UserTaskRun:

message UserTaskRun {
  // ...
  string user_id = 3;
  string user_id = 4;
  // ...
  optional UserTaskRunMetadata metadata = 13;
}

[jhosep-marin]

@coltmcnealy-lh I'm not sure I understand the Tags concept.
Could you please elaborate a bit more about it?

I like the UserTaskRunMetadata object proposal, and yes, I'm also in favor of not introducing breaking changes around userId and userGroup fields 👍

I would also like to read @eduwercamacaro thoughts on this

[jhosep-marin]

Also, @hazimoarafa @bryson-g , this would potentially improve the experience in the dashboard since it would allow the server to provide info about users and groups without connecting to any Identity Provider

[Snarr]

@jhosep-marin Tags are an implementation detail on the server that allow us to search for specific attributes on objects efficiently within RocksDB. Tags embed searchable key/value attributes into separate keys of our key/value store.

This means that we can filter for objects with specific Tag attributes at the key level. For example, Principals have a tag isAdmin, and so we can find all the admin Principals by searching for keys prefixed with __isAdmin_true. This is much faster than if we had to search for all Principals, then one by one deserialize them into Java Objects and checking if they're admins manually.

[jhosep-marin]

A way to set this metadata could be through the existing RPC AssignUserTaskRun. This way we just leverage the working code that is already there and we add a small chunk of code in charge of setting the respective UserTaskRunMetadata properties.

This metadata could probably be set after the assignment event.

[eduwercamacaro]

Thanks @jhosep-marin I like this idea because it improves the user experience by adding useful information (metadata) about the UserTaskRun, and it helps for many uses cases like user task tracking, audit or debugging.

What we can do is make the metadata an object so that we keep the top-level fields in the UserTaskRun clean.

@coltmcnealy-lh I don't think we need a metadata object for this, instead we can add a key-value (map<string, string>) structure into the UserTaskRun that can be added in both WfSpec and rpc AssignUserTaskRun. This enables users to add metadata when the UserTaskRun is not only assigned but also created. In my mind, it looks like this:

message UserTaskRun {
  // ...
  string user_id = 3;
  string user_id = 4;
  // ...
  optional map<string, string> metadata = 13;
}

At the WfSpec level, it looks like this:

UserTaskOutput assignUserTask(String userTaskDefName, Object userId, Object userGroup, String... metadata);

[jhosep-marin]

UserTaskOutput assignUserTask(String userTaskDefName, Object userId, Object userGroup, String... metadata);

Did you mean Map<String, String> as type of the metadata argument?

[coltmcnealy-lh]

@eduwercamacaro

What we can do is make the metadata an object so that we keep the top-level fields in the UserTaskRun clean.

I wasn't suggesting a new metadata object (like a UserTaskRunMetadata getable) but rather I was suggesting that the metadata should be nested inside another field to reduce the number of top-level fields inside the UserTaskRun.

What you suggest is like what I had in mind; the only difference is that I was wondering if we should have a structured object rather than just a map<string, string>. Some considerations:

1. Can we anticipate what fields a user might want to put in, so that we can have validations?
2. What types might a user want other than just string?

Basically, I prefer to have structured data rather than unstructured data (especially when the unstructured data forces people to only use string types rather than other types).

Other than that, I like your proposals for how to assign it from the WfSpec and the rpc AssignUserTask level.

---

One question though. I don't know if, at the WfSpec level, a user will "know how" to set fields such as the first_name and last_name. Sometimes we might; other times, we will not...if all we have is the IDP's user_id then that information would have to come from the IDP. But that is okay; we can just wait for someone on some form of console to assign and fill the information. Having the information late is better than never.

---

Last comment: we should include the UserTaskRunMetadata in the UTEAssigned structure in the events field of the UserTaskRun.

[coltmcnealy-lh]

(Basically, I agree with all your points @eduwercamacaro , but I prefer a structured object rather than a map<string, string>)