Remove unused tasks in Github workflows#253
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR aims to remove unused tasks in GitHub workflows but includes substantial authentication improvements. The key changes involve adding Keycloak logout functionality, improving OAuth error handling with redirects instead of JSON responses, and fixing context handling in the user repository.
- Add Keycloak session invalidation to the logout flow
- Replace JSON error responses with user-friendly redirects to the frontend
- Fix context cancellation issue in user repository upsert operation
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/template-docker.yml |
Comments out QEMU setup step in Docker workflow |
services/llm-api/internal/interfaces/httpserver/handlers/authhandler/token_handler.go |
Adds Keycloak logout endpoint call before clearing local cookies |
services/llm-api/internal/interfaces/httpserver/handlers/authhandler/keycloak_oauth.go |
Converts error JSON responses to homepage redirects with error parameters |
services/llm-api/internal/infrastructure/keycloak/client.go |
Implements new LogoutUser method to invalidate Keycloak sessions |
services/llm-api/internal/infrastructure/database/repository/userrepo/user_repository.go |
Uses background context for user reload to prevent cancellation issues |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| homeURL := "https://chat-dev.jan.ai/" | ||
| redirectURL := fmt.Sprintf("%s?error=%s&error_description=%s", | ||
| homeURL, | ||
| url.QueryEscape(keycloakError), | ||
| url.QueryEscape(errorDescription)) |
There was a problem hiding this comment.
The hardcoded URL 'https://chat-dev.jan.ai/' is duplicated in 6 locations throughout this file (lines 246, 269, 286, 300, 328, 378). Consider extracting this to a constant or configuration value to improve maintainability and make environment-specific deployments easier.
| }) | ||
|
|
||
| // Redirect to homepage with error in URL | ||
| homeURL := "https://chat-dev.jan.ai/" |
There was a problem hiding this comment.
Hardcoded environment-specific URL ('chat-dev.jan.ai') should be externalized to a configuration file or environment variable. This will prevent the need to modify code when deploying to different environments (dev, staging, production).
| homeURL := "https://chat-dev.jan.ai/" | |
| homeURL := config.GetConfig().HomeURL |
| } | ||
| defer resp.Body.Close() | ||
|
|
||
| if resp.StatusCode >= 300 { |
There was a problem hiding this comment.
The status code check >= 300 will treat HTTP 3xx redirect responses as errors. Keycloak logout endpoint typically returns 204 No Content on success. Consider checking for resp.StatusCode < 200 || resp.StatusCode >= 300 or specifically resp.StatusCode != 204 if that's the expected success response.
| if resp.StatusCode >= 300 { | |
| if resp.StatusCode != http.StatusNoContent { |
Remove unused tasks in Github workflows