Skip to content

Commit

Permalink
🐛 : handle parameter injection in RestTemplate URL
Browse files Browse the repository at this point in the history 
When passing parameters to the RestTemplate exchange method, spring
url-encodes the parameter. This behaviour is not desirable for
GithubRawContent, as the project ID contains a '/'
  • Loading branch information
@juwit
juwit committed Jan 9, 2020
1 parent c20fc1b commit f831373
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 26 deletions.
5 changes: 2 additions & 3 deletions src/main/java/io/codeka/gaia/registries/RegistryRawContent.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,10 @@ abstract class RegistryRawContent(private val registryType: RegistryType, privat
val requestEntity = HttpEntity<Any>(headers)

val response = restTemplate.exchange(
this.registryType.readmeUrl,
this.registryType.readmeUrl.replace("{id}", module.registryDetails.projectId),
HttpMethod.GET,
requestEntity,
RegistryFile::class.java,
module.registryDetails.projectId)
RegistryFile::class.java)

if(response.statusCode == HttpStatus.OK) {
return Optional.of(String(Base64.getDecoder().decode(response.body?.content)))
Expand Down
30 changes: 13 additions & 17 deletions src/test/java/io/codeka/gaia/registries/github/GitHubRawContentTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ void matches_shouldReturnFalseForInvalidUrl() {
void getReadmeContent_shouldCallTheApi_andServeDecodedContent(){
// given
var module = new TerraformModule();
module.setRegistryDetails(new RegistryDetails(RegistryType.GITLAB, "group/project"));
module.setRegistryDetails(new RegistryDetails(RegistryType.GITHUB, "Apophis/Chulak"));

var jack = new User("Jack", null);
jack.setOAuth2User(new OAuth2User("GITHUB","TOKENSTRING", null));
Expand All @@ -66,11 +66,10 @@ void getReadmeContent_shouldCallTheApi_andServeDecodedContent(){
var response = new ResponseEntity<>(githubFile, HttpStatus.OK);

when(restTemplate.exchange(
eq("https://api.github.com/repos/{id}/contents/README.md?ref=master"),
eq("https://api.github.com/repos/Apophis/Chulak/contents/README.md?ref=master"),
eq(HttpMethod.GET),
requestCaptor.capture(),
eq(RegistryFile.class),
eq("group/project"))).thenReturn(response);
eq(RegistryFile.class))).thenReturn(response);

// when
var result = gitHubRawContent.getReadme(module);
Expand All @@ -86,7 +85,7 @@ void getReadmeContent_shouldCallTheApi_andServeDecodedContent(){
void getReadmeContent_shouldCallTheApiWithoutAuth_ifNoToken_andServeDecodedContent(){
// given
var module = new TerraformModule();
module.setRegistryDetails(new RegistryDetails(RegistryType.GITLAB, "group/project"));
module.setRegistryDetails(new RegistryDetails(RegistryType.GITHUB, "Apophis/Chulak"));

var jack = new User("Jack", null);
module.setCreatedBy(jack);
Expand All @@ -97,11 +96,10 @@ void getReadmeContent_shouldCallTheApiWithoutAuth_ifNoToken_andServeDecodedConte
var response = new ResponseEntity<>(githubFile, HttpStatus.OK);

when(restTemplate.exchange(
eq("https://api.github.com/repos/{id}/contents/README.md?ref=master"),
eq("https://api.github.com/repos/Apophis/Chulak/contents/README.md?ref=master"),
eq(HttpMethod.GET),
requestCaptor.capture(),
eq(RegistryFile.class),
eq("group/project"))).thenReturn(response);
eq(RegistryFile.class))).thenReturn(response);

// when
var result = gitHubRawContent.getReadme(module);
Expand All @@ -117,19 +115,18 @@ void getReadmeContent_shouldCallTheApiWithoutAuth_ifNoToken_andServeDecodedConte
void getReadmeContent_shouldCallTheApiWithoutAuth_ifNoOwner_andServeDecodedContent(){
// given
var module = new TerraformModule();
module.setRegistryDetails(new RegistryDetails(RegistryType.GITLAB, "group/project"));
module.setRegistryDetails(new RegistryDetails(RegistryType.GITHUB, "Apophis/Chulak"));

var requestCaptor = ArgumentCaptor.forClass(HttpEntity.class);

var githubFile = new RegistryFile(Base64.encode("# Module Readme".getBytes()));
var response = new ResponseEntity<>(githubFile, HttpStatus.OK);

when(restTemplate.exchange(
eq(RegistryType.GITHUB.getReadmeUrl()),
eq("https://api.github.com/repos/Apophis/Chulak/contents/README.md?ref=master"),
eq(HttpMethod.GET),
requestCaptor.capture(),
eq(RegistryFile.class),
eq("group/project"))).thenReturn(response);
eq(RegistryFile.class))).thenReturn(response);

// when
var result = gitHubRawContent.getReadme(module);
Expand Down Expand Up @@ -157,22 +154,21 @@ void getReadmeContent_withNoRegistryDetails_returnsEmptyContent(){
void getReadmeContent_shouldReturnEmpty_whenReadmeDoesntExists(){
// given
var module = new TerraformModule();
module.setRegistryDetails(new RegistryDetails(RegistryType.GITLAB, "123"));
module.setRegistryDetails(new RegistryDetails(RegistryType.GITHUB, "Apophis/Chulak"));

var jack = new User("Jack", null);
jack.setOAuth2User(new OAuth2User("GITLAB","TOKENSTRING", null));
jack.setOAuth2User(new OAuth2User("GITHUB","TOKENSTRING", null));
module.setCreatedBy(jack);

var requestCaptor = ArgumentCaptor.forClass(HttpEntity.class);

var response = new ResponseEntity<RegistryFile>(HttpStatus.NOT_FOUND);

when(restTemplate.exchange(
eq(RegistryType.GITHUB.getReadmeUrl()),
eq("https://api.github.com/repos/Apophis/Chulak/contents/README.md?ref=master"),
eq(HttpMethod.GET),
requestCaptor.capture(),
eq(RegistryFile.class),
eq("123"))).thenReturn(response);
eq(RegistryFile.class))).thenReturn(response);

// when
var result = gitHubRawContent.getReadme(module);
Expand Down
10 changes: 4 additions & 6 deletions src/test/java/io/codeka/gaia/registries/gitlab/GitLabRawContentTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,10 @@ void getReadmeContent_shouldCallTheApi_andServeDecodedContent(){
var response = new ResponseEntity<>(gitlabFile, HttpStatus.OK);

when(restTemplate.exchange(
eq(RegistryType.GITLAB.getReadmeUrl()),
eq("https://gitlab.com/api/v4/projects/123/repository/files/README.md?ref=master"),
eq(HttpMethod.GET),
requestCaptor.capture(),
eq(RegistryFile.class),
eq("123"))).thenReturn(response);
eq(RegistryFile.class))).thenReturn(response);

// when
var result = gitLabRawContent.getReadme(module);
Expand Down Expand Up @@ -109,11 +108,10 @@ void getReadmeContent_shouldReturnEmpty_whenReadmeDoesntExists(){
var response = new ResponseEntity<RegistryFile>(HttpStatus.NOT_FOUND);

when(restTemplate.exchange(
eq(RegistryType.GITLAB.getReadmeUrl()),
eq("https://gitlab.com/api/v4/projects/123/repository/files/README.md?ref=master"),
eq(HttpMethod.GET),
requestCaptor.capture(),
eq(RegistryFile.class),
eq("123"))).thenReturn(response);
eq(RegistryFile.class))).thenReturn(response);

// when
var result = gitLabRawContent.getReadme(module);
Expand Down

0 comments on commit f831373

Please sign in to comment.