Skip to content
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

[BG-137]: JWT 발행 (6h / 3h) #8

Merged
merged 21 commits into from
Jun 27, 2024
Merged

[BG-137]: JWT 발행 (6h / 3h) #8

merged 21 commits into from
Jun 27, 2024

Conversation

Dltmd202
Copy link
Member

Why

  • 세팅값이 잘 못 들어간 것 때문에 삽질
  • Mockk 첫 도입

등으로 작업이 지연됨

Result

image

@RestController
@RequestMapping("/api/v1")
class WorkspaceController(
    private val workspaceService: WorkspaceService,
) {
    @PostMapping("/workspaces")
    fun createWorkspace(
        @AuthenticationPrincipal token: JwtAuthentication,
        @RequestBody workspaceCreateDto: WorkspaceCreateDto,
    ): ResponseEntity<Long> = ResponseEntity.ok().body(workspaceService.createWorkspace(token.id, workspaceCreateDto))

    @GetMapping
    fun findWorkspaces(
        @AuthenticationPrincipal token: JwtAuthentication,
    ) {
        workspaceService.findWorkspaces(token.id)
    }
}
  • @AuthenticationPrincipal 어노테이션 기반의 argumentResolver를 사용한 인가

Prize

@DisplayName("AuthFacadeTest 테스트")
@ExtendWith(SpringExtension::class)
@AutoConfigureMockMvc
@Transactional
@SpringBootTest
class AuthFacadeTest {

    @MockkBean
    lateinit var oauthService: OAuthService

    @Test
    @DisplayName("googleLogin 테스트")
    fun googleLoginTest() {
        // given
        every { oauthService.googleLogin(any()) } returns AuthFixture.createGoogleUserInfoDto()

        // then
        val googleLogin: JwtTokenResponse = authFacade.googleLogin("authCode")

        // then
        assertThat(jwtService.verify(googleLogin.token)).isNotNull()
    }

}
  • Mockk 처음 사용해 봄

Reference

Link

BG-137

@Dltmd202 Dltmd202 self-assigned this Jun 27, 2024
@GGHDMS GGHDMS self-requested a review June 27, 2024 02:19
data class WorkspaceCreateDto(
val userId: UUID,
val name: String,
var name: String = "",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이거 이런식으로 디폴트 값이 들어가야 RequestBody 에 넣을 수 있던데
더 좋은방법 있을까?

Copy link
Member Author

@Dltmd202 Dltmd202 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security쪽 패키지 구조는 어떤 것 같닝?

아무래도 좀 고민을 많이 하고 넣진 않았는뎅

GGHDMS
GGHDMS previously approved these changes Jun 27, 2024
Copy link
Member

@GGHDMS GGHDMS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 고생했어

Comment on lines +15 to +16
val authConfig: AuthConfig,
val authFacade: AuthFacade,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

private 붙여 주는게 좋을듯

@@ -13,6 +13,8 @@ repositories {
dependencies {
implementation(project(":domain"))
implementation("org.springframework.boot:spring-boot-starter-web")
implementation("org.springframework.boot:spring-boot-starter-security")
implementation("com.auth0:java-jwt:3.18.3")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

처음 보는데 jjwt 안쓰고 사용한 이유 있어?

userRepository.save(create.toEntity()),
)

fun saveOrGetUser(user: UserCreateDto): UserDto =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Transactional을 추가하는게 좋을듯

import java.util.Date
import java.util.UUID

@Service
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Component 로 하는게 더 좋아보여 @Transactional 이 없으니깐

}
} catch (e: Exception) {
// TODO exception handling
throw IllegalArgumentException("Invalid token")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기서 Exception을 던졌을 때 jwtAuthenticationEntryPoint 까지 도달하는지 테스트 해봐야 할듯

@GGHDMS
Copy link
Member

GGHDMS commented Jun 27, 2024

security쪽 패키지 구조는 어떤 것 같닝?

아무래도 좀 고민을 많이 하고 넣진 않았는뎅

handler, filter, component 식으로 분리 시켜도 좋을거 같은데

Copy link
Member

@GGHDMS GGHDMS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@GGHDMS GGHDMS merged commit d93b471 into develop Jun 27, 2024
1 check passed
@GGHDMS GGHDMS deleted the BG-137-publish-jwt branch June 27, 2024 03:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants