Repository navigation
fix(security): authenticate artifact token before object lookup #349
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
466c7dd
test(security): reject missing artifact token before object lookup
seonghobae cf6fdf8
fix(security): require artifact credential before object lookup
seonghobae 91b6a5d
test(controller): preserve post-auth not-found coverage
seonghobae 7e8e71b
Merge protected main into artifact-token precondition
seonghobae File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
74 changes: 74 additions & 0 deletions
74
src/test/java/com/clearfolio/viewer/controller/ArtifactControllerTokenPreconditionTest.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| package com.clearfolio.viewer.controller; | ||
|
|
||
| import static org.junit.jupiter.api.Assertions.assertEquals; | ||
| import static org.junit.jupiter.api.Assertions.assertNotNull; | ||
| import static org.mockito.Mockito.mock; | ||
| import static org.mockito.Mockito.verifyNoInteractions; | ||
|
|
||
| import java.util.UUID; | ||
|
|
||
| import org.junit.jupiter.api.Test; | ||
| import org.springframework.http.HttpStatus; | ||
| import org.springframework.http.ResponseEntity; | ||
|
|
||
| import com.clearfolio.viewer.artifact.ArtifactLinkService; | ||
| import com.clearfolio.viewer.artifact.ArtifactStore; | ||
| import com.clearfolio.viewer.auth.TenantAccessService; | ||
| import com.clearfolio.viewer.service.DocumentConversionService; | ||
|
|
||
| /** | ||
| * Proves that the public artifact route authenticates token presence before | ||
| * reading conversion-job or artifact state that could become an existence oracle. | ||
| */ | ||
| class ArtifactControllerTokenPreconditionTest { | ||
|
|
||
| @Test | ||
| void missingArtifactTokenIsRejectedBeforeSensitiveObjectLookup() { | ||
| DocumentConversionService conversionService = mock(DocumentConversionService.class); | ||
| ArtifactStore artifactStore = mock(ArtifactStore.class); | ||
| ArtifactLinkService artifactLinkService = mock(ArtifactLinkService.class); | ||
| ArtifactController controller = new ArtifactController( | ||
| conversionService, | ||
| artifactStore, | ||
| artifactLinkService, | ||
| mock(TenantAccessService.class) | ||
| ); | ||
|
|
||
| ResponseEntity<byte[]> response = controller.getPdf( | ||
| UUID.randomUUID(), | ||
| null, | ||
| null, | ||
| null, | ||
| null | ||
| ).block(); | ||
|
|
||
| assertNotNull(response); | ||
| assertEquals(HttpStatus.UNAUTHORIZED, response.getStatusCode()); | ||
| verifyNoInteractions(conversionService, artifactStore, artifactLinkService); | ||
| } | ||
|
|
||
| @Test | ||
| void unsupportedAuthorizationSchemeIsRejectedBeforeSensitiveObjectLookup() { | ||
| DocumentConversionService conversionService = mock(DocumentConversionService.class); | ||
| ArtifactStore artifactStore = mock(ArtifactStore.class); | ||
| ArtifactLinkService artifactLinkService = mock(ArtifactLinkService.class); | ||
| ArtifactController controller = new ArtifactController( | ||
| conversionService, | ||
| artifactStore, | ||
| artifactLinkService, | ||
| mock(TenantAccessService.class) | ||
| ); | ||
|
|
||
| ResponseEntity<byte[]> response = controller.getPdf( | ||
| UUID.randomUUID(), | ||
| null, | ||
| " ", | ||
| "Basic opaque-credential", | ||
| null | ||
| ).block(); | ||
|
|
||
| assertNotNull(response); | ||
| assertEquals(HttpStatus.UNAUTHORIZED, response.getStatusCode()); | ||
| verifyNoInteractions(conversionService, artifactStore, artifactLinkService); | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
객체 조회 전에 토큰을 인증하십시오.
resolveToken은 비어 있지 않은 쿼리 토큰을 그대로 반환합니다. 따라서artifactToken=opaque-present-token요청은conversionService.getJob(docId)및artifactStore.getPdf(docId)에 도달합니다. 공격자는 유효하지 않은 토큰으로 문서 존재 상태를 관찰할 수 있습니다.객체 조회 전에 서명, 만료, 범위,
docId바인딩 및 취소 상태를 검증하는 단계를 추가하십시오. 검증되지 않은 토큰은 조회 전에 토큰 실패 응답을 반환해야 합니다.src/main/java/com/clearfolio/viewer/controller/ArtifactController.java#L139-L143:resolveToken뒤에 사전 인증을 추가하고, 인증 성공 후에만 job 및 artifact 조회를 실행하십시오.src/test/java/com/clearfolio/viewer/controller/ArtifactControllerTokenPreconditionTest.java#L25-L73: 비어 있지 않은 잘못된 쿼리 토큰 및 Bearer 토큰도401과 collaborator 미호출을 보장하는 테스트를 추가하십시오.src/test/java/com/clearfolio/viewer/controller/ArtifactControllerTest.java#L56-L56: 사전 인증을 통과하는 토큰으로 job-missing404동작을 검증하십시오.src/test/java/com/clearfolio/viewer/controller/ArtifactControllerTest.java#L75-L75: 사전 인증을 통과하는 토큰으로 non-succeeded job404동작을 검증하십시오.src/test/java/com/clearfolio/viewer/controller/ArtifactControllerTest.java#L87-L87: 사전 인증을 통과하는 토큰으로 artifact-missing404동작을 검증하십시오.📍 Affects 3 files
src/main/java/com/clearfolio/viewer/controller/ArtifactController.java#L139-L143(this comment)src/test/java/com/clearfolio/viewer/controller/ArtifactControllerTokenPreconditionTest.java#L25-L73src/test/java/com/clearfolio/viewer/controller/ArtifactControllerTest.java#L56-L56src/test/java/com/clearfolio/viewer/controller/ArtifactControllerTest.java#L75-L75src/test/java/com/clearfolio/viewer/controller/ArtifactControllerTest.java#L87-L87🤖 Prompt for AI Agents
Source: Coding guidelines