From 2b6c31c9e73cb302ff574eb059a3bb4aa3389aa5 Mon Sep 17 00:00:00 2001 From: Francisco Javier Fernandez Gonzalez Date: Tue, 8 Oct 2024 18:53:02 +0200 Subject: [PATCH 01/16] Bump pac4j to 6.x (Compilation only) --- pom.xml | 15 +-- .../oic/AnythingGoesTokenValidator.java | 24 +++-- .../oic/EscapeHatchCrumbExclusion.java | 8 +- .../plugins/oic/OicCrumbExclusion.java | 8 +- .../plugins/oic/OicSecurityRealm.java | 95 +++++++++++-------- .../oic/EscapeHatchCrumbExclusionTest.java | 10 +- .../plugins/oic/MockHttpServletRequest.java | 26 ++--- .../plugins/oic/OicCrumbExclusionTest.java | 6 +- .../org/jenkinsci/plugins/oic/PluginTest.java | 2 +- 9 files changed, 108 insertions(+), 86 deletions(-) diff --git a/pom.xml b/pom.xml index 7546c6f8..780eb64d 100644 --- a/pom.xml +++ b/pom.xml @@ -4,7 +4,7 @@ org.jenkins-ci.plugins plugin - 4.87 + 5.1 @@ -45,20 +45,18 @@ 4 999999-SNAPSHOT jenkinsci/${project.artifactId}-plugin - - 2.426.3 false Max 1836.vccda_4a_122a_a_e 4.364 - - 5.7.5 + 6.0.6 io.jenkins.tools.bom + bom-2.426.x 3208.vb_21177d4b_cd9 pom @@ -95,9 +93,14 @@ org.pac4j - pac4j-javaee + pac4j-jakartaee ${pac4jVersion} + + + com.fasterxml.jackson.core + jackson-databind + com.google.guava guava diff --git a/src/main/java/org/jenkinsci/plugins/oic/AnythingGoesTokenValidator.java b/src/main/java/org/jenkinsci/plugins/oic/AnythingGoesTokenValidator.java index 2bdb56ca..6a165023 100644 --- a/src/main/java/org/jenkinsci/plugins/oic/AnythingGoesTokenValidator.java +++ b/src/main/java/org/jenkinsci/plugins/oic/AnythingGoesTokenValidator.java @@ -24,7 +24,7 @@ public class AnythingGoesTokenValidator extends TokenValidator { public static final Logger LOGGER = Logger.getLogger(AnythingGoesTokenValidator.class.getName()); public AnythingGoesTokenValidator() { - super(createFakeOidcConfiguration()); + super(createFakeOidcConfiguration(), createFakeProviderMetadata()); } @Override @@ -51,17 +51,25 @@ public IDTokenClaimsSet validate(final JWT idToken, final Nonce expectedNonce) { * So we need a configuration with this set just so the validator can say "this is valid". */ private static OidcConfiguration createFakeOidcConfiguration() { + OidcConfiguration config = new OidcConfiguration(); + config.setClientId("ignored"); + config.setSecret("ignored"); + config.setPreferredJwsAlgorithm(JWSAlgorithm.HS256); + config.setClientAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_BASIC); + return config; + } + + /** + * Annoyingly the super class needs an OIDCProviderMetadata with some values set, + * which if we are not validating we may not actually have (e.g. jwks_url). + * So we need a metadata provider with this set just so the validator can say "this is valid". + */ + private static OIDCProviderMetadata createFakeProviderMetadata() { try { - OidcConfiguration config = new OidcConfiguration(); - config.setClientId("ignored"); - config.setSecret("ignored"); OIDCProviderMetadata providerMetadata = new OIDCProviderMetadata( new Issuer("http://ignored"), List.of(SubjectType.PUBLIC), new URI("http://ignored.and.invalid./")); providerMetadata.setIDTokenJWSAlgs(List.of(JWSAlgorithm.HS256)); - config.setProviderMetadata(providerMetadata); - config.setPreferredJwsAlgorithm(JWSAlgorithm.HS256); - config.setClientAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_BASIC); - return config; + return providerMetadata; } catch (URISyntaxException e) { // should never happen the urls we are using are valid throw new IllegalStateException(e); diff --git a/src/main/java/org/jenkinsci/plugins/oic/EscapeHatchCrumbExclusion.java b/src/main/java/org/jenkinsci/plugins/oic/EscapeHatchCrumbExclusion.java index c888ed12..65ba3f8e 100644 --- a/src/main/java/org/jenkinsci/plugins/oic/EscapeHatchCrumbExclusion.java +++ b/src/main/java/org/jenkinsci/plugins/oic/EscapeHatchCrumbExclusion.java @@ -2,11 +2,11 @@ import hudson.Extension; import hudson.security.csrf.CrumbExclusion; +import jakarta.servlet.FilterChain; +import jakarta.servlet.ServletException; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; import java.io.IOException; -import javax.servlet.FilterChain; -import javax.servlet.ServletException; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; /** * Excluding the escapeHatch login from CSRF protection as the crumb is calculated based on the authentication diff --git a/src/main/java/org/jenkinsci/plugins/oic/OicCrumbExclusion.java b/src/main/java/org/jenkinsci/plugins/oic/OicCrumbExclusion.java index d7e0141e..72f11e9f 100644 --- a/src/main/java/org/jenkinsci/plugins/oic/OicCrumbExclusion.java +++ b/src/main/java/org/jenkinsci/plugins/oic/OicCrumbExclusion.java @@ -3,11 +3,11 @@ import hudson.Extension; import hudson.security.SecurityRealm; import hudson.security.csrf.CrumbExclusion; +import jakarta.servlet.FilterChain; +import jakarta.servlet.ServletException; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; import java.io.IOException; -import javax.servlet.FilterChain; -import javax.servlet.ServletException; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; import jenkins.model.Jenkins; /** diff --git a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java index 7fd4555a..99c7eaae 100644 --- a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java +++ b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java @@ -41,7 +41,7 @@ import hudson.model.Descriptor.FormException; import hudson.model.Failure; import hudson.model.User; -import hudson.security.ChainedServletFilter; +import hudson.security.ChainedServletFilter2; import hudson.security.SecurityRealm; import hudson.tasks.Mailer; import hudson.util.FormValidation; @@ -50,6 +50,15 @@ import io.burt.jmespath.JmesPath; import io.burt.jmespath.RuntimeConfiguration; import io.burt.jmespath.jcf.JcfRuntime; +import jakarta.servlet.Filter; +import jakarta.servlet.FilterChain; +import jakarta.servlet.FilterConfig; +import jakarta.servlet.ServletException; +import jakarta.servlet.ServletRequest; +import jakarta.servlet.ServletResponse; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import jakarta.servlet.http.HttpSession; import java.io.IOException; import java.io.InvalidObjectException; import java.io.ObjectStreamException; @@ -74,15 +83,6 @@ import java.util.logging.Logger; import java.util.regex.Pattern; import javax.annotation.PostConstruct; -import javax.servlet.Filter; -import javax.servlet.FilterChain; -import javax.servlet.FilterConfig; -import javax.servlet.ServletException; -import javax.servlet.ServletRequest; -import javax.servlet.ServletResponse; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; -import javax.servlet.http.HttpSession; import jenkins.model.Jenkins; import jenkins.security.ApiTokenProperty; import jenkins.security.SecurityListener; @@ -95,9 +95,11 @@ import org.kohsuke.stapler.Header; import org.kohsuke.stapler.QueryParameter; import org.kohsuke.stapler.Stapler; -import org.kohsuke.stapler.StaplerRequest; -import org.kohsuke.stapler.StaplerResponse; +import org.kohsuke.stapler.StaplerRequest2; +import org.kohsuke.stapler.StaplerResponse2; import org.kohsuke.stapler.interceptor.RequirePOST; +import org.pac4j.core.context.CallContext; +import org.pac4j.core.context.FrameworkParameters; import org.pac4j.core.context.WebContext; import org.pac4j.core.context.session.SessionStore; import org.pac4j.core.credentials.Credentials; @@ -107,6 +109,7 @@ import org.pac4j.core.http.callback.NoParameterCallbackUrlResolver; import org.pac4j.core.profile.creator.ProfileCreator; import org.pac4j.jee.context.JEEContextFactory; +import org.pac4j.jee.context.JEEFrameworkParameters; import org.pac4j.jee.context.session.JEESessionStoreFactory; import org.pac4j.jee.http.adapter.JEEHttpActionAdapter; import org.pac4j.oidc.client.OidcClient; @@ -165,23 +168,23 @@ ClientAuthenticationMethod toClientAuthenticationMethod() { @Deprecated private transient String wellKnownOpenIDConfigurationUrl; - /** @deprecated see {@link OicServerConfiguration#getTokenServerUrl()} */ + /** @deprecated see {@link OicServerManualConfiguration#getTokenServerUrl()} */ @Deprecated private transient String tokenServerUrl; - /** @deprecated see {@link OicServerConfiguration#getJwksServerUrl()} */ + /** @deprecated see {@link OicServerManualConfiguration#getJwksServerUrl()} */ @Deprecated private transient String jwksServerUrl; - /** @deprecated see {@link OicServerConfiguration#getTokenAuthMethod()} */ + /** @deprecated see {@link OicServerManualConfiguration#getTokenAuthMethod()} */ @Deprecated private transient TokenAuthMethod tokenAuthMethod; - /** @deprecated see {@link OicServerConfiguration#getAuthorizationServerUrl()} */ + /** @deprecated see {@link OicServerManualConfiguration#getAuthorizationServerUrl()} */ @Deprecated private transient String authorizationServerUrl; - /** @deprecated see {@link OicServerConfiguration#getUserInfoServerUrl()} */ + /** @deprecated see {@link OicServerManualConfiguration#getUserInfoServerUrl()} */ @Deprecated private transient String userInfoServerUrl; @@ -199,14 +202,14 @@ ClientAuthenticationMethod toClientAuthenticationMethod() { private transient String simpleGroupsFieldName = null; private transient String nestedGroupFieldName = null; - /** @deprecated see {@link OicServerConfiguration#getScopes()} */ + /** @deprecated see {@link OicServerManualConfiguration#getScopes()} */ @Deprecated private transient String scopes = null; private final boolean disableSslVerification; private boolean logoutFromOpenidProvider = true; - /** @deprecated see {@link OicServerConfiguration#getEndSessionUrl()} */ + /** @deprecated see {@link OicServerManualConfiguration#getEndSessionUrl()} */ @Deprecated private transient String endSessionEndpoint = null; @@ -481,7 +484,8 @@ private OidcConfiguration buildOidcConfiguration() { conf.setAllowUnsignedIdTokens(true); conf.setTokenValidator(new AnythingGoesTokenValidator()); } - conf.setProviderMetadata(oidcProviderMetadata); + // TODO check if it can be removed + // conf.setProviderMetadata(oidcProviderMetadata); if (oidcProviderMetadata.getScopes() != null) { // auto configuration does not need to supply scopes conf.setScope(oidcProviderMetadata.getScopes().toString()); @@ -666,9 +670,10 @@ public String getAuthenticationGatewayUrl() { return "securityRealm/escapeHatch"; } + @Override public Filter createFilter(FilterConfig filterConfig) { - return new ChainedServletFilter(super.createFilter(filterConfig), new Filter() { + return new ChainedServletFilter2(super.createFilter(filterConfig), new Filter() { @Override public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { @@ -748,7 +753,7 @@ protected String getValidRedirectUrl(String url) { } /** - * Handles the the securityRealm/commenceLogin resource and sends the user off to the IdP + * Handles the securityRealm/commenceLogin resource and sends the user off to the IdP * @param from the relative URL to the page that the user has just come from * @param referer the HTTP referer header (where to redirect the user back to after login has finished) * @throws URISyntaxException if the provided data is invalid @@ -762,11 +767,11 @@ public void doCommenceLogin(@QueryParameter String from, @Header("Referer") fina final String redirectOnFinish = getValidRedirectUrl(from != null ? from : referer); OidcRedirectionActionBuilder builder = new OidcRedirectionActionBuilder(client); - WebContext webContext = - JEEContextFactory.INSTANCE.newContext(Stapler.getCurrentRequest(), Stapler.getCurrentResponse()); - SessionStore sessionStore = JEESessionStoreFactory.INSTANCE.newSessionStore(); - RedirectionAction redirectionAction = - builder.getRedirectionAction(webContext, sessionStore).orElseThrow(); + FrameworkParameters frameworkParameters = new JEEFrameworkParameters(Stapler.getCurrentRequest2(), Stapler.getCurrentResponse2()); + WebContext webContext = JEEContextFactory.INSTANCE.newContext(frameworkParameters); + SessionStore sessionStore = JEESessionStoreFactory.INSTANCE.newSessionStore(frameworkParameters); + CallContext callContext = new CallContext(webContext, sessionStore); + RedirectionAction redirectionAction = builder.getRedirectionAction(callContext).orElseThrow(); // store the redirect url for after the login. sessionStore.set(webContext, SESSION_POST_LOGIN_REDIRECT_URL_KEY, redirectOnFinish); @@ -964,7 +969,7 @@ private List ensureString(Object field) { } @Restricted(DoNotUse.class) // stapler only - public void doLogout(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException { + public void doLogout(StaplerRequest2 req, StaplerResponse2 rsp) throws IOException, ServletException { Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); User user = User.get2(authentication); @@ -987,7 +992,7 @@ public void doLogout(StaplerRequest req, StaplerResponse rsp) throws IOException } @Override - public String getPostLogOutUrl2(StaplerRequest req, Authentication auth) { + public String getPostLogOutUrl2(StaplerRequest2 req, Authentication auth) { Object idToken = req.getAttribute(ID_TOKEN_REQUEST_ATTRIBUTE); Object state = getStateAttribute(req.getSession()); var openidLogoutEndpoint = maybeOpenIdLogoutEndpoint( @@ -1002,12 +1007,14 @@ public String getPostLogOutUrl2(StaplerRequest req, Authentication auth) { Object getStateAttribute(HttpSession session) { // return null; OidcClient client = buildOidcClient(); - WebContext webContext = - JEEContextFactory.INSTANCE.newContext(Stapler.getCurrentRequest(), Stapler.getCurrentResponse()); - SessionStore sessionStore = JEESessionStoreFactory.INSTANCE.newSessionStore(); + FrameworkParameters frameworkParameters = new JEEFrameworkParameters(Stapler.getCurrentRequest2(), Stapler.getCurrentResponse2()); + WebContext webContext = JEEContextFactory.INSTANCE.newContext(frameworkParameters); + SessionStore sessionStore = JEESessionStoreFactory.INSTANCE.newSessionStore(frameworkParameters); + CallContext callContext = new CallContext(webContext, sessionStore); + return client.getConfiguration() .getValueRetriever() - .retrieve(client.getStateSessionAttributeName(), client, webContext, sessionStore) + .retrieve(callContext, client.getStateSessionAttributeName(), client) .orElse(null); } @@ -1034,7 +1041,7 @@ private String maybeOpenIdLogoutEndpoint(String idToken, String state, String po return null; } - private String getFinalLogoutUrl(StaplerRequest req, Authentication auth) { + private String getFinalLogoutUrl(StaplerRequest2 req, Authentication auth) { if (Jenkins.get().hasPermission(Jenkins.READ)) { return super.getPostLogOutUrl2(req, auth); } @@ -1071,11 +1078,13 @@ private String buildOAuthRedirectUrl() throws NullPointerException { * @param request The user's request * @throws ParseException if the JWT (or other response) could not be parsed. */ - public void doFinishLogin(StaplerRequest request, StaplerResponse response) throws IOException, ParseException { + public void doFinishLogin(StaplerRequest2 request, StaplerResponse2 response) throws IOException, ParseException { OidcClient client = buildOidcClient(); - WebContext webContext = JEEContextFactory.INSTANCE.newContext(request, response); - SessionStore sessionStore = JEESessionStoreFactory.INSTANCE.newSessionStore(); + FrameworkParameters frameworkParameters = new JEEFrameworkParameters(request, response); + WebContext webContext = JEEContextFactory.INSTANCE.newContext(frameworkParameters); + SessionStore sessionStore = JEESessionStoreFactory.INSTANCE.newSessionStore(frameworkParameters); + CallContext callContext = new CallContext(webContext, sessionStore); try { // NB: TODO this also handles back channel logout if "logoutendpoint" parameter is set @@ -1084,14 +1093,14 @@ public void doFinishLogin(StaplerRequest request, StaplerResponse response) thro // Jenkins stuff correctly // also should have its own URL to make the code easier to follow :) - Credentials credentials = client.getCredentials(webContext, sessionStore) + Credentials credentials = client.getCredentials(callContext) .orElseThrow(() -> new Failure("Could not extract credentials from request")); ProfileCreator profileCreator = client.getProfileCreator(); // creating the profile performs validation of the token OidcProfile profile = (OidcProfile) profileCreator - .create(credentials, webContext, sessionStore) + .create(callContext, credentials) .orElseThrow(() -> new Failure("Could not build user profile")); AccessToken accessToken = profile.getAccessToken(); @@ -1208,8 +1217,10 @@ private boolean refreshExpiredToken( HttpServletResponse httpResponse) throws IOException { - WebContext webContext = JEEContextFactory.INSTANCE.newContext(httpRequest, httpResponse); - SessionStore sessionStore = JEESessionStoreFactory.INSTANCE.newSessionStore(); + FrameworkParameters frameworkParameters = new JEEFrameworkParameters(httpRequest, httpResponse); + WebContext webContext = JEEContextFactory.INSTANCE.newContext(frameworkParameters); + SessionStore sessionStore = JEESessionStoreFactory.INSTANCE.newSessionStore(frameworkParameters); + CallContext callContext = new CallContext(webContext, sessionStore); OidcClient client = buildOidcClient(); try { OidcProfile profile = new OidcProfile(); @@ -1217,7 +1228,7 @@ private boolean refreshExpiredToken( profile.setIdTokenString(credentials.getIdToken()); profile.setRefreshToken(new RefreshToken(credentials.getRefreshToken())); - profile = (OidcProfile) client.renewUserProfile(profile, webContext, sessionStore) + profile = (OidcProfile) client.renewUserProfile(callContext, profile) .orElseThrow(() -> new IllegalStateException("Could not renew user profile")); AccessToken accessToken = profile.getAccessToken(); diff --git a/src/test/java/org/jenkinsci/plugins/oic/EscapeHatchCrumbExclusionTest.java b/src/test/java/org/jenkinsci/plugins/oic/EscapeHatchCrumbExclusionTest.java index e3155ef5..14b91baa 100644 --- a/src/test/java/org/jenkinsci/plugins/oic/EscapeHatchCrumbExclusionTest.java +++ b/src/test/java/org/jenkinsci/plugins/oic/EscapeHatchCrumbExclusionTest.java @@ -1,11 +1,11 @@ package org.jenkinsci.plugins.oic; +import jakarta.servlet.FilterChain; +import jakarta.servlet.ServletException; +import jakarta.servlet.ServletRequest; +import jakarta.servlet.ServletResponse; +import jakarta.servlet.http.HttpServletResponse; import java.io.IOException; -import javax.servlet.FilterChain; -import javax.servlet.ServletException; -import javax.servlet.ServletRequest; -import javax.servlet.ServletResponse; -import javax.servlet.http.HttpServletResponse; import org.junit.Test; import static org.junit.Assert.assertFalse; diff --git a/src/test/java/org/jenkinsci/plugins/oic/MockHttpServletRequest.java b/src/test/java/org/jenkinsci/plugins/oic/MockHttpServletRequest.java index 5bb2836a..578f0554 100644 --- a/src/test/java/org/jenkinsci/plugins/oic/MockHttpServletRequest.java +++ b/src/test/java/org/jenkinsci/plugins/oic/MockHttpServletRequest.java @@ -1,24 +1,24 @@ package org.jenkinsci.plugins.oic; +import jakarta.servlet.AsyncContext; +import jakarta.servlet.DispatcherType; +import jakarta.servlet.RequestDispatcher; +import jakarta.servlet.ServletContext; +import jakarta.servlet.ServletInputStream; +import jakarta.servlet.ServletRequest; +import jakarta.servlet.ServletResponse; +import jakarta.servlet.http.Cookie; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import jakarta.servlet.http.HttpSession; +import jakarta.servlet.http.HttpUpgradeHandler; +import jakarta.servlet.http.Part; import java.io.BufferedReader; import java.security.Principal; import java.util.Collection; import java.util.Enumeration; import java.util.Locale; import java.util.Map; -import javax.servlet.AsyncContext; -import javax.servlet.DispatcherType; -import javax.servlet.RequestDispatcher; -import javax.servlet.ServletContext; -import javax.servlet.ServletInputStream; -import javax.servlet.ServletRequest; -import javax.servlet.ServletResponse; -import javax.servlet.http.Cookie; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; -import javax.servlet.http.HttpSession; -import javax.servlet.http.HttpUpgradeHandler; -import javax.servlet.http.Part; public class MockHttpServletRequest implements HttpServletRequest { diff --git a/src/test/java/org/jenkinsci/plugins/oic/OicCrumbExclusionTest.java b/src/test/java/org/jenkinsci/plugins/oic/OicCrumbExclusionTest.java index a6f0ad72..22ed2133 100644 --- a/src/test/java/org/jenkinsci/plugins/oic/OicCrumbExclusionTest.java +++ b/src/test/java/org/jenkinsci/plugins/oic/OicCrumbExclusionTest.java @@ -1,8 +1,8 @@ package org.jenkinsci.plugins.oic; -import javax.servlet.FilterChain; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; +import jakarta.servlet.FilterChain; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; import jenkins.model.Jenkins; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; diff --git a/src/test/java/org/jenkinsci/plugins/oic/PluginTest.java b/src/test/java/org/jenkinsci/plugins/oic/PluginTest.java index af6d7baa..e7fc8a4a 100644 --- a/src/test/java/org/jenkinsci/plugins/oic/PluginTest.java +++ b/src/test/java/org/jenkinsci/plugins/oic/PluginTest.java @@ -18,6 +18,7 @@ import hudson.model.User; import hudson.tasks.Mailer; import hudson.util.VersionNumber; +import jakarta.servlet.http.HttpSession; import java.io.IOException; import java.net.URI; import java.net.http.HttpClient; @@ -41,7 +42,6 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import javax.net.ssl.SSLException; -import javax.servlet.http.HttpSession; import jenkins.model.Jenkins; import jenkins.security.ApiTokenProperty; import jenkins.security.LastGrantedAuthoritiesProperty; From dccb09fee58c8e0085da3422488278217977e405 Mon Sep 17 00:00:00 2001 From: Francisco Javier Fernandez Gonzalez Date: Wed, 9 Oct 2024 17:33:03 +0200 Subject: [PATCH 02/16] TokenValidator properly set --- .../oic/AnythingGoesOpMetadataResolver.java | 17 +++++++++++++++++ .../jenkinsci/plugins/oic/OicSecurityRealm.java | 5 ++--- .../org/jenkinsci/plugins/oic/PluginTest.java | 8 ++++---- .../org/jenkinsci/plugins/oic/TestRealm.java | 13 ++++++++----- 4 files changed, 31 insertions(+), 12 deletions(-) create mode 100644 src/main/java/org/jenkinsci/plugins/oic/AnythingGoesOpMetadataResolver.java diff --git a/src/main/java/org/jenkinsci/plugins/oic/AnythingGoesOpMetadataResolver.java b/src/main/java/org/jenkinsci/plugins/oic/AnythingGoesOpMetadataResolver.java new file mode 100644 index 00000000..f45b1afc --- /dev/null +++ b/src/main/java/org/jenkinsci/plugins/oic/AnythingGoesOpMetadataResolver.java @@ -0,0 +1,17 @@ +package org.jenkinsci.plugins.oic; + +import org.pac4j.oidc.config.OidcConfiguration; +import org.pac4j.oidc.metadata.OidcOpMetadataResolver; +import org.pac4j.oidc.profile.creator.TokenValidator; + +public class AnythingGoesOpMetadataResolver extends OidcOpMetadataResolver { + + public AnythingGoesOpMetadataResolver(OidcConfiguration configuration) { + super(configuration); + } + + @Override + protected TokenValidator createTokenValidator() { + return new AnythingGoesTokenValidator(); + } +} diff --git a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java index 99c7eaae..b75b68c4 100644 --- a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java +++ b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java @@ -482,10 +482,9 @@ private OidcConfiguration buildOidcConfiguration() { OIDCProviderMetadata oidcProviderMetadata = serverConfiguration.toProviderMetadata(); if (this.isDisableTokenVerification()) { conf.setAllowUnsignedIdTokens(true); - conf.setTokenValidator(new AnythingGoesTokenValidator()); + conf.setOpMetadataResolver(new AnythingGoesOpMetadataResolver(conf)); } - // TODO check if it can be removed - // conf.setProviderMetadata(oidcProviderMetadata); + if (oidcProviderMetadata.getScopes() != null) { // auto configuration does not need to supply scopes conf.setScope(oidcProviderMetadata.getScopes().toString()); diff --git a/src/test/java/org/jenkinsci/plugins/oic/PluginTest.java b/src/test/java/org/jenkinsci/plugins/oic/PluginTest.java index e7fc8a4a..beb523fa 100644 --- a/src/test/java/org/jenkinsci/plugins/oic/PluginTest.java +++ b/src/test/java/org/jenkinsci/plugins/oic/PluginTest.java @@ -143,7 +143,7 @@ public void testLoginWithDefaults() throws Exception { .withQueryParam("nonce", matching(".+"))); verify(postRequestedFor(urlPathEqualTo("/token")).withRequestBody(notMatching(".*&scope=.*"))); webClient.executeOnServer(() -> { - HttpSession session = Stapler.getCurrentRequest().getSession(); + HttpSession session = Stapler.getCurrentRequest2().getSession(); assertNotNull(((OicSecurityRealm) Jenkins.get().getSecurityRealm()).getStateAttribute(session)); return null; }); @@ -933,7 +933,7 @@ public void testLogoutShouldBeJenkinsOnlyWhenNoProviderLogoutConfigured() throws String[] logoutURL = new String[1]; jenkinsRule.executeOnServer(() -> { - logoutURL[0] = oicsr.getPostLogOutUrl2(Stapler.getCurrentRequest(), Jenkins.ANONYMOUS2); + logoutURL[0] = oicsr.getPostLogOutUrl2(Stapler.getCurrentRequest2(), Jenkins.ANONYMOUS2); return null; }); assertEquals("/jenkins/", logoutURL[0]); @@ -947,7 +947,7 @@ public void testLogoutShouldBeProviderURLWhenProviderLogoutConfigured() throws E String[] logoutURL = new String[1]; jenkinsRule.executeOnServer(() -> { - logoutURL[0] = oicsr.getPostLogOutUrl2(Stapler.getCurrentRequest(), Jenkins.ANONYMOUS2); + logoutURL[0] = oicsr.getPostLogOutUrl2(Stapler.getCurrentRequest2(), Jenkins.ANONYMOUS2); return null; }); assertEquals("http://provider/logout?state=null", logoutURL[0]); @@ -964,7 +964,7 @@ public void testLogoutShouldBeProviderURLWithRedirectWhenProviderLogoutConfigure String[] logoutURL = new String[1]; jenkinsRule.executeOnServer(() -> { - logoutURL[0] = oicsr.getPostLogOutUrl2(Stapler.getCurrentRequest(), Jenkins.ANONYMOUS2); + logoutURL[0] = oicsr.getPostLogOutUrl2(Stapler.getCurrentRequest2(), Jenkins.ANONYMOUS2); return null; }); assertEquals( diff --git a/src/test/java/org/jenkinsci/plugins/oic/TestRealm.java b/src/test/java/org/jenkinsci/plugins/oic/TestRealm.java index fca8f795..570e26bc 100644 --- a/src/test/java/org/jenkinsci/plugins/oic/TestRealm.java +++ b/src/test/java/org/jenkinsci/plugins/oic/TestRealm.java @@ -8,11 +8,13 @@ import java.io.IOException; import java.io.ObjectStreamException; import java.text.ParseException; -import org.kohsuke.stapler.StaplerRequest; -import org.kohsuke.stapler.StaplerResponse; +import org.kohsuke.stapler.StaplerRequest2; +import org.kohsuke.stapler.StaplerResponse2; +import org.pac4j.core.context.FrameworkParameters; import org.pac4j.core.context.WebContext; import org.pac4j.core.context.session.SessionStore; import org.pac4j.jee.context.JEEContextFactory; +import org.pac4j.jee.context.JEEFrameworkParameters; import org.pac4j.jee.context.session.JEESessionStoreFactory; import org.pac4j.oidc.client.OidcClient; @@ -259,14 +261,15 @@ public Descriptor getDescriptor() { } @Override - public void doFinishLogin(StaplerRequest request, StaplerResponse response) throws IOException, ParseException { + public void doFinishLogin(StaplerRequest2 request, StaplerResponse2 response) throws IOException, ParseException { /* * PluginTest uses a hardCoded nonce "nonce" */ if (!isNonceDisabled()) { // only hack the nonce if the nonce is enabled - WebContext webContext = JEEContextFactory.INSTANCE.newContext(request, response); - SessionStore sessionStore = JEESessionStoreFactory.INSTANCE.newSessionStore(); + FrameworkParameters frameworkParameters = new JEEFrameworkParameters(request, response); + WebContext webContext = JEEContextFactory.INSTANCE.newContext(frameworkParameters); + SessionStore sessionStore = JEESessionStoreFactory.INSTANCE.newSessionStore(frameworkParameters); OidcClient oidcClient = buildOidcClient(); sessionStore.set(webContext, oidcClient.getNonceSessionAttributeName(), "nonce"); } From de0b8d6411ec08bd599cdb41ed0ad3d0b669eb6b Mon Sep 17 00:00:00 2001 From: Francisco Javier Fernandez Gonzalez Date: Wed, 9 Oct 2024 17:38:24 +0200 Subject: [PATCH 03/16] Compilation error after merging --- src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java index c2fbd60e..93012cfa 100644 --- a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java +++ b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java @@ -1293,7 +1293,7 @@ private boolean refreshExpiredToken( return false; } LOGGER.log(Level.FINE, "Failed to refresh expired token", e); - redirectToLoginUrl(Stapler.getCurrentRequest(), Stapler.getCurrentResponse()); + redirectToLoginUrl(Stapler.getCurrentRequest2(), Stapler.getCurrentResponse2()); return false; } LOGGER.log(Level.WARNING, "Failed to refresh expired token", e); From 6d610a29283b7777df9d89abd0198cc9454d5f8d Mon Sep 17 00:00:00 2001 From: Francisco Javier Fernandez Gonzalez Date: Wed, 9 Oct 2024 18:45:38 +0200 Subject: [PATCH 04/16] set always opMetadataResolver --- src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java index 93012cfa..42b4e433 100644 --- a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java +++ b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java @@ -117,6 +117,7 @@ import org.pac4j.oidc.client.OidcClient; import org.pac4j.oidc.config.OidcConfiguration; import org.pac4j.oidc.credentials.authenticator.OidcAuthenticator; +import org.pac4j.oidc.metadata.OidcOpMetadataResolver; import org.pac4j.oidc.profile.OidcProfile; import org.pac4j.oidc.redirect.OidcRedirectionActionBuilder; import org.springframework.security.authentication.AnonymousAuthenticationToken; @@ -495,6 +496,8 @@ private OidcConfiguration buildOidcConfiguration() { if (this.isDisableTokenVerification()) { conf.setAllowUnsignedIdTokens(true); conf.setOpMetadataResolver(new AnythingGoesOpMetadataResolver(conf)); + } else { + conf.setOpMetadataResolver(new OidcOpMetadataResolver(conf)); } if (oidcProviderMetadata.getScopes() != null) { From 03baf3c561356408aba6609b2ccefbc60c3aa22a Mon Sep 17 00:00:00 2001 From: Francisco Javier Fernandez Gonzalez Date: Thu, 10 Oct 2024 12:40:47 +0200 Subject: [PATCH 05/16] Use of StaticOidcOpMetadataResolver --- .../plugins/oic/AnythingGoesOpMetadataResolver.java | 9 +++++---- .../java/org/jenkinsci/plugins/oic/OicSecurityRealm.java | 7 +++++-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/oic/AnythingGoesOpMetadataResolver.java b/src/main/java/org/jenkinsci/plugins/oic/AnythingGoesOpMetadataResolver.java index f45b1afc..82850eee 100644 --- a/src/main/java/org/jenkinsci/plugins/oic/AnythingGoesOpMetadataResolver.java +++ b/src/main/java/org/jenkinsci/plugins/oic/AnythingGoesOpMetadataResolver.java @@ -1,13 +1,14 @@ package org.jenkinsci.plugins.oic; +import com.nimbusds.openid.connect.sdk.op.OIDCProviderMetadata; import org.pac4j.oidc.config.OidcConfiguration; -import org.pac4j.oidc.metadata.OidcOpMetadataResolver; +import org.pac4j.oidc.metadata.StaticOidcOpMetadataResolver; import org.pac4j.oidc.profile.creator.TokenValidator; -public class AnythingGoesOpMetadataResolver extends OidcOpMetadataResolver { +public class AnythingGoesOpMetadataResolver extends StaticOidcOpMetadataResolver { - public AnythingGoesOpMetadataResolver(OidcConfiguration configuration) { - super(configuration); + public AnythingGoesOpMetadataResolver(OidcConfiguration configuration, OIDCProviderMetadata staticMetadata) { + super(configuration, staticMetadata); } @Override diff --git a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java index 42b4e433..ba8bdd4b 100644 --- a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java +++ b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java @@ -118,6 +118,7 @@ import org.pac4j.oidc.config.OidcConfiguration; import org.pac4j.oidc.credentials.authenticator.OidcAuthenticator; import org.pac4j.oidc.metadata.OidcOpMetadataResolver; +import org.pac4j.oidc.metadata.StaticOidcOpMetadataResolver; import org.pac4j.oidc.profile.OidcProfile; import org.pac4j.oidc.redirect.OidcRedirectionActionBuilder; import org.springframework.security.authentication.AnonymousAuthenticationToken; @@ -493,12 +494,14 @@ private OidcConfiguration buildOidcConfiguration() { // set many more as needed... OIDCProviderMetadata oidcProviderMetadata = serverConfiguration.toProviderMetadata(); + OidcOpMetadataResolver metadataResolver; if (this.isDisableTokenVerification()) { conf.setAllowUnsignedIdTokens(true); - conf.setOpMetadataResolver(new AnythingGoesOpMetadataResolver(conf)); + metadataResolver = new AnythingGoesOpMetadataResolver(conf, oidcProviderMetadata); } else { - conf.setOpMetadataResolver(new OidcOpMetadataResolver(conf)); + metadataResolver = new StaticOidcOpMetadataResolver(conf, oidcProviderMetadata); } + conf = conf.withOpMetadataResolver(metadataResolver); if (oidcProviderMetadata.getScopes() != null) { // auto configuration does not need to supply scopes From 1754238ab36ee1c6108705b35403aef0c5a54b65 Mon Sep 17 00:00:00 2001 From: Francisco Javier Fernandez Gonzalez Date: Thu, 10 Oct 2024 16:13:35 +0200 Subject: [PATCH 06/16] doCommenceLogin --- .../oic/AnythingGoesOpMetadataResolver.java | 18 ---------- .../plugins/oic/OicSecurityRealm.java | 9 ++--- .../oic/OicdPluginOpMetadataResolver.java | 36 +++++++++++++++++++ 3 files changed, 41 insertions(+), 22 deletions(-) delete mode 100644 src/main/java/org/jenkinsci/plugins/oic/AnythingGoesOpMetadataResolver.java create mode 100644 src/main/java/org/jenkinsci/plugins/oic/OicdPluginOpMetadataResolver.java diff --git a/src/main/java/org/jenkinsci/plugins/oic/AnythingGoesOpMetadataResolver.java b/src/main/java/org/jenkinsci/plugins/oic/AnythingGoesOpMetadataResolver.java deleted file mode 100644 index 82850eee..00000000 --- a/src/main/java/org/jenkinsci/plugins/oic/AnythingGoesOpMetadataResolver.java +++ /dev/null @@ -1,18 +0,0 @@ -package org.jenkinsci.plugins.oic; - -import com.nimbusds.openid.connect.sdk.op.OIDCProviderMetadata; -import org.pac4j.oidc.config.OidcConfiguration; -import org.pac4j.oidc.metadata.StaticOidcOpMetadataResolver; -import org.pac4j.oidc.profile.creator.TokenValidator; - -public class AnythingGoesOpMetadataResolver extends StaticOidcOpMetadataResolver { - - public AnythingGoesOpMetadataResolver(OidcConfiguration configuration, OIDCProviderMetadata staticMetadata) { - super(configuration, staticMetadata); - } - - @Override - protected TokenValidator createTokenValidator() { - return new AnythingGoesTokenValidator(); - } -} diff --git a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java index ba8bdd4b..af9ff970 100644 --- a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java +++ b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java @@ -118,7 +118,6 @@ import org.pac4j.oidc.config.OidcConfiguration; import org.pac4j.oidc.credentials.authenticator.OidcAuthenticator; import org.pac4j.oidc.metadata.OidcOpMetadataResolver; -import org.pac4j.oidc.metadata.StaticOidcOpMetadataResolver; import org.pac4j.oidc.profile.OidcProfile; import org.pac4j.oidc.redirect.OidcRedirectionActionBuilder; import org.springframework.security.authentication.AnonymousAuthenticationToken; @@ -497,11 +496,11 @@ private OidcConfiguration buildOidcConfiguration() { OidcOpMetadataResolver metadataResolver; if (this.isDisableTokenVerification()) { conf.setAllowUnsignedIdTokens(true); - metadataResolver = new AnythingGoesOpMetadataResolver(conf, oidcProviderMetadata); + metadataResolver = new OicdPluginOpMetadataResolver(conf, oidcProviderMetadata, true); } else { - metadataResolver = new StaticOidcOpMetadataResolver(conf, oidcProviderMetadata); + metadataResolver = new OicdPluginOpMetadataResolver(conf, oidcProviderMetadata, false); } - conf = conf.withOpMetadataResolver(metadataResolver); + conf.setOpMetadataResolver(metadataResolver); if (oidcProviderMetadata.getScopes() != null) { // auto configuration does not need to supply scopes @@ -514,6 +513,8 @@ private OidcConfiguration buildOidcConfiguration() { conf.setResourceRetriever(getResourceRetriever()); if (this.isPkceEnabled()) { conf.setPkceMethod(CodeChallengeMethod.S256); + } else { + conf.setDisablePkce(true); } return conf; } diff --git a/src/main/java/org/jenkinsci/plugins/oic/OicdPluginOpMetadataResolver.java b/src/main/java/org/jenkinsci/plugins/oic/OicdPluginOpMetadataResolver.java new file mode 100644 index 00000000..6e0b6ead --- /dev/null +++ b/src/main/java/org/jenkinsci/plugins/oic/OicdPluginOpMetadataResolver.java @@ -0,0 +1,36 @@ +package org.jenkinsci.plugins.oic; + +import com.nimbusds.openid.connect.sdk.op.OIDCProviderMetadata; +import org.pac4j.oidc.config.OidcConfiguration; +import org.pac4j.oidc.metadata.StaticOidcOpMetadataResolver; +import org.pac4j.oidc.profile.creator.TokenValidator; + +public class OicdPluginOpMetadataResolver extends StaticOidcOpMetadataResolver { + + private boolean allowsAnything; + + public OicdPluginOpMetadataResolver(OidcConfiguration configuration, OIDCProviderMetadata staticMetadata, + boolean allowsAnything) { + super(configuration, staticMetadata); + this.allowsAnything = allowsAnything; + } + + @Override + protected TokenValidator createTokenValidator() { + if (allowsAnything) { + return new AnythingGoesTokenValidator(); + } + + return super.createTokenValidator(); + } + + /** + * This method is needed as there seems to be a bug in pac4j and hasChanged is not able to return true + * This will make it work until the bug is fixed. + * TODO eventually remove + */ + @Override + public boolean hasChanged() { + return true; + } +} From 1ee4af1e9e8261d38f69f463a63fb8fcabfb097c Mon Sep 17 00:00:00 2001 From: Francisco Javier Fernandez Gonzalez Date: Thu, 10 Oct 2024 19:40:01 +0200 Subject: [PATCH 07/16] doFinishLogin --- src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java index af9ff970..16b33a89 100644 --- a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java +++ b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java @@ -1114,6 +1114,9 @@ public void doFinishLogin(StaplerRequest2 request, StaplerResponse2 response) th Credentials credentials = client.getCredentials(callContext) .orElseThrow(() -> new Failure("Could not extract credentials from request")); + credentials = client.getAuthenticator().validate(callContext, credentials).orElseThrow( + () -> new Failure("Could not " + "validate " + "credentials")); + ProfileCreator profileCreator = client.getProfileCreator(); // creating the profile performs validation of the token From d6d3f2178d83673ee7306a35a19b7db9146b4e3b Mon Sep 17 00:00:00 2001 From: Francisco Javier Fernandez Gonzalez Date: Tue, 15 Oct 2024 18:25:05 +0200 Subject: [PATCH 08/16] Do not use default hostname verifier and SSL socket factory when TLS is disabled --- .../plugins/oic/CustomOidcConfiguration.java | 4 +++- .../jenkinsci/plugins/oic/OicSecurityRealm.java | 14 +++++++++----- .../plugins/oic/OicdPluginOpMetadataResolver.java | 4 ++-- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/oic/CustomOidcConfiguration.java b/src/main/java/org/jenkinsci/plugins/oic/CustomOidcConfiguration.java index 5f1878b4..2700af24 100644 --- a/src/main/java/org/jenkinsci/plugins/oic/CustomOidcConfiguration.java +++ b/src/main/java/org/jenkinsci/plugins/oic/CustomOidcConfiguration.java @@ -39,6 +39,9 @@ public void configureHttpRequest(HTTPRequest request) { } } request.setProxy(proxy); + // super class will configure the hostname verifier and the SSL socket factory and the default values in case + // the config object doesn't have those values must be overrriden in case the disableTLS is true + super.configureHttpRequest(request); if (disableTLS) { request.setHostnameVerifier(IgnoringHostNameVerifier.INSTANCE); try { @@ -47,6 +50,5 @@ public void configureHttpRequest(HTTPRequest request) { throw new IllegalStateException("could not configure the SSLFactory, this should not be possible", e); } } - super.configureHttpRequest(request); } } diff --git a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java index 35f89834..a124995f 100644 --- a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java +++ b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java @@ -957,11 +957,13 @@ public void doCommenceLogin(@QueryParameter String from, @Header("Referer") fina final String redirectOnFinish = getValidRedirectUrl(from != null ? from : referer); OidcRedirectionActionBuilder builder = new OidcRedirectionActionBuilder(client); - FrameworkParameters frameworkParameters = new JEEFrameworkParameters(Stapler.getCurrentRequest2(), Stapler.getCurrentResponse2()); + FrameworkParameters frameworkParameters = + new JEEFrameworkParameters(Stapler.getCurrentRequest2(), Stapler.getCurrentResponse2()); WebContext webContext = JEEContextFactory.INSTANCE.newContext(frameworkParameters); SessionStore sessionStore = JEESessionStoreFactory.INSTANCE.newSessionStore(frameworkParameters); CallContext callContext = new CallContext(webContext, sessionStore); - RedirectionAction redirectionAction = builder.getRedirectionAction(callContext).orElseThrow(); + RedirectionAction redirectionAction = + builder.getRedirectionAction(callContext).orElseThrow(); // store the redirect url for after the login. sessionStore.set(webContext, SESSION_POST_LOGIN_REDIRECT_URL_KEY, redirectOnFinish); @@ -1197,7 +1199,8 @@ public String getPostLogOutUrl2(StaplerRequest2 req, Authentication auth) { Object getStateAttribute(HttpSession session) { // return null; OidcClient client = buildOidcClient(); - FrameworkParameters frameworkParameters = new JEEFrameworkParameters(Stapler.getCurrentRequest2(), Stapler.getCurrentResponse2()); + FrameworkParameters frameworkParameters = + new JEEFrameworkParameters(Stapler.getCurrentRequest2(), Stapler.getCurrentResponse2()); WebContext webContext = JEEContextFactory.INSTANCE.newContext(frameworkParameters); SessionStore sessionStore = JEESessionStoreFactory.INSTANCE.newSessionStore(frameworkParameters); CallContext callContext = new CallContext(webContext, sessionStore); @@ -1286,8 +1289,9 @@ public void doFinishLogin(StaplerRequest2 request, StaplerResponse2 response) th Credentials credentials = client.getCredentials(callContext) .orElseThrow(() -> new Failure("Could not extract credentials from request")); - credentials = client.getAuthenticator().validate(callContext, credentials).orElseThrow( - () -> new Failure("Could not " + "validate " + "credentials")); + credentials = client.getAuthenticator() + .validate(callContext, credentials) + .orElseThrow(() -> new Failure("Could not " + "validate " + "credentials")); ProfileCreator profileCreator = client.getProfileCreator(); diff --git a/src/main/java/org/jenkinsci/plugins/oic/OicdPluginOpMetadataResolver.java b/src/main/java/org/jenkinsci/plugins/oic/OicdPluginOpMetadataResolver.java index 6e0b6ead..57440b40 100644 --- a/src/main/java/org/jenkinsci/plugins/oic/OicdPluginOpMetadataResolver.java +++ b/src/main/java/org/jenkinsci/plugins/oic/OicdPluginOpMetadataResolver.java @@ -9,8 +9,8 @@ public class OicdPluginOpMetadataResolver extends StaticOidcOpMetadataResolver { private boolean allowsAnything; - public OicdPluginOpMetadataResolver(OidcConfiguration configuration, OIDCProviderMetadata staticMetadata, - boolean allowsAnything) { + public OicdPluginOpMetadataResolver( + OidcConfiguration configuration, OIDCProviderMetadata staticMetadata, boolean allowsAnything) { super(configuration, staticMetadata); this.allowsAnything = allowsAnything; } From 121bd1760a119a5dc0a6ddb6d5fcaef901400a89 Mon Sep 17 00:00:00 2001 From: Francisco Javier Fernandez Gonzalez Date: Wed, 16 Oct 2024 16:17:44 +0200 Subject: [PATCH 09/16] Refactor credentials --- .../jenkinsci/plugins/oic/OicSecurityRealm.java | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java index a124995f..475a7566 100644 --- a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java +++ b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java @@ -1286,12 +1286,7 @@ public void doFinishLogin(StaplerRequest2 request, StaplerResponse2 response) th // Jenkins stuff correctly // also should have its own URL to make the code easier to follow :) - Credentials credentials = client.getCredentials(callContext) - .orElseThrow(() -> new Failure("Could not extract credentials from request")); - - credentials = client.getAuthenticator() - .validate(callContext, credentials) - .orElseThrow(() -> new Failure("Could not " + "validate " + "credentials")); + Credentials credentials = getCredentials(client, callContext); ProfileCreator profileCreator = client.getProfileCreator(); @@ -1331,6 +1326,15 @@ public void doFinishLogin(StaplerRequest2 request, StaplerResponse2 response) th } } + private Credentials getCredentials(OidcClient client, CallContext callContext) { + Credentials credentials = client.getCredentials(callContext) + .orElseThrow(() -> new Failure("Could not extract credentials from request")); + + return client.getAuthenticator() + .validate(callContext, credentials) + .orElseThrow(() -> new Failure("Could not " + "validate " + "credentials")); + } + /** * Handles Token Expiration. * @throws IOException a low level exception From 3c2260f88c0bc78497f0b0b24bbe512dd73a9c5f Mon Sep 17 00:00:00 2001 From: Francisco Javier Fernandez Gonzalez Date: Fri, 18 Oct 2024 11:29:28 +0200 Subject: [PATCH 10/16] Update .gitignore --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index d93daae7..02fb8b07 100644 --- a/.gitignore +++ b/.gitignore @@ -8,3 +8,6 @@ target/ /.apt_generated/ .*.swp + +work-cognito/ +work-keycloak/ \ No newline at end of file From 6792218db4e7c0c1ffbcf1f55c6787530049ba01 Mon Sep 17 00:00:00 2001 From: Pankaj Yadav Date: Fri, 20 Dec 2024 16:44:21 +0530 Subject: [PATCH 11/16] [JENKINS-75056] pac4j upgrade in pom --- pom.xml | 614 +++++++++++++++++++++++++++++--------------------------- 1 file changed, 321 insertions(+), 293 deletions(-) diff --git a/pom.xml b/pom.xml index c4df1c3a..599be1d1 100644 --- a/pom.xml +++ b/pom.xml @@ -1,308 +1,336 @@ - 4.0.0 - - org.jenkins-ci.plugins - plugin - 5.1 - - + 4.0.0 + + org.jenkins-ci.plugins + plugin + 5.3 + + - oic-auth - ${revision}.${changelist} - hpi + oic-auth + ${revision}.${changelist} + hpi - OpenId Connect Authentication Plugin - https://github.com/jenkinsci/${project.artifactId}-plugin + OpenId Connect Authentication Plugin + https://github.com/jenkinsci/${project.artifactId}-plugin - - - MIT License - https://opensource.org/licenses/MIT - - - - - mbischoff - Michael Bischoff - m.bischoff@controplex.com - - - agentgonzo - Steve Arch - sarch@cloudbees.com - - + + + MIT License + https://opensource.org/licenses/MIT + + + + + mbischoff + Michael Bischoff + m.bischoff@controplex.com + + + agentgonzo + Steve Arch + sarch@cloudbees.com + + - - scm:git:https://github.com/${gitHubRepo}.git - scm:git:git@github.com:${gitHubRepo}.git - ${scmTag} - https://github.com/${gitHubRepo} - + + scm:git:https://github.com/${gitHubRepo}.git + scm:git:git@github.com:${gitHubRepo}.git + ${scmTag} + https://github.com/${gitHubRepo} + - - 4 - 999999-SNAPSHOT - jenkinsci/${project.artifactId}-plugin - false - Max - 1836.vccda_4a_122a_a_e - 4.383 - 6.0.6 - + + 4 + 999999-SNAPSHOT + jenkinsci/${project.artifactId}-plugin + false + Max + 1836.vccda_4a_122a_a_e + 4.383 + + 6.1.0 + 6.1.14 + - - - - io.jenkins.tools.bom - - bom-2.426.x - 3208.vb_21177d4b_cd9 - pom - import - - - - com.nimbusds - nimbus-jose-jwt - 9.41 - - - - - com.github.stephenc.jcip - jcip-annotations - provided - - - + + + + io.jenkins.tools.bom + bom-2.426.x + 3208.vb_21177d4b_cd9 + pom + import + + + + com.nimbusds + nimbus-jose-jwt + 9.47 + + + + + com.github.stephenc.jcip + jcip-annotations + provided + + + - - - io.burt - jmespath-core - 0.6.0 - - - io.jenkins.plugins - asm-api - - - org.jenkins-ci.plugins - jackson2-api - - - org.jenkins-ci.plugins - mailer - + + + com.fasterxml.jackson.core + jackson-annotations + 2.18.1 + + + com.fasterxml.jackson.core + jackson-core + 2.18.1 + + + + com.fasterxml.jackson.core + jackson-databind + 2.18.1 + + + com.fasterxml.jackson.datatype + jackson-datatype-jsr310 + 2.18.1 + + + io.burt + jmespath-core + 0.6.0 + + + io.jenkins.plugins + asm-api + + + org.jenkins-ci.plugins + jackson2-api + + + org.jenkins-ci.plugins + mailer + - - org.pac4j - - pac4j-jakartaee - ${pac4jVersion} - - - - com.fasterxml.jackson.core - jackson-databind - - - com.google.guava - guava - - - - org.ow2.asm - asm - - - - org.slf4j - slf4j-api - - - - - org.pac4j - pac4j-oidc - ${pac4jVersion} - - - - - com.fasterxml.jackson.core - jackson-databind - - - com.google.guava - guava - - - - - com.google.code.gson - gson - 2.11.0 - test - - - com.google.oauth-client - google-oauth-client - 1.36.0 - test - - - com.google.guava - guava - - - - - io.jenkins.configuration-as-code - test-harness - test - - - org.apache.httpcomponents - httpclient - 4.5.14 - test - - - org.mockito - mockito-core - test - - - org.mockito - mockito-junit-jupiter - test - - - org.wiremock - wiremock-standalone - 3.9.1 - test - - + + org.pac4j + + pac4j-jakartaee + ${pac4jVersion} + + + com.google.guava + guava + + + + org.ow2.asm + asm + + + + org.slf4j + slf4j-api + + + + + + org.pac4j + pac4j-oidc + ${pac4jVersion} + + + + + com.fasterxml.jackson.core + jackson-databind + + + com.google.guava + guava + + + + + org.springframework + spring-core + ${springVersion} + + + org.springframework + spring-jcl + ${springVersion} + + + com.google.code.gson + gson + 2.11.0 + test + + + com.google.oauth-client + google-oauth-client + 1.36.0 + test + + + com.google.guava + guava + + + + + io.jenkins.configuration-as-code + test-harness + test + + + org.apache.httpcomponents + httpclient + 4.5.14 + test + + + org.mockito + mockito-core + test + + + org.mockito + mockito-junit-jupiter + test + + + org.wiremock + wiremock-standalone + 3.10.0 + test + + - - - repo.jenkins-ci.org - https://repo.jenkins-ci.org/public/ - - + + + repo.jenkins-ci.org + https://repo.jenkins-ci.org/public/ + + - - - repo.jenkins-ci.org - https://repo.jenkins-ci.org/public/ - - + + + repo.jenkins-ci.org + https://repo.jenkins-ci.org/public/ + + - - - - - - org.eclipse.m2e - lifecycle-mapping - 1.0.0 - - - - - - - org.apache.maven.plugins - maven-checkstyle-plugin - [3.4.0,) - - check - - - - - - - - - - - - - - - maven-release-plugin - 3.1.1 - - - com.diffplug.spotless - spotless-maven-plugin - - - - - true - - - - - - ,\# - - - - - - maven-checkstyle-plugin - 3.5.0 - - ${maven.multiModuleProjectDirectory}/.mvn/checkstyle.xml - ${maven.multiModuleProjectDirectory}/.mvn/checkstyle-suppressions.xml - true - true - - - - com.puppycrawl.tools - checkstyle - 10.18.2 - - - - - validate - - check - - validate - - - - - org.openrewrite.maven - rewrite-maven-plugin - 5.42.2 - - - org.openrewrite.jenkins.github.AddTeamToCodeowners - - - - - org.openrewrite.recipe - rewrite-jenkins - 0.14.1 - - - - - + + + + + + org.eclipse.m2e + lifecycle-mapping + 1.0.0 + + + + + + + org.apache.maven.plugins + maven-checkstyle-plugin + [3.4.0,) + + check + + + + + + + + + + + + + + + maven-release-plugin + 3.1.1 + + + com.diffplug.spotless + spotless-maven-plugin + + + + + true + + + + + + ,\# + + + + + + maven-checkstyle-plugin + 3.6.0 + + ${maven.multiModuleProjectDirectory}/.mvn/checkstyle.xml + ${maven.multiModuleProjectDirectory}/.mvn/checkstyle-suppressions.xml + true + true + + + + com.puppycrawl.tools + checkstyle + 10.20.2 + + + + + validate + + check + + validate + + + + + org.openrewrite.maven + rewrite-maven-plugin + 5.46.2 + + + org.openrewrite.jenkins.github.AddTeamToCodeowners + + + + + org.openrewrite.recipe + rewrite-jenkins + 0.18.1 + + + + + From 19b6ce157dbed0b3469b8cde8df21f04b3ebd8ed Mon Sep 17 00:00:00 2001 From: Pankaj Yadav Date: Fri, 20 Dec 2024 17:56:36 +0530 Subject: [PATCH 12/16] [JENKINS-75056] Spotless fix and code refactor --- pom.xml | 390 +++++++++--------- .../plugins/oic/OicSecurityRealm.java | 15 +- 2 files changed, 203 insertions(+), 202 deletions(-) diff --git a/pom.xml b/pom.xml index c0ee134c..35bc33c2 100644 --- a/pom.xml +++ b/pom.xml @@ -1,58 +1,58 @@ - 4.0.0 - - org.jenkins-ci.plugins - plugin - 5.3 - - + 4.0.0 + + org.jenkins-ci.plugins + plugin + 5.3 + + - oic-auth - ${revision}.${changelist} - hpi + oic-auth + ${revision}.${changelist} + hpi - OpenId Connect Authentication Plugin - https://github.com/jenkinsci/${project.artifactId}-plugin + OpenId Connect Authentication Plugin + https://github.com/jenkinsci/${project.artifactId}-plugin - - - MIT License - https://opensource.org/licenses/MIT - - - - - mbischoff - Michael Bischoff - m.bischoff@controplex.com - - - agentgonzo - Steve Arch - sarch@cloudbees.com - - + + + MIT License + https://opensource.org/licenses/MIT + + + + + mbischoff + Michael Bischoff + m.bischoff@controplex.com + + + agentgonzo + Steve Arch + sarch@cloudbees.com + + - - scm:git:https://github.com/${gitHubRepo}.git - scm:git:git@github.com:${gitHubRepo}.git - ${scmTag} - https://github.com/${gitHubRepo} - + + scm:git:https://github.com/${gitHubRepo}.git + scm:git:git@github.com:${gitHubRepo}.git + ${scmTag} + https://github.com/${gitHubRepo} + - - 4 - 999999-SNAPSHOT - jenkinsci/${project.artifactId}-plugin - false - Max - 1836.vccda_4a_122a_a_e - 4.383 - - 6.1.0 - 6.1.14 - + + 4 + 999999-SNAPSHOT + jenkinsci/${project.artifactId}-plugin + false + Max + 1836.vccda_4a_122a_a_e + 4.383 + + 6.1.0 + 6.1.14 + @@ -82,156 +82,156 @@ - - - com.fasterxml.jackson.core - jackson-annotations - 2.18.1 - - - com.fasterxml.jackson.core - jackson-core - 2.18.1 - - - - com.fasterxml.jackson.core - jackson-databind - 2.18.1 - - - com.fasterxml.jackson.datatype - jackson-datatype-jsr310 - 2.18.1 - - - io.burt - jmespath-core - 0.6.0 - - - io.jenkins.plugins - asm-api - - - org.jenkins-ci.plugins - jackson2-api - - - org.jenkins-ci.plugins - mailer - + + + com.fasterxml.jackson.core + jackson-annotations + 2.18.1 + + + com.fasterxml.jackson.core + jackson-core + 2.18.1 + + + + com.fasterxml.jackson.core + jackson-databind + 2.18.1 + + + com.fasterxml.jackson.datatype + jackson-datatype-jsr310 + 2.18.1 + + + io.burt + jmespath-core + 0.6.0 + + + io.jenkins.plugins + asm-api + + + org.jenkins-ci.plugins + jackson2-api + + + org.jenkins-ci.plugins + mailer + - - org.pac4j - - pac4j-jakartaee - ${pac4jVersion} - - - com.google.guava - guava - - - - org.ow2.asm - asm - - - - org.slf4j - slf4j-api - - - - - - org.pac4j - pac4j-oidc - ${pac4jVersion} - - - - - com.fasterxml.jackson.core - jackson-databind - - - com.google.guava - guava - - - - - org.springframework - spring-core - ${springVersion} - - - org.springframework - spring-jcl - ${springVersion} - - - com.google.code.gson - gson - 2.11.0 - test - - - com.google.oauth-client - google-oauth-client - 1.36.0 - test - - - com.google.guava - guava - - - - - io.jenkins.configuration-as-code - test-harness - test - - - org.apache.httpcomponents - httpclient - 4.5.14 - test - - - org.mockito - mockito-core - test - - - org.mockito - mockito-junit-jupiter - test - - - org.wiremock - wiremock-standalone - 3.10.0 - test - - + + org.pac4j + + pac4j-jakartaee + ${pac4jVersion} + + + com.google.guava + guava + + + + org.ow2.asm + asm + + + + org.slf4j + slf4j-api + + + + + + org.pac4j + pac4j-oidc + ${pac4jVersion} + + + + + com.fasterxml.jackson.core + jackson-databind + + + com.google.guava + guava + + + + + org.springframework + spring-core + ${springVersion} + + + org.springframework + spring-jcl + ${springVersion} + + + com.google.code.gson + gson + 2.11.0 + test + + + com.google.oauth-client + google-oauth-client + 1.36.0 + test + + + com.google.guava + guava + + + + + io.jenkins.configuration-as-code + test-harness + test + + + org.apache.httpcomponents + httpclient + 4.5.14 + test + + + org.mockito + mockito-core + test + + + org.mockito + mockito-junit-jupiter + test + + + org.wiremock + wiremock-standalone + 3.10.0 + test + + - - - repo.jenkins-ci.org - https://repo.jenkins-ci.org/public/ - - + + + repo.jenkins-ci.org + https://repo.jenkins-ci.org/public/ + + - - - repo.jenkins-ci.org - https://repo.jenkins-ci.org/public/ - - + + + repo.jenkins-ci.org + https://repo.jenkins-ci.org/public/ + + diff --git a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java index 561e4e99..36c59d3c 100644 --- a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java +++ b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java @@ -964,7 +964,6 @@ public void doCommenceLogin(@QueryParameter String from, @Header("Referer") fina CallContext callContext = new CallContext(webContext, sessionStore); RedirectionAction redirectionAction = builder.getRedirectionAction(callContext).orElseThrow(); - // store the redirect url for after the login. sessionStore.set(webContext, SESSION_POST_LOGIN_REDIRECT_URL_KEY, redirectOnFinish); JEEHttpActionAdapter.INSTANCE.adapt(redirectionAction, webContext); @@ -1286,6 +1285,11 @@ public void doFinishLogin(StaplerRequest2 request, StaplerResponse2 response) th // Jenkins stuff correctly // also should have its own URL to make the code easier to follow :) + + if (!sessionStore.renewSession(webContext)) { + throw new TechnicalException("Could not create a new session"); + } + Credentials credentials = getCredentials(client, callContext); ProfileCreator profileCreator = client.getProfileCreator(); @@ -1397,12 +1401,10 @@ public boolean handleTokenExpiration(HttpServletRequest httpRequest, HttpServlet } private void redirectToLoginUrl(HttpServletRequest req, HttpServletResponse res) throws IOException { - if (req != null && (req.getSession(false) != null || Strings.isNullOrEmpty(req.getHeader("Authorization")))) { + if (req.getSession(false) != null || Strings.isNullOrEmpty(req.getHeader("Authorization"))) { req.getSession().invalidate(); } - if (res != null) { - res.sendRedirect(Jenkins.get().getSecurityRealm().getLoginUrl()); - } + res.sendRedirect(Jenkins.get().getSecurityRealm().getLoginUrl()); } public boolean isExpired(OicCredentials credentials) { @@ -1500,8 +1502,7 @@ private boolean refreshExpiredToken( return false; } LOGGER.log(Level.FINE, "Failed to refresh expired token", e); - redirectToLoginUrl(Stapler.getCurrentRequest2(), Stapler.getCurrentResponse2()); - // Verify this latest change if we need it or notredirectToLoginUrl(httpRequest, httpResponse); + redirectToLoginUrl(httpRequest, httpResponse); return false; } LOGGER.log(Level.WARNING, "Failed to refresh expired token", e); From 5e2f1edce802f789d83d32e6f1c567e0f34c74db Mon Sep 17 00:00:00 2001 From: Pankaj Yadav Date: Mon, 23 Dec 2024 15:34:52 +0530 Subject: [PATCH 13/16] [JENKINS-75056] spotless and use variable for version in pom --- pom.xml | 23 +++++-------------- .../plugins/oic/OicSecurityRealm.java | 1 - 2 files changed, 6 insertions(+), 18 deletions(-) diff --git a/pom.xml b/pom.xml index 35bc33c2..118b9a4d 100644 --- a/pom.xml +++ b/pom.xml @@ -52,6 +52,7 @@ 6.1.0 6.1.14 + 2.18.1 @@ -63,16 +64,6 @@ pom import - - - com.nimbusds - nimbus-jose-jwt - 9.47 - - com.github.stephenc.jcip @@ -86,23 +77,22 @@ com.fasterxml.jackson.core jackson-annotations - 2.18.1 + ${jacksonVersion} com.fasterxml.jackson.core jackson-core - 2.18.1 + ${jacksonVersion} - com.fasterxml.jackson.core jackson-databind - 2.18.1 + ${jacksonVersion} com.fasterxml.jackson.datatype jackson-datatype-jsr310 - 2.18.1 + ${jacksonVersion} io.burt @@ -124,7 +114,6 @@ org.pac4j - pac4j-jakartaee ${pac4jVersion} @@ -144,7 +133,6 @@ - org.pac4j pac4j-oidc @@ -162,6 +150,7 @@ + org.springframework spring-core diff --git a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java index 36c59d3c..d25e145f 100644 --- a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java +++ b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java @@ -1285,7 +1285,6 @@ public void doFinishLogin(StaplerRequest2 request, StaplerResponse2 response) th // Jenkins stuff correctly // also should have its own URL to make the code easier to follow :) - if (!sessionStore.renewSession(webContext)) { throw new TechnicalException("Could not create a new session"); } From 13c009953cb0ccf32a577978c18c4a389354c86f Mon Sep 17 00:00:00 2001 From: Pankaj Yadav Date: Mon, 23 Dec 2024 19:11:41 +0530 Subject: [PATCH 14/16] [JENKINS-75056] Excluded the libraries coming from other dependencies like jackson lib coming from jackson2 plugin --- pom.xml | 48 +++++++++++++++++------------------------------- 1 file changed, 17 insertions(+), 31 deletions(-) diff --git a/pom.xml b/pom.xml index 118b9a4d..e10156a5 100644 --- a/pom.xml +++ b/pom.xml @@ -74,26 +74,6 @@ - - com.fasterxml.jackson.core - jackson-annotations - ${jacksonVersion} - - - com.fasterxml.jackson.core - jackson-core - ${jacksonVersion} - - - com.fasterxml.jackson.core - jackson-databind - ${jacksonVersion} - - - com.fasterxml.jackson.datatype - jackson-datatype-jsr310 - ${jacksonVersion} - io.burt jmespath-core @@ -117,6 +97,14 @@ pac4j-jakartaee ${pac4jVersion} + + com.fasterxml.jackson.core + jackson-databind + + + com.fasterxml.jackson.datatype + jackson-datatype-jsr310 + com.google.guava guava @@ -148,19 +136,18 @@ com.google.guava guava + + org.springframework + spring-core + + + org.springframework + spring-jcl + - - org.springframework - spring-core - ${springVersion} - - - org.springframework - spring-jcl - ${springVersion} - + com.google.code.gson gson @@ -321,5 +308,4 @@ - From 9d733bd9bf1a168928d33b016ad6b8faba547ab9 Mon Sep 17 00:00:00 2001 From: Pankaj Yadav <72126103+pankajy-dev@users.noreply.github.com> Date: Mon, 23 Dec 2024 19:16:13 +0530 Subject: [PATCH 15/16] Update src/main/java/org/jenkinsci/plugins/oic/OicdPluginOpMetadataResolver.java Co-authored-by: Francisco Javier Fernandez <31063239+fcojfernandez@users.noreply.github.com> --- .../org/jenkinsci/plugins/oic/OicdPluginOpMetadataResolver.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/org/jenkinsci/plugins/oic/OicdPluginOpMetadataResolver.java b/src/main/java/org/jenkinsci/plugins/oic/OicdPluginOpMetadataResolver.java index 57440b40..84451124 100644 --- a/src/main/java/org/jenkinsci/plugins/oic/OicdPluginOpMetadataResolver.java +++ b/src/main/java/org/jenkinsci/plugins/oic/OicdPluginOpMetadataResolver.java @@ -27,6 +27,7 @@ protected TokenValidator createTokenValidator() { /** * This method is needed as there seems to be a bug in pac4j and hasChanged is not able to return true * This will make it work until the bug is fixed. + * Link to the bug: https://stackoverflow.com/questions/79126478/pac4j-returns-an-error-refreshing-an-expired-token * TODO eventually remove */ @Override From fe936f143358436577b336d5d00e11479186ed3f Mon Sep 17 00:00:00 2001 From: Pankaj Yadav Date: Tue, 24 Dec 2024 14:32:26 +0530 Subject: [PATCH 16/16] [JENKINS-75056] Code review changes --- .../org/jenkinsci/plugins/oic/OicSecurityRealm.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java index d25e145f..5dfc6301 100644 --- a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java +++ b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java @@ -1399,18 +1399,20 @@ public boolean handleTokenExpiration(HttpServletRequest httpRequest, HttpServlet return true; } - private void redirectToLoginUrl(HttpServletRequest req, HttpServletResponse res) throws IOException { - if (req.getSession(false) != null || Strings.isNullOrEmpty(req.getHeader("Authorization"))) { + private void redirectToLoginUrl(@NonNull HttpServletRequest req, @NonNull HttpServletResponse res) + throws IOException { + if (req != null && req.getSession(false) != null || Strings.isNullOrEmpty(req.getHeader("Authorization"))) { req.getSession().invalidate(); } - res.sendRedirect(Jenkins.get().getSecurityRealm().getLoginUrl()); + if (res != null) { + res.sendRedirect(Jenkins.get().getSecurityRealm().getLoginUrl()); + } } public boolean isExpired(OicCredentials credentials) { if (credentials.getExpiresAtMillis() == null) { return false; } - return CLOCK.millis() >= credentials.getExpiresAtMillis(); }