diff --git a/docs/management-devin-oauth.md b/docs/management-devin-oauth.md index d26a7b810..1775694bb 100644 --- a/docs/management-devin-oauth.md +++ b/docs/management-devin-oauth.md @@ -18,7 +18,7 @@ Devin uses the existing management OAuth callback/status contract. No session to 4. Poll `GET /v0/management/get-auth-status?state=...` with the management key. `status` is `wait`, `ok` after credentials are saved, or `error` with an `error` message. A successful callback submission alone does not mean token exchange has completed. 5. Cancel a pending attempt with `DELETE /v0/management/oauth-session?state=...`. -The generated redirect uses the server's configured port and TLS mode at `127.0.0.1:/devin/callback`. When the browser can reach that loopback server (local deployment or an appropriate tunnel), the public callback route accepts the redirect automatically. Remote deployments do not require opening an extra callback port: use step 3 instead. This is a browser authorization-code flow, not a device-code flow. +The generated redirect uses the server's configured port at `http://127.0.0.1:/callback` (Devin's authorization page strictly validates that the redirect URI is `http://127.0.0.1:/callback`). When the browser can reach that loopback server (local deployment or an appropriate tunnel), the public callback route accepts the redirect automatically. Remote deployments do not require opening an extra callback port: use step 3 instead. This is a browser authorization-code flow, not a device-code flow. Authorization must finish within five minutes. Credentials use the same `devin-*.json` format as `--devin-login`, including normalized session tokens, OAuth auth kind, and best-effort profile/plan metadata. Account/quota enrichment failures do not invalidate an otherwise valid session token. diff --git a/internal/api/handlers/management/auth_files_devin_oauth.go b/internal/api/handlers/management/auth_files_devin_oauth.go index 275858ce7..f58584525 100644 --- a/internal/api/handlers/management/auth_files_devin_oauth.go +++ b/internal/api/handlers/management/auth_files_devin_oauth.go @@ -31,9 +31,19 @@ var newDevinOAuthService = func(cfg *config.Config) devinOAuthService { return devin.NewDevinAuthService(client) } +// devinCallbackURL builds the required loopback callback URL for Devin OAuth. +// Devin's authorization page strictly validates that redirect_uri matches +// http://127.0.0.1:/callback (http protocol, 127.0.0.1 host, and /callback path). +func (h *Handler) devinCallbackURL() (string, error) { + if h == nil || h.cfg == nil || h.cfg.Port <= 0 { + return "", fmt.Errorf("server port is not configured") + } + return fmt.Sprintf("http://127.0.0.1:%d/callback", h.cfg.Port), nil +} + // RequestDevinToken starts the same callback/status flow used by the other WebUI providers. func (h *Handler) RequestDevinToken(c *gin.Context) { - redirectURI, errRedirect := h.managementCallbackURL("/devin/callback") + redirectURI, errRedirect := h.devinCallbackURL() if errRedirect != nil { c.JSON(http.StatusInternalServerError, gin.H{"error": "callback server unavailable"}) return diff --git a/internal/api/handlers/management/auth_files_devin_oauth_test.go b/internal/api/handlers/management/auth_files_devin_oauth_test.go index ff6889e71..9ca05e26a 100644 --- a/internal/api/handlers/management/auth_files_devin_oauth_test.go +++ b/internal/api/handlers/management/auth_files_devin_oauth_test.go @@ -90,7 +90,7 @@ func TestDevinRemoteOAuthFlow(t *testing.T) { if start.Status != "ok" || start.State == "" || query.Get("state") != start.State || query.Get("code_challenge_method") != "S256" { t.Fatalf("invalid authorization response: %s", w.Body.String()) } - if got := query.Get("redirect_uri"); got != "http://127.0.0.1:8317/devin/callback" { + if got := query.Get("redirect_uri"); got != "http://127.0.0.1:8317/callback" { t.Fatalf("redirect_uri = %q", got) } if query.Get("code_verifier") != "" { diff --git a/internal/api/server_devin_oauth_test.go b/internal/api/server_devin_oauth_test.go index 6a2d7e3ce..f2f5290fa 100644 --- a/internal/api/server_devin_oauth_test.go +++ b/internal/api/server_devin_oauth_test.go @@ -32,14 +32,15 @@ func TestDevinOAuthRoutes(t *testing.T) { } for _, test := range []struct { - name, provider, query string - want int + name, provider, query, callbackPath string + want int }{ - {name: "success", provider: "devin", query: "code=test-code", want: http.StatusOK}, - {name: "denied", provider: "devin", query: "error=access_denied", want: http.StatusOK}, - {name: "wrong provider", provider: "codex", query: "code=test-code", want: http.StatusBadRequest}, - {name: "missing code", provider: "devin", want: http.StatusBadRequest}, - {name: "unknown state", query: "code=test-code", want: http.StatusBadRequest}, + {name: "success", provider: "devin", query: "code=test-code", callbackPath: "/callback", want: http.StatusOK}, + {name: "success legacy path", provider: "devin", query: "code=test-code", callbackPath: "/devin/callback", want: http.StatusOK}, + {name: "denied", provider: "devin", query: "error=access_denied", callbackPath: "/callback", want: http.StatusOK}, + {name: "wrong provider", provider: "codex", query: "code=test-code", callbackPath: "/callback", want: http.StatusBadRequest}, + {name: "missing code", provider: "devin", callbackPath: "/callback", want: http.StatusBadRequest}, + {name: "unknown state", query: "code=test-code", callbackPath: "/callback", want: http.StatusBadRequest}, } { t.Run(test.name, func(t *testing.T) { state := "devin-route-" + strings.ReplaceAll(test.name, " ", "-") @@ -48,12 +49,16 @@ func TestDevinOAuthRoutes(t *testing.T) { defer management.CompleteOAuthSession(state) } w := httptest.NewRecorder() - server.engine.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/devin/callback?state="+state+"&"+test.query, nil)) + path := test.callbackPath + if path == "" { + path = "/callback" + } + server.engine.ServeHTTP(w, httptest.NewRequest(http.MethodGet, path+"?state="+state+"&"+test.query, nil)) if w.Code != test.want { t.Fatalf("callback: %d %s", w.Code, w.Body.String()) } - path := filepath.Join(server.cfg.AuthDir, ".oauth-devin-"+state+".oauth") - data, errRead := os.ReadFile(path) + filePath := filepath.Join(server.cfg.AuthDir, ".oauth-devin-"+state+".oauth") + data, errRead := os.ReadFile(filePath) if test.want == http.StatusOK { if errRead != nil { t.Fatal(errRead) diff --git a/internal/api/server_routes.go b/internal/api/server_routes.go index 214e87395..1827bd530 100644 --- a/internal/api/server_routes.go +++ b/internal/api/server_routes.go @@ -184,7 +184,7 @@ func (s *Server) setupRoutes() { c.String(http.StatusOK, oauthCallbackSuccessHTML) }) - s.engine.GET("/devin/callback", func(c *gin.Context) { + devinCallbackHandler := func(c *gin.Context) { c.Header("Cache-Control", "no-store") code := strings.TrimSpace(c.Query("code")) state := strings.TrimSpace(c.Query("state")) @@ -202,7 +202,10 @@ func (s *Server) setupRoutes() { } c.Header("Content-Type", "text/html; charset=utf-8") c.String(http.StatusOK, oauthCallbackSuccessHTML) - }) + } + + s.engine.GET("/callback", devinCallbackHandler) + s.engine.GET("/devin/callback", devinCallbackHandler) // Management routes are registered lazily by registerManagementRoutes when a secret is configured. }