Skip to content

Commit

Permalink
Resolve direct access to avatar from profile scope
Browse files Browse the repository at this point in the history
  • Loading branch information
@jonesbusy
jonesbusy authored and Valentin Delaye committed Feb 18, 2025
1 parent 51cab02 commit 6149b12
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 122 deletions.
50 changes: 9 additions & 41 deletions src/main/java/org/jenkinsci/plugins/oic/OicAvatarProperty.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,12 @@

import edu.umd.cs.findbugs.annotations.NonNull;
import hudson.Extension;
import hudson.model.Action;
import hudson.model.User;
import hudson.model.UserProperty;
import hudson.model.UserPropertyDescriptor;
import java.io.File;
import java.io.FileInputStream;
import java.util.logging.Level;
import java.util.logging.Logger;
import jenkins.model.Jenkins;
import org.kohsuke.stapler.StaplerRequest2;
import org.kohsuke.stapler.StaplerResponse2;

public class OicAvatarProperty extends UserProperty implements Action {
public class OicAvatarProperty extends UserProperty {
private static final Logger LOGGER = Logger.getLogger(OicAvatarProperty.class.getName());

private final AvatarImage avatarImage;
Expand All @@ -31,37 +24,13 @@ public String getAvatarUrl() {
}

private String getAvatarImageUrl() {
return "%s%s/%s/image".formatted(Jenkins.get().getRootUrl(), user.getUrl(), getUrlName());
return avatarImage.url;
}

public boolean isHasAvatar() {
return avatarImage != null && avatarImage.isValid();

Check warning on line 31 in src/main/java/org/jenkinsci/plugins/oic/OicAvatarProperty.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line 

Line 31 is only partially covered, one branch is missing
}

/**
* Used to serve images as part of {@link OicAvatarResolver}.
*/
public void doImage(StaplerRequest2 req, StaplerResponse2 rsp) {
if (avatarImage == null) {
LOGGER.log(Level.WARNING, "No image set for user '" + user.getId() + "'");
return;
}

String imageFileName = "oic-avatar." + avatarImage.getFilenameSuffix();
File file = new File(user.getUserFolder(), imageFileName);
if (!file.exists()) {
LOGGER.log(Level.WARNING, "Avatar image for user '" + user.getId() + "' does not exist");
return;
}

try (FileInputStream fileInputStream = new FileInputStream(file); ) {
rsp.setContentType(avatarImage.mimeType);
rsp.serveFile(req, fileInputStream, file.lastModified(), file.length(), imageFileName);
} catch (Exception e) {
LOGGER.log(Level.SEVERE, "Unable to write image for user '" + user.getId() + "'", e);
}
}

public String getDisplayName() {
return "Openid Connect Avatar";
}
Expand Down Expand Up @@ -94,19 +63,18 @@ public UserProperty newInstance(User user) {
}
}

/**
* OIC avatar is standard picture field on the profile claim.
*/
public static class AvatarImage {
private final String mimeType;

public AvatarImage(String mimeType) {
this.mimeType = mimeType;
}
private final String url;

public String getFilenameSuffix() {
return mimeType.split("/")[1].split("\\+")[0];
public AvatarImage(String url) {
this.url = url;
}

public boolean isValid() {
return mimeType != null;
return url != null;

Check warning on line 77 in src/main/java/org/jenkinsci/plugins/oic/OicAvatarProperty.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line 

Line 77 is only partially covered, one branch is missing
}
}
}
71 changes: 2 additions & 69 deletions src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import hudson.Extension;
import hudson.ExtensionList;
import hudson.ProxyConfiguration;
import hudson.Util;
import hudson.model.Descriptor;
import hudson.model.Descriptor.FormException;
Expand All @@ -66,10 +65,7 @@
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import jakarta.servlet.http.HttpSession;
import java.io.ByteArrayInputStream;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.io.InvalidObjectException;
import java.io.ObjectStreamException;
import java.io.Serializable;
Expand All @@ -79,19 +75,12 @@
import java.net.URISyntaxException;
import java.net.URL;
import java.net.URLEncoder;
import java.net.http.HttpClient;
import java.net.http.HttpRequest;
import java.net.http.HttpResponse;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.StandardOpenOption;
import java.text.ParseException;
import java.time.Clock;
import java.time.Duration;
import java.util.ArrayList;
import java.util.Base64;
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand All @@ -100,8 +89,6 @@
import java.util.logging.Logger;
import java.util.regex.Pattern;
import javax.annotation.PostConstruct;
import javax.imageio.ImageIO;
import javax.imageio.ImageReader;
import jenkins.model.IdStrategy;
import jenkins.model.IdStrategyDescriptor;
import jenkins.model.Jenkins;
Expand Down Expand Up @@ -1092,18 +1079,9 @@ private UsernamePasswordAuthenticationToken loginAndSetUserData(
try {
String avatarUrl = determineStringField(avatarFieldExpr, idToken, userInfo);
if (avatarUrl != null) {
byte[] decodedAvatar = downloadAvatar(avatarUrl);

// Try to determine the content type of the avatar
String contentType = determineAvatarContentType(decodedAvatar);
LOGGER.finest("Avatar content type: " + contentType);

OicAvatarProperty.AvatarImage avatarImage = new OicAvatarProperty.AvatarImage(contentType);
LOGGER.finest("Avatar url is: " + avatarUrl);
OicAvatarProperty.AvatarImage avatarImage = new OicAvatarProperty.AvatarImage(avatarUrl);
OicAvatarProperty oicAvatarProperty = new OicAvatarProperty(avatarImage);
File targetFile = new File(user.getUserFolder(), "oic-avatar." + avatarImage.getFilenameSuffix());

Files.write(targetFile.toPath(), decodedAvatar, StandardOpenOption.CREATE, StandardOpenOption.WRITE);
LOGGER.finest("Saved avatar");
user.addProperty(oicAvatarProperty);
} else {
LOGGER.finest("No avatar URL found");
Expand All @@ -1122,51 +1100,6 @@ private UsernamePasswordAuthenticationToken loginAndSetUserData(
return token;
}

private static byte[] downloadAvatar(String avatarUrl) throws Exception {
if (StringUtils.isBlank(avatarUrl)) {
throw new IllegalStateException("Avatar URL is null or empty");
}
if (!avatarUrl.startsWith("http") && !avatarUrl.startsWith("https")) {
throw new IllegalStateException("Avatar URL is not a valid URL");
}
URI url = new URI(avatarUrl);
HttpClient client = ProxyConfiguration.newHttpClientBuilder()
.followRedirects(HttpClient.Redirect.NORMAL)
.connectTimeout(Duration.ofSeconds(5))
.build();
HttpRequest request = ProxyConfiguration.newHttpRequestBuilder(url)
.timeout(Duration.ofSeconds(5))
.GET()
.build();
HttpResponse<byte[]> resp = client.send(request, HttpResponse.BodyHandlers.ofByteArray());
return resp.body();
}

private String determineAvatarContentType(byte[] avatar) {
try (InputStream is = new ByteArrayInputStream(avatar)) {
Iterator<ImageReader> iterator = ImageIO.getImageReaders(ImageIO.createImageInputStream(is));
if (!iterator.hasNext()) {
LOGGER.warning("Failed to determine avatar content type. Falling back to image/png");
return "image/png";
}
String formatName = iterator.next().getFormatName();
switch (formatName.toLowerCase()) {
case "jpg":
case "jpeg":
return "image/jpeg";
case "png":
return "image/png";
case "gif":
return "image/gif";
default:
return "image/png";
}
} catch (IOException e) {
LOGGER.log(Level.WARNING, "Failed to determine avatar content type. Falling back to image/png", e);
return "image/png";
}
}

private String determineStringField(Expression<Object> fieldExpr, JWT idToken, Map userInfo) throws ParseException {
if (fieldExpr != null) {
if (userInfo != null) {
Expand Down
15 changes: 3 additions & 12 deletions src/test/java/org/jenkinsci/plugins/oic/PluginTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,12 +229,6 @@ private void browseLoginPage() throws IOException, SAXException {
webClient.goTo(jenkins.getSecurityRealm().getLoginUrl());
}

private void browseAvatarImagePage(User user, String contentType) throws IOException, SAXException {
String expectedAvatarUrl =
"user/%s/oic-avatar/image".formatted(user.getId().toLowerCase());
webClient.goTo(expectedAvatarUrl, contentType);
}

private void configureTestRealm(@NonNull Consumer<OicSecurityRealm> consumer) throws Exception {
var securityRealm = new TestRealm(wireMockRule);
consumer.accept(securityRealm);
Expand Down Expand Up @@ -374,8 +368,7 @@ public void testLoginUsingUserInfoEndpointWithAvatar() throws Exception {
browseLoginPage();
var user = assertTestUser();
assertTestUserEmail(user);
assertTestAvatar(jenkins, user);
browseAvatarImagePage(user, "image/png"); // Image is browsable
assertTestAvatar(user, wireMockRule);
}

@Test
Expand All @@ -389,7 +382,6 @@ public void testLoginWithMinimalConfiguration() throws Exception {
var user = assertTestUser();
assertTrue(
"User should be not be part of any group", user.getAuthorities().isEmpty());
browseAvatarImagePage(user, null);
}

@Test
Expand Down Expand Up @@ -866,9 +858,8 @@ private static void assertTestUserEmail(User user) {
user.getProperty(Mailer.UserProperty.class).getAddress());
}

private static void assertTestAvatar(Jenkins jenkins, User user) {
String expectedAvatarUrl = "%suser/%s/oic-avatar/image"
.formatted(jenkins.getRootUrl(), user.getId().toLowerCase());
private static void assertTestAvatar(User user, WireMockRule wireMockRule) {
String expectedAvatarUrl = "http://localhost:%s/my-avatar.png".formatted(wireMockRule.port());
OicAvatarProperty avatarProperty = user.getProperty(OicAvatarProperty.class);
assertEquals("Avatar url should be " + expectedAvatarUrl, expectedAvatarUrl, avatarProperty.getAvatarUrl());
assertEquals("Openid Connect Avatar", avatarProperty.getDisplayName());
Expand Down

0 comments on commit 6149b12

Please sign in to comment.