Skip to content

Commit

Permalink
Treat roles as a SortedSet (#55201)
Browse files Browse the repository at this point in the history
The Saml SP document stored the role mapping in a Set, but this made
the order in XContent inconsistent. This switched it to use a TreeSet.

Resolves: #54733
tvernum authored Apr 16, 2020

Verified

This commit was signed with the committer’s verified signature. The key has expired.
zakkak Foivos Zakkak
1 parent a219694 commit 5920d1b
Showing 2 changed files with 6 additions and 5 deletions.
Original file line number Diff line number Diff line change
@@ -38,6 +38,8 @@
import java.util.List;
import java.util.Objects;
import java.util.Set;
import java.util.SortedSet;
import java.util.TreeSet;
import java.util.function.BiConsumer;
import java.util.stream.Collectors;

@@ -52,14 +54,15 @@ public class SamlServiceProviderDocument implements ToXContentObject, Writeable

public static class Privileges {
public String resource;
public Set<String> rolePatterns = Set.of();
// we use a sorted set so that the order is consistent in XContent APIs
public SortedSet<String> rolePatterns = new TreeSet<>();

public void setResource(String resource) {
this.resource = resource;
}

public void setRolePatterns(Collection<String> rolePatterns) {
this.rolePatterns = Set.copyOf(rolePatterns);
this.rolePatterns = new TreeSet<>(rolePatterns);
}

@Override
@@ -258,7 +261,7 @@ public SamlServiceProviderDocument(StreamInput in) throws IOException {
authenticationExpiryMillis = in.readOptionalVLong();

privileges.resource = in.readString();
privileges.rolePatterns = in.readSet(StreamInput::readString);
privileges.rolePatterns = new TreeSet<>(in.readSet(StreamInput::readString));

attributeNames.principal = in.readString();
attributeNames.email = in.readOptionalString();
Original file line number Diff line number Diff line change
@@ -68,7 +68,6 @@ public void testXContentRoundTripWithMinimalFields() throws Exception {
assertThat(assertXContentRoundTrip(doc2), equalTo(doc1));
}

@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/54733")
public void testXContentRoundTripWithAllFields() throws Exception {
final SamlServiceProviderDocument doc1 = createFullDocument();
final SamlServiceProviderDocument doc2 = assertXContentRoundTrip(doc1);
@@ -81,7 +80,6 @@ public void testStreamRoundTripWithMinimalFields() throws Exception {
assertThat(assertSerializationRoundTrip(doc2), equalTo(doc1));
}

@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/54733")
public void testStreamRoundTripWithAllFields() throws Exception {
final SamlServiceProviderDocument doc1 = createFullDocument();
final SamlServiceProviderDocument doc2 = assertXContentRoundTrip(doc1);

0 comments on commit 5920d1b

Please sign in to comment.