Conversation
There was a problem hiding this comment.
Pull request overview
This PR migrates the authentication system from GitHub OAuth App to GitHub App to reduce permission requirements and improve security. The migration includes implementing JWT-based authentication for GitHub API communication, adding real-time polling for app installation detection during onboarding, and enhancing user guidance for app installation and permission management.
Changes:
- Implemented GitHub App integration with JWT authentication and private key loading for secure API communication
- Added real-time polling mechanism to detect GitHub App installation during onboarding without page navigation
- Enhanced error handling in profile view to guide users through GitHub App installation when permissions are missing
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| backend/src/main/java/com/ssafy/dash/github/application/GitHubAppService.java | New service implementing GitHub App JWT generation, installation token retrieval, and app installation verification |
| backend/src/main/java/com/ssafy/dash/github/application/GitHubPushEventWorker.java | Added token resolution logic to prefer GitHub App tokens over OAuth tokens when processing push events |
| backend/src/main/java/com/ssafy/dash/onboarding/application/OnboardingService.java | Removed webhook registration logic and added GitHub App installation pre-validation |
| backend/src/main/java/com/ssafy/dash/onboarding/presentation/OnboardingController.java | Added endpoint to check GitHub App installation status for repositories |
| backend/src/main/java/com/ssafy/dash/github/config/GitHubAppProperties.java | Configuration properties for GitHub App ID and private key path |
| backend/src/main/java/com/ssafy/dash/github/config/GitHubConfig.java | Configuration class enabling GitHub App and webhook properties |
| backend/src/main/java/com/ssafy/dash/onboarding/domain/exception/GitHubAppNotInstalledException.java | New exception for missing GitHub App installations |
| backend/src/main/java/com/ssafy/dash/common/exception/ErrorCode.java | Added error code for GitHub App not installed scenario |
| backend/src/main/resources/application.properties | Updated OAuth scopes and added GitHub App configuration |
| backend/pom.xml | Added JJWT dependencies for JWT token generation |
| frontend/src/views/onboarding/OnboardingView.vue | Added installation_id query parameter handling to redirect users back to repo step after app installation |
| frontend/src/views/onboarding/OnboardingStep5Repo.vue | Implemented real-time polling for GitHub App installation with visual states and timeout handling |
| frontend/src/views/user/ProfileView.vue | Enhanced error handling to detect app permission issues and guide users to GitHub App settings |
| frontend/src/api/onboarding.js | Added API method to check GitHub App installation status |
| docker-compose.yml | Added GitHub App environment variables and volume mount for private key |
| .gitignore | Added .pem files to prevent committing private keys |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const githubAppName = import.meta.env.VITE_GITHUB_APP_NAME; | ||
|
|
There was a problem hiding this comment.
The code uses environment variable VITE_GITHUB_APP_NAME but this variable is not defined in the docker-compose.yml or any configuration file. If this variable is undefined, the URL will be constructed as "https://github.com/apps/undefined/installations/new" which will result in a broken link. Either add this environment variable to docker-compose.yml and application configuration, or hardcode the GitHub App name if it's fixed.
| const githubAppName = import.meta.env.VITE_GITHUB_APP_NAME; | |
| const githubAppName = import.meta.env.VITE_GITHUB_APP_NAME ?? ''; | |
| if (import.meta.env.VITE_GITHUB_APP_NAME === undefined && import.meta.env.DEV) { | |
| console.warn('[OnboardingStep5Repo] VITE_GITHUB_APP_NAME is not defined. GitHub App installation link may not work as expected.'); | |
| } |
| <button | ||
| @click="confirmRepo" | ||
| class="w-full py-4 bg-brand-600 hover:bg-brand-500 text-white font-bold rounded-2xl shadow-xl shadow-brand-500/20 hover:-translate-y-0.5 transition-all flex items-center justify-center gap-2" | ||
| :disabled="saving" | ||
| class="w-full py-4 text-white font-bold rounded-2xl shadow-xl transition-all flex items-center justify-center gap-2" | ||
| :class="isAppInstalled | ||
| ? 'bg-slate-900 hover:bg-slate-800 shadow-slate-900/20 hover:-translate-y-0.5' | ||
| : pollingTimedOut | ||
| ? 'bg-amber-500 hover:bg-amber-400 shadow-amber-500/10' | ||
| : 'bg-slate-300 cursor-not-allowed opacity-70'" | ||
| :disabled="saving || (!isAppInstalled && !pollingTimedOut)" | ||
| > | ||
| <Loader2 v-if="saving" class="animate-spin" /> | ||
| <span>네, 이 저장소가 맞습니다</span> | ||
| <template v-if="saving"> | ||
| <Loader2 class="animate-spin w-5 h-5" /> | ||
| <span>완료 처리 중...</span> | ||
| </template> | ||
| <template v-else-if="isAppInstalled"> | ||
| <CheckCircle2 class="w-5 h-5" /> | ||
| <span>연동이 완료되었습니다</span> | ||
| </template> | ||
| <template v-else-if="pollingTimedOut"> | ||
| <AlertTriangle class="w-5 h-5" /> | ||
| <span>설치를 찾지 못했습니다</span> | ||
| </template> | ||
| <template v-else> | ||
| <Loader2 class="animate-spin w-5 h-5" /> | ||
| <span>App 승인을 기다리고 있습니다...</span> | ||
| </template> | ||
| </button> |
There was a problem hiding this comment.
When polling times out (pollingTimedOut is true), the button displays "설치를 찾지 못했습니다" and becomes enabled. However, clicking it will immediately return at line 330 because isAppInstalled.value is still false, providing no feedback to the user. The button should either remain disabled or trigger a different action (like manual retry of app detection) when in the timeout state. Consider changing the button's onClick to call checkAppStatus() again when pollingTimedOut is true instead of calling confirmRepo().
| public String fetchFileContent(String repositoryFullName, String filePath, String reference, String accessToken) { | ||
| RepositorySlug slug = RepositorySlug.from(repositoryFullName); | ||
| validateAccessToken(accessToken); | ||
|
|
||
| HttpHeaders headers = new HttpHeaders(); | ||
| headers.setBearerAuth(accessToken); | ||
| headers.setAccept(Collections.singletonList(MediaType.APPLICATION_JSON)); | ||
| headers.setBearerAuth(accessToken); | ||
|
|
||
| HttpEntity<Void> requestEntity = new HttpEntity<>(headers); |
There was a problem hiding this comment.
The validateAccessToken call has been removed from fetchFileContent. This method is now used with GitHub App installation tokens instead of OAuth tokens. However, the error message in validateAccessToken was updated to be more generic. Verify that all callers of fetchFileContent now pass valid tokens (either OAuth or GitHub App tokens), and that appropriate validation happens at the calling level.
| :href="isAppInstalled ? '#' : `https://github.com/apps/${githubAppName}/installations/new`" | ||
| target="_blank" | ||
| class="w-full py-3 font-black rounded-xl transition-all shadow-md flex items-center justify-center gap-2" | ||
| :class="isAppInstalled | ||
| ? 'bg-blue-600 text-white cursor-default pointer-events-none' | ||
| : 'bg-blue-600 hover:bg-blue-700 text-white hover:scale-[1.02] active:scale-[0.98]'" |
There was a problem hiding this comment.
The URL is constructed using template literals with githubAppName which comes from environment variables. If githubAppName is not properly validated and contains special characters or malicious content, it could lead to open redirect vulnerabilities. Consider validating the githubAppName value to ensure it only contains alphanumeric characters and hyphens, which are valid for GitHub App names.
| :href="isAppInstalled ? '#' : `https://github.com/apps/${githubAppName}/installations/new`" | |
| target="_blank" | |
| class="w-full py-3 font-black rounded-xl transition-all shadow-md flex items-center justify-center gap-2" | |
| :class="isAppInstalled | |
| ? 'bg-blue-600 text-white cursor-default pointer-events-none' | |
| : 'bg-blue-600 hover:bg-blue-700 text-white hover:scale-[1.02] active:scale-[0.98]'" | |
| :href="isAppInstalled || !githubAppName || !/^[A-Za-z0-9-]+$/.test(githubAppName) ? '#' : `https://github.com/apps/${githubAppName}/installations/new`" | |
| target="_blank" | |
| class="w-full py-3 font-black rounded-xl transition-all shadow-md flex items-center justify-center gap-2" | |
| :class="isAppInstalled | |
| ? 'bg-blue-600 text-white cursor-default pointer-events-none' | |
| : 'bg-blue-600 hover:bg-blue-700 text-white hover:scale-[1.02] active:scale-[0.98]'" |
| GITHUB_WEBHOOK_URL: ${GITHUB_WEBHOOK_URL} | ||
| YOUTUBE_API_KEY: ${YOUTUBE_API_KEY} | ||
| GITHUB_APP_ID: ${GITHUB_APP_ID} | ||
| GITHUB_APP_PRIVATE_KEY_PATH: file:/github-app.pem |
There was a problem hiding this comment.
The docker-compose.yml sets GITHUB_APP_PRIVATE_KEY_PATH to "file:/github-app.pem" which suggests a custom Spring ResourceLoader protocol. However, the application.properties uses "classpath:github-app.pem". These are inconsistent. The docker-compose.yml value will override application.properties, but "file:/github-app.pem" is not a standard Spring resource protocol (should be "file:///github-app.pem" for absolute path or "classpath:github-app.pem" for classpath). Verify that ResourceLoader can handle the "file:" protocol correctly or use a standard Spring resource location format.
| GITHUB_APP_PRIVATE_KEY_PATH: file:/github-app.pem | |
| GITHUB_APP_PRIVATE_KEY_PATH: classpath:github-app.pem |
| } catch (Exception e) { | ||
| log.warn("Failed to parse installation ID from raw payload for event: {}", event.getDeliveryId(), e); | ||
| } | ||
| } | ||
|
|
||
| UserOAuthToken token = oauthTokenService.requireValidAccessToken(userId); | ||
| return token.getAccessToken(); |
There was a problem hiding this comment.
The fallback logic falls back to OAuth token if installation ID parsing fails or if rawPayload is empty. However, with reduced OAuth scopes (only read:user,user:email), the OAuth token will no longer have permissions to read repository contents. This will cause failures when processing push events from users who haven't granted GitHub App permissions yet. Consider either removing the OAuth token fallback entirely, or handling the permission error gracefully with a clear message to the user.
| } catch (Exception e) { | |
| log.warn("Failed to parse installation ID from raw payload for event: {}", event.getDeliveryId(), e); | |
| } | |
| } | |
| UserOAuthToken token = oauthTokenService.requireValidAccessToken(userId); | |
| return token.getAccessToken(); | |
| log.warn("No installation ID found in raw payload for event: {}", event.getDeliveryId()); | |
| } catch (Exception e) { | |
| log.warn("Failed to parse installation ID from raw payload for event: {}", event.getDeliveryId(), e); | |
| } | |
| } else { | |
| log.warn("Raw payload is empty for event: {}", event.getDeliveryId()); | |
| } | |
| throw new GitHubWebhookException("GitHub App 권한이 필요합니다. 저장소에 Dash GitHub App을 설치하고 권한을 부여한 후 다시 시도해주세요."); |
| private PrivateKey loadPrivateKey() throws IOException, NoSuchAlgorithmException, InvalidKeySpecException { | ||
| String privateKeyContent = loadPrivateKeyContent(); | ||
|
|
||
| // PEM 헤더/푸터 제거 및 줄바꿈 제거 | ||
| String privateKeyPEM = privateKeyContent | ||
| .replace("-----BEGIN RSA PRIVATE KEY-----", "") | ||
| .replace("-----END RSA PRIVATE KEY-----", "") | ||
| .replace("-----BEGIN PRIVATE KEY-----", "") | ||
| .replace("-----END PRIVATE KEY-----", "") | ||
| .replaceAll("\\s", ""); | ||
|
|
||
| byte[] encoded = Base64.getDecoder().decode(privateKeyPEM); | ||
| KeyFactory keyFactory = KeyFactory.getInstance("RSA"); | ||
| return keyFactory.generatePrivate(new PKCS8EncodedKeySpec(encoded)); | ||
| } |
There was a problem hiding this comment.
The private key loading logic only supports PKCS#8 format, but the PR description mentions "PKCS#1/8" support. The code replaces headers for both PKCS#1 ("BEGIN RSA PRIVATE KEY") and PKCS#8 ("BEGIN PRIVATE KEY"), but only uses PKCS8EncodedKeySpec for parsing. PKCS#1 keys require conversion to PKCS#8 format before they can be parsed with PKCS8EncodedKeySpec. This will cause a failure at runtime if a PKCS#1 key is provided. Either remove PKCS#1 header handling or implement proper PKCS#1 to PKCS#8 conversion using BouncyCastle or similar library.
|
|
||
| // GitHub App 미설치/권한 부족 케이스 특별 처리 | ||
| if (errorMsg.includes("GitHub App is not installed")) { | ||
| const githubAppName = import.meta.env.VITE_GITHUB_APP_NAME; |
There was a problem hiding this comment.
The code uses environment variable VITE_GITHUB_APP_NAME but this variable is not defined in the docker-compose.yml or any configuration file. If this variable is undefined, the URL will be constructed as "https://github.com/apps/undefined/installations/new" which will result in a broken link. Either add this environment variable to docker-compose.yml and application configuration, or hardcode the GitHub App name if it's fixed.
| const githubAppName = import.meta.env.VITE_GITHUB_APP_NAME; | |
| const githubAppName = import.meta.env.VITE_GITHUB_APP_NAME; | |
| if (!githubAppName) { | |
| alert("GitHub App 설정 정보가 누락되어 있어 설치 페이지로 이동할 수 없습니다.\n관리자에게 문의해주세요."); | |
| return; | |
| } |
| console.log('Found installation_id, forcing Step 5 (Repo)'); | ||
| currentStepIndex.value = 4; // repo 단계로 강제 이동 | ||
| return; |
There was a problem hiding this comment.
After a GitHub App installation, GitHub redirects back with an installation_id query parameter. However, the code forces the user to Step 5 (repo) without considering whether the repository has already been detected. If the user completes installation but returns via GitHub's redirect, they'll be forced back to Step 5 even if they've already completed it. Consider checking if the repository is already configured before forcing the user to Step 5, or clearing the query parameter after processing to prevent loops.
| console.log('Found installation_id, forcing Step 5 (Repo)'); | |
| currentStepIndex.value = 4; // repo 단계로 강제 이동 | |
| return; | |
| console.log('Found installation_id, handling GitHub App callback'); | |
| // installation_id 파라미터는 한 번 처리 후 제거하여 루프를 방지합니다. | |
| const { installation_id, ...restQuery } = route.query; | |
| if (installation_id) { | |
| router.replace({ path: route.path, query: restQuery }); | |
| } | |
| // 리포지토리가 아직 설정되지 않은 경우에만 리포지토리 단계(4)로 강제 이동합니다. | |
| if (!hasRepo) { | |
| currentStepIndex.value = 4; // repo 단계로 강제 이동 | |
| return; | |
| } | |
| // 이미 리포지토리가 설정된 경우에는 일반 온보딩 로직을 계속 진행합니다. |
| spring.security.oauth2.client.registration.github.client-id=${GITHUB_CLIENT_ID} | ||
| spring.security.oauth2.client.registration.github.client-secret=${GITHUB_CLIENT_SECRET} | ||
| spring.security.oauth2.client.registration.github.scope=read:user,user:email,repo,admin:repo_hook | ||
| spring.security.oauth2.client.registration.github.scope=read:user,user:email |
There was a problem hiding this comment.
OAuth scopes have been reduced from "read:user,user:email,repo,admin:repo_hook" to just "read:user,user:email". While this aligns with the GitHub App migration, if any existing code still relies on the OAuth token to access repository content or manage webhooks, those operations will fail. The PR description mentions migrating to GitHub App tokens, but verify that all OAuth token usage has been migrated to use GitHub App tokens instead.
| spring.security.oauth2.client.registration.github.scope=read:user,user:email | |
| spring.security.oauth2.client.registration.github.scope=read:user,user:email,repo,admin:repo_hook |
🚀 작업 배경
기존 OAuth App의 과도한 권한 요구 문제를 해결하고, 필요한 권한만 세밀하게 제어하기 위해 GitHub App으로 마이그레이션했습니다.
🛠️ 주요 변경 사항
🔗 관련 이슈