Skip to content

Commit 3850b15

Browse files
oharstaoharsta
committed
Fixes #686 
1 parent b1f7153 commit 3850b15

5 files changed

Lines changed: 97 additions & 25 deletions

File tree

‎dashboard-gui/pom.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
<parent>
33
<groupId>org.openconext</groupId>
44
<artifactId>dashboard</artifactId>
5-
<version>13.0.12</version>
5+
<version>13.0.13</version>
66
<relativePath>../pom.xml</relativePath>
77
</parent>
88
<modelVersion>4.0.0</modelVersion>

‎dashboard-server/pom.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
<parent>
2020
<groupId>org.openconext</groupId>
2121
<artifactId>dashboard</artifactId>
22-
<version>13.0.12</version>
22+
<version>13.0.13</version>
2323
<relativePath>../pom.xml</relativePath>
2424
</parent>
2525
<modelVersion>4.0.0</modelVersion>

‎dashboard-server/src/main/java/dashboard/stats/StatsImpl.java‎

Lines changed: 45 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,14 @@
22

33
import com.google.common.collect.ImmutableList;
44
import dashboard.control.Constants;
5+
import org.slf4j.Logger;
6+
import org.slf4j.LoggerFactory;
57
import org.springframework.beans.factory.annotation.Value;
68
import org.springframework.http.HttpHeaders;
79
import org.springframework.web.client.RestTemplate;
10+
import org.springframework.web.util.UriComponentsBuilder;
811

9-
import java.net.URLEncoder;
10-
import java.nio.charset.Charset;
12+
import java.net.URI;
1113
import java.util.List;
1214
import java.util.Optional;
1315

@@ -20,6 +22,8 @@ public class StatsImpl implements Stats, Constants {
2022
private final RestTemplate restTemplate;
2123
private final String baseUrl;
2224

25+
private static final Logger LOG = LoggerFactory.getLogger(StatsImpl.class);
26+
2327
public StatsImpl(@Value("${statsUser}") String user,
2428
@Value("${statsPassword}") String password,
2529
@Value("${statsBaseUrl}") String baseUrl) {
@@ -32,38 +36,57 @@ public StatsImpl(@Value("${statsUser}") String user,
3236
headers.setAccept(ImmutableList.of(APPLICATION_JSON));
3337
headers.set(AUTHORIZATION, authorizationHeaderValue(user, password));
3438

39+
if (LOG.isDebugEnabled()) {
40+
LOG.debug("Outgoing request URI: " + request.getURI());
41+
}
42+
3543
return execution.execute(request, body);
3644
}));
3745
}
3846

3947
public List<Object> loginTimeFrame(long from, long to, String scale, Optional<String> spEntityIdOptional) {
40-
String idp = currentIdPEncoded();
41-
StringBuilder url = new StringBuilder(String.format(
42-
"%s/public/login_time_frame?from=%s&to=%s&include_unique=true&scale=%s&epoch=ms&idp_id=%s",
43-
baseUrl, from, to, scale, idp));
44-
spEntityIdOptional.ifPresent(spEntityId -> url.append(String.format("&sp_id=%s", spEntityId)));
45-
return restTemplate.getForEntity(url.toString(), List.class).getBody();
46-
}
48+
UriComponentsBuilder builder = baseBuilder("/public/login_time_frame")
49+
.queryParam("from", from)
50+
.queryParam("to", to)
51+
.queryParam("scale", scale)
52+
.queryParam("epoch", "ms");
53+
spEntityIdOptional.ifPresent(sp -> builder.queryParam("sp_id", sp));
4754

48-
private String currentIdPEncoded() {
49-
return URLEncoder.encode(currentUserIdp(), Charset.defaultCharset());
55+
URI uri = builder.build().encode().toUri();
56+
return restTemplate.getForEntity(uri, List.class).getBody();
5057
}
5158

5259
public List<Object> loginAggregated(String period, Optional<String> spEntityIdOptional) {
53-
String idp = currentIdPEncoded();
54-
StringBuilder url = new StringBuilder(String.format(
55-
"%s/public/login_aggregated?period=%s&include_unique=true&idp_id=%s&group_by=sp_id",
56-
baseUrl, period, idp));
57-
spEntityIdOptional.ifPresent(spEntityId -> url.append(String.format("&sp_id=%s", spEntityId)));
58-
return restTemplate.getForEntity(url.toString(), List.class).getBody();
60+
UriComponentsBuilder builder = baseBuilder("/public/login_aggregated")
61+
.queryParam("period", period)
62+
.queryParam("group_by", "sp_id");
63+
spEntityIdOptional.ifPresent(sp -> builder.queryParam("sp_id", sp));
64+
65+
URI uri = builder.build().encode().toUri();
66+
return restTemplate.getForEntity(uri, List.class).getBody();
5967
}
6068

6169
public List<Object> uniqueLoginCount(long from, long to, String spEntityId) {
62-
String idp = currentIdPEncoded();
63-
String url = String.format(
64-
"%s/public/unique_login_count?from=%s&to=%s&include_unique=true&epoch=ms&idp_id=%s&sp_id=%s",
65-
baseUrl, from, to, idp, spEntityId);
66-
return restTemplate.getForEntity(url, List.class).getBody();
70+
UriComponentsBuilder builder = baseBuilder("/public/unique_login_count")
71+
.queryParam("from", from)
72+
.queryParam("to", to)
73+
.queryParam("epoch", "ms")
74+
.queryParam("sp_id", spEntityId);
75+
76+
URI uri = builder.build().encode().toUri();
77+
return restTemplate.getForEntity(uri, List.class).getBody();
78+
}
79+
80+
private UriComponentsBuilder baseBuilder(String path) {
81+
return UriComponentsBuilder
82+
.fromHttpUrl(baseUrl)
83+
.path(path)
84+
.queryParam("include_unique", true)
85+
.queryParam("idp_id", getCurrentUserIdp());
86+
}
87+
88+
protected String getCurrentUserIdp() {
89+
return currentUserIdp();
6790
}
6891

6992
}

‎dashboard-server/src/test/java/dashboard/stats/StatsImplTest.java‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
package dashboard.stats;
2+
3+
import com.github.tomakehurst.wiremock.junit.WireMockRule;
4+
import com.github.tomakehurst.wiremock.stubbing.ServeEvent;
5+
import org.junit.Rule;
6+
import org.junit.Test;
7+
8+
import java.util.List;
9+
import java.util.Optional;
10+
11+
import static com.github.tomakehurst.wiremock.client.WireMock.*;
12+
import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo;
13+
import static org.junit.Assert.assertEquals;
14+
import static org.junit.Assert.assertNotNull;
15+
16+
public class StatsImplTest {
17+
18+
private final String idp = "http://login.surf.nl/adfs/services/trust?q=1&k=2";
19+
20+
@Rule
21+
public WireMockRule wireMockRule = new WireMockRule(8891);
22+
23+
private final StatsImpl stats = new StatsImpl("user", "password","http://localhost:8891"){
24+
@Override
25+
protected String getCurrentUserIdp() {
26+
return idp;
27+
}
28+
};
29+
30+
@Test
31+
public void shouldEncodeIdpIdExactlyOnce() {
32+
stubFor(get(urlPathEqualTo("/public/login_time_frame"))
33+
.withQueryParam("idp_id", equalTo(idp))
34+
.withQueryParam("from", equalTo("1000"))
35+
.withQueryParam("to", equalTo("2000"))
36+
.willReturn(okJson("[]")));
37+
38+
List<Object> result = stats.loginTimeFrame(1000, 2000, "day", Optional.empty());
39+
40+
assertNotNull(result);
41+
42+
// Verify request was made correctly
43+
verify(getRequestedFor(urlPathEqualTo("/public/login_time_frame"))
44+
.withQueryParam("idp_id", equalTo(idp)));
45+
46+
List<ServeEvent> events = wireMockRule.getAllServeEvents();
47+
String rawUrl = events.get(0).getRequest().getUrl();
48+
assertEquals("/public/login_time_frame?include_unique=true&idp_id=http://login.surf.nl/adfs/services/trust?q%3D1%26k%3D2&from=1000&to=2000&scale=day&epoch=ms", rawUrl);
49+
}}

‎pom.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131

3232
<groupId>org.openconext</groupId>
3333
<artifactId>dashboard</artifactId>
34-
<version>13.0.12</version>
34+
<version>13.0.13</version>
3535
<name>dashboard</name>
3636
<description>OpenConext-Dashboard</description>
3737
<inceptionYear>2012</inceptionYear>

0 commit comments

Comments
 (0)