Skip to content

Commit

Permalink
Merge pull request #401 from jtnord/remove-deprecated-ctor
Browse files Browse the repository at this point in the history
Remove deprecated constructor
  • Loading branch information
jtnord authored Sep 19, 2024
2 parents 347c3b8 + a83c698 commit cfe74a7
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 138 deletions.
74 changes: 0 additions & 74 deletions src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@
import jenkins.model.Jenkins;
import jenkins.security.ApiTokenProperty;
import jenkins.security.SecurityListener;
import org.apache.commons.lang.StringUtils;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.DoNotUse;
import org.kohsuke.accmod.restrictions.NoExternalUse;
Expand Down Expand Up @@ -279,79 +278,6 @@ public static enum TokenAuthMethod {
private static final JmesPath<Object> JMESPATH = new JcfRuntime(
new RuntimeConfiguration.Builder().withSilentTypeErrors(true).build());

/**
* @deprecated retained for backwards binary compatibility.
*/
@Deprecated
public OicSecurityRealm(
String clientId,
String clientSecret,
String wellKnownOpenIDConfigurationUrl,
String tokenServerUrl,
String jwksServerUrl,
String tokenAuthMethod,
String authorizationServerUrl,
String userInfoServerUrl,
String userNameField,
String tokenFieldToCheckKey,
String tokenFieldToCheckValue,
String fullNameFieldName,
String emailFieldName,
String scopes,
String groupsFieldName,
Boolean disableSslVerification,
Boolean logoutFromOpenidProvider,
String endSessionEndpoint,
String postLogoutRedirectUrl,
Boolean escapeHatchEnabled,
String escapeHatchUsername,
String escapeHatchSecret,
String escapeHatchGroup,
String automanualconfigure)
throws IOException {
this.disableSslVerification = Util.fixNull(disableSslVerification, Boolean.FALSE);
this.httpTransport = constructHttpTransport(this.disableSslVerification);

this.clientId = clientId;
this.clientSecret = clientSecret != null && !clientSecret.toLowerCase().equals(NO_SECRET)
? Secret.fromString(clientSecret)
: null;
// last known config
this.authorizationServerUrl = authorizationServerUrl;
this.tokenServerUrl = tokenServerUrl;
this.tokenAuthMethod =
TokenAuthMethod.valueOf(StringUtils.defaultIfBlank(tokenAuthMethod, "client_secret_post"));
this.userInfoServerUrl = userInfoServerUrl;
this.jwksServerUrl = jwksServerUrl;
this.scopes = scopes;
this.endSessionEndpoint = endSessionEndpoint;

if ("auto".equals(automanualconfigure)
|| (Util.fixNull(automanualconfigure).isEmpty()
&& !Util.fixNull(wellKnownOpenIDConfigurationUrl).isEmpty())) {
this.automanualconfigure = "auto";
this.wellKnownOpenIDConfigurationUrl = Util.fixEmptyAndTrim(wellKnownOpenIDConfigurationUrl);
} else {
this.automanualconfigure = "manual";
this.wellKnownOpenIDConfigurationUrl = null; // Remove the autoconfig URL
}

this.setTokenFieldToCheckKey(tokenFieldToCheckKey);
this.setTokenFieldToCheckValue(tokenFieldToCheckValue);
this.setUserNameField(userNameField);
this.setFullNameFieldName(fullNameFieldName);
this.setEmailFieldName(emailFieldName);
this.setGroupsFieldName(groupsFieldName);
this.logoutFromOpenidProvider = Util.fixNull(logoutFromOpenidProvider, Boolean.TRUE);
this.postLogoutRedirectUrl = postLogoutRedirectUrl;
this.escapeHatchEnabled = Util.fixNull(escapeHatchEnabled, Boolean.FALSE);
this.escapeHatchUsername = Util.fixEmptyAndTrim(escapeHatchUsername);
this.setEscapeHatchSecret(Secret.fromString(escapeHatchSecret));
this.escapeHatchGroup = Util.fixEmptyAndTrim(escapeHatchGroup);
// hack to avoid rewriting lots of tests :-)
readResolve();
}

@DataBoundConstructor
public OicSecurityRealm(
String clientId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@

import static org.hamcrest.CoreMatchers.instanceOf;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.jenkinsci.plugins.oic.TestRealm.AUTO_CONFIG_FIELD;
import static org.jenkinsci.plugins.oic.TestRealm.MANUAL_CONFIG_FIELD;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
Expand Down Expand Up @@ -46,8 +44,7 @@ public void testOicSecurityRealmDescriptorImplManual() throws Exception {
"Client secret is required.", descriptor.doCheckClientSecret("").getMessage());
assertEquals(FormValidation.ok(), descriptor.doCheckClientSecret("password"));

TestRealm realm = new TestRealm(
new TestRealm.Builder("http://ignored.test/").WithAutomanualconfigure(MANUAL_CONFIG_FIELD));
TestRealm realm = new TestRealm(new TestRealm.Builder("http://ignored.test/").WithAutomanualconfigure(false));
jenkins.setSecurityRealm(realm);

descriptor = (DescriptorImpl) realm.getDescriptor();
Expand All @@ -74,8 +71,7 @@ public void testOicSecurityRealmDescriptorImplAuto() throws Exception {
"Client secret is required.", descriptor.doCheckClientSecret("").getMessage());
assertEquals(FormValidation.ok(), descriptor.doCheckClientSecret("password"));

TestRealm realm =
new TestRealm(new TestRealm.Builder("http://ignored.test/").WithAutomanualconfigure(AUTO_CONFIG_FIELD));
TestRealm realm = new TestRealm(new TestRealm.Builder("http://ignored.test/").WithAutomanualconfigure(true));
jenkins.setSecurityRealm(realm);

descriptor = (DescriptorImpl) jenkins.getSecurityRealm().getDescriptor();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,6 @@ public void testShouldSetNullClientSecretWhenSecretIsNull() throws IOException {
assertEquals("none", Secret.toString(realm.getClientSecret()));
}

@Test
public void testShouldSetNullClientSecretWhenSecretIsNone() throws IOException {
TestRealm realm = new TestRealm.Builder(wireMockRule)
.WithMinimalDefaults().WithClient("id with none secret", "NoNE").build();
assertEquals("none", Secret.toString(realm.getClientSecret()));
}

@Test
public void testGetValidRedirectUrl() throws IOException {
// root url is http://localhost:????/jenkins/
Expand Down
32 changes: 15 additions & 17 deletions src/test/java/org/jenkinsci/plugins/oic/PluginTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,9 @@
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.is;
import static org.jenkinsci.plugins.oic.TestRealm.AUTO_CONFIG_FIELD;
import static org.jenkinsci.plugins.oic.TestRealm.EMAIL_FIELD;
import static org.jenkinsci.plugins.oic.TestRealm.FULL_NAME_FIELD;
import static org.jenkinsci.plugins.oic.TestRealm.GROUPS_FIELD;
import static org.jenkinsci.plugins.oic.TestRealm.MANUAL_CONFIG_FIELD;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
Expand Down Expand Up @@ -275,7 +273,7 @@ public void testLoginWithAutoConfiguration() throws Exception {
mockTokenReturnsIdTokenWithGroup();
mockUserInfoWithTestGroups();
configureWellKnown(null, null);
jenkins.setSecurityRealm(new TestRealm(wireMockRule, null, EMAIL_FIELD, GROUPS_FIELD, AUTO_CONFIG_FIELD));
jenkins.setSecurityRealm(new TestRealm(wireMockRule, null, EMAIL_FIELD, GROUPS_FIELD, true));
assertAnonymous();
browseLoginPage();
var user = assertTestUser();
Expand All @@ -289,10 +287,10 @@ public void testLoginWithAutoConfiguration_WithNoScope() throws Exception {
mockTokenReturnsIdTokenWithValues(setUpKeyValuesNoGroup());
mockUserInfoWithGroups(null);
configureWellKnown(null, null);
jenkins.setSecurityRealm(new TestRealm(wireMockRule, null, EMAIL_FIELD, GROUPS_FIELD, AUTO_CONFIG_FIELD));
jenkins.setSecurityRealm(new TestRealm(wireMockRule, null, EMAIL_FIELD, GROUPS_FIELD, true));
assertAnonymous();
configureWellKnown(null, null);
jenkins.setSecurityRealm(new TestRealm(wireMockRule, null, EMAIL_FIELD, GROUPS_FIELD, AUTO_CONFIG_FIELD));
jenkins.setSecurityRealm(new TestRealm(wireMockRule, null, EMAIL_FIELD, GROUPS_FIELD, true));
assertAnonymous();
browseLoginPage();
var user = assertTestUser();
Expand All @@ -304,7 +302,7 @@ public void testLoginWithAutoConfiguration_WithNoScope() throws Exception {
public void testConfigurationWithAutoConfiguration_withScopeOverride() throws Exception {
configureWellKnown(null, List.of("openid", "profile", "scope1", "scope2", "scope3"));
TestRealm oicsr = new TestRealm.Builder(wireMockRule)
.WithMinimalDefaults().WithAutomanualconfigure("auto").build();
.WithMinimalDefaults().WithAutomanualconfigure(true).build();
jenkins.setSecurityRealm(oicsr);
assertEquals(
"All scopes of WellKnown should be used",
Expand All @@ -326,7 +324,7 @@ public void testConfigurationWithAutoConfiguration_withScopeOverride() throws Ex
public void testConfigurationWithAutoConfiguration_withRefreshToken() throws Exception {
configureWellKnown(null, null, "authorization_code", "refresh_token");
TestRealm oicsr = new TestRealm.Builder(wireMockRule)
.WithMinimalDefaults().WithAutomanualconfigure("auto").build();
.WithMinimalDefaults().WithAutomanualconfigure(true).build();
jenkins.setSecurityRealm(oicsr);
assertTrue(
"Refresh token should be enabled",
Expand All @@ -337,7 +335,7 @@ public void testConfigurationWithAutoConfiguration_withRefreshToken() throws Exc
public void testRefreshToken_validAndExtendedToken() throws Exception {
mockAuthorizationRedirectsToFinishLogin();
configureWellKnown(null, null, "authorization_code", "refresh_token");
jenkins.setSecurityRealm(new TestRealm(wireMockRule, null, EMAIL_FIELD, GROUPS_FIELD, AUTO_CONFIG_FIELD));
jenkins.setSecurityRealm(new TestRealm(wireMockRule, null, EMAIL_FIELD, GROUPS_FIELD, true));
// user groups on first login
mockTokenReturnsIdTokenWithGroup();
mockUserInfoWithTestGroups();
Expand Down Expand Up @@ -412,7 +410,7 @@ private HttpResponse<String> getPageWithGet(String user, String token, String ur
public void testRefreshTokenAndTokenExpiration_withoutRefreshToken() throws Exception {
mockAuthorizationRedirectsToFinishLogin();
configureWellKnown(null, null, "authorization_code");
jenkins.setSecurityRealm(new TestRealm(wireMockRule, null, EMAIL_FIELD, GROUPS_FIELD, AUTO_CONFIG_FIELD));
jenkins.setSecurityRealm(new TestRealm(wireMockRule, null, EMAIL_FIELD, GROUPS_FIELD, true));
// login
mockTokenReturnsIdTokenWithGroup(PluginTest::withoutRefreshToken);
mockUserInfoWithTestGroups();
Expand All @@ -430,7 +428,7 @@ public void testRefreshTokenAndTokenExpiration_withoutRefreshToken() throws Exce
public void testRefreshTokenWithTokenExpirationCheckDisabled_withoutRefreshToken() throws Exception {
mockAuthorizationRedirectsToFinishLogin();
configureWellKnown(null, null, "authorization_code");
var realm = new TestRealm(wireMockRule, null, EMAIL_FIELD, GROUPS_FIELD, AUTO_CONFIG_FIELD);
var realm = new TestRealm(wireMockRule, null, EMAIL_FIELD, GROUPS_FIELD, true);
realm.setTokenExpirationCheckDisabled(true);
jenkins.setSecurityRealm(realm);
// login
Expand All @@ -449,7 +447,7 @@ public void testRefreshTokenWithTokenExpirationCheckDisabled_withoutRefreshToken
public void testRefreshTokenWithTokenExpirationCheckDisabled_expiredRefreshToken() throws Exception {
mockAuthorizationRedirectsToFinishLogin();
configureWellKnown(null, null, "authorization_code", "refresh_token");
TestRealm testRealm = new TestRealm(wireMockRule, null, EMAIL_FIELD, GROUPS_FIELD, AUTO_CONFIG_FIELD);
TestRealm testRealm = new TestRealm(wireMockRule, null, EMAIL_FIELD, GROUPS_FIELD, true);
testRealm.setTokenExpirationCheckDisabled(true);
jenkins.setSecurityRealm(testRealm);
// login
Expand All @@ -473,7 +471,7 @@ public void testRefreshTokenWithTokenExpirationCheckDisabled_expiredRefreshToken
public void testRefreshTokenAndTokenExpiration_expiredRefreshToken() throws Exception {
mockAuthorizationRedirectsToFinishLogin();
configureWellKnown(null, null, "authorization_code", "refresh_token");
TestRealm testRealm = new TestRealm(wireMockRule, null, EMAIL_FIELD, GROUPS_FIELD, AUTO_CONFIG_FIELD);
TestRealm testRealm = new TestRealm(wireMockRule, null, EMAIL_FIELD, GROUPS_FIELD, true);
jenkins.setSecurityRealm(testRealm);
// login
mockTokenReturnsIdTokenWithGroup();
Expand All @@ -496,7 +494,7 @@ public void testRefreshTokenAndTokenExpiration_expiredRefreshToken() throws Exce
public void testTokenExpiration_withoutExpiresInValue() throws Exception {
mockAuthorizationRedirectsToFinishLogin();
configureWellKnown(null, null, "authorization_code", "refresh_token");
TestRealm testRealm = new TestRealm(wireMockRule, null, EMAIL_FIELD, GROUPS_FIELD, AUTO_CONFIG_FIELD);
TestRealm testRealm = new TestRealm(wireMockRule, null, EMAIL_FIELD, GROUPS_FIELD, true);
jenkins.setSecurityRealm(testRealm);
// login
mockTokenReturnsIdTokenWithGroup(PluginTest::withoutExpiresIn);
Expand Down Expand Up @@ -536,7 +534,7 @@ public void testreadResolve_withNulls() throws Exception {

configureWellKnown(null, null);

TestRealm realm = new TestRealm(wireMockRule, null, null, null, AUTO_CONFIG_FIELD);
TestRealm realm = new TestRealm(wireMockRule, null, null, null, true);
jenkins.setSecurityRealm(realm);

assertEquals(realm, realm.readResolve());
Expand All @@ -548,7 +546,7 @@ public void testreadResolve_withNonNulls() throws Exception {
mockTokenReturnsIdTokenWithGroup();
mockUserInfoWithTestGroups();
configureWellKnown("http://localhost/endSession", null);
TestRealm realm = new TestRealm(wireMockRule, null, null, null, AUTO_CONFIG_FIELD);
TestRealm realm = new TestRealm(wireMockRule, null, null, null, true);
jenkins.setSecurityRealm(realm);
assertEquals(realm, realm.readResolve());
}
Expand Down Expand Up @@ -802,7 +800,7 @@ public void testLastGrantedAuthoritiesProperty() throws Exception {

mockTokenReturnsIdTokenWithValues(setUpKeyValuesWithGroup());

jenkins.setSecurityRealm(new TestRealm(wireMockRule, null, EMAIL_FIELD, GROUPS_FIELD, MANUAL_CONFIG_FIELD));
jenkins.setSecurityRealm(new TestRealm(wireMockRule, null, EMAIL_FIELD, GROUPS_FIELD, false));

assertAnonymous();

Expand Down Expand Up @@ -993,7 +991,7 @@ public void loginWithCheckTokenFailure() throws Exception {
public void testAccessUsingJenkinsApiTokens() throws Exception {
mockAuthorizationRedirectsToFinishLogin();
configureWellKnown(null, null, "authorization_code");
jenkins.setSecurityRealm(new TestRealm(wireMockRule, null, EMAIL_FIELD, GROUPS_FIELD, AUTO_CONFIG_FIELD));
jenkins.setSecurityRealm(new TestRealm(wireMockRule, null, EMAIL_FIELD, GROUPS_FIELD, true));
// explicitly ensure allowTokenAccessWithoutOicSession is disabled
TestRealm testRealm = (TestRealm) jenkins.getSecurityRealm();
testRealm.setAllowTokenAccessWithoutOicSession(false);
Expand Down
Loading

0 comments on commit cfe74a7

Please sign in to comment.