Skip to content

Commit

Permalink
User Profile - Rename access to labels for request, response and mapp…
Browse files Browse the repository at this point in the history
…ings (#85797)

It is agreed that the "access" field should be renamed to the more
generic "labels". Data stored in this field are meant to be application
specific (similar to "data" but searchable) and ES does not attempt to
understand its content or purpose. Hence the more generic name is a
better fit.
  • Loading branch information
ywangd committed Apr 13, 2022
1 parent df5cb3d commit 1269378
Show file tree
Hide file tree
Showing 16 changed files with 58 additions and 52 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ users with information that is extracted from the user's authentication object,
including `username`, `full_name`, `roles`, and the authentication realm.

When updating a profile document, the API enables the document if it was
disabled. Any updates do not change existing content for either the `access` or
disabled. Any updates do not change existing content for either the `labels` or
`data` fields.

This API is intended only for use by applications (such as {kib}) that need to
Expand Down Expand Up @@ -116,7 +116,7 @@ The API returns the following response:
"email": "jacknich@example.com",
"active": true
},
"access": {},
"labels": {},
"data": {},
"_doc": {
"_primary_term": 88,
Expand Down
4 changes: 2 additions & 2 deletions x-pack/docs/en/rest-api/security/get-user-profile.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ The API returns the following response for a `uid` matching `u_kd2JMqwUQwSCCOxMv
"email": "jacknich@example.com",
"active": true
},
"access": {},
"labels": {},
"data": {}, <1>
"_doc": {
"_primary_term": 1,
Expand Down Expand Up @@ -120,7 +120,7 @@ GET /_security/profile/u_kd2JMqwUQwSCCOxMv7M1vw?data=app1.key1
"email": "jacknich@example.com",
"active": true
},
"access": {},
"labels": {},
"data": {
"app1": {
"key1": "value1"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,12 @@ referenced in the request.
[[security-api-update-user-profile-data-desc]]
==== {api-description-title}

The update user profile API updates the `access` and `data` fields of an
The update user profile API updates the `labels` and `data` fields of an
existing user profile document with JSON objects. New keys and their values are
added to the profile document, and conflicting keys are replaced by data that's
included in the request.

For both `access` and `data`, content is namespaced by the top-level fields.
For both `labels` and `data`, content is namespaced by the top-level fields.
The `update_profile_data` global privilege grants privileges for updating only
the allowed namespaces.

Expand All @@ -57,10 +57,10 @@ include::{es-repo-dir}/rest-api/common-parms.asciidoc[tag=refresh]
[[security-api-update-user-profile-data-request-body]]
==== {api-request-body-title}

`access`::
`labels`::
(Required*, object)
Searchable data that you want to associate with the user profile.
This field supports a nested data structure. Within the `access` object,
This field supports a nested data structure. Within the `labels` object,
top-level keys cannot begin with an underscore (`_`) or contain a period (`.`).

`data`::
Expand Down Expand Up @@ -97,7 +97,7 @@ The following request updates a profile document for a `uid` matching
----
POST /_security/profile/u_kd2JMqwUQwSCCOxMv7M1vw/_data
{
"access": {
"labels": {
"app1": {
"tag": "prod"
}
Expand All @@ -117,7 +117,7 @@ You can update the profile data to replace some keys and add new keys:
----
POST /_security/profile/u_kd2JMqwUQwSCCOxMv7M1vw/_data
{
"access": {
"labels": {
"app1": {
"tag": "dev"
}
Expand Down Expand Up @@ -150,7 +150,7 @@ If you run the request again, the consolidated profile data is returned:
"email": "jacknich@example.com",
"active": true
},
"access": {
"labels": {
"app1": {
"tag": "dev"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public record Profile(
boolean enabled,
long lastSynchronized,
ProfileUser user,
Map<String, Object> access,
Map<String, Object> labels,
Map<String, Object> applicationData,
VersionControl versionControl
) implements Writeable, ToXContentObject {
Expand Down Expand Up @@ -129,7 +129,7 @@ public void innerToXContent(XContentBuilder builder, Params params) throws IOExc
builder.field("enabled", enabled);
builder.field("last_synchronized", lastSynchronized);
user.toXContent(builder, params);
builder.field("access", access);
builder.field("labels", labels);
builder.field("data", applicationData);
}

Expand All @@ -139,7 +139,7 @@ public void writeTo(StreamOutput out) throws IOException {
out.writeBoolean(enabled);
out.writeLong(lastSynchronized);
user.writeTo(out);
out.writeGenericMap(access);
out.writeGenericMap(labels);
out.writeGenericMap(applicationData);
versionControl.writeTo(out);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
{
builder.field("uid", profile.uid());
profile.user().toXContent(builder, params);
builder.field("access", profile.access());
builder.field("labels", profile.labels());
builder.field("data", profile.applicationData());
}
builder.endObject();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,22 +25,22 @@
public class UpdateProfileDataRequest extends ActionRequest {

private final String uid;
private final Map<String, Object> access;
private final Map<String, Object> labels;
private final Map<String, Object> data;
private final long ifPrimaryTerm;
private final long ifSeqNo;
private final RefreshPolicy refreshPolicy;

public UpdateProfileDataRequest(
String uid,
Map<String, Object> access,
Map<String, Object> labels,
Map<String, Object> data,
long ifPrimaryTerm,
long ifSeqNo,
RefreshPolicy refreshPolicy
) {
this.uid = Objects.requireNonNull(uid, "profile uid must not be null");
this.access = access != null ? access : Map.of();
this.labels = labels != null ? labels : Map.of();
this.data = data != null ? data : Map.of();
this.ifPrimaryTerm = ifPrimaryTerm;
this.ifSeqNo = ifSeqNo;
Expand All @@ -50,7 +50,7 @@ public UpdateProfileDataRequest(
public UpdateProfileDataRequest(StreamInput in) throws IOException {
super(in);
this.uid = in.readString();
this.access = in.readMap();
this.labels = in.readMap();
this.data = in.readMap();
this.ifPrimaryTerm = in.readLong();
this.ifSeqNo = in.readLong();
Expand All @@ -61,8 +61,8 @@ public String getUid() {
return uid;
}

public Map<String, Object> getAccess() {
return access;
public Map<String, Object> getLabels() {
return labels;
}

public Map<String, Object> getData() {
Expand All @@ -82,7 +82,7 @@ public RefreshPolicy getRefreshPolicy() {
}

public Set<String> getApplicationNames() {
final Set<String> names = new HashSet<>(access.keySet());
final Set<String> names = new HashSet<>(labels.keySet());
names.addAll(data.keySet());
return Set.copyOf(names);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public class ProfileIT extends ESRestTestCase {
"active": true
},
"last_synchronized": %s,
"access": {
"labels": {
},
"application_data": {
"app1": { "name": "app1" },
Expand Down Expand Up @@ -127,7 +127,7 @@ public void testUpdateProfileData() throws IOException {
final Request updateProfileRequest1 = new Request(randomFrom("PUT", "POST"), "_security/profile/" + uid + "/_data");
updateProfileRequest1.setJsonEntity("""
{
"access": {
"labels": {
"app1": { "tags": [ "prod", "east" ] }
},
"data": {
Expand All @@ -137,7 +137,7 @@ public void testUpdateProfileData() throws IOException {
assertOK(adminClient().performRequest(updateProfileRequest1));

final Map<String, Object> profileMap1 = doGetProfile(uid, "app1");
assertThat(castToMap(profileMap1.get("access")), equalTo(Map.of("app1", Map.of("tags", List.of("prod", "east")))));
assertThat(castToMap(profileMap1.get("labels")), equalTo(Map.of("app1", Map.of("tags", List.of("prod", "east")))));
assertThat(castToMap(profileMap1.get("data")), equalTo(Map.of("app1", Map.of("theme", "default"))));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public void testProfileIndexAutoCreation() {
"properties"
);

assertThat(userProfileProperties.keySet(), hasItems("uid", "enabled", "last_synchronized", "user", "access", "application_data"));
assertThat(userProfileProperties.keySet(), hasItems("uid", "enabled", "last_synchronized", "user", "labels", "application_data"));
}

public void testActivateProfile() {
Expand Down Expand Up @@ -123,7 +123,7 @@ public void testActivateProfile() {
assertThat(profile3.user().email(), equalTo(RAC_USER_NAME + "@example.com"));
assertThat(profile3.user().fullName(), nullValue());
assertThat(profile3.user().roles(), contains(RAC_ROLE));
assertThat(profile3.access(), anEmptyMap());
assertThat(profile3.labels(), anEmptyMap());
// Get by ID immediately should get the same document and content as the response to activate
assertThat(getProfile(profile3.uid(), Set.of()), equalTo(profile3));

Expand All @@ -132,7 +132,7 @@ public void testActivateProfile() {
.setDoc("""
{
"user_profile": {
"access": {
"labels": {
"my_app": {
"tag": "prod"
}
Expand All @@ -151,7 +151,7 @@ public void testActivateProfile() {
// Above manual update should be successful
final Profile profile4 = getProfile(profile3.uid(), Set.of("my_app"));
assertThat(profile4.uid(), equalTo(profile3.uid()));
assertThat(profile4.access(), equalTo(Map.of("my_app", Map.of("tag", "prod"))));
assertThat(profile4.labels(), equalTo(Map.of("my_app", Map.of("tag", "prod"))));
assertThat(profile4.applicationData(), equalTo(Map.of("my_app", Map.of("theme", "default"))));

// Update native rac user
Expand All @@ -168,8 +168,8 @@ public void testActivateProfile() {
assertThat(profile5.user().email(), nullValue());
assertThat(profile5.user().fullName(), equalTo("Native RAC User"));
assertThat(profile5.user().roles(), containsInAnyOrder(RAC_ROLE, "superuser"));
// Re-activate should not change access
assertThat(profile5.access(), equalTo(Map.of("my_app", Map.of("tag", "prod"))));
// Re-activate should not change labels
assertThat(profile5.labels(), equalTo(Map.of("my_app", Map.of("tag", "prod"))));
// Get by ID immediately should get the same document and content as the response to activate
assertThat(getProfile(profile5.uid(), Set.of()), equalTo(profile5));
// Re-activate should not change application data
Expand All @@ -192,7 +192,7 @@ public void testUpdateProfileData() {
final Profile profile2 = getProfile(profile1.uid(), Set.of("app1", "app2"));

assertThat(profile2.uid(), equalTo(profile1.uid()));
assertThat(profile2.access(), equalTo(Map.of("app1", List.of("tab1", "tab2"))));
assertThat(profile2.labels(), equalTo(Map.of("app1", List.of("tab1", "tab2"))));
assertThat(profile2.applicationData(), equalTo(Map.of("app1", Map.of("name", "app1", "type", "app"))));

// Update again should be incremental
Expand All @@ -208,7 +208,7 @@ public void testUpdateProfileData() {

final Profile profile3 = getProfile(profile1.uid(), Set.of("app1", "app2"));
assertThat(profile3.uid(), equalTo(profile1.uid()));
assertThat(profile3.access(), equalTo(profile2.access()));
assertThat(profile3.labels(), equalTo(profile2.labels()));
assertThat(
profile3.applicationData(),
equalTo(Map.of("app1", Map.of("name", "app1_take2", "type", "app", "active", false), "app2", Map.of("name", "app2")))
Expand All @@ -217,7 +217,7 @@ public void testUpdateProfileData() {
// Activate profile again should not affect the data section
doActivateProfile(RAC_USER_NAME, TEST_PASSWORD_SECURE_STRING);
final Profile profile4 = getProfile(profile1.uid(), Set.of("app1", "app2"));
assertThat(profile4.access(), equalTo(profile3.access()));
assertThat(profile4.labels(), equalTo(profile3.labels()));
assertThat(profile4.applicationData(), equalTo(profile3.applicationData()));

// Update non-existent profile should throw error
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1403,7 +1403,7 @@ LogEntryBuilder withRequestBody(UpdateProfileDataRequest updateProfileDataReques
XContentBuilder builder = JsonXContent.contentBuilder().humanReadable(true);
builder.startObject()
.field("uid", updateProfileDataRequest.getUid())
.field("access", updateProfileDataRequest.getAccess())
.field("labels", updateProfileDataRequest.getLabels())
.field("data", updateProfileDataRequest.getData())
.endObject();
logEntry.with(PUT_CONFIG_FIELD_NAME, Strings.toString(builder));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public record ProfileDocument(
boolean enabled,
long lastSynchronized,
ProfileDocumentUser user,
Map<String, Object> access,
Map<String, Object> labels,
BytesReference applicationData
) implements ToXContentObject {

Expand Down Expand Up @@ -80,10 +80,10 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
builder.field("last_synchronized", lastSynchronized);
user.toXContent(builder, params);

if (params.paramAsBoolean("include_access", true) && access != null) {
builder.field("access", access);
if (params.paramAsBoolean("include_labels", true) && labels != null) {
builder.field("labels", labels);
} else {
builder.startObject("access").endObject();
builder.startObject("labels").endObject();
}
if (params.paramAsBoolean("include_data", true) && applicationData != null) {
builder.field("application_data", applicationData);
Expand Down Expand Up @@ -199,7 +199,7 @@ public static ProfileDocument fromXContent(XContentParser parser) {
PROFILE_DOC_PARSER.declareBoolean(constructorArg(), new ParseField("enabled"));
PROFILE_DOC_PARSER.declareLong(constructorArg(), new ParseField("last_synchronized"));
PROFILE_DOC_PARSER.declareObject(constructorArg(), (p, c) -> PROFILE_DOC_USER_PARSER.parse(p, null), new ParseField("user"));
PROFILE_DOC_PARSER.declareObject(constructorArg(), (p, c) -> p.map(), new ParseField("access"));
PROFILE_DOC_PARSER.declareObject(constructorArg(), (p, c) -> p.map(), new ParseField("labels"));
ObjectParserHelper.declareRawObject(PROFILE_DOC_PARSER, constructorArg(), new ParseField("application_data"));

PARSER.declareObject(constructorArg(), (p, c) -> PROFILE_DOC_PARSER.parse(p, null), new ParseField("user_profile"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,8 @@ public void updateProfileData(UpdateProfileDataRequest request, ActionListener<A
builder.field("user_profile");
builder.startObject();
{
if (false == request.getAccess().isEmpty()) {
builder.field("access", request.getAccess());
if (false == request.getLabels().isEmpty()) {
builder.field("labels", request.getLabels());
}
if (false == request.getData().isEmpty()) {
builder.field("application_data", request.getData());
Expand Down Expand Up @@ -581,8 +581,8 @@ private static XContentBuilder wrapProfileDocumentWithoutApplicationData(Profile
builder.field(
"user_profile",
profileDocument,
// NOT including the access and data in the update request so they will not be changed
new ToXContent.MapParams(Map.of("include_access", Boolean.FALSE.toString(), "include_data", Boolean.FALSE.toString()))
// NOT including the labels and data in the update request so they will not be changed
new ToXContent.MapParams(Map.of("include_labels", Boolean.FALSE.toString(), "include_data", Boolean.FALSE.toString()))
);
builder.endObject();
return builder;
Expand Down Expand Up @@ -620,7 +620,7 @@ private static ProfileDocument updateWithSubject(ProfileDocument doc, Subject su
subjectUser.fullName(),
subjectUser.enabled()
),
doc.access(),
doc.labels(),
doc.applicationData()
);
}
Expand All @@ -645,7 +645,7 @@ Profile toProfile(Set<String> dataKeys) {
doc.enabled(),
doc.lastSynchronized(),
doc.user().toProfileUser(),
doc.access(),
doc.labels(),
applicationData,
new Profile.VersionControl(primaryTerm, seqNo)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public class RestUpdateProfileDataAction extends SecurityBaseRestHandler {
);

static {
PARSER.declareObject(optionalConstructorArg(), (p, c) -> p.map(), new ParseField("access"));
PARSER.declareObject(optionalConstructorArg(), (p, c) -> p.map(), new ParseField("labels"));
PARSER.declareObject(optionalConstructorArg(), (p, c) -> p.map(), new ParseField("data"));
}

Expand Down Expand Up @@ -64,7 +64,7 @@ protected RestChannelConsumer innerPrepareRequest(RestRequest request, NodeClien

final UpdateProfileDataRequest updateProfileDataRequest = new UpdateProfileDataRequest(
uid,
payload.access,
payload.labels,
payload.data,
ifPrimaryTerm,
ifSeqNo,
Expand All @@ -74,5 +74,5 @@ protected RestChannelConsumer innerPrepareRequest(RestRequest request, NodeClien
return channel -> client.execute(UpdateProfileDataAction.INSTANCE, updateProfileDataRequest, new RestToXContentListener<>(channel));
}

record Payload(Map<String, Object> access, Map<String, Object> data) {}
record Payload(Map<String, Object> labels, Map<String, Object> data) {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -848,7 +848,7 @@ private XContentBuilder getProfileIndexMappings() {
builder.endObject();

// Searchable application specific data
builder.startObject("access");
builder.startObject("labels");
builder.field("type", "flattened");
builder.endObject();

Expand Down

0 comments on commit 1269378

Please sign in to comment.