Skip BasicAuth in some cases and add disable option to env config #1383
Conversation
src/api/rest/server/server.go
Outdated
| return false, nil | ||
| e.Use(middleware.BasicAuthWithConfig(middleware.BasicAuthConfig{ | ||
| Skipper: func(c echo.Context) bool { | ||
| if strings.HasPrefix(c.Request().Host, "localhost") || |
There was a problem hiding this comment.
감사합니다! 해당 미들웨어는 향후 더 복잡한 인증이 들어오게 되면 유용할 것 같네요. :)
다만, localhost인 경우 자동으로 인증을 skip하도록 하는 형태보다는
필요시 사용자/개발자가 명시적으로 disabled 하도록 처리하는 것이 좋을 것 같습니다.
https://github.com/cloud-barista/cb-tumblebug/blob/main/conf/setup.env#L11
와 같은 형태로 정리하면 어떨까 싶네요.
.Path() == "/tumblebug/health"
c.Path() == "/tumblebug/httpVersion"
등은 자동 pass 하도록 처리해도 문제 없을 것 같습니다. (향후에 롤 기반으로 사용할 수 있는 API가 바뀌게 될 것이라고 예상이 되는데, 그때 더 고도화하게 될 것 같습니다.)
There was a problem hiding this comment.
@seokho-son 좋은 의견 감사드립니다.
ALLOW_ORIGINS와 유사한 형태로 수정하도록 하곘습니다 ^^
src/api/rest/server/server.go
Outdated
| @@ -106,16 +106,30 @@ func RunServer(port string) { | |||
| AllowMethods: []string{http.MethodGet, http.MethodPut, http.MethodPost, http.MethodDelete}, | |||
| })) | |||
|
|
|||
| // Conditions to prevent abnormal operation due to typos (e.g., ture, falss, etc.) | |||
| skipBasicAuthOption := os.Getenv("SKIP_BASIC_AUTH") == "true" | |||
There was a problem hiding this comment.
| skipBasicAuthOption := os.Getenv("SKIP_BASIC_AUTH") == "true" | |
| skipBasicAuthOption := os.Getenv("SKIP_BASIC_AUTH") == "false" |
기본 옵션이니, env와 동일하게 해두는 것이 일단 좋을 것 같아보이는데 어떠신가요? ^^
이외에는 lgtm입니다!
There was a problem hiding this comment.
@seokho-son
"true"인 경우에만 Skip 하도록 만드려는 의도가 있었습니다. 그래서 "true"로 유지하는 것이 좋을 것 같습니다.
예를 들어, "false" 인 경우는 당연히 false가 되어 skip하지 않고요.
"ture", "fals" 등으로 사용자가 잘못 입력하는 경우에도 false가 되어 기본적으로 인증을 하게 만들려고 의도하였습니다 ^^
별개로, 사용자가 SKIP을 원하는 경우는, middleware 자체를 활용하지 않는게 바람직한 것으로 판단됩니다. 이 부분은 바로 수정하여 Push하겠습니다.
There was a problem hiding this comment.
아! 기본값 설정인줄 알고 오해하였습니다. 이슈 없습니다. :)
seokho-son
changed the title
- Perform skipping BasicAuth separated from AUTH enablement
|
코드 가독성을 위해 기본설정 값인 "true"를 활용하면서 |
|
/approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
제가 cb-tumblebug-maintainer 팀의 Member가 아니라 auto-merge가 동작되지 않았습니다. 그래서 조금 전 추가하였습니다 ^^ |
This PR will skip BasicAuth in the following cases:
I think it will make it a bit more convenient during development.
Please let me know if you have any issues.