Skip to content

Commit 6cc620b

Browse files
kuisathaveratkuisathaverat
authoredOct 23, 2022
[JENKINS-61747] Do not write the SP metadata with every login (#98)
* test: add test for the cache class * chore: dual implumentations * feat: make the selection configurable * fix: reimplement tests * Delete settings.json * fix: NPE when advanced config is not set
·
4.525.v4f6a_7209447e4.363.v6cc620b_d37a_2
1 parent 3902210 commit 6cc620b

File tree

8 files changed

+553
-29
lines changed

8 files changed

+553
-29
lines changed
 

‎src/main/java/org/jenkinsci/plugins/saml/SamlAdvancedConfiguration.java

+14-1
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import edu.umd.cs.findbugs.annotations.NonNull;
2121
import org.apache.commons.lang.StringUtils;
2222
import org.kohsuke.stapler.DataBoundConstructor;
23+
import org.kohsuke.stapler.DataBoundSetter;
2324
import hudson.Extension;
2425
import hudson.Util;
2526
import hudson.model.AbstractDescribableImpl;
@@ -37,6 +38,8 @@ public class SamlAdvancedConfiguration extends AbstractDescribableImpl<SamlAdvan
3738
private final String spEntityId;
3839
private final String nameIdPolicyFormat;
3940

41+
private Boolean useDiskCache = false;
42+
4043
@DataBoundConstructor
4144
public SamlAdvancedConfiguration(Boolean forceAuthn,
4245
String authnContextClassRef,
@@ -64,12 +67,22 @@ public String getNameIdPolicyFormat() {
6467
return nameIdPolicyFormat;
6568
}
6669

70+
public Boolean getUseDiskCache() {
71+
return useDiskCache;
72+
}
73+
74+
@DataBoundSetter
75+
public void setUseDiskCache(Boolean useDiskCache) {
76+
this.useDiskCache = useDiskCache;
77+
}
78+
6779
@Override
6880
public String toString() {
6981
return "SamlAdvancedConfiguration{" + "forceAuthn=" + getForceAuthn() + ", authnContextClassRef='"
7082
+ StringUtils.defaultIfBlank(getAuthnContextClassRef(), "none") + '\'' + ", spEntityId='"
7183
+ StringUtils.defaultIfBlank(getSpEntityId(), "none") + '\'' + ", nameIdPolicyFormat='"
72-
+ StringUtils.defaultIfBlank(getNameIdPolicyFormat(), "none") + '\'' + '}';
84+
+ StringUtils.defaultIfBlank(getNameIdPolicyFormat(), "none") + '\''
85+
+ "useDiskCache=" + getUseDiskCache() + '}';
7386
}
7487

7588
@SuppressWarnings("unused")

‎src/main/java/org/jenkinsci/plugins/saml/SamlFileResource.java

+36-26
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,11 @@
2121
import java.io.IOException;
2222
import java.io.InputStream;
2323
import java.io.OutputStream;
24-
import java.io.UnsupportedEncodingException;
2524
import java.net.URI;
2625
import java.net.URL;
27-
import java.nio.charset.StandardCharsets;
2826
import javax.annotation.Nonnull;
2927
import edu.umd.cs.findbugs.annotations.NonNull;
30-
import org.apache.commons.io.FileUtils;
3128
import org.apache.commons.lang.NotImplementedException;
32-
import org.pac4j.core.exception.TechnicalException;
3329
import org.springframework.core.io.Resource;
3430
import org.springframework.core.io.WritableResource;
3531

@@ -38,31 +34,45 @@
3834
*/
3935
class SamlFileResource implements WritableResource {
4036

41-
private final String fileName;
37+
private final WritableResource resource;
4238

4339
public SamlFileResource(@Nonnull String fileName) {
44-
this.fileName = fileName;
40+
if(getUseDiskCache()){
41+
this.resource = new SamlFileResourceCache(fileName);
42+
} else {
43+
this.resource = new SamlFileResourceDisk(fileName);
44+
}
4545
}
4646

4747
public SamlFileResource(@Nonnull String fileName, @Nonnull String data) {
48-
this.fileName = fileName;
49-
try {
50-
FileUtils.writeByteArrayToFile(getFile(), data.getBytes(StandardCharsets.UTF_8));
51-
} catch (UnsupportedEncodingException e) {
52-
throw new TechnicalException("Could not get string bytes.", e);
53-
} catch (java.io.IOException e) {
54-
throw new TechnicalException("Could not save the " + fileName + " file.", e);
48+
if(getUseDiskCache()){
49+
this.resource = new SamlFileResourceCache(fileName, data);
50+
} else {
51+
this.resource = new SamlFileResourceDisk(fileName, data);
52+
}
53+
}
54+
55+
private boolean getUseDiskCache() {
56+
boolean ret = false;
57+
jenkins.model.Jenkins j = jenkins.model.Jenkins.get();
58+
if (j.getSecurityRealm() instanceof SamlSecurityRealm) {
59+
SamlSecurityRealm samlSecurityRealm = (SamlSecurityRealm) j.getSecurityRealm();
60+
SamlAdvancedConfiguration config = samlSecurityRealm.getAdvancedConfiguration();
61+
if(config != null ) {
62+
ret = config.getUseDiskCache();
63+
}
5564
}
65+
return ret;
5666
}
5767

5868
@Override
5969
public boolean exists() {
60-
return getFile().exists();
70+
return resource.exists();
6171
}
6272

6373
@Override
6474
public boolean isReadable() {
65-
return getFile().canRead();
75+
return resource.isReadable();
6676
}
6777

6878
@Override
@@ -84,35 +94,35 @@ public URI getURI() {
8494

8595
@Override
8696
public String getFilename() {
87-
return fileName;
97+
return resource.getFilename();
8898
}
8999

90100
@NonNull
91101
@Override
92102
public String getDescription() {
93-
return fileName;
103+
return resource.getDescription();
94104
}
95105

96106
@NonNull
97107
@Override
98108
public InputStream getInputStream() throws IOException {
99-
return FileUtils.openInputStream(getFile());
109+
return resource.getInputStream();
100110
}
101111

102112
@NonNull
103113
@Override
104-
public File getFile() {
105-
return new File(fileName);
114+
public File getFile() throws IOException {
115+
return resource.getFile();
106116
}
107117

108118
@Override
109-
public long contentLength() {
110-
return getFile().length();
119+
public long contentLength() throws IOException {
120+
return resource.contentLength();
111121
}
112122

113123
@Override
114-
public long lastModified() {
115-
return getFile().lastModified();
124+
public long lastModified() throws IOException {
125+
return resource.lastModified();
116126
}
117127

118128
@NonNull
@@ -123,12 +133,12 @@ public Resource createRelative(@NonNull String s) {
123133

124134
@Override
125135
public boolean isWritable() {
126-
return getFile().canWrite();
136+
return resource.isWritable();
127137
}
128138

129139
@NonNull
130140
@Override
131141
public OutputStream getOutputStream() throws IOException {
132-
return FileUtils.openOutputStream(getFile());
142+
return resource.getOutputStream();
133143
}
134144
}

‎src/main/java/org/jenkinsci/plugins/saml/SamlFileResourceCache.java

+159
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,159 @@
1+
/* Licensed to Jenkins CI under one or more contributor license
2+
agreements. See the NOTICE file distributed with this work
3+
for additional information regarding copyright ownership.
4+
Jenkins CI licenses this file to you under the Apache License,
5+
Version 2.0 (the "License"); you may not use this file except
6+
in compliance with the License. You may obtain a copy of the
7+
License at
8+
9+
http://www.apache.org/licenses/LICENSE-2.0
10+
11+
Unless required by applicable law or agreed to in writing,
12+
software distributed under the License is distributed on an
13+
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
KIND, either express or implied. See the License for the
15+
specific language governing permissions and limitations
16+
under the License. */
17+
18+
package org.jenkinsci.plugins.saml;
19+
20+
import org.apache.commons.io.FileUtils;
21+
import org.apache.commons.io.IOUtils;
22+
import org.apache.commons.lang.NotImplementedException;
23+
import org.pac4j.core.exception.TechnicalException;
24+
import org.springframework.core.io.Resource;
25+
import org.springframework.core.io.WritableResource;
26+
27+
import javax.annotation.Nonnull;
28+
import java.io.ByteArrayOutputStream;
29+
import java.io.File;
30+
import java.io.IOException;
31+
import java.io.InputStream;
32+
import java.io.OutputStream;
33+
import java.io.UnsupportedEncodingException;
34+
import java.net.URI;
35+
import java.net.URL;
36+
import java.util.HashMap;
37+
import java.util.Map;
38+
import java.util.logging.Logger;
39+
40+
/**
41+
* Class to manage the metadata files using cache.
42+
* It will only write the files if the content is different.
43+
*/
44+
class SamlFileResourceCache implements WritableResource {
45+
46+
private static final Logger LOG = Logger.getLogger(SamlFileResource.class.getName());
47+
48+
private String fileName;
49+
50+
private final static Map<String,String> cache = new HashMap<>();
51+
52+
public SamlFileResourceCache(@Nonnull String fileName) {
53+
this.fileName = fileName;
54+
}
55+
56+
public SamlFileResourceCache(@Nonnull String fileName, @Nonnull String data) {
57+
this.fileName = fileName;
58+
try {
59+
save(fileName, data);
60+
} catch (UnsupportedEncodingException e) {
61+
throw new TechnicalException("Could not get string bytes.", e);
62+
} catch (java.io.IOException e) {
63+
throw new TechnicalException("Could not save the " + fileName + " file.", e);
64+
}
65+
}
66+
67+
@Override
68+
public boolean exists() {
69+
return new File(fileName).exists();
70+
}
71+
72+
@Override
73+
public boolean isReadable() {
74+
return new File(fileName).canRead();
75+
}
76+
77+
@Override
78+
public boolean isOpen() {
79+
return false;
80+
}
81+
82+
@Override
83+
public URL getURL() {
84+
throw new NotImplementedException();
85+
}
86+
87+
@Override
88+
public URI getURI() {
89+
throw new NotImplementedException();
90+
}
91+
92+
@Override
93+
public String getFilename() {
94+
return fileName;
95+
}
96+
97+
@Override
98+
public String getDescription() {
99+
return fileName;
100+
}
101+
102+
@Override
103+
public InputStream getInputStream() throws IOException {
104+
if (cache.containsKey(fileName)){
105+
return IOUtils.toInputStream(cache.get(fileName),"UTF-8");
106+
} else {
107+
return FileUtils.openInputStream(new File(fileName));
108+
}
109+
}
110+
111+
@Override
112+
public File getFile() {
113+
throw new NotImplementedException();
114+
}
115+
116+
@Override
117+
public long contentLength() {
118+
return new File(fileName).length();
119+
}
120+
121+
@Override
122+
public long lastModified() {
123+
return new File(fileName).lastModified();
124+
}
125+
126+
@Override
127+
public Resource createRelative(String s) {
128+
throw new NotImplementedException();
129+
}
130+
131+
@Override
132+
public boolean isWritable() {
133+
return new File(fileName).canWrite();
134+
}
135+
136+
@Override
137+
public OutputStream getOutputStream() throws IOException {
138+
return new ByteArrayOutputStream(){
139+
@Override
140+
public void close() throws IOException {
141+
save(fileName, IOUtils.toString(this.buf, "UTF-8").trim());
142+
}
143+
};
144+
}
145+
146+
private boolean isNew(String fileName, String data){
147+
String oldData = cache.containsKey(fileName) ? cache.get(fileName) : "";
148+
String md5SumNew = org.apache.commons.codec.digest.DigestUtils.md5Hex(data);
149+
String md5SumOld = org.apache.commons.codec.digest.DigestUtils.md5Hex(oldData);
150+
return !md5SumNew.equals(md5SumOld);
151+
}
152+
153+
private void save(@Nonnull String fileName, @Nonnull String data) throws IOException {
154+
if(isNew(fileName, data)) {
155+
FileUtils.writeByteArrayToFile(new File(fileName), data.getBytes("UTF-8"));
156+
cache.put(fileName, data);
157+
}
158+
}
159+
}

0 commit comments

Comments
 (0)
Please sign in to comment.