Skip to content

Commit 636f17b

Browse files
mauromolmauromol
committed
Polish AuthnRequest constructor and Auth.login overloadings
Input parameters, which drive how the AuthnRequest should be generated, are now encapsulated into an AuthnRequestParams object which is far more extensible and help to polish the API of AuthnRequest constructors and Auth.login, which are crowded with overloadings and cannot be easily extended. Also, these input params are passed to postProcessXml: in this way, if an extension plans to use a specialization of AuthnRequestParams, it can access such a specialized instance also within a proper postProcessXml overriding.
1 parent 38f3480 commit 636f17b

File tree

6 files changed

+337
-112
lines changed

6 files changed

+337
-112
lines changed

‎core/src/main/java/com/onelogin/saml2/authn/AuthnRequest.java‎

Lines changed: 51 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -42,26 +42,6 @@ public class AuthnRequest {
4242
*/
4343
private final Saml2Settings settings;
4444

45-
/**
46-
* When true the AuthNRequest will set the ForceAuthn='true'
47-
*/
48-
private final boolean forceAuthn;
49-
50-
/**
51-
* When true the AuthNRequest will set the IsPassive='true'
52-
*/
53-
private final boolean isPassive;
54-
55-
/**
56-
* When true the AuthNReuqest will set a nameIdPolicy
57-
*/
58-
private final boolean setNameIdPolicy;
59-
60-
/**
61-
* Indicates to the IdP the subject that should be authenticated
62-
*/
63-
private final String nameIdValueReq;
64-
6545
/**
6646
* Time stamp that indicates when the AuthNRequest was created
6747
*/
@@ -72,56 +52,74 @@ public class AuthnRequest {
7252
*
7353
* @param settings
7454
* OneLogin_Saml2_Settings
55+
* @see #AuthnRequest(Saml2Settings, AuthnRequestParams)
7556
*/
7657
public AuthnRequest(Saml2Settings settings) {
77-
this(settings, false, false, true);
58+
this(settings, new AuthnRequestParams(false, false, true));
7859
}
7960

8061
/**
8162
* Constructs the AuthnRequest object.
8263
*
8364
* @param settings
84-
* OneLogin_Saml2_Settings
65+
* OneLogin_Saml2_Settings
8566
* @param forceAuthn
86-
* When true the AuthNReuqest will set the ForceAuthn='true'
67+
* When true the AuthNReuqest will set the ForceAuthn='true'
8768
* @param isPassive
88-
* When true the AuthNReuqest will set the IsPassive='true'
69+
* When true the AuthNReuqest will set the IsPassive='true'
8970
* @param setNameIdPolicy
90-
* When true the AuthNReuqest will set a nameIdPolicy
71+
* When true the AuthNReuqest will set a nameIdPolicy
9172
* @param nameIdValueReq
92-
* Indicates to the IdP the subject that should be authenticated
73+
* Indicates to the IdP the subject that should be authenticated
74+
* @deprecated use {@link #AuthnRequest(Saml2Settings, AuthnRequestParams)} with
75+
* {@link AuthnRequestParams#AuthnRequestParams(boolean, boolean, boolean, String)}
76+
* instead
9377
*/
78+
@Deprecated
9479
public AuthnRequest(Saml2Settings settings, boolean forceAuthn, boolean isPassive, boolean setNameIdPolicy, String nameIdValueReq) {
95-
this.id = Util.generateUniqueID(settings.getUniqueIDPrefix());
96-
issueInstant = Calendar.getInstance();
97-
this.isPassive = isPassive;
98-
this.settings = settings;
99-
this.forceAuthn = forceAuthn;
100-
this.setNameIdPolicy = setNameIdPolicy;
101-
this.nameIdValueReq = nameIdValueReq;
102-
103-
StrSubstitutor substitutor = generateSubstitutor(settings);
104-
authnRequestString = postProcessXml(substitutor.replace(getAuthnRequestTemplate()), settings);
105-
LOGGER.debug("AuthNRequest --> " + authnRequestString);
80+
this(settings, new AuthnRequestParams(forceAuthn, isPassive, setNameIdPolicy, nameIdValueReq));
10681
}
107-
82+
10883
/**
10984
* Constructs the AuthnRequest object.
11085
*
11186
* @param settings
112-
* OneLogin_Saml2_Settings
87+
* OneLogin_Saml2_Settings
11388
* @param forceAuthn
114-
* When true the AuthNReuqest will set the ForceAuthn='true'
89+
* When true the AuthNReuqest will set the ForceAuthn='true'
11590
* @param isPassive
116-
* When true the AuthNReuqest will set the IsPassive='true'
91+
* When true the AuthNReuqest will set the IsPassive='true'
11792
* @param setNameIdPolicy
118-
* When true the AuthNReuqest will set a nameIdPolicy
93+
* When true the AuthNReuqest will set a nameIdPolicy
94+
* @deprecated use {@link #AuthnRequest(Saml2Settings, AuthnRequestParams)} with
95+
* {@link AuthnRequestParams#AuthnRequestParams(boolean, boolean, boolean)}
96+
* instead
11997
*/
98+
@Deprecated
12099
public AuthnRequest(Saml2Settings settings, boolean forceAuthn, boolean isPassive, boolean setNameIdPolicy) {
121100
this(settings, forceAuthn, isPassive, setNameIdPolicy, null);
122101
}
123102

124103
/**
104+
* Constructs the AuthnRequest object.
105+
*
106+
* @param settings
107+
* OneLogin_Saml2_Settings
108+
* @param params
109+
* a set of authentication request input parameters that shape the
110+
* request to create
111+
*/
112+
public AuthnRequest(Saml2Settings settings, AuthnRequestParams params) {
113+
this.id = Util.generateUniqueID(settings.getUniqueIDPrefix());
114+
issueInstant = Calendar.getInstance();
115+
this.settings = settings;
116+
117+
StrSubstitutor substitutor = generateSubstitutor(params, settings);
118+
authnRequestString = postProcessXml(substitutor.replace(getAuthnRequestTemplate()), params, settings);
119+
LOGGER.debug("AuthNRequest --> " + authnRequestString);
120+
}
121+
122+
/*
125123
* Allows for an extension class to post-process the AuthnRequest XML generated
126124
* for this request, in order to customize the result.
127125
* <p>
@@ -132,15 +130,17 @@ public AuthnRequest(Saml2Settings settings, boolean forceAuthn, boolean isPassiv
132130
* @param authnRequestXml
133131
* the XML produced for this AuthnRequest by the standard
134132
* implementation provided by {@link AuthnRequest}
133+
* @param params
134+
* the authentication request input parameters
135135
* @param settings
136136
* the settings
137137
* @return the post-processed XML for this AuthnRequest, which will then be
138138
* returned by any call to {@link #getAuthnRequestXml()}
139139
*/
140-
protected String postProcessXml(final String authnRequestXml, final Saml2Settings settings) {
140+
protected String postProcessXml(final String authnRequestXml, final AuthnRequestParams params, final Saml2Settings settings) {
141141
return authnRequestXml;
142142
}
143-
143+
144144
/**
145145
* @return the base64 encoded unsigned AuthnRequest (deflated or not)
146146
*
@@ -181,22 +181,24 @@ public String getAuthnRequestXml() {
181181
/**
182182
* Substitutes AuthnRequest variables within a string by values.
183183
*
184+
* @param params
185+
* the authentication request input parameters
184186
* @param settings
185187
* Saml2Settings object. Setting data
186188
*
187189
* @return the StrSubstitutor object of the AuthnRequest
188190
*/
189-
private StrSubstitutor generateSubstitutor(Saml2Settings settings) {
191+
private StrSubstitutor generateSubstitutor(AuthnRequestParams params, Saml2Settings settings) {
190192

191193
Map<String, String> valueMap = new HashMap<String, String>();
192194

193195
String forceAuthnStr = "";
194-
if (forceAuthn) {
196+
if (params.isForceAuthn()) {
195197
forceAuthnStr = " ForceAuthn=\"true\"";
196198
}
197199

198200
String isPassiveStr = "";
199-
if (isPassive) {
201+
if (params.isPassive()) {
200202
isPassiveStr = " IsPassive=\"true\"";
201203
}
202204

@@ -211,6 +213,7 @@ private StrSubstitutor generateSubstitutor(Saml2Settings settings) {
211213
valueMap.put("destinationStr", destinationStr);
212214

213215
String subjectStr = "";
216+
String nameIdValueReq = params.getNameIdValueReq();
214217
if (nameIdValueReq != null && !nameIdValueReq.isEmpty()) {
215218
String nameIDFormat = settings.getSpNameIDFormat();
216219
subjectStr = "<saml:Subject>";
@@ -221,7 +224,7 @@ private StrSubstitutor generateSubstitutor(Saml2Settings settings) {
221224
valueMap.put("subjectStr", subjectStr);
222225

223226
String nameIDPolicyStr = "";
224-
if (setNameIdPolicy) {
227+
if (params.isSetNameIdPolicy()) {
225228
String nameIDPolicyFormat = settings.getSpNameIDFormat();
226229
if (settings.getWantNameIdEncrypted()) {
227230
nameIDPolicyFormat = Constants.NAMEID_ENCRYPTED;

‎core/src/main/java/com/onelogin/saml2/authn/AuthnRequestParams.java‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
package com.onelogin.saml2.authn;
2+
3+
/**
4+
* Input parameters for a SAML 2 authentication request.
5+
*/
6+
public class AuthnRequestParams {
7+
8+
/**
9+
* When true the AuthNRequest will set the ForceAuthn='true'
10+
*/
11+
private final boolean forceAuthn;
12+
/**
13+
* When true the AuthNRequest will set the IsPassive='true'
14+
*/
15+
private final boolean isPassive;
16+
/**
17+
* When true the AuthNReuqest will set a nameIdPolicy
18+
*/
19+
private final boolean setNameIdPolicy;
20+
/**
21+
* Indicates to the IdP the subject that should be authenticated
22+
*/
23+
private final String nameIdValueReq;
24+
25+
/**
26+
* Create a set of authentication request input parameters.
27+
*
28+
* @param forceAuthn
29+
* whether the <code>ForceAuthn</code> attribute should be set to
30+
* <code>true</code>
31+
* @param isPassive
32+
* whether the <code>isPassive</code> attribute should be set to
33+
* <code>true</code>
34+
* @param setNameIdPolicy
35+
* whether a <code>NameIDPolicy</code> should be set
36+
*/
37+
public AuthnRequestParams(boolean forceAuthn, boolean isPassive, boolean setNameIdPolicy) {
38+
this(forceAuthn, isPassive, setNameIdPolicy, null);
39+
}
40+
41+
/**
42+
* Create a set of authentication request input parameters.
43+
*
44+
* @param forceAuthn
45+
* whether the <code>ForceAuthn</code> attribute should be set to
46+
* <code>true</code>
47+
* @param isPassive
48+
* whether the <code>isPassive</code> attribute should be set to
49+
* <code>true</code>
50+
* @param setNameIdPolicy
51+
* whether a <code>NameIDPolicy</code> should be set
52+
* @param nameIdValueReq
53+
* the subject that should be authenticated
54+
*/
55+
public AuthnRequestParams(boolean forceAuthn, boolean isPassive, boolean setNameIdPolicy, String nameIdValueReq) {
56+
this.forceAuthn = forceAuthn;
57+
this.isPassive = isPassive;
58+
this.setNameIdPolicy = setNameIdPolicy;
59+
this.nameIdValueReq = nameIdValueReq;
60+
}
61+
62+
/**
63+
* Create a set of authentication request input parameters, by copying them from
64+
* another set.
65+
*
66+
* @param source
67+
* the source set of authentication request input parameters
68+
*/
69+
protected AuthnRequestParams(AuthnRequestParams source) {
70+
this.forceAuthn = source.isForceAuthn();
71+
this.isPassive = source.isPassive();
72+
this.setNameIdPolicy = source.isSetNameIdPolicy();
73+
this.nameIdValueReq = source.getNameIdValueReq();
74+
}
75+
76+
/**
77+
* @return whether the <code>ForceAuthn</code> attribute should be set to
78+
* <code>true</code>
79+
*/
80+
protected boolean isForceAuthn() {
81+
return forceAuthn;
82+
}
83+
84+
/**
85+
* @return whether the <code>isPassive</code> attribute should be set to
86+
* <code>true</code>
87+
*/
88+
protected boolean isPassive() {
89+
return isPassive;
90+
}
91+
92+
/**
93+
* @return whether a <code>NameIDPolicy</code> should be set
94+
*/
95+
protected boolean isSetNameIdPolicy() {
96+
return setNameIdPolicy;
97+
}
98+
99+
/**
100+
* @return the subject that should be authenticated
101+
*/
102+
protected String getNameIdValueReq() {
103+
return nameIdValueReq;
104+
}
105+
}

‎core/src/main/java/com/onelogin/saml2/settings/Metadata.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
package com.onelogin.saml2.settings;
22

3-
import static com.onelogin.saml2.util.Util.toXml;
4-
53
import java.net.URL;
64
import java.util.Arrays;
75
import java.util.Calendar;

‎core/src/test/java/com/onelogin/saml2/test/authn/AuthnRequestTest.java‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import org.junit.Test;
1818

1919
import com.onelogin.saml2.authn.AuthnRequest;
20+
import com.onelogin.saml2.authn.AuthnRequestParams;
2021
import com.onelogin.saml2.settings.Saml2Settings;
2122
import com.onelogin.saml2.settings.SettingsBuilder;
2223
import com.onelogin.saml2.util.Util;
@@ -164,13 +165,13 @@ public void testForceAuthN() throws Exception {
164165
assertThat(authnRequestStr, containsString("<samlp:AuthnRequest"));
165166
assertThat(authnRequestStr, not(containsString("ForceAuthn=\"true\"")));
166167

167-
authnRequest = new AuthnRequest(settings, false, false, false);
168+
authnRequest = new AuthnRequest(settings, new AuthnRequestParams(false, false, false));
168169
authnRequestStringBase64 = authnRequest.getEncodedAuthnRequest();
169170
authnRequestStr = Util.base64decodedInflated(authnRequestStringBase64);
170171
assertThat(authnRequestStr, containsString("<samlp:AuthnRequest"));
171172
assertThat(authnRequestStr, not(containsString("ForceAuthn=\"true\"")));
172173

173-
authnRequest = new AuthnRequest(settings, true, false, false);
174+
authnRequest = new AuthnRequest(settings, new AuthnRequestParams(true, false, false));
174175
authnRequestStringBase64 = authnRequest.getEncodedAuthnRequest();
175176
authnRequestStr = Util.base64decodedInflated(authnRequestStringBase64);
176177
assertThat(authnRequestStr, containsString("<samlp:AuthnRequest"));
@@ -195,13 +196,13 @@ public void testIsPassive() throws Exception {
195196
assertThat(authnRequestStr, containsString("<samlp:AuthnRequest"));
196197
assertThat(authnRequestStr, not(containsString("IsPassive=\"true\"")));
197198

198-
authnRequest = new AuthnRequest(settings, false, false, false);
199+
authnRequest = new AuthnRequest(settings, new AuthnRequestParams(false, false, false));
199200
authnRequestStringBase64 = authnRequest.getEncodedAuthnRequest();
200201
authnRequestStr = Util.base64decodedInflated(authnRequestStringBase64);
201202
assertThat(authnRequestStr, containsString("<samlp:AuthnRequest"));
202203
assertThat(authnRequestStr, not(containsString("IsPassive=\"true\"")));
203204

204-
authnRequest = new AuthnRequest(settings, false, true, false);
205+
authnRequest = new AuthnRequest(settings, new AuthnRequestParams(false, true, false));
205206
authnRequestStringBase64 = authnRequest.getEncodedAuthnRequest();
206207
authnRequestStr = Util.base64decodedInflated(authnRequestStringBase64);
207208
assertThat(authnRequestStr, containsString("<samlp:AuthnRequest"));
@@ -227,13 +228,13 @@ public void testNameIDPolicy() throws Exception {
227228
assertThat(authnRequestStr, containsString("<samlp:NameIDPolicy"));
228229
assertThat(authnRequestStr, containsString("Format=\"urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified\""));
229230

230-
authnRequest = new AuthnRequest(settings, false, false, false);
231+
authnRequest = new AuthnRequest(settings, new AuthnRequestParams(false, false, false));
231232
authnRequestStringBase64 = authnRequest.getEncodedAuthnRequest();
232233
authnRequestStr = Util.base64decodedInflated(authnRequestStringBase64);
233234
assertThat(authnRequestStr, containsString("<samlp:AuthnRequest"));
234235
assertThat(authnRequestStr, not(containsString("<samlp:NameIDPolicy")));
235236

236-
authnRequest = new AuthnRequest(settings, false, false, true);
237+
authnRequest = new AuthnRequest(settings, new AuthnRequestParams(false, false, true));
237238
authnRequestStringBase64 = authnRequest.getEncodedAuthnRequest();
238239
authnRequestStr = Util.base64decodedInflated(authnRequestStringBase64);
239240
assertThat(authnRequestStr, containsString("<samlp:AuthnRequest"));
@@ -365,7 +366,7 @@ public void testSubject() throws Exception {
365366
assertThat(authnRequestStr, containsString("<samlp:AuthnRequest"));
366367
assertThat(authnRequestStr, not(containsString("<saml:Subject")));
367368

368-
authnRequest = new AuthnRequest(settings, false, false, false, "testuser@example.com");
369+
authnRequest = new AuthnRequest(settings, new AuthnRequestParams(false, false, false, "testuser@example.com"));
369370
authnRequestStringBase64 = authnRequest.getEncodedAuthnRequest();
370371
authnRequestStr = Util.base64decodedInflated(authnRequestStringBase64);
371372
assertThat(authnRequestStr, containsString("<samlp:AuthnRequest"));
@@ -374,7 +375,7 @@ public void testSubject() throws Exception {
374375
assertThat(authnRequestStr, containsString("<saml:SubjectConfirmation Method=\"urn:oasis:names:tc:SAML:2.0:cm:bearer\">"));
375376

376377
settings = new SettingsBuilder().fromFile("config/config.emailaddressformat.properties").build();
377-
authnRequest = new AuthnRequest(settings, false, false, false, "testuser@example.com");
378+
authnRequest = new AuthnRequest(settings, new AuthnRequestParams(false, false, false, "testuser@example.com"));
378379
authnRequestStringBase64 = authnRequest.getEncodedAuthnRequest();
379380
authnRequestStr = Util.base64decodedInflated(authnRequestStringBase64);
380381
assertThat(authnRequestStr, containsString("<samlp:AuthnRequest"));
@@ -523,8 +524,8 @@ public void testPostProcessXml() throws Exception {
523524
Saml2Settings settings = new SettingsBuilder().fromFile("config/config.min.properties").build();
524525
AuthnRequest authnRequest = new AuthnRequest(settings) {
525526
@Override
526-
protected String postProcessXml(String authRequestXml, Saml2Settings sett) {
527-
assertEquals(authRequestXml, super.postProcessXml(authRequestXml, sett));
527+
protected String postProcessXml(String authRequestXml, AuthnRequestParams params, Saml2Settings sett) {
528+
assertEquals(authRequestXml, super.postProcessXml(authRequestXml, params, sett));
528529
assertSame(settings, sett);
529530
return "changed";
530531
}

0 commit comments

Comments
 (0)