Fix bugs when encryption flow is enabled in the API #33
Conversation
| @@ -1,5 +1,6 @@ | |||
| env: LOCAL | |||
| cloud: LOCAL | |||
| encryptedFlow: true | |||
There was a problem hiding this comment.
Just a note, for the encrypted flow, we need to bring up the worker binary separately when testing locally right? Is there already documentation around that?
There was a problem hiding this comment.
Reverted. This is not ready yet.
| @@ -16,14 +16,15 @@ | |||
| package org.dataportabilityproject.webapp; | |||
|
|
|||
| import static org.apache.axis.transport.http.HTTPConstants.HEADER_SET_COOKIE; | |||
| import static org.apache.axis.transport.http.HTTPConstants.HEADER_SET_COOKIE2; | |||
| @@ -0,0 +1,58 @@ | |||
| package org.dataportabilityproject.job; | |||
| @@ -1,2 +1,11 @@ | |||
| env: LOCAL | |||
There was a problem hiding this comment.
Isnt this file copied over by the build script?
There was a problem hiding this comment.
Let me add a .gitignore
There was a problem hiding this comment.
This file still shows up in the diff of the request
| /** | ||
| * Deletes an existing {@code job} in storage with the given {@code jobState}. | ||
| */ | ||
| public void deleteJob(String id, JobState jobState) throws IOException { |
There was a problem hiding this comment.
It looks like deleteJob is only called in testing - is this something that we can expose for TestOnly?
There was a problem hiding this comment.
I think we'll eventually need it when implementing the restart/recovery parts of the lifecycle
There was a problem hiding this comment.
Oh right, that makes sense! Thanks!
| @@ -39,7 +39,7 @@ | |||
| * The value ASSIGNED_WITHOUT_AUTH_DATA indicates the client has submitted all data required, such as the | |||
| * encrypted auth data, in order to begin processing the portability job. | |||
| */ | |||
| enum JobState { | |||
| public enum JobState { | |||
There was a problem hiding this comment.
This also looks like (outside of this class) its only used in Test, is there a way we can limit the visibility?
There was a problem hiding this comment.
Annotated with @VisibleForTesting
|
Thanks for the review. I think I addressed all the comments. PTAL. |
| @@ -1,2 +1,11 @@ | |||
| env: LOCAL | |||
There was a problem hiding this comment.
This file still shows up in the diff of the request
| /** | ||
| * Deletes an existing {@code job} in storage with the given {@code jobState}. | ||
| */ | ||
| public void deleteJob(String id, JobState jobState) throws IOException { |
There was a problem hiding this comment.
Oh right, that makes sense! Thanks!
This is a collection of bug fixes to complete the implementation of the encryption flow (using encrypted auth credentials in cookies and storing metadata state in storage) for the API server. #3