Skip to content

[TRANSFORM] Add authorization info to transform config listings#87570

Merged
droberts195 merged 10 commits intoelastic:masterfrom
droberts195:include_run_as_info
Jun 20, 2022
Merged

[TRANSFORM] Add authorization info to transform config listings#87570
droberts195 merged 10 commits intoelastic:masterfrom
droberts195:include_run_as_info

Conversation

@droberts195
Copy link
Copy Markdown

@droberts195 droberts195 commented Jun 9, 2022

Transforms remember permissions from their time of creation
or most recent update. It can be quite hard to determine what
these are when listing transform configs, which can lead to
uncertainty about whether some minor update changed the
permissions.

This change adds information about the permissions to the
config listings.

If the last update was from a user then the permissions used
are the roles they had at the time, and a list of these roles
is added to the new authorization field of the transform config
listing.

If the last update was using an API key then the permissions
of that API key are used for subsequent transform operations,
and the new authorization field of the transform config listing
shows the API key name and ID.

If a service account did the last update then the service
account name is added to the new authorization field of the transform
config listing.

Transforms remember permissions from their time of creation
or most recent update. It can be quite hard to determine what
these are when listing transform configs, which can lead to
uncertainty about whether some minor update changed the
permissions.

This change adds information about the permissions to the
config listings.

If the last update was from a user then the permissions used
are the roles they had at the time, and a list of these roles
is added to the new `run_as` field of the transform config
listing.

If the last update was using an API key then the permissions
of that API key are used for subsequent transform operations,
and the new `run_as` field of the transform config listing
shows the API key name and ID.

If a service account did the last update then the service
account name is added to the new `run_as` field of the transform
config listing.
@droberts195 droberts195 added >enhancement :ml/Transform Transform v8.4.0 cloud-deploy Publish cloud docker image for Cloud-First-Testing labels Jun 9, 2022
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

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

@droberts195 droberts195 removed the cloud-deploy Publish cloud docker image for Cloud-First-Testing label Jun 15, 2022
@droberts195 droberts195 added the :Security/Security Security issues without another label label Jun 15, 2022
@droberts195 droberts195 marked this pull request as ready for review June 15, 2022 16:19
@elasticmachine elasticmachine added Team:Security Meta label for security team Team:ML Meta label for the ML team labels Jun 15, 2022
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-security (Team:Security)

@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/ml-core (Team:ML)

if (authKey == null) {
return;
}
builder.startObject(RoleDescriptor.Fields.RUN_AS.getPreferredName());
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it's better to find a different name for this field. run_as in RoleDescriptor as a specific meaning for user impersonation, which includes an authenticating user and an effective user. It is also a previlege that user can configure when creating a role. For the use case here, maybe something more generic like authorization could be better?

Copy link
Copy Markdown
Contributor

@dimitris-athanasiou dimitris-athanasiou left a comment

Choose a reason for hiding this comment

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

Just a question about renaming the method. Also, please make sure to update the PR + the changelog message to reflect the added field is authorization and not run_as.

Copy link
Copy Markdown
Contributor

@dimitris-athanasiou dimitris-athanasiou left a comment

Choose a reason for hiding this comment

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

LGTM

@droberts195 droberts195 changed the title [TRANSFORM] Add run-as info to transform config listings [TRANSFORM] Add authorization info to transform config listings Jun 20, 2022
@droberts195 droberts195 merged commit 80ad8cc into elastic:master Jun 20, 2022
@droberts195 droberts195 deleted the include_run_as_info branch June 20, 2022 12:44
droberts195 added a commit that referenced this pull request Jun 20, 2022
Subject authenticationSubject = AuthenticationContextSerializer.decode(authKey).getEffectiveSubject();
switch (authenticationSubject.getType()) {
case USER -> builder.array(User.Fields.ROLES.getPreferredName(), authenticationSubject.getUser().roles());
case API_KEY -> {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Don't we need break; statements after each case?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

No, break statements aren’t used with the modern switch syntax that uses -> for each case. Only the expression immediately after the -> is executed for each case (which can be a code block in braces).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ok, thanks. I missed that this is the new syntax.

Copy link
Copy Markdown

@przemekwitek przemekwitek left a comment

Choose a reason for hiding this comment

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

LGTM

Subject authenticationSubject = AuthenticationContextSerializer.decode(authKey).getEffectiveSubject();
switch (authenticationSubject.getType()) {
case USER -> builder.array(User.Fields.ROLES.getPreferredName(), authenticationSubject.getUser().roles());
case API_KEY -> {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ok, thanks. I missed that this is the new syntax.

droberts195 added a commit that referenced this pull request Jun 27, 2022
…88052)

In #87570 authorization info was added to transform listings.
Authorization information was also implicitly added to transform
update responses. However, YAML tests were not added to either.
This PR adds some assertions to the YAML tests for getting transforms
and transform updates to prove that the responses really do include
authorization information.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>enhancement :ml/Transform Transform :Security/Security Security issues without another label Team:ML Meta label for the ML team Team:Security Meta label for security team v8.4.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants