Skip to content

Commit e7e4209

Browse files
jrivardjrivard
committed
fix issue 615 - improve sortyness and null handling for challenge policy values
1 parent 634cd3c commit e7e4209

File tree

5 files changed

+51
-18
lines changed

5 files changed

+51
-18
lines changed

‎server/src/main/java/password/pwm/config/value/ChallengeValue.java

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,13 @@
2121
package password.pwm.config.value;
2222

2323
import com.google.gson.reflect.TypeToken;
24+
import password.pwm.PwmConstants;
2425
import password.pwm.config.PwmSetting;
2526
import password.pwm.config.stored.XmlOutputProcessData;
2627
import password.pwm.config.value.data.ChallengeItemConfiguration;
28+
import password.pwm.util.i18n.LocaleComparators;
2729
import password.pwm.util.i18n.LocaleHelper;
30+
import password.pwm.util.java.JavaHelper;
2831
import password.pwm.util.java.JsonUtil;
2932
import password.pwm.util.java.XmlElement;
3033
import password.pwm.util.java.XmlFactory;
@@ -33,10 +36,13 @@
3336

3437
import java.util.ArrayList;
3538
import java.util.Collections;
39+
import java.util.Comparator;
40+
import java.util.LinkedHashMap;
3641
import java.util.List;
3742
import java.util.Locale;
3843
import java.util.Map;
3944
import java.util.TreeMap;
45+
import java.util.stream.Collectors;
4046

4147
public class ChallengeValue extends AbstractValue implements StoredValue
4248
{
@@ -47,7 +53,15 @@ public class ChallengeValue extends AbstractValue implements StoredValue
4753

4854
ChallengeValue( final Map<String, List<ChallengeItemConfiguration>> values )
4955
{
50-
this.values = values == null ? Collections.emptyMap() : Collections.unmodifiableMap( values );
56+
// values created via js, so need to be defensive and strip all nulls.
57+
final Comparator<String> comparator = LocaleComparators.stringLocaleComparator( PwmConstants.DEFAULT_LOCALE, LocaleComparators.Flag.DefaultFirst );
58+
final Map<String, List<ChallengeItemConfiguration>> sortedMap = new TreeMap<>( comparator );
59+
sortedMap.putAll( JavaHelper.stripNulls( values ).entrySet().stream()
60+
.collect( Collectors.toUnmodifiableMap(
61+
Map.Entry::getKey,
62+
v -> JavaHelper.stripNulls( v.getValue() )
63+
) ) );
64+
this.values = Collections.unmodifiableMap( sortedMap );
5165
}
5266

5367
public static StoredValueFactory factory( )
@@ -69,7 +83,7 @@ public ChallengeValue fromJson( final String input )
6983
{
7084
}
7185
);
72-
srcMap = srcMap == null ? Collections.emptyMap() : new TreeMap<>(
86+
srcMap = srcMap == null ? Collections.emptyMap() : new LinkedHashMap<>(
7387
srcMap );
7488
return new ChallengeValue( Collections.unmodifiableMap( srcMap ) );
7589
}
@@ -83,7 +97,7 @@ public ChallengeValue fromXmlElement(
8397
)
8498
{
8599
final List<XmlElement> valueElements = settingElement.getChildren( "value" );
86-
final Map<String, List<ChallengeItemConfiguration>> values = new TreeMap<>();
100+
final Map<String, List<ChallengeItemConfiguration>> values = new LinkedHashMap<>();
87101
final boolean oldStyle = "LOCALIZED_STRING_ARRAY".equals( settingElement.getAttributeValue( "syntax" ) );
88102
for ( final XmlElement loopValueElement : valueElements )
89103
{
@@ -92,7 +106,7 @@ public ChallengeValue fromXmlElement(
92106
final String value = loopValueElement.getText();
93107
if ( !values.containsKey( localeString ) )
94108
{
95-
values.put( localeString, new ArrayList<ChallengeItemConfiguration>() );
109+
values.put( localeString, new ArrayList<>() );
96110
}
97111
final ChallengeItemConfiguration challengeItemBean;
98112
if ( oldStyle )
@@ -140,7 +154,7 @@ public List<XmlElement> toXmlValues( final String valueElementName, final XmlOut
140154
@Override
141155
public Map<String, List<ChallengeItemConfiguration>> toNativeObject( )
142156
{
143-
return Collections.unmodifiableMap( values );
157+
return values;
144158
}
145159

146160
@Override

‎server/src/main/java/password/pwm/http/servlet/ClientApiServlet.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import password.pwm.svc.stats.EpsStatistic;
4848
import password.pwm.svc.stats.Statistic;
4949
import password.pwm.svc.stats.StatisticsManager;
50+
import password.pwm.util.i18n.LocaleComparators;
5051
import password.pwm.util.i18n.LocaleHelper;
5152
import password.pwm.util.java.StringUtil;
5253
import password.pwm.util.java.TimeDuration;
@@ -404,7 +405,7 @@ private static Map<String, Object> makeClientData(
404405
final Map<String, String> localeFlags = new LinkedHashMap<>();
405406

406407
final List<Locale> knownLocales = new ArrayList<>( pwmApplication.getConfig().getKnownLocales() );
407-
knownLocales.sort( LocaleHelper.localeComparator( PwmConstants.DEFAULT_LOCALE ) );
408+
knownLocales.sort( LocaleComparators.localeComparator( ) );
408409

409410
for ( final Locale locale : knownLocales )
410411
{

‎server/src/main/java/password/pwm/util/i18n/LocaleHelper.java

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@
4141
import java.util.ArrayList;
4242
import java.util.Collection;
4343
import java.util.Collections;
44-
import java.util.Comparator;
4544
import java.util.Iterator;
4645
import java.util.LinkedHashMap;
4746
import java.util.List;
@@ -58,6 +57,7 @@ public class LocaleHelper
5857
{
5958
private static final PwmLogger LOGGER = PwmLogger.forClass( LocaleHelper.class );
6059

60+
6161
public enum TextDirection
6262
{
6363
rtl,
@@ -440,16 +440,6 @@ public static String getBrowserLocaleString( final Locale locale )
440440
: locale.toString().replace( "_", "-" );
441441
}
442442

443-
public static Comparator<Locale> localeComparator( final Locale comparisonLocale )
444-
{
445-
return ( o1, o2 ) ->
446-
{
447-
final String name1 = o1.getDisplayName( comparisonLocale );
448-
final String name2 = o2.getDisplayName( comparisonLocale );
449-
return name1.compareToIgnoreCase( name2 );
450-
};
451-
}
452-
453443
public static List<Locale> highLightedLocales()
454444
{
455445
final List<String> strValues = PwmConstants.HIGHLIGHT_LOCALES;

‎server/src/main/java/password/pwm/util/java/JavaHelper.java

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858
import java.util.EnumSet;
5959
import java.util.Enumeration;
6060
import java.util.LinkedHashSet;
61+
import java.util.List;
6162
import java.util.Map;
6263
import java.util.Objects;
6364
import java.util.Optional;
@@ -723,5 +724,28 @@ public static <K extends Enum<K>, V> EnumMap<K, V> copiedEnumMap( final Map<K, V
723724
}
724725
return returnMap;
725726
}
726-
727+
728+
public static <K, V> Map<K, V> stripNulls( final Map<K, V> input )
729+
{
730+
if ( input == null )
731+
{
732+
return Collections.emptyMap();
733+
}
734+
735+
return input.entrySet().stream()
736+
.filter( e -> e.getKey() != null && e.getValue() != null )
737+
.collect( Collectors.toUnmodifiableMap( Map.Entry::getKey, Map.Entry::getValue ) );
738+
}
739+
740+
public static <V> List<V> stripNulls( final List<V> input )
741+
{
742+
if ( input == null )
743+
{
744+
return Collections.emptyList();
745+
}
746+
747+
return input.stream()
748+
.filter( Objects::nonNull )
749+
.collect( Collectors.toUnmodifiableList() );
750+
}
727751
}

‎webapp/src/main/webapp/public/resources/js/configeditor-settings-challenges.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,7 +365,11 @@ ChallengeSettingHandler.toggleAdminDefinedRow = function(toggleElement,inputID,k
365365
};
366366

367367
ChallengeSettingHandler.deleteRow = function(keyName, localeKey, rowName) {
368+
var questionText = PWM_VAR['clientSettingCache'][keyName][localeKey][rowName]['text'];
369+
var adminDefined = PWM_VAR['clientSettingCache'][keyName][localeKey][rowName]['adminDefined'];
370+
var output = (adminDefined ? 'the question "' + questionText + '"': 'the [User Defined] question?');
368371
PWM_MAIN.showConfirmDialog({
372+
text: 'Are you sure you want to remove ' + output,
369373
okAction:function(){
370374
PWM_MAIN.showWaitDialog({loadFunction:function(){
371375
delete PWM_VAR['clientSettingCache'][keyName][localeKey][rowName];

0 commit comments

Comments
 (0)