Skip to content

Commit

Permalink
🔒 : avoid credentials leakage through Jobs API
Browse files Browse the repository at this point in the history
  • Loading branch information
juwit committed Jul 12, 2020
1 parent 60ef68d commit 9059bc5
Show file tree
Hide file tree
Showing 7 changed files with 99 additions and 21 deletions.
20 changes: 13 additions & 7 deletions src/main/java/io/gaia_app/credentials/Credentials.kt
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import org.springframework.data.annotation.Id

import com.fasterxml.jackson.annotation.JsonTypeInfo.As.PROPERTY
import com.fasterxml.jackson.annotation.JsonTypeInfo.Id.NAME
import com.fasterxml.jackson.annotation.JsonView
import com.fasterxml.jackson.databind.annotation.JsonSerialize
import io.gaia_app.teams.User
import org.springframework.data.annotation.CreatedBy
import org.springframework.data.mongodb.core.mapping.DBRef
Expand All @@ -31,24 +33,28 @@ abstract class Credentials {
@DBRef
lateinit var createdBy: User

var provider: String

constructor(provider: String) {
this.provider = provider
}

abstract fun toEnv(): List<String>
abstract fun provider(): String
}

@Document
data class AWSCredentials(val accessKey:String, val secretKey:String):Credentials() {
data class AWSCredentials(val accessKey:String, val secretKey:String):Credentials("aws") {
override fun toEnv() = listOf("AWS_ACCESS_KEY_ID=$accessKey", "AWS_SECRET_ACCESS_KEY=$secretKey")
override fun provider() = "aws"
}

@Document
data class GoogleCredentials(val serviceAccountJSONContents:String):Credentials() {
data class GoogleCredentials(val serviceAccountJSONContents:String):Credentials("google") {
override fun toEnv() = listOf("GOOGLE_CREDENTIALS=$serviceAccountJSONContents")
override fun provider() = "google"
}

@Document
data class AzureRMCredentials(val clientId:String, val clientSecret:String):Credentials() {
data class AzureRMCredentials(val clientId:String, val clientSecret:String):Credentials("azurerm") {
override fun toEnv() = listOf("ARM_CLIENT_ID=$clientId", "ARM_CLIENT_SECRET=$clientSecret")
override fun provider() = "azurerm"
}

data class EmptyCredentials(val id: String, val name: String, val provider: String)
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import java.util.*

interface CredentialsRepository: MongoRepository<Credentials, String> {

fun findAllByCreatedBy(user: User): List<Credentials>
fun findAllByCreatedBy(user: User): List<EmptyCredentials>

fun findByIdAndCreatedBy(id: String, createdBy: User): Optional<Credentials>
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public CredentialsRestController(CredentialsRepository credentialsRepository) {
}

@GetMapping
public List<Credentials> getAllCredentials(User user) {
public List<EmptyCredentials> getAllCredentials(User user) {
return this.credentialsRepository.findAllByCreatedBy(user);
}

Expand Down
22 changes: 13 additions & 9 deletions src/main/java/io/gaia_app/runner/StackRunner.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package io.gaia_app.runner;

import io.gaia_app.credentials.CredentialsRepository;
import io.gaia_app.modules.bo.TerraformModule;
import io.gaia_app.stacks.bo.Job;
import io.gaia_app.stacks.bo.JobType;
Expand All @@ -9,14 +10,6 @@
import io.gaia_app.stacks.repository.StackRepository;
import io.gaia_app.stacks.repository.StepRepository;
import io.gaia_app.stacks.workflow.JobWorkflow;
import io.gaia_app.modules.bo.TerraformModule;
import io.gaia_app.stacks.bo.Job;
import io.gaia_app.stacks.bo.JobType;
import io.gaia_app.stacks.bo.Stack;
import io.gaia_app.stacks.bo.StackState;
import io.gaia_app.stacks.repository.StackRepository;
import io.gaia_app.stacks.repository.StepRepository;
import io.gaia_app.stacks.workflow.JobWorkflow;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.scheduling.annotation.Async;
import org.springframework.stereotype.Service;
Expand All @@ -39,17 +32,19 @@ public class StackRunner {
private StackRepository stackRepository;
private JobRepository jobRepository;
private StepRepository stepRepository;
private CredentialsRepository credentialsRepository;

private Map<String, Job> jobs = new HashMap<>();

@Autowired
public StackRunner(DockerRunner dockerRunner, StackCommandBuilder stackCommandBuilder,
StackRepository stackRepository, JobRepository jobRepository, StepRepository stepRepository) {
StackRepository stackRepository, JobRepository jobRepository, StepRepository stepRepository, CredentialsRepository credentialsRepository) {
this.dockerRunner = dockerRunner;
this.stackCommandBuilder = stackCommandBuilder;
this.stackRepository = stackRepository;
this.jobRepository = jobRepository;
this.stepRepository = stepRepository;
this.credentialsRepository = credentialsRepository;
}

private String managePlanScript(Job job, Stack stack, TerraformModule module) {
Expand Down Expand Up @@ -127,6 +122,9 @@ private void treatJob(JobWorkflow jobWorkflow, Consumer<JobWorkflow> jobActionFn

@Async
public void plan(JobWorkflow jobWorkflow, TerraformModule module, Stack stack) {
if(stack.getCredentialsId() != null){
jobWorkflow.getJob().setCredentials(this.credentialsRepository.findById(stack.getCredentialsId()).orElseThrow());
}
treatJob(
jobWorkflow,
JobWorkflow::plan,
Expand All @@ -137,6 +135,9 @@ public void plan(JobWorkflow jobWorkflow, TerraformModule module, Stack stack) {

@Async
public void apply(JobWorkflow jobWorkflow, TerraformModule module, Stack stack) {
if(stack.getCredentialsId() != null){
jobWorkflow.getJob().setCredentials(this.credentialsRepository.findById(stack.getCredentialsId()).orElseThrow());
}
treatJob(
jobWorkflow,
JobWorkflow::apply,
Expand All @@ -147,6 +148,9 @@ public void apply(JobWorkflow jobWorkflow, TerraformModule module, Stack stack)

@Async
public void retry(JobWorkflow jobWorkflow, TerraformModule module, Stack stack) {
if(stack.getCredentialsId() != null){
jobWorkflow.getJob().setCredentials(this.credentialsRepository.findById(stack.getCredentialsId()).orElseThrow());
}
stepRepository.deleteByJobId(jobWorkflow.getJob().getId());
treatJob(
jobWorkflow,
Expand Down
5 changes: 5 additions & 0 deletions src/main/java/io/gaia_app/stacks/bo/Job.java
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package io.gaia_app.stacks.bo;

import com.fasterxml.jackson.annotation.JsonIgnore;
import io.gaia_app.credentials.Credentials;
import io.gaia_app.modules.bo.TerraformImage;
import io.gaia_app.teams.User;
import io.gaia_app.modules.bo.TerraformImage;
import org.springframework.data.annotation.Transient;
import org.springframework.data.mongodb.core.mapping.DBRef;

import java.time.Duration;
Expand All @@ -28,6 +30,9 @@ public class Job {
private List<Step> steps = new ArrayList<>(2);
@DBRef
private User user;

@Transient
@JsonIgnore
private Credentials credentials;

public Job() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,21 @@ void setup() {
@Test
@WithMockUser("Darth Vader")
void users_shouldBeAbleToViewTheirOwnCredentials_forListAccess() throws Exception {
mockMvc.perform(get("/api/credentials"))
.andExpect(status().isOk())
.andExpect(jsonPath("$", hasSize(1)))
.andExpect(jsonPath("$[0].provider", is("aws")));
}

@Test
@WithMockUser("Darth Vader")
void credentialsDetailsShouldNotLeak_forListAccess() throws Exception {
mockMvc.perform(get("/api/credentials"))
.andExpect(status().isOk())
.andExpect(jsonPath("$", hasSize(1)))
.andExpect(jsonPath("$[0].provider", is("aws")))
.andExpect(jsonPath("$[0].accessKey", is("DEATH_STAR_KEY")))
.andExpect(jsonPath("$[0].secretKey", is("DEATH_STAR_SECRET")))
.andExpect(jsonPath("$[0].createdBy.username", is("Darth Vader")));
.andExpect(jsonPath("$[0].accessKey").doesNotExist())
.andExpect(jsonPath("$[0].secretKey").doesNotExist());
}

@Test
Expand Down
55 changes: 55 additions & 0 deletions src/test/java/io/gaia_app/runner/StackRunnerTest.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
package io.gaia_app.runner;

import io.gaia_app.credentials.AWSCredentials;
import io.gaia_app.credentials.CredentialsRepository;
import io.gaia_app.credentials.EmptyCredentials;
import io.gaia_app.modules.bo.TerraformModule;
import io.gaia_app.stacks.bo.Job;
import io.gaia_app.stacks.bo.JobType;
Expand All @@ -18,6 +21,7 @@

import java.util.Optional;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.Mockito.*;

Expand All @@ -39,6 +43,9 @@ class StackRunnerTest {
@Mock
private StepRepository stepRepository;

@Mock
private CredentialsRepository credentialsRepository;

@Mock
private JobWorkflow jobWorkflow;

Expand Down Expand Up @@ -393,4 +400,52 @@ void retry_shouldSaveJobAndSteps() {
verify(stepRepository, times(2)).saveAll(job.getSteps());
}

@Test
void credentialsShouldBeInjected_whenRunningAPlan(){
// given
stack.setCredentialsId("dummy");
var credentials = new AWSCredentials("dummy","dummy");

when(credentialsRepository.findById("dummy")).thenReturn(Optional.of(credentials));

// when
stackRunner.plan(jobWorkflow, module, stack);

// then
verify(credentialsRepository).findById("dummy");
assertThat(job.getCredentials()).isEqualTo(credentials);
}

@Test
void credentialsShouldBeInjected_whenRunningAnApply(){
// given
stack.setCredentialsId("dummy");
var credentials = new AWSCredentials("dummy","dummy");

when(credentialsRepository.findById("dummy")).thenReturn(Optional.of(credentials));

// when
stackRunner.apply(jobWorkflow, module, stack);

// then
verify(credentialsRepository).findById("dummy");
assertThat(job.getCredentials()).isEqualTo(credentials);
}

@Test
void credentialsShouldBeInjected_whenRunningARetry(){
// given
stack.setCredentialsId("dummy");
var credentials = new AWSCredentials("dummy","dummy");

when(credentialsRepository.findById("dummy")).thenReturn(Optional.of(credentials));

// when
stackRunner.retry(jobWorkflow, module, stack);

// then
verify(credentialsRepository).findById("dummy");
assertThat(job.getCredentials()).isEqualTo(credentials);
}

}

0 comments on commit 9059bc5

Please sign in to comment.