Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces dynamic OAuth2 redirect URI selection by identifying whether the request originates from a local or deployed environment. It achieves this by checking the 'Referer' header in the CookieOAuth2AuthorizationRequestRepository and storing the target environment in a cookie, which is then used by the OAuth2LoginSuccessHandler to determine the final redirect URL. Review feedback suggested improving the security of the environment detection logic to prevent spoofing via the 'Referer' header and recommended using constants instead of hardcoded strings for better maintainability and consistency across the codebase.
| implements AuthorizationRequestRepository<OAuth2AuthorizationRequest> { | ||
|
|
||
| private static final String COOKIE_NAME = "oauth2_auth_request"; | ||
| public static final String REDIRECT_TARGET_COOKIE = "redirect_target"; |
There was a problem hiding this comment.
"local" ๋ฐ "deploy"์ ๊ฐ์ ํ๊ฒฝ ์๋ณ ๋ฌธ์์ด์ ์์๋ก ์ ์ํ์ฌ ๊ด๋ฆฌํ๋ฉด ์คํ๋ฅผ ๋ฐฉ์งํ๊ณ OAuth2LoginSuccessHandler ๋ฑ ๋ค๋ฅธ ํด๋์ค์์ ์์ ํ๊ฒ ์ฌ์ฌ์ฉํ ์ ์์ต๋๋ค.
| public static final String REDIRECT_TARGET_COOKIE = "redirect_target"; | |
| public static final String REDIRECT_TARGET_COOKIE = "redirect_target"; | |
| public static final String TARGET_LOCAL = "local"; | |
| public static final String TARGET_DEPLOY = "deploy"; |
|
|
||
| // Referer ํค๋๋ก ์ถ๋ฐ์ง(local/deploy) ํ๋จ ํ ์ฟ ํค ์ ์ฅ | ||
| String referer = request.getHeader("Referer"); | ||
| String target = (referer != null && referer.contains("localhost")) ? "local" : "deploy"; |
There was a problem hiding this comment.
referer.contains("localhost") ์กฐ๊ฑด์ http://attacker.com/localhost์ ๊ฐ์ด ๋๋ฉ์ธ์ด ์๋ ๊ฒฝ๋ก์ localhost๊ฐ ํฌํจ๋ ๊ฒฝ์ฐ์๋ ์๋์น ์๊ฒ local ํ๊ฒฝ์ผ๋ก ์คํํ ์ํ์ด ์์ต๋๋ค. startsWith๋ฅผ ์ฌ์ฉํ์ฌ ํธ์คํธ ๋ถ๋ถ์ ๋ณด๋ค ๋ช
ํํ๊ฒ ๊ฒ์ฌํ๋ ๊ฒ์ด ์์ ํฉ๋๋ค.
| String target = (referer != null && referer.contains("localhost")) ? "local" : "deploy"; | |
| String target = (referer != null && (referer.startsWith("http://localhost") || referer.startsWith("https://localhost"))) ? TARGET_LOCAL : TARGET_DEPLOY; |
| .orElse("deploy"); | ||
| String redirectUri = "local".equals(target) ? localRedirectUri : deployRedirectUri; |
There was a problem hiding this comment.
ํ๋์ฝ๋ฉ๋ ๋ฌธ์์ด ๋์ CookieOAuth2AuthorizationRequestRepository์ ์ ์๋ ์์๋ฅผ ์ฌ์ฉํ์ฌ ์ฝ๋์ ์ผ๊ด์ฑ์ ์ ์งํ๊ณ ์ ์ง๋ณด์์ฑ์ ๋์ด๋ ๊ฒ์ ๊ถ์ฅํฉ๋๋ค.
| .orElse("deploy"); | |
| String redirectUri = "local".equals(target) ? localRedirectUri : deployRedirectUri; | |
| .orElse(CookieOAuth2AuthorizationRequestRepository.TARGET_DEPLOY); | |
| String redirectUri = CookieOAuth2AuthorizationRequestRepository.TARGET_LOCAL.equals(target) ? localRedirectUri : deployRedirectUri; |
๐ ๊ด๋ จ ์ด์
โ ์์ ๋ถ๋ฅ
โจ ์์ ๋ด์ฉ
application.properties์app.oauth2.front-redirect-uri๋ฅผ.local/.deploy๋ ๊ฐ๋ก ๋ถ๋ฆฌCookieOAuth2AuthorizationRequestRepository์REDIRECT_TARGET_COOKIE์์ ์ถ๊ฐsaveAuthorizationRequest)์์ ์์ฒญ์Refererํค๋๋ฅผ ๋ณด๊ณ"local"๋๋"deploy"๋ฅผ ์ฟ ํค์ ์ ์ฅOAuth2LoginSuccessHandler์์ ์ ์ฟ ํค๋ฅผ ์ฝ์ด ํ๊ฒฝ๋ณ ํ๋ก ํธ ๋ฆฌ๋ค์ด๋ ํธ URI๋ฅผ ์ ํ, ์ฌ์ฉ ํ ์ฟ ํค ์ญ์ deploy๋ก ํด๋ฐฑ๐ฅ ์ ๋ฌ์ฌํญ
โ ์ฒดํฌ๋ฆฌ์คํธ
๐ธ ์คํฌ๋ฆฐ์ท
๐ก ๋ฐฐ์ด ๊ฒ / ์๋ํ ๊ฒ / ๊ณ ๋ฏผํ ์