Skip to content
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

HLRC: Implement get-user-privileges API #36292

Merged
merged 11 commits into from
Dec 12, 2018

Conversation

tvernum
Copy link
Contributor

@tvernum tvernum commented Dec 6, 2018

This adds the _xpack/security/user/_privileges API to the High
Level Rest Client.

This also makes some changes to the Java model for the Role APIs
in order to better accommodate the GetUserPrivileges API

This adds the _xpack/security/user/_privileges API to the High
Level Rest Client.

This also makes some changes to the Java model for the Role APIs
in order to better accommodate the GetPrivileges API
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features

@tvernum
Copy link
Contributor Author

tvernum commented Dec 6, 2018

Ping: @elastic/es-security

Copy link
Member

@jkakavas jkakavas left a comment

Choose a reason for hiding this comment

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

I left some minor comments, I'll let Albert take a closer look at the Privilege changes first ❤️

private final Request request;

private GetUserPrivilegesRequest() {
request = new Request(HttpGet.METHOD_NAME, "/_xpack/security/user/_privileges");
Copy link
Member

Choose a reason for hiding this comment

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

Since you're out tomorrow, #36293 will be merged before hand so /_xpack/security/user/_privileges -> /_security/user/_privileges

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We'll see who merges first :) Probably you, but I can't update this just yet.

Copy link
Member

Choose a reason for hiding this comment

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

I was TOO optimistic.

[id="{upid}-{api}"]
=== Get User Privileges API

include::../execution-no-req.asciidoc[]
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for this. We could/should change to include it in get-certificates.asciidoc and authenticate.asciidoc too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes - I plan to do so (but that doesn't have to get done by FF, so I haven't tackled it yet)

Copy link
Contributor

@albertzaharovits albertzaharovits left a comment

Choose a reason for hiding this comment

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

LGTM 👍 (except for minor nits)

@@ -307,6 +309,29 @@ public void hasPrivilegesAsync(HasPrivilegesRequest request, RequestOptions opti
HasPrivilegesResponse::fromXContent, listener, emptySet());
}

/**
* Retrieve the set of effective privileges held by the current user.
* See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/security-api-get-user-privileges.html">
Copy link
Contributor

Choose a reason for hiding this comment

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

elastic.co/guide/en/elasticsearch/reference/current/security-api-get -user-privileges.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a different API.
What happened to my get-user-privileges API docs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, so git is telling me that I never wrote API docs for get-user-privileges (see #33928). It's weird, I'm sure I did write some...

I'll add it to my To Do, and remove this javadoc link until they exist.

@@ -83,17 +83,17 @@

private final String name;
private final Set<String> clusterPrivileges;
private final @Nullable GlobalPrivileges globalApplicationPrivileges;
private final @Nullable GlobalPrivileges globalPrivileges;
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@tvernum tvernum merged commit 143f151 into elastic:master Dec 12, 2018
tvernum added a commit that referenced this pull request Dec 17, 2018
This adds the _security/user/_privileges API to the High
Level Rest Client.

This also makes some changes to the Java model for the Role APIs
in order to better accommodate the GetPrivileges API
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants