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

HLRest: model role and privileges #35128

Merged
merged 34 commits into from
Nov 11, 2018
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
706b736
ClusterPrivilege
albertzaharovits Oct 24, 2018
21bc280
IndexPrivilege
albertzaharovits Oct 24, 2018
9e174eb
Index Privileges
albertzaharovits Oct 26, 2018
64a91e2
ApplicationResourcePrivilege WIP
albertzaharovits Oct 29, 2018
d98232b
Merge branch 'master' into hlrc_put_role
albertzaharovits Oct 30, 2018
aabbc5f
IndicesPrivileges is neat!
albertzaharovits Oct 30, 2018
9883726
Oh my!
albertzaharovits Oct 30, 2018
ba34f65
Proper manage app priv
albertzaharovits Oct 30, 2018
e03a05b
Bare RoleDescriptor
albertzaharovits Oct 30, 2018
a7b176b
Slightly less bare RoleDescriptor
albertzaharovits Oct 31, 2018
701f85e
All Collections are Lists, well..
albertzaharovits Oct 31, 2018
523f108
WIP :((
albertzaharovits Oct 31, 2018
ee37904
Fields are Set
albertzaharovits Oct 31, 2018
36f7be5
Role entity
albertzaharovits Oct 31, 2018
395e29c
Merge branch 'master' into hlrc_put_role
albertzaharovits Nov 1, 2018
4604127
Follow Tim's advice
albertzaharovits Nov 4, 2018
beaa373
Follow Tim's advice
albertzaharovits Nov 5, 2018
05ccbd0
Removed ManageApplicationsPrivilege
albertzaharovits Nov 5, 2018
4c339e9
Rename to GlobalPrivileges
albertzaharovits Nov 5, 2018
9b75cdb
public ApplicationResourcePrivileges constructor
albertzaharovits Nov 5, 2018
4839eaa
Merge branch 'master' into hlrc_put_role
albertzaharovits Nov 6, 2018
192295a
Merge branch 'master' into hlrc_put_role
albertzaharovits Nov 7, 2018
d83113d
Nits + javadoc
albertzaharovits Nov 7, 2018
1d9f874
ApplicationResourcePrivilegesTests
albertzaharovits Nov 7, 2018
d4170f2
WIP
albertzaharovits Nov 7, 2018
9869124
More tests
albertzaharovits Nov 8, 2018
3ead50c
More docs
albertzaharovits Nov 8, 2018
b68a3cf
WIP
albertzaharovits Nov 8, 2018
769a0e1
Merge branch 'master' into hlrc_put_role
albertzaharovits Nov 8, 2018
24351b4
Done!
albertzaharovits Nov 8, 2018
36d3203
Merge branch 'master' into hlrc_put_role
albertzaharovits Nov 9, 2018
20f14f7
Javadocs and privilegesByCategoryMap as member
albertzaharovits Nov 9, 2018
59014c6
Merge branch 'master' into hlrc_put_role
albertzaharovits Nov 9, 2018
7fd629a
Merge branch 'master' into hlrc_put_role
albertzaharovits Nov 9, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

package org.elasticsearch.client.security.user.privileges;

import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.xcontent.ConstructingObjectParser;
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.XContentType;

import java.io.IOException;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.Objects;
import java.util.Set;

import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg;

/**
* Represents privileges over resources that are scoped under an application.
* The application, resources and privileges are completely managed by the
* client and can be "arbitrary" string identifiers. Elasticsearch is not
* concerned by any resources under an application scope.
*/
public final class ApplicationResourcePrivileges implements ToXContentObject {

private static final ParseField APPLICATION = new ParseField("application");
private static final ParseField PRIVILEGES = new ParseField("privileges");
private static final ParseField RESOURCES = new ParseField("resources");

@SuppressWarnings("unchecked")
static final ConstructingObjectParser<ApplicationResourcePrivileges, Void> PARSER = new ConstructingObjectParser<>(
"application_privileges", false, constructorObjects -> {
// Don't ignore unknown fields. It is dangerous if the object we parse is also
// part of a request that we build later on, and the fields that we now ignore will
// end up being implicitly set to null in that request.
int i = 0;
final String application = (String) constructorObjects[i++];
final Collection<String> privileges = (Collection<String>) constructorObjects[i++];
final Collection<String> resources = (Collection<String>) constructorObjects[i];
return new ApplicationResourcePrivileges(application, privileges, resources);
});

static {
PARSER.declareString(constructorArg(), APPLICATION);
PARSER.declareStringArray(constructorArg(), PRIVILEGES);
PARSER.declareStringArray(constructorArg(), RESOURCES);
}

private final String application;
private final Set<String> privileges;
private final Set<String> resources;

/**
* Constructs privileges for resources under an application scope.
*
* @param application
* The application name. This identifier is completely under the
* clients control.
* @param privileges
* The privileges names. Cannot be null or empty. Privilege
* identifiers are completely under the clients control.
* @param resources
* The resources names. Cannot be null or empty. Resource identifiers
* are completely under the clients control.
*/
public ApplicationResourcePrivileges(String application, Collection<String> privileges, Collection<String> resources) {
if (Strings.isNullOrEmpty(application)) {
throw new IllegalArgumentException("application privileges must have an application name");
}
if (null == privileges || privileges.isEmpty()) {
throw new IllegalArgumentException("application privileges must define at least one privilege");
}
if (null == resources || resources.isEmpty()) {
throw new IllegalArgumentException("application privileges must refer to at least one resource");
}
this.application = application;
this.privileges = Collections.unmodifiableSet(new HashSet<>(privileges));
this.resources = Collections.unmodifiableSet(new HashSet<>(resources));
}

public String getApplication() {
return application;
}

public Set<String> getResources() {
return this.resources;
}

public Set<String> getPrivileges() {
return this.privileges;
}

@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (o == null || this.getClass() != o.getClass()) {
return false;
}
ApplicationResourcePrivileges that = (ApplicationResourcePrivileges) o;
return application.equals(that.application)
&& privileges.equals(that.privileges)
&& resources.equals(that.resources);
}

@Override
public int hashCode() {
return Objects.hash(application, privileges, resources);
}

@Override
public String toString() {
try {
return XContentHelper.toXContent(this, XContentType.JSON, true).utf8ToString();
} catch (IOException e) {
throw new RuntimeException("Unexpected", e);
}
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();
builder.field(APPLICATION.getPreferredName(), application);
builder.field(PRIVILEGES.getPreferredName(), privileges);
builder.field(RESOURCES.getPreferredName(), resources);
return builder.endObject();
}

public static ApplicationResourcePrivileges fromXContent(XContentParser parser) {
return PARSER.apply(parser, null);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

package org.elasticsearch.client.security.user.privileges;

import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.xcontent.ConstructingObjectParser;
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;

import java.io.IOException;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Collectors;

import static org.elasticsearch.common.xcontent.ConstructingObjectParser.optionalConstructorArg;

/**
* Represents global privileges. "Global Privilege" is a mantra for granular
* privileges over applications. {@code ApplicationResourcePrivileges} model
* application privileges over resources. This models user privileges over
* applications. Every client is responsible to manage the applications as well
* as the privileges for them.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is kind of true for now, but not really the intent.

"global" privileges are just cluster privileges that
(a) Have some sort of "parameters" (that is, they aren't just a plain string). So my expectation is that if we werer to implement it "manage_watcher" is a cluster privilege but { "manage_watches_by_name": ["albert-*"] } is a global privilege.
(a) In order to accomodate the above, have a custom JSON format.

The only ones that exist "manage applications", so the Javadoc is true, but I don't think it explains it to someone trying to use the API.

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 tried to keep the balance between abstract and pragmatic documentation.
I leaned towards documenting the pragmatic, present behavior. I know it discounts the technical marvel behind, but if I had used the scope, category or conditional jargon, it was my impression that javadocs will lose their benefit to the user. We can always update the javadoc when we stretch the abstraction later on. Yet, the counter argument is that we should aim for more stability in the docs as well, because these are client classes.

Noted. I'll take them to one more round of polish.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, my thought was that the Javadoc is probably there to explain to users the point of "GlobalPrivileges".
The short answer is, in this release they probably don't care (as long as they handle round-trip GET + PUT correct) because it's just "application privilege management", but in the future it will do more.

*/
public final class GlobalPrivileges implements ToXContentObject {

static final ParseField APPLICATION = new ParseField("application");

@SuppressWarnings("unchecked")
static final ConstructingObjectParser<GlobalPrivileges, Void> PARSER = new ConstructingObjectParser<>("global_application_privileges",
false, constructorObjects -> {
// ignore_unknown_fields is irrelevant here anyway, but let's keep it to false
// because this conveys strictness (woop woop)
return new GlobalPrivileges((Collection<GlobalScopedPrivilege>) constructorObjects[0]);
});

static {
PARSER.declareNamedObjects(optionalConstructorArg(), (p, c, n) -> GlobalScopedPrivilege.fromXContent(n, p), APPLICATION);
}

private final Set<? extends GlobalScopedPrivilege> applicationPrivileges;

/**
* Constructs global privileges by bundling the set of application privileges.
*
* @param applicationPrivileges
* The privileges over applications.
*/
public GlobalPrivileges(Collection<? extends GlobalScopedPrivilege> applicationPrivileges) {
if (applicationPrivileges == null || applicationPrivileges.isEmpty()) {
throw new IllegalArgumentException("Application privileges cannot be empty or null");
}
this.applicationPrivileges = Collections.unmodifiableSet(new HashSet<>(Objects.requireNonNull(applicationPrivileges)));
final Set<String> allScopes = this.applicationPrivileges.stream().map(p -> p.getScope()).collect(Collectors.toSet());
if (allScopes.size() != this.applicationPrivileges.size()) {
throw new IllegalArgumentException(
"Application privileges have the same scope but the privileges differ. Only one privilege for any one scope is allowed.");
}
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();
builder.startObject(APPLICATION.getPreferredName());
for (final GlobalScopedPrivilege privilege : applicationPrivileges) {
builder.field(privilege.getScope(), privilege.getRaw());
}
builder.endObject();
return builder.endObject();
}

public static GlobalPrivileges fromXContent(XContentParser parser) {
return PARSER.apply(parser, null);
}

public Set<? extends GlobalScopedPrivilege> getPrivileges() {
return applicationPrivileges;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure this is quite right, but it might be - I guess it depends where you think you'd take it next.
It's obviously tricky since we're not 100% sure where we will go with "global" privileges, but let's assume the next thing we do is restict the ability to modify some cluster settings.

To do that we might end up with JSON like:

"global" : {
   "application": { "manage": { "applications": [ "*" ] } },
   "settings" : { "update" : { "names": [ "logger.*" ] } }
}

In that case, what does getPrivileges return? Is it a set like:

[ ManageApplicationPrivilege("*") , UpdateClusterSettingsPrivilege("logger.*") ]

Or do we have separate getApplicationPrivileges() and getSettingsPrivileges() (probably not).

Or should this return a Map<String, Set<GlobalScopedPrivilege>> where the keys are "application" and "settings" ?

If we think we only need a single Set<...> getPrivileges() method, then maybe we only need 1 field too, and it should simply be privileges not applicationPrivileges.

I don't have a strong opinion (really!), but I'd like us to have an idea of how to evolve this when we add more global 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.

Thank you for the comprehensive explanation!

Really my though was that there's no way we can future proof this, the object will have to evolve in lock step with the JSON response (the parser is static, we cannot ignore unknwon fields, etc..). So I didn't gave it much though about how this "evolution" would look like. It would be breaking anyway, so who cares, design it for today.

But! I got a small idea from your Map<String, Set<GlobalScopedPrivilege>>, stay tuned!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've decided to use a Set. I think a map exposes internal implementation, the client should not be informed that we broken them down by category.

}

@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (o == null || this.getClass() != o.getClass()) {
return false;
}
final GlobalPrivileges that = (GlobalPrivileges) o;
return applicationPrivileges.equals(that.applicationPrivileges);
}

@Override
public int hashCode() {
return Objects.hash(applicationPrivileges);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

package org.elasticsearch.client.security.user.privileges;

import org.elasticsearch.common.xcontent.XContentParser;

import java.io.IOException;
import java.util.Collections;
import java.util.Map;
import java.util.Objects;

/**
* Represents generic global application privileges that can be scoped for each
* application. The privilege definition, as well as the scope identifier, are
* outside of the Elasticsearch jurisdiction.
*/
public class GlobalScopedPrivilege {

private final String scope;
albertzaharovits marked this conversation as resolved.
Show resolved Hide resolved
private final Map<String, Object> privilege;
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if it would help to include the "category" ("application", and in theory "settings") in this class too.
We can wait add it in the future if we need it, but I think we will, depending on how GlobalPrivileges ends up looking.

Copy link
Contributor Author

@albertzaharovits albertzaharovits Nov 8, 2018

Choose a reason for hiding this comment

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

Hmmm, I cannot think of a reason why not to do that 👍
More so, since I am now leaning towards having a set in GlobalPrivileges because a map would be exposing unnecessary implementation details.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was a very good idea. I know this is how they are modeled on the server, but I didn't give it much interest at that time.


/**
* Constructs privileges under some "scope". The "scope" is commonly an
* "application" name but there is really no constraint over this identifier
* from Elasticsearch's POV. The privilege definition is also out of
* Elasticsearch's control.
*
* @param scope
* The scope of the privilege.
* @param privilege
* The privilege definition. This is out of the Elasticsearch's
* control.
*/
public GlobalScopedPrivilege(String scope, Map<String, Object> privilege) {
this.scope = Objects.requireNonNull(scope);
if (privilege == null || privilege.isEmpty()) {
throw new IllegalArgumentException("Privileges cannot be empty or null");
}
this.privilege = Collections.unmodifiableMap(privilege);
}

public String getScope() {
return scope;
}

public Map<String, Object> getRaw() {
return privilege;
}

public static GlobalScopedPrivilege fromXContent(String scope, XContentParser parser) throws IOException {
// parser is still placed on the field name, advance to next token (field value)
assert parser.currentToken().equals(XContentParser.Token.FIELD_NAME);
parser.nextToken();
return new GlobalScopedPrivilege(scope, parser.map());
}

@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (o == null || this.getClass() != o.getClass()) {
return false;
}
final GlobalScopedPrivilege that = (GlobalScopedPrivilege) o;
return scope.equals(that.scope) && privilege.equals(that.privilege);
}

@Override
public int hashCode() {
return Objects.hash(scope, privilege);
}

}
Loading