diff --git a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java index 3acdfb56..488f33fd 100644 --- a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java +++ b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java @@ -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; @@ -279,79 +278,6 @@ public static enum TokenAuthMethod { private static final JmesPath 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, diff --git a/src/test/java/org/jenkinsci/plugins/oic/DescriptorImplTest.java b/src/test/java/org/jenkinsci/plugins/oic/DescriptorImplTest.java index e279d863..8599cccd 100644 --- a/src/test/java/org/jenkinsci/plugins/oic/DescriptorImplTest.java +++ b/src/test/java/org/jenkinsci/plugins/oic/DescriptorImplTest.java @@ -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; @@ -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(); @@ -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(); diff --git a/src/test/java/org/jenkinsci/plugins/oic/OicSecurityRealmTest.java b/src/test/java/org/jenkinsci/plugins/oic/OicSecurityRealmTest.java index 90e0a913..98d8658d 100644 --- a/src/test/java/org/jenkinsci/plugins/oic/OicSecurityRealmTest.java +++ b/src/test/java/org/jenkinsci/plugins/oic/OicSecurityRealmTest.java @@ -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/ diff --git a/src/test/java/org/jenkinsci/plugins/oic/PluginTest.java b/src/test/java/org/jenkinsci/plugins/oic/PluginTest.java index 6e315c3d..35048967 100644 --- a/src/test/java/org/jenkinsci/plugins/oic/PluginTest.java +++ b/src/test/java/org/jenkinsci/plugins/oic/PluginTest.java @@ -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; @@ -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(); @@ -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(); @@ -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", @@ -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", @@ -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(); @@ -412,7 +410,7 @@ private HttpResponse 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(); @@ -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 @@ -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 @@ -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(); @@ -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); @@ -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()); @@ -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()); } @@ -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(); @@ -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); diff --git a/src/test/java/org/jenkinsci/plugins/oic/TestRealm.java b/src/test/java/org/jenkinsci/plugins/oic/TestRealm.java index 06c106b0..d7f3cc5a 100644 --- a/src/test/java/org/jenkinsci/plugins/oic/TestRealm.java +++ b/src/test/java/org/jenkinsci/plugins/oic/TestRealm.java @@ -3,6 +3,7 @@ import com.github.tomakehurst.wiremock.junit.WireMockRule; import hudson.model.Descriptor; import hudson.security.SecurityRealm; +import hudson.util.Secret; import io.burt.jmespath.Expression; import java.io.IOException; import java.io.ObjectStreamException; @@ -15,16 +16,14 @@ public class TestRealm extends OicSecurityRealm { public static final String EMAIL_FIELD = "email"; public static final String FULL_NAME_FIELD = "fullName"; public static final String GROUPS_FIELD = "groups"; - public static final String MANUAL_CONFIG_FIELD = "manual"; - public static final String AUTO_CONFIG_FIELD = "auto"; public static class Builder { public String clientId = CLIENT_ID; - public String clientSecret = "secret"; + public Secret clientSecret = Secret.fromString("secret"); public String wellKnownOpenIDConfigurationUrl; public String tokenServerUrl; public String jwksServerUrl = null; - public String tokenAuthMethod = "client_secret_post"; + public TokenAuthMethod tokenAuthMethod = TokenAuthMethod.client_secret_post; public String authorizationServerUrl; public String userInfoServerUrl = null; public String userNameField = null; @@ -40,9 +39,9 @@ public static class Builder { public String postLogoutRedirectUrl = null; public boolean escapeHatchEnabled = false; public String escapeHatchUsername = null; - public String escapeHatchSecret = null; + public Secret escapeHatchSecret = null; public String escapeHatchGroup = null; - public String automanualconfigure = MANUAL_CONFIG_FIELD; + public boolean automanualconfigure = false; public Builder(WireMockRule wireMockRule) throws IOException { this("http://localhost:" + wireMockRule.port() + "/"); @@ -56,7 +55,7 @@ public Builder(String rootUrl) throws IOException { public Builder WithClient(String clientId, String clientSecret) { this.clientId = clientId; - this.clientSecret = clientSecret; + this.clientSecret = clientSecret == null ? null : Secret.fromString(clientSecret); return this; } @@ -85,7 +84,7 @@ public Builder WithPostLogoutRedirectUrl(String postLogoutRedirectUrl) { return this; } - public Builder WithAutomanualconfigure(String automanualconfigure) { + public Builder WithAutomanualconfigure(boolean automanualconfigure) { this.automanualconfigure = automanualconfigure; return this; } @@ -112,7 +111,7 @@ public Builder WithEscapeHatch( String escapeHatchGroup) { this.escapeHatchEnabled = escapeHatchEnabled; this.escapeHatchUsername = escapeHatchUsername; - this.escapeHatchSecret = escapeHatchSecret; + this.escapeHatchSecret = escapeHatchSecret == null ? null : Secret.fromString(escapeHatchSecret); this.escapeHatchGroup = escapeHatchGroup; return this; } @@ -120,35 +119,51 @@ public Builder WithEscapeHatch( public TestRealm build() throws IOException { return new TestRealm(this); } + + public OicServerConfiguration buildServerConfiguration() { + try { + if (automanualconfigure) { + OicServerWellKnownConfiguration conf = + new OicServerWellKnownConfiguration(wellKnownOpenIDConfigurationUrl); + if (scopes != null) { + conf.setScopesOverride(scopes); + } + return conf; + } + OicServerManualConfiguration conf = + new OicServerManualConfiguration(tokenServerUrl, authorizationServerUrl); + conf.setTokenAuthMethod(tokenAuthMethod); + conf.setUserInfoServerUrl(userInfoServerUrl); + if (scopes != null) { + conf.setScopes(scopes); + } + conf.setJwksServerUrl(jwksServerUrl); + conf.setEndSessionUrl(endSessionEndpoint); + return conf; + } catch (Exception e) { + throw new IllegalArgumentException(e); + } + } } - ; public TestRealm(Builder builder) throws IOException { super( builder.clientId, builder.clientSecret, - builder.wellKnownOpenIDConfigurationUrl, - builder.tokenServerUrl, - builder.jwksServerUrl, - builder.tokenAuthMethod, - builder.authorizationServerUrl, - builder.userInfoServerUrl, - builder.userNameField, - builder.tokenFieldToCheckKey, - builder.tokenFieldToCheckValue, - builder.fullNameFieldName, - builder.emailFieldName, - builder.scopes, - builder.groupsFieldName, - builder.disableSslVerification, - builder.logoutFromOpenidProvider, - builder.endSessionEndpoint, - builder.postLogoutRedirectUrl, - builder.escapeHatchEnabled, - builder.escapeHatchUsername, - builder.escapeHatchSecret, - builder.escapeHatchGroup, - builder.automanualconfigure); + builder.buildServerConfiguration(), + builder.disableSslVerification); + this.setUserNameField(builder.userNameField); + this.setTokenFieldToCheckKey(builder.tokenFieldToCheckKey); + this.setTokenFieldToCheckValue(builder.tokenFieldToCheckValue); + this.setFullNameFieldName(builder.fullNameFieldName); + this.setEmailFieldName(builder.emailFieldName); + this.setGroupsFieldName(builder.groupsFieldName); + this.setLogoutFromOpenidProvider(builder.logoutFromOpenidProvider); + this.setPostLogoutRedirectUrl(builder.postLogoutRedirectUrl); + this.setEscapeHatchEnabled(builder.escapeHatchEnabled); + this.setEscapeHatchUsername(builder.escapeHatchUsername); + this.setEscapeHatchSecret(builder.escapeHatchSecret); + this.setEscapeHatchGroup(builder.escapeHatchGroup); } public TestRealm(WireMockRule wireMockRule, String userInfoServerUrl, String emailFieldName, String groupsFieldName) @@ -172,7 +187,7 @@ public TestRealm( String userInfoServerUrl, String emailFieldName, String groupFieldName, - String automanualconfigure) + boolean automanualconfigure) throws IOException { this(new Builder(wireMockRule) .WithMinimalDefaults() @@ -187,7 +202,7 @@ public TestRealm( String userInfoServerUrl, String emailFieldName, String groupFieldName, - String automanualconfigure, + boolean automanualconfigure, boolean escapeHatchEnabled, String escapeHatchUsername, String escapeHatchSecret,