fix(devin): use loopback callback endpoint for oauth redirect uri

- Enforce `http://127.0.0.1:<port>/callback` redirect URI to satisfy Devin authorization validation requirements. - Register `/callback` route while preserving `/devin/callback` handler.

Luis Pater committed 2 months ago

Commit cca35ae1 parent 8c5f6e15 files · +33 −15Browse files

Unified · Split

5 files

docs/management-devin-oauth.md +1 −1
internal/api/handlers/management/auth_files_devin_oauth.go +11 −1
internal/api/handlers/management/auth_files_devin_oauth_test.go +1 −1
internal/api/server_devin_oauth_test.go +15 −10
internal/api/server_routes.go +5 −2

docs/management-devin-oauth.md+1−1

@@ -18,7 +18,7 @@ Devin uses the existing management OAuth callback/status contract. No session to · 17 unmodified lines
18	18	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.
19	19	5. Cancel a pending attempt with `DELETE /v0/management/oauth-session?state=...`.
20	20
21		−The generated redirect uses the server's configured port and TLS mode at `127.0.0.1:<port>/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.
	21	+The generated redirect uses the server's configured port at `http://127.0.0.1:<port>/callback` (Devin's authorization page strictly validates that the redirect URI is `http://127.0.0.1:<port>/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.
22	22
23	23	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.
24	24

internal/api/handlers/management/auth_files_devin_oauth.go+11−1

@@ -31,9 +31,19 @@ var newDevinOAuthService = func(cfg *config.Config) devinOAuthService { · 30 unmodified lines
31	31	return devin.NewDevinAuthService(client)
32	32	}
33	33
	34	+// devinCallbackURL builds the required loopback callback URL for Devin OAuth.
	35	+// Devin's authorization page strictly validates that redirect_uri matches
	36	+// http://127.0.0.1:<port>/callback (http protocol, 127.0.0.1 host, and /callback path).
	37	+func (h *Handler) devinCallbackURL() (string, error) {
	38	+ if h == nil \|\| h.cfg == nil \|\| h.cfg.Port <= 0 {
	39	+ return "", fmt.Errorf("server port is not configured")
	40	+ }
	41	+ return fmt.Sprintf("http://127.0.0.1:%d/callback", h.cfg.Port), nil
	42	+}
	43	+
34	44	// RequestDevinToken starts the same callback/status flow used by the other WebUI providers.
35	45	func (h Handler) RequestDevinToken(c gin.Context) {
36		− redirectURI, errRedirect := h.managementCallbackURL("/devin/callback")
	46	+ redirectURI, errRedirect := h.devinCallbackURL()
37	47	if errRedirect != nil {
38	48	c.JSON(http.StatusInternalServerError, gin.H{"error": "callback server unavailable"})
39	49	return

internal/api/handlers/management/auth_files_devin_oauth_test.go+1−1

@@ -90,7 +90,7 @@ func TestDevinRemoteOAuthFlow(t *testing.T) { · 89 unmodified lines
90	90	if start.Status != "ok" \|\| start.State == "" \|\| query.Get("state") != start.State \|\| query.Get("code_challenge_method") != "S256" {
91	91	t.Fatalf("invalid authorization response: %s", w.Body.String())
92	92	}
93		− if got := query.Get("redirect_uri"); got != "http://127.0.0.1:8317/devin/callback" {
	93	+ if got := query.Get("redirect_uri"); got != "http://127.0.0.1:8317/callback" {
94	94	t.Fatalf("redirect_uri = %q", got)
95	95	}
96	96	if query.Get("code_verifier") != "" {

internal/api/server_devin_oauth_test.go+15−10

@@ -32,14 +32,15 @@ func TestDevinOAuthRoutes(t *testing.T) { · 31 unmodified lines
32	32	}
33	33
34	34	for _, test := range []struct {
35		− name, provider, query string
36		− want int
	35	+ name, provider, query, callbackPath string
	36	+ want int
37	37	}{
38		− {name: "success", provider: "devin", query: "code=test-code", want: http.StatusOK},
39		− {name: "denied", provider: "devin", query: "error=access_denied", want: http.StatusOK},
40		− {name: "wrong provider", provider: "codex", query: "code=test-code", want: http.StatusBadRequest},
41		− {name: "missing code", provider: "devin", want: http.StatusBadRequest},
42		− {name: "unknown state", query: "code=test-code", want: http.StatusBadRequest},
	38	+ {name: "success", provider: "devin", query: "code=test-code", callbackPath: "/callback", want: http.StatusOK},
	39	+ {name: "success legacy path", provider: "devin", query: "code=test-code", callbackPath: "/devin/callback", want: http.StatusOK},
	40	+ {name: "denied", provider: "devin", query: "error=access_denied", callbackPath: "/callback", want: http.StatusOK},
	41	+ {name: "wrong provider", provider: "codex", query: "code=test-code", callbackPath: "/callback", want: http.StatusBadRequest},
	42	+ {name: "missing code", provider: "devin", callbackPath: "/callback", want: http.StatusBadRequest},
	43	+ {name: "unknown state", query: "code=test-code", callbackPath: "/callback", want: http.StatusBadRequest},
43	44	} {
44	45	t.Run(test.name, func(t *testing.T) {
45	46	state := "devin-route-" + strings.ReplaceAll(test.name, " ", "-")
@@ -48,12 +49,16 @@ func TestDevinOAuthRoutes(t *testing.T) { · 2 unmodified lines
48	49	defer management.CompleteOAuthSession(state)
49	50	}
50	51	w := httptest.NewRecorder()
51		− server.engine.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/devin/callback?state="+state+"&"+test.query, nil))
	52	+ path := test.callbackPath
	53	+ if path == "" {
	54	+ path = "/callback"
	55	+ }
	56	+ server.engine.ServeHTTP(w, httptest.NewRequest(http.MethodGet, path+"?state="+state+"&"+test.query, nil))
52	57	if w.Code != test.want {
53	58	t.Fatalf("callback: %d %s", w.Code, w.Body.String())
54	59	}
55		− path := filepath.Join(server.cfg.AuthDir, ".oauth-devin-"+state+".oauth")
56		− data, errRead := os.ReadFile(path)
	60	+ filePath := filepath.Join(server.cfg.AuthDir, ".oauth-devin-"+state+".oauth")
	61	+ data, errRead := os.ReadFile(filePath)
57	62	if test.want == http.StatusOK {
58	63	if errRead != nil {
59	64	t.Fatal(errRead)

internal/api/server_routes.go+5−2

@@ -184,7 +184,7 @@ func (s *Server) setupRoutes() { · 183 unmodified lines
184	184	c.String(http.StatusOK, oauthCallbackSuccessHTML)
185	185	})
186	186
187		− s.engine.GET("/devin/callback", func(c *gin.Context) {
	187	+ devinCallbackHandler := func(c *gin.Context) {
188	188	c.Header("Cache-Control", "no-store")
189	189	code := strings.TrimSpace(c.Query("code"))
190	190	state := strings.TrimSpace(c.Query("state"))
@@ -202,7 +202,10 @@ func (s *Server) setupRoutes() { · 11 unmodified lines
202	202	}
203	203	c.Header("Content-Type", "text/html; charset=utf-8")
204	204	c.String(http.StatusOK, oauthCallbackSuccessHTML)
205		− })
	205	+ }
	206	+
	207	+ s.engine.GET("/callback", devinCallbackHandler)
	208	+ s.engine.GET("/devin/callback", devinCallbackHandler)
206	209
207	210	// Management routes are registered lazily by registerManagementRoutes when a secret is configured.
208	211	}