Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove deprecated constructor #401

Merged
merged 1 commit into from
Sep 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@
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 All @@ -373,7 +299,7 @@
httpTransport = constructHttpTransport(isDisableSslVerification());
}
if (!Strings.isNullOrEmpty(endSessionUrl)) {
this.endSessionEndpoint = endSessionUrl + "/";

Check warning on line 302 in src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 302 is not covered by tests
}

// backward compatibility with wrong groupsFieldName split
Expand All @@ -394,33 +320,33 @@
// ensure escapeHatchSecret is encrypted
this.setEscapeHatchSecret(this.escapeHatchSecret);
try {
if (automanualconfigure != null) {

Check warning on line 323 in src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 323 is only partially covered, one branch is missing
if ("auto".equals(automanualconfigure)) {
OicServerWellKnownConfiguration conf =
new OicServerWellKnownConfiguration(wellKnownOpenIDConfigurationUrl);
conf.setScopesOverride(this.overrideScopes);
serverConfiguration = conf;
} else {
OicServerManualConfiguration conf =
new OicServerManualConfiguration(tokenServerUrl, authorizationServerUrl);
if (tokenAuthMethod != null) {
conf.setTokenAuthMethod(tokenAuthMethod);
}
conf.setEndSessionUrl(endSessionEndpoint);
conf.setJwksServerUrl(jwksServerUrl);
conf.setScopes(scopes != null ? scopes : "openid email");
conf.setUseRefreshTokens(useRefreshTokens);
conf.setUserInfoServerUrl(userInfoServerUrl);
serverConfiguration = conf;
}
}
} catch (FormException e) {
// FormException does not override toString() so looses info on the fields set and the message may not have
// context
// extract if into a better message until this is fixed.
ObjectStreamException ose = new InvalidObjectException(e.getFormField() + ": " + e.getMessage());
ose.initCause(e);
throw ose;

Check warning on line 349 in src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines

Lines 324-349 are not covered by tests
}
return this;
}
Expand Down Expand Up @@ -761,7 +687,7 @@
AccessMethod tokenAccessMethod = BearerToken.queryParameterAccessMethod();
HttpExecuteInterceptor authInterceptor =
new ClientParametersAuthentication(clientId, Secret.toString(clientSecret));
if (TokenAuthMethod.client_secret_basic.equals(serverConfiguration.getTokenAuthMethod())) {

Check warning on line 690 in src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 690 is only partially covered, one branch is missing
tokenAccessMethod = BearerToken.authorizationHeaderAccessMethod();
authInterceptor = new BasicAuthentication(clientId, Secret.toString(clientSecret));
}
Expand Down Expand Up @@ -1151,7 +1077,7 @@
OicCredentials credentials = user.getProperty(OicCredentials.class);

if (credentials != null) {
if (this.logoutFromOpenidProvider && !Strings.isNullOrEmpty(serverConfiguration.getEndSessionUrl())) {

Check warning on line 1080 in src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 1080 is not covered by tests
// This ensures that token will be expired at the right time with API Key calls, but no refresh can be
// made.
user.addProperty(new OicCredentials(null, null, null, CLOCK.millis()));
Expand Down Expand Up @@ -1187,7 +1113,7 @@
@CheckForNull
private String maybeOpenIdLogoutEndpoint(String idToken, String state, String postLogoutRedirectUrl) {
final String url = serverConfiguration.getEndSessionUrl();
if (this.logoutFromOpenidProvider && !Strings.isNullOrEmpty(url)) {

Check warning on line 1116 in src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 1116 is only partially covered, one branch is missing
StringBuilder openidLogoutEndpoint = new StringBuilder(url);

if (!Strings.isNullOrEmpty(idToken)) {
Expand Down Expand Up @@ -1298,7 +1224,7 @@
}

if (isExpired(credentials)) {
if (serverConfiguration.isUseRefreshTokens() && !Strings.isNullOrEmpty(credentials.getRefreshToken())) {

Check warning on line 1227 in src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 1227 is only partially covered, one branch is missing
return refreshExpiredToken(user.getId(), credentials, httpRequest, httpResponse);
} else if (!isTokenExpirationCheckDisabled()) {
redirectOrRejectRequest(httpRequest, httpResponse);
Expand Down Expand Up @@ -1403,7 +1329,7 @@
return false;
}

if (!Strings.isNullOrEmpty(serverConfiguration.getUserInfoServerUrl())) {

Check warning on line 1332 in src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 1332 is only partially covered, one branch is missing
userInfo = getUserInfo(flow, tokenResponse.getAccessToken());
}

Expand Down Expand Up @@ -1517,7 +1443,7 @@

@Restricted(NoExternalUse.class) // jelly only
public Descriptor<OicServerConfiguration> getDefaultServerConfigurationType() {
return Jenkins.get().getDescriptor(OicServerWellKnownConfiguration.class);

Check warning on line 1446 in src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 1446 is not covered by tests
}
}
}
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
Loading