Add Dex authentication configuration to support simple auth - #6
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @BetterAndBetterII, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request significantly enhances the application's authentication capabilities by introducing OpenID Connect (OIDC) support through Dex. The primary purpose is to provide more flexible and generalized authentication options beyond the previously sole GitHub OAuth. This involves a comprehensive update across the codebase, from backend user model changes and new OIDC handlers to frontend UI adjustments and new Docker Compose configurations, ultimately allowing users to authenticate via various identity sources federated through Dex.
Highlights
- OIDC Authentication Integration: This pull request integrates Dex as an OpenID Connect (OIDC) identity provider, offering a flexible authentication alternative to the existing GitHub OAuth. This allows for broader authentication options, including simple username/password setups via Dex's static password feature.
- Generalized User Model: The user data model in both the Go backend and the React/TypeScript frontend has been generalized. User identification now relies on a
Subjectstring instead of a GitHub-specific numeric ID, making the system more adaptable to various identity providers. - New Docker Compose Configuration for Dex: A new
docker-compose.dex.ymlfile has been added, providing a streamlined way to deploy the Excalidraw application alongside a Dex identity provider. This simplifies the setup process for developers and users wanting to leverage OIDC. - Frontend Adaptation for OIDC: The Excalidraw frontend has been updated to utilize the new OIDC login endpoints and correctly process user information from OIDC-compliant JWTs. UI elements related to login have also been adjusted for a more generic 'Login' experience.
- Backend OIDC Handlers: New Go handlers (
handlers/auth/dex.go) have been implemented to manage the OIDC authentication flow, including initiating login, handling callbacks, verifying ID tokens, and creating application-specific JWTs based on OIDC claims.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| return | ||
| } | ||
|
|
||
| url := oidcOauthConfig.AuthCodeURL("random", oauth2.AccessTypeOffline) |
Check failure
Code scanning / CodeQL
Use of constant `state` value in OAuth 2.0 URL
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 year ago
To fix the problem, we need to generate a unique, cryptographically secure random state value for each authentication request in HandleOIDCLogin. This value should be stored in a way that it can be validated in the callback handler (HandleOIDCCallback). The most common approach is to generate the state, store it in a secure cookie (or session), and then compare the returned state in the callback to the stored value.
Steps:
- Add a helper function to generate a secure random state string.
- In
HandleOIDCLogin, generate a new state value, store it in a cookie, and use it in the call toAuthCodeURL. - In
HandleOIDCCallback, retrieve the state from the cookie and compare it to thestateparameter returned by the OAuth provider. If they do not match, reject the request.
Required changes:
- Add imports for
crypto/rand,encoding/base64, and possiblyerrorsif needed. - Add a helper function to generate the state and set it as a cookie.
- Update
HandleOIDCLoginto use the generated state. - Update
HandleOIDCCallbackto validate the state.
| @@ -2,6 +2,8 @@ | ||
|
|
||
| import ( | ||
| "context" | ||
| "crypto/rand" | ||
| "encoding/base64" | ||
| "excalidraw-complete/core" | ||
| "fmt" | ||
| "net/http" | ||
| @@ -62,13 +64,40 @@ | ||
| }) | ||
| } | ||
|
|
||
| // generateStateOauthCookie generates a random state string, sets it as a cookie, and returns it. | ||
| func generateStateOauthCookie(w http.ResponseWriter) (string, error) { | ||
| b := make([]byte, 32) | ||
| if _, err := rand.Read(b); err != nil { | ||
| return "", err | ||
| } | ||
| state := base64.URLEncoding.EncodeToString(b) | ||
| // Set the state in a secure, HTTP-only cookie | ||
| http.SetCookie(w, &http.Cookie{ | ||
| Name: "oidc_state", | ||
| Value: state, | ||
| Path: "/", | ||
| HttpOnly: true, | ||
| Secure: true, | ||
| SameSite: http.SameSiteLaxMode, | ||
| MaxAge: 300, // 5 minutes | ||
| }) | ||
| return state, nil | ||
| } | ||
|
|
||
| func HandleOIDCLogin(w http.ResponseWriter, r *http.Request) { | ||
| if oidcOauthConfig == nil { | ||
| http.Error(w, "OIDC is not configured", http.StatusInternalServerError) | ||
| return | ||
| } | ||
|
|
||
| url := oidcOauthConfig.AuthCodeURL("random", oauth2.AccessTypeOffline) | ||
| state, err := generateStateOauthCookie(w) | ||
| if err != nil { | ||
| logrus.Errorf("Failed to generate OIDC state: %v", err) | ||
| http.Error(w, "Internal server error", http.StatusInternalServerError) | ||
| return | ||
| } | ||
|
|
||
| url := oidcOauthConfig.AuthCodeURL(state, oauth2.AccessTypeOffline) | ||
| http.Redirect(w, r, url, http.StatusTemporaryRedirect) | ||
| } | ||
|
|
||
| @@ -78,6 +101,15 @@ | ||
| return | ||
| } | ||
|
|
||
| // Validate state to prevent CSRF | ||
| stateFromQuery := r.FormValue("state") | ||
| stateCookie, err := r.Cookie("oidc_state") | ||
| if err != nil || stateFromQuery == "" || stateCookie.Value != stateFromQuery { | ||
| logrus.Error("Invalid OIDC state in callback") | ||
| http.Error(w, "Invalid state parameter", http.StatusBadRequest) | ||
| return | ||
| } | ||
|
|
||
| code := r.FormValue("code") | ||
| if code == "" { | ||
| logrus.Error("no code in callback") |
There was a problem hiding this comment.
Code Review
This pull request adds Dex authentication support, which is a great step towards more flexible authentication. The changes are comprehensive, touching the backend, frontend, and configuration. I've identified a few critical security and configuration issues that need to be addressed before merging. These include a CSRF vulnerability in the OIDC flow, insecure default configurations for Dex, and a networking issue in the Docker Compose setup. There's also a critical bug in the frontend code that will cause a compilation error. Addressing these points will significantly improve the security and robustness of the new authentication feature.
…ex 配置以使用环境变量。此更改增强了 OIDC 认证的灵活性和安全性。
…ie 中存储，验证状态以提高安全性。此更改提升了 OIDC 认证的安全性和灵活性。
…维护性。更新文档以反映新的认证流程和环境变量设置。
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces Dex OIDC authentication as an alternative to GitHub OAuth, which is a great feature for self-hosting. The changes are mostly well-structured, centralizing authentication logic. However, I've found several issues, including two critical security vulnerabilities related to missing CSRF protection in the OAuth callback handlers. There are also some bugs in configuration files and logic for loading frontend configuration. I've provided suggestions to fix these issues.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…确引用。此更改提高了文档的准确性和可用性。
2 participants
Add Dex authentication configuration, and update docker-compose configuration to support OIDC authentication
issue: #4